| Message ID | 20240405123434.24822-3-jgross@suse.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | x86: Two fixes related to Xen PV guest mode | expand |
On 05/04/2024 1:34 pm, Juergen Gross wrote: > With recent sanity checks for topology information added, there are now > warnings issued for APs when running as a Xen PV guest: > > [Firmware Bug]: CPU 1: APIC ID mismatch. CPUID: 0x0000 APIC: 0x0001 > > This is due to the initial APIC ID obtained via CPUID for PV guests is > always 0. /sigh From Xen: switch ( leaf ) { case 0x1: /* TODO: Rework topology logic. */ res->b &= 0x00ffffffu; if ( is_hvm_domain(d) ) res->b |= (v->vcpu_id * 2) << 24; I think there's a very good chance it was random prior to Xen 4.6. That used to come straight out of a CPUID value, so would get the APIC ID of whichever pCPU it was scheduled on. > Avoid the warnings by synthesizing the CPUID data to contain the same > initial APIC ID as xen_pv_smp_config() is using for registering the > APIC IDs of all CPUs. > > Fixes: 52128a7a21f7 ("86/cpu/topology: Make the APIC mismatch warnings complete") > Signed-off-by: Juergen Gross <jgross@suse.com> > --- > arch/x86/xen/enlighten_pv.c | 10 +++++++++- > 1 file changed, 9 insertions(+), 1 deletion(-) > > diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c > index ace2eb054053..965e4ca36024 100644 > --- a/arch/x86/xen/enlighten_pv.c > +++ b/arch/x86/xen/enlighten_pv.c > @@ -219,13 +219,20 @@ static __read_mostly unsigned int cpuid_leaf5_edx_val; > static void xen_cpuid(unsigned int *ax, unsigned int *bx, > unsigned int *cx, unsigned int *dx) > { > - unsigned maskebx = ~0; > + unsigned int maskebx = ~0; > + unsigned int or_ebx = 0; > > /* > * Mask out inconvenient features, to try and disable as many > * unsupported kernel subsystems as possible. > */ > switch (*ax) { > + case 0x1: > + /* Replace initial APIC ID in bits 24-31 of EBX. */ > + maskebx = 0x00ffffff; > + or_ebx = smp_processor_id() << 24; I think the comment wants to cross-reference explicitly with xen_pv_smp_config(), because what we care about here is the two sources of information matching. Also while you're at it, the x2APIC ID in leaf 0xb. ~Andrew
On 05.04.24 14:50, Andrew Cooper wrote: > On 05/04/2024 1:34 pm, Juergen Gross wrote: >> With recent sanity checks for topology information added, there are now >> warnings issued for APs when running as a Xen PV guest: >> >> [Firmware Bug]: CPU 1: APIC ID mismatch. CPUID: 0x0000 APIC: 0x0001 >> >> This is due to the initial APIC ID obtained via CPUID for PV guests is >> always 0. > > /sigh > > From Xen: > > switch ( leaf ) > { > case 0x1: > /* TODO: Rework topology logic. */ > res->b &= 0x00ffffffu; > if ( is_hvm_domain(d) ) > res->b |= (v->vcpu_id * 2) << 24; > > > I think there's a very good chance it was random prior to Xen 4.6. That > used to come straight out of a CPUID value, so would get the APIC ID of > whichever pCPU it was scheduled on. > >> Avoid the warnings by synthesizing the CPUID data to contain the same >> initial APIC ID as xen_pv_smp_config() is using for registering the >> APIC IDs of all CPUs. >> >> Fixes: 52128a7a21f7 ("86/cpu/topology: Make the APIC mismatch warnings complete") >> Signed-off-by: Juergen Gross <jgross@suse.com> >> --- >> arch/x86/xen/enlighten_pv.c | 10 +++++++++- >> 1 file changed, 9 insertions(+), 1 deletion(-) >> >> diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c >> index ace2eb054053..965e4ca36024 100644 >> --- a/arch/x86/xen/enlighten_pv.c >> +++ b/arch/x86/xen/enlighten_pv.c >> @@ -219,13 +219,20 @@ static __read_mostly unsigned int cpuid_leaf5_edx_val; >> static void xen_cpuid(unsigned int *ax, unsigned int *bx, >> unsigned int *cx, unsigned int *dx) >> { >> - unsigned maskebx = ~0; >> + unsigned int maskebx = ~0; >> + unsigned int or_ebx = 0; >> >> /* >> * Mask out inconvenient features, to try and disable as many >> * unsupported kernel subsystems as possible. >> */ >> switch (*ax) { >> + case 0x1: >> + /* Replace initial APIC ID in bits 24-31 of EBX. */ >> + maskebx = 0x00ffffff; >> + or_ebx = smp_processor_id() << 24; > > I think the comment wants to cross-reference explicitly with > xen_pv_smp_config(), because what we care about here is the two sources > of information matching. I can add that as a comment. OTOH I'd really hope someone changing this code later would look into the commit message of the patch adding it. :-) > > Also while you're at it, the x2APIC ID in leaf 0xb. I'm not sure this is functionally relevant in PV guests. Note that my patch is only meant to silence warnings during boot. It is not needed for the system working correctly (at least I think so). Juergen
diff --git a/arch/x86/xen/enlighten_pv.c b/arch/x86/xen/enlighten_pv.c index ace2eb054053..965e4ca36024 100644 --- a/arch/x86/xen/enlighten_pv.c +++ b/arch/x86/xen/enlighten_pv.c @@ -219,13 +219,20 @@ static __read_mostly unsigned int cpuid_leaf5_edx_val; static void xen_cpuid(unsigned int *ax, unsigned int *bx, unsigned int *cx, unsigned int *dx) { - unsigned maskebx = ~0; + unsigned int maskebx = ~0; + unsigned int or_ebx = 0; /* * Mask out inconvenient features, to try and disable as many * unsupported kernel subsystems as possible. */ switch (*ax) { + case 0x1: + /* Replace initial APIC ID in bits 24-31 of EBX. */ + maskebx = 0x00ffffff; + or_ebx = smp_processor_id() << 24; + break; + case CPUID_MWAIT_LEAF: /* Synthesize the values.. */ *ax = 0; @@ -248,6 +255,7 @@ static void xen_cpuid(unsigned int *ax, unsigned int *bx, : "0" (*ax), "2" (*cx)); *bx &= maskebx; + *bx |= or_ebx; } static bool __init xen_check_mwait(void)
With recent sanity checks for topology information added, there are now warnings issued for APs when running as a Xen PV guest: [Firmware Bug]: CPU 1: APIC ID mismatch. CPUID: 0x0000 APIC: 0x0001 This is due to the initial APIC ID obtained via CPUID for PV guests is always 0. Avoid the warnings by synthesizing the CPUID data to contain the same initial APIC ID as xen_pv_smp_config() is using for registering the APIC IDs of all CPUs. Fixes: 52128a7a21f7 ("86/cpu/topology: Make the APIC mismatch warnings complete") Signed-off-by: Juergen Gross <jgross@suse.com> --- arch/x86/xen/enlighten_pv.c | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-)