Add multiprocessor wakeup support using MADT ACPI table for x86
platforms. It uses mailbox based mechanism to wake up the APs. You
can get more details about the ACPI table and mailbox protocol in
Guest-Host-Communication Interface (GHCI) for Intel Trust Domain
Extensions (Intel TDX) specification document (sec 4.1)
https://software.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface.pdf
Changes since v1:
* Removed signoff from Rob and Erik.
Kuppuswamy Sathyanarayanan (3):
ACPICA: ACPI 6.4: MADT: add Multiprocessor Wakeup Mailbox Structure
ACPI/table: Print MADT Wake table information
x86/acpi, x86/boot: Add multiprocessor wake-up support
arch/x86/include/asm/apic.h | 3 ++
arch/x86/kernel/acpi/boot.c | 56 +++++++++++++++++++++++++++++++++
arch/x86/kernel/apic/probe_32.c | 8 +++++
arch/x86/kernel/apic/probe_64.c | 8 +++++
drivers/acpi/tables.c | 11 +++++++
include/acpi/actbl2.h | 14 +++++++++
6 files changed, 100 insertions(+)
--
2.25.1
ACPICA commit f1ee04207a212f6c519441e7e25397649ebc4cea
Add Multiprocessor Wakeup Mailbox Structure definition. It is useful
in parsing MADT Wake table.
Link: https://github.com/acpica/acpica/commit/f1ee0420
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
include/acpi/actbl2.h | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index b2362600b9ff..7dce422f6119 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -733,6 +733,20 @@ struct acpi_madt_multiproc_wakeup {
u64 base_address;
};
+#define ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE 2032
+#define ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE 2048
+
+struct acpi_madt_multiproc_wakeup_mailbox {
+ u16 command;
+ u16 reserved; /* reserved - must be zero */
+ u32 apic_id;
+ u64 wakeup_vector;
+ u8 reserved_os[ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE]; /* reserved for OS use */
+ u8 reserved_firmware[ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE]; /* reserved for firmware use */
+};
+
+#define ACPI_MP_WAKE_COMMAND_WAKEUP 1
+
/*
* Common flags fields for MADT subtables
*/
--
2.25.1
As per ACPI specification r6.4, sec 5.2.12.19, a new sub
structure – multiprocessor wake-up structure - is added to the
ACPI Multiple APIC Description Table (MADT) to describe the
information of the mailbox. If a platform firmware produces the
multiprocessor wake-up structure, then OS may use this new
mailbox-based mechanism to wake up the APs.
Add ACPI MADT wake table parsing support for x86 platform and if
MADT wake table is present, update apic->wakeup_secondary_cpu with
new API which uses MADT wake mailbox to wake-up CPU.
Co-developed-by: Sean Christopherson <[email protected]>
Signed-off-by: Sean Christopherson <[email protected]>
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
Reviewed-by: Andi Kleen <[email protected]>
---
arch/x86/include/asm/apic.h | 3 ++
arch/x86/kernel/acpi/boot.c | 56 +++++++++++++++++++++++++++++++++
arch/x86/kernel/apic/probe_32.c | 8 +++++
arch/x86/kernel/apic/probe_64.c | 8 +++++
4 files changed, 75 insertions(+)
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
index 412b51e059c8..3e94e1f402ea 100644
--- a/arch/x86/include/asm/apic.h
+++ b/arch/x86/include/asm/apic.h
@@ -487,6 +487,9 @@ static inline unsigned int read_apic_id(void)
return apic->get_apic_id(reg);
}
+typedef int (*wakeup_cpu_handler)(int apicid, unsigned long start_eip);
+extern void acpi_wake_cpu_handler_update(wakeup_cpu_handler handler);
+
extern int default_apic_id_valid(u32 apicid);
extern int default_acpi_madt_oem_check(char *, char *);
extern void default_setup_apic_routing(void);
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 14cd3186dc77..a4a6b97910e1 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -65,6 +65,9 @@ int acpi_fix_pin2_polarity __initdata;
static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
#endif
+static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
+static u64 acpi_mp_wake_mailbox_paddr;
+
#ifdef CONFIG_X86_IO_APIC
/*
* Locks related to IOAPIC hotplug
@@ -329,6 +332,29 @@ acpi_parse_lapic_nmi(union acpi_subtable_headers * header, const unsigned long e
return 0;
}
+static void acpi_mp_wake_mailbox_init(void)
+{
+ if (acpi_mp_wake_mailbox)
+ return;
+
+ acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
+ sizeof(*acpi_mp_wake_mailbox), MEMREMAP_WB);
+}
+
+static int acpi_wakeup_cpu(int apicid, unsigned long start_ip)
+{
+ acpi_mp_wake_mailbox_init();
+
+ if (!acpi_mp_wake_mailbox)
+ return -EINVAL;
+
+ WRITE_ONCE(acpi_mp_wake_mailbox->apic_id, apicid);
+ WRITE_ONCE(acpi_mp_wake_mailbox->wakeup_vector, start_ip);
+ WRITE_ONCE(acpi_mp_wake_mailbox->command, ACPI_MP_WAKE_COMMAND_WAKEUP);
+
+ return 0;
+}
+
#endif /*CONFIG_X86_LOCAL_APIC */
#ifdef CONFIG_X86_IO_APIC
@@ -1086,6 +1112,30 @@ static int __init acpi_parse_madt_lapic_entries(void)
}
return 0;
}
+
+static int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
+ const unsigned long end)
+{
+ struct acpi_madt_multiproc_wakeup *mp_wake;
+
+ if (acpi_mp_wake_mailbox)
+ return -EINVAL;
+
+ if (!IS_ENABLED(CONFIG_SMP))
+ return -ENODEV;
+
+ mp_wake = (struct acpi_madt_multiproc_wakeup *) header;
+ if (BAD_MADT_ENTRY(mp_wake, end))
+ return -EINVAL;
+
+ acpi_table_print_madt_entry(&header->common);
+
+ acpi_mp_wake_mailbox_paddr = mp_wake->base_address;
+
+ acpi_wake_cpu_handler_update(acpi_wakeup_cpu);
+
+ return 0;
+}
#endif /* CONFIG_X86_LOCAL_APIC */
#ifdef CONFIG_X86_IO_APIC
@@ -1284,6 +1334,12 @@ static void __init acpi_process_madt(void)
smp_found_config = 1;
}
+
+ /*
+ * Parse MADT MP Wake entry.
+ */
+ acpi_table_parse_madt(ACPI_MADT_TYPE_MULTIPROC_WAKEUP,
+ acpi_parse_mp_wake, 1);
}
if (error == -EINVAL) {
/*
diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
index a61f642b1b90..d450014841b2 100644
--- a/arch/x86/kernel/apic/probe_32.c
+++ b/arch/x86/kernel/apic/probe_32.c
@@ -207,3 +207,11 @@ int __init default_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
}
return 0;
}
+
+void __init acpi_wake_cpu_handler_update(wakeup_cpu_handler handler)
+{
+ struct apic **drv;
+
+ for (drv = __apicdrivers; drv < __apicdrivers_end; drv++)
+ (*drv)->wakeup_secondary_cpu = handler;
+}
diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
index c46720f185c0..986dbb68d3c4 100644
--- a/arch/x86/kernel/apic/probe_64.c
+++ b/arch/x86/kernel/apic/probe_64.c
@@ -50,3 +50,11 @@ int __init default_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
}
return 0;
}
+
+void __init acpi_wake_cpu_handler_update(wakeup_cpu_handler handler)
+{
+ struct apic **drv;
+
+ for (drv = __apicdrivers; drv < __apicdrivers_end; drv++)
+ (*drv)->wakeup_secondary_cpu = handler;
+}
--
2.25.1
When MADT is parsed, print MADT Wake table information as
debug message. It will be useful to debug CPU boot issues
related to MADT wake table.
Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
---
drivers/acpi/tables.c | 11 +++++++++++
1 file changed, 11 insertions(+)
diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
index 9d581045acff..206df4ad8b2b 100644
--- a/drivers/acpi/tables.c
+++ b/drivers/acpi/tables.c
@@ -207,6 +207,17 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
}
break;
+ case ACPI_MADT_TYPE_MULTIPROC_WAKEUP:
+ {
+ struct acpi_madt_multiproc_wakeup *p;
+
+ p = (struct acpi_madt_multiproc_wakeup *) header;
+
+ pr_debug("MP Wake (Mailbox version[%d] base_address[%llx])\n",
+ p->mailbox_version, p->base_address);
+ }
+ break;
+
default:
pr_warn("Found unsupported MADT entry (type = 0x%x)\n",
header->type);
--
2.25.1
On Thu, Apr 22, 2021 at 02:47:05PM -0700, Kuppuswamy Sathyanarayanan wrote:
> Add multiprocessor wakeup support using MADT ACPI table for x86
> platforms. It uses mailbox based mechanism to wake up the APs. You
> can get more details about the ACPI table and mailbox protocol in
> Guest-Host-Communication Interface (GHCI) for Intel Trust Domain
> Extensions (Intel TDX) specification document (sec 4.1)
>
> https://software.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface.pdf
>
> Changes since v1:
> * Removed signoff from Rob and Erik.
For the future: please do not resend your patchset immediately but give
reviewers time to have a look at it.
Your current patchset comprises of only 3 patches - now imagine if it
were, 15, or 20 or more? Now also imagine if you were not the only one
submitter who would resend immediately... you'd soon have maintainers
drowning in email - not that they don't do so already anyway.
While waiting, you could read
Documentation/process/submitting-patches.rst
for example.
Thx.
--
Regards/Gruss,
Boris.
https://people.kernel.org/tglx/notes-about-netiquette
On 4/22/21 3:01 PM, Borislav Petkov wrote:
> On Thu, Apr 22, 2021 at 02:47:05PM -0700, Kuppuswamy Sathyanarayanan wrote:
>> Add multiprocessor wakeup support using MADT ACPI table for x86
>> platforms. It uses mailbox based mechanism to wake up the APs. You
>> can get more details about the ACPI table and mailbox protocol in
>> Guest-Host-Communication Interface (GHCI) for Intel Trust Domain
>> Extensions (Intel TDX) specification document (sec 4.1)
>>
>> https://software.intel.com/content/dam/develop/external/us/en/documents/intel-tdx-guest-hypervisor-communication-interface.pdf
>>
>> Changes since v1:
>> * Removed signoff from Rob and Erik.
>
> For the future: please do not resend your patchset immediately but give
> reviewers time to have a look at it.
>
> Your current patchset comprises of only 3 patches - now imagine if it
> were, 15, or 20 or more? Now also imagine if you were not the only one
> submitter who would resend immediately... you'd soon have maintainers
> drowning in email - not that they don't do so already anyway.
>
> While waiting, you could read
>
> Documentation/process/submitting-patches.rst
Sorry for the trouble. I will keep it in mind for next submission.
>
> for example.
>
> Thx.
>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
On Thu, Apr 22, 2021 at 11:47 PM Kuppuswamy Sathyanarayanan
<[email protected]> wrote:
>
> ACPICA commit f1ee04207a212f6c519441e7e25397649ebc4cea
>
> Add Multiprocessor Wakeup Mailbox Structure definition. It is useful
> in parsing MADT Wake table.
>
> Link: https://github.com/acpica/acpica/commit/f1ee0420
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
I'm expecting to receive this patch from Erik at one point, so I'm
guessing that it's been added to this series for reference.
> ---
> include/acpi/actbl2.h | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> index b2362600b9ff..7dce422f6119 100644
> --- a/include/acpi/actbl2.h
> +++ b/include/acpi/actbl2.h
> @@ -733,6 +733,20 @@ struct acpi_madt_multiproc_wakeup {
> u64 base_address;
> };
>
> +#define ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE 2032
> +#define ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE 2048
> +
> +struct acpi_madt_multiproc_wakeup_mailbox {
> + u16 command;
> + u16 reserved; /* reserved - must be zero */
> + u32 apic_id;
> + u64 wakeup_vector;
> + u8 reserved_os[ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE]; /* reserved for OS use */
> + u8 reserved_firmware[ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE]; /* reserved for firmware use */
> +};
> +
> +#define ACPI_MP_WAKE_COMMAND_WAKEUP 1
> +
> /*
> * Common flags fields for MADT subtables
> */
> --
> 2.25.1
>
On Thu, Apr 22, 2021 at 11:47 PM Kuppuswamy Sathyanarayanan
<[email protected]> wrote:
>
> When MADT is parsed, print MADT Wake table information as
> debug message. It will be useful to debug CPU boot issues
> related to MADT wake table.
>
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
This obviously depends on the ACPICA changes from the previous patch,
so I can pick it up when that material gets integrated.
For now
Acked-by: Rafael J. Wysocki <[email protected]>
> ---
> drivers/acpi/tables.c | 11 +++++++++++
> 1 file changed, 11 insertions(+)
>
> diff --git a/drivers/acpi/tables.c b/drivers/acpi/tables.c
> index 9d581045acff..206df4ad8b2b 100644
> --- a/drivers/acpi/tables.c
> +++ b/drivers/acpi/tables.c
> @@ -207,6 +207,17 @@ void acpi_table_print_madt_entry(struct acpi_subtable_header *header)
> }
> break;
>
> + case ACPI_MADT_TYPE_MULTIPROC_WAKEUP:
> + {
> + struct acpi_madt_multiproc_wakeup *p;
> +
> + p = (struct acpi_madt_multiproc_wakeup *) header;
> +
> + pr_debug("MP Wake (Mailbox version[%d] base_address[%llx])\n",
> + p->mailbox_version, p->base_address);
> + }
> + break;
> +
> default:
> pr_warn("Found unsupported MADT entry (type = 0x%x)\n",
> header->type);
> --
> 2.25.1
>
On Thu, Apr 22, 2021 at 11:47 PM Kuppuswamy Sathyanarayanan
<[email protected]> wrote:
>
> As per ACPI specification r6.4, sec 5.2.12.19, a new sub
> structure – multiprocessor wake-up structure - is added to the
> ACPI Multiple APIC Description Table (MADT) to describe the
> information of the mailbox. If a platform firmware produces the
> multiprocessor wake-up structure, then OS may use this new
> mailbox-based mechanism to wake up the APs.
>
> Add ACPI MADT wake table parsing support for x86 platform and if
> MADT wake table is present, update apic->wakeup_secondary_cpu with
> new API which uses MADT wake mailbox to wake-up CPU.
>
> Co-developed-by: Sean Christopherson <[email protected]>
> Signed-off-by: Sean Christopherson <[email protected]>
> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
> Reviewed-by: Andi Kleen <[email protected]>
> ---
> arch/x86/include/asm/apic.h | 3 ++
> arch/x86/kernel/acpi/boot.c | 56 +++++++++++++++++++++++++++++++++
> arch/x86/kernel/apic/probe_32.c | 8 +++++
> arch/x86/kernel/apic/probe_64.c | 8 +++++
> 4 files changed, 75 insertions(+)
>
> diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h
> index 412b51e059c8..3e94e1f402ea 100644
> --- a/arch/x86/include/asm/apic.h
> +++ b/arch/x86/include/asm/apic.h
> @@ -487,6 +487,9 @@ static inline unsigned int read_apic_id(void)
> return apic->get_apic_id(reg);
> }
>
> +typedef int (*wakeup_cpu_handler)(int apicid, unsigned long start_eip);
> +extern void acpi_wake_cpu_handler_update(wakeup_cpu_handler handler);
> +
> extern int default_apic_id_valid(u32 apicid);
> extern int default_acpi_madt_oem_check(char *, char *);
> extern void default_setup_apic_routing(void);
> diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
> index 14cd3186dc77..a4a6b97910e1 100644
> --- a/arch/x86/kernel/acpi/boot.c
> +++ b/arch/x86/kernel/acpi/boot.c
> @@ -65,6 +65,9 @@ int acpi_fix_pin2_polarity __initdata;
> static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE;
> #endif
>
> +static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox;
> +static u64 acpi_mp_wake_mailbox_paddr;
> +
> #ifdef CONFIG_X86_IO_APIC
> /*
> * Locks related to IOAPIC hotplug
> @@ -329,6 +332,29 @@ acpi_parse_lapic_nmi(union acpi_subtable_headers * header, const unsigned long e
> return 0;
> }
>
> +static void acpi_mp_wake_mailbox_init(void)
> +{
> + if (acpi_mp_wake_mailbox)
> + return;
> +
> + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr,
> + sizeof(*acpi_mp_wake_mailbox), MEMREMAP_WB);
> +}
> +
> +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip)
> +{
> + acpi_mp_wake_mailbox_init();
> +
> + if (!acpi_mp_wake_mailbox)
> + return -EINVAL;
> +
> + WRITE_ONCE(acpi_mp_wake_mailbox->apic_id, apicid);
> + WRITE_ONCE(acpi_mp_wake_mailbox->wakeup_vector, start_ip);
> + WRITE_ONCE(acpi_mp_wake_mailbox->command, ACPI_MP_WAKE_COMMAND_WAKEUP);
> +
> + return 0;
> +}
> +
> #endif /*CONFIG_X86_LOCAL_APIC */
>
> #ifdef CONFIG_X86_IO_APIC
> @@ -1086,6 +1112,30 @@ static int __init acpi_parse_madt_lapic_entries(void)
> }
> return 0;
> }
> +
> +static int __init acpi_parse_mp_wake(union acpi_subtable_headers *header,
> + const unsigned long end)
> +{
> + struct acpi_madt_multiproc_wakeup *mp_wake;
> +
> + if (acpi_mp_wake_mailbox)
> + return -EINVAL;
> +
> + if (!IS_ENABLED(CONFIG_SMP))
> + return -ENODEV;
> +
> + mp_wake = (struct acpi_madt_multiproc_wakeup *) header;
> + if (BAD_MADT_ENTRY(mp_wake, end))
> + return -EINVAL;
> +
> + acpi_table_print_madt_entry(&header->common);
> +
> + acpi_mp_wake_mailbox_paddr = mp_wake->base_address;
> +
> + acpi_wake_cpu_handler_update(acpi_wakeup_cpu);
> +
> + return 0;
> +}
> #endif /* CONFIG_X86_LOCAL_APIC */
>
> #ifdef CONFIG_X86_IO_APIC
> @@ -1284,6 +1334,12 @@ static void __init acpi_process_madt(void)
>
> smp_found_config = 1;
> }
> +
> + /*
> + * Parse MADT MP Wake entry.
> + */
> + acpi_table_parse_madt(ACPI_MADT_TYPE_MULTIPROC_WAKEUP,
> + acpi_parse_mp_wake, 1);
> }
> if (error == -EINVAL) {
> /*
> diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c
> index a61f642b1b90..d450014841b2 100644
> --- a/arch/x86/kernel/apic/probe_32.c
> +++ b/arch/x86/kernel/apic/probe_32.c
> @@ -207,3 +207,11 @@ int __init default_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
> }
> return 0;
> }
> +
> +void __init acpi_wake_cpu_handler_update(wakeup_cpu_handler handler)
> +{
> + struct apic **drv;
> +
> + for (drv = __apicdrivers; drv < __apicdrivers_end; drv++)
> + (*drv)->wakeup_secondary_cpu = handler;
> +}
> diff --git a/arch/x86/kernel/apic/probe_64.c b/arch/x86/kernel/apic/probe_64.c
> index c46720f185c0..986dbb68d3c4 100644
> --- a/arch/x86/kernel/apic/probe_64.c
> +++ b/arch/x86/kernel/apic/probe_64.c
> @@ -50,3 +50,11 @@ int __init default_acpi_madt_oem_check(char *oem_id, char *oem_table_id)
> }
> return 0;
> }
> +
> +void __init acpi_wake_cpu_handler_update(wakeup_cpu_handler handler)
> +{
> + struct apic **drv;
> +
> + for (drv = __apicdrivers; drv < __apicdrivers_end; drv++)
> + (*drv)->wakeup_secondary_cpu = handler;
> +}
Although I've looked at this patch already, I now realize that
according to the spec the mailbox is only suitable for the first AP
wakeup during system initialization.
Shouldn't the original handler be restored then to handle subsequent wakeups?
On 4/23/21 5:33 AM, Rafael J. Wysocki wrote:
> On Thu, Apr 22, 2021 at 11:47 PM Kuppuswamy Sathyanarayanan
> <[email protected]> wrote:
>>
>> ACPICA commit f1ee04207a212f6c519441e7e25397649ebc4cea
>>
>> Add Multiprocessor Wakeup Mailbox Structure definition. It is useful
>> in parsing MADT Wake table.
>>
>> Link: https://github.com/acpica/acpica/commit/f1ee0420
>> Signed-off-by: Kuppuswamy Sathyanarayanan <[email protected]>
>
> I'm expecting to receive this patch from Erik at one point, so I'm
> guessing that it's been added to this series for reference.
Yes. Since I had dependent patches, Erik sent it to me for
submission.
>
>> ---
>> include/acpi/actbl2.h | 14 ++++++++++++++
>> 1 file changed, 14 insertions(+)
>>
>> diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
>> index b2362600b9ff..7dce422f6119 100644
>> --- a/include/acpi/actbl2.h
>> +++ b/include/acpi/actbl2.h
>> @@ -733,6 +733,20 @@ struct acpi_madt_multiproc_wakeup {
>> u64 base_address;
>> };
>>
>> +#define ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE 2032
>> +#define ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE 2048
>> +
>> +struct acpi_madt_multiproc_wakeup_mailbox {
>> + u16 command;
>> + u16 reserved; /* reserved - must be zero */
>> + u32 apic_id;
>> + u64 wakeup_vector;
>> + u8 reserved_os[ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE]; /* reserved for OS use */
>> + u8 reserved_firmware[ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE]; /* reserved for firmware use */
>> +};
>> +
>> +#define ACPI_MP_WAKE_COMMAND_WAKEUP 1
>> +
>> /*
>> * Common flags fields for MADT subtables
>> */
>> --
>> 2.25.1
>>
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
On 4/23/21 6:05 AM, Rafael J. Wysocki wrote:
> Although I've looked at this patch already, I now realize that
> according to the spec the mailbox is only suitable for the first AP
> wakeup during system initialization.
>
> Shouldn't the original handler be restored then to handle subsequent wakeups?
For TDX use case, since it does not support CPU hotplug/offline features, once
the AP is turned on, it will never be off-lined and hence need not support
subsequent wakeups.
AFAIK, this MADT wake table is mainly defined for TDX use case.
Please also check the TDX GHCI spec, sec 4.1.
For each TD-guest, application processor, the mailbox can be used once for the
wakeup command. After the guest-application processor takes the action according
to the command, the intention is the mailbox will no longer be checked by the
guest-application processor. Other guest processors can continue using the mailbox
for the next command.
--
Sathyanarayanan Kuppuswamy
Linux Kernel Developer
> -----Original Message-----
> From: Rafael J. Wysocki <[email protected]>
> Sent: Friday, April 23, 2021 5:34 AM
> To: Kuppuswamy Sathyanarayanan
> <[email protected]>; Kaneda, Erik
> <[email protected]>
> Cc: Rafael J Wysocki <[email protected]>; Thomas Gleixner
> <[email protected]>; Ingo Molnar <[email protected]>; H . Peter Anvin
> <[email protected]>; Peter Zijlstra <[email protected]>; Len Brown
> <[email protected]>; Moore, Robert <[email protected]>; ACPI Devel
> Maling List <[email protected]>; open list:ACPI COMPONENT
> ARCHITECTURE (ACPICA) <[email protected]>; Linux Kernel Mailing List
> <[email protected]>; the arch/x86 maintainers
> <[email protected]>
> Subject: Re: [PATCH v2 1/3] ACPICA: ACPI 6.4: MADT: add Multiprocessor
> Wakeup Mailbox Structure
>
> On Thu, Apr 22, 2021 at 11:47 PM Kuppuswamy Sathyanarayanan
> <[email protected]> wrote:
> >
> > ACPICA commit f1ee04207a212f6c519441e7e25397649ebc4cea
> >
> > Add Multiprocessor Wakeup Mailbox Structure definition. It is useful
> > in parsing MADT Wake table.
> >
> > Link: https://github.com/acpica/acpica/commit/f1ee0420
> > Signed-off-by: Kuppuswamy Sathyanarayanan
> <[email protected]>
>
> I'm expecting to receive this patch from Erik at one point, so I'm
> guessing that it's been added to this series for reference.
Hi Rafael,
This patch has been Linux-ized so that you can take this and the SVKL patch. I was told that this was urgent for this merge window. Since they were simple, I thought it would be easier to have Sathya submit to Linux and ACPICA upstream separately. Going through this process made things a little more difficult so I'll be sure to communicate with you ahead of time if I do this again.
Erik
>
> > ---
> > include/acpi/actbl2.h | 14 ++++++++++++++
> > 1 file changed, 14 insertions(+)
> >
> > diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
> > index b2362600b9ff..7dce422f6119 100644
> > --- a/include/acpi/actbl2.h
> > +++ b/include/acpi/actbl2.h
> > @@ -733,6 +733,20 @@ struct acpi_madt_multiproc_wakeup {
> > u64 base_address;
> > };
> >
> > +#define ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE 2032
> > +#define ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE 2048
> > +
> > +struct acpi_madt_multiproc_wakeup_mailbox {
> > + u16 command;
> > + u16 reserved; /* reserved - must be zero */
> > + u32 apic_id;
> > + u64 wakeup_vector;
> > + u8 reserved_os[ACPI_MULTIPROC_WAKEUP_MB_OS_SIZE]; /*
> reserved for OS use */
> > + u8
> reserved_firmware[ACPI_MULTIPROC_WAKEUP_MB_FIRMWARE_SIZE]; /*
> reserved for firmware use */
> > +};
> > +
> > +#define ACPI_MP_WAKE_COMMAND_WAKEUP 1
> > +
> > /*
> > * Common flags fields for MADT subtables
> > */
> > --
> > 2.25.1
> >
On Fri, Apr 23, 2021 at 7:58 PM Kuppuswamy, Sathyanarayanan
<[email protected]> wrote:
>
>
>
> On 4/23/21 6:05 AM, Rafael J. Wysocki wrote:
> > Although I've looked at this patch already, I now realize that
> > according to the spec the mailbox is only suitable for the first AP
> > wakeup during system initialization.
> >
> > Shouldn't the original handler be restored then to handle subsequent wakeups?
>
> For TDX use case, since it does not support CPU hotplug/offline features, once
> the AP is turned on, it will never be off-lined and hence need not support
> subsequent wakeups.
>
> AFAIK, this MADT wake table is mainly defined for TDX use case.
>
> Please also check the TDX GHCI spec, sec 4.1.
>
> For each TD-guest, application processor, the mailbox can be used once for the
> wakeup command. After the guest-application processor takes the action according
> to the command, the intention is the mailbox will no longer be checked by the
> guest-application processor. Other guest processors can continue using the mailbox
> for the next command.
But this patch modifies generic code and the ACPI mechanism used by it
is generic. TDX is not even mentioned by the spec section pointed to
by the changelog.
IMO it should at least ensure that the AP wakeup callback will return
an error on an attempt to use it more than once on a given CPU.