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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 28.06.18 at 15:36, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 28/06/18 14:00, Jan Beulich wrote:
>>>>> On 26.06.18 at 15:18, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> @@ -49,6 +28,18 @@
>>>  #define ARCH_CAPS_RSBA                     (_AC(1, ULL) << 2)
>>>  #define ARCH_CAPS_SSB_NO           (_AC(1, ULL) << 4)
>>>  
>>> +#define MSR_EFER                        0xc0000080 /* Extended Feature 
>>> Enable Register */
>>> +#define EFER_SCE                        (_AC(1, ULL) <<  0) /* SYSCALL 
>>> Enable */
>>> +#define EFER_LME                        (_AC(1, ULL) <<  8) /* Long Mode 
>>> Enable */
>>> +#define EFER_LMA                        (_AC(1, ULL) << 10) /* Long Mode 
>>> Active */
>>> +#define EFER_NXE                        (_AC(1, ULL) << 11) /* No Execute 
>>> Enable */
>>> +#define EFER_SVME                       (_AC(1, ULL) << 12) /* Secure 
>>> Virtual Machine Enable */
>>> +#define EFER_LMSLE                      (_AC(1, ULL) << 13) /* Long Mode 
>>> Segment Limit Enable */
>>> +#define EFER_FFXSE                      (_AC(1, ULL) << 14) /* Fast 
>>> FXSAVE/FXRSTOR */
>>> +
>>> +#define EFER_KNOWN_MASK (EFER_SCE | EFER_LME | EFER_LMA | EFER_NXE | \
>>> +                         EFER_SVME | EFER_LMSLE | EFER_FFXSE)
>> When meaning to clean up and consolidate these and others, why
>> don't we switch to architectural MSR names at the same time? While
>> this will increase source size a little, it'll
>> - allow grep-ing for the MSRs' uses by their SDM names,
>> - significantly reduce the risk of name clashes with something on e.g.
>>   the arm side (EFER may not be the most risky one here, but some
>>   of the subsequent patches certainly seem to incur such a risk).
>>
>> I.e. here MSR_IA32_EFER and IA32_EFER_SCE etc.
>>
>> Other than this I'm certainly fine in general with this cleanup.
> 
> Removing IA32 is a deliberate and intended properly.  The
> non-architectural vs architectural nature of MSRs changes over time
> meaning the names here get stale.

But I don't think they've ever changed from IA32 to no IA32. I.e.
once an MSR becomes architectural, we could rename it once and
be done.

> As for grepability, most MSRs can't currently be located like that, and
> (naming instability aside) I believe the reduction in code volume is
> more important property to have.

The fact that "most MSRs can't currently be located like that" is
what I've long hoped we could overcome.

> There is no chance of clashing with ARM, as these are all arch-specific
> constants.  Any common code referencing them should be fixed by becoming
> arch-specific code.

The risk is for this to go unnoticed for a while.

Jan



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