Re: [PATCH v7 02/10] xen/x86: Add initial x2APIC ID to the per-vLAPIC save area

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.10.2024 13:03, Alejandro Vallejo wrote:
> On Wed Oct 30, 2024 at 6:37 AM GMT, Jan Beulich wrote:
>> On 29.10.2024 21:30, Andrew Cooper wrote:
>>> On 21/10/2024 4:45 pm, Alejandro Vallejo wrote:
>>>> @@ -310,19 +309,16 @@ void guest_cpuid(const struct vcpu *v, uint32_t leaf,
>>>>          break;
>>>>  
>>>>      case 0xb:
>>>> -        /*
>>>> -         * In principle, this leaf is Intel-only.  In practice, it is 
>>>> tightly
>>>> -         * coupled with x2apic, and we offer an x2apic-capable APIC 
>>>> emulation
>>>> -         * to guests on AMD hardware as well.
>>>> -         *
>>>> -         * TODO: Rework topology logic.
>>>> -         */
>>>>          if ( p->basic.x2apic )
>>>>          {
>>>>              *(uint8_t *)&res->c = subleaf;
>>>>  
>>>> -            /* Fix the x2APIC identifier. */
>>>> -            res->d = v->vcpu_id * 2;
>>>> +            /*
>>>> +             * Fix the x2APIC identifier. The PV side is nonsensical, but
>>>> +             * we've always shown it like this so it's kept for compat.
>>>> +             */
>>>
>>> In hindsight I should changed "Fix the x2APIC identifier." when I
>>> reworked this logic, but oh well - better late than never.
>>>
>>> /* The x2APIC_ID is per-vCPU, and fixed irrespective of the requested
>>> subleaf. */
>>
>> Can we perhaps avoid "fix" in this comment? "Adjusted", "overwritten", or
>> some such ought to do, without carrying a hint towards some bug somewhere.
> 
> I understood "fix" there as "pin" rather than "unbreak".

Oh, right - that possible meaning escaped me.

Jan