Re: [Xen-devel] [PATCH] x86/faulting: Use formal defines instead of opencoded bits

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 25.02.14 at 12:23, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
> On 25/02/14 11:21, Jan Beulich wrote:
>>>>> On 25.02.14 at 12:02, Andrew Cooper <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/cpu/intel.c
>>> +++ b/xen/arch/x86/cpu/intel.c
>>> @@ -21,7 +21,8 @@
>>>  static unsigned int probe_intel_cpuid_faulting(void)
>>>  {
>>>     uint64_t x;
>>> -   return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) && (x & (1u<<31));
>>> +   return !rdmsr_safe(MSR_INTEL_PLATFORM_INFO, x) &&
>>> +            (x & PLATFORM_INFO_CPUID_FAULTING);
>> Indentation (a single hard tab ought to come first at least).
>>
>>> --- a/xen/include/asm-x86/msr-index.h
>>> +++ b/xen/include/asm-x86/msr-index.h
>>> @@ -486,7 +486,12 @@
>>>  
>>>  /* Intel cpuid faulting MSRs */
>>>  #define MSR_INTEL_PLATFORM_INFO            0x000000ce
>>> +#define _PLATFORM_INFO_CPUID_FAULTING      31
>>> +#define PLATFORM_INFO_CPUID_FAULTING       (1ULL << 
>>> _PLATFORM_INFO_CPUID_FAULTING)
>>> +
>>>  #define MSR_INTEL_MISC_FEATURES_ENABLES    0x00000140
>>> +#define _MISC_FEATURES_CPUID_FAULTING      0
>>> +#define MISC_FEATURES_CPUID_FAULTING       (1ULL << 
>>> _MISC_FEATURES_CPUID_FAULTING)
>> I wonder whether, from a name space pov, it wouldn't be better
>> if these new constants had at least MSR_ as additional prefix. Both
>> are rather generic without...
> 
> How about MSR_INTEL_ to match their MSR number names?

I'd be fine with that. I merely didn't require it to be the full name
because it gets rather long. But with only a single use site that's
probably acceptable.

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel