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(-)
On Wed, Sep 13, 2023 at 04:37:48PM +0000, James Morse wrote:
> 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 using
> GENERIC_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 _STA method'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 _STA has 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 it ACPI_MADT_GICC_CPU_CAPABLE as 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 the Kconfig symbol 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-rc6
>
> Replace your '-smp' argument with something like:
> | -smp cpus=1,maxcpus=3,cores=3,threads=1,sockets=1
>
> then feed the following to the Qemu montior;
> | (qemu) device_add driver=host-arm-cpu,core-id=1,id=cpu1
> | (qemu) device_del cpu1
>
>
> Why 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/v2
>
>
> Thanks,
>
> James Morse (34):
> ACPI: Move ACPI_HOTPLUG_CPU to be disabled on arm64 and riscv
> drivers: base: Use present CPUs in GENERIC_CPU_DEVICES
> drivers: base: Allow parts of GENERIC_CPU_DEVICES to be overridden
> drivers: base: Move cpu_dev_init() after node_dev_init()
> drivers: base: Print a warning instead of panic() when register_cpu()
> fails
> arm64: setup: Switch over to GENERIC_CPU_DEVICES using
> arch_register_cpu()
> x86: intel_epb: Don't rely on link order
> x86/topology: Switch over to GENERIC_CPU_DEVICES
> LoongArch: Switch over to GENERIC_CPU_DEVICES
> riscv: Switch over to GENERIC_CPU_DEVICES
> arch_topology: Make register_cpu_capacity_sysctl() tolerant to late
> CPUs
> ACPI: Use the acpi_device_is_present() helper in more places
> ACPI: Rename acpi_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 from acpi_processor_get_info()
> ACPI: Rename ACPI_HOTPLUG_CPU to include 'present'
> ACPI: Move acpi_bus_trim_one() before acpi_scan_hot_remove()
> ACPI: Rename acpi_processor_hotadd_init and remove pre-processor
> guards
> ACPI: Add post_eject to struct acpi_scan_handler for cpu hotplug
> ACPI: Check _STA present bit before making CPUs not present
> ACPI: Warn when the present bit changes but the feature is not enabled
> drivers: base: Implement weak arch_unregister_cpu()
> LoongArch: Use the __weak version of arch_unregister_cpu()
> arm64: acpi: Move get_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 from gic_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 _STA enabled bit
> arm64: document virtual CPU hotplug's expectations
> ACPI: Add _OSC bits to advertise OS support for toggling CPU
> present/enabled
> cpumask: Add enabled cpumask for present CPUs that can be brought
> online
>
> Jean-Philippe Brucker (1):
> arm64: psci: Ignore DENIED CPUs
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
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().
Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/parisc/kernel/smp.c | 8 +-------
1 file changed, 1 insertion(+), 7 deletions(-)
diff --git a/arch/parisc/kernel/smp.c b/arch/parisc/kernel/smp.c
index 2019c1f04bd0..444154271f23 100644
--- a/arch/parisc/kernel/smp.c
+++ b/arch/parisc/kernel/smp.c
@@ -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");
}
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Reviewed-by: Miguel Luis <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/acpi/scan.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 691d4b7686ee..ed01e19514ef 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -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;
}
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/acpi/scan.c | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index ed01e19514ef..17ab875a7d4e 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -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)
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 960b98b43506..8c8f55721786 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -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;
}
diff --git a/drivers/acpi/processor_core.c b/drivers/acpi/processor_core.c
index 7dd6dbaa98c3..b203cfe28550 100644
--- a/drivers/acpi/processor_core.c
+++ b/drivers/acpi/processor_core.c
@@ -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
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index f59ac9586b7b..d50d9414f471 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -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 *hea
int 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) ?
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index afd94c9b8b8a..ebfea7bf663d 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/base/cpu.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 9ea22e165acd..34b48f660b6b 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -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");
}
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 34b48f660b6b..579064fda97b 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -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
diff --git a/include/linux/cpu.h b/include/linux/cpu.h
index eb768a866fe3..e117c06e0c6b 100644
--- a/include/linux/cpu.h
+++ b/include/linux/cpu.h
@@ -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
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/drivers/base/init.c b/drivers/base/init.c
index 397eb9880cec..c4954835128c 100644
--- a/drivers/base/init.c
+++ b/drivers/base/init.c
@@ -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();
}
--
2.30.2
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.
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/base/node.c | 7 -------
1 file changed, 7 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 493d533f8375..4d5ac7cf8757 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/base/cpu.c | 7 ++++---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 579064fda97b..d31c936f0955 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -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);
}
}
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Russell King (Oracle) <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig
index 78f20e632712..4042cd6fc6b2 100644
--- a/arch/arm64/Kconfig
+++ b/arch/arm64/Kconfig
@@ -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
diff --git a/arch/arm64/include/asm/cpu.h b/arch/arm64/include/asm/cpu.h
index e749838b9c5d..887bd0d992bb 100644
--- a/arch/arm64/include/asm/cpu.h
+++ b/arch/arm64/include/asm/cpu.h
@@ -38,7 +38,6 @@ struct cpuinfo_32bit {
};
struct cpuinfo_arm64 {
- struct cpu cpu;
struct kobject kobj;
u64 reg_ctr;
u64 reg_cntfrq;
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 417a8a86b2db..165bd2c0dd5a 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -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)
{
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index a3bfd42467ab..06692e1c7c62 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -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
diff --git a/arch/ia64/include/asm/cpu.h b/arch/ia64/include/asm/cpu.h
index 642d71675ddb..3b36c6a382bb 100644
--- a/arch/ia64/include/asm/cpu.h
+++ b/arch/ia64/include/asm/cpu.h
@@ -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_ */
diff --git a/arch/ia64/kernel/topology.c b/arch/ia64/kernel/topology.c
index 741863a187a6..8f5cafde2bc9 100644
--- a/arch/ia64/kernel/topology.c
+++ b/arch/ia64/kernel/topology.c
@@ -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
*/
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 53faa122b0f4..a3bfd42467ab 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -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
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index e14396a2ddcb..2bddd202470e 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -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
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 66bfabae8814..18729edc879d 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index cee82b473dc5..8456d48ba702 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -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"
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 0f5218e361df..4fe2ef54088c 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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;
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
----
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(-)
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 18729edc879d..a11c0aea5176 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
index 25050d953eee..91867a6a9f8e 100644
--- a/arch/x86/include/asm/cpu.h
+++ b/arch/x86/include/asm/cpu.h
@@ -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
diff --git a/arch/x86/kernel/topology.c b/arch/x86/kernel/topology.c
index 0bab03130033..027897054424 100644
--- a/arch/x86/kernel/topology.c
+++ b/arch/x86/kernel/topology.c
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 2bddd202470e..5bed51adc68c 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -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
diff --git a/arch/loongarch/kernel/topology.c b/arch/loongarch/kernel/topology.c
index 3fd166006698..bf58351beac1 100644
--- a/arch/loongarch/kernel/topology.c
+++ b/arch/loongarch/kernel/topology.c
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/x86/kernel/cpu/intel_epb.c b/arch/x86/kernel/cpu/intel_epb.c
index e4c3ba91321c..f18d35fe27a9 100644
--- a/arch/x86/kernel/cpu/intel_epb.c
+++ b/arch/x86/kernel/cpu/intel_epb.c
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
drivers/acpi/acpi_processor.c | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 4fe2ef54088c..6a542e0ce396 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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;
}
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/riscv/Kconfig b/arch/riscv/Kconfig
index d607ab0f7c6d..eeb80fb55acc 100644
--- a/arch/riscv/Kconfig
+++ b/arch/riscv/Kconfig
@@ -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
diff --git a/arch/riscv/kernel/setup.c b/arch/riscv/kernel/setup.c
index aac853ae4eb7..311e99741cf8 100644
--- a/arch/riscv/kernel/setup.c
+++ b/arch/riscv/kernel/setup.c
@@ -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)
{
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
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(-)
diff --git a/drivers/base/arch_topology.c b/drivers/base/arch_topology.c
index b741b5ba82bd..9ccb7daee78e 100644
--- a/drivers/base/arch_topology.c
+++ b/drivers/base/arch_topology.c
@@ -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;
}
--
2.30.2
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.
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
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(-)
diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
index f007116a8427..76c38478a502 100644
--- a/drivers/acpi/device_pm.c
+++ b/drivers/acpi/device_pm.c
@@ -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;
}
diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
index b9bbf0746199..16e586d74aa2 100644
--- a/drivers/acpi/device_sysfs.c
+++ b/drivers/acpi/device_sysfs.c
@@ -141,7 +141,7 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modalia
struct 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;
/*
diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
index 866c7c4ed233..a1b45e345bcc 100644
--- a/drivers/acpi/internal.h
+++ b/drivers/acpi/internal.h
@@ -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);
diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
index 413e4fcadcaf..e03f00b98701 100644
--- a/drivers/acpi/property.c
+++ b/drivers/acpi/property.c
@@ -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 *
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 17ab875a7d4e..06e9bb4a633f 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -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
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 0511f2bc10bc..e7ed4730cbbe 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index d31c936f0955..6c70a004c198 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -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) {
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/acpi/acpi_processor.c | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 6a542e0ce396..0511f2bc10bc 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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.
--
2.30.2
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.
CC: Jean-Philippe Brucker <[email protected]>
Signed-off-by: James Morse <[email protected]>
---
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(-)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 6c70a004c198..2b9cb2667654 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -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)
{
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/acpi/scan.c | 76 ++++++++++++++++++++++-----------------------
1 file changed, 38 insertions(+), 38 deletions(-)
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 06e9bb4a633f..4343888c76d5 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -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;
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
drivers/acpi/acpi_processor.c | 14 +++++---------
include/linux/acpi.h | 2 --
2 files changed, 5 insertions(+), 11 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index c8e960ff0aca..26e3efb74614 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 48ee36086577..3672312e15eb 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 06692e1c7c62..3b30305407ac 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -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
diff --git a/arch/ia64/include/asm/acpi.h b/arch/ia64/include/asm/acpi.h
index 58500a964238..482ea994d1e1 100644
--- a/arch/ia64/include/asm/acpi.h
+++ b/arch/ia64/include/asm/acpi.h
@@ -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
diff --git a/arch/ia64/kernel/acpi.c b/arch/ia64/kernel/acpi.c
index 41e8fe55cd98..381ce50fa54c 100644
--- a/arch/ia64/kernel/acpi.c
+++ b/arch/ia64/kernel/acpi.c
@@ -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,
diff --git a/arch/ia64/kernel/setup.c b/arch/ia64/kernel/setup.c
index 5a55ac82c13a..44591716d07b 100644
--- a/arch/ia64/kernel/setup.c
+++ b/arch/ia64/kernel/setup.c
@@ -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) ?
diff --git a/arch/loongarch/Kconfig b/arch/loongarch/Kconfig
index 5bed51adc68c..754c22214c4c 100644
--- a/arch/loongarch/Kconfig
+++ b/arch/loongarch/Kconfig
@@ -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
diff --git a/arch/loongarch/configs/loongson3_defconfig b/arch/loongarch/configs/loongson3_defconfig
index a3b52aaa83b3..ef3bc76313e4 100644
--- a/arch/loongarch/configs/loongson3_defconfig
+++ b/arch/loongarch/configs/loongson3_defconfig
@@ -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
diff --git a/arch/loongarch/kernel/acpi.c b/arch/loongarch/kernel/acpi.c
index 8e00a754e548..dfa56119b56f 100644
--- a/arch/loongarch/kernel/acpi.c
+++ b/arch/loongarch/kernel/acpi.c
@@ -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 */
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index a11c0aea5176..2a859f597a94 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 2a0ea38955df..84dd4133754b 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -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)
{
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 8456d48ba702..417f9f3077d2 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -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).
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index e7ed4730cbbe..c8e960ff0aca 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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 = {
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ebfea7bf663d..48ee36086577 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
drivers/acpi/acpi_processor.c | 31 +++++++++++++++++++++++++------
1 file changed, 25 insertions(+), 6 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index b6f5005985c3..19fceb3ec4e2 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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,
},
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Joanthan Cameron <[email protected]>
---
drivers/acpi/acpi_processor.c | 4 +--
drivers/acpi/scan.c | 52 ++++++++++++++++++++++++++++++-----
include/acpi/acpi_bus.h | 1 +
3 files changed, 48 insertions(+), 9 deletions(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 26e3efb74614..b6f5005985c3 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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,
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 4343888c76d5..adfd24f4d00c 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -244,18 +244,28 @@ 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)
+/**
+ * acpi_bus_trim_one() - Detach scan handlers and drivers from ACPI device
+ * objects.
+ * @adev: Root of the ACPI namespace scope to walk.
+ * @eject: Pointer to a bool that indicates if this was due to an
+ * eject-request.
+ *
+ * Must be called under acpi_scan_lock.
+ * If @eject points to true, clearing the device enumeration is deferred until
+ * acpi_bus_post_eject() is called.
+ */
+static int acpi_bus_trim_one(struct acpi_device *adev, void *eject)
{
struct acpi_scan_handler *handler = adev->handler;
+ bool is_eject = *(bool *)eject;
- acpi_dev_for_each_child_reverse(adev, acpi_bus_trim_one, NULL);
+ acpi_dev_for_each_child_reverse(adev, acpi_bus_trim_one, eject);
adev->flags.match_driver = false;
if (handler) {
if (handler->detach)
handler->detach(adev);
-
- adev->handler = NULL;
} else {
device_release_driver(&adev->dev);
}
@@ -265,7 +275,12 @@ static int acpi_bus_trim_one(struct acpi_device *adev, void *not_used)
*/
acpi_device_set_power(adev, ACPI_STATE_D3_COLD);
adev->flags.initialized = false;
- acpi_device_clear_enumerated(adev);
+
+ /* For eject this is deferred to acpi_bus_post_eject() */
+ if (!is_eject) {
+ adev->handler = NULL;
+ acpi_device_clear_enumerated(adev);
+ }
return 0;
}
@@ -278,15 +293,36 @@ static int acpi_bus_trim_one(struct acpi_device *adev, void *not_used)
*/
void acpi_bus_trim(struct acpi_device *adev)
{
- acpi_bus_trim_one(adev, NULL);
+ bool eject = false;
+
+ acpi_bus_trim_one(adev, &eject);
}
EXPORT_SYMBOL_GPL(acpi_bus_trim);
+static int acpi_bus_post_eject(struct acpi_device *adev, void *not_used)
+{
+ struct acpi_scan_handler *handler = adev->handler;
+
+ acpi_dev_for_each_child_reverse(adev, acpi_bus_post_eject, NULL);
+
+ if (handler) {
+ if (handler->post_eject)
+ handler->post_eject(adev);
+
+ adev->handler = NULL;
+ }
+
+ acpi_device_clear_enumerated(adev);
+
+ return 0;
+}
+
static int acpi_scan_hot_remove(struct acpi_device *device)
{
acpi_handle handle = device->handle;
unsigned long long sta;
acpi_status status;
+ bool eject = true;
if (device->handler && device->handler->hotplug.demand_offline) {
if (!acpi_scan_is_offline(device, true))
@@ -299,7 +335,7 @@ static int acpi_scan_hot_remove(struct acpi_device *device)
acpi_handle_debug(handle, "Ejecting\n");
- acpi_bus_trim(device);
+ acpi_bus_trim_one(device, &eject);
acpi_evaluate_lck(handle, 0);
/*
@@ -322,6 +358,8 @@ static int acpi_scan_hot_remove(struct acpi_device *device)
} else if (sta & ACPI_STA_DEVICE_ENABLED) {
acpi_handle_warn(handle,
"Eject incomplete - status 0x%llx\n", sta);
+ } else {
+ acpi_bus_post_eject(device, NULL);
}
return 0;
diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
index 254685085c82..1b7e1acf925b 100644
--- a/include/acpi/acpi_bus.h
+++ b/include/acpi/acpi_bus.h
@@ -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;
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
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(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 19fceb3ec4e2..b7a94c1348b0 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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)
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/loongarch/kernel/topology.c | 9 ---------
1 file changed, 9 deletions(-)
diff --git a/arch/loongarch/kernel/topology.c b/arch/loongarch/kernel/topology.c
index bf58351beac1..b595edbfdbcd 100644
--- a/arch/loongarch/kernel/topology.c
+++ b/arch/loongarch/kernel/topology.c
@@ -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
--
2.30.2
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().
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
arch/arm64/include/asm/acpi.h | 11 +++++++++++
arch/arm64/kernel/acpi_numa.c | 11 -----------
2 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/arch/arm64/include/asm/acpi.h b/arch/arm64/include/asm/acpi.h
index 6792a1f83f2a..bc9a6656fc0c 100644
--- a/arch/arm64/include/asm/acpi.h
+++ b/arch/arm64/include/asm/acpi.h
@@ -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);
diff --git a/arch/arm64/kernel/acpi_numa.c b/arch/arm64/kernel/acpi_numa.c
index e51535a5f939..0c036a9a3c33 100644
--- a/arch/arm64/kernel/acpi_numa.c
+++ b/arch/arm64/kernel/acpi_numa.c
@@ -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)
{
--
2.30.2
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.
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: James Morse <[email protected]>
---
drivers/acpi/acpi_processor.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 539412ff59a1..5bb207a7a1dd 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Reviewed-by: Jonathan Cameron <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/acpi/acpi_processor.c | 31 ++++++++++++++++++++++++++++++-
1 file changed, 30 insertions(+), 1 deletion(-)
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index b7a94c1348b0..5dabb426481f 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
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(-)
diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 3b30305407ac..a7267d8a4d3d 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -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
diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 2a859f597a94..026f358b7b28 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -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
diff --git a/drivers/acpi/Kconfig b/drivers/acpi/Kconfig
index 417f9f3077d2..c49978b4b11f 100644
--- a/drivers/acpi/Kconfig
+++ b/drivers/acpi/Kconfig
@@ -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
diff --git a/drivers/acpi/acpi_processor.c b/drivers/acpi/acpi_processor.c
index 5dabb426481f..539412ff59a1 100644
--- a/drivers/acpi/acpi_processor.c
+++ b/drivers/acpi/acpi_processor.c
@@ -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;
}
diff --git a/drivers/acpi/bus.c b/drivers/acpi/bus.c
index a4aa53b7e2bb..b42f17bfbb09 100644
--- a/drivers/acpi/bus.c
+++ b/drivers/acpi/bus.c
@@ -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);
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index ed1ef5d8687f..53515ff1318f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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 0x01000000
extern 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
--
2.30.2
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(+)
diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
index 2b9cb2667654..f8bf1d4c7d71 100644
--- a/drivers/base/cpu.c
+++ b/drivers/base/cpu.c
@@ -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,
diff --git a/include/linux/cpumask.h b/include/linux/cpumask.h
index f10fb87d49db..a29ee03f13ff 100644
--- a/include/linux/cpumask.h
+++ b/include/linux/cpumask.h
@@ -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;
diff --git a/kernel/cpu.c b/kernel/cpu.c
index 6de7c6bb74ee..2201a6a449b5 100644
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -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);
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
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(+)
diff --git a/include/acpi/actbl2.h b/include/acpi/actbl2.h
index 3751ae69432f..c433a079d8e1 100644
--- a/include/acpi/actbl2.h
+++ b/include/acpi/actbl2.h
@@ -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) */
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
---
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
diff --git a/Documentation/arch/arm64/cpu-hotplug.rst b/Documentation/arch/arm64/cpu-hotplug.rst
new file mode 100644
index 000000000000..76ba8d932c72
--- /dev/null
+++ b/Documentation/arch/arm64/cpu-hotplug.rst
@@ -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.
diff --git a/Documentation/arch/arm64/index.rst b/Documentation/arch/arm64/index.rst
index d08e924204bf..78544de0a8a9 100644
--- a/Documentation/arch/arm64/index.rst
+++ b/Documentation/arch/arm64/index.rst
@@ -13,6 +13,7 @@ ARM64 Architecture
asymmetric-32bit
booting
cpu-feature-registers
+ cpu-hotplug
elf_hwcaps
hugetlbpage
kdump
--
2.30.2
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.
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 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(-)
diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c
index 29a8e444db83..4fcc0cdd757b 100644
--- a/arch/arm64/kernel/psci.c
+++ b/arch/arm64/kernel/psci.c
@@ -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;
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 8c8f55721786..68ec7fbe166f 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -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;
}
diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c
index d9629ff87861..ee82e7880d8c 100644
--- a/drivers/firmware/psci/psci.c
+++ b/drivers/firmware/psci/psci.c
@@ -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
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!
Signed-off-by: James Morse <[email protected]>
Reviewed-by: Gavin Shan <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
---
drivers/irqchip/irq-gic-v3.c | 18 ++++++------------
1 file changed, 6 insertions(+), 12 deletions(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index d50d9414f471..e787e7bbb5a2 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -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)
--
2.30.2
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.
Signed-off-by: James Morse <[email protected]>
Signed-off-by: Russell King (Oracle) <[email protected]>
----
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(-)
diff --git a/drivers/irqchip/irq-gic-v3.c b/drivers/irqchip/irq-gic-v3.c
index e787e7bbb5a2..8a89b853a5b9 100644
--- a/drivers/irqchip/irq-gic-v3.c
+++ b/drivers/irqchip/irq-gic-v3.c
@@ -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;
diff --git a/include/linux/acpi.h b/include/linux/acpi.h
index 3672312e15eb..ed1ef5d8687f 100644
--- a/include/linux/acpi.h
+++ b/include/linux/acpi.h
@@ -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 */
--
2.30.2
On Tue, Oct 24, 2023 at 04:19:24PM +0100, Russell King wrote:
> 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.
...
> +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() ?
--
With Best Regards,
Andy Shevchenko
On Tue, Oct 24, 2023 at 04:19:24PM +0100, Russell King wrote:
> 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(+)
>
> diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> index 2b9cb2667654..f8bf1d4c7d71 100644
> --- a/drivers/base/cpu.c
> +++ b/drivers/base/cpu.c
> @@ -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);
This needs to be documented somewhere in Documentation/ABI/ did I miss
that patch?
thanks,
greg k-h
On Tue, Oct 24, 2023 at 06:02:30PM +0200, Greg Kroah-Hartman wrote:
> On Tue, Oct 24, 2023 at 04:19:24PM +0100, Russell King wrote:
> > 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(+)
> >
> > diff --git a/drivers/base/cpu.c b/drivers/base/cpu.c
> > index 2b9cb2667654..f8bf1d4c7d71 100644
> > --- a/drivers/base/cpu.c
> > +++ b/drivers/base/cpu.c
> > @@ -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);
>
> 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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Oct 24, 2023 at 04:15:28PM +0100, Russell King (Oracle) wrote:
> 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.
Update: Catalin has just merged this patch! Thanks.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Oct 24, 2023 at 5:17 PM Russell King <[email protected]> wrote:
>
> 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.
>
> 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.
> arch opt-in: ACPI_IGNORE_STA_ENABLED, that causes
> acpi_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.
>
> 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(-)
>
> diff --git a/drivers/acpi/device_pm.c b/drivers/acpi/device_pm.c
> index f007116a8427..76c38478a502 100644
> --- a/drivers/acpi/device_pm.c
> +++ b/drivers/acpi/device_pm.c
> @@ -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;
> }
> diff --git a/drivers/acpi/device_sysfs.c b/drivers/acpi/device_sysfs.c
> index b9bbf0746199..16e586d74aa2 100644
> --- a/drivers/acpi/device_sysfs.c
> +++ b/drivers/acpi/device_sysfs.c
> @@ -141,7 +141,7 @@ static int create_pnp_modalias(const struct acpi_device *acpi_dev, char *modalia
> struct 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;
>
> /*
> diff --git a/drivers/acpi/internal.h b/drivers/acpi/internal.h
> index 866c7c4ed233..a1b45e345bcc 100644
> --- a/drivers/acpi/internal.h
> +++ b/drivers/acpi/internal.h
> @@ -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);
> diff --git a/drivers/acpi/property.c b/drivers/acpi/property.c
> index 413e4fcadcaf..e03f00b98701 100644
> --- a/drivers/acpi/property.c
> +++ b/drivers/acpi/property.c
> @@ -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 *
> diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
> index 17ab875a7d4e..06e9bb4a633f 100644
> --- a/drivers/acpi/scan.c
> +++ b/drivers/acpi/scan.c
> @@ -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
>
On Tue, Oct 24, 2023 at 5:15 PM Russell King (Oracle)
<[email protected]> wrote:
>
> 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.
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!
On Tue, Oct 24, 2023 at 08:26:58PM +0200, Rafael J. Wysocki wrote:
> On Tue, Oct 24, 2023 at 5:15 PM Russell King (Oracle)
> <[email protected]> wrote:
> >
> > 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.
>
> 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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
> On 24 Oct 2023, at 15:15, Russell King (Oracle) <[email protected]> wrote:
>
> 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(-)
>
>
> On Wed, Sep 13, 2023 at 04:37:48PM +0000, James Morse wrote:
>> 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 using
>> GENERIC_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 _STA method'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 _STA has 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 it ACPI_MADT_GICC_CPU_CAPABLE as 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 the Kconfig symbol 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-rc6
>>
>> Replace your '-smp' argument with something like:
>> | -smp cpus=1,maxcpus=3,cores=3,threads=1,sockets=1
>>
>> then feed the following to the Qemu montior;
>> | (qemu) device_add driver=host-arm-cpu,core-id=1,id=cpu1
>> | (qemu) device_del cpu1
>>
>>
>> Why 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/v2
>>
>>
>> Thanks,
>>
>> James Morse (34):
>> ACPI: Move ACPI_HOTPLUG_CPU to be disabled on arm64 and riscv
>> drivers: base: Use present CPUs in GENERIC_CPU_DEVICES
>> drivers: base: Allow parts of GENERIC_CPU_DEVICES to be overridden
>> drivers: base: Move cpu_dev_init() after node_dev_init()
>> drivers: base: Print a warning instead of panic() when register_cpu()
>> fails
>> arm64: setup: Switch over to GENERIC_CPU_DEVICES using
>> arch_register_cpu()
>> x86: intel_epb: Don't rely on link order
>> x86/topology: Switch over to GENERIC_CPU_DEVICES
>> LoongArch: Switch over to GENERIC_CPU_DEVICES
>> riscv: Switch over to GENERIC_CPU_DEVICES
>> arch_topology: Make register_cpu_capacity_sysctl() tolerant to late
>> CPUs
>> ACPI: Use the acpi_device_is_present() helper in more places
>> ACPI: Rename acpi_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 from acpi_processor_get_info()
>> ACPI: Rename ACPI_HOTPLUG_CPU to include 'present'
>> ACPI: Move acpi_bus_trim_one() before acpi_scan_hot_remove()
>> ACPI: Rename acpi_processor_hotadd_init and remove pre-processor
>> guards
>> ACPI: Add post_eject to struct acpi_scan_handler for cpu hotplug
>> ACPI: Check _STA present bit before making CPUs not present
>> ACPI: Warn when the present bit changes but the feature is not enabled
>> drivers: base: Implement weak arch_unregister_cpu()
>> LoongArch: Use the __weak version of arch_unregister_cpu()
>> arm64: acpi: Move get_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 from gic_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 _STA enabled bit
>> arm64: document virtual CPU hotplug's expectations
>> ACPI: Add _OSC bits to advertise OS support for toggling CPU
>> present/enabled
>> cpumask: Add enabled cpumask for present CPUs that can be brought
>> online
>>
>> Jean-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:
Tested-by: Miguel Luis <[email protected]>
Thank you
Miguel
[1] https://lore.kernel.org/qemu-devel/[email protected]/
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
>
On Mon, Oct 30, 2023 at 04:41:19PM +0000, Miguel Luis wrote:
> 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)
>
> Feel free to add:
> Tested-by: Miguel Luis <[email protected]>
>
> Thank you
> Miguel
>
> [1] https://lore.kernel.org/qemu-devel/[email protected]/
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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
Hi Russell,
One inline comment.
> -----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 CPUs
>
> 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.
>
> 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 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(-)
>
> diff --git a/arch/arm64/kernel/psci.c b/arch/arm64/kernel/psci.c index
> 29a8e444db83..4fcc0cdd757b 100644
> --- a/arch/arm64/kernel/psci.c
> +++ b/arch/arm64/kernel/psci.c
> @@ -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
> pr_err("failed to boot CPU%d (%d)\n", cpu, err);
>
> return err;
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c index
> 8c8f55721786..68ec7fbe166f 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -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;
> }
>
> diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index
> d9629ff87861..ee82e7880d8c 100644
> --- a/drivers/firmware/psci/psci.c
> +++ b/drivers/firmware/psci/psci.c
> @@ -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
On Thu, Nov 16, 2023 at 07:45:51AM +0000, Jianyong Wu wrote:
> Hi Russell,
>
> One inline comment.
...
> > Changes since RFC v2
> > * Add specification reference
> > * Use EPERM rather than EPROBE_DEFER
...
> > @@ -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.
It looks like James didn't properly update all places. Also,
> > diff --git a/drivers/firmware/psci/psci.c b/drivers/firmware/psci/psci.c index
> > d9629ff87861..ee82e7880d8c 100644
> > --- a/drivers/firmware/psci/psci.c
> > +++ b/drivers/firmware/psci/psci.c
> > @@ -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);
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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> -----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 CPUs
>
> On Thu, Nov 16, 2023 at 07:45:51AM +0000, Jianyong Wu wrote:
> > Hi Russell,
> >
> > One inline comment.
> ...
> > > Changes since RFC v2
> > > * Add specification reference
> > > * Use EPERM rather than EPROBE_DEFER
> ...
> > > @@ -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.
>
> It looks like James didn't properly update all places. Also,
>
> > > diff --git a/drivers/firmware/psci/psci.c
> > > b/drivers/firmware/psci/psci.c index d9629ff87861..ee82e7880d8c
> > > 100644
> > > --- a/drivers/firmware/psci/psci.c
> > > +++ b/drivers/firmware/psci/psci.c
> > > @@ -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);
>
> 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
>
> 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!
On Mon, Nov 20, 2023 at 09:36:05AM +0000, Jianyong Wu wrote:
>
>
> > -----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 CPUs
> >
> > On Thu, Nov 16, 2023 at 07:45:51AM +0000, Jianyong Wu wrote:
> > > Hi Russell,
> > >
> > > One inline comment.
> > ...
> > > > Changes since RFC v2
> > > > * Add specification reference
> > > > * Use EPERM rather than EPROBE_DEFER
> > ...
> > > > @@ -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.
> >
> > It looks like James didn't properly update all places. Also,
> >
> > > > diff --git a/drivers/firmware/psci/psci.c
> > > > b/drivers/firmware/psci/psci.c index d9629ff87861..ee82e7880d8c
> > > > 100644
> > > > --- a/drivers/firmware/psci/psci.c
> > > > +++ b/drivers/firmware/psci/psci.c
> > > > @@ -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);
> >
> > 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?
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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
> -----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 CPUs
>
> On Mon, Nov 20, 2023 at 09:36:05AM +0000, Jianyong Wu wrote:
> >
> >
> > > -----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 CPUs
> > >
> > > On Thu, Nov 16, 2023 at 07:45:51AM +0000, Jianyong Wu wrote:
> > > > Hi Russell,
> > > >
> > > > One inline comment.
> > > ...
> > > > > Changes since RFC v2
> > > > > * Add specification reference
> > > > > * Use EPERM rather than EPROBE_DEFER
> > > ...
> > > > > @@ -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.
> > >
> > > It looks like James didn't properly update all places. Also,
> > >
> > > > > diff --git a/drivers/firmware/psci/psci.c
> > > > > b/drivers/firmware/psci/psci.c index d9629ff87861..ee82e7880d8c
> > > > > 100644
> > > > > --- a/drivers/firmware/psci/psci.c
> > > > > +++ b/drivers/firmware/psci/psci.c
> > > > > @@ -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);
> > >
> > > 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?
>
> 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.
>
Oh, sorry, my mistake. Ignore my last comment.
Thanks
Jianyong
> Thanks.
>
> --
> RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
> FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Oct 24, 2023 at 08:26:58PM +0200, Rafael J. Wysocki wrote:
> On Tue, Oct 24, 2023 at 5:15 PM Russell King (Oracle)
> <[email protected]> wrote:
> >
> > 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.
>
> 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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Mon, Dec 04, 2023 at 06:23:02PM +0000, Russell King (Oracle) wrote:
> On Tue, Oct 24, 2023 at 08:26:58PM +0200, Rafael J. Wysocki wrote:
> > On Tue, Oct 24, 2023 at 5:15 PM Russell King (Oracle)
> > <[email protected]> wrote:
> > >
> > > 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.
> >
> > 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.
Hi Rafael,
Another gentle prod on this...
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
On Tue, Dec 12, 2023 at 8:58 PM Russell King (Oracle)
<[email protected]> wrote:
>
> On Mon, Dec 04, 2023 at 06:23:02PM +0000, Russell King (Oracle) wrote:
> > On Tue, Oct 24, 2023 at 08:26:58PM +0200, Rafael J. Wysocki wrote:
> > > On Tue, Oct 24, 2023 at 5:15 PM Russell King (Oracle)
> > > <[email protected]> wrote:
> > > >
> > > > 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.
> > >
> > > 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.
>
> 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?
On Tue, Dec 12, 2023 at 09:05:14PM +0100, Rafael J. Wysocki wrote:
> On Tue, Dec 12, 2023 at 8:58 PM Russell King (Oracle)
> <[email protected]> wrote:
> >
> > On Mon, Dec 04, 2023 at 06:23:02PM +0000, Russell King (Oracle) wrote:
> > > On Tue, Oct 24, 2023 at 08:26:58PM +0200, Rafael J. Wysocki wrote:
> > > > 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.
> >
> > 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?
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.
--
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!