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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<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

Powered by Openwall GNU/*/Linux Powered by OpenVZ