ACPI/arm64: add support for virtual cpuhotplug
Subsystems (18)
v3 cover ACPI/arm64: add support for virtual cpuhotplug
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740c cpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.
2. New patch - simplification of PA-RISC's smp_prepare_boot_cpu()
3. Moved "ACPI: Use the acpi_device_is_present() helper in more places" and "ACPI: Rename acpi_scan_device_not_present() to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.
4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call arch_unregister_cpu() if HOTPLUG_CPU is selected" which looks to me to be a workaround for arch_unregister_cpu() being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.
I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
Please note that I haven't tested this beyond building for aarch64 at the present time.
The series can be found at:
git://git.armlinux.org.uk/~rmk/linux-arm.git aarch64/hotplug-vcpu/v6.6-rc7
Documentation/arch/arm64/cpu-hotplug.rst | 79 +++++++++++++++ Documentation/arch/arm64/index.rst | 1 + arch/arm64/Kconfig | 1 + arch/arm64/include/asm/acpi.h | 11 +++ arch/arm64/include/asm/cpu.h | 1 - arch/arm64/kernel/acpi_numa.c | 11 --- arch/arm64/kernel/psci.c | 2 +- arch/arm64/kernel/setup.c | 13 +-- arch/arm64/kernel/smp.c | 5 +- arch/ia64/Kconfig | 3 + arch/ia64/include/asm/acpi.h | 2 +- arch/ia64/include/asm/cpu.h | 6 -- arch/ia64/kernel/acpi.c | 6 +- arch/ia64/kernel/setup.c | 2 +- arch/ia64/kernel/topology.c | 35 +------ arch/loongarch/Kconfig | 2 + arch/loongarch/configs/loongson3_defconfig | 2 +- arch/loongarch/kernel/acpi.c | 4 +- arch/loongarch/kernel/topology.c | 38 +------- arch/parisc/kernel/smp.c | 8 +- arch/riscv/Kconfig | 1 + arch/riscv/kernel/setup.c | 19 +--- arch/x86/Kconfig | 3 + arch/x86/include/asm/cpu.h | 4 - arch/x86/kernel/acpi/boot.c | 4 +- arch/x86/kernel/cpu/intel_epb.c | 2 +- arch/x86/kernel/topology.c | 27 +----- drivers/acpi/Kconfig | 14 ++- drivers/acpi/acpi_processor.c | 151 +++++++++++++++++++++++------ drivers/acpi/bus.c | 16 +++ drivers/acpi/device_pm.c | 2 +- drivers/acpi/device_sysfs.c | 2 +- drivers/acpi/internal.h | 1 - drivers/acpi/processor_core.c | 2 +- drivers/acpi/property.c | 2 +- drivers/acpi/scan.c | 148 ++++++++++++++++++---------- drivers/base/arch_topology.c | 38 +++++--- drivers/base/cpu.c | 44 +++++++-- drivers/base/init.c | 2 +- drivers/base/node.c | 7 -- drivers/firmware/psci/psci.c | 2 + drivers/irqchip/irq-gic-v3.c | 38 +++++--- include/acpi/acpi_bus.h | 1 + include/acpi/actbl2.h | 1 + include/linux/acpi.h | 13 ++- include/linux/cpu.h | 4 + include/linux/cpumask.h | 25 +++++ kernel/cpu.c | 3 + 48 files changed, 516 insertions(+), 292 deletions(-)
Quoted 151 lines · James Morse
Hello!
Changes since RFC-v1:
* riscv is new, ia64 is gone
* The KVM support is different, and upstream - no need to patch the host.---
This series adds what looks like cpuhotplug support to arm64 for use in virtual machines. It does this by moving the
cpu_register()calls for architectures that support ACPI out of the arch code by usingGENERIC_CPU_DEVICES, then into the ACPI processor driver.The kubernetes folk really want to be able to add CPUs to an existing VM, in exactly the same way they do on x86. The use-case is pre-booting guests with one CPU, then adding the number that were actually needed when the workload is provisioned.
Wait? Doesn't arm64 support cpuhotplug already!?
In the arm world, cpuhotplug gets used to mean removing the power from a CPU. The CPU is offline, and remains present. For x86, and ACPI, cpuhotplug has the additional step of physically removing the CPU, so that it isn't present anymore.Arm64 doesn't support this, and can't support it: CPUs are really a slice of the SoC, and there is not enough information in the existing ACPI tables to describe which bits of the slice also got removed. Without a reference
machine: adding this support to the spec is a wild goose chase.Critically: everything described in the firmware tables must remain present.
For a virtual machine this is easy as all the other bits of 'virtual SoC' are emulated, so they can (and do) remain present when a vCPU is 'removed'.
On a system that supports cpuhotplug the MADT has to describe every possible CPU at boot. Under KVM, the vGIC needs to know about every possible vCPU before the guest is started.
With these constraints, virtual-cpuhotplug is really just a hypervisor/firmware policy about which CPUs can be brought online.This series adds support for virtual-cpuhotplug as exactly that: firmware policy. This may even work on a physical machine too; for a guest the part of firmware is played by the VMM. (typically Qemu).
PSCI support is modified to return 'DENIED' if the CPU can't be brought online/enabled yet. The CPU object's
_STAmethod's enabled bit is used to indicate firmware's current disposition. If the CPU has its enabled bit clear, it will not be registered with sysfs, and attempts to bring it online will fail. The notifications that_STAhas changed its value then work in the same way as physical hotplug, and firmware can cause the CPU to be registered some time later, allowing it to be brought online.This creates something that looks like cpuhotplug to user-space, as the sysfs files appear and disappear, and the udev notifications look the same.
One notable difference is the CPU present mask, which is exposed via sysfs. Because the CPUs remain present throughout, they can still be seen in that mask. This value does get used by webbrowsers to estimate the number of CPUs as the CPU online mask is constantly changed on mobile phones.
Linux is tolerant of PSCI returning errors, as its always been allowed to do that. To avoid confusing OS that can't tolerate this, we needed an additional bit in the MADT GICC flags. This series copies
ACPI_MADT_ONLINE_CAPABLE, which appears to be for this purpose, but calls itACPI_MADT_GICC_CPU_CAPABLEas it has a different bit position in the GICC.This code is unconditionally enabled for all ACPI architectures. If there are problems with firmware tables on some devices, the CPUs will already be online by the time the
acpi_processor_make_enabled()is called. A mismatch here causes a firmware-bug message and kernel taint. This should only affect people with broken firmware who also boot with maxcpus=1, and bring CPUs online later.I had a go at switching the remaining architectures over to
GENERIC_CPU_DEVICES, so that theKconfigsymbol can be removed, but I got stuck with powerpc and s390.I've only build tested Loongarch and riscv. I've removed the ia64 specific patches, but left the changes in other patches to make git-grep review of renames easier.
If folk want to play along at home, you'll need a copy of Qemu that supports this.
https://github.com/salil-mehta/qemu.git salil/virt-cpuhp-armv8/rfc-v2-rc6Replace your '-smp' argument with something like:
| -smp cpus=1,maxcpus=3,cores=3,threads=1,sockets=1then feed the following to the Qemu montior;
| (qemu)device_adddriver=host-arm-cpu,core-id=1,id=cpu1 | (qemu)device_delcpu1Why is this still an RFC? I'm still looking for confirmation from the kubernetes/kata folk that this works for them. Because of this I've culled the CC list...
This series is based on v6.6-rc1, and can be retrieved from:
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/ virtual_cpu_hotplug/rfc/v2Thanks,
James Morse (34):
ACPI: MoveACPI_HOTPLUG_CPUto be disabled on arm64 and riscv
drivers: base: Use present CPUs inGENERIC_CPU_DEVICES
drivers: base: Allow parts ofGENERIC_CPU_DEVICESto be overridden
drivers: base: Movecpu_dev_init()afternode_dev_init()
drivers: base: Print a warning instead ofpanic()whenregister_cpu()
fails
arm64: setup: Switch over toGENERIC_CPU_DEVICESusing
arch_register_cpu()
x86:intel_epb: Don't rely on link order
x86/topology: Switch over toGENERIC_CPU_DEVICES
LoongArch: Switch over toGENERIC_CPU_DEVICES
riscv: Switch over toGENERIC_CPU_DEVICES
arch_topology: Makeregister_cpu_capacity_sysctl()tolerant to late
CPUs
ACPI: Use theacpi_device_is_present()helper in more places
ACPI: Renameacpi_scan_device_not_present()to be about enumeration
ACPI: Only enumerate enabled (or functional) devices
ACPI: processor: Add support for processors described as container
packages
ACPI: processor: Register CPUs that are online, but not described in
the DSDT
ACPI: processor: Register all CPUs fromacpi_processor_get_info()
ACPI: RenameACPI_HOTPLUG_CPUto include 'present'
ACPI: Moveacpi_bus_trim_one()beforeacpi_scan_hot_remove()
ACPI: Renameacpi_processor_hotadd_initand remove pre-processor
guards
ACPI: Addpost_ejectto structacpi_scan_handlerfor cpu hotplug
ACPI: Check_STApresent bit before making CPUs not present
ACPI: Warn when the present bit changes but the feature is not enabled
drivers: base: Implement weakarch_unregister_cpu()
LoongArch: Use the__weakversion ofarch_unregister_cpu()
arm64: acpi: Moveget_cpu_for_acpi_id()to a header
ACPICA: Add new MADT GICC flags fields [code first?] arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a
helper
irqchip/gic-v3: Don't return errors fromgic_acpi_match_gicc()irqchip/gic-v3: Add support for ACPI's disabled but 'online capable'
CPUs
ACPI: add support to register CPUs based on the_STAenabled bit
arm64: document virtual CPU hotplug's expectations
ACPI: Add_OSCbits to advertise OS support for toggling CPU
present/enabled
cpumask: Add enabled cpumask for present CPUs that can be brought
onlineJean-Philippe Brucker (1):
arm64: psci: Ignore DENIED CPUs
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
PATCH 1/39 parisc: simplify smp_prepare_boot_cpu()
smp_prepare_boot_cpu() reads the cpuid of the first CPU, printing a message to state which processor booted, and setting it online and present.
This cpuid is retrieved from per_cpu(cpu_data, 0).cpuid, which is initialised in arch/parisc/kernel/processor.c:processor_probe() thusly:
p = &per_cpu(cpu_data, cpuid); ... p->cpuid = cpuid; /* save CPU id */
Consequently, the cpuid retrieved seems to be guaranteed to also be zero, meaning that the message printed in this boils down to:
pr_info("SMP: bootstrap CPU ID is 0\n");Moreover, since kernel/cpu.c::boot_cpu_init() already sets CPU 0 to be present and online, there is no need to do this again in smp_prepare_boot_cpu().
Remove this code, and simplify the printk().
--- arch/parisc/kernel/smp.c | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-)
arch/parisc/kernel/smp.c +1 -7
@@ -404,13 +404,7 @@ static int smp_boot_one_cpu(int cpuid, struct task_struct *idle)void __init smp_prepare_boot_cpu (void ) { -int bootstrap_processor =per_cpu (cpu_data,0 ).cpuid; - -/* Setup BSP mappings */ -printk (KERN_INFO "SMP: bootstrap CPU ID is %d\n" , bootstrap_processor); - -set_cpu_online (bootstrap_processor, true); -set_cpu_present (bootstrap_processor, true); +pr_info ("SMP: bootstrap CPU ID is 0\n" ); }
Signature · 1 line
2.30.2
PATCH 2/39 ACPI: Use the acpi_device_is_present() helper in more places
In reply to Russell King (Oracle)From: James Morse <[email protected]>
acpi_device_is_present() checks the present or functional bits from the cached copy of _STA.
A few places open-code this check. Use the helper instead to improve readability.
--- drivers/acpi/scan.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
drivers/acpi/scan.c +2 -2
@@ -304,7 +304,7 @@ static int acpi_scan_device_check(struct acpi_device *adev)int error;acpi_bus_get_status (adev); -if (adev->status.present || adev->status.functional) { +if (acpi_device_is_present (adev)) {/* * This function is only called for device objects for which * matching scan handlers exist. The only situation in which
@@ -338,7 +338,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used)int error;acpi_bus_get_status (adev); -if (!(adev->status.present || adev->status.functional)) { +if (!acpi_device_is_present (adev)) {acpi_scan_device_not_present (adev);return 0 ; }
Signature · 1 line
2.30.2
PATCH 3/39 ACPI: Rename acpi_scan_device_not_present() to be about enumeration
In reply to Russell King (Oracle)From: James Morse <[email protected]>
acpi_scan_device_not_present() is called when a device in the hierarchy is not available for enumeration. Historically enumeration was only based on whether the device was present.
To add support for only enumerating devices that are both present and enabled, this helper should be renamed. It was only ever about enumeration, rename it acpi_scan_device_not_enumerated().
No change in behaviour is intended.
--- drivers/acpi/scan.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/acpi/scan.c +4 -4
@@ -289,10 +289,10 @@ static int acpi_scan_hot_remove(struct acpi_device *device)return 0 ; } -static int acpi_scan_device_not_present (struct acpi_device *adev) +static int acpi_scan_device_not_enumerated (struct acpi_device *adev) {if (!acpi_device_enumerated (adev)) { -dev_warn (&adev->dev,"Still not present\n" ); +dev_warn (&adev->dev,"Still not enumerated\n" );return -EALREADY ; }acpi_bus_trim (adev);
@@ -327,7 +327,7 @@ static int acpi_scan_device_check(struct acpi_device *adev) error = -ENODEV ; } }else { - error =acpi_scan_device_not_present (adev); + error =acpi_scan_device_not_enumerated (adev); }return error; }
@@ -339,7 +339,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used)acpi_bus_get_status (adev);if (!acpi_device_is_present (adev)) { -acpi_scan_device_not_present (adev); +acpi_scan_device_not_enumerated (adev);return 0 ; }if (handler && handler->hotplug.scan_dependent)
Signature · 1 line
2.30.2
PATCH 4/39 arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper
In reply to Russell King (Oracle)From: James Morse <[email protected]>
ACPI, irqchip and the architecture code all inspect the MADT enabled bit for a GICC entry in the MADT.
The addition of an 'online capable' bit means all these sites need updating.
Move the current checks behind a helper to make future updates easier.
---
Changes since RFC v2:
* Remove unnecessary parens
* Moved earlier in series
--- arch/arm64/kernel/smp.c | 2 +- drivers/acpi/processor_core.c | 2 +- drivers/irqchip/irq-gic-v3.c | 10 ++++------ include/linux/acpi.h | 5 +++++ 4 files changed, 11 insertions(+), 8 deletions(-)
arch/arm64/kernel/smp.c +1 -1
@@ -520,7 +520,7 @@ acpi_map_gic_cpu_interface(struct acpi_madt_generic_interrupt *processor) {u64 hwid = processor->arm_mpidr; -if (!(processor->flags &ACPI_MADT_ENABLED )) { +if (!acpi_gicc_is_usable (processor)) {pr_debug ("skipping disabled CPU entry with 0x%llx MPIDR\n" , hwid);return ; }
drivers/acpi/processor_core.c +1 -1
@@ -90,7 +90,7 @@ static int map_gicc_mpidr(struct acpi_subtable_header *entry,struct acpi_madt_generic_interrupt *gicc = container_of(entry, struct acpi_madt_generic_interrupt, header); -if (!(gicc->flags &ACPI_MADT_ENABLED )) +if (!acpi_gicc_is_usable (gicc))return -ENODEV ;/* device_declaration means Device object in DSDT, in the
drivers/irqchip/irq-gic-v3.c +4 -6
@@ -2380,8 +2380,7 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header,u32 size = reg == GIC_PIDR2_ARCH_GICv4 ?SZ_64K *4 :SZ_64K *2 ;void __iomem *redist_base; -/* GICC entry which has !ACPI_MADT_ENABLED is not unusable so skip */ -if (!(gicc->flags &ACPI_MADT_ENABLED )) +if (!acpi_gicc_is_usable (gicc))return 0 ; redist_base =ioremap (gicc->gicr_base_address, size);
@@ -2431,7 +2430,7 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,* If GICC is enabled and has valid gicr base address, then it means * GICR base is presented via GICC */ -if ((gicc->flags &ACPI_MADT_ENABLED ) && gicc->gicr_base_address) { +if (acpi_gicc_is_usable (gicc) && gicc->gicr_base_address) { acpi_data.enabled_rdists++;return 0 ; }
@@ -2440,7 +2439,7 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,* It's perfectly valid firmware can pass disabled GICC entry, driver * should not treat as errors, skip the entry instead of probe fail. */ -if (!(gicc->flags &ACPI_MADT_ENABLED )) +if (!acpi_gicc_is_usable (gicc))return 0 ;return -ENODEV ;
@@ -2499,8 +2498,7 @@ static int __init gic_acpi_parse_virt_madt_gicc(union acpi_subtable_headers *heaint maint_irq_mode;static int first_madt = true; -/* Skip unusable CPUs */ -if (!(gicc->flags &ACPI_MADT_ENABLED )) +if (!acpi_gicc_is_usable (gicc))return 0 ; maint_irq_mode = (gicc->flags & ACPI_MADT_VGIC_IRQ_MODE) ?
include/linux/acpi.h +5 -0
@@ -256,6 +256,11 @@ acpi_table_parse_cedt(enum acpi_cedt_type id,int acpi_parse_mcfg (struct acpi_table_header *header);void acpi_table_print_madt_entry (struct acpi_subtable_header *madt); +static inline bool acpi_gicc_is_usable (struct acpi_madt_generic_interrupt *gicc) +{ +return gicc->flags &ACPI_MADT_ENABLED ; +} +/* the following numa functions are architecture-dependent */ void acpi_numa_slit_init (struct acpi_table_slit *slit);
Signature · 1 line
2.30.2
PATCH 5/39 ACPI: Move ACPI_HOTPLUG_CPU to be disabled on arm64 and riscv
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Neither arm64 nor riscv support physical hotadd of CPUs that were not present at boot. For arm64 much of the platform description is in static tables which do not have update methods. arm64 does support HOTPLUG_CPU, which is backed by a firmware interface to turn CPUs on and off.
acpi_processor_hotadd_init() and acpi_processor_remove() are for adding and removing CPUs that were not present at boot. arm64 systems that do this are not supported as there is currently insufficient information in the platform description. (e.g. did the GICR get removed too?)
arm64 currently relies on the MADT enabled flag check in map_gicc_mpidr() to prevent CPUs that were not described as present at boot from being added to the system. Similarly, riscv relies on the same check in map_rintc_hartid(). Both architectures also rely on the weak 'always fails' definitions of acpi_map_cpu() and arch_register_cpu().
Subsequent changes will redefine ACPI_HOTPLUG_CPU as making possible CPUs present. Neither arm64 nor riscv support this.
Disable ACPI_HOTPLUG_CPU for arm64 and riscv by removing 'default y' and selecting it on the other three ACPI architectures. This allows the weak definitions of some symbols to be removed.
---
Changes since RFC:
* Expanded conditions to avoid ACPI_HOTPLUG_CPU being enabled when HOTPLUG_CPU isn't.
--- arch/ia64/Kconfig | 1 + arch/loongarch/Kconfig | 1 + arch/x86/Kconfig | 1 + drivers/acpi/Kconfig | 1 - drivers/acpi/acpi_processor.c | 18 ------------------ 5 files changed, 3 insertions(+), 19 deletions(-)
arch/ia64/Kconfig +1 -0
@@ -16,6 +16,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_MIGHT_HAVE_PC_SERIO select ACPI + select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_NUMA if NUMA select ARCH_ENABLE_MEMORY_HOTPLUG select ARCH_ENABLE_MEMORY_HOTREMOVE
arch/loongarch/Kconfig +1 -0
@@ -5,6 +5,7 @@ config LOONGARCH select ACPI select ACPI_GENERIC_GSI if ACPI select ACPI_MCFG if ACPI + select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_PPTT if ACPI select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI select ARCH_BINFMT_ELF_STATE
arch/x86/Kconfig +1 -0
@@ -60,6 +60,7 @@ config X86 # select ACPI_LEGACY_TABLES_LOOKUP if ACPI select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI + select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU select ARCH_32BIT_OFF_T if X86_32 select ARCH_CLOCKSOURCE_INIT select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
drivers/acpi/Kconfig +0 -1
@@ -309,7 +309,6 @@ config ACPI_HOTPLUG_CPU bool depends on ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_CONTAINER - default y config ACPI_PROCESSOR_AGGREGATOR tristate "Processor Aggregator"
drivers/acpi/acpi_processor.c +0 -18
@@ -184,24 +184,6 @@ static void __init acpi_pcc_cpufreq_init(void) {}/* Initialization */ #ifdef CONFIG_ACPI_HOTPLUG_CPU -int __weak acpi_map_cpu (acpi_handle handle, -phys_cpuid_t physid,u32 acpi_id,int *pcpu) -{ -return -ENODEV ; -} - -int __weak acpi_unmap_cpu (int cpu) -{ -return -ENODEV ; -} - -int __weak arch_register_cpu (int cpu) -{ -return -ENODEV ; -} - -void __weak arch_unregister_cpu (int cpu) {} -static int acpi_processor_hotadd_init (struct acpi_processor *pr) {unsigned long long sta;
Signature · 1 line
2.30.2
PATCH 6/39 drivers: base: Use present CPUs in GENERIC_CPU_DEVICES
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Three of the five ACPI architectures create sysfs entries using register_cpu() for present CPUs, whereas arm64, riscv and all GENERIC_CPU_DEVICES do this for possible CPUs.
Registering a CPU is what causes them to show up in sysfs.
It makes very little sense to register all possible CPUs. Registering a CPU is what triggers the udev notifications allowing user-space to react to newly added CPUs.
To allow all five ACPI architectures to use GENERIC_CPU_DEVICES, change it to use for_each_present_cpu(). Making the ACPI architectures use GENERIC_CPU_DEVICES is a pre-requisite step to centralise their cpu_register() logic, before moving it into the ACPI processor driver. When ACPI is disabled this work would be done by cpu_dev_register_generic().
Of the ACPI architectures that register possible CPUs, arm64 and riscv do not support making possible CPUs present as they use the weak 'always fails' version of arch_register_cpu().
Only two of the eight architectures that use GENERIC_CPU_DEVICES have a distinction between present and possible CPUs.
The following architectures use GENERIC_CPU_DEVICES but are not SMP, so possible == present:
* m68k
* microblaze
* nios2
The following architectures use GENERIC_CPU_DEVICES and consider possible == present:
* csky: setup_smp()
* processor_probe() sets possible for all CPUs and present for all CPUs except the boot cpu, which will have been done by init/main.c::start_kernel().
um appears to be a subarchitecture of x86.
The remaining architecture using GENERIC_CPU_DEVICES are:
* openrisc and hexagon:
where smp_init_cpus() makes all CPUs < NR_CPUS possible, whereas smp_prepare_cpus() only makes CPUs < setup_max_cpus present.
After this change, openrisc and hexagon systems that use the max_cpus command line argument would not see the other CPUs present in sysfs. This should not be a problem as these CPUs can't bre brought online as _cpu_up() checks cpu_present().
After this change, only CPUs which are present appear in sysfs.
--- drivers/base/cpu.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/cpu.c +1 -1
@@ -533,7 +533,7 @@ static void __init cpu_dev_register_generic(void)#ifdef CONFIG_GENERIC_CPU_DEVICES int i; -for_each_possible_cpu (i) { +for_each_present_cpu (i) {if (register_cpu (&per_cpu (cpu_devices, i), i))panic ("Failed to register CPU device" ); }
Signature · 1 line
2.30.2
PATCH 7/39 drivers: base: Allow parts of GENERIC_CPU_DEVICES to be overridden
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Architectures often have extra per-cpu work that needs doing before a CPU is registered, often to determine if a CPU is hotpluggable.
To allow the ACPI architectures to use GENERIC_CPU_DEVICES, move the cpu_register() call into arch_register_cpu(), which is made __weak so architectures with extra work can override it.
This aligns with the way x86, ia64 and loongarch register hotplug CPUs when they become present.
---
Changes since RFC:
* Dropped __init from x86/ia64 arch_register_cpu()
Changes since RFC v2:
* Dropped unnecessary Loongarch asm/cpu.h changes
--- drivers/base/cpu.c | 14 ++++++++++---- include/linux/cpu.h | 4 ++++ 2 files changed, 14 insertions(+), 4 deletions(-)
drivers/base/cpu.c +10 -4
@@ -525,19 +525,25 @@ bool cpu_is_hotpluggable(unsigned int cpu)EXPORT_SYMBOL_GPL (cpu_is_hotpluggable);#ifdef CONFIG_GENERIC_CPU_DEVICES -static DEFINE_PER_CPU(struct cpu, cpu_devices); +DEFINE_PER_CPU(struct cpu, cpu_devices); + +int __weak arch_register_cpu (int cpu) +{ +return register_cpu (&per_cpu (cpu_devices, cpu), cpu); +}#endif static void __init cpu_dev_register_generic (void ) { -#ifdef CONFIG_GENERIC_CPU_DEVICES int i; +if (!IS_ENABLED (CONFIG_GENERIC_CPU_DEVICES )) +return ; +for_each_present_cpu (i) { -if (register_cpu (&per_cpu (cpu_devices, i), i)) +if (arch_register_cpu (i))panic ("Failed to register CPU device" ); } -#endif }#ifdef CONFIG_GENERIC_CPU_VULNERABILITIES
include/linux/cpu.h +4 -0
@@ -88,6 +88,10 @@ extern ssize_t arch_cpu_probe(const char *, size_t);extern ssize_t arch_cpu_release (const char *,size_t );#endif +#ifdef CONFIG_GENERIC_CPU_DEVICES +DECLARE_PER_CPU(struct cpu, cpu_devices); +#endif +/* * These states are not related to the core CPU hotplug mechanism. They are * used by various (sub)architectures to track internal state
Signature · 1 line
2.30.2
PATCH 8/39 drivers: base: Move cpu_dev_init() after node_dev_init()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
NUMA systems require the node descriptions to be ready before CPUs are registered. This is so that the node symlinks can be created in sysfs.
Currently no NUMA platform uses GENERIC_CPU_DEVICES, meaning that CPUs are registered by arch code, instead of cpu_dev_init().
Move cpu_dev_init() after node_dev_init() so that NUMA architectures can use GENERIC_CPU_DEVICES.
---
Note: Jonathan's comment still needs addressing - see
https://lore.kernel.org/r/[email protected]
--- drivers/base/init.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/base/init.c +1 -1
@@ -35,8 +35,8 @@ void __init driver_init(void)of_core_init ();platform_bus_init ();auxiliary_bus_init (); -cpu_dev_init ();memory_dev_init ();node_dev_init (); +cpu_dev_init ();container_dev_init (); }
Signature · 1 line
2.30.2
PATCH 9/39 drivers: base: remove unnecessary call to register_cpu_under_node()
In reply to Russell King (Oracle)Since "drivers: base: Move cpu_dev_init() after node_dev_init()", we can remove some redundant code.
node_dev_init() will walk through the nodes calling register_one_node() on each. This will trickle down to __register_one_node() which walks all present CPUs, calling register_cpu_under_node() on each.
register_cpu_under_node() will call get_cpu_device(cpu) for each, which will return NULL until the CPU is registered using register_cpu(). This now happens _after_ node_dev_init().
Therefore, calling register_cpu_under_node() from __register_one_node() becomes a no-op, and can be removed.
--- drivers/base/node.c | 7 ------- 1 file changed, 7 deletions(-)
drivers/base/node.c +0 -7
@@ -867,7 +867,6 @@ void register_memory_blocks_under_node(int nid, unsigned long start_pfn,int __register_one_node (int nid) {int error; -int cpu; node_devices[nid] =kzalloc (sizeof (struct node ),GFP_KERNEL );if (!node_devices[nid])
@@ -875,12 +874,6 @@ int __register_one_node(int nid) error =register_node (node_devices[nid], nid); -/* link cpu under this node */ -for_each_present_cpu (cpu) { -if (cpu_to_node (cpu) == nid) -register_cpu_under_node (cpu, nid); - } -INIT_LIST_HEAD (&node_devices[nid]->access_list);node_init_caches (nid);
Signature · 1 line
2.30.2
PATCH 10/39 drivers: base: Print a warning instead of panic() when register_cpu() fails
In reply to Russell King (Oracle)From: James Morse <[email protected]>
loongarch, mips, parisc, riscv and sh all print a warning if register_cpu() returns an error. Architectures that use GENERIC_CPU_DEVICES call panic() instead.
Errors in this path indicate something is wrong with the firmware description of the platform, but the kernel is able to keep running.
Downgrade this to a warning to make it easier to debug this issue.
This will allow architectures that switching over to GENERIC_CPU_DEVICES to drop their warning, but keep the existing behaviour.
--- drivers/base/cpu.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
drivers/base/cpu.c +4 -3
@@ -535,14 +535,15 @@ int __weak arch_register_cpu(int cpu)static void __init cpu_dev_register_generic (void ) { -int i; +int i, ret;if (!IS_ENABLED (CONFIG_GENERIC_CPU_DEVICES ))return ;for_each_present_cpu (i) { -if (arch_register_cpu (i)) -panic ("Failed to register CPU device" ); + ret =arch_register_cpu (i); +if (ret) +pr_warn ("register_cpu %d failed (%d)\n" , i, ret); } }
Signature · 1 line
2.30.2
PATCH 11/39 arm64: setup: Switch over to GENERIC_CPU_DEVICES using arch_register_cpu()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
To allow ACPI's _STA value to hide CPUs that are present, but not available to online right now due to VMM or firmware policy, the register_cpu() call needs to be made by the ACPI machinery when ACPI is in use. This allows it to hide CPUs that are unavailable from sysfs.
Switching to GENERIC_CPU_DEVICES is an intermediate step to allow all five ACPI architectures to be modified at once.
Switch over to GENERIC_CPU_DEVICES, and provide an arch_register_cpu() that populates the hotpluggable flag. arch_register_cpu() is also the interface the ACPI machinery expects.
The struct cpu in struct cpuinfo_arm64 is never used directly, remove it to use the one GENERIC_CPU_DEVICES provides.
This changes the CPUs visible in sysfs from possible to present, but on arm64 smp_prepare_cpus() ensures these are the same.
This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.
---
Changes since RFC v2:
* Add note about initialisation order change.
--- arch/arm64/Kconfig | 1 + arch/arm64/include/asm/cpu.h | 1 - arch/arm64/kernel/setup.c | 13 ++++--------- 3 files changed, 5 insertions(+), 10 deletions(-)
arch/arm64/Kconfig +1 -0
@@ -132,6 +132,7 @@ config ARM64 select GENERIC_ARCH_TOPOLOGY select GENERIC_CLOCKEVENTS_BROADCAST select GENERIC_CPU_AUTOPROBE + select GENERIC_CPU_DEVICES select GENERIC_CPU_VULNERABILITIES select GENERIC_EARLY_IOREMAP select GENERIC_IDLE_POLL_SETUP
arch/arm64/include/asm/cpu.h +0 -1
@@ -38,7 +38,6 @@ struct cpuinfo_32bit { };struct cpuinfo_arm64 { -struct cpu cpu;struct kobject kobj;u64 reg_ctr;u64 reg_cntfrq;
arch/arm64/kernel/setup.c +4 -9
@@ -402,19 +402,14 @@ static inline bool cpu_can_disable(unsigned int cpu)return false; } -static int __init topology_init (void ) +int arch_register_cpu (int num) { -int i; +struct cpu *cpu = &per_cpu (cpu_devices, num); -for_each_possible_cpu (i) { -struct cpu *cpu = &per_cpu (cpu_data.cpu, i); - cpu->hotpluggable =cpu_can_disable (i); -register_cpu (cpu, i); - } + cpu->hotpluggable =cpu_can_disable (num); -return 0 ; +return register_cpu (cpu, num); } -subsys_initcall (topology_init);static void dump_kernel_offset (void ) {
Signature · 1 line
2.30.2
PATCH 12/39 ia64/topology: Switch over to GENERIC_CPU_DEVICES
In reply to Russell King (Oracle)From: James Morse <[email protected]>
ia64 has its own arch specific data structure for cpus: struct ia64_cpu. This has one member, making ia64's cpu_devices the same as that provided be GENERIC_CPU_DEVICES.
ia64 craetes a percpu struct ia64_cpu called cpu_devices, which has no users. Instead it uses the struct ia64_cpu named sysfs_cpus allocated at boot.
Remove the arch specific structure allocation and initialisation. ia64's arch_register_cpu() now overrides the weak version from GENERIC_CPU_DEVICES, and uses the percpu cpu_devices defined by core code.
All uses of sysfs_cpus are changed to use the percpu cpu_devices.
This is an intermediate step to the logic being moved to drivers/acpi, where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.
This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.
---
Changes since RFC v2:
* Add note about initialisation order change.
--- arch/ia64/Kconfig | 1 + arch/ia64/include/asm/cpu.h | 6 ------ arch/ia64/kernel/topology.c | 35 +++++------------------------------ 3 files changed, 6 insertions(+), 36 deletions(-)
arch/ia64/Kconfig +1 -0
@@ -42,6 +42,7 @@ config IA64 select HAVE_FUNCTION_DESCRIPTORS select HAVE_VIRT_CPU_ACCOUNTING select HUGETLB_PAGE_SIZE_VARIABLE if HUGETLB_PAGE + select GENERIC_CPU_DEVICES select GENERIC_IRQ_PROBE select GENERIC_PENDING_IRQ if SMP select GENERIC_IRQ_SHOW
arch/ia64/include/asm/cpu.h +0 -6
@@ -7,12 +7,6 @@#include <linux/topology.h> #include <linux/percpu.h> -struct ia64_cpu { -struct cpu cpu; -}; - -DECLARE_PER_CPU(struct ia64_cpu, cpu_devices); -DECLARE_PER_CPU (int, cpu_state);#endif /* _ASM_IA64_CPU_H_ */
arch/ia64/kernel/topology.c +5 -30
@@ -26,8 +26,6 @@#include <asm/numa.h> #include <asm/cpu.h> -static struct ia64_cpu *sysfs_cpus; -void arch_fix_phys_package_id (int num,u32 slot) {#ifdef CONFIG_SMP
@@ -41,50 +39,27 @@ EXPORT_SYMBOL_GPL(arch_fix_phys_package_id);#ifdef CONFIG_HOTPLUG_CPU int __ref arch_register_cpu (int num) { +struct cpu *cpu = &per_cpu (cpu_devices, num); +/* * If CPEI can be re-targeted or if this is not * CPEI target, then it is hotpluggable */ if (can_cpei_retarget () || !is_cpu_cpei_target (num)) - sysfs_cpus[num].cpu.hotpluggable =1 ; + cpu->hotpluggable =1 ;map_cpu_to_node (num, node_cpuid[num].nid); -return register_cpu (&sysfs_cpus[num].cpu, num); +return register_cpu (cpu, num); }EXPORT_SYMBOL (arch_register_cpu);void __ref arch_unregister_cpu (int num) { -unregister_cpu (&sysfs_cpus[num].cpu); +unregister_cpu (&per_cpu (cpu_devices, num));unmap_cpu_from_node (num,cpu_to_node (num)); }EXPORT_SYMBOL (arch_unregister_cpu); -#else -int __init arch_register_cpu (int num) -{ -return register_cpu (&sysfs_cpus[num].cpu, num); -}#endif /*CONFIG_HOTPLUG_CPU*/ - -static int __init topology_init (void ) -{ -int i, err =0 ; - - sysfs_cpus =kcalloc (NR_CPUS ,sizeof (struct ia64_cpu ),GFP_KERNEL ); -if (!sysfs_cpus) -panic ("kzalloc in topology_init failed - NR_CPUS too big?" ); - -for_each_present_cpu (i) { -if ((err =arch_register_cpu (i))) - goto out; - } -out: -return err; -} - -subsys_initcall (topology_init); - -/* * Export cpu cache information through sysfs */
Signature · 1 line
2.30.2
PATCH 13/39 x86: intel_epb: Don't rely on link order
In reply to Russell King (Oracle)From: James Morse <[email protected]>
intel_epb_init() is called as a subsys_initcall() to register cpuhp callbacks. The callbacks make use of get_cpu_device() which will return NULL unless register_cpu() has been called. register_cpu() is called from topology_init(), which is also a subsys_initcall().
This is fragile. Moving the register_cpu() to a different subsys_initcall() leads to a NULL dereference during boot.
Make intel_epb_init() a late_initcall(), user-space can't provide a policy before this point anyway.
---subsys_initcall_sync() would be an option, but moving the register_cpu() calls into ACPI also means adding a safety net for CPUs that are online but not described properly by firmware. This lives in subsys_initcall_sync().
--- arch/x86/kernel/cpu/intel_epb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
arch/x86/kernel/cpu/intel_epb.c +1 -1
@@ -237,4 +237,4 @@ static __init int intel_epb_init(void)cpuhp_remove_state (CPUHP_AP_X86_INTEL_EPB_ONLINE );return ret; } -subsys_initcall (intel_epb_init); +late_initcall (intel_epb_init);
Signature · 1 line
2.30.2
PATCH 14/39 x86/topology: Switch over to GENERIC_CPU_DEVICES
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Now that GENERIC_CPU_DEVICES calls arch_register_cpu(), which can be overridden by the arch code, switch over to this to allow common code to choose when the register_cpu() call is made.
x86's struct cpus come from struct x86_cpu, which has no other members or users. Remove this and use the version defined by common code.
This is an intermediate step to the logic being moved to drivers/acpi, where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.
This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.
----
Changes since RFC:
* Fixed the second copy of arch_register_cpu() used for non-hotplug
Changes since RFC v2:
* Remove duplicate of the weak generic arch_register_cpu(), spotted by Jonathan Cameron. Add note about initialisation order change.
--- arch/x86/Kconfig | 1 + arch/x86/include/asm/cpu.h | 4 ---- arch/x86/kernel/topology.c | 27 ++++----------------------- 3 files changed, 5 insertions(+), 27 deletions(-)
arch/x86/Kconfig +1 -0
@@ -148,6 +148,7 @@ config X86 select GENERIC_CLOCKEVENTS_MIN_ADJUST select GENERIC_CMOS_UPDATE select GENERIC_CPU_AUTOPROBE + select GENERIC_CPU_DEVICES select GENERIC_CPU_VULNERABILITIES select GENERIC_EARLY_IOREMAP select GENERIC_ENTRY
arch/x86/include/asm/cpu.h +0 -4
@@ -23,10 +23,6 @@ static inline void prefill_possible_map(void) {}#endif /* CONFIG_SMP */ -struct x86_cpu { -struct cpu cpu; -}; -#ifdef CONFIG_HOTPLUG_CPU extern void soft_restart_cpu (void );#endif
arch/x86/kernel/topology.c +4 -23
@@ -35,38 +35,19 @@#include <asm/io_apic.h> #include <asm/cpu.h> -static DEFINE_PER_CPU(struct x86_cpu, cpu_devices); -#ifdef CONFIG_HOTPLUG_CPU int arch_register_cpu (int cpu) { -struct x86_cpu *xc =per_cpu_ptr (&cpu_devices, cpu); +struct cpu *c =per_cpu_ptr (&cpu_devices, cpu); - xc->cpu.hotpluggable = cpu >0 ; -return register_cpu (&xc->cpu, cpu); + c->hotpluggable = cpu >0 ; +return register_cpu (c, cpu); }EXPORT_SYMBOL (arch_register_cpu);void arch_unregister_cpu (int num) { -unregister_cpu (&per_cpu (cpu_devices, num).cpu); +unregister_cpu (&per_cpu (cpu_devices, num)); }EXPORT_SYMBOL (arch_unregister_cpu); -#else /* CONFIG_HOTPLUG_CPU */ - -int __init arch_register_cpu (int num) -{ -return register_cpu (&per_cpu (cpu_devices, num).cpu, num); -}#endif /* CONFIG_HOTPLUG_CPU */ - -static int __init topology_init (void ) -{ -int i; - -for_each_present_cpu (i) -arch_register_cpu (i); - -return 0 ; -} -subsys_initcall (topology_init);
Signature · 1 line
2.30.2
PATCH 15/39 LoongArch: Switch over to GENERIC_CPU_DEVICES
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Now that GENERIC_CPU_DEVICES calls arch_register_cpu(), which can be overridden by the arch code, switch over to this to allow common code to choose when the register_cpu() call is made.
This allows topology_init() to be removed.
This is an intermediate step to the logic being moved to drivers/acpi, where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.
This is a subtle change. Originally:
- on boot, topology_init() would have marked present CPUs that io_master() is true for as hotplug-incapable.
- if a CPU is hotplugged that is an io_master(), it can later be hot-unplugged.
The new behaviour is that any CPU that io_master() is true for will now always be marked as hotplug-incapable, thus even if it was hotplugged, it can no longer be hot-unplugged.
This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.
---
Changes since RFC v2:
* Explain the change in behaviour in the patch description (highlighted by Jonathan Cameron - thanks.) Add note about initialisation order change.
--- arch/loongarch/Kconfig | 1 + arch/loongarch/kernel/topology.c | 29 ++--------------------------- 2 files changed, 3 insertions(+), 27 deletions(-)
arch/loongarch/Kconfig +1 -0
@@ -72,6 +72,7 @@ config LOONGARCH select GENERIC_CLOCKEVENTS select GENERIC_CMOS_UPDATE select GENERIC_CPU_AUTOPROBE + select GENERIC_CPU_DEVICES select GENERIC_ENTRY select GENERIC_GETTIMEOFDAY select GENERIC_IOREMAP if !ARCH_IOREMAP
arch/loongarch/kernel/topology.c +2 -27
@@ -10,20 +10,13 @@#include <acpi/processor.h> -static DEFINE_PER_CPU(struct cpu, cpu_devices); -#ifdef CONFIG_HOTPLUG_CPU int arch_register_cpu (int cpu) { -int ret;struct cpu *c = &per_cpu (cpu_devices, cpu); - c->hotpluggable =1 ; - ret =register_cpu (c, cpu); -if (ret <0 ) -pr_warn ("register_cpu %d failed (%d)\n" , cpu, ret); - -return ret; + c->hotpluggable = !io_master (cpu); +return register_cpu (c, cpu); }EXPORT_SYMBOL (arch_register_cpu);
@@ -36,21 +29,3 @@ void arch_unregister_cpu(int cpu) }EXPORT_SYMBOL (arch_unregister_cpu);#endif - -static int __init topology_init (void ) -{ -int i, ret; - -for_each_present_cpu (i) { -struct cpu *c = &per_cpu (cpu_devices, i); - - c->hotpluggable = !io_master (i); - ret =register_cpu (c, i); -if (ret <0 ) -pr_warn ("topology_init: register_cpu %d failed (%d)\n" , i, ret); - } - -return 0 ; -} - -subsys_initcall (topology_init);
Signature · 1 line
2.30.2
PATCH 16/39 riscv: Switch over to GENERIC_CPU_DEVICES
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Now that GENERIC_CPU_DEVICES calls arch_register_cpu(), which can be overridden by the arch code, switch over to this to allow common code to choose when the register_cpu() call is made.
This allows topology_init() to be removed.
This is an intermediate step to the logic being moved to drivers/acpi, where GENERIC_CPU_DEVICES will do the work when booting with acpi=off.
This patch also has the effect of moving the registration of CPUs from subsys to driver core initialisation, prior to any initcalls running.
---
Changes since RFC v2:
* Add note about initialisation order change.
--- arch/riscv/Kconfig | 1 + arch/riscv/kernel/setup.c | 19 ++++--------------- 2 files changed, 5 insertions(+), 15 deletions(-)
arch/riscv/Kconfig +1 -0
@@ -69,6 +69,7 @@ config RISCV select GENERIC_ARCH_TOPOLOGY select GENERIC_ATOMIC64 if !64BIT select GENERIC_CLOCKEVENTS_BROADCAST if SMP + select GENERIC_CPU_DEVICES select GENERIC_EARLY_IOREMAP select GENERIC_ENTRY select GENERIC_GETTIMEOFDAY if HAVE_GENERIC_VDSO
arch/riscv/kernel/setup.c +4 -15
@@ -62,7 +62,6 @@ atomic_t hart_lottery __section(".sdata")#endif ;unsigned long boot_cpu_hartid; -static DEFINE_PER_CPU(struct cpu, cpu_devices);/* * Place kernel memory regions on the resource tree so that
@@ -307,23 +306,13 @@ void __init setup_arch(char **cmdline_p)riscv_set_dma_cache_alignment (); } -static int __init topology_init (void ) +int arch_register_cpu (int cpu) { -int i, ret; +struct cpu *c = &per_cpu (cpu_devices, cpu); -for_each_possible_cpu (i) { -struct cpu *cpu = &per_cpu (cpu_devices, i); - - cpu->hotpluggable =cpu_has_hotplug (i); - ret =register_cpu (cpu, i); -if (unlikely (ret)) -pr_warn ("Warning: %s: register_cpu %d failed (%d)\n" , - __func__, i, ret); - } - -return 0 ; + c->hotpluggable =cpu_has_hotplug (cpu); +return register_cpu (c, cpu); } -subsys_initcall (topology_init);void free_initmem (void ) {
Signature · 1 line
2.30.2
PATCH 17/39 arch_topology: Make register_cpu_capacity_sysctl() tolerant to late CPUs
In reply to Russell King (Oracle)From: James Morse <[email protected]>
register_cpu_capacity_sysctl() adds a property to sysfs that describes the CPUs capacity. This is done from a subsys_initcall() that assumes all possible CPUs are registered.
With CPU hotplug, possible CPUs aren't registered until they become present, (or for arm64 enabled). This leads to messages during boot:
| register_cpu_capacity_sysctl: too early to get CPU1 device! and once these CPUs are added to the system, the file is missing.
Move this to a cpuhp callback, so that the file is created once CPUs are brought online. This covers CPUs that are added late by mechanisms like hotplug.
One observable difference is the file is now missing for offline CPUs.
---
If the offline CPUs thing is a problem for the tools that consume this value, we'd need to move cpu_capacity to be part of cpu.c's common_cpu_attr_groups.
--- drivers/base/arch_topology.c | 38 ++++++++++++++++++++++++------------ 1 file changed, 26 insertions(+), 12 deletions(-)
drivers/base/arch_topology.c +26 -12
@@ -220,20 +220,34 @@ static DECLARE_WORK(update_topology_flags_work, update_topology_flags_workfn);static DEVICE_ATTR_RO (cpu_capacity); -static int register_cpu_capacity_sysctl (void ) +static int cpu_capacity_sysctl_add (unsigned int cpu) { -int i; -struct device *cpu; +struct device *cpu_dev =get_cpu_device (cpu); -for_each_possible_cpu (i) { - cpu =get_cpu_device (i); -if (!cpu) { -pr_err ("%s: too early to get CPU%d device!\n" , - __func__, i); -continue ; - } -device_create_file (cpu, &dev_attr_cpu_capacity); - } +if (!cpu_dev) +return -ENOENT ; + +device_create_file (cpu_dev, &dev_attr_cpu_capacity); + +return 0 ; +} + +static int cpu_capacity_sysctl_remove (unsigned int cpu) +{ +struct device *cpu_dev =get_cpu_device (cpu); + +if (!cpu_dev) +return -ENOENT ; + +device_remove_file (cpu_dev, &dev_attr_cpu_capacity); + +return 0 ; +} + +static int register_cpu_capacity_sysctl (void ) +{ +cpuhp_setup_state (CPUHP_AP_ONLINE_DYN ,"topology/cpu-capacity" , + cpu_capacity_sysctl_add, cpu_capacity_sysctl_remove);return 0 ; }
Signature · 1 line
2.30.2
PATCH 18/39 ACPI: Only enumerate enabled (or functional) devices
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Today the ACPI enumeration code 'visits' all devices that are present.
This is a problem for arm64, where CPUs are always present, but not always enabled. When a device-check occurs because the firmware-policy has changed and a CPU is now enabled, the following error occurs:
| acpi ACPI0007:48: Enumeration failure
This is ultimately because acpi_dev_ready_for_enumeration() returns true for a device that is not enabled. The ACPI Processor driver will not register such CPUs as they are not 'decoding their resources'.
Change acpi_dev_ready_for_enumeration() to also check the enabled bit. ACPI allows a device to be functional instead of maintaining the present and enabled bit. Make this behaviour an explicit check with a reference to the spec, and then check the present and enabled bits. This is needed to avoid enumerating present && functional devices that are not enabled.
---
If this change causes problems on deployed hardware, I suggest an arch opt-in: ACPI_IGNORE_STA_ENABLED, that causes acpi_dev_ready_for_enumeration() to only check the present bit.
Changes since RFC v2:
* Incorporate comment suggestion by Gavin Shan.
Other review comments from Jonathan Cameron not yet addressed.
--- drivers/acpi/device_pm.c | 2 +- drivers/acpi/device_sysfs.c | 2 +- drivers/acpi/internal.h | 1 - drivers/acpi/property.c | 2 +- drivers/acpi/scan.c | 24 ++++++++++++++---------- 5 files changed, 17 insertions(+), 14 deletions(-)
drivers/acpi/device_pm.c +1 -1
@@ -313,7 +313,7 @@ int acpi_bus_init_power(struct acpi_device *device)return -EINVAL ; device->power.state =ACPI_STATE_UNKNOWN ; -if (!acpi_device_is_present (device)) { +if (!acpi_dev_ready_for_enumeration (device)) { device->flags.initialized = false;return -ENXIO ; }
drivers/acpi/device_sysfs.c +1 -1
@@ -141,7 +141,7 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modaliastruct acpi_hardware_id *id;/* Avoid unnecessarily loading modules for non present devices. */ -if (!acpi_device_is_present (acpi_dev)) +if (!acpi_dev_ready_for_enumeration (acpi_dev))return 0 ;/*
drivers/acpi/internal.h +0 -1
@@ -107,7 +107,6 @@ int acpi_device_setup_files(struct acpi_device *dev);void acpi_device_remove_files (struct acpi_device *dev);void acpi_device_add_finalize (struct acpi_device *device);void acpi_free_pnp_ids (struct acpi_device_pnp *pnp); -bool acpi_device_is_present (const struct acpi_device *adev);bool acpi_device_is_battery (struct acpi_device *adev);bool acpi_device_is_first_physical_node (struct acpi_device *adev,const struct device *dev);
drivers/acpi/property.c +1 -1
@@ -1418,7 +1418,7 @@ static bool acpi_fwnode_device_is_available(const struct fwnode_handle *fwnode)if (!is_acpi_device_node (fwnode))return false; -return acpi_device_is_present (to_acpi_device_node (fwnode)); +return acpi_dev_ready_for_enumeration (to_acpi_device_node (fwnode)); } static const void *
drivers/acpi/scan.c +14 -10
@@ -304,7 +304,7 @@ static int acpi_scan_device_check(struct acpi_device *adev)int error;acpi_bus_get_status (adev); -if (acpi_device_is_present (adev)) { +if (acpi_dev_ready_for_enumeration (adev)) {/* * This function is only called for device objects for which * matching scan handlers exist. The only situation in which
@@ -338,7 +338,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used)int error;acpi_bus_get_status (adev); -if (!acpi_device_is_present (adev)) { +if (!acpi_dev_ready_for_enumeration (adev)) {acpi_scan_device_not_enumerated (adev);return 0 ; }
@@ -1908,11 +1908,6 @@ static bool acpi_device_should_be_hidden(acpi_handle handle)return true; } -bool acpi_device_is_present (const struct acpi_device *adev) -{ -return adev->status.present || adev->status.functional; -} -static bool acpi_scan_handler_matching (struct acpi_scan_handler *handler,const char *idstr,const struct acpi_device_id **matchid)
@@ -2375,16 +2370,25 @@ EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies);* acpi_dev_ready_for_enumeration - Check if the ACPI device is ready for enumeration * @device: Pointer to the &struct acpi_device to check * -* Check if the device is present and has no unmet dependencies. +* Check if the device is functional or enabled and has no unmet dependencies. * -* Return true if the device is ready for enumeratino. Otherwise, return false. +* Return true if the device is ready for enumeration. Otherwise, return false. */ bool acpi_dev_ready_for_enumeration (const struct acpi_device *device) {if (device->flags.honor_deps && device->dep_unmet)return false; -return acpi_device_is_present (device); +/* +* ACPI 6.5's 6.3.7 "_STA (Device Status)" allows firmware to return +* (!present && functional) for certain types of devices that should be +* enumerated. Note that the enabled bit can't be sert until the present +* bit is set. +*/ +if (device->status.present) +return device->status.enabled; +else +return device->status.functional; }EXPORT_SYMBOL_GPL (acpi_dev_ready_for_enumeration);
Signature · 1 line
2.30.2
Rafael J. Wysocki
maintainerRe: [PATCH 18/39] ACPI: Only enumerate enabled (or functional) devices
Quoted 21 lines · Russell King
From: James Morse <[email protected]>
Today the ACPI enumeration code 'visits' all devices that are present.
This is a problem for arm64, where CPUs are always present, but not always enabled. When a device-check occurs because the firmware-policy has changed and a CPU is now enabled, the following error occurs:
| acpi ACPI0007:48: Enumeration failureThis is ultimately because
acpi_dev_ready_for_enumeration()returns true for a device that is not enabled. The ACPI Processor driver will not register such CPUs as they are not 'decoding their resources'.Change
acpi_dev_ready_for_enumeration()to also check the enabled bit. ACPI allows a device to be functional instead of maintaining the present and enabled bit. Make this behaviour an explicit check with a reference to the spec, and then check the present and enabled bits. This is needed to avoid enumerating present && functional devices that are not enabled.
Quoted 4 lines
Signed-off-by James Morse <[email protected]>Signed-off-by Russell King (Oracle) <[email protected]>---
If this change causes problems on deployed hardware, I suggest an
TBH, I am expecting problems to be there.
If something has been interpreted in a specific way for several years, then changing that interpretation is just incompatible with the entire installed base, at least potentially.
It is not even possible to estimate the potential adverse impact of this change, as it causes a firmware-provided bit that has never been taken into account so far to become meaningful and it does so for every device in the system.
It will be very hard to convince me that this change is a good idea.
Quoted 2 lines
arch opt-in:
ACPI_IGNORE_STA_ENABLED, that causesacpi_dev_ready_for_enumeration()to only check the present bit.
But this can work as long as the given arch does not care about platforms in which the "enabled" bit may not be set as expected for some devices.
Quoted 129 lines
Changes since RFC v2:
* Incorporate comment suggestion by Gavin Shan.
Other review comments from Jonathan Cameron not yet addressed.--- drivers/acpi/device_pm.c | 2 +- drivers/acpi/device_sysfs.c | 2 +- drivers/acpi/internal.h | 1 - drivers/acpi/property.c | 2 +- drivers/acpi/scan.c | 24 ++++++++++++++---------- 5 files changed, 17 insertions(+), 14 deletions(-)drivers/acpi/device_pm.c +1
-1@@ -313,7 +313,7 @@ int acpi_bus_init_power(struct acpi_device *device)return -EINVAL ; device->power.state =ACPI_STATE_UNKNOWN ; -if (!acpi_device_is_present (device)) { +if (!acpi_dev_ready_for_enumeration (device)) { device->flags.initialized = false;return -ENXIO ; }drivers/acpi/device_sysfs.c +1
-1@@ -141,7 +141,7 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modaliastruct acpi_hardware_id *id;/* Avoid unnecessarily loading modules for non present devices. */ -if (!acpi_device_is_present (acpi_dev)) +if (!acpi_dev_ready_for_enumeration (acpi_dev))return 0 ;/* drivers/acpi/internal.h +0
-1@@ -107,7 +107,6 @@ int acpi_device_setup_files(struct acpi_device *dev);void acpi_device_remove_files (struct acpi_device *dev);void acpi_device_add_finalize (struct acpi_device *device);void acpi_free_pnp_ids (struct acpi_device_pnp *pnp); -bool acpi_device_is_present (const struct acpi_device *adev);bool acpi_device_is_battery (struct acpi_device *adev);bool acpi_device_is_first_physical_node (struct acpi_device *adev,const struct device *dev);drivers/acpi/property.c +1
-1@@ -1418,7 +1418,7 @@ static bool acpi_fwnode_device_is_available(const struct fwnode_handle *fwnode)if (!is_acpi_device_node (fwnode))return false; -return acpi_device_is_present (to_acpi_device_node (fwnode)); +return acpi_dev_ready_for_enumeration (to_acpi_device_node (fwnode)); } static const void *drivers/acpi/scan.c +14
-11@@ -304,7 +304,7 @@ static int acpi_scan_device_check(struct acpi_device *adev)int error;acpi_bus_get_status (adev); -if (acpi_device_is_present (adev)) { +if (acpi_dev_ready_for_enumeration (adev)) {/* * This function is only called for device objects for which * matching scan handlers exist. The only situation in which @@ -338,7 +338,7 @@ static int acpi_scan_bus_check(struct acpi_device *adev, void *not_used)int error;acpi_bus_get_status (adev); -if (!acpi_device_is_present (adev)) { +if (!acpi_dev_ready_for_enumeration (adev)) {acpi_scan_device_not_enumerated (adev);return 0 ; }@@ -1908,11 +1908,6 @@ static bool acpi_device_should_be_hidden(acpi_handle handle)return true; } -bool acpi_device_is_present (const struct acpi_device *adev) -{ -return adev->status.present || adev->status.functional; -} -static bool acpi_scan_handler_matching (struct acpi_scan_handler *handler,const char *idstr,const struct acpi_device_id **matchid)@@ -2375,16 +2370,25 @@ EXPORT_SYMBOL_GPL(acpi_dev_clear_dependencies);* acpi_dev_ready_for_enumeration - Check if the ACPI device is ready for enumeration * @device: Pointer to the &struct acpi_device to check * -* Check if the device is present and has no unmet dependencies. +* Check if the device is functional or enabled and has no unmet dependencies. * -* Return true if the device is ready for enumeratino. Otherwise, return false. +* Return true if the device is ready for enumeration. Otherwise, return false. */ bool acpi_dev_ready_for_enumeration (const struct acpi_device *device) {if (device->flags.honor_deps && device->dep_unmet)return false; -return acpi_device_is_present (device); +/* +* ACPI 6.5's 6.3.7 "_STA (Device Status)" allows firmware to return +* (!present && functional) for certain types of devices that should be +* enumerated. Note that the enabled bit can't be sert until the present +* bit is set. +*/ +if (device->status.present) +return device->status.enabled; +else +return device->status.functional; }EXPORT_SYMBOL_GPL (acpi_dev_ready_for_enumeration); --2.30.2
PATCH 19/39 ACPI: processor: Add support for processors described as container packages
In reply to Russell King (Oracle)From: James Morse <[email protected]>
ACPI has two ways of describing processors in the DSDT. Either as a device object with HID ACPI0007, or as a type 'C' package inside a Processor Container. The ACPI processor driver probes CPUs described as devices, but not those described as packages.
Duplicate descriptions are not allowed, the ACPI processor driver already parses the UID from both devices and containers. acpi_processor_get_info() returns an error if the UID exists twice in the DSDT.
The missing probe for CPUs described as packages creates a problem for moving the cpu_register() calls into the acpi_processor driver, as CPUs described like this don't get registered, leading to errors from other subsystems when they try to add new sysfs entries to the CPU node. (e.g. topology_sysfs_init()'s use of topology_add_dev() via cpuhp)
To fix this, parse the processor container and call acpi_processor_add() for each processor that is discovered like this. The processor container handler is added with acpi_scan_add_handler(), so no detach call will arrive.
Qemu TCG describes CPUs using packages in a processor container.
--- drivers/acpi/acpi_processor.c | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+)
drivers/acpi/acpi_processor.c +22 -0
@@ -626,9 +626,31 @@ static struct acpi_scan_handler processor_handler = { }, }; +static acpi_status acpi_processor_container_walk (acpi_handle handle, +u32 lvl, +void *context, +void **rv) +{ +struct acpi_device *adev; +acpi_status status; + + adev =acpi_get_acpi_dev (handle); +if (!adev) +return AE_ERROR ; + + status =acpi_processor_add (adev, &processor_device_ids[0 ]); +acpi_put_acpi_dev (adev); + +return status; +} +static int acpi_processor_container_attach (struct acpi_device *dev,const struct acpi_device_id *id) { +acpi_walk_namespace (ACPI_TYPE_PROCESSOR , dev->handle, +ACPI_UINT32_MAX , acpi_processor_container_walk, +NULL ,NULL ,NULL ); +return 1 ; }
Signature · 1 line
2.30.2
PATCH 20/39 ACPI: processor: Register CPUs that are online, but not described in the DSDT
In reply to Russell King (Oracle)From: James Morse <[email protected]>
ACPI has two descriptions of CPUs, one in the MADT/APIC table, the other in the DSDT. Both are required. (ACPI 6.5's 8.4 "Declaring Processors" says "Each processor in the system must be declared in the ACPI namespace"). Having two descriptions allows firmware authors to get this wrong.
If CPUs are described in the MADT/APIC, they will be brought online early during boot. Once the register_cpu() calls are moved to ACPI, they will be based on the DSDT description of the CPUs. When CPUs are missing from the DSDT description, they will end up online, but not registered.
Add a helper that runs after acpi_init() has completed to register CPUs that are online, but weren't found in the DSDT. Any CPU that is registered by this code triggers a firmware-bug warning and kernel taint.
Qemu TCG only describes the first CPU in the DSDT, unless cpu-hotplug is configured.
--- drivers/acpi/acpi_processor.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+)
drivers/acpi/acpi_processor.c +19 -0
@@ -791,6 +791,25 @@ void __init acpi_processor_init(void)acpi_pcc_cpufreq_init (); } +static int __init acpi_processor_register_missing_cpus (void ) +{ +int cpu; + +if (acpi_disabled) +return 0 ; + +for_each_online_cpu (cpu) { +if (!get_cpu_device (cpu)) { +pr_err_once (FW_BUG "CPU %u has no ACPI namespace description!\n" , cpu); +add_taint (TAINT_FIRMWARE_WORKAROUND ,LOCKDEP_STILL_OK ); +arch_register_cpu (cpu); + } + } + +return 0 ; +} +subsys_initcall_sync (acpi_processor_register_missing_cpus); +#ifdef CONFIG_ACPI_PROCESSOR_CSTATE /** * acpi_processor_claim_cst_control - Request _CST control from the platform.
Signature · 1 line
2.30.2
PATCH 21/39 ACPI: processor: Register all CPUs from acpi_processor_get_info()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
To allow ACPI to skip the call to arch_register_cpu() when the _STA value indicates the CPU can't be brought online right now, move the arch_register_cpu() call into acpi_processor_get_info().
Systems can still be booted with 'acpi=off', or not include an ACPI description at all. For these, the CPUs continue to be registered by cpu_dev_register_generic().
This moves the CPU register logic back to a subsys_initcall(), while the memory nodes will have been registered earlier.
---
Changes since RFC v2:
* Fixup comment in acpi_processor_get_info() (Gavin Shan)
* Add comment in cpu_dev_register_generic() (Gavin Shan)
--- drivers/acpi/acpi_processor.c | 12 ++++++++++++ drivers/base/cpu.c | 6 +++++- 2 files changed, 17 insertions(+), 1 deletion(-)
drivers/acpi/acpi_processor.c +12 -0
@@ -314,6 +314,18 @@ static int acpi_processor_get_info(struct acpi_device *device)cpufreq_add_device ("acpi-cpufreq" ); } +/* +* Register CPUs that are present. get_cpu_device() is used to skip +* duplicate CPU descriptions from firmware. +*/ +if (!invalid_logical_cpuid (pr->id) &&cpu_present (pr->id) && + !get_cpu_device (pr->id)) { +int ret =arch_register_cpu (pr->id); + +if (ret) +return ret; + } +/* * Extra Processor objects may be enumerated on MP systems with * less than the max # of CPUs. They should be ignored _iff
drivers/base/cpu.c +5 -1
@@ -537,7 +537,11 @@ static void __init cpu_dev_register_generic(void) {int i, ret; -if (!IS_ENABLED (CONFIG_GENERIC_CPU_DEVICES )) +/* +* When ACPI is enabled, CPUs are registered via +* acpi_processor_get_info(). +*/ +if (!IS_ENABLED (CONFIG_GENERIC_CPU_DEVICES ) || !acpi_disabled)return ;for_each_present_cpu (i) {
Signature · 1 line
2.30.2
PATCH 22/39 ACPI: Rename ACPI_HOTPLUG_CPU to include 'present'
In reply to Russell King (Oracle)From: James Morse <[email protected]>
The code behind ACPI_HOTPLUG_CPU allows a not-present CPU to become present. This isn't the only use of HOTPLUG_CPU. On arm64 and riscv CPUs can be taken offline as a power saving measure.
On arm64 an offline CPU may be disabled by firmware, preventing it from being brought back online, but it remains present throughout.
Adding code to prevent user-space trying to online these disabled CPUs needs some additional terminology.
Rename the Kconfig symbol CONFIG_ACPI_HOTPLUG_PRESENT_CPU to reflect that it makes possible CPUs present.
HOTPLUG_CPU is untouched as this is only about the ACPI mechanism.
---
Changes since RFC v2:
* Add Loongarch update
--- arch/ia64/Kconfig | 2 +- arch/ia64/include/asm/acpi.h | 2 +- arch/ia64/kernel/acpi.c | 6 +++--- arch/ia64/kernel/setup.c | 2 +- arch/loongarch/Kconfig | 2 +- arch/loongarch/configs/loongson3_defconfig | 2 +- arch/loongarch/kernel/acpi.c | 4 ++-- arch/x86/Kconfig | 2 +- arch/x86/kernel/acpi/boot.c | 4 ++-- drivers/acpi/Kconfig | 4 ++-- drivers/acpi/acpi_processor.c | 10 +++++----- include/linux/acpi.h | 6 +++--- 12 files changed, 23 insertions(+), 23 deletions(-)
arch/ia64/Kconfig +1 -1
@@ -16,7 +16,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_PARPORT select ARCH_MIGHT_HAVE_PC_SERIO select ACPI - select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU + select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_NUMA if NUMA select ARCH_ENABLE_MEMORY_HOTPLUG select ARCH_ENABLE_MEMORY_HOTREMOVE
arch/ia64/include/asm/acpi.h +1 -1
@@ -52,7 +52,7 @@ extern unsigned int is_cpu_cpei_target(unsigned int cpu);extern void set_cpei_target_cpu (unsigned int cpu);extern unsigned int get_cpei_target_cpu (void );extern void prefill_possible_map (void ); -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU extern int additional_cpus;#else #define additional_cpus 0
arch/ia64/kernel/acpi.c +3 -3
@@ -194,7 +194,7 @@ acpi_parse_plat_int_src(union acpi_subtable_headers * header,return 0 ; } -#ifdef CONFIG_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU unsigned int can_cpei_retarget (void ) {extern int cpe_vector;
@@ -711,7 +711,7 @@ int acpi_isa_irq_to_gsi(unsigned isa_irq, u32 *gsi)/* * ACPI based hotplug CPU support */ -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU int acpi_map_cpu2node (acpi_handle handle,int cpu,int physid) {#ifdef CONFIG_ACPI_NUMA
@@ -820,7 +820,7 @@ int acpi_unmap_cpu(int cpu)return (0 ); }EXPORT_SYMBOL (acpi_unmap_cpu); -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ +#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ #ifdef CONFIG_ACPI_NUMA static acpi_status acpi_map_iosapic(acpi_handle handle, u32 depth,
arch/ia64/kernel/setup.c +1 -1
@@ -569,7 +569,7 @@ setup_arch (char **cmdline_p)#ifdef CONFIG_ACPI_NUMA acpi_numa_init ();acpi_numa_fixup (); -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU prefill_possible_map ();#endif per_cpu_scan_finalize((cpumask_empty(&early_cpu_possible_map) ?
arch/loongarch/Kconfig +1 -1
@@ -5,7 +5,7 @@ config LOONGARCH select ACPI select ACPI_GENERIC_GSI if ACPI select ACPI_MCFG if ACPI - select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU + select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_PPTT if ACPI select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI select ARCH_BINFMT_ELF_STATE
arch/loongarch/configs/loongson3_defconfig +1 -1
@@ -59,7 +59,7 @@ CONFIG_ACPI_SPCR_TABLE=y CONFIG_ACPI_TAD=y CONFIG_ACPI_DOCK=y CONFIG_ACPI_IPMI=m -CONFIG_ACPI_HOTPLUG_CPU=y +CONFIG_ACPI_HOTPLUG_PRESENT_CPU=y CONFIG_ACPI_PCI_SLOT=y CONFIG_ACPI_HOTPLUG_MEMORY=y CONFIG_EFI_ZBOOT=y
arch/loongarch/kernel/acpi.c +2 -2
@@ -288,7 +288,7 @@ void __init arch_reserve_mem_area(acpi_physical_address addr, size_t size)memblock_reserve (addr, size); } -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU #include <acpi/processor.h>
@@ -340,4 +340,4 @@ int acpi_unmap_cpu(int cpu) }EXPORT_SYMBOL (acpi_unmap_cpu); -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ +#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */
arch/x86/Kconfig +1 -1
@@ -60,7 +60,7 @@ config X86 # select ACPI_LEGACY_TABLES_LOOKUP if ACPI select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI - select ACPI_HOTPLUG_CPU if ACPI_PROCESSOR && HOTPLUG_CPU + select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU select ARCH_32BIT_OFF_T if X86_32 select ARCH_CLOCKSOURCE_INIT select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
arch/x86/kernel/acpi/boot.c +2 -2
@@ -814,7 +814,7 @@ static void __init acpi_set_irq_model_ioapic(void)/* * ACPI based hotplug support for CPU */ -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU #include <acpi/processor.h> static int acpi_map_cpu2node (acpi_handle handle,int cpu,int physid)
@@ -863,7 +863,7 @@ int acpi_unmap_cpu(int cpu) return (0); }EXPORT_SYMBOL (acpi_unmap_cpu); -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ +#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ int acpi_register_ioapic (acpi_handle handle,u64 phys_addr,u32 gsi_base) {
drivers/acpi/Kconfig +2 -2
@@ -305,7 +305,7 @@ config ACPI_IPMI To compile this driver as a module, choose M here: the module will be called as acpi_ipmi. -config ACPI_HOTPLUG_CPU +config ACPI_HOTPLUG_PRESENT_CPU bool depends on ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_CONTAINER
@@ -399,7 +399,7 @@ config ACPI_PCI_SLOT config ACPI_CONTAINER bool "Container and Module Devices" - default (ACPI_HOTPLUG_MEMORY || ACPI_HOTPLUG_CPU) + default (ACPI_HOTPLUG_MEMORY || ACPI_HOTPLUG_PRESENT_CPU) help This driver supports ACPI Container and Module devices (IDs ACPI0004, PNP0A05, and PNP0A06).
drivers/acpi/acpi_processor.c +5 -5
@@ -183,7 +183,7 @@ static void __init acpi_pcc_cpufreq_init(void) {}#endif /* CONFIG_X86 */ /* Initialization */ -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU static int acpi_processor_hotadd_init (struct acpi_processor *pr) {unsigned long long sta;
@@ -228,7 +228,7 @@ static inline int acpi_processor_hotadd_init(struct acpi_processor *pr) {return -ENODEV ; } -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ +#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ static int acpi_processor_get_info (struct acpi_device *device) {
@@ -461,7 +461,7 @@ static int acpi_processor_add(struct acpi_device *device,return result; } -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU /* Removal */ static void acpi_processor_remove (struct acpi_device *device) {
@@ -505,7 +505,7 @@ static void acpi_processor_remove(struct acpi_device *device)free_cpumask_var (pr->throttling.shared_cpu_map); kfree(pr); } -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ +#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ #ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC bool __init processor_physically_present (acpi_handle handle)
@@ -630,7 +630,7 @@ static const struct acpi_device_id processor_device_ids[] = {static struct acpi_scan_handler processor_handler = { .ids = processor_device_ids, .attach = acpi_processor_add, -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU .detach = acpi_processor_remove, #endif .hotplug = {
include/linux/acpi.h +3 -3
@@ -321,12 +321,12 @@ static inline int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, }#endif -#ifdef CONFIG_ACPI_HOTPLUG_CPU +#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU /* Arch dependent functions for cpu hotplug support */ int acpi_map_cpu (acpi_handle handle,phys_cpuid_t physid,u32 acpi_id,int *pcpu);int acpi_unmap_cpu (int cpu); -#endif /* CONFIG_ACPI_HOTPLUG_CPU */ +#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC int acpi_get_ioapic_id (acpi_handle handle,u32 gsi_base,u64 *phys_addr);
@@ -649,7 +649,7 @@ static inline u32 acpi_osc_ctx_get_cxl_control(struct acpi_osc_context *context)#define ACPI_GSB_ACCESS_ATTRIB_RAW_PROCESS 0x0000000F/* Enable _OST when all relevant hotplug operations are enabled */ -#if defined(CONFIG_ACPI_HOTPLUG_CPU ) && \ +#if defined(CONFIG_ACPI_HOTPLUG_PRESENT_CPU ) && \ defined(CONFIG_ACPI_HOTPLUG_MEMORY ) && \ defined(CONFIG_ACPI_CONTAINER )#define ACPI_HOTPLUG_OST
Signature · 1 line
2.30.2
PATCH 23/39 drivers: base: Implement weak arch_unregister_cpu()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Add arch_unregister_cpu() to allow the ACPI machinery to call unregister_cpu(). This is enough for arm64, riscv and loongarch, but needs to be overridden by x86 and ia64 who need to do more work.
---
Changes since v1:
* Added CONFIG_HOTPLUG_CPU ifdeffery around unregister_cpu
Changes since RFC v2:
* Move earlier in the series
--- drivers/base/cpu.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/base/cpu.c +8 -1
@@ -531,7 +531,14 @@ int __weak arch_register_cpu(int cpu) {return register_cpu (&per_cpu (cpu_devices, cpu), cpu); } -#endif + +#ifdef CONFIG_HOTPLUG_CPU +void __weak arch_unregister_cpu (int num) +{ +unregister_cpu (&per_cpu (cpu_devices, num)); +} +#endif /* CONFIG_HOTPLUG_CPU */ +#endif /* CONFIG_GENERIC_CPU_DEVICES */ static void __init cpu_dev_register_generic (void ) {
Signature · 1 line
2.30.2
PATCH 24/39 ACPI: Move acpi_bus_trim_one() before acpi_scan_hot_remove()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
A subsequent patch will change acpi_scan_hot_remove() to call acpi_bus_trim_one() instead of acpi_bus_trim(), meaning it can no longer rely on the prototype in the header file.
Move these functions further up the file.
No change in behaviour.
--- drivers/acpi/scan.c | 76 ++++++++++++++++++++++----------------------- 1 file changed, 38 insertions(+), 38 deletions(-)
drivers/acpi/scan.c +38 -38
@@ -244,6 +244,44 @@ static int acpi_scan_try_to_offline(struct acpi_device *device)return 0 ; } +static int acpi_bus_trim_one (struct acpi_device *adev,void *not_used) +{ +struct acpi_scan_handler *handler = adev->handler; + +acpi_dev_for_each_child_reverse (adev, acpi_bus_trim_one,NULL ); + + adev->flags.match_driver = false; +if (handler) { +if (handler->detach) + handler->detach (adev); + + adev->handler =NULL ; + }else { +device_release_driver (&adev->dev); + } +/* +* Most likely, the device is going away, so put it into D3cold before +* that. +*/ +acpi_device_set_power (adev,ACPI_STATE_D3_COLD ); + adev->flags.initialized = false; +acpi_device_clear_enumerated (adev); + +return 0 ; +} + +/** +* acpi_bus_trim - Detach scan handlers and drivers from ACPI device objects. +* @adev: Root of the ACPI namespace scope to walk. +* +* Must be called under acpi_scan_lock. +*/ +void acpi_bus_trim (struct acpi_device *adev) +{ +acpi_bus_trim_one (adev,NULL ); +} +EXPORT_SYMBOL_GPL (acpi_bus_trim); +static int acpi_scan_hot_remove (struct acpi_device *device) {acpi_handle handle = device->handle;
@@ -2507,44 +2545,6 @@ int acpi_bus_scan(acpi_handle handle) }EXPORT_SYMBOL (acpi_bus_scan); -static int acpi_bus_trim_one (struct acpi_device *adev,void *not_used) -{ -struct acpi_scan_handler *handler = adev->handler; - -acpi_dev_for_each_child_reverse (adev, acpi_bus_trim_one,NULL ); - - adev->flags.match_driver = false; -if (handler) { -if (handler->detach) - handler->detach (adev); - - adev->handler =NULL ; - }else { -device_release_driver (&adev->dev); - } -/* -* Most likely, the device is going away, so put it into D3cold before -* that. -*/ -acpi_device_set_power (adev,ACPI_STATE_D3_COLD ); - adev->flags.initialized = false; -acpi_device_clear_enumerated (adev); - -return 0 ; -} - -/** -* acpi_bus_trim - Detach scan handlers and drivers from ACPI device objects. -* @adev: Root of the ACPI namespace scope to walk. -* -* Must be called under acpi_scan_lock. -*/ -void acpi_bus_trim (struct acpi_device *adev) -{ -acpi_bus_trim_one (adev,NULL ); -} -EXPORT_SYMBOL_GPL (acpi_bus_trim); -int acpi_bus_register_early_device (int type) {struct acpi_device *device =NULL ;
Signature · 1 line
2.30.2
PATCH 25/39 ACPI: Rename acpi_processor_hotadd_init and remove pre-processor guards
In reply to Russell King (Oracle)From: James Morse <[email protected]>
acpi_processor_hotadd_init() will make a CPU present by mapping it based on its hardware id.
'hotadd_init' is ambiguous once there are two different behaviours for cpu hotplug. This is for toggling the _STA present bit. Subsequent patches will add support for toggling the _STA enabled bit, named acpi_processor_make_enabled().
Rename it acpi_processor_make_present() to make it clear this is for CPUs that were not previously present.
Expose the function prototypes it uses to allow the preprocessor guards to be removed. The IS_ENABLED() check will let the compiler dead-code elimination pass remove this if it isn't going to be used.
--- drivers/acpi/acpi_processor.c | 14 +++++--------- include/linux/acpi.h | 2 -- 2 files changed, 5 insertions(+), 11 deletions(-)
drivers/acpi/acpi_processor.c +5 -9
@@ -183,13 +183,15 @@ static void __init acpi_pcc_cpufreq_init(void) {}#endif /* CONFIG_X86 */ /* Initialization */ -#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU -static int acpi_processor_hotadd_init (struct acpi_processor *pr) +static int acpi_processor_make_present (struct acpi_processor *pr) {unsigned long long sta;acpi_status status;int ret; +if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) +return -ENODEV ; +if (invalid_phys_cpuid (pr->phys_id))return -ENODEV ;
@@ -223,12 +225,6 @@ static int acpi_processor_hotadd_init(struct acpi_processor *pr)cpu_maps_update_done ();return ret; } -#else -static inline int acpi_processor_hotadd_init (struct acpi_processor *pr) -{ -return -ENODEV ; -} -#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ static int acpi_processor_get_info (struct acpi_device *device) {
@@ -335,7 +331,7 @@ static int acpi_processor_get_info(struct acpi_device *device)* because cpuid <-> apicid mapping is persistent now. */ if (invalid_logical_cpuid (pr->id) || !cpu_present (pr->id)) { -int ret =acpi_processor_hotadd_init (pr); +int ret =acpi_processor_make_present (pr);if (ret)return ret;
include/linux/acpi.h +0 -2
@@ -321,12 +321,10 @@ static inline int acpi_processor_evaluate_cst(acpi_handle handle, u32 cpu, }#endif -#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU /* Arch dependent functions for cpu hotplug support */ int acpi_map_cpu (acpi_handle handle,phys_cpuid_t physid,u32 acpi_id,int *pcpu);int acpi_unmap_cpu (int cpu); -#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ #ifdef CONFIG_ACPI_HOTPLUG_IOAPIC int acpi_get_ioapic_id (acpi_handle handle,u32 gsi_base,u64 *phys_addr);
Signature · 1 line
2.30.2
PATCH 26/39 ACPI: Add post_eject to struct acpi_scan_handler for cpu hotplug
In reply to Russell King (Oracle)From: James Morse <[email protected]>
struct acpi_scan_handler has a detach callback that is used to remove a driver when a bus is changed. When interacting with an eject-request, the detach callback is called before _EJ0.
This means the ACPI processor driver can't use _STA to determine if a CPU has been made not-present, or some of the other _STA bits have been changed. acpi_processor_remove() needs to know the value of _STA after _EJ0 has been called.
Add a post_eject callback to struct acpi_scan_handler. This is called after acpi_scan_hot_remove() has successfully called _EJ0. Because acpi_bus_trim_one() also clears the handler pointer, it needs to be told if the caller will go on to call acpi_bus_post_eject(), so that acpi_device_clear_enumerated() and clearing the handler pointer can be deferred. The existing not-used pointer is used for this.
--- drivers/acpi/acpi_processor.c | 4 +-- drivers/acpi/scan.c | 52 ++++++++++++++++++++++++++++++----- include/acpi/acpi_bus.h | 1 + 3 files changed, 48 insertions(+), 9 deletions(-)
drivers/acpi/acpi_processor.c +2 -2
@@ -459,7 +459,7 @@ static int acpi_processor_add(struct acpi_device *device,#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU /* Removal */ -static void acpi_processor_remove (struct acpi_device *device) +static void acpi_processor_post_eject (struct acpi_device *device) {struct acpi_processor *pr;
@@ -627,7 +627,7 @@ static struct acpi_scan_handler processor_handler = { .ids = processor_device_ids, .attach = acpi_processor_add, #ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU - .detach = acpi_processor_remove, + .post_eject = acpi_processor_post_eject, #endif .hotplug = { .enabled = true,
drivers/acpi/scan.c +45 -7
98 lines not on this page · raw message
include/acpi/acpi_bus.h +1 -0
@@ -127,6 +127,7 @@ struct acpi_scan_handler {bool (*match)(const char *idstr,const struct acpi_device_id **matchid);int (*attach)(struct acpi_device *dev,const struct acpi_device_id *id);void (*detach)(struct acpi_device *dev); +void (*post_eject)(struct acpi_device *dev);void (*bind)(struct device *phys_dev);void (*unbind)(struct device *phys_dev);struct acpi_hotplug_profile hotplug;
Signature · 1 line
2.30.2
PATCH 27/39 ACPI: Check _STA present bit before making CPUs not present
In reply to Russell King (Oracle)From: James Morse <[email protected]>
When called acpi_processor_post_eject() unconditionally make a CPU not-present and unregisters it.
To add support for AML events where the CPU has become disabled, but remains present, the _STA method should be checked before calling acpi_processor_remove().
Rename acpi_processor_post_eject() acpi_processor_remove_possible(), and check the _STA before calling.
Adding the function prototype for arch_unregister_cpu() allows the preprocessor guards to be removed.
After this change CPUs will remain registered and visible to user-space as offline if buggy firmware triggers an eject-request, but doesn't clear the corresponding _STA bits after _EJ0 has been called.
--- drivers/acpi/acpi_processor.c | 31 +++++++++++++++++++++++++------ 1 file changed, 25 insertions(+), 6 deletions(-)
drivers/acpi/acpi_processor.c +25 -6
@@ -457,13 +457,12 @@ static int acpi_processor_add(struct acpi_device *device,return result; } -#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU /* Removal */ -static void acpi_processor_post_eject (struct acpi_device *device) +static void acpi_processor_make_not_present (struct acpi_device *device) {struct acpi_processor *pr; -if (!device || !acpi_driver_data (device)) +if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU ))return ; pr =acpi_driver_data (device);
@@ -501,7 +500,29 @@ static void acpi_processor_post_eject(struct acpi_device *device)free_cpumask_var (pr->throttling.shared_cpu_map);kfree (pr); } -#endif /* CONFIG_ACPI_HOTPLUG_PRESENT_CPU */ + +static void acpi_processor_post_eject (struct acpi_device *device) +{ +struct acpi_processor *pr; +unsigned long long sta; +acpi_status status; + +if (!device) +return ; + + pr =acpi_driver_data (device); +if (!pr || pr->id >= nr_cpu_ids ||invalid_phys_cpuid (pr->phys_id)) +return ; + + status =acpi_evaluate_integer (pr->handle,"_STA" ,NULL , &sta); +if (ACPI_FAILURE (status)) +return ; + +if (cpu_present (pr->id) && !(sta &ACPI_STA_DEVICE_PRESENT )) { +acpi_processor_make_not_present (device); +return ; + } +}#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC bool __init processor_physically_present (acpi_handle handle)
@@ -626,9 +647,7 @@ static const struct acpi_device_id processor_device_ids[] = {static struct acpi_scan_handler processor_handler = { .ids = processor_device_ids, .attach = acpi_processor_add, -#ifdef CONFIG_ACPI_HOTPLUG_PRESENT_CPU .post_eject = acpi_processor_post_eject, -#endif .hotplug = { .enabled = true, },
Signature · 1 line
2.30.2
PATCH 28/39 ACPI: Warn when the present bit changes but the feature is not enabled
In reply to Russell King (Oracle)From: James Morse <[email protected]>
ACPI firmware can trigger the events to add and remove CPUs, but the OS may not support this.
Print an error message when this happens.
This gives early warning on arm64 systems that don't support CONFIG_ACPI_HOTPLUG_PRESENT_CPU, as making CPUs not present has side effects for other parts of the system.
---
Changes since RFC v2:
* Update commit message with suggestion from Gavin Shan
--- drivers/acpi/acpi_processor.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-)
drivers/acpi/acpi_processor.c +6 -2
@@ -189,8 +189,10 @@ static int acpi_processor_make_present(struct acpi_processor *pr)acpi_status status;int ret; -if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) +if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) { +pr_err_once ("Changing CPU present bit is not supported\n" );return -ENODEV ; + }if (invalid_phys_cpuid (pr->phys_id))return -ENODEV ;
@@ -462,8 +464,10 @@ static void acpi_processor_make_not_present(struct acpi_device *device) {struct acpi_processor *pr; -if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) +if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) { +pr_err_once ("Changing CPU present bit is not supported" );return ; + } pr =acpi_driver_data (device);if (pr->id >= nr_cpu_ids)
Signature · 1 line
2.30.2
PATCH 29/39 LoongArch: Use the __weak version of arch_unregister_cpu()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
LoongArch provides its own arch_unregister_cpu(). This clears the hotpluggable flag, then unregisters the CPU.
It isn't necessary to clear the hotpluggable flag when unregistering a cpu. unregister_cpu() writes NULL to the percpu cpu_sys_devices pointer, meaning cpu_is_hotpluggable() will return false, as get_cpu_device() has returned NULL.
Remove arch_unregister_cpu() and use the __weak version.
--- arch/loongarch/kernel/topology.c | 9 --------- 1 file changed, 9 deletions(-)
arch/loongarch/kernel/topology.c +0 -9
@@ -19,13 +19,4 @@ int arch_register_cpu(int cpu) return register_cpu(c, cpu); }EXPORT_SYMBOL (arch_register_cpu); - -void arch_unregister_cpu (int cpu) -{ -struct cpu *c = &per_cpu (cpu_devices, cpu); - - c->hotpluggable =0 ; -unregister_cpu (c); -} -EXPORT_SYMBOL (arch_unregister_cpu);#endif
Signature · 1 line
2.30.2
PATCH 30/39 arm64: acpi: Move get_cpu_for_acpi_id() to a header
In reply to Russell King (Oracle)From: James Morse <[email protected]>
ACPI identifies CPUs by UID. get_cpu_for_acpi_id() maps the ACPI UID to the linux CPU number.
The helper to retrieve this mapping is only available in arm64's numa code.
Move it to live next to get_acpi_id_for_cpu().
--- arch/arm64/include/asm/acpi.h | 11 +++++++++++ arch/arm64/kernel/acpi_numa.c | 11 ----------- 2 files changed, 11 insertions(+), 11 deletions(-)
arch/arm64/include/asm/acpi.h +11 -0
@@ -119,6 +119,17 @@ static inline u32 get_acpi_id_for_cpu(unsigned int cpu)return acpi_cpu_get_madt_gicc (cpu)->uid; } +static inline int get_cpu_for_acpi_id (u32 uid) +{ +int cpu; + +for (cpu =0 ; cpu < nr_cpu_ids; cpu++) +if (uid ==get_acpi_id_for_cpu (cpu)) +return cpu; + +return -EINVAL ; +} +static inline void arch_fix_phys_package_id (int num,u32 slot) { }void __init acpi_init_cpus (void );int apei_claim_sea (struct pt_regs *regs);
arch/arm64/kernel/acpi_numa.c +0 -11
@@ -34,17 +34,6 @@ int __init acpi_numa_get_nid(unsigned int cpu)return acpi_early_node_map[cpu]; } -static inline int get_cpu_for_acpi_id (u32 uid) -{ -int cpu; - -for (cpu =0 ; cpu < nr_cpu_ids; cpu++) -if (uid ==get_acpi_id_for_cpu (cpu)) -return cpu; - -return -EINVAL ; -} -static int __init acpi_parse_gicc_pxm (union acpi_subtable_headers *header,const unsigned long end) {
Signature · 1 line
2.30.2
PATCH 31/39 ACPICA: Add new MADT GICC flags fields
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Add the new flag field to the MADT's GICC structure.
'Online Capable' indicates a disabled CPU can be enabled later. See ACPI specification 6.5 Tabel 5.37: GICC CPU Interface Flags.
---
This patch probably needs to go via the upstream acpica project, but is included here so the feature can be tested.
If the ACPICA header files are updated before merging this patch set, this patch will need to be dropped.
Changes since RFC v2:
* Add ACPI specification reference.
--- include/acpi/actbl2.h | 1 + 1 file changed, 1 insertion(+)
include/acpi/actbl2.h +1 -0
@@ -1046,6 +1046,7 @@ struct acpi_madt_generic_interrupt {/* ACPI_MADT_ENABLED (1) Processor is usable if set */ #define ACPI_MADT_PERFORMANCE_IRQ_MODE (1<<1)/* 01: Performance Interrupt Mode */ #define ACPI_MADT_VGIC_IRQ_MODE (1<<2)/* 02: VGIC Maintenance Interrupt mode */ +#define ACPI_MADT_GICC_CPU_CAPABLE (1<<3)/* 03: CPU is online capable */ /* 12: Generic Distributor (ACPI 5.0 + ACPI 6.0 changes) */
Signature · 1 line
2.30.2
PATCH 32/39 irqchip/gic-v3: Don't return errors from gic_acpi_match_gicc()
In reply to Russell King (Oracle)From: James Morse <[email protected]>
gic_acpi_match_gicc() is only called via gic_acpi_count_gicr_regions(). It should only count the number of enabled redistributors, but it also tries to sanity check the GICC entry, currently returning an error if the Enabled bit is set, but the gicr_base_address is zero.
Adding support for the online-capable bit to the sanity check complicates it, for no benefit. The existing check implicitly depends on gic_acpi_count_gicr_regions() previous failing to find any GICR regions (as it is valid to have gicr_base_address of zero if the redistributors are described via a GICR entry).
Instead of complicating the check, remove it. Failures that happen at this point cause the irqchip not to register, meaning no irqs can be requested. The kernel grinds to a panic() pretty quickly.
Without the check, MADT tables that exhibit this problem are still caught by gic_populate_rdist(), which helpfully also prints what went wrong:
| CPU4: mpidr 100 has no re-distributor!
--- drivers/irqchip/irq-gic-v3.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
drivers/irqchip/irq-gic-v3.c +6 -12
@@ -2428,21 +2428,15 @@ static int __init gic_acpi_match_gicc(union acpi_subtable_headers *header,/* * If GICC is enabled and has valid gicr base address, then it means -* GICR base is presented via GICC +* GICR base is presented via GICC. The redistributor is only known to +* be accessible if the GICC is marked as enabled. If this bit is not +* set, we'd need to add the redistributor at runtime, which isn't +* supported. */ -if (acpi_gicc_is_usable (gicc) && gicc->gicr_base_address) { +if (gicc->flags &ACPI_MADT_ENABLED && gicc->gicr_base_address) acpi_data.enabled_rdists++; -return 0 ; - } -/* -* It's perfectly valid firmware can pass disabled GICC entry, driver -* should not treat as errors, skip the entry instead of probe fail. -*/ -if (!acpi_gicc_is_usable (gicc)) -return 0 ; - -return -ENODEV ; +return 0 ; }static int __init gic_acpi_count_gicr_regions (void )
Signature · 1 line
2.30.2
PATCH 33/39 irqchip/gic-v3: Add support for ACPI's disabled but 'online capable' CPUs
In reply to Russell King (Oracle)From: James Morse <[email protected]>
To support virtual CPU hotplug, ACPI has added an 'online capable' bit to the MADT GICC entries. This indicates a disabled CPU entry may not be possible to online via PSCI until firmware has set enabled bit in _STA.
What about the redistributor in the GICC entry? ACPI doesn't want to say. Assume the worst: When a redistributor is described in the GICC entry, but the entry is marked as disabled at boot, assume the redistributor is inaccessible.
The GICv3 driver doesn't support late online of redistributors, so this means the corresponding CPU can't be brought online either. Clear the possible and present bits.
Systems that want CPU hotplug in a VM can ensure their redistributors are always-on, and describe them that way with a GICR entry in the MADT.
When mapping redistributors found via GICC entries, handle the case where the arch code believes the CPU is present and possible, but it does not have an accessible redistributor. Print a warning and clear the present and possible bits.
----
Disabled but online-capable CPUs cause this message to be printed if their redistributors are described via GICC:
| GICv3: CPU 3's redistributor is inaccessible: this CPU can't be brought online
If ACPI's _STA tries to make the cpu present later, this message is printed:
| Changing CPU present bit is not supported
Changes since RFC v2:
* use gicc->flags & (ACPI_MADT_ENABLED | ACPI_MADT_GICC_CPU_CAPABLE)
--- drivers/irqchip/irq-gic-v3.c | 14 ++++++++++++++ include/linux/acpi.h | 2 +- 2 files changed, 15 insertions(+), 1 deletion(-)
drivers/irqchip/irq-gic-v3.c +14 -0
@@ -2378,11 +2378,25 @@ gic_acpi_parse_madt_gicc(union acpi_subtable_headers *header, (struct acpi_madt_generic_interrupt *)header;u32 reg =readl_relaxed (acpi_data.dist_base +GICD_PIDR2 ) &GIC_PIDR2_ARCH_MASK ;u32 size = reg == GIC_PIDR2_ARCH_GICv4 ?SZ_64K *4 :SZ_64K *2 ; +int cpu =get_cpu_for_acpi_id (gicc->uid);void __iomem *redist_base;if (!acpi_gicc_is_usable (gicc))return 0 ; +/* +* Capable but disabled CPUs can be brought online later. What about +* the redistributor? ACPI doesn't want to say! +* Virtual hotplug systems can use the MADT's "always-on" GICR entries. +* Otherwise, prevent such CPUs from being brought online. +*/ +if (!(gicc->flags &ACPI_MADT_ENABLED )) { +pr_warn_once ("CPU %u's redistributor is inaccessible: this CPU can't be brought online\n" , cpu); +set_cpu_present (cpu, false); +set_cpu_possible (cpu, false); +return 0 ; + } + redist_base =ioremap (gicc->gicr_base_address, size);if (!redist_base)return -ENOMEM ;
include/linux/acpi.h +1 -1
@@ -258,7 +258,7 @@ void acpi_table_print_madt_entry (struct acpi_subtable_header *madt);static inline bool acpi_gicc_is_usable (struct acpi_madt_generic_interrupt *gicc) { -return gicc->flags &ACPI_MADT_ENABLED ; +return gicc->flags & (ACPI_MADT_ENABLED |ACPI_MADT_GICC_CPU_CAPABLE ); }/* the following numa functions are architecture-dependent */
Signature · 1 line
2.30.2
PATCH 34/39 arm64: psci: Ignore DENIED CPUs
In reply to Russell King (Oracle)From: Jean-Philippe Brucker <[email protected]>
When a CPU is marked as disabled, but online capable in the MADT, PSCI applies some firmware policy to control when it can be brought online. PSCI returns DENIED to a CPU_ON request if this is not currently permitted. The OS can learn the current policy from the _STA enabled bit.
Handle the PSCI DENIED return code gracefully instead of printing an error.
See https://developer.arm.com/documentation/den0022/f/?lang=en page 58.
---
Changes since RFC v2
* Add specification reference
* Use EPERM rather than EPROBE_DEFER
--- arch/arm64/kernel/psci.c | 2 +- arch/arm64/kernel/smp.c | 3 ++- drivers/firmware/psci/psci.c | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-)
arch/arm64/kernel/psci.c +1 -1
@@ -40,7 +40,7 @@ static int cpu_psci_cpu_boot(unsigned int cpu) {phys_addr_t pa_secondary_entry =__pa_symbol (secondary_entry);int err = psci_ops.cpu_on (cpu_logical_map (cpu), pa_secondary_entry); -if (err) +if (err && err != -EPROBE_DEFER )pr_err ("failed to boot CPU%d (%d)\n" , cpu, err);return err;
arch/arm64/kernel/smp.c +2 -1
@@ -124,7 +124,8 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)/* Now bring the CPU into our world */ ret =boot_secondary (cpu, idle);if (ret) { -pr_err ("CPU%u: failed to boot: %d\n" , cpu, ret); +if (ret != -EPERM ) +pr_err ("CPU%u: failed to boot: %d\n" , cpu, ret);return ret; }
drivers/firmware/psci/psci.c +2 -0
@@ -218,6 +218,8 @@ static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point)int err; err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err); }
Signature · 1 line
2.30.2
RE: [PATCH 34/39] arm64: psci: Ignore DENIED CPUs
Hi Russell,
One inline comment.
Quoted 46 lines
-----Original Message-----
From: Russell King <[email protected]> On Behalf Of Russell King
Sent: 2023年10月24日 23:19
To: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]
Cc: Salil Mehta <[email protected]>; Jean-Philippe Brucker <[email protected]>; Jianyong Wu <[email protected]>; Justin He <[email protected]>; James Morse <[email protected]>; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Mark Rutland <[email protected]>; Lorenzo Pieralisi <[email protected]>
Subject: [PATCH 34/39] arm64: psci: Ignore DENIED CPUsFrom: Jean-Philippe Brucker <[email protected]>
When a CPU is marked as disabled, but online capable in the MADT, PSCI applies some firmware policy to control when it can be brought online. PSCI returns DENIED to a
CPU_ONrequest if this is not currently permitted. The OS can learn the current policy from the_STAenabled bit.Handle the PSCI DENIED return code gracefully instead of printing an error.
See https://developer.arm.com/documentation/den0022/f/?lang=en page 58.
Signed-off-by Jean-Philippe Brucker <[email protected]> [ morse:Rewrote commit message ]
Signed-off-by James Morse <[email protected]>Signed-off-by Russell King (Oracle) <[email protected]>---
Changes since RFC v2
* Add specification reference
* Use EPERM rather thanEPROBE_DEFER--- arch/arm64/kernel/psci.c | 2 +- arch/arm64/kernel/smp.c | 3 ++- drivers/firmware/psci/psci.c | 2 ++ 3 files changed, 5 insertions(+), 2 deletions(-)arch/arm64/kernel/psci.c index renamed from arch/arm64/kernel/psci.c +0
-0
29a8e444db83..4fcc0cdd757b100644
arch/arm64/kernel/psci.c static int cpu_psci_cpu_boot(unsigned int cpu) { · line 42
Quoted 4 lines
arch/arm64/kernel/psci.c +1
-1@@ -40,7 +40,7 @@ static int cpu_psci_cpu_boot(unsigned int cpu) {phys_addr_t pa_secondary_entry =__pa_symbol (secondary_entry);int err = psci_ops.cpu_on (cpu_logical_map (cpu), pa_secondary_entry); -if (err) +if (err && err != -EPROBE_DEFER )
Should this be EPERM? As the following psci cpu_on op will return it. I think you miss to change this when apply Jean-Philippe's patch.
Thanks
Jianyong
Quoted 33 lines
arch/arm64/kernel/psci.c +0
-0pr_err ("failed to boot CPU%d (%d)\n" , cpu, err);return err;arch/arm64/kernel/smp.c index renamed from arch/arm64/kernel/smp.c +0
-0
8c8f55721786..68ec7fbe166f100644arch/arm64/kernel/smp.c +2
-1@@ -124,7 +124,8 @@ int __cpu_up(unsigned int cpu, struct task_struct *idle)/* Now bring the CPU into our world */ ret =boot_secondary (cpu, idle);if (ret) { -pr_err ("CPU%u: failed to boot: %d\n" , cpu, ret); +if (ret != -EPERM ) +pr_err ("CPU%u: failed to boot: %d\n" , cpu, ret);return ret; }drivers/firmware/psci/psci.c index renamed from drivers/firmware/psci/psci.c +0
-0
d9629ff87861..ee82e7880d8c100644drivers/firmware/psci/psci.c +2
-1@@ -218,6 +218,8 @@ static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point)int err; err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err); } --2.30.2
Re: [PATCH 34/39] arm64: psci: Ignore DENIED CPUs
Quoted 3 lines · Jianyong Wu
Hi Russell,
One inline comment.
...
Quoted 3 lines
Quoted 3 lines
Changes since RFC v2
* Add specification reference
* Use EPERM rather thanEPROBE_DEFER
...
Quoted 8 lines
arch/arm64/kernel/psci.c static int cpu_psci_cpu_boot(unsigned int cpu) { · line 42
Quoted 5 lines
arch/arm64/kernel/psci.c +1
-1@@ -40,7 +40,7 @@ static int cpu_psci_cpu_boot(unsigned int cpu) {phys_addr_t pa_secondary_entry =__pa_symbol (secondary_entry);int err = psci_ops.cpu_on (cpu_logical_map (cpu), pa_secondary_entry); -if (err) +if (err && err != -EPROBE_DEFER )Should this be EPERM? As the following psci
cpu_onop will return it. I think you miss to change this when apply Jean-Philippe's patch.
It looks like James didn't properly update all places. Also,
Quoted 8 lines
Quoted 8 lines
drivers/firmware/psci/psci.c index renamed from drivers/firmware/psci/psci.c +0
-0
d9629ff87861..ee82e7880d8c100644drivers/firmware/psci/psci.c +0
-0@@ -218,6 +218,8 @@ static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point)int err;
Quoted 4 lines
drivers/firmware/psci/psci.c static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point) · line 223
Quoted 4 lines
drivers/firmware/psci/psci.c +2
-0err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err);
This change is unnecessary - probably comes from when -EPROBE_DEFER was being used. psci_to_linux_errno() already does:
case PSCI_RET_DENIED:
return -EPERM;Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
RE: [PATCH 34/39] arm64: psci: Ignore DENIED CPUs
Quoted 48 lines
-----Original Message-----
From: Russell King <[email protected]>
Sent: 2023年11月20日 17:25
To: Jianyong Wu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Salil Mehta <[email protected]>; Jean-Philippe Brucker <[email protected]>; Justin He <[email protected]>; James Morse <[email protected]>; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Mark Rutland <[email protected]>; Lorenzo Pieralisi <[email protected]>
Subject: Re: [PATCH 34/39] arm64: psci: Ignore DENIED CPUsQuoted 3 lines · Jianyong Wu
Hi Russell,
One inline comment.
...
Quoted 3 lines
Quoted 3 lines
Changes since RFC v2
* Add specification reference
* Use EPERM rather thanEPROBE_DEFER...
Quoted 8 lines
arch/arm64/kernel/psci.c static int cpu_psci_cpu_boot(unsigned int cpu) { · line 42
Quoted 5 lines
arch/arm64/kernel/psci.c +1
-1@@ -40,7 +40,7 @@ static int cpu_psci_cpu_boot(unsigned int cpu) {phys_addr_t pa_secondary_entry =__pa_symbol (secondary_entry);int err = psci_ops.cpu_on (cpu_logical_map (cpu), pa_secondary_entry); -if (err) +if (err && err != -EPROBE_DEFER )Should this be EPERM? As the following psci
cpu_onop will return it. I think you miss to change this when apply Jean-Philippe's patch.It looks like James didn't properly update all places. Also,
Quoted 9 lines
Quoted 9 lines
(quoted diff) +0
-0
b/drivers/firmware/psci/psci.cindexd9629ff87861..ee82e7880d8c100644drivers/firmware/psci/psci.c +0
-0@@ -218,6 +218,8 @@ static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point)int err;
Quoted 7 lines
Quoted 4 lines
drivers/firmware/psci/psci.c static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point) · line 223
Quoted 4 lines
drivers/firmware/psci/psci.c +2
-0err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err);This change is unnecessary - probably comes from when -EPROBE_DEFER was being used.
psci_to_linux_errno()already does:
But may print lots of noise like:
[ 0.008955] smp: Bringing up secondary CPUs ... [ 0.009661] psci: failed to boot CPU1 (-1) [ 0.010360] psci: failed to boot CPU2 (-1) [ 0.011164] psci: failed to boot CPU3 (-1) [ 0.011946] psci: failed to boot CPU4 (-1) [ 0.012764] psci: failed to boot CPU5 (-1) [ 0.013534] psci: failed to boot CPU6 (-1) [ 0.014349] psci: failed to boot CPU7 (-1) [ 0.014820] smp: Brought up 1 node, 1 CPU
Is this expected?
Thanks
Jianyong
Quoted 9 lines
case PSCI_RET_DENIED: return -EPERM;Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Re: [PATCH 34/39] arm64: psci: Ignore DENIED CPUs
Quoted 68 lines · Jianyong Wu
Quoted 48 lines
-----Original Message-----
From: Russell King <[email protected]>
Sent: 2023年11月20日 17:25
To: Jianyong Wu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Salil Mehta <[email protected]>; Jean-Philippe Brucker <[email protected]>; Justin He <[email protected]>; James Morse <[email protected]>; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Mark Rutland <[email protected]>; Lorenzo Pieralisi <[email protected]>
Subject: Re: [PATCH 34/39] arm64: psci: Ignore DENIED CPUsQuoted 3 lines · Jianyong Wu
Hi Russell,
One inline comment.
...
Quoted 3 lines
Quoted 3 lines
Changes since RFC v2
* Add specification reference
* Use EPERM rather thanEPROBE_DEFER...
Quoted 8 lines
arch/arm64/kernel/psci.c static int cpu_psci_cpu_boot(unsigned int cpu) { · line 42
Quoted 5 lines
arch/arm64/kernel/psci.c +1
-1@@ -40,7 +40,7 @@ static int cpu_psci_cpu_boot(unsigned int cpu) {phys_addr_t pa_secondary_entry =__pa_symbol (secondary_entry);int err = psci_ops.cpu_on (cpu_logical_map (cpu), pa_secondary_entry); -if (err) +if (err && err != -EPROBE_DEFER )Should this be EPERM? As the following psci
cpu_onop will return it. I think you miss to change this when apply Jean-Philippe's patch.It looks like James didn't properly update all places. Also,
Quoted 9 lines
Quoted 9 lines
(quoted diff) +0
-0
b/drivers/firmware/psci/psci.cindexd9629ff87861..ee82e7880d8c100644drivers/firmware/psci/psci.c +0
-0@@ -218,6 +218,8 @@ static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point)int err;Quoted 7 lines
Quoted 4 lines
drivers/firmware/psci/psci.c static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point) · line 223
Quoted 4 lines
drivers/firmware/psci/psci.c +2
-0err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err);This change is unnecessary - probably comes from when -EPROBE_DEFER was being used.
psci_to_linux_errno()already does:But may print lots of noise like:
[ 0.008955] smp: Bringing up secondary CPUs ... [ 0.009661] psci: failed to boot CPU1 (-1) [ 0.010360] psci: failed to boot CPU2 (-1) [ 0.011164] psci: failed to boot CPU3 (-1) [ 0.011946] psci: failed to boot CPU4 (-1) [ 0.012764] psci: failed to boot CPU5 (-1) [ 0.013534] psci: failed to boot CPU6 (-1) [ 0.014349] psci: failed to boot CPU7 (-1) [ 0.014820] smp: Brought up 1 node, 1 CPU
Quoted 1 line
Is this expected?
Please read my email again, and take note of the _context_ above the places that I've commented. Context matters.
What I'm saying is that this change:
err = invoke_psci_fn(fn, cpuid, entry_point, 0);
+ if (err == PSCI_RET_DENIED)
+ return -EPERM;
return psci_to_linux_errno(err);
Is unnecessary when psci_to_linux_errno() already does:
static __always_inline int psci_to_linux_errno(int errno)
{
switch (errno) {
...
case PSCI_RET_DENIED:
return -EPERM;So, a return of PSCI_RET_DENIED from invoke_psci_fn() above will _already_ be translated to -EPERM (which is -1) by psci_to_linux_errno(). There is no need to add that extra if() statement in __psci_cpu_on().
I was _not_ saying that the entire patch was unnecessary.
Context matters. That's why we include context in replies.
Standard email etiquette (before Microsoft messed it up) is to quote the email that is being replied to, trimming hard irrelevant content, and to place the reply comments immediately below the original content to which the comments relate, to give the reply comments the context necessary for correct interpretation.
Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
RE: [PATCH 34/39] arm64: psci: Ignore DENIED CPUs
Quoted 116 lines
-----Original Message-----
From: Russell King <[email protected]>
Sent: 2023年11月20日 17:58
To: Jianyong Wu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Salil Mehta <[email protected]>; Jean-Philippe Brucker <[email protected]>; Justin He <[email protected]>; James Morse <[email protected]>; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Mark Rutland <[email protected]>; Lorenzo Pieralisi <[email protected]>
Subject: Re: [PATCH 34/39] arm64: psci: Ignore DENIED CPUsQuoted 24 lines · Jianyong Wu
Quoted 24 lines
-----Original Message-----
From: Russell King <[email protected]>
Sent: 2023年11月20日 17:25
To: Jianyong Wu <[email protected]>
Cc: [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; [email protected]; Salil Mehta <[email protected]>; Jean-Philippe Brucker <[email protected]>; Justin He <[email protected]>; James Morse <[email protected]>; Catalin Marinas <[email protected]>; Will Deacon <[email protected]>; Mark Rutland <[email protected]>; Lorenzo Pieralisi <[email protected]>
Subject: Re: [PATCH 34/39] arm64: psci: Ignore DENIED CPUsQuoted 3 lines · Jianyong Wu
Hi Russell,
One inline comment.
Quoted 8 lines
Quoted 8 lines
...
Quoted 3 lines
Quoted 3 lines
Changes since RFC v2
* Add specification reference
* Use EPERM rather thanEPROBE_DEFER...
Quoted 3 lines
Review static int cpu_psci_cpu_boot(unsigned int cpu) { · line 41
Quoted 3 lines
(quoted diff) +0
-0@@ -40,7 +40,7 @@ static int cpu_psci_cpu_boot(unsigned int cpu) { phys_addr_t pa_secondary_entry = __pa_symbol(secondary_entry); int err = psci_ops.cpu_on(cpu_logical_map(cpu),
pa_secondary_entry);Quoted 37 lines
Quoted 17 lines
Quoted 5 lines
arch/arm64/kernel/psci.c static int cpu_psci_cpu_boot(unsigned int cpu) · line 43
Quoted 2 lines
arch/arm64/kernel/psci.c +1
-1-if (err) +if (err && err != -EPROBE_DEFER )Should this be EPERM? As the following psci
cpu_onop will return it. I think you miss to change this when apply Jean-Philippe's patch.It looks like James didn't properly update all places. Also,
Quoted 9 lines
Quoted 9 lines
(quoted diff) +0
-0
b/drivers/firmware/psci/psci.cindexd9629ff87861..ee82e7880d8c100644drivers/firmware/psci/psci.c +0
-0@@ -218,6 +218,8 @@ static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point) int err;Quoted 7 lines
Quoted 4 lines
drivers/firmware/psci/psci.c static int __psci_cpu_on(u32 fn, unsigned long cpuid, unsigned long entry_point) · line 223
Quoted 4 lines
drivers/firmware/psci/psci.c +2
-0err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err);This change is unnecessary - probably comes from when -EPROBE_DEFER was being used.
psci_to_linux_errno()already does:But may print lots of noise like:
[ 0.008955] smp: Bringing up secondary CPUs ... [ 0.009661] psci: failed to boot CPU1 (-1) [ 0.010360] psci: failed to boot CPU2 (-1) [ 0.011164] psci: failed to boot CPU3 (-1) [ 0.011946] psci: failed to boot CPU4 (-1) [ 0.012764] psci: failed to boot CPU5 (-1) [ 0.013534] psci: failed to boot CPU6 (-1) [ 0.014349] psci: failed to boot CPU7 (-1) [ 0.014820] smp: Brought up 1 node, 1 CPUQuoted 1 line
Is this expected?
Please read my email again, and take note of the _context_ above the places that I've commented. Context matters.
What I'm saying is that this change:
drivers/firmware/psci/psci.c +2
-0err =invoke_psci_fn (fn, cpuid, entry_point,0 ); +if (err ==PSCI_RET_DENIED ) +return -EPERM ;return psci_to_linux_errno (err);Is unnecessary when
psci_to_linux_errno()already does:static __always_inline int psci_to_linux_errno(int errno) { switch (errno) { ... case PSCI_RET_DENIED: return -EPERM;So, a return of
PSCI_RET_DENIEDfrominvoke_psci_fn()above will _already_ be translated to-EPERM(which is -1) bypsci_to_linux_errno(). There is no need to add that extraif()statement in__psci_cpu_on().I was _not_ saying that the entire patch was unnecessary.
Quoted 8 lines
Context matters. That's why we include context in replies.
Standard email etiquette (before Microsoft messed it up) is to quote the email that is being replied to, trimming hard irrelevant content, and to place the reply comments immediately below the original content to which the comments relate, to give the reply comments the context necessary for correct interpretation.
Oh, sorry, my mistake. Ignore my last comment.
Thanks
Jianyong
Quoted 5 lines
Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
PATCH 35/39 ACPI: add support to register CPUs based on the _STA enabled bit
In reply to Russell King (Oracle)From: James Morse <[email protected]>
acpi_processor_get_info() registers all present CPUs. Registering a CPU is what creates the sysfs entries and triggers the udev notifications.
arm64 virtual machines that support 'virtual cpu hotplug' use the enabled bit to indicate whether the CPU can be brought online, as the existing ACPI tables require all hardware to be described and present.
If firmware describes a CPU as present, but disabled, skip the registration. Such CPUs are present, but can't be brought online for whatever reason. (e.g. firmware/hypervisor policy).
Once firmware sets the enabled bit, the CPU can be registered and brought online by user-space. Online CPUs, or CPUs that are missing an _STA method must always be registered.
--- drivers/acpi/acpi_processor.c | 31 ++++++++++++++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-)
drivers/acpi/acpi_processor.c +30 -1
@@ -228,6 +228,32 @@ static int acpi_processor_make_present(struct acpi_processor *pr)return ret; } +static int acpi_processor_make_enabled (struct acpi_processor *pr) +{ +unsigned long long sta; +acpi_status status; +bool present, enabled; + +if (!acpi_has_method (pr->handle,"_STA" )) +return arch_register_cpu (pr->id); + + status =acpi_evaluate_integer (pr->handle,"_STA" ,NULL , &sta); +if (ACPI_FAILURE (status)) +return -ENODEV ; + + present = sta &ACPI_STA_DEVICE_PRESENT ; + enabled = sta &ACPI_STA_DEVICE_ENABLED ; + +if (cpu_online (pr->id) && (!present || !enabled)) { +pr_err_once (FW_BUG "CPU %u is online, but described as not present or disabled!\n" , pr->id); +add_taint (TAINT_FIRMWARE_WORKAROUND ,LOCKDEP_STILL_OK ); + }else if (!present || !enabled) { +return -ENODEV ; + } + +return arch_register_cpu (pr->id); +} +static int acpi_processor_get_info (struct acpi_device *device) {union acpi_object object = {0 };
@@ -318,7 +344,7 @@ static int acpi_processor_get_info(struct acpi_device *device)*/ if (!invalid_logical_cpuid (pr->id) &&cpu_present (pr->id) && !get_cpu_device (pr->id)) { -int ret =arch_register_cpu (pr->id); +int ret =acpi_processor_make_enabled (pr);if (ret)return ret;
@@ -526,6 +552,9 @@ static void acpi_processor_post_eject(struct acpi_device *device)acpi_processor_make_not_present (device);return ; } + +if (cpu_present (pr->id) && !(sta &ACPI_STA_DEVICE_ENABLED )) +arch_unregister_cpu (pr->id); }#ifdef CONFIG_ARCH_MIGHT_HAVE_ACPI_PDC
Signature · 1 line
2.30.2
PATCH 36/39 arm64: document virtual CPU hotplug's expectations
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Add a description of physical and virtual CPU hotplug, explain the differences and elaborate on what is required in ACPI for a working virtual hotplug system.
--- Documentation/arch/arm64/cpu-hotplug.rst | 79 ++++++++++++++++++++++++ Documentation/arch/arm64/index.rst | 1 + 2 files changed, 80 insertions(+) create mode 100644 Documentation/arch/arm64/cpu-hotplug.rst
Documentation/arch/arm64/cpu-hotplug.rst new file +79 -0
@@ -0,0 +1,79 @@ +.. SPDX-License-Identifier: GPL-2.0 +.. _cpuhp_index: + +==================== +CPU Hotplug and ACPI +==================== + +CPU hotplug in the arm64 world is commonly used to describe the kernel taking +CPUs online/offline using PSCI. This document is about ACPI firmware allowing +CPUs that were not available during boot to be added to the system later. + +``possible`` and ``present`` refer to the state of the CPU as seen by linux. + + +CPU Hotplug on physical systems - CPUs not present at boot +---------------------------------------------------------- + +Physical systems need to mark a CPU that is ``possible`` but not ``present`` as +being ``present``. An example would be a dual socket machine, where the package +in one of the sockets can be replaced while the system is running. + +This is not supported. + +In the arm64 world CPUs are not a single device but a slice of the system. +There are no systems that support the physical addition (or removal) of CPUs +while the system is running, and ACPI is not able to sufficiently describe +them. + +e.g. New CPUs come with new caches, but the platform's cache toplogy is +described in a static table, the PPTT. How caches are shared between CPUs is +not discoverable, and must be described by firmware. + +e.g. The GIC redistributor for each CPU must be accessed by the driver during +boot to discover the system wide supported features. ACPI's MADT GICC +structures can describe a redistributor associated with a disabled CPU, but +can't describe whether the redistributor is accessible, only that it is not +'always on'. + +arm64's ACPI tables assume that everything described is ``present``. + + +CPU Hotplug on virtual systems - CPUs not enabled at boot +--------------------------------------------------------- + +Virtual systems have the advantage that all the properties the system will +ever have can be described at boot. There are no power-domain considerations +as such devices are emulated. + +CPU Hotplug on virtual systems is supported. It is distinct from physical +CPU Hotplug as all resources are described as ``present``, but CPUs may be +marked as disabled by firmware. Only the CPU's online/offline behaviour is +influenced by firmware. An example is where a virtual machine boots with a +single CPU, and additional CPUs are added once a cloud orchestrator deploys +the workload. + +For a virtual machine, the VMM (e.g. Qemu) plays the part of firmware. + +Virtual hotplug is implemented as a firmware policy affecting which CPUs can be +brought online. Firmware can enforce its policy via PSCI's return codes. e.g. +``DENIED``. + +The ACPI tables must describe all the resources of the virtual machine. CPUs +that firmware wishes to disable either from boot (or later) should not be +``enabled`` in the MADT GICC structures, but should have the ``online capable`` +bit set, to indicate they can be enabled later. The boot CPU must be marked as +``enabled``. The 'always on' GICR structure must be used to describe the +redistributors. + +CPUs described as ``online capable`` but not ``enabled`` can be set to enabled +by the DSDT's Processor object's _STA method. On virtual systems the _STA method +must always report the CPU as ``present``. Changes to the firmware policy can +be notified to the OS via device-check or eject-request. + +CPUs described as ``enabled`` in the static table, should not have their _STA +modified dynamically by firmware. Soft-restart features such as kexec will +re-read the static properties of the system from these static tables, and +may malfunction if these no longer describe the running system. Linux will +re-discover the dynamic properties of the system from the _STA method later +during boot.
Documentation/arch/arm64/index.rst +1 -0
@@ -13,6 +13,7 @@ ARM64 Architecture asymmetric-32bit booting cpu-feature-registers + cpu-hotplug elf_hwcaps hugetlbpage kdump
Signature · 1 line
2.30.2
PATCH 37/39 ACPI: Add _OSC bits to advertise OS support for toggling CPU present/enabled
In reply to Russell King (Oracle)From: James Morse <[email protected]>
Platform firmware can disabled a CPU, or make it not-present by making an eject-request notification, then waiting for the os to make it offline and call _EJx. After the firmware updates _STA with the new status.
Not all operating systems support this. For arm64 making CPUs not-present has never been supported. For all ACPI architectures, making CPUs disabled has recently been added. Firmware can't know what the OS has support for.
Add two new _OSC bits to advertise whether the OS supports the _STA enabled or present bits being toggled for CPUs. This will be important for arm64 if systems that support physical CPU hotplug ever appear as arm64 linux doesn't currently support this, so firmware shouldn't try.
Advertising this support to firmware is useful for cloud orchestrators to know whether they can scale a particular VM by adding CPUs.
---
I'm assuming ia64 with physical hotplug machines once existed, and that Loongarch machines with support for this don't.
--- arch/ia64/Kconfig | 1 + arch/x86/Kconfig | 1 + drivers/acpi/Kconfig | 9 +++++++++ drivers/acpi/acpi_processor.c | 14 +++++++++++++- drivers/acpi/bus.c | 16 ++++++++++++++++ include/linux/acpi.h | 4 ++++ 6 files changed, 44 insertions(+), 1 deletion(-)
arch/ia64/Kconfig +1 -0
@@ -17,6 +17,7 @@ config IA64 select ARCH_MIGHT_HAVE_PC_SERIO select ACPI select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU + select ACPI_HOTPLUG_IGNORE_OSC if ACPI select ACPI_NUMA if NUMA select ARCH_ENABLE_MEMORY_HOTPLUG select ARCH_ENABLE_MEMORY_HOTREMOVE
arch/x86/Kconfig +1 -0
@@ -61,6 +61,7 @@ config X86 select ACPI_LEGACY_TABLES_LOOKUP if ACPI select ACPI_SYSTEM_POWER_STATES_SUPPORT if ACPI select ACPI_HOTPLUG_PRESENT_CPU if ACPI_PROCESSOR && HOTPLUG_CPU + select ACPI_HOTPLUG_IGNORE_OSC if ACPI && HOTPLUG_CPU select ARCH_32BIT_OFF_T if X86_32 select ARCH_CLOCKSOURCE_INIT select ARCH_CORRECT_STACKTRACE_ON_KRETPROBE
drivers/acpi/Kconfig +9 -0
@@ -310,6 +310,15 @@ config ACPI_HOTPLUG_PRESENT_CPU depends on ACPI_PROCESSOR && HOTPLUG_CPU select ACPI_CONTAINER +config ACPI_HOTPLUG_IGNORE_OSC + bool + depends on ACPI_HOTPLUG_PRESENT_CPU + help + Ignore whether firmware acknowledged support for toggling the CPU + present bit in _STA. Some architectures predate the _OSC bits, so + firmware doesn't know to do this. + + config ACPI_PROCESSOR_AGGREGATOR tristate "Processor Aggregator" depends on ACPI_PROCESSOR
drivers/acpi/acpi_processor.c +13 -1
@@ -182,6 +182,18 @@ static void __init acpi_pcc_cpufreq_init(void)static void __init acpi_pcc_cpufreq_init (void ) {}#endif /* CONFIG_X86 */ +static bool acpi_processor_hotplug_present_supported (void ) +{ +if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) +return false; + +/* x86 systems pre-date the _OSC bit */ +if (IS_ENABLED (CONFIG_ACPI_HOTPLUG_IGNORE_OSC )) +return true; + +return osc_sb_hotplug_present_support_acked; +} +/* Initialization */ static int acpi_processor_make_present (struct acpi_processor *pr) {
@@ -189,7 +201,7 @@ static int acpi_processor_make_present(struct acpi_processor *pr)acpi_status status;int ret; -if (!IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) { +if (!acpi_processor_hotplug_present_supported ()) {pr_err_once ("Changing CPU present bit is not supported\n" );return -ENODEV ; }
drivers/acpi/bus.c +16 -0
@@ -298,6 +298,13 @@ EXPORT_SYMBOL_GPL(osc_sb_native_usb4_support_confirmed);bool osc_sb_cppc2_support_acked; +/* +* ACPI 6.? Proposed Operating System Capabilities for modifying CPU +* present/enable. +*/ +bool osc_sb_hotplug_enabled_support_acked; +bool osc_sb_hotplug_present_support_acked; +static u8 sb_uuid_str[] ="0811B06E-4A27-44F9-8D60-3CBBC22E7B48" ;static void acpi_bus_osc_negotiate_platform_control (void ) {
@@ -346,6 +353,11 @@ static void acpi_bus_osc_negotiate_platform_control(void)if (!ghes_disable) capbuf[OSC_SUPPORT_DWORD ] |=OSC_SB_APEI_SUPPORT ; + + capbuf[OSC_SUPPORT_DWORD ] |=OSC_SB_HOTPLUG_ENABLED_SUPPORT ; +if (IS_ENABLED (CONFIG_ACPI_HOTPLUG_PRESENT_CPU )) + capbuf[OSC_SUPPORT_DWORD ] |=OSC_SB_HOTPLUG_PRESENT_SUPPORT ; +if (ACPI_FAILURE (acpi_get_handle (NULL ,"\\_SB" , &handle)))return ;
@@ -383,6 +395,10 @@ static void acpi_bus_osc_negotiate_platform_control(void) capbuf_ret[OSC_SUPPORT_DWORD ] &OSC_SB_NATIVE_USB4_SUPPORT ; osc_cpc_flexible_adr_space_confirmed = capbuf_ret[OSC_SUPPORT_DWORD ] &OSC_SB_CPC_FLEXIBLE_ADR_SPACE ; + osc_sb_hotplug_enabled_support_acked = + capbuf_ret[OSC_SUPPORT_DWORD ] &OSC_SB_HOTPLUG_ENABLED_SUPPORT ; + osc_sb_hotplug_present_support_acked = + capbuf_ret[OSC_SUPPORT_DWORD ] &OSC_SB_HOTPLUG_PRESENT_SUPPORT ; }kfree (context.ret.pointer);
include/linux/acpi.h +4 -0
@@ -579,12 +579,16 @@ acpi_status acpi_run_osc(acpi_handle handle, struct acpi_osc_context *context);#define OSC_SB_NATIVE_USB4_SUPPORT 0x00040000#define OSC_SB_PRM_SUPPORT 0x00200000#define OSC_SB_FFH_OPR_SUPPORT 0x00400000 +#define OSC_SB_HOTPLUG_ENABLED_SUPPORT 0x00800000 +#define OSC_SB_HOTPLUG_PRESENT_SUPPORT 0x01000000extern bool osc_sb_apei_support_acked;extern bool osc_pc_lpi_support_confirmed;extern bool osc_sb_native_usb4_support_confirmed;extern bool osc_sb_cppc2_support_acked;extern bool osc_cpc_flexible_adr_space_confirmed; +extern bool osc_sb_hotplug_enabled_support_acked; +extern bool osc_sb_hotplug_present_support_acked;/* USB4 Capabilities */ #define OSC_USB_USB3_TUNNELING 0x00000001
Signature · 1 line
2.30.2
PATCH 38/39 cpumask: Add enabled cpumask for present CPUs that can be brought online
In reply to Russell King (Oracle)From: James Morse <[email protected]>
The 'offline' file in sysfs shows all offline CPUs, including those that aren't present. User-space is expected to remove not-present CPUs from this list to learn which CPUs could be brought online.
CPUs can be present but not-enabled. These CPUs can't be brought online until the firmware policy changes, which comes with an ACPI notification that will register the CPUs.
With only the offline and present files, user-space is unable to determine which CPUs it can try to bring online. Add a new CPU mask that shows this based on all the registered CPUs.
--- drivers/base/cpu.c | 10 ++++++++++ include/linux/cpumask.h | 25 +++++++++++++++++++++++++ kernel/cpu.c | 3 +++ 3 files changed, 38 insertions(+)
drivers/base/cpu.c +10 -0
@@ -95,6 +95,7 @@ void unregister_cpu(struct cpu *cpu) {int logical_cpu = cpu->dev.id; +set_cpu_enabled (logical_cpu, false);unregister_cpu_under_node (logical_cpu,cpu_to_node (logical_cpu));device_unregister (&cpu->dev);
@@ -273,6 +274,13 @@ static ssize_t print_cpus_offline(struct device *dev, } static DEVICE_ATTR(offline, 0444, print_cpus_offline, NULL); +static ssize_t print_cpus_enabled (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sysfs_emit (buf,"%*pbl\n" ,cpumask_pr_args (cpu_enabled_mask)); +} +static DEVICE_ATTR(enabled, 0444, print_cpus_enabled, NULL); +static ssize_t print_cpus_isolated (struct device *dev,struct device_attribute *attr,char *buf) {
@@ -413,6 +421,7 @@ int register_cpu(struct cpu *cpu, int num)register_cpu_under_node (num,cpu_to_node (num));dev_pm_qos_expose_latency_limit (&cpu->dev,PM_QOS_RESUME_LATENCY_NO_CONSTRAINT ); +set_cpu_enabled (num, true);return 0 ; }
@@ -494,6 +503,7 @@ static struct attribute *cpu_root_attrs[] = { &cpu_attrs[2 ].attr.attr, &dev_attr_kernel_max.attr, &dev_attr_offline.attr, + &dev_attr_enabled.attr, &dev_attr_isolated.attr, #ifdef CONFIG_NO_HZ_FULL &dev_attr_nohz_full.attr,
include/linux/cpumask.h +25 -0
@@ -92,6 +92,7 @@ static inline void set_nr_cpu_ids(unsigned int nr)* * cpu_possible_mask- has bit 'cpu' set iff cpu is populatable * cpu_present_mask - has bit 'cpu' set iff cpu is populated +* cpu_enabled_mask - has bit 'cpu' set iff cpu can be brought online * cpu_online_mask - has bit 'cpu' set iff cpu available to scheduler * cpu_active_mask - has bit 'cpu' set iff cpu available to migration *
@@ -124,11 +125,13 @@ static inline void set_nr_cpu_ids(unsigned int nr)extern struct cpumask __cpu_possible_mask;extern struct cpumask __cpu_online_mask; +extern struct cpumask __cpu_enabled_mask;extern struct cpumask __cpu_present_mask;extern struct cpumask __cpu_active_mask;extern struct cpumask __cpu_dying_mask;#define cpu_possible_mask ((const struct cpumask *)&__cpu_possible_mask)#define cpu_online_mask ((const struct cpumask *)&__cpu_online_mask) +#define cpu_enabled_mask ((const struct cpumask *)&__cpu_enabled_mask)#define cpu_present_mask ((const struct cpumask *)&__cpu_present_mask)#define cpu_active_mask ((const struct cpumask *)&__cpu_active_mask)#define cpu_dying_mask ((const struct cpumask *)&__cpu_dying_mask)
@@ -973,6 +976,7 @@ extern const DECLARE_BITMAP(cpu_all_bits, NR_CPUS);#else #define for_each_possible_cpu (cpu) for_each_cpu((cpu), cpu_possible_mask)#define for_each_online_cpu (cpu) for_each_cpu((cpu), cpu_online_mask) +#define for_each_enabled_cpu (cpu) for_each_cpu((cpu), cpu_enabled_mask)#define for_each_present_cpu (cpu) for_each_cpu((cpu), cpu_present_mask)#endif
@@ -995,6 +999,15 @@ set_cpu_possible(unsigned int cpu, bool possible)cpumask_clear_cpu (cpu, &__cpu_possible_mask); } +static inline void +set_cpu_enabled (unsigned int cpu,bool can_be_onlined) +{ +if (can_be_onlined) +cpumask_set_cpu (cpu, &__cpu_enabled_mask); +else +cpumask_clear_cpu (cpu, &__cpu_enabled_mask); +} +static inline void set_cpu_present (unsigned int cpu,bool present) {
@@ -1074,6 +1087,7 @@ static __always_inline unsigned int num_online_cpus(void)return raw_atomic_read (&__num_online_cpus); }#define num_possible_cpus () cpumask_weight(cpu_possible_mask) +#define num_enabled_cpus () cpumask_weight(cpu_enabled_mask)#define num_present_cpus () cpumask_weight(cpu_present_mask)#define num_active_cpus () cpumask_weight(cpu_active_mask)
@@ -1082,6 +1096,11 @@ static inline bool cpu_online(unsigned int cpu)return cpumask_test_cpu (cpu, cpu_online_mask); } +static inline bool cpu_enabled (unsigned int cpu) +{ +return cpumask_test_cpu (cpu, cpu_enabled_mask); +} +static inline bool cpu_possible (unsigned int cpu) {return cpumask_test_cpu (cpu, cpu_possible_mask);
@@ -1106,6 +1125,7 @@ static inline bool cpu_dying(unsigned int cpu)#define num_online_cpus () 1U#define num_possible_cpus () 1U +#define num_enabled_cpus () 1U#define num_present_cpus () 1U#define num_active_cpus () 1U
@@ -1119,6 +1139,11 @@ static inline bool cpu_possible(unsigned int cpu)return cpu ==0 ; } +static inline bool cpu_enabled (unsigned int cpu) +{ +return cpu ==0 ; +} +static inline bool cpu_present (unsigned int cpu) {return cpu ==0 ;
kernel/cpu.c +3 -0
@@ -3101,6 +3101,9 @@ EXPORT_SYMBOL(__cpu_possible_mask);struct cpumask __cpu_online_mask__read_mostly ;EXPORT_SYMBOL (__cpu_online_mask); +struct cpumask __cpu_enabled_mask__read_mostly ; +EXPORT_SYMBOL (__cpu_enabled_mask); +struct cpumask __cpu_present_mask__read_mostly ;EXPORT_SYMBOL (__cpu_present_mask);
Signature · 1 line
2.30.2
Re: [PATCH 38/39] cpumask: Add enabled cpumask for present CPUs that can be brought online
Quoted 13 lines · Russell King
From: James Morse <[email protected]>
The 'offline' file in sysfs shows all offline CPUs, including those that aren't present. User-space is expected to remove not-present CPUs from this list to learn which CPUs could be brought online.
CPUs can be present but not-enabled. These CPUs can't be brought online until the firmware policy changes, which comes with an ACPI notification that will register the CPUs.
With only the offline and present files, user-space is unable to determine which CPUs it can try to bring online. Add a new CPU mask that shows this based on all the registered CPUs.
...
drivers/base/cpu.c static ssize_t print_cpus_offline(struct device *dev, · line 282
Quoted 6 lines
drivers/base/cpu.c +6
-0+static ssize_t print_cpus_enabled (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sysfs_emit (buf,"%*pbl\n" ,cpumask_pr_args (cpu_enabled_mask)); +} +static DEVICE_ATTR(enabled, 0444, print_cpus_enabled, NULL);
Hmm... DEVICE_ATTR_RO() ?
Signature · 2 lines
With Best Regards, Andy Shevchenko
Greg Kroah-Hartman
maintainerRe: [PATCH 38/39] cpumask: Add enabled cpumask for present CPUs that can be brought online
In reply to Russell King (Oracle)Quoted 34 lines · Russell King
From: James Morse <[email protected]>
The 'offline' file in sysfs shows all offline CPUs, including those that aren't present. User-space is expected to remove not-present CPUs from this list to learn which CPUs could be brought online.
CPUs can be present but not-enabled. These CPUs can't be brought online until the firmware policy changes, which comes with an ACPI notification that will register the CPUs.
With only the offline and present files, user-space is unable to determine which CPUs it can try to bring online. Add a new CPU mask that shows this based on all the registered CPUs.
Signed-off-by James Morse <[email protected]>--- drivers/base/cpu.c | 10 ++++++++++ include/linux/cpumask.h | 25 +++++++++++++++++++++++++ kernel/cpu.c | 3 +++ 3 files changed, 38 insertions(+)drivers/base/cpu.c +1
-0@@ -95,6 +95,7 @@ void unregister_cpu(struct cpu *cpu) {int logical_cpu = cpu->dev.id; +set_cpu_enabled (logical_cpu, false);unregister_cpu_under_node (logical_cpu,cpu_to_node (logical_cpu));device_unregister (&cpu->dev);
drivers/base/cpu.c static ssize_t print_cpus_offline(struct device *dev, · line 282
Quoted 9 lines
drivers/base/cpu.c +6
-0@@ -273,6 +274,13 @@ static ssize_t print_cpus_offline(struct device *dev, } static DEVICE_ATTR(offline, 0444, print_cpus_offline, NULL); +static ssize_t print_cpus_enabled (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sysfs_emit (buf,"%*pbl\n" ,cpumask_pr_args (cpu_enabled_mask)); +} +static DEVICE_ATTR(enabled, 0444, print_cpus_enabled, NULL);
This needs to be documented somewhere in Documentation/ABI/ did I miss that patch?
thanks,
greg k-h
Re: [PATCH 38/39] cpumask: Add enabled cpumask for present CPUs that can be brought online
Quoted 38 lines · Greg Kroah-Hartman
Quoted 37 lines · Russell King
From: James Morse <[email protected]>
The 'offline' file in sysfs shows all offline CPUs, including those that aren't present. User-space is expected to remove not-present CPUs from this list to learn which CPUs could be brought online.
CPUs can be present but not-enabled. These CPUs can't be brought online until the firmware policy changes, which comes with an ACPI notification that will register the CPUs.
With only the offline and present files, user-space is unable to determine which CPUs it can try to bring online. Add a new CPU mask that shows this based on all the registered CPUs.
Signed-off-by James Morse <[email protected]>--- drivers/base/cpu.c | 10 ++++++++++ include/linux/cpumask.h | 25 +++++++++++++++++++++++++ kernel/cpu.c | 3 +++ 3 files changed, 38 insertions(+)drivers/base/cpu.c +1
-0@@ -95,6 +95,7 @@ void unregister_cpu(struct cpu *cpu) {int logical_cpu = cpu->dev.id; +set_cpu_enabled (logical_cpu, false);unregister_cpu_under_node (logical_cpu,cpu_to_node (logical_cpu));device_unregister (&cpu->dev);@@ -273,6 +274,13 @@ static ssize_t print_cpus_offline(struct device *dev, } static DEVICE_ATTR(offline, 0444, print_cpus_offline, NULL);
Quoted 9 lines
drivers/base/cpu.c static ssize_t print_cpus_offline(struct device *dev, · line 282
Quoted 6 lines
drivers/base/cpu.c +6
-0+static ssize_t print_cpus_enabled (struct device *dev, +struct device_attribute *attr,char *buf) +{ +return sysfs_emit (buf,"%*pbl\n" ,cpumask_pr_args (cpu_enabled_mask)); +} +static DEVICE_ATTR(enabled, 0444, print_cpus_enabled, NULL);This needs to be documented somewhere in
Documentation/ABI/did I miss that patch?
Thanks for pointing that out, no you missed the patch as nothing touches Documentation/ABI/ in this patch series. I'll add some blurb for it for the next iteration.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
PATCH 39/39 ACPI: processor: Only call arch_unregister_cpu() if HOTPLUG_CPU is selected
In reply to Russell King (Oracle)From: James Morse <[email protected]>
The kbuild robot points out that configurations without HOTPLUG_CPU selected can try to build acpi_processor_post_eject() without success as arch_unregister_cpu() is not defined.
Check this explicitly. This will be merged into:
| ACPI: Add post_eject to struct acpi_scan_handler for cpu hotplug for any subsequent posting.
--- drivers/acpi/acpi_processor.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/acpi/acpi_processor.c +1 -1
@@ -549,7 +549,7 @@ static void acpi_processor_post_eject(struct acpi_device *device)unsigned long long sta;acpi_status status; -if (!device) +if (!IS_ENABLED (CONFIG_HOTPLUG_CPU ) || !device)return ; pr =acpi_driver_data (device);
Signature · 1 line
2.30.2
Quoted 30 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()
Quoted 8 lines
3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
Update: Catalin has just merged this patch! Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Rafael J. Wysocki
maintainerQuoted 49 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call
arch_unregister_cpu()ifHOTPLUG_CPUis selected" which looks to me to be a workaround forarch_unregister_cpu()being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.
Quoted 8 lines
I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
I've gone through the series and there is at least one thing in it that concerns me a lot and some others that at least appear to be really questionable.
I need more time to send comments which I'm not going to do before the 6.7 merge window (sorry), but from what I can say right now, this is not looking good.
Thanks!
Quoted 60 lines · Rafael J. Wysocki
Quoted 49 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call
arch_unregister_cpu()ifHOTPLUG_CPUis selected" which looks to me to be a workaround forarch_unregister_cpu()being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.Quoted 8 lines
I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
Quoted 7 lines
I've gone through the series and there is at least one thing in it that concerns me a lot and some others that at least appear to be really questionable.
I need more time to send comments which I'm not going to do before the 6.7 merge window (sorry), but from what I can say right now, this is not looking good.
Thanks for having a look - there was the feeling that this was ready for merging based on the review comments from the previous series sent by James.
However, when I sent this series, I did notice that some mailing lists were missing, so I guess that's could be why you haven't commented before, and we find out now that there are major concerns.
My interest in it is because my employer wants to be able to hotplug CPUs in a virtual machine, and this saga with aarch64 has been running for years with different approaches ending up dead in the water. I hope your concerns do not result in this approach being entirely scrapped, and there can be some solution found.
I think James Morse will need to be involved in addressing your concerns since he has the detailed background about the history of this series. However, James seemed to fall totally silent after the last posting back in September, so whether that is possible is currently unknown.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Quoted 60 lines · Rafael J. Wysocki
Quoted 49 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call
arch_unregister_cpu()ifHOTPLUG_CPUis selected" which looks to me to be a workaround forarch_unregister_cpu()being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.Quoted 8 lines
I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
Quoted 7 lines
I've gone through the series and there is at least one thing in it that concerns me a lot and some others that at least appear to be really questionable.
I need more time to send comments which I'm not going to do before the 6.7 merge window (sorry), but from what I can say right now, this is not looking good.
Hi Rafael,
Will you be able to send your comments, so that we can find out what your other concerns are please? I'm getting questions from interested parties who want to know what your concerns are.
Nothing much has changed to the ACPI changes, so I think it's still valid to have the comments back for this.
Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Quoted 69 lines · Russell King (Oracle)
Quoted 60 lines · Rafael J. Wysocki
Quoted 49 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call
arch_unregister_cpu()ifHOTPLUG_CPUis selected" which looks to me to be a workaround forarch_unregister_cpu()being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.Quoted 8 lines
I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
Quoted 7 lines
I've gone through the series and there is at least one thing in it that concerns me a lot and some others that at least appear to be really questionable.
I need more time to send comments which I'm not going to do before the 6.7 merge window (sorry), but from what I can say right now, this is not looking good.
Quoted 8 lines
Hi Rafael,
Will you be able to send your comments, so that we can find out what your other concerns are please? I'm getting questions from interested parties who want to know what your concerns are.
Nothing much has changed to the ACPI changes, so I think it's still valid to have the comments back for this.
Hi Rafael,
Another gentle prod on this...
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Rafael J. Wysocki
maintainerQuoted 77 lines · Russell King (Oracle)
Quoted 75 lines · Russell King (Oracle)
Quoted 60 lines · Rafael J. Wysocki
Quoted 49 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call
arch_unregister_cpu()ifHOTPLUG_CPUis selected" which looks to me to be a workaround forarch_unregister_cpu()being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.Quoted 8 lines
I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
Quoted 7 lines
I've gone through the series and there is at least one thing in it that concerns me a lot and some others that at least appear to be really questionable.
I need more time to send comments which I'm not going to do before the 6.7 merge window (sorry), but from what I can say right now, this is not looking good.
Hi Rafael,
Will you be able to send your comments, so that we can find out what your other concerns are please? I'm getting questions from interested parties who want to know what your concerns are.
Quoted 6 lines
Quoted 2 lines
Nothing much has changed to the ACPI changes, so I think it's still valid to have the comments back for this.
Hi Rafael,
Another gentle prod on this...
There was a selection of the patches in the series sent separately and I believe that some of them have been applied already.
Can you please send the remaining patches again so it is clear what's still outstanding?
Quoted 24 lines · Rafael J. Wysocki
Quoted 22 lines · Russell King (Oracle)
Quoted 9 lines · Russell King (Oracle)
Quoted 7 lines · Rafael J. Wysocki
I've gone through the series and there is at least one thing in it that concerns me a lot and some others that at least appear to be really questionable.
I need more time to send comments which I'm not going to do before the 6.7 merge window (sorry), but from what I can say right now, this is not looking good.
Quoted 8 lines
Hi Rafael,
Will you be able to send your comments, so that we can find out what your other concerns are please? I'm getting questions from interested parties who want to know what your concerns are.
Nothing much has changed to the ACPI changes, so I think it's still valid to have the comments back for this.
Hi Rafael,
Quoted 7 lines
Quoted 1 line
Another gentle prod on this...
There was a selection of the patches in the series sent separately and I believe that some of them have been applied already.
Can you please send the remaining patches again so it is clear what's still outstanding?
I can do tomorrow, thanks. I will re-post as RFC again because it will depend on the cleanup part that was merged into Greg's tree - and thus the series isn't standalone (and I'll mention that in the cover message.)
We need to hear your concerns, which sounded quite damning for the series, so that if it's possible to save this a plan to address your concerns can be formulated.
Thanks.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
Quoted 117 lines · Russell King (Oracle)
Hi,
I'm posting James' patch set updated with most of the review comments from his RFC v2 series back in September. Individual patches have a changelog attached at the bottom of the commit message. Those which I have finished updating have my S-o-b on them, those which still have outstanding review comments from RFC v2 do not. In some of these cases I've asked questions and am waiting for responses.
I'm posting this as RFC v3 because there's still some unaddressed comments and it's clearly not ready for merging. Even if it was ready to be merged, it is too late in this development cycle to be taking this change in, so there would be little point posting it non-RFC. Also James stated that he's waiting for confirmation from the Kubernetes/Kata folk - I have no idea what the status is there.
I will be sending each patch individually to a wider audience appropriate for that patch - apologies to those missing out on this cover message. I have added more mailing lists to the series with the exception of the acpica list in a hope of this cover message also reaching those folk.
The changes that aren't included are:
1. Updates for my patch that was merged via Thomas (thanks!):
c4dd854f740ccpu-hotplug: Provide prototypes for arch CPU registration rather than having this change spread through James' patches.2. New patch - simplification of PA-RISC's
smp_prepare_boot_cpu()3. Moved "ACPI: Use the
acpi_device_is_present()helper in more places" and "ACPI: Renameacpi_scan_device_not_present()to be about enumeration" to the beginning of the series - these two patches are already queued up for merging into 6.7.4. Moved "arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a helper" to the beginning of the series, which has been submitted, but as yet the fate of that posting isn't known.
The first four patches in this series are provided for completness only.
There is an additional patch in James' git tree that isn't in the set of patches that James posted: "ACPI: processor: Only call
arch_unregister_cpu()ifHOTPLUG_CPUis selected" which looks to me to be a workaround forarch_unregister_cpu()being under the ifdef. I've commented on this on the RFC v2 posting making a suggestion, but as yet haven't had any response.I've included almost all of James' original covering body below the diffstat.
The reason that I'm doing this is to help move this code forward so hopefully it can be merged - which is why I have been keen to dig out from James' patches anything that can be merged and submit it separately, since this is a feature for which some users have a definite need for.
Please note that I haven't tested this beyond building for aarch64 at the present time.
The series can be found at:
git://git.armlinux.org.uk/~rmk/linux-arm.gitaarch64/hotplug-vcpu/v6.6-rc7Documentation/arch/arm64/cpu-hotplug.rst | 79 +++++++++++++++ Documentation/arch/arm64/index.rst | 1 + arch/arm64/Kconfig | 1 + arch/arm64/include/asm/acpi.h | 11 +++ arch/arm64/include/asm/cpu.h | 1 - arch/arm64/kernel/acpi_numa.c | 11 --- arch/arm64/kernel/psci.c | 2 +- arch/arm64/kernel/setup.c | 13 +-- arch/arm64/kernel/smp.c | 5 +- arch/ia64/Kconfig | 3 + arch/ia64/include/asm/acpi.h | 2 +- arch/ia64/include/asm/cpu.h | 6 -- arch/ia64/kernel/acpi.c | 6 +- arch/ia64/kernel/setup.c | 2 +- arch/ia64/kernel/topology.c | 35 +------ arch/loongarch/Kconfig | 2 + arch/loongarch/configs/loongson3_defconfig | 2 +- arch/loongarch/kernel/acpi.c | 4 +- arch/loongarch/kernel/topology.c | 38 +------- arch/parisc/kernel/smp.c | 8 +- arch/riscv/Kconfig | 1 + arch/riscv/kernel/setup.c | 19 +--- arch/x86/Kconfig | 3 + arch/x86/include/asm/cpu.h | 4 - arch/x86/kernel/acpi/boot.c | 4 +- arch/x86/kernel/cpu/intel_epb.c | 2 +- arch/x86/kernel/topology.c | 27 +----- drivers/acpi/Kconfig | 14 ++- drivers/acpi/acpi_processor.c | 151 +++++++++++++++++++++++------ drivers/acpi/bus.c | 16 +++ drivers/acpi/device_pm.c | 2 +- drivers/acpi/device_sysfs.c | 2 +- drivers/acpi/internal.h | 1 - drivers/acpi/processor_core.c | 2 +- drivers/acpi/property.c | 2 +- drivers/acpi/scan.c | 148 ++++++++++++++++++---------- drivers/base/arch_topology.c | 38 +++++--- drivers/base/cpu.c | 44 +++++++-- drivers/base/init.c | 2 +- drivers/base/node.c | 7 -- drivers/firmware/psci/psci.c | 2 + drivers/irqchip/irq-gic-v3.c | 38 +++++--- include/acpi/acpi_bus.h | 1 + include/acpi/actbl2.h | 1 + include/linux/acpi.h | 13 ++- include/linux/cpu.h | 4 + include/linux/cpumask.h | 25 +++++ kernel/cpu.c | 3 + 48 files changed, 516 insertions(+), 292 deletions(-)
Quoted 153 lines
Quoted 151 lines · James Morse
Hello!
Changes since RFC-v1:
* riscv is new, ia64 is gone
* The KVM support is different, and upstream - no need to patch the host.---
This series adds what looks like cpuhotplug support to arm64 for use in virtual machines. It does this by moving the
cpu_register()calls for architectures that support ACPI out of the arch code by usingGENERIC_CPU_DEVICES, then into the ACPI processor driver.The kubernetes folk really want to be able to add CPUs to an existing VM, in exactly the same way they do on x86. The use-case is pre-booting guests with one CPU, then adding the number that were actually needed when the workload is provisioned.
Wait? Doesn't arm64 support cpuhotplug already!?
In the arm world, cpuhotplug gets used to mean removing the power from a CPU. The CPU is offline, and remains present. For x86, and ACPI, cpuhotplug has the additional step of physically removing the CPU, so that it isn't present anymore.Arm64 doesn't support this, and can't support it: CPUs are really a slice of the SoC, and there is not enough information in the existing ACPI tables to describe which bits of the slice also got removed. Without a reference
machine: adding this support to the spec is a wild goose chase.Critically: everything described in the firmware tables must remain present.
For a virtual machine this is easy as all the other bits of 'virtual SoC' are emulated, so they can (and do) remain present when a vCPU is 'removed'.
On a system that supports cpuhotplug the MADT has to describe every possible CPU at boot. Under KVM, the vGIC needs to know about every possible vCPU before the guest is started.
With these constraints, virtual-cpuhotplug is really just a hypervisor/firmware policy about which CPUs can be brought online.This series adds support for virtual-cpuhotplug as exactly that: firmware policy. This may even work on a physical machine too; for a guest the part of firmware is played by the VMM. (typically Qemu).
PSCI support is modified to return 'DENIED' if the CPU can't be brought online/enabled yet. The CPU object's
_STAmethod's enabled bit is used to indicate firmware's current disposition. If the CPU has its enabled bit clear, it will not be registered with sysfs, and attempts to bring it online will fail. The notifications that_STAhas changed its value then work in the same way as physical hotplug, and firmware can cause the CPU to be registered some time later, allowing it to be brought online.This creates something that looks like cpuhotplug to user-space, as the sysfs files appear and disappear, and the udev notifications look the same.
One notable difference is the CPU present mask, which is exposed via sysfs. Because the CPUs remain present throughout, they can still be seen in that mask. This value does get used by webbrowsers to estimate the number of CPUs as the CPU online mask is constantly changed on mobile phones.
Linux is tolerant of PSCI returning errors, as its always been allowed to do that. To avoid confusing OS that can't tolerate this, we needed an additional bit in the MADT GICC flags. This series copies
ACPI_MADT_ONLINE_CAPABLE, which appears to be for this purpose, but calls itACPI_MADT_GICC_CPU_CAPABLEas it has a different bit position in the GICC.This code is unconditionally enabled for all ACPI architectures. If there are problems with firmware tables on some devices, the CPUs will already be online by the time the
acpi_processor_make_enabled()is called. A mismatch here causes a firmware-bug message and kernel taint. This should only affect people with broken firmware who also boot with maxcpus=1, and bring CPUs online later.I had a go at switching the remaining architectures over to
GENERIC_CPU_DEVICES, so that theKconfigsymbol can be removed, but I got stuck with powerpc and s390.I've only build tested Loongarch and riscv. I've removed the ia64 specific patches, but left the changes in other patches to make git-grep review of renames easier.
If folk want to play along at home, you'll need a copy of Qemu that supports this.
https://github.com/salil-mehta/qemu.git salil/virt-cpuhp-armv8/rfc-v2-rc6Replace your '-smp' argument with something like:
| -smp cpus=1,maxcpus=3,cores=3,threads=1,sockets=1then feed the following to the Qemu montior;
| (qemu)device_adddriver=host-arm-cpu,core-id=1,id=cpu1 | (qemu)device_delcpu1Why is this still an RFC? I'm still looking for confirmation from the kubernetes/kata folk that this works for them. Because of this I've culled the CC list...
This series is based on v6.6-rc1, and can be retrieved from:
https://git.kernel.org/pub/scm/linux/kernel/git/morse/linux.git/ virtual_cpu_hotplug/rfc/v2Thanks,
James Morse (34):
ACPI: MoveACPI_HOTPLUG_CPUto be disabled on arm64 and riscv
drivers: base: Use present CPUs inGENERIC_CPU_DEVICES
drivers: base: Allow parts ofGENERIC_CPU_DEVICESto be overridden
drivers: base: Movecpu_dev_init()afternode_dev_init()
drivers: base: Print a warning instead ofpanic()whenregister_cpu()
fails
arm64: setup: Switch over toGENERIC_CPU_DEVICESusing
arch_register_cpu()
x86:intel_epb: Don't rely on link order
x86/topology: Switch over toGENERIC_CPU_DEVICES
LoongArch: Switch over toGENERIC_CPU_DEVICES
riscv: Switch over toGENERIC_CPU_DEVICES
arch_topology: Makeregister_cpu_capacity_sysctl()tolerant to late
CPUs
ACPI: Use theacpi_device_is_present()helper in more places
ACPI: Renameacpi_scan_device_not_present()to be about enumeration
ACPI: Only enumerate enabled (or functional) devices
ACPI: processor: Add support for processors described as container
packages
ACPI: processor: Register CPUs that are online, but not described in
the DSDT
ACPI: processor: Register all CPUs fromacpi_processor_get_info()
ACPI: RenameACPI_HOTPLUG_CPUto include 'present'
ACPI: Moveacpi_bus_trim_one()beforeacpi_scan_hot_remove()
ACPI: Renameacpi_processor_hotadd_initand remove pre-processor
guards
ACPI: Addpost_ejectto structacpi_scan_handlerfor cpu hotplug
ACPI: Check_STApresent bit before making CPUs not present
ACPI: Warn when the present bit changes but the feature is not enabled
drivers: base: Implement weakarch_unregister_cpu()
LoongArch: Use the__weakversion ofarch_unregister_cpu()
arm64: acpi: Moveget_cpu_for_acpi_id()to a header
ACPICA: Add new MADT GICC flags fields [code first?]
arm64, irqchip/gic-v3, ACPI: Move MADT GICC enabled check into a
helper
irqchip/gic-v3: Don't return errors fromgic_acpi_match_gicc()irqchip/gic-v3: Add support for ACPI's disabled but 'online capable'
CPUs
ACPI: add support to register CPUs based on the_STAenabled bit
arm64: document virtual CPU hotplug's expectations
ACPI: Add_OSCbits to advertise OS support for toggling CPU
present/enabled
cpumask: Add enabled cpumask for present CPUs that can be brought
onlineJean-Philippe Brucker (1):
arm64: psci: Ignore DENIED CPUs
Tested on QEMU, based on Salil's RFC v2 [1], running with KVM.
- boot
- hotplug up to 'maxcpus'
- hotunplug down to the number of boot cpus
- hotplug vcpus and migrate with vcpus offline
- hotplug vcpus and migrate with vcpus online
- hotplug vcpus then unplug vcpus then migrate
- successive live migrations (up until 6)
Feel free to add:
Thank you
Miguel
[1] https://lore.kernel.org/qemu-devel/[email protected]/
Quoted 4 lines
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Quoted 11 lines · Miguel Luis
Hi Russell,
Tested on QEMU, based on Salil's RFC v2 [1], running with KVM.
- boot
- hotplug up to 'maxcpus'
- hotunplug down to the number of boot cpus
- hotplug vcpus and migrate with vcpus offline
- hotplug vcpus and migrate with vcpus online
- hotplug vcpus then unplug vcpus then migrate
- successive live migrations (up until 6)
Quoted 7 lines
Feel free to add:
Tested-by Miguel Luis <[email protected]>Thank you
Miguel
That's good news, thanks for testing!
I've pushed out an updated series against v6.6 earlier today in case anyone wants something specifically against v6.6, but I don't think there is any pressing reason to re-test. The only ACPI change between the two is:
9b311b7313d6 ACPI: NFIT: Install Notify() handler before getting NFIT table
and the only arm64 changes are in dts files. Nothing significant in kernel/ and nothing in drivers/base/.
So I think at this point, I will pause waiting for 6.7-rc1 (which I'll do an updated patch set, since it will include changes queued up) and hopefully followed by Rafael's comments. Maybe James will also have some time to work on this again.
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!