Re: [Xen-devel] [PATCH 4/6] x86/msr: Clean up the MSR_FEATURE_CONTROL constants

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 26.06.18 at 15:18, <andrew.cooper3@xxxxxxxxxx> wrote:
> --- a/xen/include/asm-x86/msr-index.h
> +++ b/xen/include/asm-x86/msr-index.h
> @@ -15,6 +15,13 @@
>   * abbreviated name.
>   */
>  
> +#define MSR_FEATURE_CONTROL             0x0000003a
> +#define FEAT_CTL_LOCK                   (_AC(1, ULL) <<  0)
> +#define FEAT_CTL_VMX_INSIDE_SMX         (_AC(1, ULL) <<  1)
> +#define FEAT_CTL_VMX_OUTSIDE_SMX        (_AC(1, ULL) <<  2)
> +#define FEAT_CTL_SGX                    (_AC(1, ULL) << 18)
> +#define FEAT_CTL_LMCE                   (_AC(1, ULL) << 20)

So this is a good example a case where I'd be rather afraid of possible
name clashes. I fully agree ...

> @@ -321,15 +328,6 @@
>  #define MSR_IA32_EBL_CR_POWERON              0x0000002a
>  #define MSR_IA32_EBC_FREQUENCY_ID    0x0000002c
>  
> -#define MSR_IA32_FEATURE_CONTROL     0x0000003a
> -#define IA32_FEATURE_CONTROL_LOCK                     0x0001
> -#define IA32_FEATURE_CONTROL_ENABLE_VMXON_INSIDE_SMX  0x0002
> -#define IA32_FEATURE_CONTROL_ENABLE_VMXON_OUTSIDE_SMX 0x0004

... that especially these two are excessively long. But omitting the
IA32 infix plus shortening FEATURE_CONTROL to FEAT_CTL is not
helpful. The latter even is against the naming rules set forth in
patch 2.

I'd be fine with deviating from the SDM here, using MSR_IA32_FEAT_CTL
and IA32_FEAT_CTL_* (perhaps with a comment citing the SDM name,
to make it noticable to someone grep-ing).

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel