Re: [PATCH v3 09/27] KVM: VMX: Do not use MAX_POSSIBLE_PASSTHROUGH_MSRS in array definition

[Xin Li]

On 11/26/2024 10:02 AM, Borislav Petkov wrote:
> On Mon, Sep 30, 2024 at 10:00:52PM -0700, Xin Li (Intel) wrote:
> > No need to use MAX_POSSIBLE_PASSTHROUGH_MSRS in the definition of array
> > vmx_possible_passthrough_msrs, as the macro name indicates the _possible_
> > maximum size of passthrough MSRs.
> >
> > Use ARRAY_SIZE instead of MAX_POSSIBLE_PASSTHROUGH_MSRS when the size of
> > the array is needed and add a BUILD_BUG_ON to make sure the actual array
> > size does not exceed the possible maximum size of passthrough MSRs.
>
> This commit message needs to talk about the why - not the what. Latter should
> be visible from the diff itself.

I should not write such a changelog...

> What you're not talking about is the sneaked increase of
> MAX_POSSIBLE_PASSTHROUGH_MSRS to 64. Something you *should* mention because
> the array is full and blablabla...

It's still far from full in a bitmap on x86-64, but just that the
existing use of MAX_POSSIBLE_PASSTHROUGH_MSRS tastes bad.


A better one?

Per the definition, a bitmap on x86-64 is an array of 'unsigned long',
and is at least 64-bit long.

#define DECLARE_BITMAP(name,bits) \
	unsigned long name[BITS_TO_LONGS(bits)]

It's not accurate and error-prone to use a hard-coded possible size of
a bitmap, Use ARRAY_SIZE with an overflow build check instead.

> > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h
> > index e0d76d2460ef..e7409f8f28b1 100644
> > --- a/arch/x86/kvm/vmx/vmx.h
> > +++ b/arch/x86/kvm/vmx/vmx.h
> > @@ -356,7 +356,7 @@ struct vcpu_vmx {
> >   	struct lbr_desc lbr_desc;
> >   
> >   	/* Save desired MSR intercept (read: pass-through) state */
> > -#define MAX_POSSIBLE_PASSTHROUGH_MSRS	16
> > +#define MAX_POSSIBLE_PASSTHROUGH_MSRS	64
> 						^^^