From: Yunhong Jiang <yunhong.ji...@linux.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.ji...@linux.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
> 


Reply via email to