Re: [PATCH RFC 04/17] acpi: Do not return struct iommu_ops from acpi_iommu_configure_id()
On Fri, Nov 3, 2023 at 5:45 PM Jason Gunthorpe wrote:
>
> Nothing needs this pointer. Return a normal error code with the usual
> IOMMU semantic that ENODEV means 'there is no IOMMU driver'.
>
> Signed-off-by: Jason Gunthorpe
Acked-by: Rafael J. Wysocki
> ---
> drivers/acpi/scan.c | 24 +++-
> 1 file changed, 15 insertions(+), 9 deletions(-)
>
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index a6891ad0ceee2c..fbabde001a23a2 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -1562,8 +1562,7 @@ static inline const struct iommu_ops
> *acpi_iommu_fwspec_ops(struct device *dev)
> return fwspec ? fwspec->ops : NULL;
> }
>
> -static const struct iommu_ops *acpi_iommu_configure_id(struct device *dev,
> - const u32 *id_in)
> +static int acpi_iommu_configure_id(struct device *dev, const u32 *id_in)
> {
> int err;
> const struct iommu_ops *ops;
> @@ -1574,7 +1573,7 @@ static const struct iommu_ops
> *acpi_iommu_configure_id(struct device *dev,
> */
> ops = acpi_iommu_fwspec_ops(dev);
> if (ops)
> - return ops;
> + return 0;
>
> err = iort_iommu_configure_id(dev, id_in);
> if (err && err != -EPROBE_DEFER)
> @@ -1589,12 +1588,14 @@ static const struct iommu_ops
> *acpi_iommu_configure_id(struct device *dev,
>
> /* Ignore all other errors apart from EPROBE_DEFER */
> if (err == -EPROBE_DEFER) {
> - return ERR_PTR(err);
> + return err;
> } else if (err) {
> dev_dbg(dev, "Adding to IOMMU failed: %d\n", err);
> - return NULL;
> + return -ENODEV;
> }
> - return acpi_iommu_fwspec_ops(dev);
> + if (!acpi_iommu_fwspec_ops(dev))
> + return -ENODEV;
> + return 0;
> }
>
> #else /* !CONFIG_IOMMU_API */
> @@ -1623,7 +1624,7 @@ static const struct iommu_ops
> *acpi_iommu_configure_id(struct device *dev,
> int acpi_dma_configure_id(struct device *dev, enum dev_dma_attr attr,
> const u32 *input_id)
> {
> - const struct iommu_ops *iommu;
> + int ret;
>
> if (attr == DEV_DMA_NOT_SUPPORTED) {
> set_dma_ops(dev, &dma_dummy_ops);
> @@ -1632,10 +1633,15 @@ int acpi_dma_configure_id(struct device *dev, enum
> dev_dma_attr attr,
>
> acpi_arch_dma_setup(dev);
>
> - iommu = acpi_iommu_configure_id(dev, input_id);
> - if (PTR_ERR(iommu) == -EPROBE_DEFER)
> + ret = acpi_iommu_configure_id(dev, input_id);
> + if (ret == -EPROBE_DEFER)
> return -EPROBE_DEFER;
>
> + /*
> +* Historically this routine doesn't fail driver probing due to errors
> +* in acpi_iommu_configure()
> +*/
> +
> arch_setup_dma_ops(dev, 0, U64_MAX, attr == DEV_DMA_COHERENT);
>
> return 0;
> --
> 2.42.0
>
Re: [PATCH RFC 10/17] acpi: Do not use dev->iommu within acpi_iommu_configure()
On Fri, Nov 3, 2023 at 5:45 PM Jason Gunthorpe wrote:
>
> This call chain is using dev->iommu->fwspec to pass around the fwspec
> between the three parts (acpi_iommu_configure(), acpi_iommu_fwspec_init(),
> iommu_probe_device()).
>
> However there is no locking around the accesses to dev->iommu, so this is
> all racy.
>
> Allocate a clean, local, fwspec at the start of acpu_iommu_configure(),
> pass it through all functions on the stack to fill it with data, and
> finally pass it into iommu_probe_device_fwspec() which will load it into
> dev->iommu under a lock.
>
> Signed-off-by: Jason Gunthorpe
Acked-by: Rafael J. Wysocki
> ---
> drivers/acpi/arm64/iort.c | 39 -
> drivers/acpi/scan.c | 89 ++-
> drivers/acpi/viot.c | 44 ++-
> drivers/iommu/iommu.c | 5 +--
> include/acpi/acpi_bus.h | 8 ++--
> include/linux/acpi_iort.h | 3 +-
> include/linux/acpi_viot.h | 5 ++-
> include/linux/iommu.h | 2 +
> 8 files changed, 97 insertions(+), 98 deletions(-)
>
> diff --git a/drivers/acpi/arm64/iort.c b/drivers/acpi/arm64/iort.c
> index 6496ff5a6ba20d..accd01dcfe93f5 100644
> --- a/drivers/acpi/arm64/iort.c
> +++ b/drivers/acpi/arm64/iort.c
> @@ -1218,10 +1218,9 @@ static bool iort_pci_rc_supports_ats(struct
> acpi_iort_node *node)
> return pci_rc->ats_attribute & ACPI_IORT_ATS_SUPPORTED;
> }
>
> -static int iort_iommu_xlate(struct device *dev, struct acpi_iort_node *node,
> - u32 streamid)
> +static int iort_iommu_xlate(struct iommu_fwspec *fwspec, struct device *dev,
> + struct acpi_iort_node *node, u32 streamid)
> {
> - const struct iommu_ops *ops;
> struct fwnode_handle *iort_fwnode;
>
> if (!node)
> @@ -1239,17 +1238,14 @@ static int iort_iommu_xlate(struct device *dev,
> struct acpi_iort_node *node,
> * in the kernel or not, defer the IOMMU configuration
> * or just abort it.
> */
> - ops = iommu_ops_from_fwnode(iort_fwnode);
> - if (!ops)
> - return iort_iommu_driver_enabled(node->type) ?
> - -EPROBE_DEFER : -ENODEV;
> -
> - return acpi_iommu_fwspec_init(dev, streamid, iort_fwnode, ops);
> + return acpi_iommu_fwspec_init(fwspec, dev, streamid, iort_fwnode,
> + iort_iommu_driver_enabled(node->type));
> }
>
> struct iort_pci_alias_info {
> struct device *dev;
> struct acpi_iort_node *node;
> + struct iommu_fwspec *fwspec;
> };
>
> static int iort_pci_iommu_init(struct pci_dev *pdev, u16 alias, void *data)
> @@ -1260,7 +1256,7 @@ static int iort_pci_iommu_init(struct pci_dev *pdev,
> u16 alias, void *data)
>
> parent = iort_node_map_id(info->node, alias, &streamid,
> IORT_IOMMU_TYPE);
> - return iort_iommu_xlate(info->dev, parent, streamid);
> + return iort_iommu_xlate(info->fwspec, info->dev, parent, streamid);
> }
>
> static void iort_named_component_init(struct device *dev,
> @@ -1280,7 +1276,8 @@ static void iort_named_component_init(struct device
> *dev,
> dev_warn(dev, "Could not add device properties\n");
> }
>
> -static int iort_nc_iommu_map(struct device *dev, struct acpi_iort_node *node)
> +static int iort_nc_iommu_map(struct iommu_fwspec *fwspec, struct device *dev,
> +struct acpi_iort_node *node)
> {
> struct acpi_iort_node *parent;
> int err = -ENODEV, i = 0;
> @@ -1293,13 +1290,13 @@ static int iort_nc_iommu_map(struct device *dev,
> struct acpi_iort_node *node)
>i++);
>
> if (parent)
> - err = iort_iommu_xlate(dev, parent, streamid);
> + err = iort_iommu_xlate(fwspec, dev, parent, streamid);
> } while (parent && !err);
>
> return err;
> }
>
> -static int iort_nc_iommu_map_id(struct device *dev,
> +static int iort_nc_iommu_map_id(struct iommu_fwspec *fwspec, struct device
> *dev,
> struct acpi_iort_node *node,
> const u32 *in_id)
> {
> @@ -1308,7 +1305,7 @@ static int iort_nc_iommu_map_id(struct device *dev,
>
> parent = iort_node_map_id(node, *in_id, &streamid, IORT_IOMMU_TYPE);
> if (parent)
> - return iort_iommu_xlate(dev, parent, streamid);
> + return iort_iommu_xlate(fwspec, dev, parent, streamid);
>
>
Re: [PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Thu, May 16, 2024 at 7:35 PM Steven Rostedt wrote: > > From: "Steven Rostedt (Google)" > > [ >This is a treewide change. I will likely re-create this patch again in >the second week of the merge window of v6.10 and submit it then. Hoping >to keep the conflicts that it will cause to a minimum. > ] > > With the rework of how the __string() handles dynamic strings where it > saves off the source string in field in the helper structure[1], the > assignment of that value to the trace event field is stored in the helper > value and does not need to be passed in again. > > This means that with: > > __string(field, mystring) > > Which use to be assigned with __assign_str(field, mystring), no longer > needs the second parameter and it is unused. With this, __assign_str() > will now only get a single parameter. > > There's over 700 users of __assign_str() and because coccinelle does not > handle the TRACE_EVENT() macro I ended up using the following sed script: > > git grep -l __assign_str | while read a ; do > sed -e 's/\(__assign_str([^,]*[^ ,]\) *,[^;]*/\1)/' $a > /tmp/test-file; > mv /tmp/test-file $a; > done > > I then searched for __assign_str() that did not end with ';' as those > were multi line assignments that the sed script above would fail to catch. > > Note, the same updates will need to be done for: > > __assign_str_len() > __assign_rel_str() > __assign_rel_str_len() > > I tested this with both an allmodconfig and an allyesconfig (build only for > both). > > [1] > https://lore.kernel.org/linux-trace-kernel/2024011442.634192...@goodmis.org/ > > Cc: Masami Hiramatsu > Cc: Mathieu Desnoyers > Cc: Linus Torvalds > Cc: Julia Lawall > Signed-off-by: Steven Rostedt (Google) Acked-by: Rafael J. Wysocki # for thermal
Re: [PATCH hyperv-next v6 10/11] ACPI: irq: Introduce acpi_get_gsi_dispatcher()
On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel wrote:
>
> Using acpi_irq_create_hierarchy() in the cases where the code
> also handles OF leads to code duplication as the ACPI subsystem
> doesn't provide means to compute the IRQ domain parent whereas
> the OF does.
>
> Introduce acpi_get_gsi_dispatcher() so that the drivers relying
> on both ACPI and OF may use irq_domain_create_hierarchy() in the
> common code paths.
>
> No functional changes.
>
> Signed-off-by: Roman Kisel
> Reviewed-by: Michael Kelley
This basically looks OK to me except for a couple of coding style
related nits below.
> ---
> drivers/acpi/irq.c | 15 +--
> include/linux/acpi.h | 5 -
> 2 files changed, 17 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/acpi/irq.c b/drivers/acpi/irq.c
> index 1687483ff319..8eb09e45e5c5 100644
> --- a/drivers/acpi/irq.c
> +++ b/drivers/acpi/irq.c
> @@ -12,7 +12,7 @@
>
> enum acpi_irq_model_id acpi_irq_model;
>
> -static struct fwnode_handle *(*acpi_get_gsi_domain_id)(u32 gsi);
> +static acpi_gsi_domain_disp_fn acpi_get_gsi_domain_id;
> static u32 (*acpi_gsi_to_irq_fallback)(u32 gsi);
>
> /**
> @@ -307,12 +307,23 @@ EXPORT_SYMBOL_GPL(acpi_irq_get);
> * for a given GSI
> */
> void __init acpi_set_irq_model(enum acpi_irq_model_id model,
> - struct fwnode_handle *(*fn)(u32))
Please retain the indentation here and analogously below.
> + acpi_gsi_domain_disp_fn fn)
> {
> acpi_irq_model = model;
> acpi_get_gsi_domain_id = fn;
> }
>
> +/**
> + * acpi_get_gsi_dispatcher - Returns dispatcher function that
> + * computes the domain fwnode for a
> + * given GSI.
> + */
I would format this kerneldoc comment a bit differently:
/*
* acpi_get_gsi_dispatcher() - Get the GSI dispatcher function
*
* Return the dispatcher function that computes the domain fwnode for
a given GSI.
*/
> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void)
> +{
> + return acpi_get_gsi_domain_id;
> +}
> +EXPORT_SYMBOL_GPL(acpi_get_gsi_dispatcher);
> +
> /**
> * acpi_set_gsi_to_irq_fallback - Register a GSI transfer
> * callback to fallback to arch specified implementation.
> diff --git a/include/linux/acpi.h b/include/linux/acpi.h
> index 4e495b29c640..abc51288e867 100644
> --- a/include/linux/acpi.h
> +++ b/include/linux/acpi.h
> @@ -336,8 +336,11 @@ int acpi_register_gsi (struct device *dev, u32 gsi, int
> triggering, int polarity
> int acpi_gsi_to_irq (u32 gsi, unsigned int *irq);
> int acpi_isa_irq_to_gsi (unsigned isa_irq, u32 *gsi);
>
> +typedef struct fwnode_handle *(*acpi_gsi_domain_disp_fn)(u32);
> +
> void acpi_set_irq_model(enum acpi_irq_model_id model,
> - struct fwnode_handle *(*)(u32));
> + acpi_gsi_domain_disp_fn fn);
> +acpi_gsi_domain_disp_fn acpi_get_gsi_dispatcher(void);
> void acpi_set_gsi_to_irq_fallback(u32 (*)(u32));
>
> struct irq_domain *acpi_irq_create_hierarchy(unsigned int flags,
> --
With the above addressed, please feel free to add
Acked-by: Rafael J. Wysocki
to the patch and route it along with the rest of the series.
Thanks!
Re: [PATCH hyperv-next v6 11/11] PCI: hv: Get vPCI MSI IRQ domain from DeviceTree
On Sat, Mar 15, 2025 at 1:19 AM Roman Kisel wrote:
>
> The hyperv-pci driver uses ACPI for MSI IRQ domain configuration on
> arm64. It won't be able to do that in the VTL mode where only DeviceTree
> can be used.
>
> Update the hyperv-pci driver to get vPCI MSI IRQ domain in the DeviceTree
> case, too.
>
> Signed-off-by: Roman Kisel
> ---
> drivers/pci/controller/pci-hyperv.c | 73 ++---
> 1 file changed, 67 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/pci/controller/pci-hyperv.c
> b/drivers/pci/controller/pci-hyperv.c
> index 6084b38bdda1..cbff19e8a07c 100644
> --- a/drivers/pci/controller/pci-hyperv.c
> +++ b/drivers/pci/controller/pci-hyperv.c
> @@ -50,6 +50,7 @@
> #include
> #include
> #include
> +#include
> #include
>
> /*
> @@ -817,9 +818,17 @@ static int hv_pci_vec_irq_gic_domain_alloc(struct
> irq_domain *domain,
> int ret;
>
> fwspec.fwnode = domain->parent->fwnode;
> - fwspec.param_count = 2;
> - fwspec.param[0] = hwirq;
> - fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> + if (is_of_node(fwspec.fwnode)) {
> + /* SPI lines for OF translations start at offset 32 */
> + fwspec.param_count = 3;
> + fwspec.param[0] = 0;
> + fwspec.param[1] = hwirq - 32;
> + fwspec.param[2] = IRQ_TYPE_EDGE_RISING;
> + } else {
> + fwspec.param_count = 2;
> + fwspec.param[0] = hwirq;
> + fwspec.param[1] = IRQ_TYPE_EDGE_RISING;
> + }
>
> ret = irq_domain_alloc_irqs_parent(domain, virq, 1, &fwspec);
> if (ret)
> @@ -887,10 +896,47 @@ static const struct irq_domain_ops hv_pci_domain_ops = {
> .activate = hv_pci_vec_irq_domain_activate,
> };
>
> +#ifdef CONFIG_OF
> +
> +static struct irq_domain *hv_pci_of_irq_domain_parent(void)
> +{
> + struct device_node *parent;
> + struct irq_domain *domain;
> +
> + parent = of_irq_find_parent(hv_get_vmbus_root_device()->of_node);
> + if (!parent)
> + return NULL;
> + domain = irq_find_host(parent);
> + of_node_put(parent);
> +
> + return domain;
> +}
> +
> +#endif
> +
> +#ifdef CONFIG_ACPI
> +
> +static struct irq_domain *hv_pci_acpi_irq_domain_parent(void)
> +{
> + struct irq_domain *domain;
> + acpi_gsi_domain_disp_fn gsi_domain_disp_fn;
> +
> + if (acpi_irq_model != ACPI_IRQ_MODEL_GIC)
> + return NULL;
> + gsi_domain_disp_fn = acpi_get_gsi_dispatcher();
> + if (!gsi_domain_disp_fn)
> + return NULL;
> + return irq_find_matching_fwnode(gsi_domain_disp_fn(0),
> +DOMAIN_BUS_ANY);
> +}
> +
> +#endif
> +
> static int hv_pci_irqchip_init(void)
> {
> static struct hv_pci_chip_data *chip_data;
> struct fwnode_handle *fn = NULL;
> + struct irq_domain *irq_domain_parent = NULL;
> int ret = -ENOMEM;
>
> chip_data = kzalloc(sizeof(*chip_data), GFP_KERNEL);
> @@ -907,9 +953,24 @@ static int hv_pci_irqchip_init(void)
> * way to ensure that all the corresponding devices are also gone and
> * no interrupts will be generated.
> */
> - hv_msi_gic_irq_domain = acpi_irq_create_hierarchy(0,
> HV_PCI_MSI_SPI_NR,
> - fn,
> &hv_pci_domain_ops,
> - chip_data);
> +#ifdef CONFIG_ACPI
> + if (!acpi_disabled)
> + irq_domain_parent = hv_pci_acpi_irq_domain_parent();
> +#endif
> +#if defined(CONFIG_OF)
Why don't you do
#ifdef CONFIG_OF
here for consistency?
> + if (!irq_domain_parent)
> + irq_domain_parent = hv_pci_of_irq_domain_parent();
> +#endif
> + if (!irq_domain_parent) {
> + WARN_ONCE(1, "Invalid firmware configuration for VMBus
> interrupts\n");
> + ret = -EINVAL;
> + goto free_chip;
> + }
> +
> + hv_msi_gic_irq_domain = irq_domain_create_hierarchy(
> + irq_domain_parent, 0, HV_PCI_MSI_SPI_NR,
> + fn, &hv_pci_domain_ops,
> + chip_data);
>
> if (!hv_msi_gic_irq_domain) {
> pr_err("Failed to create Hyper-V arm64 vPCI MSI IRQ
> domain\n");
> --
Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
On Sat, May 3, 2025 at 9:10 PM Ricardo Neri wrote: > > Add DeviceTree bindings for the wakeup mailbox used on Intel processors. > > x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert > followed by Start-Up IPI messages. The wakeup mailbox can be used when this > mechanism unavailable. > > The wakeup mailbox offers more control to the operating system to boot > secondary CPUs than a spin-table. It allows the reuse of same wakeup vector > for all CPUs while maintaining control over which CPUs to boot and when. > While it is possible to achieve the same level of control using a spin- > table, it would require to specify a separate cpu-release-addr for each > secondary CPU. > > Originally-by: Yunhong Jiang > Signed-off-by: Ricardo Neri > --- > Changes since v2: > - Implemented the mailbox as a reserved-memory node. Add to it a >`compatible` property. (Krzysztof) > - Explained the relationship between the mailbox and the `enable-mehod` >property of the CPU nodes. > - Expanded the documentation of the binding. > > Changes since v1: > - Added more details to the description of the binding. > - Added requirement a new requirement for cpu@N nodes to add an >`enable-method`. > --- > .../reserved-memory/intel,wakeup-mailbox.yaml | 87 +++ > 1 file changed, 87 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > > diff --git > a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > new file mode 100644 > index ..d97755b4673d > --- /dev/null > +++ > b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > @@ -0,0 +1,87 @@ > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > +%YAML 1.2 > +--- > +$id: http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml# > +$schema: http://devicetree.org/meta-schemas/core.yaml# > + > +title: Wakeup Mailbox for Intel processors > + > +description: | > + The Wakeup Mailbox provides a mechanism for the operating system to wake up > + secondary CPUs on Intel processors. It is an alternative to the INIT-!INIT- > + SIPI sequence used on most x86 systems. > + > + Firmware must define the enable-method property in the CPU nodes as > + "intel,wakeup-mailbox" to use the mailbox. > + > + Firmware implements the wakeup mailbox as a 4KB-aligned memory region of > size > + of 4KB. It is memory that the firmware reserves so that each secondary CPU > can > + have the operating system send a single message to them. The firmware is > + responsible for putting the secondary CPUs in a state to check the mailbox. > + > + The structure of the mailbox is as follows: > + > + Field Byte Byte Description > + Length Offset > + > -- > + Command 2 0Command to wake up the secondary CPU: > +0: Noop > +1: Wakeup: Jump to the wakeup_vector > +2-0x: Reserved: > + Reserved 2 2Must be 0. > + APIC_ID 4 4APIC ID of the secondary CPU to wake up. > + Wakeup_Vector8 8The wakeup address for the secondary CPU. > + ReservedForOs 2032 16Reserved for OS use. > + ReservedForFW 2048 2048Reserved for firmware use. > + > -- > + > + To wake up a secondary CPU, the operating system 1) prepares the wakeup > + routine; 2) populates the address of the wakeup routine address into the > + Wakeup_Vector field; 3) populates the APIC_ID field with the APIC ID of the > + secondary CPU; 4) writes Wakeup in the Command field. Upon receiving the > + Wakeup command, the secondary CPU acknowledges the command by writing Noop > in > + the Command field and jumps to the Wakeup_Vector. The operating system can > + send the next command only after the Command field is changed to Noop. > + > + The secondary CPU will no longer check the mailbox after waking up. The > + secondary CPU must ignore the command if its APIC_ID written in the mailbox > + does not match its own. > + > + When entering the Wakeup_Vector, interrupts must be disabled and 64-bit > + addressing mode must be enabled. Paging mode must be enabled. The virtual > + address of the Wakeup_Vector page must be equal to its physical address. > + Segment selectors are not used. This interface is defined in the ACPI specification and all of the above information is present there. Why are you copying it without acknowledging the source of it instead of just saying where this interface is defined and pointing to its definition? > + > +maintainers: > + - Ricardo Neri > + > +allOf: > + - $ref: r
Re: [PATCH v3 01/13] x86/acpi: Add a helper function to setup the wakeup mailbox
On Sat, May 3, 2025 at 9:10 PM Ricardo Neri
wrote:
>
> In preparation to move the functionality to wake secondary CPUs up out of
> the ACPI code, add a helper function that stores the physical address of
> the mailbox and updates the wakeup_secondary_cpu_64() APIC callback.
>
> There is a slight change in behavior: now the APIC callback is updated
> before configuring CPU hotplug offline behavior. This is fine as the APIC
> callback continues to be updated unconditionally, regardless of the
> restriction on CPU offlining.
>
> The wakeup mailbox is only supported for CONFIG_X86_64 and needed only with
> CONFIG_SMP=y.
>
> Signed-off-by: Ricardo Neri
> ---
> Changes since v2:
> - Introduced this patch.
>
> Changes since v1:
> - N/A
> ---
> arch/x86/include/asm/smp.h | 4
> arch/x86/kernel/acpi/madt_wakeup.c | 10 +++---
> 2 files changed, 11 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 0c1c68039d6f..3622951d2ee0 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -146,6 +146,10 @@ static inline struct cpumask *cpu_l2c_shared_mask(int
> cpu)
> return per_cpu(cpu_l2c_shared_map, cpu);
> }
>
> +#ifdef CONFIG_X86_64
> +void setup_mp_wakeup_mailbox(u64 addr);
> +#endif
The #ifdef is only necessary if you are going to provide an
alternative for builds in which the symbol is unset.
> +
> #else /* !CONFIG_SMP */
> #define wbinvd_on_cpu(cpu) wbinvd()
> static inline int wbinvd_on_all_cpus(void)
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
> b/arch/x86/kernel/acpi/madt_wakeup.c
> index f36f28405dcc..04de3db307de 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -227,7 +227,7 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers
> *header,
>
> acpi_table_print_madt_entry(&header->common);
>
> - acpi_mp_wake_mailbox_paddr = mp_wake->mailbox_address;
> + setup_mp_wakeup_mailbox(mp_wake->mailbox_address);
I'd prefer acpi_setup_mp_wakeup_mailbox().
>
> if (mp_wake->version >= ACPI_MADT_MP_WAKEUP_VERSION_V1 &&
> mp_wake->header.length >= ACPI_MADT_MP_WAKEUP_SIZE_V1) {
> @@ -243,7 +243,11 @@ int __init acpi_parse_mp_wake(union
> acpi_subtable_headers *header,
> acpi_mp_disable_offlining(mp_wake);
> }
>
> - apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> -
> return 0;
> }
> +
> +void __init setup_mp_wakeup_mailbox(u64 mailbox_paddr)
> +{
> + acpi_mp_wake_mailbox_paddr = mailbox_paddr;
> + apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +}
> --
> 2.43.0
>
>
Re: [PATCH v3 02/13] x86/acpi: Add a helper function to get a pointer to the wakeup mailbox
On Sat, May 3, 2025 at 9:10 PM Ricardo Neri
wrote:
>
> In preparation to move the functionality to wake secondary CPUs up out
> of the ACPI code, add a helper function to get a pointer to the mailbox.
>
> Use this helper function only in the portions of the code for which the
> variable acpi_mp_wake_mailbox will be out of scope once it is relocated
> out of the ACPI directory.
>
> The wakeup mailbox is only supported for CONFIG_X86_64 and needed only
> with CONFIG_SMP.
>
> Signed-off-by: Ricardo Neri
> ---
> Changes since v2:
> - Introduced this patch.
Have you considered merging it with the previous patch? They both do
analogous things.
> Changes since v1:
> - N/A
> ---
> arch/x86/include/asm/smp.h | 1 +
> arch/x86/kernel/acpi/madt_wakeup.c | 12 +---
> 2 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 3622951d2ee0..97bfbd0d24d4 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -148,6 +148,7 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
>
> #ifdef CONFIG_X86_64
> void setup_mp_wakeup_mailbox(u64 addr);
> +struct acpi_madt_multiproc_wakeup_mailbox *get_mp_wakeup_mailbox(void);
> #endif
>
> #else /* !CONFIG_SMP */
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
> b/arch/x86/kernel/acpi/madt_wakeup.c
> index 04de3db307de..6b9e41a24574 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -37,6 +37,7 @@ static void acpi_mp_play_dead(void)
>
> static void acpi_mp_cpu_die(unsigned int cpu)
> {
> + struct acpi_madt_multiproc_wakeup_mailbox *mailbox =
> get_mp_wakeup_mailbox();
I'd prefer acpi_get_mp_wakeup_mailbox().
> u32 apicid = per_cpu(x86_cpu_to_apicid, cpu);
> unsigned long timeout;
>
> @@ -46,13 +47,13 @@ static void acpi_mp_cpu_die(unsigned int cpu)
> *
> * BIOS has to clear 'command' field of the mailbox.
> */
> - acpi_mp_wake_mailbox->apic_id = apicid;
> - smp_store_release(&acpi_mp_wake_mailbox->command,
> + mailbox->apic_id = apicid;
> + smp_store_release(&mailbox->command,
> ACPI_MP_WAKE_COMMAND_TEST);
>
> /* Don't wait longer than a second. */
> timeout = USEC_PER_SEC;
> - while (READ_ONCE(acpi_mp_wake_mailbox->command) && --timeout)
> + while (READ_ONCE(mailbox->command) && --timeout)
> udelay(1);
>
> if (!timeout)
> @@ -251,3 +252,8 @@ void __init setup_mp_wakeup_mailbox(u64 mailbox_paddr)
> acpi_mp_wake_mailbox_paddr = mailbox_paddr;
> apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> }
> +
> +struct acpi_madt_multiproc_wakeup_mailbox *get_mp_wakeup_mailbox(void)
> +{
> + return acpi_mp_wake_mailbox;
> +}
> --
Re: [PATCH v3 03/13] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpboot.c
On Sat, May 3, 2025 at 9:10 PM Ricardo Neri
wrote:
>
> The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware that
> it wants to boot a secondary CPU using a mailbox as described in the
> Multiprocessor Wakeup Structure of the ACPI specification.
>
> The wakeup mailbox does not strictly require support from ACPI.
Well, except that it is defined by the ACPI specification.
> The platform firmware can implement a mailbox compatible in structure and
> operation and enumerate it using other mechanisms such a DeviceTree graph.
So is there a specification defining this mechanism?
It is generally not sufficient to put the code and DT bindings
unilaterally into the OS and expect the firmware to follow suit.
> Move the code used to setup and use the mailbox out of the ACPI
> directory to use it when support for ACPI is not available or needed.
I think that the code implementing interfaces defined by the ACPI
specification is not generic and so it should not be built when the
kernel is configured without ACPI support.
> No functional changes are intended.
>
> Originally-by: Yunhong Jiang
> Signed-off-by: Ricardo Neri
> ---
> Changes since v2:
> - Only move to smpboot.c the portions of the code that configure and
>use the mailbox. This also resolved the compile warnings about unused
>functions that Michael Kelley reported.
> - Edited the commit message for clarity.
>
> Changes since v1:
> - None.
> ---
> arch/x86/kernel/acpi/madt_wakeup.c | 75
> arch/x86/kernel/smpboot.c | 78 ++
> 2 files changed, 78 insertions(+), 75 deletions(-)
>
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
> b/arch/x86/kernel/acpi/madt_wakeup.c
> index 6b9e41a24574..15627f63f9f5 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -2,7 +2,6 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -15,12 +14,6 @@
> #include
> #include
>
> -/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> -static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> -
> -/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> -static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> -
> static u64 acpi_mp_pgd __ro_after_init;
> static u64 acpi_mp_reset_vector_paddr __ro_after_init;
>
> @@ -127,63 +120,6 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
> return 0;
> }
>
> -static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> -{
> - if (!acpi_mp_wake_mailbox_paddr) {
> - pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs.
> Booting with kexec?\n");
> - return -EOPNOTSUPP;
> - }
> -
> - /*
> -* Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> -*
> -* Wakeup of secondary CPUs is fully serialized in the core code.
> -* No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> -*/
> - if (!acpi_mp_wake_mailbox) {
> - acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> - sizeof(*acpi_mp_wake_mailbox),
> - MEMREMAP_WB);
> - }
> -
> - /*
> -* Mailbox memory is shared between the firmware and OS. Firmware will
> -* listen on mailbox command address, and once it receives the wakeup
> -* command, the CPU associated with the given apicid will be booted.
> -*
> -* The value of 'apic_id' and 'wakeup_vector' must be visible to the
> -* firmware before the wakeup command is visible. smp_store_release()
> -* ensures ordering and visibility.
> -*/
> - acpi_mp_wake_mailbox->apic_id = apicid;
> - acpi_mp_wake_mailbox->wakeup_vector = start_ip;
> - smp_store_release(&acpi_mp_wake_mailbox->command,
> - ACPI_MP_WAKE_COMMAND_WAKEUP);
> -
> - /*
> -* Wait for the CPU to wake up.
> -*
> -* The CPU being woken up is essentially in a spin loop waiting to be
> -* woken up. It should not take long for it wake up and acknowledge by
> -* zeroing out ->command.
> -*
> -* ACPI specification doesn't provide any guidance on how long kernel
> -* has to wait for a wake up acknowledgment. It also doesn't provide
> -* a way to cancel a wake up request if it takes too long.
> -*
> -* In TDX environment, the VMM has control over how long it takes to
> -* wake up secondary. It can postpone scheduling secondary vCPU
> -* indefinitely. Giving up on wake up request and reporting error
> opens
> -* possible attack vector for VMM: it can wake up a secondary CPU when
> -* kernel doesn't expect it. Wait until positive result of the wake up
> -
Re: [PATCH v3 06/13] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
On Tue, May 6, 2025 at 7:46 AM Ricardo Neri wrote: > > On Mon, May 05, 2025 at 03:07:43PM +0200, Rafael J. Wysocki wrote: > > On Sat, May 3, 2025 at 9:10 PM Ricardo Neri > > wrote: > > > > > > Add DeviceTree bindings for the wakeup mailbox used on Intel processors. > > > > > > x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert > > > followed by Start-Up IPI messages. The wakeup mailbox can be used when > > > this > > > mechanism unavailable. > > > > > > The wakeup mailbox offers more control to the operating system to boot > > > secondary CPUs than a spin-table. It allows the reuse of same wakeup > > > vector > > > for all CPUs while maintaining control over which CPUs to boot and when. > > > While it is possible to achieve the same level of control using a spin- > > > table, it would require to specify a separate cpu-release-addr for each > > > secondary CPU. > > > > > > Originally-by: Yunhong Jiang > > > Signed-off-by: Ricardo Neri > > > --- > > > Changes since v2: > > > - Implemented the mailbox as a reserved-memory node. Add to it a > > >`compatible` property. (Krzysztof) > > > - Explained the relationship between the mailbox and the `enable-mehod` > > >property of the CPU nodes. > > > - Expanded the documentation of the binding. > > > > > > Changes since v1: > > > - Added more details to the description of the binding. > > > - Added requirement a new requirement for cpu@N nodes to add an > > >`enable-method`. > > > --- > > > .../reserved-memory/intel,wakeup-mailbox.yaml | 87 +++ > > > 1 file changed, 87 insertions(+) > > > create mode 100644 > > > Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > > > > > > diff --git > > > a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > > > > > > b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > > > new file mode 100644 > > > index ..d97755b4673d > > > --- /dev/null > > > +++ > > > b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml > > > @@ -0,0 +1,87 @@ > > > +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause > > > +%YAML 1.2 > > > +--- > > > +$id: > > > http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml# > > > +$schema: http://devicetree.org/meta-schemas/core.yaml# > > > + > > > +title: Wakeup Mailbox for Intel processors > > > + > > > +description: | > > > + The Wakeup Mailbox provides a mechanism for the operating system to > > > wake up > > > + secondary CPUs on Intel processors. It is an alternative to the > > > INIT-!INIT- > > > + SIPI sequence used on most x86 systems. > > > + > > > + Firmware must define the enable-method property in the CPU nodes as > > > + "intel,wakeup-mailbox" to use the mailbox. > > > + > > > + Firmware implements the wakeup mailbox as a 4KB-aligned memory region > > > of size > > > + of 4KB. It is memory that the firmware reserves so that each secondary > > > CPU can > > > + have the operating system send a single message to them. The firmware > > > is > > > + responsible for putting the secondary CPUs in a state to check the > > > mailbox. > > > + > > > + The structure of the mailbox is as follows: > > > + > > > + Field Byte Byte Description > > > + Length Offset > > > + > > > -- > > > + Command 2 0Command to wake up the secondary CPU: > > > +0: Noop > > > +1: Wakeup: Jump to the > > > wakeup_vector > > > +2-0x: Reserved: > > > + Reserved 2 2Must be 0. > > > + APIC_ID 4 4APIC ID of the secondary CPU to wake up. > > > + Wakeup_Vector8 8The wakeup address for the secondary CPU. > > > + ReservedForOs 2032 16Reserved for OS use. > > > + ReservedForFW 2048 2048Reserved for firmware use. > > > + > > > -
Re: [PATCH v3 03/13] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpboot.c
On Tue, May 6, 2025 at 7:32 AM Ricardo Neri wrote: > > On Mon, May 05, 2025 at 12:03:13PM +0200, Rafael J. Wysocki wrote: > > On Sat, May 3, 2025 at 9:10 PM Ricardo Neri > > wrote: > > > > > > The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware > > > that > > > it wants to boot a secondary CPU using a mailbox as described in the > > > Multiprocessor Wakeup Structure of the ACPI specification. > > > > > > The wakeup mailbox does not strictly require support from ACPI. > > > > Well, except that it is defined by the ACPI specification. > > That is true. > > > > > > The platform firmware can implement a mailbox compatible in structure and > > > operation and enumerate it using other mechanisms such a DeviceTree graph. > > > > So is there a specification defining this mechanism? > > > > It is generally not sufficient to put the code and DT bindings > > unilaterally into the OS and expect the firmware to follow suit. > > > > This mechanism is described in the section 4.3.5 of the Intel TDX > Virtual Firmware Design Guide [1], but it refeers to the ACPI > specification for the interface. Yes, it does. > > > Move the code used to setup and use the mailbox out of the ACPI > > > directory to use it when support for ACPI is not available or needed. > > > > I think that the code implementing interfaces defined by the ACPI > > specification is not generic and so it should not be built when the > > kernel is configured without ACPI support. > > Support for ACPI would not be used on systems describing hardware using > a DeviceTree graph. My goal is to have a common interface that both > DT and ACPI can use. I think what is missing is that common interface. I get the general idea. :-) > Would it be preferred to have a separate implementation that is > functionally equivalent? Functionally equivalent on the OS side, that is. It would be better, but honestly I'm not sure if this is going to be sufficient to eliminate the dependency on the ACPI spec. It is just as you want the firmware to implement this tiny bit of the ACPI spec even though it is not implementing the rest of it. > [1]. > https://cdrdv2-public.intel.com/733585/tdx-virtual-firmware-design-guide-rev-004-20231206.pdf
Re: [PATCH hyperv-next 2/2] arch/x86: Provide the CPU number in the wakeup AP callback
On Wed, May 7, 2025 at 8:22 PM Roman Kisel wrote:
>
> When starting APs, confidential guests and paravisor guests
> need to know the CPU number, and the pattern of using the linear
> search has emerged in several places. With N processors that leads
> to the O(N^2) time complexity.
>
> Provide the CPU number in the AP wake up callback so that one can
> get the CPU number in constant time.
>
> Suggested-by: Michael Kelley
> Signed-off-by: Roman Kisel
> Reviewed-by: Tom Lendacky
> Reviewed-by: Thomas Gleixner
> Reviewed-by: Michael Kelley
For the ACPI bits
Acked-by: Rafael J. Wysocki
and I'm assuming that the x86 folks will take care of this.
Thanks!
> ---
> arch/x86/coco/sev/core.c | 13 ++---
> arch/x86/hyperv/hv_vtl.c | 12 ++--
> arch/x86/hyperv/ivm.c| 15 ++-
> arch/x86/include/asm/apic.h | 8
> arch/x86/include/asm/mshyperv.h | 5 +++--
> arch/x86/kernel/acpi/madt_wakeup.c | 2 +-
> arch/x86/kernel/apic/apic_noop.c | 8 +++-
> arch/x86/kernel/apic/apic_numachip.c | 2 +-
> arch/x86/kernel/apic/x2apic_uv_x.c | 2 +-
> arch/x86/kernel/smpboot.c| 10 +-
> 10 files changed, 28 insertions(+), 49 deletions(-)
>
> diff --git a/arch/x86/coco/sev/core.c b/arch/x86/coco/sev/core.c
> index b0c1a7a57497..7780d55d1833 100644
> --- a/arch/x86/coco/sev/core.c
> +++ b/arch/x86/coco/sev/core.c
> @@ -1177,7 +1177,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area
> *vmsa, int apic_id)
> free_page((unsigned long)vmsa);
> }
>
> -static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip)
> +static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned long start_ip,
> unsigned int cpu)
> {
> struct sev_es_save_area *cur_vmsa, *vmsa;
> struct ghcb_state state;
> @@ -1185,7 +1185,7 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id, unsigned
> long start_ip)
> unsigned long flags;
> struct ghcb *ghcb;
> u8 sipi_vector;
> - int cpu, ret;
> + int ret;
> u64 cr4;
>
> /*
> @@ -1206,15 +1206,6 @@ static int wakeup_cpu_via_vmgexit(u32 apic_id,
> unsigned long start_ip)
>
> /* Override start_ip with known protected guest start IP */
> start_ip = real_mode_header->sev_es_trampoline_start;
> -
> - /* Find the logical CPU for the APIC ID */
> - for_each_present_cpu(cpu) {
> - if (arch_match_cpu_phys_id(cpu, apic_id))
> - break;
> - }
> - if (cpu >= nr_cpu_ids)
> - return -EINVAL;
> -
> cur_vmsa = per_cpu(sev_vmsa, cpu);
>
> /*
> diff --git a/arch/x86/hyperv/hv_vtl.c b/arch/x86/hyperv/hv_vtl.c
> index 2f32ac1ae40e..3d149a2ca4c8 100644
> --- a/arch/x86/hyperv/hv_vtl.c
> +++ b/arch/x86/hyperv/hv_vtl.c
> @@ -211,17 +211,9 @@ static int hv_vtl_bringup_vcpu(u32 target_vp_index, int
> cpu, u64 eip_ignored)
> return ret;
> }
>
> -static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip)
> +static int hv_vtl_wakeup_secondary_cpu(u32 apicid, unsigned long start_eip,
> unsigned int cpu)
> {
> - int vp_index, cpu;
> -
> - /* Find the logical CPU for the APIC ID */
> - for_each_present_cpu(cpu) {
> - if (arch_match_cpu_phys_id(cpu, apicid))
> - break;
> - }
> - if (cpu >= nr_cpu_ids)
> - return -EINVAL;
> + int vp_index;
>
> pr_debug("Bringing up CPU with APIC ID %d in VTL2...\n", apicid);
> vp_index = hv_apicid_to_vp_index(apicid);
> diff --git a/arch/x86/hyperv/ivm.c b/arch/x86/hyperv/ivm.c
> index 0cc239cdb4da..e21557b24d19 100644
> --- a/arch/x86/hyperv/ivm.c
> +++ b/arch/x86/hyperv/ivm.c
> @@ -289,7 +289,7 @@ static void snp_cleanup_vmsa(struct sev_es_save_area
> *vmsa)
> free_page((unsigned long)vmsa);
> }
>
> -int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
> +int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip, unsigned int cpu)
> {
> struct sev_es_save_area *vmsa = (struct sev_es_save_area *)
> __get_free_page(GFP_KERNEL | __GFP_ZERO);
> @@ -298,7 +298,7 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
> u64 ret, retry = 5;
> struct hv_enable_vp_vtl *start_vp_input;
> unsigned long flags;
> - int cpu, vp_index;
> + int vp_index;
>
> if (!vmsa)
> return -ENOMEM;
> @@ -308,17 +308,6 @@ int hv_snp_boot_ap(u32 apic_id, unsigned long start_ip)
> if (vp_index &l
Re: [PATCH v4 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
wrote:
>
> The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware that
> it wants to boot a secondary CPU using a mailbox as described in the
> Multiprocessor Wakeup Structure of the ACPI specification.
>
> The platform firmware may implement the mailbox as described in the ACPI
> specification but enumerate it using a DeviceTree graph. An example of
> this is OpenHCL paravisor.
>
> Move the code used to setup and use the mailbox for CPU wakeup out of the
> ACPI directory into a new smpwakeup.c file that both ACPI and DeviceTree
> can use.
>
> No functional changes are intended.
>
> Co-developed-by: Yunhong Jiang
> Signed-off-by: Yunhong Jiang
> Signed-off-by: Ricardo Neri
> ---
> Changes since v3:
> - Create a new file smpwakeup.c instead of relocating it to smpboot.c.
>(Rafael)
>
> Changes since v2:
> - Only move to smpboot.c the portions of the code that configure and
>use the mailbox. This also resolved the compile warnings about unused
>functions that Michael Kelley reported.
> - Edited the commit message for clarity.
>
> Changes since v1:
> - None.
> ---
> arch/x86/Kconfig | 7
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/acpi/madt_wakeup.c | 76 --
> arch/x86/kernel/smpwakeup.c| 83
> ++
> 4 files changed, 91 insertions(+), 76 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index cb0f4af31789..82147edb355a 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1113,6 +1113,13 @@ config X86_LOCAL_APIC
> depends on X86_64 || SMP || X86_UP_APIC || PCI_MSI
> select IRQ_DOMAIN_HIERARCHY
>
> +config X86_MAILBOX_WAKEUP
> + def_bool y
> + depends on OF || ACPI_MADT_WAKEUP
At this point the dependency on OF is premature. IMV it should be
added in a later patch.
> + depends on X86_64
> + depends on SMP
> + depends on X86_LOCAL_APIC
> +
> config ACPI_MADT_WAKEUP
> def_bool y
> depends on X86_64
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 99a783fd4691..8f078af42a71 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -94,6 +94,7 @@ apm-y := apm_32.o
> obj-$(CONFIG_APM) += apm.o
> obj-$(CONFIG_SMP) += smp.o
> obj-$(CONFIG_SMP) += smpboot.o
> +obj-$(CONFIG_X86_MAILBOX_WAKEUP) += smpwakeup.o
> obj-$(CONFIG_X86_TSC) += tsc_sync.o
> obj-$(CONFIG_SMP) += setup_percpu.o
> obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
> b/arch/x86/kernel/acpi/madt_wakeup.c
> index 4033c804307a..a7e0158269b0 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -2,12 +2,10 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -15,12 +13,6 @@
> #include
> #include
>
> -/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> -static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> -
> -/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> -static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> -
> static u64 acpi_mp_pgd __ro_after_init;
> static u64 acpi_mp_reset_vector_paddr __ro_after_init;
>
> @@ -127,63 +119,6 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
> return 0;
> }
>
> -static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip)
> -{
> - if (!acpi_mp_wake_mailbox_paddr) {
> - pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs.
> Booting with kexec?\n");
> - return -EOPNOTSUPP;
> - }
> -
> - /*
> -* Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> -*
> -* Wakeup of secondary CPUs is fully serialized in the core code.
> -* No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> -*/
> - if (!acpi_mp_wake_mailbox) {
> - acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> - sizeof(*acpi_mp_wake_mailbox),
> - MEMREMAP_WB);
> - }
> -
> - /*
> -* Mailbox memory is shared between the firmware and OS. Firmware will
> -* listen on mailbox command address, and once it receives the wakeup
> -* command, the CPU associated with the given apicid will be booted.
> -*
> -* The value of 'apic_id' and 'wakeup_vector' must be visible to the
> -* firmware before the wakeup command is visible. smp_store_release()
> -* ensures ordering and visibility.
> -*/
> - acpi_mp_wake_mailbox->apic_id = apicid;
> -
Re: [PATCH v4 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
On Wed, Jun 4, 2025 at 2:18 AM Ricardo Neri
wrote:
>
> Add DeviceTree bindings to enumerate the wakeup mailbox used in platform
> firmware for Intel processors.
>
> x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> mechanism is unavailable.
>
> The wakeup mailbox offers more control to the operating system to boot
> secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> for all CPUs while maintaining control over which CPUs to boot and when.
> While it is possible to achieve the same level of control using a spin-
> table, it would require to specify a separate `cpu-release-addr` for each
> secondary CPU.
>
> The operation and structure of the mailbox is described in the
> Multiprocessor Wakeup Structure defined in the ACPI specification. Note
> that this structure does not specify how to publish the mailbox to the
> operating system (ACPI-based platform firmware uses a separate table). No
> ACPI table is needed in DeviceTree-based firmware to enumerate the mailbox.
>
> Add a `compatible` property that the operating system can use to discover
> the mailbox. Nodes wanting to refer to the reserved memory usually define a
> `memory-region` property. /cpus/cpu* nodes would want to refer to the
> mailbox, but they do not have such property defined in the DeviceTree
> specification. Moreover, it would imply that there is a memory region per
> CPU.
>
> Co-developed-by: Yunhong Jiang
> Signed-off-by: Yunhong Jiang
> Signed-off-by: Ricardo Neri
> ---
> Changes since v3:
> - Removed redefinitions of the mailbox and instead referred to ACPI
>specification as per discussion on LKML.
> - Clarified that DeviceTree-based firmware do not require the use of
>ACPI tables to enumerate the mailbox. (Rob)
> - Described the need of using a `compatible` property.
> - Dropped the `alignment` property. (Krzysztof, Rafael)
> - Used a real address for the mailbox node. (Krzysztof)
>
> Changes since v2:
> - Implemented the mailbox as a reserved-memory node. Add to it a
>`compatible` property. (Krzysztof)
> - Explained the relationship between the mailbox and the `enable-mehod`
>property of the CPU nodes.
> - Expanded the documentation of the binding.
>
> Changes since v1:
> - Added more details to the description of the binding.
> - Added requirement a new requirement for cpu@N nodes to add an
>`enable-method`.
> ---
> .../reserved-memory/intel,wakeup-mailbox.yaml | 48
> ++
> 1 file changed, 48 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> new file mode 100644
> index ..f18643805866
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> @@ -0,0 +1,48 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wakeup Mailbox for Intel processors
> +
> +description: |
> + The Wakeup Mailbox provides a mechanism for the operating system to wake up
> + secondary CPUs on Intel processors. It is an alternative to the INIT-!INIT-
> + SIPI sequence used on most x86 systems.
> +
> + The structure and operation of the mailbox is described in the
> Multiprocessor
> + Wakeup Structure of the ACPI specification.
Please make this more specific: Which specification version and what section.
You may as well add a URL here too.
> +
> + The implementation of the mailbox in platform firmware is described in the
> + Intel TDX Virtual Firmware Design Guide section 4.3.5.
> +
> + See
> https://www.intel.com/content/www/us/en/content-details/733585/intel-tdx-virtual-firmware-design-guide.html
> +
> +maintainers:
> + - Ricardo Neri
> +
> +allOf:
> + - $ref: reserved-memory.yaml
> +
> +properties:
> + compatible:
> +const: intel,wakeup-mailbox
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +reserved-memory {
> +#address-cells = <2>;
> +#size-cells = <1>;
> +
> +wakeup-mailbox@ {
> +compatible = "intel,wakeup-mailbox";
> +reg = <0x0 0x 0x1000>;
> +};
> +};
>
> --
Re: [PATCH v5 01/10] x86/acpi: Add a helper functions to setup and access the wakeup mailbox
s/a helper/helper/ in the subject.
On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
wrote:
>
> In preparation to move the functionality to wake secondary CPUs up out of
> the ACPI code, add two helper functions.
>
> The function acpi_setup_mp_wakeup_mailbox() stores the physical address of
> the mailbox and updates the wakeup_secondary_cpu_64() APIC callback.
>
> There is a slight change in behavior: now the APIC callback is updated
> before configuring CPU hotplug offline behavior. This is fine as the APIC
> callback continues to be updated unconditionally, regardless of the
> restriction on CPU offlining.
>
> The function acpi_madt_multiproc_wakeup_mailbox() returns a pointer to the
> mailbox. Use this helper function only in the portions of the code for
> which the variable acpi_mp_wake_mailbox will be out of scope once it is
> relocated out of the ACPI directory.
>
> The wakeup mailbox is only supported for CONFIG_X86_64 and needed only with
> CONFIG_SMP=y.
>
> Signed-off-by: Ricardo Neri
With the above nit addressed
Acked-by: Rafael J. Wysocki
> ---
> Changes since v4:
> - None
>
> Changes since v3:
> - Squashed the two first patches of the series into one, both introduce
>helper functions. (Rafael)
> - Renamed setup_mp_wakeup_mailbox() as acpi_setup_mp_wakeup_mailbox().
>(Rafael)
> - Dropped the function prototype for !CONFIG_X86_64. (Rafael)
>
> Changes since v2:
> - Introduced this patch.
>
> Changes since v1:
> - N/A
> ---
> arch/x86/include/asm/smp.h | 3 +++
> arch/x86/kernel/acpi/madt_wakeup.c | 20 +++-
> 2 files changed, 18 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
> index 0c1c68039d6f..77dce560a70a 100644
> --- a/arch/x86/include/asm/smp.h
> +++ b/arch/x86/include/asm/smp.h
> @@ -146,6 +146,9 @@ static inline struct cpumask *cpu_l2c_shared_mask(int cpu)
> return per_cpu(cpu_l2c_shared_map, cpu);
> }
>
> +void acpi_setup_mp_wakeup_mailbox(u64 addr);
> +struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void);
> +
> #else /* !CONFIG_SMP */
> #define wbinvd_on_cpu(cpu) wbinvd()
> static inline int wbinvd_on_all_cpus(void)
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
> b/arch/x86/kernel/acpi/madt_wakeup.c
> index 6d7603511f52..c3ac5ecf3e7d 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -37,6 +37,7 @@ static void acpi_mp_play_dead(void)
>
> static void acpi_mp_cpu_die(unsigned int cpu)
> {
> + struct acpi_madt_multiproc_wakeup_mailbox *mailbox =
> acpi_get_mp_wakeup_mailbox();
> u32 apicid = per_cpu(x86_cpu_to_apicid, cpu);
> unsigned long timeout;
>
> @@ -46,13 +47,13 @@ static void acpi_mp_cpu_die(unsigned int cpu)
> *
> * BIOS has to clear 'command' field of the mailbox.
> */
> - acpi_mp_wake_mailbox->apic_id = apicid;
> - smp_store_release(&acpi_mp_wake_mailbox->command,
> + mailbox->apic_id = apicid;
> + smp_store_release(&mailbox->command,
> ACPI_MP_WAKE_COMMAND_TEST);
>
> /* Don't wait longer than a second. */
> timeout = USEC_PER_SEC;
> - while (READ_ONCE(acpi_mp_wake_mailbox->command) && --timeout)
> + while (READ_ONCE(mailbox->command) && --timeout)
> udelay(1);
>
> if (!timeout)
> @@ -227,7 +228,7 @@ int __init acpi_parse_mp_wake(union acpi_subtable_headers
> *header,
>
> acpi_table_print_madt_entry(&header->common);
>
> - acpi_mp_wake_mailbox_paddr = mp_wake->mailbox_address;
> + acpi_setup_mp_wakeup_mailbox(mp_wake->mailbox_address);
>
> if (mp_wake->version >= ACPI_MADT_MP_WAKEUP_VERSION_V1 &&
> mp_wake->header.length >= ACPI_MADT_MP_WAKEUP_SIZE_V1) {
> @@ -243,7 +244,16 @@ int __init acpi_parse_mp_wake(union
> acpi_subtable_headers *header,
> acpi_mp_disable_offlining(mp_wake);
> }
>
> + return 0;
> +}
> +
> +void __init acpi_setup_mp_wakeup_mailbox(u64 mailbox_paddr)
> +{
> + acpi_mp_wake_mailbox_paddr = mailbox_paddr;
> apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu);
> +}
>
> - return 0;
> +struct acpi_madt_multiproc_wakeup_mailbox *acpi_get_mp_wakeup_mailbox(void)
> +{
> + return acpi_mp_wake_mailbox;
> }
>
> --
> 2.43.0
>
Re: [PATCH v5 02/10] x86/acpi: Move acpi_wakeup_cpu() and helpers to smpwakeup.c
On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
wrote:
>
> The bootstrap processor uses acpi_wakeup_cpu() to indicate to firmware that
> it wants to boot a secondary CPU using a mailbox as described in the
> Multiprocessor Wakeup Structure of the ACPI specification.
>
> The platform firmware may implement the mailbox as described in the ACPI
> specification but enumerate it using a DeviceTree graph. An example of
> this is OpenHCL paravisor.
>
> Move the code used to setup and use the mailbox for CPU wakeup out of the
> ACPI directory into a new smpwakeup.c file that both ACPI and DeviceTree
> can use.
IMV "that can be used by both ACPI and DeviceTree" would sound better.
> No functional changes are intended.
>
> Co-developed-by: Yunhong Jiang
> Signed-off-by: Yunhong Jiang
> Signed-off-by: Ricardo Neri
With the above addressed
Acked-by: Rafael J. Wysocki
> ---
> Changes since v4:
> - Removed dependency on CONFIG_OF. It will be added in a later patch.
>(Rafael)
> - Rebased on v6.16-rc3.
>
> Changes since v3:
> - Create a new file smpwakeup.c instead of relocating it to smpboot.c.
>(Rafael)
>
> Changes since v2:
> - Only move to smpboot.c the portions of the code that configure and
>use the mailbox. This also resolved the compile warnings about unused
>functions that Michael Kelley reported.
> - Edited the commit message for clarity.
>
> Changes since v1:
> - None.
> ---
> arch/x86/Kconfig | 7
> arch/x86/kernel/Makefile | 1 +
> arch/x86/kernel/acpi/madt_wakeup.c | 76 --
> arch/x86/kernel/smpwakeup.c| 83
> ++
> 4 files changed, 91 insertions(+), 76 deletions(-)
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 71019b3b54ea..e3009cb59928 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1114,6 +1114,13 @@ config X86_LOCAL_APIC
> depends on X86_64 || SMP || X86_UP_APIC || PCI_MSI
> select IRQ_DOMAIN_HIERARCHY
>
> +config X86_MAILBOX_WAKEUP
> + def_bool y
> + depends on ACPI_MADT_WAKEUP
> + depends on X86_64
> + depends on SMP
> + depends on X86_LOCAL_APIC
> +
> config ACPI_MADT_WAKEUP
> def_bool y
> depends on X86_64
> diff --git a/arch/x86/kernel/Makefile b/arch/x86/kernel/Makefile
> index 0d2a6d953be9..1fce3d20cf2d 100644
> --- a/arch/x86/kernel/Makefile
> +++ b/arch/x86/kernel/Makefile
> @@ -94,6 +94,7 @@ apm-y := apm_32.o
> obj-$(CONFIG_APM) += apm.o
> obj-$(CONFIG_SMP) += smp.o
> obj-$(CONFIG_SMP) += smpboot.o
> +obj-$(CONFIG_X86_MAILBOX_WAKEUP) += smpwakeup.o
> obj-$(CONFIG_X86_TSC) += tsc_sync.o
> obj-$(CONFIG_SMP) += setup_percpu.o
> obj-$(CONFIG_X86_MPPARSE) += mpparse.o
> diff --git a/arch/x86/kernel/acpi/madt_wakeup.c
> b/arch/x86/kernel/acpi/madt_wakeup.c
> index c3ac5ecf3e7d..a7e0158269b0 100644
> --- a/arch/x86/kernel/acpi/madt_wakeup.c
> +++ b/arch/x86/kernel/acpi/madt_wakeup.c
> @@ -2,12 +2,10 @@
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> #include
> -#include
> #include
> #include
> #include
> @@ -15,12 +13,6 @@
> #include
> #include
>
> -/* Physical address of the Multiprocessor Wakeup Structure mailbox */
> -static u64 acpi_mp_wake_mailbox_paddr __ro_after_init;
> -
> -/* Virtual address of the Multiprocessor Wakeup Structure mailbox */
> -static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> -
> static u64 acpi_mp_pgd __ro_after_init;
> static u64 acpi_mp_reset_vector_paddr __ro_after_init;
>
> @@ -127,63 +119,6 @@ static int __init acpi_mp_setup_reset(u64 reset_vector)
> return 0;
> }
>
> -static int acpi_wakeup_cpu(u32 apicid, unsigned long start_ip, unsigned int
> cpu)
> -{
> - if (!acpi_mp_wake_mailbox_paddr) {
> - pr_warn_once("No MADT mailbox: cannot bringup secondary CPUs.
> Booting with kexec?\n");
> - return -EOPNOTSUPP;
> - }
> -
> - /*
> -* Remap mailbox memory only for the first call to acpi_wakeup_cpu().
> -*
> -* Wakeup of secondary CPUs is fully serialized in the core code.
> -* No need to protect acpi_mp_wake_mailbox from concurrent accesses.
> -*/
> - if (!acpi_mp_wake_mailbox) {
> - acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> - sizeof(*acpi_mp_wake_mailbox),
> -
Re: [PATCH v5 03/10] dt-bindings: reserved-memory: Wakeup Mailbox for Intel processors
On Sat, Jun 28, 2025 at 5:35 AM Ricardo Neri
wrote:
>
> Add DeviceTree bindings to enumerate the wakeup mailbox used in platform
> firmware for Intel processors.
>
> x86 platforms commonly boot secondary CPUs using an INIT assert, de-assert
> followed by Start-Up IPI messages. The wakeup mailbox can be used when this
> mechanism is unavailable.
>
> The wakeup mailbox offers more control to the operating system to boot
> secondary CPUs than a spin-table. It allows the reuse of same wakeup vector
> for all CPUs while maintaining control over which CPUs to boot and when.
> While it is possible to achieve the same level of control using a spin-
> table, it would require to specify a separate `cpu-release-addr` for each
> secondary CPU.
>
> The operation and structure of the mailbox is described in the
> Multiprocessor Wakeup Structure defined in the ACPI specification. Note
> that this structure does not specify how to publish the mailbox to the
> operating system (ACPI-based platform firmware uses a separate table). No
> ACPI table is needed in DeviceTree-based firmware to enumerate the mailbox.
>
> Add a `compatible` property that the operating system can use to discover
> the mailbox. Nodes wanting to refer to the reserved memory usually define a
> `memory-region` property. /cpus/cpu* nodes would want to refer to the
> mailbox, but they do not have such property defined in the DeviceTree
> specification. Moreover, it would imply that there is a memory region per
> CPU.
>
> Co-developed-by: Yunhong Jiang
> Signed-off-by: Yunhong Jiang
> Signed-off-by: Ricardo Neri
LGTM from the ACPI specification cross-referencing perspective, so
Acked-by: Rafael J. Wysocki
> ---
> Changes since v4:
> - Specified the version and section of the ACPI spec in which the
>wakeup mailbox is defined. (Rafael)
> - Fixed a warning from yamllint about line lengths of URLs.
>
> Changes since v3:
> - Removed redefinitions of the mailbox and instead referred to ACPI
>specification as per discussion on LKML.
> - Clarified that DeviceTree-based firmware do not require the use of
>ACPI tables to enumerate the mailbox. (Rob)
> - Described the need of using a `compatible` property.
> - Dropped the `alignment` property. (Krzysztof, Rafael)
> - Used a real address for the mailbox node. (Krzysztof)
>
> Changes since v2:
> - Implemented the mailbox as a reserved-memory node. Add to it a
>`compatible` property. (Krzysztof)
> - Explained the relationship between the mailbox and the `enable-mehod`
>property of the CPU nodes.
> - Expanded the documentation of the binding.
>
> Changes since v1:
> - Added more details to the description of the binding.
> - Added requirement a new requirement for cpu@N nodes to add an
>`enable-method`.
> ---
> .../reserved-memory/intel,wakeup-mailbox.yaml | 50
> ++
> 1 file changed, 50 insertions(+)
>
> diff --git
> a/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> new file mode 100644
> index ..a80d3bac44c2
> --- /dev/null
> +++
> b/Documentation/devicetree/bindings/reserved-memory/intel,wakeup-mailbox.yaml
> @@ -0,0 +1,50 @@
> +# SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause
> +%YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/reserved-memory/intel,wakeup-mailbox.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Wakeup Mailbox for Intel processors
> +
> +description: |
> + The Wakeup Mailbox provides a mechanism for the operating system to wake up
> + secondary CPUs on Intel processors. It is an alternative to the INIT-!INIT-
> + SIPI sequence used on most x86 systems.
> +
> + The structure and operation of the mailbox is described in the
> Multiprocessor
> + Wakeup Structure of the ACPI specification version 6.6 section 5.2.12.19
> [1].
> +
> + The implementation of the mailbox in platform firmware is described in the
> + Intel TDX Virtual Firmware Design Guide section 4.3.5 [2].
> +
> + 1:
> https://uefi.org/specs/ACPI/6.6/05_ACPI_Software_Programming_Model.html#multiprocessor-wakeup-structure
> + 2:
> https://www.intel.com/content/www/us/en/content-details/733585/intel-tdx-virtual-firmware-design-guide.html
> +
> +
> +maintainers:
> + - Ricardo Neri
> +
> +allOf:
> + - $ref: reserved-memory.yaml
> +
> +properties:
> + compatible:
> +const: intel,wakeup-mailbox
> +
> +required:
> + - compatible
> + - reg
> +
> +unevaluatedProperties: false
> +
> +examples:
> + - |
> +reserved-memory {
> +#address-cells = <2>;
> +#size-cells = <1>;
> +
> +wakeup-mailbox@ {
> +compatible = "intel,wakeup-mailbox";
> +reg = <0x0 0x 0x1000>;
> +};
> +};
>
> --
> 2.43.0
>
