Re: [PATCH v3 01/19] x86/msr-index: Clean up bit defines for IA32_FEATURE_CONTROL MSR

Sean Christopherson
2019-11-19T23:18:28Z

On Tue, Nov 19, 2019 at 12:15:08PM +0100, Borislav Petkov wrote:
> On Mon, Nov 18, 2019 at 07:12:22PM -0800, Sean Christopherson wrote:
> > As pointed out by Boris, the defines for bits in IA32_FEATURE_CONTROL
> > are quite a mouthful, especially the VMX bits which must differentiate
> > between enabling VMX inside and outside SMX (TXT) operation. Rename the
> > bit defines to abbreviate FEATURE_CONTROL as FEAT_CTL so that they're a
> > little friendlier on the eyes. Keep the full name for the MSR itself to
> > help even the most obtuse reader decipher the abbreviation, and to match
> > the name used by the Intel SDM.
> >
> > Opportunistically fix a few other annoyances with the defines:
> >
> > - Relocate the bit defines so that they immediately follow the MSR
> > define, e.g. aren't mistaken as belonging to MISC_FEATURE_CONTROL.
> > - Add whitespace around the block of feature control defines to make
> > it clear that FEAT_CTL is indeed short for FEATURE_CONTROL.
> > - Use BIT() instead of manually encoding the bit shift.
> > - Use "VMX" instead of "VMXON" to match the SDM.
> > - Append "_ENABLED" to the LMCE bit to be consistent with the verbiage
> > used for all other feature control bits. (LCME is an acronym for
> > Local Machine Check Exception, i.e. LMCE_ENABLED is not redundant).
>
> Sure but SDM calls it LMCE_ON. What is our current decision on sticking
> to SDM bit names? I guess we don't...
>
> But above you say "to match the SDM"...

Ugh. Match the SDM unless it's obviously "wrong"? :-) It might literally
be the only instance of the SDM using "on" instead of "enable(d)" for an
MSR or CR bit. The SDM even refers to it as an enable bit, e.g. "platform
software has not enabled LMCE by setting IA32_FEATURE_CONTROL.LMCE_ON (bit 20)".

Whining aside, I'm ok going with LMCE_ON, I have a feeling "on" was
deliberately chosen differentiate it from IA32_MCG_EXT_CTL.LMCE_EN.