RE: [PATCH v2 3/9] x86/dt: Support the ACPI multiprocessor wakeup for device tree

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Yunhong Jiang <yunhong.jiang@xxxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxxx>
> ---
>  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
> 






[Index of Archives]     [Device Tree Compilter]     [Device Tree Spec]     [Linux Driver Backports]     [Video for Linux]     [Linux USB Devel]     [Linux PCI Devel]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Yosemite Backpacking]


  Powered by Linux