[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID:
<BN7PR02MB4148C25575F1C98531B6164CD4922@BN7PR02MB4148.namprd02.prod.outlook.com>
Date: Mon, 2 Sep 2024 03:35:06 +0000
From: Michael Kelley <mhklinux@...look.com>
To: Yunhong Jiang <yunhong.jiang@...ux.intel.com>, "tglx@...utronix.de"
<tglx@...utronix.de>, "mingo@...hat.com" <mingo@...hat.com>, "bp@...en8.de"
<bp@...en8.de>, "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
"x86@...nel.org" <x86@...nel.org>, "hpa@...or.com" <hpa@...or.com>,
"robh@...nel.org" <robh@...nel.org>, "krzk+dt@...nel.org"
<krzk+dt@...nel.org>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"kys@...rosoft.com" <kys@...rosoft.com>, "haiyangz@...rosoft.com"
<haiyangz@...rosoft.com>, "wei.liu@...nel.org" <wei.liu@...nel.org>,
"decui@...rosoft.com" <decui@...rosoft.com>, "rafael@...nel.org"
<rafael@...nel.org>, "lenb@...nel.org" <lenb@...nel.org>,
"kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
"linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
"linux-acpi@...r.kernel.org" <linux-acpi@...r.kernel.org>
Subject: RE: [PATCH v2 3/9] x86/dt: Support the ACPI multiprocessor wakeup for
device tree
From: Yunhong Jiang <yunhong.jiang@...ux.intel.com> Sent: Friday, August 23, 2024 4:23 PM
>
> When a TDX guest boots with the device tree instead of ACPI, it can
> reuse the ACPI multiprocessor wakeup mechanism to wake up application
> processors(AP), without introducing a new mechanism from scrach.
>
> In the ACPI spec, two structures are defined to wake up the APs: the
> multiprocessor wakeup structure and the multiprocessor wakeup mailbox
> structure. The multiprocessor wakeup structure is passed to OS through a
> Multiple APIC Description Table(MADT), one field specifying the physical
> address of the multiprocessor wakeup mailbox structure. The OS sends
> a message to firmware through the multiprocessor wakeup mailbox
> structure, to bring up the APs.
>
> In device tree environment, the multiprocessor wakeup structure is not
> used, to reduce the dependency on the generic ACPI table. The
> information defined in this structure is defined in the properties of
> cpus node in the device tree. The "wakeup-mailbox-addr" property
> specifies the physical address of the multiprocessor wakeup mailbox
> structure. The OS will follow the ACPI spec to send the message to the
> firmware to bring up the APs.
>
> Signed-off-by: Yunhong Jiang <yunhong.jiang@...ux.intel.com>
> ---
> MAINTAINERS | 1 +
> arch/x86/Kconfig | 2 +-
> arch/x86/include/asm/acpi.h | 1 -
> arch/x86/include/asm/madt_wakeup.h | 16 +++++++++++++
> arch/x86/kernel/madt_wakeup.c | 38 ++++++++++++++++++++++++++++++
> 5 files changed, 56 insertions(+), 2 deletions(-)
> create mode 100644 arch/x86/include/asm/madt_wakeup.h
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 5555a3bbac5f..900de6501eba 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -288,6 +288,7 @@ T: git
> git://git.kernel.org/pub/scm/linux/kernel/git/rafael/linux-pm
> F: Documentation/ABI/testing/configfs-acpi
> F: Documentation/ABI/testing/sysfs-bus-acpi
> F: Documentation/firmware-guide/acpi/
> +F: arch/x86/include/asm/madt_wakeup.h
> F: arch/x86/kernel/acpi/
> F: arch/x86/kernel/madt_playdead.S
> F: arch/x86/kernel/madt_wakeup.c
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index d422247b2882..dba46dd30049 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1123,7 +1123,7 @@ config X86_LOCAL_APIC
> config ACPI_MADT_WAKEUP
> def_bool y
> depends on X86_64
> - depends on ACPI
> + depends on ACPI || OF
> depends on SMP
> depends on X86_LOCAL_APIC
>
> diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h
> index 21bc53f5ed0c..0e082303ca26 100644
> --- a/arch/x86/include/asm/acpi.h
> +++ b/arch/x86/include/asm/acpi.h
> @@ -83,7 +83,6 @@ union acpi_subtable_headers;
> int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
> const unsigned long end);
>
> -void asm_acpi_mp_play_dead(u64 reset_vector, u64 pgd_pa);
>
> /*
> * Check if the CPU can handle C2 and deeper
> diff --git a/arch/x86/include/asm/madt_wakeup.h
> b/arch/x86/include/asm/madt_wakeup.h
> new file mode 100644
> index 000000000000..a8cd50af581a
> --- /dev/null
> +++ b/arch/x86/include/asm/madt_wakeup.h
> @@ -0,0 +1,16 @@
> +/* SPDX-License-Identifier: GPL-2.0 */
> +#ifndef __ASM_X86_MADT_WAKEUP_H
> +#define __ASM_X86_MADT_WAKEUP_H
> +
> +void asm_acpi_mp_play_dead(u64 reset_vector, u64 pgd_pa);
> +
> +#if defined(CONFIG_OF) && defined(CONFIG_ACPI_MADT_WAKEUP)
> +u64 dtb_parse_mp_wake(void);
> +#else
> +static inline u64 dtb_parse_mp_wake(void)
> +{
> + return 0;
> +}
> +#endif
> +
> +#endif /* __ASM_X86_MADT_WAKEUP_H */
> diff --git a/arch/x86/kernel/madt_wakeup.c b/arch/x86/kernel/madt_wakeup.c
> index d5ef6215583b..7257e7484569 100644
> --- a/arch/x86/kernel/madt_wakeup.c
> +++ b/arch/x86/kernel/madt_wakeup.c
> @@ -14,6 +14,8 @@
> #include <asm/nmi.h>
> #include <asm/processor.h>
> #include <asm/reboot.h>
> +#include <asm/madt_wakeup.h>
> +#include <asm/prom.h>
>
> /* Physical address of the Multiprocessor Wakeup Structure mailbox */
> static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> @@ -122,6 +124,7 @@ static int __init init_transition_pgtable(pgd_t *pgd)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> static int __init acpi_mp_setup_reset(u64 reset_vector)
> {
> struct x86_mapping_info info = {
> @@ -168,6 +171,7 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
>
> return 0;
> }
> +#endif
When acpi_mp_setup_reset() is #ifdef'ed out because of CONFIG_ACPI
not being set, doesn't this generate compile warnings about
init_transition_pgtable(), alloc_pgt_page(), free_pgt_page(), etc. being
unused?
It appears that the only code in madt_wakeup.c that is shared between
the ACPI and OF cases is acpi_wakeup_cpu()? Is that correct?
>
> static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> {
> @@ -226,6 +230,7 @@ static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> return 0;
> }
>
> +#ifdef CONFIG_ACPI
> static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
> {
> cpu_hotplug_disable_offlining();
> @@ -290,3 +295,36 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
>
> return 0;
> }
> +#endif /* CONFIG_ACPI */
> +
> +#ifdef CONFIG_OF
> +u64 __init dtb_parse_mp_wake(void)
> +{
> + struct device_node *node;
> + u64 mailaddr = 0;
> +
> + node = of_find_node_by_path("/cpus");
> + if (!node)
> + return 0;
> +
> + if (of_property_match_string(node, "enable-method", "acpi-wakeup-mailbox") < 0) {
> + pr_err("No acpi wakeup mailbox enable-method\n");
> + goto done;
Patch 4 of this series unconditionally calls dtb_parse_mp_wake(),
so it will be called in normal VMs, as a well as SEV-SNP and TDX
kernels built for VTL 2 (assuming CONFIG_OF is set). Normal VMs
presumably won't be using DT and won't have the "/cpus" node,
so this function will silently do nothing, which is fine. But do you
expect the DT "/cpus" node to be present in an SEV-SNP VTL 2
environment? If so, this function will either output some spurious
error messages, or SEV-SNP will use the ACPI wakeup mailbox
method, which is probably not what you want.
Michael
> + }
> +
> + if (of_property_read_u64(node, "wakeup-mailbox-addr", &mailaddr)) {
> + pr_err("Invalid wakeup mailbox addr\n");
> + goto done;
> + }
> + acpi_mp_wake_mailbox_paddr = mailaddr;
> + pr_info("dt wakeup-mailbox: addr 0x%llx\n", mailaddr);
> +
> + /* No support for the MADT reset vector yet */
> + cpu_hotplug_disable_offlining();
> + apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +
> +done:
> + of_node_put(node);
> + return mailaddr;
> +}
> +#endif /* CONFIG_OF */
> --
> 2.25.1
>
Powered by blists - more mailing lists