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

["Tian, Kevin" <kevin.tian@xxxxxxxxx>]

> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: Thursday, June 28, 2018 9:11 PM
> 
> >>> 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).
> 

Agree. FEAT_CTL alone is too broad...

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