[PATCH 0/7] x86/acpi: Move ACPI MADT wakeup to generic code

[PATCH 0/7] x86/acpi: Move ACPI MADT wakeup to generic code

[Yunhong Jiang]

This set of patches add ACPI multiprocessor wakeup support to TDX VMs
booting with device tree instead of ACPI.

Historically, x86 platforms have booted secondary processors (APs) using
INIT followed by the start up IPI (SIPI) messages. However, TDX VMs
can't use this protocol because this protocol requires assistance from
VMMs while VMMs are not trusted by TDX guest.

ACPI specification version 6.4 introduced a new wakeup mailbox model to
address this issue. A "Multiprocessor Wakeup Structure" has been added to
an existing ACPI table (MADT). This structure provides the physical of a
"Multiprocessor Wakeup Mailbox Structure". Message written to the mailbox
structure steers the APs to the boot code.

With this new wakeup model, TDX VMs with ACPI support boot the APs
securely. However, TDX VMs with the device tree have no ACPI support and
still face the challenge.

To fix this challenge, either a new mechanism from scratch is
introduced, or the TDX VMs with device tree can utilize the ACPI wakeup
model.

By reusing the ACPI wakeup mailbox model, the Multiprocessor Wakeup Mailbox
Structure will be kept and the message mechanism will be the same as ACPI.
This will reduce maintenance effort in the long term.

The first patch moves the madt wakeup implementation to generic code.

The second/third patches add the mailbox support to the device tree.

The last four patches apply the mailbox support to the hyper-v TDX VMs
with device tree.

Yunhong Jiang (7):
  x86/acpi: Move ACPI MADT wakeup to generic code
  dt-bindings: x86: Add ACPI wakeup mailbox
  x86/dt: Support the ACPI multiprocessor wakeup for device tree
  x86/hyperv: Parse the ACPI wakeup mailbox
  x86/hyperv: Mark ACPI wakeup mailbox page as private
  x86/hyperv: Reserve real mode when ACPI wakeup mailbox is available
  x86/hyperv: Use the ACPI wakeup mailbox for VTL2 guests when available

 .../devicetree/bindings/x86/wakeup.yaml       | 41 ++++++++++++++++
 MAINTAINERS                                   |  3 ++
 arch/x86/Kconfig                              |  2 +-
 arch/x86/hyperv/hv_vtl.c                      | 43 +++++++++++++++--
 arch/x86/include/asm/acpi.h                   |  1 -
 arch/x86/include/asm/madt_wakeup.h            | 16 +++++++
 arch/x86/include/asm/mshyperv.h               |  3 ++
 arch/x86/kernel/Makefile                      |  1 +
 arch/x86/kernel/acpi/Makefile                 |  1 -
 arch/x86/kernel/cpu/mshyperv.c                |  2 +
 arch/x86/kernel/{acpi => }/madt_playdead.S    |  0
 arch/x86/kernel/{acpi => }/madt_wakeup.c      | 47 ++++++++++++++++++-
 drivers/hv/hv_common.c                        |  8 ++++
 13 files changed, 159 insertions(+), 9 deletions(-)
 create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml
 create mode 100644 arch/x86/include/asm/madt_wakeup.h
 rename arch/x86/kernel/{acpi => }/madt_playdead.S (100%)
 rename arch/x86/kernel/{acpi => }/madt_wakeup.c (87%)


base-commit: 9ebdc7589cbb5c976e6c8807cbe13f263d70d32c
-- 
2.25.1

[PATCH 1/7] x86/acpi: Move ACPI MADT wakeup to generic code

[Yunhong Jiang]

In order to support the ACPI mailbox wakeup in device tree, move the MADT
wakeup code out of the acpi directory, so that both ACPI and device tree
can use it.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 MAINTAINERS                                | 2 ++
 arch/x86/kernel/Makefile                   | 1 +
 arch/x86/kernel/acpi/Makefile              | 1 -
 arch/x86/kernel/{acpi => }/madt_playdead.S | 0
 arch/x86/kernel/{acpi => }/madt_wakeup.c   | 0
 5 files changed, 3 insertions(+), 1 deletion(-)
 rename arch/x86/kernel/{acpi => }/madt_playdead.S (100%)
 rename arch/x86/kernel/{acpi => }/madt_wakeup.c (100%)

diff --git a/MAINTAINERS b/MAINTAINERS
index eebc4fa2ab9a..6cc6d5c367df 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -289,6 +289,8 @@ F:	Documentation/ABI/testing/configfs-acpi
 F:	Documentation/ABI/testing/sysfs-bus-acpi
 F:	Documentation/firmware-guide/acpi/
 F:	arch/x86/kernel/acpi/
