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

[Xin Li]

On 11/26/2024 12:06 PM, Borislav Petkov wrote:
> On Tue, Nov 26, 2024 at 11:22:45AM -0800, Xin Li wrote:
> > 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.
>
> Far from full?
>
> It is full:
>
> static u32 vmx_possible_passthrough_msrs[MAX_POSSIBLE_PASSTHROUGH_MSRS] = {
>          MSR_IA32_SPEC_CTRL,
>          MSR_IA32_PRED_CMD,
>          MSR_IA32_FLUSH_CMD,
>          MSR_IA32_TSC,
> #ifdef CONFIG_X86_64
>          MSR_FS_BASE,
>          MSR_GS_BASE,
>          MSR_KERNEL_GS_BASE,
>          MSR_IA32_XFD,
>          MSR_IA32_XFD_ERR,
> #endif
>          MSR_IA32_SYSENTER_CS,
>          MSR_IA32_SYSENTER_ESP,
>          MSR_IA32_SYSENTER_EIP,
>          MSR_CORE_C1_RES,
>          MSR_CORE_C3_RESIDENCY,
>          MSR_CORE_C6_RESIDENCY,
>          MSR_CORE_C7_RESIDENCY,
> };
>
> I count 16 here.
>
> If you need to add more, you need to increment MAX_POSSIBLE_PASSTHROUGH_MSRS.

Yes, the most obvious approach is to simply increase
MAX_POSSIBLE_PASSTHROUGH_MSRS by the number of MSRs to be added into the 
array.

However I hate to count it myself, especially we have ARRAY_SIZE.

> > A better one?
>
> Not really.
>
> You're not explaining why MAX_POSSIBLE_PASSTHROUGH_MSRS becomes 64.
>
> > 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.
>
> It becomes 64 because a bitmap has 64 bits?

Yes, maybe better to name the macro as MAX_ALLOWED_PASSTHROUGH_MSRS?

> Not because you need to add more MSRs to it and thus raise the limit?

Right.  It triggered me to look at the code further, though, I think the
existing code could be written in a better way no matter whether I need
to add more MSRs.  And whoever wants to add more won't need to increase
MAX_POSSIBLE_PASSTHROUGH_MSRS (ofc unless overflow 64).

Thanks!
    Xin