On 14.11.2023 18:50, Krystian Hebel wrote: > It used to be called from smp_callin(), however BUG_ON() was invoked on > multiple occasions before that. It may end up calling machine_restart() > which tries to get APIC ID for CPU running this code. If BSP detected > that x2APIC is enabled, get_apic_id() will try to use it for all CPUs. > Enabling x2APIC on secondary CPUs earlier protects against an endless > loop of #GP exceptions caused by attempts to read IA32_X2APIC_APICID > MSR while x2APIC is disabled in IA32_APIC_BASE. > > Signed-off-by: Krystian Hebel <[email protected]> > --- > xen/arch/x86/smpboot.c | 9 ++++++++- > 1 file changed, 8 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c > index 8ae65ab1769f..a3895dafa267 100644 > --- a/xen/arch/x86/smpboot.c > +++ b/xen/arch/x86/smpboot.c > @@ -184,7 +184,6 @@ static void smp_callin(void) > * update until we finish. We are free to set up this CPU: first the > APIC. > */ > Dprintk("CALLIN, before setup_local_APIC().\n"); > - x2apic_ap_setup(); > setup_local_APIC(false); > > /* Save our processor parameters. */ > @@ -351,6 +350,14 @@ void start_secondary(void *unused) > get_cpu_info()->xen_cr3 = 0; > get_cpu_info()->pv_cr3 = 0; > > + /* > + * BUG_ON() used in load_system_tables() and later code may end up > calling > + * machine_restart() which tries to get APIC ID for CPU running this > code. > + * If BSP detected that x2APIC is enabled, get_apic_id() will try to use > it > + * for _all_ CPUs. Enable x2APIC on secondary CPUs now so we won't end up > + * with endless #GP loop. > + */ > + x2apic_ap_setup(); > load_system_tables();
While I find the argument convincing, I seem to recall that there was a firm plan to have load_system_tables() as early as possible. Andrew? Jan