+F:	arch/x86/kernel/madt_playdead.S
+F:	arch/x86/kernel/madt_wakeup.c
 F:	arch/x86/pci/acpi.c
 F:	drivers/acpi/
 F:	drivers/pci/*/*acpi*
diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
index a847180836e4..b2de8a4698c3 100644
--- a/arch/x86/kernel/Makefile
+++ b/arch/x86/kernel/Makefile
@@ -155,4 +155,5 @@ ifeq ($(CONFIG_X86_64),y)
 
 	obj-$(CONFIG_MMCONF_FAM10H)	+= mmconf-fam10h_64.o
 	obj-y				+= vsmp_64.o
+	obj-$(CONFIG_ACPI_MADT_WAKEUP)	+= madt_wakeup.o madt_playdead.o
 endif
diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile
index 842a5f449404..fc17b3f136fe 100644
--- a/arch/x86/kernel/acpi/Makefile
+++ b/arch/x86/kernel/acpi/Makefile
@@ -4,7 +4,6 @@ obj-$(CONFIG_ACPI)		+= boot.o
 obj-$(CONFIG_ACPI_SLEEP)	+= sleep.o wakeup_$(BITS).o
 obj-$(CONFIG_ACPI_APEI)		+= apei.o
 obj-$(CONFIG_ACPI_CPPC_LIB)	+= cppc.o
-obj-$(CONFIG_ACPI_MADT_WAKEUP)	+= madt_wakeup.o madt_playdead.o
 
 ifneq ($(CONFIG_ACPI_PROCESSOR),)
 obj-y				+= cstate.o
diff --git a/arch/x86/kernel/acpi/madt_playdead.S b/arch/x86/kernel/madt_playdead.S
similarity index 100%
rename from arch/x86/kernel/acpi/madt_playdead.S
rename to arch/x86/kernel/madt_playdead.S
diff --git a/arch/x86/kernel/acpi/madt_wakeup.c b/arch/x86/kernel/madt_wakeup.c
similarity index 100%
rename from arch/x86/kernel/acpi/madt_wakeup.c
rename to arch/x86/kernel/madt_wakeup.c
-- 
2.25.1

[PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Yunhong Jiang]

Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 .../devicetree/bindings/x86/wakeup.yaml       | 41 +++++++++++++++++++
 1 file changed, 41 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml

diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml
new file mode 100644
index 000000000000..8af40dcdb592
--- /dev/null
+++ b/Documentation/devicetree/bindings/x86/wakeup.yaml
@@ -0,0 +1,41 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+
+$id: http://devicetree.org/schemas/x86/wakeup.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: x86 acpi wakeup mailbox
+
+description: |
+
+This document describes the "acpi-wakeup-mailbox" method for enabling secondary
+CPUs.
+
+The ACPI spec defines a mechanism to let the bootstrap processor wake up
+application processors with a mailbox. The "acpi-wakeup-mailbox" enable-method
+follows the "Multiprocessor Wakeup Mailbox Structure" defined in the ACPI
+spec[1].
+
+Since the ACPI mailbox structure is shared by all the CPUs, this enable method
+applies to all CPUs and should be defined in the "cpus" node and should not be
+defined on each "cpu" node.
+
+select: false
+
+properties:
+  wakeup-mailbox-addr:
+    $ref: /schemas/types.yaml#/definitions/uint64
+    description: |
+    The physical address of the wakeup mailbox data structure. The address must
+    meet the ACPI spec requirement, like be 4K bytes aligned and it should be in
+    the reserved memory.
+
+  wakeup-mailbox-version:
+    $ref: /schemas/types.yaml#/definitions/uint64
+    description: |
+    The MailBoxVersion defined in the ACPI spec that this binding follows.
+
+required:
+  - wakeup-mailbox-addr
+
+[1] https://uefi.org/htmlspecs/ACPI_Spec_6_4_html/05_ACPI_Software_Programming_Model/ACPI_Software_Programming_Model.html#multiprocessor-wakeup-structure
-- 
2.25.1

Re: [PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Rob Herring (Arm)]

On Tue, 06 Aug 2024 15:12:32 -0700, Yunhong Jiang wrote:
> Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.
> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> ---
>  .../devicetree/bindings/x86/wakeup.yaml       | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml
> 

My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:
./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: [error] syntax error: expected '<document start>', but found '<block mapping start>' (syntax)
./Documentation/devicetree/bindings/x86/wakeup.yaml:41:111: [warning] line too long (153 > 110 characters) (line-length)

dtschema/dtc warnings/errors:
make[2]: *** Deleting file 'Documentation/devicetree/bindings/x86/wakeup.example.dts'
Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start>
make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/x86/wakeup.example.dts] Error 1
make[2]: *** Waiting for unfinished jobs....
./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start>
/builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/x86/wakeup.yaml: ignoring, error parsing file
make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
make: *** [Makefile:240: __sub-make] Error 2

doc reference errors (make refcheckdocs):

See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240806221237.1634126-3-yunhong.jiang@linux.intel.com

The base for the series is generally the latest rc1. A different dependency
should be noted in *this* patch.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit after running the above command yourself. Note
that DT_SCHEMA_FILES can be set to your schema file to speed up checking
your schema. However, it must be unset to test all examples with your schema.

Re: [PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Yunhong Jiang]

On Tue, Aug 06, 2024 at 05:38:25PM -0600, Rob Herring (Arm) wrote:
> 
> On Tue, 06 Aug 2024 15:12:32 -0700, Yunhong Jiang wrote:
> > Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.
> > 
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > ---
> >  .../devicetree/bindings/x86/wakeup.yaml       | 41 +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml
> > 
> 
> My bot found errors running 'make dt_binding_check' on your patch:
> 
> yamllint warnings/errors:
> ./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: [error] syntax error: expected '<document start>', but found '<block mapping start>' (syntax)
> ./Documentation/devicetree/bindings/x86/wakeup.yaml:41:111: [warning] line too long (153 > 110 characters) (line-length)
> 
> dtschema/dtc warnings/errors:
> make[2]: *** Deleting file 'Documentation/devicetree/bindings/x86/wakeup.example.dts'
> Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start>
> make[2]: *** [Documentation/devicetree/bindings/Makefile:26: Documentation/devicetree/bindings/x86/wakeup.example.dts] Error 1
> make[2]: *** Waiting for unfinished jobs....
> ./Documentation/devicetree/bindings/x86/wakeup.yaml:4:1: did not find expected <document start>
> /builds/robherring/dt-review-ci/linux/Documentation/devicetree/bindings/x86/wakeup.yaml: ignoring, error parsing file
> make[1]: *** [/builds/robherring/dt-review-ci/linux/Makefile:1430: dt_binding_check] Error 2
> make: *** [Makefile:240: __sub-make] Error 2
> 
> doc reference errors (make refcheckdocs):
> 
> See https://patchwork.ozlabs.org/project/devicetree-bindings/patch/20240806221237.1634126-3-yunhong.jiang@linux.intel.com
> 
> The base for the series is generally the latest rc1. A different dependency
> should be noted in *this* patch.
> 
> If you already ran 'make dt_binding_check' and didn't see the above
> error(s), then make sure 'yamllint' is installed and dt-schema is up to
> date:
> 
> pip3 install dtschema --upgrade
> 
> Please check and re-submit after running the above command yourself. Note
> that DT_SCHEMA_FILES can be set to your schema file to speed up checking
> your schema. However, it must be unset to test all examples with your schema.
> 
I remember I ran 'make dt_binding_check' in the early development stage, but
possibly missed the error information. Will fix this.

Thank you
--jyh

Re: [PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Krzysztof Kozlowski]

On 07/08/2024 00:12, Yunhong Jiang wrote:
> Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.

We do not have bindings for ACPI. I think in the past it was mentioned
pretty clear - we do not care what ACPI has in the wild.

> 
> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> ---
>  .../devicetree/bindings/x86/wakeup.yaml       | 41 +++++++++++++++++++
>  1 file changed, 41 insertions(+)
>  create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml
> 
> diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml
> new file mode 100644
> index 000000000000..8af40dcdb592
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml
> @@ -0,0 +1,41 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +%YAML 1.2
> +
> +$id: http://devicetree.org/schemas/x86/wakeup.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#

This was absolutely never tested and does not look like proper bindings
file. This just does not work. Go to example-schema and use it as template.

NAK

Best regards,
Krzysztof

Re: [PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Yunhong Jiang]

On Wed, Aug 07, 2024 at 07:57:43AM +0200, Krzysztof Kozlowski wrote:
> On 07/08/2024 00:12, Yunhong Jiang wrote:
> > Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.
> 
> We do not have bindings for ACPI. I think in the past it was mentioned
> pretty clear - we do not care what ACPI has in the wild.

Thank you for review.
Can you please give a bit more information on "do not have bindings for ACPI"?
We don't put the ACPI table into the device tree, but reuse some existing ACPI
mailbox mechanism. Is this acceptable for you?

> 
> > 
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> > ---
> >  .../devicetree/bindings/x86/wakeup.yaml       | 41 +++++++++++++++++++
> >  1 file changed, 41 insertions(+)
> >  create mode 100644 Documentation/devicetree/bindings/x86/wakeup.yaml
> > 
> > diff --git a/Documentation/devicetree/bindings/x86/wakeup.yaml b/Documentation/devicetree/bindings/x86/wakeup.yaml
> > new file mode 100644
> > index 000000000000..8af40dcdb592
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/x86/wakeup.yaml
> > @@ -0,0 +1,41 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +%YAML 1.2
> > +
> > +$id: http://devicetree.org/schemas/x86/wakeup.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> 
> This was absolutely never tested and does not look like proper bindings
> file. This just does not work. Go to example-schema and use it as template.
> 
> NAK
> 
> Best regards,
> Krzysztof
Oops, I used the example-schema but apparently did something wrong. Will have a
check.

--jyh
> 

Re: [PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Krzysztof Kozlowski]

On 07/08/2024 18:56, Yunhong Jiang wrote:
> On Wed, Aug 07, 2024 at 07:57:43AM +0200, Krzysztof Kozlowski wrote:
>> On 07/08/2024 00:12, Yunhong Jiang wrote:
>>> Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.
>>
>> We do not have bindings for ACPI. I think in the past it was mentioned
>> pretty clear - we do not care what ACPI has in the wild.
> 
> Thank you for review.
> Can you please give a bit more information on "do not have bindings for ACPI"?
> We don't put the ACPI table into the device tree, but reuse some existing ACPI
> mailbox mechanism. Is this acceptable for you?

I understood that rationale behind this patch is "ACPI" thus that reply.
This one sentence in commit msg is not helping. Entire binding
description speaks about ACPI, so yeah - I don't care what ACPI does.
Provide proper explanation/description of firmware or hardware, then
sure. But the patch saying ACPI is doing something, so bindings will be
doing the same is for me NAK. Whatever ACPI is doing is never a reason
alone to do the same in Devicetree.

Best regards,
Krzysztof

Re: [PATCH 2/7] dt-bindings: x86: Add ACPI wakeup mailbox

[Yunhong Jiang]

On Thu, Aug 08, 2024 at 09:41:16AM +0200, Krzysztof Kozlowski wrote:
> On 07/08/2024 18:56, Yunhong Jiang wrote:
> > On Wed, Aug 07, 2024 at 07:57:43AM +0200, Krzysztof Kozlowski wrote:
> >> On 07/08/2024 00:12, Yunhong Jiang wrote:
> >>> Add the binding to use the ACPI wakeup mailbox mechanism to bringup APs.
> >>
> >> We do not have bindings for ACPI. I think in the past it was mentioned
> >> pretty clear - we do not care what ACPI has in the wild.
> > 
> > Thank you for review.
> > Can you please give a bit more information on "do not have bindings for ACPI"?
> > We don't put the ACPI table into the device tree, but reuse some existing ACPI
> > mailbox mechanism. Is this acceptable for you?
> 
> I understood that rationale behind this patch is "ACPI" thus that reply.
> This one sentence in commit msg is not helping. Entire binding
> description speaks about ACPI, so yeah - I don't care what ACPI does.
> Provide proper explanation/description of firmware or hardware, then
> sure. But the patch saying ACPI is doing something, so bindings will be
> doing the same is for me NAK. Whatever ACPI is doing is never a reason
> alone to do the same in Devicetree.

Thank you for the explanation. I will make the description as ACPI independent.

> 
> Best regards,
> Krzysztof
> 
> 

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

[Yunhong Jiang]

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@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      | 47 ++++++++++++++++++++++++++++--
 5 files changed, 63 insertions(+), 4 deletions(-)
 create mode 100644 arch/x86/include/asm/madt_wakeup.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 6cc6d5c367df..de3eaa0fdaaa 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 02775a61b557..e92dfefba675 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..faabf51c1e8f
--- /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)
+int dtb_parse_mp_wake(u64 *wake_mailbox_paddr);
+#else
+static inline int dtb_parse_mp_wake(u64 *wake_mailbox_paddr)
+{
+	return -ENODEV;
+}
+#endif
+
+#endif /* __ASM_X86_MADT_WAKEUP_H */
diff --git a/arch/x86/kernel/madt_wakeup.c b/arch/x86/kernel/madt_wakeup.c
index 6cfe762be28b..f6795a6ab2d3 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,7 +124,7 @@ static int __init init_transition_pgtable(pgd_t *pgd)
 	return 0;
 }
 
