Re: [PATCH 5/6] x86/msr: Clean up the MSR_APIC_BASE constants

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 27/06/18 14:35, Andrew Cooper wrote:
> On 27/06/18 14:32, Roger Pau Monné wrote:
>> On Tue, Jun 26, 2018 at 02:18:17PM +0100, Andrew Cooper wrote:
>>> We currently have MSR_IA32_APICBASE and MSR_IA32_APICBASE_MSR which are
>>> synonymous from a naming point of view, but refer to very different
>>> things.
>>>
>>> Cleave out the handling of MSR_APIC_BASE (0x1b), and rename
>>> MSR_IA32_APICBASE_BASE to APIC_BASE_ADDR_MASK to better describe its
>>> purpose.
>>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
>> Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
>>
>>> diff --git a/xen/arch/x86/apic.c b/xen/arch/x86/apic.c
>>> index ffa5a69..aa677e0 100644
>>> --- a/xen/arch/x86/apic.c
>>> +++ b/xen/arch/x86/apic.c
>>> @@ -1446,23 +1448,21 @@ void __init record_boot_APIC_mode(void)
>>>                  apic_mode_to_str(apic_boot_mode));
>>>  }
>>>  
>>> -/* Look at the bits in MSR_IA32_APICBASE and work out which
>>> - * APIC mode we are in */
>>> +/* Look at the bits in MSR_APIC_BASE and work out which APIC mode we are in */
>>>  enum apic_mode current_local_apic_mode(void)
>>>  {
>>>      u64 msr_contents;
>>>  
>>> -    rdmsrl(MSR_IA32_APICBASE, msr_contents);
>>> +    rdmsrl(MSR_APIC_BASE, msr_contents);
>>>  
>>>      /* Reading EXTD bit from the MSR is only valid if CPUID
>>>       * says so, else reserved */
>>> -    if ( boot_cpu_has(X86_FEATURE_X2APIC)
>>> -         && (msr_contents & MSR_IA32_APICBASE_EXTD) )
>>> +    if ( boot_cpu_has(X86_FEATURE_X2APIC) && (msr_contents & APIC_BASE_EXTD) )
>> While there you could change it to cpu_has_x2apic.
> So I can.  Thanks,

Actually, this code is some of my earliest contributions to Xen, and my
current self is really regretting the lack of review of my older self's
code.

Both comments are false and the check isn't necessary.  I've fixed all
this up and added a comment to the commit message.

~Andrew

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