* [PATCH v3 0/3] Add multiprocessor wake-up support @ 2021-04-26 2:39 Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 1/3] ACPICA: ACPI 6.4: MADT: add Multiprocessor Wakeup Mailbox Structure Kuppuswamy Sathyanarayanan ` (2 more replies) 0 siblings, 3 replies; 25+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Robert Moore, Erik Kaneda, linux-acpi, devel, linux-kernel, x86, Kuppuswamy Sathyanarayanan 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 v2: * Moved acpi_wake_cpu_handler_update() definition to arch/x86/kernel/apic/apic.c * Added comments for WRITE_ONCE() usage. * Added error handling support to prevent re-trigger of acpi_wakeup_cpu() and to detect the failure case. 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 | 79 +++++++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c | 8 ++++ drivers/acpi/tables.c | 11 ++++++ include/acpi/actbl2.h | 14 +++++++ 5 files changed, 115 insertions(+) -- 2.25.1 ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v3 1/3] ACPICA: ACPI 6.4: MADT: add Multiprocessor Wakeup Mailbox Structure 2021-04-26 2:39 [PATCH v3 0/3] Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 ` Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 2/3] ACPI/table: Print MADT Wake table information Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan 2 siblings, 0 replies; 25+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Robert Moore, Erik Kaneda, linux-acpi, devel, linux-kernel, x86, Kuppuswamy Sathyanarayanan 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 <sathyanarayanan.kuppuswamy@linux.intel.com> --- 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 2/3] ACPI/table: Print MADT Wake table information 2021-04-26 2:39 [PATCH v3 0/3] Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 1/3] ACPICA: ACPI 6.4: MADT: add Multiprocessor Wakeup Mailbox Structure Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 ` Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan 2 siblings, 0 replies; 25+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Robert Moore, Erik Kaneda, linux-acpi, devel, linux-kernel, x86, Kuppuswamy Sathyanarayanan, Rafael J . Wysocki 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. Acked-by: Rafael J. Wysocki <rafael.j.wysocki@intel.com> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> --- 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 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-04-26 2:39 [PATCH v3 0/3] Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 1/3] ACPICA: ACPI 6.4: MADT: add Multiprocessor Wakeup Mailbox Structure Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 2/3] ACPI/table: Print MADT Wake table information Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 ` Kuppuswamy Sathyanarayanan 2021-05-10 16:32 ` Kuppuswamy, Sathyanarayanan 2 siblings, 1 reply; 25+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2021-04-26 2:39 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Robert Moore, Erik Kaneda, linux-acpi, devel, linux-kernel, x86, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen 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 <sean.j.christopherson@intel.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> Reviewed-by: Andi Kleen <ak@linux.intel.com> --- arch/x86/include/asm/apic.h | 3 ++ arch/x86/kernel/acpi/boot.c | 79 +++++++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c | 8 ++++ 3 files changed, 90 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..fce2aa7d718f 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,52 @@ 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) +{ + u8 timeout = 0xFF; + + acpi_mp_wake_mailbox_init(); + + if (!acpi_mp_wake_mailbox) + return -EINVAL; + + /* + * Mailbox memory is shared between firmware and OS. Firmware will + * listen on mailbox command address, and once it receives the wakeup + * command, CPU associated with the given apicid will be booted. So, + * the value of apic_id and wakeup_vector has to be set before updating + * the wakeup command. So use WRITE_ONCE to let the compiler know about + * it and preserve the order of writes. + */ + 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); + + /* + * After writing wakeup command, wait for maximum timeout of 0xFF + * for firmware to reset the command address back zero to indicate + * the successful reception of command. + * NOTE: 255 as timeout value is decided based on our experiments. + * + * XXX: Change the timeout once ACPI specification comes up with + * standard maximum timeout value. + */ + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) + cpu_relax(); + + /* If timedout, return error */ + return timeout ? 0 : -EIO; +} + #endif /*CONFIG_X86_LOCAL_APIC */ #ifdef CONFIG_X86_IO_APIC @@ -1086,6 +1135,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 +1357,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/apic.c b/arch/x86/kernel/apic/apic.c index 4f26700f314d..f1b90a4b89e8 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) } EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); +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; +} + /* * Override the generic EOI implementation with an optimized version. * Only called during early boot when only one CPU is active and with -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-04-26 2:39 ` [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan @ 2021-05-10 16:32 ` Kuppuswamy, Sathyanarayanan 2021-05-10 16:55 ` [Devel] " Rafael J. Wysocki 0 siblings, 1 reply; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-10 16:32 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Robert Moore, Erik Kaneda, linux-acpi, devel, linux-kernel, x86, Sean Christopherson, Andi Kleen Hi Rafael/Thomas, On 4/25/21 7:39 PM, Kuppuswamy Sathyanarayanan 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. Gentle ping. Any comments on this patch? I think I have addressed the concerns raised by you in previous version. > > Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > Reviewed-by: Andi Kleen <ak@linux.intel.com> > --- > arch/x86/include/asm/apic.h | 3 ++ > arch/x86/kernel/acpi/boot.c | 79 +++++++++++++++++++++++++++++++++++++ > arch/x86/kernel/apic/apic.c | 8 ++++ > 3 files changed, 90 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..fce2aa7d718f 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,52 @@ 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) > +{ > + u8 timeout = 0xFF; > + > + acpi_mp_wake_mailbox_init(); > + > + if (!acpi_mp_wake_mailbox) > + return -EINVAL; > + > + /* > + * Mailbox memory is shared between firmware and OS. Firmware will > + * listen on mailbox command address, and once it receives the wakeup > + * command, CPU associated with the given apicid will be booted. So, > + * the value of apic_id and wakeup_vector has to be set before updating > + * the wakeup command. So use WRITE_ONCE to let the compiler know about > + * it and preserve the order of writes. > + */ > + 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); > + > + /* > + * After writing wakeup command, wait for maximum timeout of 0xFF > + * for firmware to reset the command address back zero to indicate > + * the successful reception of command. > + * NOTE: 255 as timeout value is decided based on our experiments. > + * > + * XXX: Change the timeout once ACPI specification comes up with > + * standard maximum timeout value. > + */ > + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > + cpu_relax(); > + > + /* If timedout, return error */ > + return timeout ? 0 : -EIO; > +} > + > #endif /*CONFIG_X86_LOCAL_APIC */ > > #ifdef CONFIG_X86_IO_APIC > @@ -1086,6 +1135,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 +1357,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/apic.c b/arch/x86/kernel/apic/apic.c > index 4f26700f314d..f1b90a4b89e8 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) > } > EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); > > +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; > +} > + > /* > * Override the generic EOI implementation with an optimized version. > * Only called during early boot when only one CPU is active and with > -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-10 16:55 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-10 16:55 UTC (permalink / raw) To: Kuppuswamy, Sathyanarayanan Cc: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson, Andi Kleen On Mon, May 10, 2021 at 6:33 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > Hi Rafael/Thomas, > > On 4/25/21 7:39 PM, Kuppuswamy Sathyanarayanan 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. > > Gentle ping. Any comments on this patch? I think I have addressed > the concerns raised by you in previous version. > > > > > Co-developed-by: Sean Christopherson <sean.j.christopherson@intel.com> > > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > > Reviewed-by: Andi Kleen <ak@linux.intel.com> > > --- > > arch/x86/include/asm/apic.h | 3 ++ > > arch/x86/kernel/acpi/boot.c | 79 +++++++++++++++++++++++++++++++++++++ > > arch/x86/kernel/apic/apic.c | 8 ++++ > > 3 files changed, 90 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..fce2aa7d718f 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,52 @@ 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) > > +{ > > + u8 timeout = 0xFF; > > + > > + acpi_mp_wake_mailbox_init(); > > + > > + if (!acpi_mp_wake_mailbox) > > + return -EINVAL; > > + > > + /* > > + * Mailbox memory is shared between firmware and OS. Firmware will > > + * listen on mailbox command address, and once it receives the wakeup > > + * command, CPU associated with the given apicid will be booted. So, > > + * the value of apic_id and wakeup_vector has to be set before updating > > + * the wakeup command. So use WRITE_ONCE to let the compiler know about > > + * it and preserve the order of writes. > > + */ > > + 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); > > + > > + /* > > + * After writing wakeup command, wait for maximum timeout of 0xFF > > + * for firmware to reset the command address back zero to indicate > > + * the successful reception of command. > > + * NOTE: 255 as timeout value is decided based on our experiments. > > + * > > + * XXX: Change the timeout once ACPI specification comes up with > > + * standard maximum timeout value. > > + */ > > + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > > + cpu_relax(); > > + > > + /* If timedout, return error */ > > + return timeout ? 0 : -EIO; I'm not sure how my comment regarding the fact that for a given CPU this function is only usable once has been addressed. While it may not be a practical concern in the use case that you are after (TDX), this is a generic mechanism and it needs to cover other possible usage scenarios. > > +} > > + > > #endif /*CONFIG_X86_LOCAL_APIC */ > > > > #ifdef CONFIG_X86_IO_APIC > > @@ -1086,6 +1135,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 +1357,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/apic.c b/arch/x86/kernel/apic/apic.c > > index 4f26700f314d..f1b90a4b89e8 100644 > > --- a/arch/x86/kernel/apic/apic.c > > +++ b/arch/x86/kernel/apic/apic.c > > @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) > > } > > EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); > > > > +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; > > +} > > + > > /* > > * Override the generic EOI implementation with an optimized version. > > * Only called during early boot when only one CPU is active and with > > > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* [Devel] Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-10 16:55 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-10 16:55 UTC (permalink / raw) To: devel [-- Attachment #1: Type: text/plain, Size: 7488 bytes --] On Mon, May 10, 2021 at 6:33 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> wrote: > > Hi Rafael/Thomas, > > On 4/25/21 7:39 PM, Kuppuswamy Sathyanarayanan 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. > > Gentle ping. Any comments on this patch? I think I have addressed > the concerns raised by you in previous version. > > > > > Co-developed-by: Sean Christopherson <sean.j.christopherson(a)intel.com> > > Signed-off-by: Sean Christopherson <sean.j.christopherson(a)intel.com> > > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> > > Reviewed-by: Andi Kleen <ak(a)linux.intel.com> > > --- > > arch/x86/include/asm/apic.h | 3 ++ > > arch/x86/kernel/acpi/boot.c | 79 +++++++++++++++++++++++++++++++++++++ > > arch/x86/kernel/apic/apic.c | 8 ++++ > > 3 files changed, 90 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..fce2aa7d718f 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,52 @@ 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) > > +{ > > + u8 timeout = 0xFF; > > + > > + acpi_mp_wake_mailbox_init(); > > + > > + if (!acpi_mp_wake_mailbox) > > + return -EINVAL; > > + > > + /* > > + * Mailbox memory is shared between firmware and OS. Firmware will > > + * listen on mailbox command address, and once it receives the wakeup > > + * command, CPU associated with the given apicid will be booted. So, > > + * the value of apic_id and wakeup_vector has to be set before updating > > + * the wakeup command. So use WRITE_ONCE to let the compiler know about > > + * it and preserve the order of writes. > > + */ > > + 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); > > + > > + /* > > + * After writing wakeup command, wait for maximum timeout of 0xFF > > + * for firmware to reset the command address back zero to indicate > > + * the successful reception of command. > > + * NOTE: 255 as timeout value is decided based on our experiments. > > + * > > + * XXX: Change the timeout once ACPI specification comes up with > > + * standard maximum timeout value. > > + */ > > + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > > + cpu_relax(); > > + > > + /* If timedout, return error */ > > + return timeout ? 0 : -EIO; I'm not sure how my comment regarding the fact that for a given CPU this function is only usable once has been addressed. While it may not be a practical concern in the use case that you are after (TDX), this is a generic mechanism and it needs to cover other possible usage scenarios. > > +} > > + > > #endif /*CONFIG_X86_LOCAL_APIC */ > > > > #ifdef CONFIG_X86_IO_APIC > > @@ -1086,6 +1135,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 +1357,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/apic.c b/arch/x86/kernel/apic/apic.c > > index 4f26700f314d..f1b90a4b89e8 100644 > > --- a/arch/x86/kernel/apic/apic.c > > +++ b/arch/x86/kernel/apic/apic.c > > @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) > > } > > EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); > > > > +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; > > +} > > + > > /* > > * Override the generic EOI implementation with an optimized version. > > * Only called during early boot when only one CPU is active and with > > > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-10 16:55 ` [Devel] " Rafael J. Wysocki (?) @ 2021-05-10 17:10 ` Kuppuswamy, Sathyanarayanan 2021-05-10 17:22 ` Andi Kleen 2021-05-10 17:23 ` [Devel] " Rafael J. Wysocki -1 siblings, 2 replies; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-10 17:10 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson, Andi Kleen On 5/10/21 9:55 AM, Rafael J. Wysocki wrote: > I'm not sure how my comment regarding the fact that for a given CPU > this function is only usable once has been addressed. > > While it may not be a practical concern in the use case that you are > after (TDX), this is a generic mechanism and it needs to cover other > possible usage scenarios. For the same CPU, if we try to use mailbox again, firmware will not respond to it. So the command will timeout and return error. -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-10 17:10 ` Kuppuswamy, Sathyanarayanan @ 2021-05-10 17:22 ` Andi Kleen 2021-05-10 17:24 ` [Devel] " Rafael J. Wysocki 2021-05-10 17:23 ` [Devel] " Rafael J. Wysocki 1 sibling, 1 reply; 25+ messages in thread From: Andi Kleen @ 2021-05-10 17:22 UTC (permalink / raw) To: Kuppuswamy, Sathyanarayanan Cc: Rafael J. Wysocki, Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson On Mon, May 10, 2021 at 10:10:24AM -0700, Kuppuswamy, Sathyanarayanan wrote: > > > On 5/10/21 9:55 AM, Rafael J. Wysocki wrote: > > I'm not sure how my comment regarding the fact that for a given CPU > > this function is only usable once has been addressed. > > > > While it may not be a practical concern in the use case that you are > > after (TDX), this is a generic mechanism and it needs to cover other > > possible usage scenarios. > > For the same CPU, if we try to use mailbox again, firmware will not > respond to it. So the command will timeout and return error. Right because the firmware code doesn't run anymore. The only possibility would be for Linux to put back some code that spins and waits again, but that would be quite pointless and wasteful. -Andi ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-10 17:24 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-10 17:24 UTC (permalink / raw) To: Andi Kleen Cc: Kuppuswamy, Sathyanarayanan, Rafael J. Wysocki, Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson On Mon, May 10, 2021 at 7:22 PM Andi Kleen <ak@linux.intel.com> wrote: > > On Mon, May 10, 2021 at 10:10:24AM -0700, Kuppuswamy, Sathyanarayanan wrote: > > > > > > On 5/10/21 9:55 AM, Rafael J. Wysocki wrote: > > > I'm not sure how my comment regarding the fact that for a given CPU > > > this function is only usable once has been addressed. > > > > > > While it may not be a practical concern in the use case that you are > > > after (TDX), this is a generic mechanism and it needs to cover other > > > possible usage scenarios. > > > > For the same CPU, if we try to use mailbox again, firmware will not > > respond to it. So the command will timeout and return error. > > Right because the firmware code doesn't run anymore. > > The only possibility would be for Linux to put back some code that spins > and waits again, but that would be quite pointless and wasteful. The wakeup function can return an error when it is called for the second time on the same CPU. ^ permalink raw reply [flat|nested] 25+ messages in thread
* [Devel] Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-10 17:24 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-10 17:24 UTC (permalink / raw) To: devel [-- Attachment #1: Type: text/plain, Size: 992 bytes --] On Mon, May 10, 2021 at 7:22 PM Andi Kleen <ak(a)linux.intel.com> wrote: > > On Mon, May 10, 2021 at 10:10:24AM -0700, Kuppuswamy, Sathyanarayanan wrote: > > > > > > On 5/10/21 9:55 AM, Rafael J. Wysocki wrote: > > > I'm not sure how my comment regarding the fact that for a given CPU > > > this function is only usable once has been addressed. > > > > > > While it may not be a practical concern in the use case that you are > > > after (TDX), this is a generic mechanism and it needs to cover other > > > possible usage scenarios. > > > > For the same CPU, if we try to use mailbox again, firmware will not > > respond to it. So the command will timeout and return error. > > Right because the firmware code doesn't run anymore. > > The only possibility would be for Linux to put back some code that spins > and waits again, but that would be quite pointless and wasteful. The wakeup function can return an error when it is called for the second time on the same CPU. ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-10 17:24 ` [Devel] " Rafael J. Wysocki (?) @ 2021-05-10 21:15 ` Kuppuswamy, Sathyanarayanan 2021-05-11 10:45 ` [Devel] " Rafael J. Wysocki -1 siblings, 1 reply; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-10 21:15 UTC (permalink / raw) To: Rafael J. Wysocki, Andi Kleen Cc: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson On 5/10/21 10:24 AM, Rafael J. Wysocki wrote: > The wakeup function can return an error when it is called for the > second time on the same CPU. To do this, we can only maintain the wakeup status of the CPUs. Can you check whether following physid_mask based status maintenance is acceptable? --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -67,6 +67,7 @@ static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE; static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox; static u64 acpi_mp_wake_mailbox_paddr; +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; #ifdef CONFIG_X86_IO_APIC /* @@ -347,6 +348,13 @@ static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) acpi_mp_wake_mailbox_init(); + /* Check if the given CPU (apicid) is already awake */ + if (physid_isset(apicid, apic_id_wakemap)) { + pr_err("APIC ID %x is already awake, so failed to wakeup\n", + apicid); + return -EINVAL; + } + if (!acpi_mp_wake_mailbox) return -EINVAL; @@ -374,8 +382,18 @@ static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) cpu_relax(); - /* If timedout, return error */ - return timeout ? 0 : -EIO; + if (timeout) { + /* + * If the CPU wakeup process is successful, store the + * status in apic_id_wakemap to prevent re-wakeup + * requests. + */ + physid_set(apicid, apic_id_wakemap); + return 0; + } + + /* If timed out (timeout == 0), return error */ + return -EIO; } -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-11 10:45 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-11 10:45 UTC (permalink / raw) To: Kuppuswamy, Sathyanarayanan Cc: Rafael J. Wysocki, Andi Kleen, Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson On Mon, May 10, 2021 at 11:15 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > On 5/10/21 10:24 AM, Rafael J. Wysocki wrote: > > The wakeup function can return an error when it is called for the > > second time on the same CPU. > > To do this, we can only maintain the wakeup status of the CPUs. Can > you check whether following physid_mask based status maintenance is > acceptable? It would work for me except for a couple of nits below. > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -67,6 +67,7 @@ static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE; > > static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox; > static u64 acpi_mp_wake_mailbox_paddr; > +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; > > #ifdef CONFIG_X86_IO_APIC > /* > @@ -347,6 +348,13 @@ static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > > acpi_mp_wake_mailbox_init(); > > + /* Check if the given CPU (apicid) is already awake */ The reason why is that the wakeup mechanism used here is only usable once per CPU by the spec, so I would add this information to the comment. Maybe something like "According to the ACPI specification (ACPI 6.4, Section ...), the mailbox-based wakeup mechanism cannot be used more than once for the same CPU, so avoid doing that." > + if (physid_isset(apicid, apic_id_wakemap)) { > + pr_err("APIC ID %x is already awake, so failed to wakeup\n", > + apicid); And I would reword the message like this "CPU already awake (APIC ID %x), skipping wakeup\n". > + return -EINVAL; > + } > + > if (!acpi_mp_wake_mailbox) Note, though, that instead of having this additional flag, you may as well create a mask that is fully populated initially and clear the IDs of the woken-up CPUs in it. Then, you'd only need one check here instead of two. > return -EINVAL; > > @@ -374,8 +382,18 @@ static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > cpu_relax(); > > - /* If timedout, return error */ > - return timeout ? 0 : -EIO; > + if (timeout) { > + /* > + * If the CPU wakeup process is successful, store the > + * status in apic_id_wakemap to prevent re-wakeup > + * requests. > + */ > + physid_set(apicid, apic_id_wakemap); > + return 0; > + } > + > + /* If timed out (timeout == 0), return error */ > + return -EIO; > } > > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* [Devel] Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-11 10:45 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-11 10:45 UTC (permalink / raw) To: devel [-- Attachment #1: Type: text/plain, Size: 2852 bytes --] On Mon, May 10, 2021 at 11:15 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> wrote: > > > > On 5/10/21 10:24 AM, Rafael J. Wysocki wrote: > > The wakeup function can return an error when it is called for the > > second time on the same CPU. > > To do this, we can only maintain the wakeup status of the CPUs. Can > you check whether following physid_mask based status maintenance is > acceptable? It would work for me except for a couple of nits below. > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -67,6 +67,7 @@ static u64 acpi_lapic_addr __initdata = APIC_DEFAULT_PHYS_BASE; > > static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox; > static u64 acpi_mp_wake_mailbox_paddr; > +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; > > #ifdef CONFIG_X86_IO_APIC > /* > @@ -347,6 +348,13 @@ static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > > acpi_mp_wake_mailbox_init(); > > + /* Check if the given CPU (apicid) is already awake */ The reason why is that the wakeup mechanism used here is only usable once per CPU by the spec, so I would add this information to the comment. Maybe something like "According to the ACPI specification (ACPI 6.4, Section ...), the mailbox-based wakeup mechanism cannot be used more than once for the same CPU, so avoid doing that." > + if (physid_isset(apicid, apic_id_wakemap)) { > + pr_err("APIC ID %x is already awake, so failed to wakeup\n", > + apicid); And I would reword the message like this "CPU already awake (APIC ID %x), skipping wakeup\n". > + return -EINVAL; > + } > + > if (!acpi_mp_wake_mailbox) Note, though, that instead of having this additional flag, you may as well create a mask that is fully populated initially and clear the IDs of the woken-up CPUs in it. Then, you'd only need one check here instead of two. > return -EINVAL; > > @@ -374,8 +382,18 @@ static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > cpu_relax(); > > - /* If timedout, return error */ > - return timeout ? 0 : -EIO; > + if (timeout) { > + /* > + * If the CPU wakeup process is successful, store the > + * status in apic_id_wakemap to prevent re-wakeup > + * requests. > + */ > + physid_set(apicid, apic_id_wakemap); > + return 0; > + } > + > + /* If timed out (timeout == 0), return error */ > + return -EIO; > } > > > -- > Sathyanarayanan Kuppuswamy > Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v4 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-11 10:45 ` [Devel] " Rafael J. Wysocki (?) @ 2021-05-13 21:37 ` Kuppuswamy Sathyanarayanan 2021-05-21 14:18 ` Kuppuswamy, Sathyanarayanan 2021-05-21 14:45 ` Peter Zijlstra -1 siblings, 2 replies; 25+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2021-05-13 21:37 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: linux-kernel, x86, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen 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 <sean.j.christopherson@intel.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> Reviewed-by: Andi Kleen <ak@linux.intel.com> --- Changes since v3: * Removed acpi_mp_wake_mailbox_init() and moved init code to acpi_wakeup_cpu(). * Removed redundant NULL pointer check for acpi_mp_wake_mailbox. * Added comments/debug prints as per Rafael's suggestion. * Removed MADT/SVKL ACPI patches from this patchset. It will be merged via ACPICA submission. arch/x86/include/asm/apic.h | 3 ++ arch/x86/kernel/acpi/boot.c | 95 +++++++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c | 8 ++++ 3 files changed, 106 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..e81fc605312d 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -65,6 +65,10 @@ 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; +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; + #ifdef CONFIG_X86_IO_APIC /* * Locks related to IOAPIC hotplug @@ -329,6 +333,67 @@ acpi_parse_lapic_nmi(union acpi_subtable_headers * header, const unsigned long e return 0; } +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) +{ + u8 timeout = 0xFF; + + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */ + if (physids_empty(apic_id_wakemap)) + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, + sizeof(*acpi_mp_wake_mailbox), + MEMREMAP_WB); + + /* + * According to the ACPI specification r6.4, sec 5.2.12.19, the + * mailbox-based wakeup mechanism cannot be used more than once + * for the same CPU, so skip sending wake commands to already + * awake CPU. + */ + if (physid_isset(apicid, apic_id_wakemap)) { + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n", + apicid); + return -EINVAL; + } + + + /* + * Mailbox memory is shared between firmware and OS. Firmware will + * listen on mailbox command address, and once it receives the wakeup + * command, CPU associated with the given apicid will be booted. So, + * the value of apic_id and wakeup_vector has to be set before updating + * the wakeup command. So use WRITE_ONCE to let the compiler know about + * it and preserve the order of writes. + */ + 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); + + /* + * After writing wakeup command, wait for maximum timeout of 0xFF + * for firmware to reset the command address back zero to indicate + * the successful reception of command. + * NOTE: 255 as timeout value is decided based on our experiments. + * + * XXX: Change the timeout once ACPI specification comes up with + * standard maximum timeout value. + */ + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) + cpu_relax(); + + if (timeout) { + /* + * If the CPU wakeup process is successful, store the + * status in apic_id_wakemap to prevent re-wakeup + * requests. + */ + physid_set(apicid, apic_id_wakemap); + return 0; + } + + /* If timed out (timeout == 0), return error */ + return -EIO; +} + #endif /*CONFIG_X86_LOCAL_APIC */ #ifdef CONFIG_X86_IO_APIC @@ -1086,6 +1151,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 +1373,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/apic.c b/arch/x86/kernel/apic/apic.c index 4f26700f314d..f1b90a4b89e8 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) } EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); +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; +} + /* * Override the generic EOI implementation with an optimized version. * Only called during early boot when only one CPU is active and with -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v4 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-13 21:37 ` [PATCH v4 1/1] " Kuppuswamy Sathyanarayanan @ 2021-05-21 14:18 ` Kuppuswamy, Sathyanarayanan 2021-05-21 14:45 ` Peter Zijlstra 1 sibling, 0 replies; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-21 14:18 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: linux-kernel, x86, Sean Christopherson, Andi Kleen Hi Rafael, On 5/13/21 2:37 PM, Kuppuswamy Sathyanarayanan 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<sean.j.christopherson@intel.com> > Signed-off-by: Sean Christopherson<sean.j.christopherson@intel.com> > Signed-off-by: Kuppuswamy Sathyanarayanan<sathyanarayanan.kuppuswamy@linux.intel.com> > Reviewed-by: Andi Kleen<ak@linux.intel.com> > --- Any comments on this patch? > > Changes since v3: > * Removed acpi_mp_wake_mailbox_init() and moved init code to > acpi_wakeup_cpu(). > * Removed redundant NULL pointer check for acpi_mp_wake_mailbox. > * Added comments/debug prints as per Rafael's suggestion. > * Removed MADT/SVKL ACPI patches from this patchset. It will be > merged via ACPICA submission. > > arch/x86/include/asm/apic.h | 3 ++ > arch/x86/kernel/acpi/boot.c | 95 +++++++++++++++++++++++++++++++++++++ > arch/x86/kernel/apic/apic.c | 8 ++++ > 3 files changed, 106 insertions(+) -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-13 21:37 ` [PATCH v4 1/1] " Kuppuswamy Sathyanarayanan 2021-05-21 14:18 ` Kuppuswamy, Sathyanarayanan @ 2021-05-21 14:45 ` Peter Zijlstra 2021-05-21 15:14 ` Kuppuswamy, Sathyanarayanan 1 sibling, 1 reply; 25+ messages in thread From: Peter Zijlstra @ 2021-05-21 14:45 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan Cc: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, linux-kernel, x86, Sean Christopherson, Andi Kleen On Thu, May 13, 2021 at 02:37:32PM -0700, Kuppuswamy Sathyanarayanan wrote: > +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > +{ > + u8 timeout = 0xFF; > + > + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */ > + if (physids_empty(apic_id_wakemap)) > + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, > + sizeof(*acpi_mp_wake_mailbox), > + MEMREMAP_WB); { } for being multi-line > + /* > + * According to the ACPI specification r6.4, sec 5.2.12.19, the > + * mailbox-based wakeup mechanism cannot be used more than once > + * for the same CPU, so skip sending wake commands to already > + * awake CPU. > + */ > + if (physid_isset(apicid, apic_id_wakemap)) { > + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n", > + apicid); > + return -EINVAL; > + } > + > + > + /* > + * Mailbox memory is shared between firmware and OS. Firmware will > + * listen on mailbox command address, and once it receives the wakeup > + * command, CPU associated with the given apicid will be booted. So, > + * the value of apic_id and wakeup_vector has to be set before updating > + * the wakeup command. So use WRITE_ONCE to let the compiler know about > + * it and preserve the order of writes. > + */ > + 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); Do those want to be smp_store_release(), in addition to being a volatile write, those also include compiler barriers to make sure the compiler doesn't lift stuff around. > + > + /* > + * After writing wakeup command, wait for maximum timeout of 0xFF > + * for firmware to reset the command address back zero to indicate > + * the successful reception of command. > + * NOTE: 255 as timeout value is decided based on our experiments. > + * > + * XXX: Change the timeout once ACPI specification comes up with > + * standard maximum timeout value. > + */ > + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > + cpu_relax(); What's the unit of the timeout? The mailbox reads, the PAUSE instruction? > + > + if (timeout) { > + /* > + * If the CPU wakeup process is successful, store the > + * status in apic_id_wakemap to prevent re-wakeup > + * requests. > + */ > + physid_set(apicid, apic_id_wakemap); > + return 0; > + } > + > + /* If timed out (timeout == 0), return error */ > + return -EIO; > +} > + > #endif /*CONFIG_X86_LOCAL_APIC */ ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v4 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-21 14:45 ` Peter Zijlstra @ 2021-05-21 15:14 ` Kuppuswamy, Sathyanarayanan 2021-05-24 6:02 ` [PATCH v5 " Kuppuswamy Sathyanarayanan 0 siblings, 1 reply; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-21 15:14 UTC (permalink / raw) To: Peter Zijlstra Cc: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, linux-kernel, x86, Sean Christopherson, Andi Kleen On 5/21/21 7:45 AM, Peter Zijlstra wrote: > On Thu, May 13, 2021 at 02:37:32PM -0700, Kuppuswamy Sathyanarayanan wrote: >> +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) >> +{ >> + u8 timeout = 0xFF; >> + >> + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */ >> + if (physids_empty(apic_id_wakemap)) >> + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, >> + sizeof(*acpi_mp_wake_mailbox), >> + MEMREMAP_WB); > > { } for being multi-line Yes. I will fix it. > >> + /* >> + * According to the ACPI specification r6.4, sec 5.2.12.19, the >> + * mailbox-based wakeup mechanism cannot be used more than once >> + * for the same CPU, so skip sending wake commands to already >> + * awake CPU. >> + */ >> + if (physid_isset(apicid, apic_id_wakemap)) { >> + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n", >> + apicid); >> + return -EINVAL; >> + } >> + >> + >> + /* >> + * Mailbox memory is shared between firmware and OS. Firmware will >> + * listen on mailbox command address, and once it receives the wakeup >> + * command, CPU associated with the given apicid will be booted. So, >> + * the value of apic_id and wakeup_vector has to be set before updating >> + * the wakeup command. So use WRITE_ONCE to let the compiler know about >> + * it and preserve the order of writes. >> + */ >> + 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); > > Do those want to be smp_store_release(), in addition to being a volatile > write, those also include compiler barriers to make sure the compiler > doesn't lift stuff around. I think we can use smp_store_release(). Let me test and add it in next version. > >> + >> + /* >> + * After writing wakeup command, wait for maximum timeout of 0xFF >> + * for firmware to reset the command address back zero to indicate >> + * the successful reception of command. >> + * NOTE: 255 as timeout value is decided based on our experiments. >> + * >> + * XXX: Change the timeout once ACPI specification comes up with >> + * standard maximum timeout value. >> + */ >> + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) >> + cpu_relax(); > > What's the unit of the timeout? The mailbox reads, the PAUSE > instruction? Read mailbox memory, timeout dec and then pause. Its more like busy wait loop. And timeout count is decided based on our experiments. Once spec defines a standard, we can modify it. > >> + >> + if (timeout) { >> + /* >> + * If the CPU wakeup process is successful, store the >> + * status in apic_id_wakemap to prevent re-wakeup >> + * requests. >> + */ >> + physid_set(apicid, apic_id_wakemap); >> + return 0; >> + } >> + >> + /* If timed out (timeout == 0), return error */ >> + return -EIO; >> +} >> + >> #endif /*CONFIG_X86_LOCAL_APIC */ -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* [PATCH v5 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-21 15:14 ` Kuppuswamy, Sathyanarayanan @ 2021-05-24 6:02 ` Kuppuswamy Sathyanarayanan 2021-05-24 6:40 ` Mika Penttilä 2021-05-24 14:51 ` Rafael J. Wysocki 0 siblings, 2 replies; 25+ messages in thread From: Kuppuswamy Sathyanarayanan @ 2021-05-24 6:02 UTC (permalink / raw) To: Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen, x86, linux-kernel 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 <sean.j.christopherson@intel.com> Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> Reviewed-by: Andi Kleen <ak@linux.intel.com> --- Changes since v4: * Used smp_store_release() in place of WRITE_ONCE(). * Addressed some checkpatch warnings. Changes since v3: * Removed acpi_mp_wake_mailbox_init() and moved init code to acpi_wakeup_cpu(). * Removed redundant NULL pointer check for acpi_mp_wake_mailbox. * Added comments/debug prints as per Rafael's suggestion. * Removed MADT/SVKL ACPI patches from this patchset. It will be merged via ACPICA submission. arch/x86/include/asm/apic.h | 3 ++ arch/x86/kernel/acpi/boot.c | 96 +++++++++++++++++++++++++++++++++++++ arch/x86/kernel/apic/apic.c | 8 ++++ 3 files changed, 107 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..c51134eb55d0 100644 --- a/arch/x86/kernel/acpi/boot.c +++ b/arch/x86/kernel/acpi/boot.c @@ -65,6 +65,10 @@ 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; +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; + #ifdef CONFIG_X86_IO_APIC /* * Locks related to IOAPIC hotplug @@ -329,6 +333,68 @@ acpi_parse_lapic_nmi(union acpi_subtable_headers * header, const unsigned long e return 0; } +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) +{ + u8 timeout = 0xFF; + + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */ + if (physids_empty(apic_id_wakemap)) { + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, + sizeof(*acpi_mp_wake_mailbox), + MEMREMAP_WB); + } + + /* + * According to the ACPI specification r6.4, sec 5.2.12.19, the + * mailbox-based wakeup mechanism cannot be used more than once + * for the same CPU, so skip sending wake commands to already + * awake CPU. + */ + if (physid_isset(apicid, apic_id_wakemap)) { + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n", + apicid); + return -EINVAL; + } + + /* + * Mailbox memory is shared between firmware and OS. Firmware will + * listen on mailbox command address, and once it receives the wakeup + * command, CPU associated with the given apicid will be booted. So, + * the value of apic_id and wakeup_vector has to be set before updating + * the wakeup command. So use smp_store_release to let the compiler know + * about it and preserve the order of writes. + */ + smp_store_release(&acpi_mp_wake_mailbox->apic_id, apicid); + smp_store_release(&acpi_mp_wake_mailbox->wakeup_vector, start_ip); + smp_store_release(&acpi_mp_wake_mailbox->command, + ACPI_MP_WAKE_COMMAND_WAKEUP); + + /* + * After writing wakeup command, wait for maximum timeout of 0xFF + * for firmware to reset the command address back zero to indicate + * the successful reception of command. + * NOTE: 255 as timeout value is decided based on our experiments. + * + * XXX: Change the timeout once ACPI specification comes up with + * standard maximum timeout value. + */ + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) + cpu_relax(); + + if (timeout) { + /* + * If the CPU wakeup process is successful, store the + * status in apic_id_wakemap to prevent re-wakeup + * requests. + */ + physid_set(apicid, apic_id_wakemap); + return 0; + } + + /* If timed out (timeout == 0), return error */ + return -EIO; +} + #endif /*CONFIG_X86_LOCAL_APIC */ #ifdef CONFIG_X86_IO_APIC @@ -1086,6 +1152,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 +1374,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/apic.c b/arch/x86/kernel/apic/apic.c index 4f26700f314d..f1b90a4b89e8 100644 --- a/arch/x86/kernel/apic/apic.c +++ b/arch/x86/kernel/apic/apic.c @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) } EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); +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; +} + /* * Override the generic EOI implementation with an optimized version. * Only called during early boot when only one CPU is active and with -- 2.25.1 ^ permalink raw reply related [flat|nested] 25+ messages in thread
* Re: [PATCH v5 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-24 6:02 ` [PATCH v5 " Kuppuswamy Sathyanarayanan @ 2021-05-24 6:40 ` Mika Penttilä 2021-05-24 13:42 ` Kuppuswamy, Sathyanarayanan 2021-05-24 14:51 ` Rafael J. Wysocki 1 sibling, 1 reply; 25+ messages in thread From: Mika Penttilä @ 2021-05-24 6:40 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan, Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen, x86, linux-kernel On 24.5.2021 9.02, Kuppuswamy Sathyanarayanan 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 <sean.j.christopherson@intel.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > Reviewed-by: Andi Kleen <ak@linux.intel.com> > --- > > Changes since v4: > * Used smp_store_release() in place of WRITE_ONCE(). > * Addressed some checkpatch warnings. > > Changes since v3: > * Removed acpi_mp_wake_mailbox_init() and moved init code to > acpi_wakeup_cpu(). > * Removed redundant NULL pointer check for acpi_mp_wake_mailbox. > * Added comments/debug prints as per Rafael's suggestion. > * Removed MADT/SVKL ACPI patches from this patchset. It will be > merged via ACPICA submission. > > arch/x86/include/asm/apic.h | 3 ++ > arch/x86/kernel/acpi/boot.c | 96 +++++++++++++++++++++++++++++++++++++ > arch/x86/kernel/apic/apic.c | 8 ++++ > 3 files changed, 107 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..c51134eb55d0 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -65,6 +65,10 @@ 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; > +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; > + > #ifdef CONFIG_X86_IO_APIC > /* > * Locks related to IOAPIC hotplug > @@ -329,6 +333,68 @@ acpi_parse_lapic_nmi(union acpi_subtable_headers * header, const unsigned long e > return 0; > } > So this isn't supporting suspend/resume if AP cannot started again? > +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > +{ > + u8 timeout = 0xFF; > + > + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */ > + if (physids_empty(apic_id_wakemap)) { > + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, > + sizeof(*acpi_mp_wake_mailbox), > + MEMREMAP_WB); > + } > + > + /* > + * According to the ACPI specification r6.4, sec 5.2.12.19, the > + * mailbox-based wakeup mechanism cannot be used more than once > + * for the same CPU, so skip sending wake commands to already > + * awake CPU. > + */ > + if (physid_isset(apicid, apic_id_wakemap)) { > + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n", > + apicid); > + return -EINVAL; > + } > + > + /* > + * Mailbox memory is shared between firmware and OS. Firmware will > + * listen on mailbox command address, and once it receives the wakeup > + * command, CPU associated with the given apicid will be booted. So, > + * the value of apic_id and wakeup_vector has to be set before updating > + * the wakeup command. So use smp_store_release to let the compiler know > + * about it and preserve the order of writes. > + */ > + smp_store_release(&acpi_mp_wake_mailbox->apic_id, apicid); > + smp_store_release(&acpi_mp_wake_mailbox->wakeup_vector, start_ip); > + smp_store_release(&acpi_mp_wake_mailbox->command, > + ACPI_MP_WAKE_COMMAND_WAKEUP); > + > + /* > + * After writing wakeup command, wait for maximum timeout of 0xFF > + * for firmware to reset the command address back zero to indicate > + * the successful reception of command. > + * NOTE: 255 as timeout value is decided based on our experiments. > + * > + * XXX: Change the timeout once ACPI specification comes up with > + * standard maximum timeout value. > + */ > + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > + cpu_relax(); > + > + if (timeout) { > + /* > + * If the CPU wakeup process is successful, store the > + * status in apic_id_wakemap to prevent re-wakeup > + * requests. > + */ > + physid_set(apicid, apic_id_wakemap); > + return 0; > + } > + > + /* If timed out (timeout == 0), return error */ > + return -EIO; > +} > + > #endif /*CONFIG_X86_LOCAL_APIC */ > > #ifdef CONFIG_X86_IO_APIC > @@ -1086,6 +1152,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 +1374,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/apic.c b/arch/x86/kernel/apic/apic.c > index 4f26700f314d..f1b90a4b89e8 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) > } > EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); > > +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; > +} > + > /* > * Override the generic EOI implementation with an optimized version. > * Only called during early boot when only one CPU is active and with ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-24 6:40 ` Mika Penttilä @ 2021-05-24 13:42 ` Kuppuswamy, Sathyanarayanan 0 siblings, 0 replies; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-24 13:42 UTC (permalink / raw) To: Mika Penttilä, Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra Cc: Len Brown, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen, x86, linux-kernel On 5/23/21 11:40 PM, Mika Penttilä wrote: > > So this isn't supporting suspend/resume if AP cannot started again? Yes. You are correct. It can be used only once per AP. Please find the spec reference below. /* * According to the ACPI specification r6.4, sec 5.2.12.19, the * mailbox-based wakeup mechanism cannot be used more than once * for the same CPU, so skip sending wake commands to already * awake CPU. */ -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-24 6:02 ` [PATCH v5 " Kuppuswamy Sathyanarayanan 2021-05-24 6:40 ` Mika Penttilä @ 2021-05-24 14:51 ` Rafael J. Wysocki 2021-05-24 15:35 ` Kuppuswamy, Sathyanarayanan 1 sibling, 1 reply; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-24 14:51 UTC (permalink / raw) To: Kuppuswamy Sathyanarayanan Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen, x86, linux-kernel On Monday, May 24, 2021 8:02:21 AM CEST Kuppuswamy Sathyanarayanan 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 <sean.j.christopherson@intel.com> > Signed-off-by: Sean Christopherson <sean.j.christopherson@intel.com> > Signed-off-by: Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> > Reviewed-by: Andi Kleen <ak@linux.intel.com> It would help if you CCed this to linux-acpi@vger.kernel.org. > --- > > Changes since v4: > * Used smp_store_release() in place of WRITE_ONCE(). > * Addressed some checkpatch warnings. > > Changes since v3: > * Removed acpi_mp_wake_mailbox_init() and moved init code to > acpi_wakeup_cpu(). > * Removed redundant NULL pointer check for acpi_mp_wake_mailbox. > * Added comments/debug prints as per Rafael's suggestion. > * Removed MADT/SVKL ACPI patches from this patchset. It will be > merged via ACPICA submission. > > arch/x86/include/asm/apic.h | 3 ++ > arch/x86/kernel/acpi/boot.c | 96 +++++++++++++++++++++++++++++++++++++ > arch/x86/kernel/apic/apic.c | 8 ++++ > 3 files changed, 107 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..c51134eb55d0 100644 > --- a/arch/x86/kernel/acpi/boot.c > +++ b/arch/x86/kernel/acpi/boot.c > @@ -65,6 +65,10 @@ 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; > +static physid_mask_t apic_id_wakemap = PHYSID_MASK_NONE; > + > #ifdef CONFIG_X86_IO_APIC > /* > * Locks related to IOAPIC hotplug > @@ -329,6 +333,68 @@ acpi_parse_lapic_nmi(union acpi_subtable_headers * header, const unsigned long e > return 0; > } > > +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > +{ > + u8 timeout = 0xFF; > + > + /* Remap mailbox memory only for the first call to acpi_wakeup_cpu() */ > + if (physids_empty(apic_id_wakemap)) { > + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, > + sizeof(*acpi_mp_wake_mailbox), > + MEMREMAP_WB); > + } > + > + /* > + * According to the ACPI specification r6.4, sec 5.2.12.19, the > + * mailbox-based wakeup mechanism cannot be used more than once > + * for the same CPU, so skip sending wake commands to already > + * awake CPU. > + */ > + if (physid_isset(apicid, apic_id_wakemap)) { > + pr_err("CPU already awake (APIC ID %x), skipping wakeup\n", > + apicid); > + return -EINVAL; > + } > + > + /* > + * Mailbox memory is shared between firmware and OS. Firmware will > + * listen on mailbox command address, and once it receives the wakeup > + * command, CPU associated with the given apicid will be booted. So, > + * the value of apic_id and wakeup_vector has to be set before updating > + * the wakeup command. So use smp_store_release to let the compiler know > + * about it and preserve the order of writes. > + */ > + smp_store_release(&acpi_mp_wake_mailbox->apic_id, apicid); > + smp_store_release(&acpi_mp_wake_mailbox->wakeup_vector, start_ip); > + smp_store_release(&acpi_mp_wake_mailbox->command, > + ACPI_MP_WAKE_COMMAND_WAKEUP); > + > + /* > + * After writing wakeup command, wait for maximum timeout of 0xFF > + * for firmware to reset the command address back zero to indicate > + * the successful reception of command. > + * NOTE: 255 as timeout value is decided based on our experiments. > + * > + * XXX: Change the timeout once ACPI specification comes up with > + * standard maximum timeout value. > + */ > + while (READ_ONCE(acpi_mp_wake_mailbox->command) && timeout--) > + cpu_relax(); > + > + if (timeout) { > + /* > + * If the CPU wakeup process is successful, store the > + * status in apic_id_wakemap to prevent re-wakeup > + * requests. > + */ > + physid_set(apicid, apic_id_wakemap); > + return 0; > + } > + > + /* If timed out (timeout == 0), return error */ > + return -EIO; > +} > + > #endif /*CONFIG_X86_LOCAL_APIC */ > > #ifdef CONFIG_X86_IO_APIC > @@ -1086,6 +1152,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 +1374,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/apic.c b/arch/x86/kernel/apic/apic.c > index 4f26700f314d..f1b90a4b89e8 100644 > --- a/arch/x86/kernel/apic/apic.c > +++ b/arch/x86/kernel/apic/apic.c > @@ -2554,6 +2554,14 @@ u32 x86_msi_msg_get_destid(struct msi_msg *msg, bool extid) > } > EXPORT_SYMBOL_GPL(x86_msi_msg_get_destid); > > +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; > +} > + > /* > * Override the generic EOI implementation with an optimized version. > * Only called during early boot when only one CPU is active and with > ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v5 1/1] x86/acpi, x86/boot: Add multiprocessor wake-up support 2021-05-24 14:51 ` Rafael J. Wysocki @ 2021-05-24 15:35 ` Kuppuswamy, Sathyanarayanan 0 siblings, 0 replies; 25+ messages in thread From: Kuppuswamy, Sathyanarayanan @ 2021-05-24 15:35 UTC (permalink / raw) To: Rafael J. Wysocki Cc: Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Kuppuswamy Sathyanarayanan, Sean Christopherson, Andi Kleen, x86, linux-kernel On 5/24/21 7:51 AM, Rafael J. Wysocki wrote: > It would help if you CCed this tolinux-acpi@vger.kernel.org. Sorry. Since it did not touch code in drivers/acpi, I missed to add it. Shall I bump the version and resend it with proper CC? -- Sathyanarayanan Kuppuswamy Linux Kernel Developer ^ permalink raw reply [flat|nested] 25+ messages in thread
* Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-10 17:23 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-10 17:23 UTC (permalink / raw) To: Kuppuswamy, Sathyanarayanan Cc: Rafael J. Wysocki, Rafael J Wysocki, Thomas Gleixner, Ingo Molnar, H . Peter Anvin, Peter Zijlstra, Len Brown, Robert Moore, Erik Kaneda, ACPI Devel Maling List, open list:ACPI COMPONENT ARCHITECTURE (ACPICA), Linux Kernel Mailing List, the arch/x86 maintainers, Sean Christopherson, Andi Kleen On Mon, May 10, 2021 at 7:10 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> wrote: > > > > On 5/10/21 9:55 AM, Rafael J. Wysocki wrote: > > I'm not sure how my comment regarding the fact that for a given CPU > > this function is only usable once has been addressed. > > > > While it may not be a practical concern in the use case that you are > > after (TDX), this is a generic mechanism and it needs to cover other > > possible usage scenarios. > > For the same CPU, if we try to use mailbox again, firmware will not > respond to it. Well, theoretically, but what if it checks the mailbox every time for all CPUs? Or similar? > So the command will timeout and return error. If the firmware behaves as expected, but what if it doesn't? ^ permalink raw reply [flat|nested] 25+ messages in thread
* [Devel] Re: [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support @ 2021-05-10 17:23 ` Rafael J. Wysocki 0 siblings, 0 replies; 25+ messages in thread From: Rafael J. Wysocki @ 2021-05-10 17:23 UTC (permalink / raw) To: devel [-- Attachment #1: Type: text/plain, Size: 791 bytes --] On Mon, May 10, 2021 at 7:10 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy(a)linux.intel.com> wrote: > > > > On 5/10/21 9:55 AM, Rafael J. Wysocki wrote: > > I'm not sure how my comment regarding the fact that for a given CPU > > this function is only usable once has been addressed. > > > > While it may not be a practical concern in the use case that you are > > after (TDX), this is a generic mechanism and it needs to cover other > > possible usage scenarios. > > For the same CPU, if we try to use mailbox again, firmware will not > respond to it. Well, theoretically, but what if it checks the mailbox every time for all CPUs? Or similar? > So the command will timeout and return error. If the firmware behaves as expected, but what if it doesn't? ^ permalink raw reply [flat|nested] 25+ messages in thread
end of thread, other threads:[~2021-05-24 15:44 UTC | newest] Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-04-26 2:39 [PATCH v3 0/3] Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 1/3] ACPICA: ACPI 6.4: MADT: add Multiprocessor Wakeup Mailbox Structure Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 2/3] ACPI/table: Print MADT Wake table information Kuppuswamy Sathyanarayanan 2021-04-26 2:39 ` [PATCH v3 3/3] x86/acpi, x86/boot: Add multiprocessor wake-up support Kuppuswamy Sathyanarayanan 2021-05-10 16:32 ` Kuppuswamy, Sathyanarayanan 2021-05-10 16:55 ` Rafael J. Wysocki 2021-05-10 16:55 ` [Devel] " Rafael J. Wysocki 2021-05-10 17:10 ` Kuppuswamy, Sathyanarayanan 2021-05-10 17:22 ` Andi Kleen 2021-05-10 17:24 ` Rafael J. Wysocki 2021-05-10 17:24 ` [Devel] " Rafael J. Wysocki 2021-05-10 21:15 ` Kuppuswamy, Sathyanarayanan 2021-05-11 10:45 ` Rafael J. Wysocki 2021-05-11 10:45 ` [Devel] " Rafael J. Wysocki 2021-05-13 21:37 ` [PATCH v4 1/1] " Kuppuswamy Sathyanarayanan 2021-05-21 14:18 ` Kuppuswamy, Sathyanarayanan 2021-05-21 14:45 ` Peter Zijlstra 2021-05-21 15:14 ` Kuppuswamy, Sathyanarayanan 2021-05-24 6:02 ` [PATCH v5 " Kuppuswamy Sathyanarayanan 2021-05-24 6:40 ` Mika Penttilä 2021-05-24 13:42 ` Kuppuswamy, Sathyanarayanan 2021-05-24 14:51 ` Rafael J. Wysocki 2021-05-24 15:35 ` Kuppuswamy, Sathyanarayanan 2021-05-10 17:23 ` [PATCH v3 3/3] " Rafael J. Wysocki 2021-05-10 17:23 ` [Devel] " Rafael J. Wysocki
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.