-static int __init acpi_mp_setup_reset(u64 reset_vector)
+static int __init __maybe_unused acpi_mp_setup_reset(u64 reset_vector)
 {
 	struct x86_mapping_info info = {
 		.alloc_pgt_page = alloc_pgt_page,
@@ -226,7 +228,7 @@ static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
 	return 0;
 }
 
-static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
+static void __maybe_unused acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
 {
 	cpu_hotplug_disable_offlining();
 
@@ -248,6 +250,7 @@ static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake
 	mp_wake->mailbox_address = 0;
 }
 
+#ifdef CONFIG_ACPI
 int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
 			      const unsigned long end)
 {
@@ -290,3 +293,43 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
 
 	return 0;
 }
+#endif /* CONFIG_ACPI */
+
+#ifdef CONFIG_OF
+int __init dtb_parse_mp_wake(u64 *wake_mailbox_paddr)
+{
+	struct device_node *node;
+	u64 mailaddr;
+	int ret = 0;
+
+	node = of_find_node_by_path("/cpus");
+	if (!node)
+		return -ENODEV;
+
+	if (of_property_match_string(node, "enable-method",
+				     "acpi-wakeup-mailbox") < 0) {
+		pr_err("No acpi wakeup mailbox enable-method\n");
+		ret = -ENODEV;
+		goto done;
+	}
+
+	/*
+	 * No support to the MADT reset vector yet.
+	 */
+	cpu_hotplug_disable_offlining();
+
+	if (of_property_read_u64(node, "wakeup-mailbox-addr", &mailaddr)) {
+		pr_err("Invalid wakeup mailbox addr\n");
+		ret = -EINVAL;
+		goto done;
+	}
+	acpi_mp_wake_mailbox_paddr = mailaddr;
+	if (wake_mailbox_paddr)
+		*wake_mailbox_paddr = mailaddr;
+	pr_info("dt wakeup-mailbox: addr 0x%llx\n", mailaddr);
+	apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
+done:
+	of_node_put(node);
+	return ret;
+}
+#endif /* CONFIG_OF */
-- 
2.25.1

