[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f8fb1990-11e8-459d-a365-6dc277f34a6b@citrix.com>
Date: Fri, 5 Apr 2024 13:50:04 +0100
From: Andrew Cooper <andrew.cooper3@...rix.com>
To: Juergen Gross <jgross@...e.com>, linux-kernel@...r.kernel.org,
x86@...nel.org
Cc: Boris Ostrovsky <boris.ostrovsky@...cle.com>,
Thomas Gleixner <tglx@...utronix.de>, Ingo Molnar <mingo@...hat.com>,
Borislav Petkov <bp@...en8.de>, Dave Hansen <dave.hansen@...ux.intel.com>,
"H. Peter Anvin" <hpa@...or.com>, xen-devel@...ts.xenproject.org
Subject: Re: [PATCH 2/2] x86/xen: return a sane initial apic id when running
as PV guest
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@...e.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
Powered by blists - more mailing lists