On Mon, Nov 13, 2023 at 04:50:23PM +0000, Alejandro Vallejo wrote:
> Both Intel and AMD manuals agree that on x2APIC mode, the APIC LDR and ID
> registers are derivable from each other through a fixed formula.
>
> Xen uses that formula, but applies it to vCPU IDs (which are sequential)
> rather than x2APIC_IDs (which are not, at the moment). As I understand it,
> this is an attempt to tightly pack vCPUs into clusters so each cluster has
> 16 vCPUs rather than 8, but this is problematic for OSs that might read the
> x2APIC_ID and internally derive LDR (or the other way around)
I would replace the underscore from x2APIC ID with a space instead.
Seeing the commit that introduced the bogus LDR value, I'm not sure it
was intentional, as previous Xen code had:
u32 id = vlapic_get_reg(vlapic, APIC_ID);
u32 ldr = ((id & ~0xf) << 16) | (1 << (id & 0xf));
vlapic_set_reg(vlapic, APIC_LDR, ldr);
Which was correct, as the LDR was derived from the APIC ID and not the
vCPU ID.
> This patch fixes the implementation so we follow the rules in the x2APIC
> spec(s).
>
> While in the neighborhood, replace the u32 type with the standard uint32_t
Likely wants:
Fixes: f9e0cccf7b35 ('x86/HVM: fix ID handling of x2APIC emulation')
> Signed-off-by: Alejandro Vallejo <alejandro.vall...@cloud.com>
I do wonder whether we need to take any precautions with guests being
able to trigger an APIC reset, and thus seeing a changed LDR register
if the guest happens to be migrated from an older hypervisor version
that doesn't have this fix. IOW: I wonder whether Xen should keep the
previous bogus LDR value across APIC resets for guests that have
already seen it.
Thanks, Roger.