Re: [PATCH 3/7] x86/dt: Support the ACPI multiprocessor wakeup for device tree

[Thomas Gleixner]

On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
>  
> -static int __init acpi_mp_setup_reset(u64 reset_vector)
> +static int __init __maybe_unused acpi_mp_setup_reset(u64 reset_vector)
>  {
>  	struct x86_mapping_info info = {
>  		.alloc_pgt_page = alloc_pgt_page,
> @@ -226,7 +228,7 @@ static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
>  	return 0;
>  }
>  
> -static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake)
> +static void __maybe_unused acpi_mp_disable_offlining(struct
> acpi_madt_multiproc_wakeup *mp_wake)

Please move those functions into the #ifdef CONFIG_ACPI

>  {
>  	cpu_hotplug_disable_offlining();
>  
> @@ -248,6 +250,7 @@ static void acpi_mp_disable_offlining(struct acpi_madt_multiproc_wakeup *mp_wake
>  	mp_wake->mailbox_address = 0;
>  }
>  
> +#ifdef CONFIG_ACPI
>  int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
> +
> +#ifdef CONFIG_OF
> +int __init dtb_parse_mp_wake(u64 *wake_mailbox_paddr)

Why not returning the mailbox physical address and 0 on failure instead
of that pointer and a integer return value which is ignored at the call
site?

> +{
> +	struct device_node *node;
> +	u64 mailaddr;
> +	int ret = 0;
> +
> +	node = of_find_node_by_path("/cpus");
> +	if (!node)
> +		return -ENODEV;
> +
> +	if (of_property_match_string(node, "enable-method",
> +				     "acpi-wakeup-mailbox") < 0) {

Please use the 100 characters line width and spare the line break.

> +		pr_err("No acpi wakeup mailbox enable-method\n");
> +		ret = -ENODEV;
> +		goto done;
> +	}
> +
> +	/*
> +	 * No support to the MADT reset vector yet.

s/to/for/

Also a single line comment is sufficient for this.

> +	 */
> +	cpu_hotplug_disable_offlining();
> +
> +	if (of_property_read_u64(node, "wakeup-mailbox-addr", &mailaddr)) {
> +		pr_err("Invalid wakeup mailbox addr\n");
> +		ret = -EINVAL;
> +		goto done;
> +	}
> +	acpi_mp_wake_mailbox_paddr = mailaddr;
> +	if (wake_mailbox_paddr)
> +		*wake_mailbox_paddr = mailaddr;
> +	pr_info("dt wakeup-mailbox: addr 0x%llx\n", mailaddr);
> +	apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +done:

newline before the label please. Up there you waste 3 lines for a
trivial comment and here you make the code unreadable...

Thanks,

        tglx

[PATCH 4/7] x86/hyperv: Parse the ACPI wakeup mailbox

[Yunhong Jiang]

Parse the wakeup mailbox in the guest_late_init. Put it to the
guest_late_init, so that it will be invoked before hyperv_init() where
the mailbox address will be checked.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 arch/x86/include/asm/mshyperv.h | 3 +++
 arch/x86/kernel/cpu/mshyperv.c  | 2 ++
 drivers/hv/hv_common.c          | 8 ++++++++
 3 files changed, 13 insertions(+)

diff --git a/arch/x86/include/asm/mshyperv.h b/arch/x86/include/asm/mshyperv.h
index 390c4d13956d..5178b96c7fc9 100644
--- a/arch/x86/include/asm/mshyperv.h
+++ b/arch/x86/include/asm/mshyperv.h
@@ -10,6 +10,7 @@
 #include <asm/nospec-branch.h>
 #include <asm/paravirt.h>
 #include <asm/mshyperv.h>
+#include <asm/madt_wakeup.h>
 
 /*
  * Hyper-V always provides a single IO-APIC at this MMIO address.
@@ -49,6 +50,8 @@ extern u64 hv_current_partition_id;
 
 extern union hv_ghcb * __percpu *hv_ghcb_pg;
 
+extern u64 wakeup_mailbox_addr;
+
 bool hv_isolation_type_snp(void);
 bool hv_isolation_type_tdx(void);
 u64 hv_tdx_hypercall(u64 control, u64 param1, u64 param2);
diff --git a/arch/x86/kernel/cpu/mshyperv.c b/arch/x86/kernel/cpu/mshyperv.c
index 3d4237f27569..f6b727b4bd0b 100644
--- a/arch/x86/kernel/cpu/mshyperv.c
+++ b/arch/x86/kernel/cpu/mshyperv.c
@@ -43,6 +43,8 @@ struct ms_hyperv_info ms_hyperv;
 bool hyperv_paravisor_present __ro_after_init;
 EXPORT_SYMBOL_GPL(hyperv_paravisor_present);
 
+u64 wakeup_mailbox_addr;
+
 #if IS_ENABLED(CONFIG_HYPERV)
 static inline unsigned int hv_get_nested_msr(unsigned int reg)
 {
diff --git a/drivers/hv/hv_common.c b/drivers/hv/hv_common.c
index 9c452bfbd571..08b9f4a39763 100644
--- a/drivers/hv/hv_common.c
+++ b/drivers/hv/hv_common.c
@@ -365,6 +365,14 @@ void __init ms_hyperv_late_init(void)
 	u8 *randomdata;
 	u32 length, i;
 
+	/*
+	 * Parse the ACPI wakeup structure information from device tree.
+	 * Currently TDX VTL2 guest only.
+	 */
+#ifdef CONFIG_X86_64
+	dtb_parse_mp_wake(&wakeup_mailbox_addr);
+#endif
+
 	/*
 	 * Seed the Linux random number generator with entropy provided by
 	 * the Hyper-V host in ACPI table OEM0.
-- 
2.25.1

[PATCH 5/7] x86/hyperv: Mark ACPI wakeup mailbox page as private

[Yunhong Jiang]

The ACPI wakeup mailbox is accessed by the OS and the firmware, both are
in the guest's context, instead of the hypervisor/VMM context. Mark the
address private explicitly.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 arch/x86/hyperv/hv_vtl.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index 04775346369c..bfe54afcdf1d 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -22,12 +22,28 @@ static bool __init hv_vtl_msi_ext_dest_id(void)
 	return true;
 }
 
+/*
+ * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
+ * guest's context, instead of the hypervisor/VMM context.
+ */
+static bool hv_is_private_mmio_tdx(u64 addr)
+{
+	if (wakeup_mailbox_addr && (addr >= wakeup_mailbox_addr &&
+	    addr < (wakeup_mailbox_addr + PAGE_SIZE)))
+		return true;
+	return false;
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
 
-	x86_platform.realmode_reserve = x86_init_noop;
-	x86_platform.realmode_init = x86_init_noop;
+	if (wakeup_mailbox_addr) {
+		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
+	} else {
+		x86_platform.realmode_reserve = x86_init_noop;
+		x86_platform.realmode_init = x86_init_noop;
+	}
 	x86_init.irqs.pre_vector_init = x86_init_noop;
 	x86_init.timers.timer_init = x86_init_noop;
 
-- 
2.25.1

Re: [PATCH 5/7] x86/hyperv: Mark ACPI wakeup mailbox page as private

[Thomas Gleixner]

On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> The ACPI wakeup mailbox is accessed by the OS and the firmware, both are
> in the guest's context, instead of the hypervisor/VMM context. Mark the
> address private explicitly.

This lacks information why the realmode area must be reserved and
initialized, which is what the change is doing implicitely.

> Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
>  
> +/*
> + * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
> + * guest's context, instead of the hypervisor/VMM context.
> + */
> +static bool hv_is_private_mmio_tdx(u64 addr)
> +{
> +	if (wakeup_mailbox_addr && (addr >= wakeup_mailbox_addr &&
> +	    addr < (wakeup_mailbox_addr + PAGE_SIZE)))
> +		return true;
> +	return false;
> +}

static inline bool within_page(u64 addr, u64 start)
{
	return addr >= start && addr < (start + PAGE_SIZE);
}

static bool hv_is_private_mmio_tdx(u64 addr)
{
        return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr)
}

Hmm?

> +
>  void __init hv_vtl_init_platform(void)
>  {
>  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
>  
> -	x86_platform.realmode_reserve = x86_init_noop;
> -	x86_platform.realmode_init = x86_init_noop;
> +	if (wakeup_mailbox_addr) {

Wants a comment vs. realmode here.

> +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> +	} else {
> +		x86_platform.realmode_reserve = x86_init_noop;
> +		x86_platform.realmode_init = x86_init_noop;
> +	}
>  	x86_init.irqs.pre_vector_init = x86_init_noop;
>  	x86_init.timers.timer_init = x86_init_noop;

Thanks,

        tglx

Re: [PATCH 5/7] x86/hyperv: Mark ACPI wakeup mailbox page as private

[Yunhong Jiang]

On Wed, Aug 07, 2024 at 06:59:28PM +0200, Thomas Gleixner wrote:
> On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> > The ACPI wakeup mailbox is accessed by the OS and the firmware, both are
> > in the guest's context, instead of the hypervisor/VMM context. Mark the
> > address private explicitly.
> 
> This lacks information why the realmode area must be reserved and
> initialized, which is what the change is doing implicitely.
> 
> > Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
> >  
> > +/*
> > + * The ACPI wakeup mailbox are accessed by the OS and the BIOS, both are in the
> > + * guest's context, instead of the hypervisor/VMM context.
> > + */
> > +static bool hv_is_private_mmio_tdx(u64 addr)
> > +{
> > +	if (wakeup_mailbox_addr && (addr >= wakeup_mailbox_addr &&
> > +	    addr < (wakeup_mailbox_addr + PAGE_SIZE)))
> > +		return true;
> > +	return false;
> > +}
> 
> static inline bool within_page(u64 addr, u64 start)
> {
> 	return addr >= start && addr < (start + PAGE_SIZE);
> }
> 
> static bool hv_is_private_mmio_tdx(u64 addr)
> {
>         return wakeup_mailbox_addr && within_page(addr, wakeup_mailbox_addr)
> }
> 
> Hmm?
> 
> > +
> >  void __init hv_vtl_init_platform(void)
> >  {
> >  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> >  
> > -	x86_platform.realmode_reserve = x86_init_noop;
> > -	x86_platform.realmode_init = x86_init_noop;
> > +	if (wakeup_mailbox_addr) {
> 
> Wants a comment vs. realmode here.

Sorry for the confusion. This patch is not related to real mode. This change is
similar to 88e378d400fa0544d51cf62037e7774d8a4b4379. Current code maps MMIO
devices as shared (decrypted) by default in a confidential computing VM. But the
wakeup mailbox must be accessed as private (encrypted) because all the access to
it are encrypted.

It's my fault to leave the realmode_reserve/realmode_init change here and cause
this confusion. Originally this patch and the real mode patch were included in
one patch. When I splitted them, I wrongly left this change here. Will put it to
the realmode patch on my next submission.

Will address all your other comments on my next submission.

Thank you
--jyh

> 
> > +		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> > +	} else {
> > +		x86_platform.realmode_reserve = x86_init_noop;
> > +		x86_platform.realmode_init = x86_init_noop;
> > +	}
> >  	x86_init.irqs.pre_vector_init = x86_init_noop;
> >  	x86_init.timers.timer_init = x86_init_noop;
> 
> Thanks,
> 
>         tglx

[PATCH 6/7] x86/hyperv: Reserve real mode when ACPI wakeup mailbox is available

[Yunhong Jiang]

The trampoline_start64 is utilized when the BSP wakes up the APs through
ACPI wakeup mailbox mechanism. Because trampoline_start64 is currently
part of the real model startup code, the real mode memory need to be
reserved.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 arch/x86/hyperv/hv_vtl.c | 20 +++++++++++++++++++-
 1 file changed, 19 insertions(+), 1 deletion(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index bfe54afcdf1d..a1eb5548bd4d 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -13,6 +13,7 @@
 #include <asm/mshyperv.h>
 #include <asm/realmode.h>
 #include <../kernel/smpboot.h>
+#include <linux/memblock.h>
 
 extern struct boot_params boot_params;
 static struct real_mode_header hv_vtl_real_mode_header;
@@ -34,12 +35,28 @@ static bool hv_is_private_mmio_tdx(u64 addr)
 	return false;
 }
 
+static void __init hv_reserve_real_mode(void)
+{
+	phys_addr_t mem;
+	size_t size = real_mode_size_needed();
+
+	/*
+	 * We only need the memory to be <4GB since the 64-bit trampoline goes
+	 * down to 32-bit mode.
+	 */
+	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, SZ_4G);
+	if (!mem)
+		panic("No sub-4G memory is available for the trampoline\n");
+	set_real_mode_mem(mem);
+}
+
 void __init hv_vtl_init_platform(void)
 {
 	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
 
 	if (wakeup_mailbox_addr) {
 		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
+		x86_platform.realmode_reserve = hv_reserve_real_mode;
 	} else {
 		x86_platform.realmode_reserve = x86_init_noop;
 		x86_platform.realmode_init = x86_init_noop;
@@ -259,7 +276,8 @@ int __init hv_vtl_early_init(void)
 		panic("XSAVE has to be disabled as it is not supported by this module.\n"
 			  "Please add 'noxsave' to the kernel command line.\n");
 
-	real_mode_header = &hv_vtl_real_mode_header;
+	if (!wakeup_mailbox_addr)
+		real_mode_header = &hv_vtl_real_mode_header;
 	apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
 
 	return 0;
-- 
2.25.1

Re: [PATCH 6/7] x86/hyperv: Reserve real mode when ACPI wakeup mailbox is available

[Thomas Gleixner]

On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> +static void __init hv_reserve_real_mode(void)
> +{
> +	phys_addr_t mem;
> +	size_t size = real_mode_size_needed();
> +
> +	/*
> +	 * We only need the memory to be <4GB since the 64-bit trampoline goes
> +	 * down to 32-bit mode.
> +	 */
> +	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, SZ_4G);
> +	if (!mem)
> +		panic("No sub-4G memory is available for the trampoline\n");
> +	set_real_mode_mem(mem);
> +}

We really don't need another copy of reserve_real_mode(). See uncompiled
patch below. It does not panic when the allocation fails, but why do you
want to panic in that case? If it's not there then the system boots with
a single CPU, so what.

>  void __init hv_vtl_init_platform(void)
>  {
>  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
>  
>  	if (wakeup_mailbox_addr) {
>  		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> +		x86_platform.realmode_reserve = hv_reserve_real_mode;
>  	} else {
>  		x86_platform.realmode_reserve = x86_init_noop;
>  		x86_platform.realmode_init = x86_init_noop;
> @@ -259,7 +276,8 @@ int __init hv_vtl_early_init(void)
>  		panic("XSAVE has to be disabled as it is not supported by this module.\n"
>  			  "Please add 'noxsave' to the kernel command line.\n");
>  
> -	real_mode_header = &hv_vtl_real_mode_header;
> +	if (!wakeup_mailbox_addr)
> +		real_mode_header = &hv_vtl_real_mode_header;

Why is that not suffient to be done in hv_vtl_init_platform() inside the
condition which clears x86_platform.realmode_reserve/init?

x86_platform.realmode_init() is invoked from an early initcall while
hv_vtl_init_platform() is called during early boot.

Thanks,

        tglx
---
--- a/arch/x86/include/asm/x86_init.h
+++ b/arch/x86/include/asm/x86_init.h
@@ -31,12 +31,18 @@ struct x86_init_mpparse {
  *				platform
  * @memory_setup:		platform specific memory setup
  * @dmi_setup:			platform specific DMI setup
+ * @realmode_limit:		platform specific address limit for the realmode trampoline
+ *				(default 1M)
+ * @reserve_bios:		platform specific address limit for reserving the BIOS area
+ *				(default 1M)
  */
 struct x86_init_resources {
 	void (*probe_roms)(void);
 	void (*reserve_resources)(void);
 	char *(*memory_setup)(void);
 	void (*dmi_setup)(void);
+	unsigned long realmode_limit;
+	unsigned long reserve_bios;
 };
 
 /**
--- a/arch/x86/kernel/x86_init.c
+++ b/arch/x86/kernel/x86_init.c
@@ -8,6 +8,7 @@
 #include <linux/ioport.h>
 #include <linux/export.h>
 #include <linux/pci.h>
+#include <linux/sizes.h>
 
 #include <asm/acpi.h>
 #include <asm/bios_ebda.h>
@@ -68,6 +69,8 @@ struct x86_init_ops x86_init __initdata
 		.reserve_resources	= reserve_standard_io_resources,
 		.memory_setup		= e820__memory_setup_default,
 		.dmi_setup		= dmi_setup,
+		.realmode_limit		= SZ_1M,
+		.reserve_bios		= SZ_1M,
 	},
 
 	.mpparse = {
--- a/arch/x86/realmode/init.c
+++ b/arch/x86/realmode/init.c
@@ -45,7 +45,7 @@ void load_trampoline_pgtable(void)
 
 void __init reserve_real_mode(void)
 {
-	phys_addr_t mem;
+	phys_addr_t mem, limit = x86_init.resources.realmode_limit;
 	size_t size = real_mode_size_needed();
 
 	if (!size)
@@ -54,17 +54,15 @@ void __init reserve_real_mode(void)
 	WARN_ON(slab_is_available());
 
 	/* Has to be under 1M so we can execute real-mode AP code. */
-	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, 1<<20);
+	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, limit);
 	if (!mem)
-		pr_info("No sub-1M memory is available for the trampoline\n");
+		pr_info("No memory below %lluM for the real-mode trampoline\n", limit >> 20);
 	else
 		set_real_mode_mem(mem);
 
-	/*
-	 * Unconditionally reserve the entire first 1M, see comment in
-	 * setup_arch().
-	 */
-	memblock_reserve(0, SZ_1M);
+	/* Reserve the entire first 1M, if enabled. See comment in setup_arch(). */
+	if (x86_init.resources.reserve_bios)
+		memblock_reserve(0, x86_init.resources.reserve_bios);
 }
 
 static void __init sme_sev_setup_real_mode(struct trampoline_header *th)

Re: [PATCH 6/7] x86/hyperv: Reserve real mode when ACPI wakeup mailbox is available

[Yunhong Jiang]

On Wed, Aug 07, 2024 at 07:33:26PM +0200, Thomas Gleixner wrote:
> On Tue, Aug 06 2024 at 15:12, Yunhong Jiang wrote:
> > +static void __init hv_reserve_real_mode(void)
> > +{
> > +	phys_addr_t mem;
> > +	size_t size = real_mode_size_needed();
> > +
> > +	/*
> > +	 * We only need the memory to be <4GB since the 64-bit trampoline goes
> > +	 * down to 32-bit mode.
> > +	 */
> > +	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, SZ_4G);
> > +	if (!mem)
> > +		panic("No sub-4G memory is available for the trampoline\n");
> > +	set_real_mode_mem(mem);
> > +}
> 
> We really don't need another copy of reserve_real_mode(). See uncompiled
> patch below. It does not panic when the allocation fails, but why do you
> want to panic in that case? If it's not there then the system boots with
> a single CPU, so what.

I created a separated patch on my v2 patch set with "Originally-by:" tag
following suggestion at
https://lore.kernel.org/lkml/20240711081317.GD4587@noisy.programming.kicks-ass.net/

Please let me know if that's not the appropriate solution.
 
> 
> >  void __init hv_vtl_init_platform(void)
> >  {
> >  	pr_info("Linux runs in Hyper-V Virtual Trust Level\n");
> >  
> >  	if (wakeup_mailbox_addr) {
> >  		x86_platform.hyper.is_private_mmio = hv_is_private_mmio_tdx;
> > +		x86_platform.realmode_reserve = hv_reserve_real_mode;
> >  	} else {
> >  		x86_platform.realmode_reserve = x86_init_noop;
> >  		x86_platform.realmode_init = x86_init_noop;
> > @@ -259,7 +276,8 @@ int __init hv_vtl_early_init(void)
> >  		panic("XSAVE has to be disabled as it is not supported by this module.\n"
> >  			  "Please add 'noxsave' to the kernel command line.\n");
> >  
> > -	real_mode_header = &hv_vtl_real_mode_header;
> > +	if (!wakeup_mailbox_addr)
> > +		real_mode_header = &hv_vtl_real_mode_header;
> 
> Why is that not suffient to be done in hv_vtl_init_platform() inside the
> condition which clears x86_platform.realmode_reserve/init?
> 
> x86_platform.realmode_init() is invoked from an early initcall while
> hv_vtl_init_platform() is called during early boot.

I created a separated patch for this change on my v2 patch set, since it's a
a separated logic change. I added a Suggested-by: tag to the patch, hope it's
ok.

Thanks
--jyh

> 
> Thanks,
> 
>         tglx
> ---
> --- a/arch/x86/include/asm/x86_init.h
> +++ b/arch/x86/include/asm/x86_init.h
> @@ -31,12 +31,18 @@ struct x86_init_mpparse {
>   *				platform
>   * @memory_setup:		platform specific memory setup
>   * @dmi_setup:			platform specific DMI setup
> + * @realmode_limit:		platform specific address limit for the realmode trampoline
> + *				(default 1M)
> + * @reserve_bios:		platform specific address limit for reserving the BIOS area
> + *				(default 1M)
>   */
>  struct x86_init_resources {
>  	void (*probe_roms)(void);
>  	void (*reserve_resources)(void);
>  	char *(*memory_setup)(void);
>  	void (*dmi_setup)(void);
> +	unsigned long realmode_limit;
> +	unsigned long reserve_bios;
>  };
>  
>  /**
> --- a/arch/x86/kernel/x86_init.c
> +++ b/arch/x86/kernel/x86_init.c
> @@ -8,6 +8,7 @@
>  #include <linux/ioport.h>
>  #include <linux/export.h>
>  #include <linux/pci.h>
> +#include <linux/sizes.h>
>  
>  #include <asm/acpi.h>
>  #include <asm/bios_ebda.h>
> @@ -68,6 +69,8 @@ struct x86_init_ops x86_init __initdata
>  		.reserve_resources	= reserve_standard_io_resources,
>  		.memory_setup		= e820__memory_setup_default,
>  		.dmi_setup		= dmi_setup,
> +		.realmode_limit		= SZ_1M,
> +		.reserve_bios		= SZ_1M,
>  	},
>  
>  	.mpparse = {
> --- a/arch/x86/realmode/init.c
> +++ b/arch/x86/realmode/init.c
> @@ -45,7 +45,7 @@ void load_trampoline_pgtable(void)
>  
>  void __init reserve_real_mode(void)
>  {
> -	phys_addr_t mem;
> +	phys_addr_t mem, limit = x86_init.resources.realmode_limit;
>  	size_t size = real_mode_size_needed();
>  
>  	if (!size)
> @@ -54,17 +54,15 @@ void __init reserve_real_mode(void)
>  	WARN_ON(slab_is_available());
>  
>  	/* Has to be under 1M so we can execute real-mode AP code. */
> -	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, 1<<20);
> +	mem = memblock_phys_alloc_range(size, PAGE_SIZE, 0, limit);
>  	if (!mem)
> -		pr_info("No sub-1M memory is available for the trampoline\n");
> +		pr_info("No memory below %lluM for the real-mode trampoline\n", limit >> 20);
>  	else
>  		set_real_mode_mem(mem);
>  
> -	/*
> -	 * Unconditionally reserve the entire first 1M, see comment in
> -	 * setup_arch().
> -	 */
> -	memblock_reserve(0, SZ_1M);
> +	/* Reserve the entire first 1M, if enabled. See comment in setup_arch(). */
> +	if (x86_init.resources.reserve_bios)
> +		memblock_reserve(0, x86_init.resources.reserve_bios);
>  }
>  
>  static void __init sme_sev_setup_real_mode(struct trampoline_header *th)

[PATCH 7/7] x86/hyperv: Use the ACPI wakeup mailbox for VTL2 guests when available

[Yunhong Jiang]

The VTL2 in a TDX guest can boot utilizing the device tree instead of
ACPI tables. When the ACPI wakeup mailbox is present in device tree,
don't overwrite wakeup_secondary_cpu_64 so that the acpi_wakeup_cpu will
be used to bring up the APs.

Signed-off-by: Yunhong Jiang <yunhong.jiang@linux.intel.com>
---
 arch/x86/hyperv/hv_vtl.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
index a1eb5548bd4d..132d05fd9136 100644
--- a/arch/x86/hyperv/hv_vtl.c
+++ b/arch/x86/hyperv/hv_vtl.c
@@ -276,9 +276,10 @@ int __init hv_vtl_early_init(void)
 		panic("XSAVE has to be disabled as it is not supported by this module.\n"
 			  "Please add 'noxsave' to the kernel command line.\n");
 
-	if (!wakeup_mailbox_addr)
+	if (!wakeup_mailbox_addr) {
 		real_mode_header = &hv_vtl_real_mode_header;
-	apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
+		apic_update_callback(wakeup_secondary_cpu_64, hv_vtl_wakeup_secondary_cpu);
+	}
 
 	return 0;
 }
-- 
2.25.1