| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <abfa4548-4367-d8fa-f23f-b2ca4a912258@suse.com>
Date: Mon, 31 Jul 2023 13:31:48 +0200
From: Juergen Gross <jgross@...e.com>
To: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>
Cc: x86@...nel.org, Andrew Cooper <andrew.cooper3@...rix.com>,
Tom Lendacky <thomas.lendacky@....com>,
Paolo Bonzini <pbonzini@...hat.com>,
Wei Liu <wei.liu@...nel.org>,
Arjan van de Ven <arjan@...ux.intel.com>,
Michael Kelley <mikelley@...rosoft.com>,
Peter Keresztes Schmidt <peter@...esztesschmidt.de>,
"Peter Zijlstra (Intel)" <peterz@...radead.org>
Subject: Re: [patch V2 50/58] x86/apic: Provide common init infrastructure
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@...utronix.de>
> Acked-by: Peter Zijlstra (Intel) <peterz@...radead.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
Download attachment "OpenPGP_0xB0DE9DD628BF132F.asc" of type "application/pgp-keys" (3099 bytes)
Download attachment "OpenPGP_signature" of type "application/pgp-signature" (496 bytes)
Powered by blists - more mailing lists