| 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: <BYAPR21MB16885B21A0572AE4BE12C3B2D73FA@BYAPR21MB1688.namprd21.prod.outlook.com>
Date: Fri, 21 Jul 2023 04:27:39 +0000
From: "Michael Kelley (LINUX)" <mikelley@...rosoft.com>
To: Thomas Gleixner <tglx@...utronix.de>,
LKML <linux-kernel@...r.kernel.org>
CC: "x86@...nel.org" <x86@...nel.org>,
Linus Torvalds <torvalds@...ux-foundation.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>,
Juergen Gross <jgross@...e.com>
Subject: RE: [patch 51/58] x86/apic: Provide apic_update_callback()
From: Thomas Gleixner <tglx@...utronix.de> Sent: Monday, July 17, 2023 4:16 PM
>
> There are already two variants of update mechanism for particular callbacks
> and virtualization just writes into the data structure.
>
> Provide an interface and use a shadow data structure to preserve callbacks
> so they can be reapplied when the APIC driver is replaced.
>
> The extra data structure is intentional as any new callback needs to be
> also updated in the core code. This also prepares for static calls.
>
> Signed-off-by: Thomas Gleixner <tglx@...utronix.de>
> ---
> arch/x86/include/asm/apic.h | 25 +++++++++++++++++++++++++
> arch/x86/kernel/apic/init.c | 39 ++++++++++++++++++++++++++++++++++++++-
> arch/x86/kernel/setup.c | 2 ++
> 3 files changed, 65 insertions(+), 1 deletion(-)
>
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -308,6 +308,23 @@ struct apic {
> char *name;
> };
>
> +struct apic_override {
> + void (*eoi)(void);
> + void (*native_eoi)(void);
> + void (*write)(u32 reg, u32 v);
> + u32 (*read)(u32 reg);
> + void (*send_IPI)(int cpu, int vector);
> + void (*send_IPI_mask)(const struct cpumask *mask, int vector);
> + void (*send_IPI_mask_allbutself)(const struct cpumask *msk, int vec);
> + void (*send_IPI_allbutself)(int vector);
> + void (*send_IPI_all)(int vector);
> + void (*send_IPI_self)(int vector);
> + u64 (*icr_read)(void);
> + void (*icr_write)(u32 low, u32 high);
> + int (*wakeup_secondary_cpu)(int apicid, unsigned long start_eip);
> + int (*wakeup_secondary_cpu_64)(int apicid, unsigned long start_eip);
> +};
> +
> /*
> * Pointer to the local APIC driver in use on this system (there's
> * always just one such driver in use - the kernel decides via an
> @@ -343,9 +360,17 @@ extern int lapic_can_unplug_cpu(void);
> #endif
>
> #ifdef CONFIG_X86_LOCAL_APIC
> +extern struct apic_override __x86_apic_override;
>
> +void __init apic_setup_apic_calls(void);
> void __init apic_install_driver(struct apic *driver);
>
> +#define apic_update_callback(_callback, _fn) { \
> + __x86_apic_override._callback = _fn; \
> + apic->_callback = _fn; \
> + pr_info("APIC::%s() replaced with %ps()\n", #_callback, _fn); \
FWIW, when I saw the dmesg output, I thought the double colon in the above
message was a typo. When juxtaposed against the nearly adjacent APIC-related
messages that have just a single colon, it's a little unexpected.
[ 0.914587] APIC: Switch to symmetric I/O mode setup
[ 0.918289] APIC: Switched APIC routing to: physical flat
[ 0.925577] Hyper-V: Using IPI hypercalls
[ 0.928460] APIC::send_IPI() replaced with hv_send_ipi()
[ 0.931862] APIC::send_IPI_mask() replaced with hv_send_ipi_mask()
[ 0.935960] APIC::send_IPI_mask_allbutself() replaced with hv_send_ipi_mask_allbutself()
Just my $.02 ....
Michael
> +}
> +
> static inline u32 apic_read(u32 reg)
> {
> return apic->read(reg);
> --- a/arch/x86/kernel/apic/init.c
> +++ b/arch/x86/kernel/apic/init.c
> @@ -5,6 +5,37 @@
>
> #include "local.h"
>
> +/* The container for function call overrides */
> +struct apic_override __x86_apic_override __initdata;
> +
> +#define apply_override(__cb) \
> + if (__x86_apic_override.__cb) \
> + apic->__cb = __x86_apic_override.__cb
> +
> +static __init void restore_override_callbacks(void)
> +{
> + apply_override(eoi);
> + apply_override(native_eoi);
> + apply_override(write);
> + apply_override(read);
> + apply_override(send_IPI);
> + apply_override(send_IPI_mask);
> + apply_override(send_IPI_mask_allbutself);
> + apply_override(send_IPI_allbutself);
> + apply_override(send_IPI_all);
> + apply_override(send_IPI_self);
> + apply_override(icr_read);
> + apply_override(icr_write);
> + apply_override(wakeup_secondary_cpu);
> + apply_override(wakeup_secondary_cpu_64);
> +}
> +
> +void __init apic_setup_apic_calls(void)
> +{
> + /* Ensure that the default APIC has native_eoi populated */
> + apic->native_eoi = apic->eoi;
> +}
> +
> void __init apic_install_driver(struct apic *driver)
> {
> if (apic == driver)
> @@ -15,6 +46,13 @@ void __init apic_install_driver(struct a
> if (IS_ENABLED(CONFIG_X86_X2APIC) && apic->x2apic_set_max_apicid)
> apic->max_apic_id = x2apic_max_apicid;
>
> + /* Copy the original eoi() callback as KVM/HyperV might overwrite it */
> + if (!apic->native_eoi)
> + apic->native_eoi = apic->eoi;
> +
> + /* Apply any already installed callback overrides */
> + restore_override_callbacks();
> +
> pr_info("Switched APIC routing to: %s\n", driver->name);
> }
>
> @@ -41,7 +79,6 @@ void __init apic_set_eoi_cb(void (*eoi)(
> 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/setup.c
> +++ b/arch/x86/kernel/setup.c
> @@ -1018,6 +1018,8 @@ void __init setup_arch(char **cmdline_p)
>
> x86_report_nx();
>
> + apic_setup_apic_calls();
> +
> if (acpi_mps_check()) {
> #ifdef CONFIG_X86_LOCAL_APIC
> apic_is_disabled = true;
Powered by blists - more mailing lists