Re: [PATCH 1/6] xen/x86: Add initial x2APIC ID to the per-vLAPIC save area

[Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx>]

On 20/03/2024 09:33, Roger Pau Monné wrote:
>>
>> Why not just return the x2apic_id field from the cpu_policy object?
>> (topo.subleaf[X].x2apic_id)
> 
> Scratch that, the cpu policy is per-domain, not per-vcpu, and hence
> cannot hold the x{,2}apic IDs.
Yes, that :)

Originally I tried to make the policy per-vCPU, tbf, but that's a very,
very deep and complicated hole that was very hard to deal with and I'm
not tempted to try that again.

>>> diff --git a/xen/include/public/arch-x86/hvm/save.h 
>>> b/xen/include/public/arch-x86/hvm/save.h
>>> index 7ecacadde1..1c2ec669ff 100644
>>> --- a/xen/include/public/arch-x86/hvm/save.h
>>> +++ b/xen/include/public/arch-x86/hvm/save.h
>>> @@ -394,6 +394,8 @@ struct hvm_hw_lapic {
>>>      uint32_t             disabled; /* VLAPIC_xx_DISABLED */
>>>      uint32_t             timer_divisor;
>>>      uint64_t             tdt_msr;
>>> +    uint32_t             x2apic_id;
>>> +    uint32_t             rsvd_zero;
>>
>> Do we really to add a new field, couldn't we get the lapic IDs from
>> the cpu_policy?
> 
> Since getting from the cpu_policy is not possible, what about getting
> it from the registers itself?  It's already present in hvm_hw_lapic_regs.
> 
> Regards, Roger.

If every APIC is in x2APIC mode, yes. But if any of them is in xAPIC
mode there's problems. The xAPIC ID is overridable by the guest simply
by writing into it, so it's tricky to know whether it's sane or not.
This new field is effectively an immutable "initial APIC ID", which we
can recreate to the old convention when not present in the stream.

If you can think of an alternative that doesn't involve adding a field
or fixing in stone the mapping strategy I'm happy to do that instead,
but I think this is the lesser evil.

Cheers,
Alejandro