Messages in this thread Patch in this message | | | Date | Mon, 31 Jul 2023 13:31:48 +0200 | Subject | Re: [patch V2 50/58] x86/apic: Provide common init infrastructure | From | Juergen Gross <> |
| |
On 24.07.23 15:35, Thomas Gleixner wrote: > In preparation for converting the hotpath APIC callbacks to static keys, > provide common initialization inforastructure. > > Lift apic_install_drivers() from probe_64.c and convert all places which > switch the apic instance by storing the pointer to use apic_install_driver() > as a first step. > > Signed-off-by: Thomas Gleixner <tglx@linutronix.de> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org> > --- > arch/x86/include/asm/apic.h | 2 + > arch/x86/kernel/apic/Makefile | 2 - > arch/x86/kernel/apic/apic.c | 31 ----------------------- > arch/x86/kernel/apic/apic_flat_64.c | 6 ---- > arch/x86/kernel/apic/bigsmp_32.c | 6 +--- > arch/x86/kernel/apic/init.c | 47 ++++++++++++++++++++++++++++++++++++ > arch/x86/kernel/apic/probe_32.c | 5 +-- > arch/x86/kernel/apic/probe_64.c | 13 --------- > arch/x86/xen/apic.c | 10 ++----- > 9 files changed, 59 insertions(+), 63 deletions(-) > > --- a/arch/x86/include/asm/apic.h > +++ b/arch/x86/include/asm/apic.h > @@ -344,6 +344,8 @@ extern int lapic_can_unplug_cpu(void); > > #ifdef CONFIG_X86_LOCAL_APIC > > +void __init apic_install_driver(struct apic *driver); > + > static inline u32 apic_read(u32 reg) > { > return apic->read(reg); > --- a/arch/x86/kernel/apic/Makefile > +++ b/arch/x86/kernel/apic/Makefile > @@ -7,7 +7,7 @@ > # In particualr, smp_apic_timer_interrupt() is called in random places. > KCOV_INSTRUMENT := n > > -obj-$(CONFIG_X86_LOCAL_APIC) += apic.o apic_common.o apic_noop.o ipi.o vector.o > +obj-$(CONFIG_X86_LOCAL_APIC) += apic.o apic_common.o apic_noop.o ipi.o vector.o init.o > obj-y += hw_nmi.o > > obj-$(CONFIG_X86_IO_APIC) += io_apic.o > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -236,8 +236,7 @@ static int modern_apic(void) > */ > static void __init apic_disable(void) > { > - pr_info("APIC: switched to apic NOOP\n"); > - apic = &apic_noop; > + apic_install_driver(&apic_noop); > } > > void native_apic_icr_write(u32 low, u32 id) > @@ -2486,34 +2485,6 @@ u32 x86_msi_msg_get_destid(struct msi_ms > } > EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); > > -#ifdef CONFIG_X86_64 > -void __init acpi_wake_cpu_handler_update(wakeup_cpu_handler handler) > -{ > - struct apic **drv; > - > - for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) > - (*drv)->wakeup_secondary_cpu_64 = handler; > -} > -#endif > - > -/* > - * Override the generic EOI implementation with an optimized version. > - * Only called during early boot when only one CPU is active and with > - * interrupts disabled, so we know this does not race with actual APIC driver > - * use. > - */ > -void __init apic_set_eoi_cb(void (*eoi)(void)) > -{ > - struct apic **drv; > - > - for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) { > - /* Should happen once for each apic */ > - WARN_ON((*drv)->eoi == eoi); > - (*drv)->native_eoi = (*drv)->eoi; > - (*drv)->eoi = eoi; > - } > -} > - > static void __init apic_bsp_up_setup(void) > { > #ifdef CONFIG_X86_64 > --- a/arch/x86/kernel/apic/apic_flat_64.c > +++ b/arch/x86/kernel/apic/apic_flat_64.c > @@ -143,11 +143,7 @@ static int physflat_acpi_madt_oem_check( > > static int physflat_probe(void) > { > - if (apic == &apic_physflat || num_possible_cpus() > 8 || > - jailhouse_paravirt()) > - return 1; > - > - return 0; > + return apic == &apic_physflat || num_possible_cpus() > 8 || jailhouse_paravirt(); > } > > static struct apic apic_physflat __ro_after_init = { > --- a/arch/x86/kernel/apic/bigsmp_32.c > +++ b/arch/x86/kernel/apic/bigsmp_32.c > @@ -119,10 +119,8 @@ bool __init apic_bigsmp_possible(bool cm > > void __init apic_bigsmp_force(void) > { > - if (apic != &apic_bigsmp) { > - apic = &apic_bigsmp; > - pr_info("Overriding APIC driver with bigsmp\n"); > - } > + if (apic != &apic_bigsmp) > + apic_install_driver(&apic_bigsmp); > } > > apic_driver(apic_bigsmp); > --- /dev/null > +++ b/arch/x86/kernel/apic/init.c > @@ -0,0 +1,47 @@ > +// SPDX-License-Identifier: GPL-2.0-only > +#define pr_fmt(fmt) "APIC: " fmt > + > +#include <asm/apic.h> > + > +#include "local.h" > + > +void __init apic_install_driver(struct apic *driver) > +{ > + if (apic == driver) > + return; > + > + apic = driver; > + > + if (IS_ENABLED(CONFIG_X86_X2APIC) && apic->x2apic_set_max_apicid) > + apic->max_apic_id = x2apic_max_apicid; > + > + pr_info("Switched APIC routing to: %s\n", driver->name); > +} > + > +#ifdef CONFIG_X86_64 > +void __init acpi_wake_cpu_handler_update(wakeup_cpu_handler handler) > +{ > + struct apic **drv; > + > + for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) > + (*drv)->wakeup_secondary_cpu_64 = handler; > +} > +#endif > + > +/* > + * Override the generic EOI implementation with an optimized version. > + * Only called during early boot when only one CPU is active and with > + * interrupts disabled, so we know this does not race with actual APIC driver > + * use. > + */ > +void __init apic_set_eoi_cb(void (*eoi)(void)) > +{ > + struct apic **drv; > + > + for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) { > + /* Should happen once for each apic */ > + WARN_ON((*drv)->eoi == eoi); > + (*drv)->native_eoi = (*drv)->eoi; > + (*drv)->eoi = eoi; > + } > +} > --- a/arch/x86/kernel/apic/probe_32.c > +++ b/arch/x86/kernel/apic/probe_32.c > @@ -82,7 +82,7 @@ static int __init parse_apic(char *arg) > > for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) { > if (!strcmp((*drv)->name, arg)) { > - apic = *drv; > + apic_install_driver(*drv); > cmdline_apic = 1; > return 0; > } > @@ -129,7 +129,7 @@ void __init x86_32_probe_apic(void) > > for (drv = __apicdrivers; drv < __apicdrivers_end; drv++) { > if ((*drv)->probe()) { > - apic = *drv; > + apic_install_driver(*drv); > break; > } > } > @@ -137,5 +137,4 @@ void __init x86_32_probe_apic(void) > if (drv == __apicdrivers_end) > panic("Didn't find an APIC driver"); > } > - printk(KERN_INFO "Using APIC driver %s\n", apic->name); > } > --- a/arch/x86/kernel/apic/probe_64.c > +++ b/arch/x86/kernel/apic/probe_64.c > @@ -13,19 +13,6 @@ > > #include "local.h" > > -static __init void apic_install_driver(struct apic *driver) > -{ > - if (apic == driver) > - return; > - > - apic = driver; > - > - if (IS_ENABLED(CONFIG_X86_X2APIC) && apic->x2apic_set_max_apicid) > - apic->max_apic_id = x2apic_max_apicid; > - > - pr_info("Switched APIC routing to %s:\n", apic->name); > -} > - > /* Select the appropriate APIC driver */ > void __init x86_64_probe_apic(void) > { > --- a/arch/x86/xen/apic.c > +++ b/arch/x86/xen/apic.c > @@ -160,20 +160,16 @@ static struct apic xen_pv_apic = { > > static void __init xen_apic_check(void) > { > - if (apic == &xen_pv_apic) > - return; > - > - pr_info("Switched APIC routing from %s to %s.\n", apic->name, > - xen_pv_apic.name); > - apic = &xen_pv_apic; > + apic_install_driver(&xen_pv_apic); > } > + > void __init xen_init_apic(void) > { > x86_apic_ops.io_apic_read = xen_io_apic_read; > /* On PV guests the APIC CPUID bit is disabled so none of the > * routines end up executing. */ > if (!xen_initial_domain()) > - apic = &xen_pv_apic; > + apic_install_driver(&xen_pv_apic);
This is working, but it produces a WARN() splat when booting as an unprivileged Xen PV guest from static_call patching (static_call_init() hasn't been called yet).
The diff below on top is fixing the issue:
diff --git a/arch/x86/xen/apic.c b/arch/x86/xen/apic.c index 1838aefc632f..84f24268670b 100644 --- a/arch/x86/xen/apic.c +++ b/arch/x86/xen/apic.c @@ -163,14 +163,18 @@ static void __init xen_apic_check(void) apic_install_driver(&xen_pv_apic); }
+void __init xen_apic_install(void) +{ + /* + * On PV guests the APIC CPUID bit is disabled so none of the + * routines end up executing. + */ + apic_install_driver(&xen_pv_apic); +} + void __init xen_init_apic(void) { x86_apic_ops.io_apic_read = xen_io_apic_read; - /* On PV guests the APIC CPUID bit is disabled so none of the - * routines end up executing. */ - if (!xen_initial_domain()) - apic_install_driver(&xen_pv_apic); - x86_platform.apic_post_init = xen_apic_check; } apic_driver(xen_pv_apic); diff --git a/arch/x86/xen/smp_pv.c b/arch/x86/xen/smp_pv.c index c6b42c66c60c..ff2d0754ce62 100644 --- a/arch/x86/xen/smp_pv.c +++ b/arch/x86/xen/smp_pv.c @@ -188,6 +188,9 @@ static void __init _get_smp_config(unsigned int early) static void __init xen_pv_smp_prepare_boot_cpu(void) { BUG_ON(smp_processor_id() != 0); + + xen_apic_install(); + native_smp_prepare_boot_cpu();
if (!xen_feature(XENFEAT_writable_page_tables)) diff --git a/arch/x86/xen/xen-ops.h b/arch/x86/xen/xen-ops.h index 408a2aa66c69..217e4b625e4d 100644 --- a/arch/x86/xen/xen-ops.h +++ b/arch/x86/xen/xen-ops.h @@ -119,6 +119,7 @@ static inline void __init xen_init_vga(const struct dom0_vga_console_info *info, void xen_add_preferred_consoles(void);
void __init xen_init_apic(void); +void __init xen_apic_install(void);
#ifdef CONFIG_XEN_EFI extern void xen_efi_init(struct boot_params *boot_params);
Juergen [unhandled content-type:application/pgp-keys][unhandled content-type:application/pgp-signature] | |