[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH] xen/x86: On x2APIC mode, derive LDR from APIC_ID
On 13.11.2023 18:53, Roger Pau Monné wrote: > 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, Hard to reconstruct over 9 years later. It feels like Alejandro may be right with his derivation. > 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. Well, it gave the appearance of deriving from the APIC ID. Just that it was missing GET_xAPIC_ID() around the vlapic_get_reg() (hence why LDR was uniformly 1 on all CPUs). >> 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') +1 >> Signed-off-by: Alejandro Vallejo <alejandro.vallejo@xxxxxxxxx> > > 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. That earlier change deliberately fixed up any bogus values. I wonder whether what you suggest will do more good or more harm than going even farther and once again fixing up bad values in lapic_load_fixup(). After all LDR being wrong affects vlapic_match_logical_addr()'s outcome. I think one of the two wants adding to the change, though. Jan
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |