| Message ID | 20210422214708.716164-4-sathyanarayanan.kuppuswamy@linux.intel.com (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | Add multiprocessor wake-up support | expand |
On Thu, Apr 22, 2021 at 11:47 PM Kuppuswamy Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> 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> > --- > 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 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.
On Fri, Apr 23, 2021 at 7:58 PM Kuppuswamy, Sathyanarayanan <sathyanarayanan.kuppuswamy@linux.intel.com> 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.
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; +}