On Thu, 2023-10-05 at 16:13 +0300, Kirill A. Shutemov wrote: > In order to prepare for the expansion of support for the ACPI MADT > wakeup method, the relevant code has been moved into a separate file. > A new configuration option has been introduced to clearly indicate > dependencies without the use of ifdefs.
Use imperative mood? - Move the relevant code into ... - Introduce a new configuration option to ... > > There have been no functional changes. > > Signed-off-by: Kirill A. Shutemov <[email protected]> > --- > arch/x86/Kconfig | 7 +++ > arch/x86/include/asm/acpi.h | 5 ++ > arch/x86/kernel/acpi/Makefile | 11 ++-- > arch/x86/kernel/acpi/boot.c | 86 +----------------------------- > arch/x86/kernel/acpi/madt_wakeup.c | 80 +++++++++++++++++++++++++++ > 5 files changed, 99 insertions(+), 90 deletions(-) > create mode 100644 arch/x86/kernel/acpi/madt_wakeup.c > > diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig > index 3154dbc49cf5..7368d254d01f 100644 > --- a/arch/x86/Kconfig > +++ b/arch/x86/Kconfig > @@ -1108,6 +1108,13 @@ config X86_LOCAL_APIC > depends on X86_64 || SMP || X86_32_NON_STANDARD || X86_UP_APIC || > PCI_MSI > select IRQ_DOMAIN_HIERARCHY > > +config X86_ACPI_MADT_WAKEUP > + def_bool y > + depends on X86_64 > + depends on ACPI > + depends on SMP > + depends on X86_LOCAL_APIC > + > config X86_IO_APIC > def_bool y > depends on X86_LOCAL_APIC || X86_UP_IOAPIC > diff --git a/arch/x86/include/asm/acpi.h b/arch/x86/include/asm/acpi.h > index c8a7fc23f63c..b536b5a6a57b 100644 > --- a/arch/x86/include/asm/acpi.h > +++ b/arch/x86/include/asm/acpi.h > @@ -73,6 +73,11 @@ static inline bool acpi_skip_set_wakeup_address(void) > > #define acpi_skip_set_wakeup_address acpi_skip_set_wakeup_address > > +union acpi_subtable_headers; > + > +int __init acpi_parse_mp_wake(union acpi_subtable_headers *header, > + const unsigned long end); > + > /* > * Check if the CPU can handle C2 and deeper > */ > diff --git a/arch/x86/kernel/acpi/Makefile b/arch/x86/kernel/acpi/Makefile > index fc17b3f136fe..8c7329c88a75 100644 > --- a/arch/x86/kernel/acpi/Makefile > +++ b/arch/x86/kernel/acpi/Makefile > @@ -1,11 +1,12 @@ > # SPDX-License-Identifier: GPL-2.0 > > -obj-$(CONFIG_ACPI) += boot.o > -obj-$(CONFIG_ACPI_SLEEP) += sleep.o wakeup_$(BITS).o > -obj-$(CONFIG_ACPI_APEI) += apei.o > -obj-$(CONFIG_ACPI_CPPC_LIB) += cppc.o > +obj-$(CONFIG_ACPI) += boot.o > +obj-$(CONFIG_ACPI_SLEEP) += sleep.o wakeup_$(BITS).o > +obj-$(CONFIG_ACPI_APEI) += apei.o > +obj-$(CONFIG_ACPI_CPPC_LIB) += cppc.o > +obj-$(CONFIG_X86_ACPI_MADT_WAKEUP) += madt_wakeup.o > > ifneq ($(CONFIG_ACPI_PROCESSOR),) > -obj-y += cstate.o > +obj-y += cstate.o > endif unintended code change? [...] > > diff --git a/arch/x86/kernel/acpi/madt_wakeup.c > b/arch/x86/kernel/acpi/madt_wakeup.c > new file mode 100644 > index 000000000000..1b9747bfd5b9 > --- /dev/null > +++ b/arch/x86/kernel/acpi/madt_wakeup.c > @@ -0,0 +1,80 @@ > +#include <linux/acpi.h> > +#include <asm/apic.h> Functions like memremap(), smp_store_release() and cpu_relax() are used in this file. Should we explicitly include the relevant headers? > + > +/* Physical address of the Multiprocessor Wakeup Structure mailbox */ > +static u64 acpi_mp_wake_mailbox_paddr; > +/* Virtual address of the Multiprocessor Wakeup Structure mailbox */ > +static struct acpi_madt_multiproc_wakeup_mailbox *acpi_mp_wake_mailbox; > + > +static int acpi_wakeup_cpu(int apicid, unsigned long start_ip) > +{ > + /* > + * Remap mailbox memory only for the first call to acpi_wakeup_cpu(). > + * > + * Wakeup of secondary CPUs is fully serialized in the core code. > + * No need to protect acpi_mp_wake_mailbox from concurrent accesses. > + */ > + if (!acpi_mp_wake_mailbox) { > + acpi_mp_wake_mailbox = memremap(acpi_mp_wake_mailbox_paddr, > + sizeof(*acpi_mp_wake_mailbox), > + MEMREMAP_WB); > + } > + > + /* > + * Mailbox memory is shared between the firmware and OS. Firmware will > + * listen on mailbox command address, and once it receives the wakeup > + * command, the CPU associated with the given apicid will be booted. > + * > + * The value of 'apic_id' and 'wakeup_vector' must be visible to the > + * firmware before the wakeup command is visible. smp_store_release() > + * ensures ordering and visibility. > + */ > + acpi_mp_wake_mailbox->apic_id = apicid; > + acpi_mp_wake_mailbox->wakeup_vector = start_ip; > + smp_store_release(&acpi_mp_wake_mailbox->command, > + ACPI_MP_WAKE_COMMAND_WAKEUP); > + > + /* > + * Wait for the CPU to wake up. > + * > + * The CPU being woken up is essentially in a spin loop waiting to be > + * woken up. It should not take long for it wake up and acknowledge by > + * zeroing out ->command. > + * > + * ACPI specification doesn't provide any guidance on how long kernel > + * has to wait for a wake up acknowledgement. It also doesn't provide > + * a way to cancel a wake up request if it takes too long. > + * > + * In TDX environment, the VMM has control over how long it takes to > + * wake up secondary. It can postpone scheduling secondary vCPU > + * indefinitely. Giving up on wake up request and reporting error opens > + * possible attack vector for VMM: it can wake up a secondary CPU when > + * kernel doesn't expect it. Wait until positive result of the wake up > + * request. > + */ > + while (READ_ONCE(acpi_mp_wake_mailbox->command)) > + cpu_relax(); > + > + return 0; > +} > + > +int __init acpi_parse_mp_wake(union acpi_subtable_headers *header, > + const unsigned long end) > +{ > + struct acpi_madt_multiproc_wakeup *mp_wake; > + > + if (!IS_ENABLED(CONFIG_SMP)) > + return -ENODEV; Now you have made X86_ACPI_MADT_WAKEUP depend on SMP, and this file will only be compiled when X86_ACPI_MADT_WAKEUP is turned on. IIUC this essentially means IS_ENABLED(CONFIG_SMP) will always report true? > + > + 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; > + > + apic_update_callback(wakeup_secondary_cpu_64, acpi_wakeup_cpu); > + > + return 0; > +} _______________________________________________ kexec mailing list [email protected] http://lists.infradead.org/mailman/listinfo/kexec
