Hi James,
From: James Morse [mailto:james.morse@arm.com] Sent: Wednesday, August 31, 2022 12:09 PM To: linaro-open-discussions@op-lists.linaro.org Cc: Salil Mehta salil.mehta@huawei.com; james.morse@arm.com; lorenzo.pieralisi@linaro.org; Jean-Philippe Brucker jean-philippe@linaro.org Subject: [RFC PATCH v0.1 22/25] ACPI: add support to register CPUs based on the _STA enabled bit
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 james.morse@arm.com
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 1bd6e4b8ab66..42521d89c378 100644 --- a/drivers/acpi/acpi_processor.c +++ b/drivers/acpi/acpi_processor.c @@ -194,6 +194,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 ordisabled!\n", pr->id);
add_taint(TAINT_FIRMWARE_WORKAROUND, LOCKDEP_STILL_OK);- } else if (!present || !enabled) {
return -ENODEV;- }
This change and setting all possible cpus as *present* in smp_prepare_cpus() will always cause all present == possible in the guest kernel. I think we can avoid that by the trick which Jean-Phillipe exploited in his patch-set[1] sent earlier last year.
Please check Bit[2] in the ACPI specification: Spec Version: "Advanced Configuration and Power Interface (ACPI) Specification, Release 6.5, Section 6.3.7 _STA (Device Status)"
* Bit [0] - Set if the device is present. * Bit [1] - Set if the device is enabled and decoding its resources. * Bit [2] - Set if the device should be shown in the UI
File: linux/include/acpi/actypes.h /* Flags for _STA method */ #define ACPI_STA_DEVICE_PRESENT 0x01 #define ACPI_STA_DEVICE_ENABLED 0x02 #define ACPI_STA_DEVICE_UI 0x04---> (This bit)
_STA[0] & _STA[1] bits ensures that devices is present and enabled at the firmware level and OS can now decode its resources but if for some reason this device does not have to be made visible in the OS then _STA[2] bit could be *unset*. We can leverage this flag.
I guess we need a _STA[2]/UI bit check here before, [1] Registering the cpus and [2] Setting the present bit in __cpu_present_mask using set_cpu_present().
Both of above can co-exist in arch_{un}register_cpu().
This shall ensure that we correctly reflect only present vcpus to the linux kernel although the sizing and initialization of the GICC/GICR would have already happened for the complete set for possible vcpus i.e. the ones with
[1] _STA[0] is set & _STA[1] bit is set and [2] Either GICC_flag_Intf_Flag.Enabled set OR GICC_flag.online_capable set
so effectively we are only deferring populating the cpu present mask for the disabled cpus but which are now online capable(or Hotplug capable in future?)
Question: Q1: Current acpi_processor.c code is not using ACPI_STA_DEVICE_{ENABLED, UI} bits. Could it break other architecture if we use these bits but some of their legacy devices or firmware does not initialize these bits to their defaults?
I am trying to test this change. If you don't have any apprehensions then we can include this change (both at the kernel and qemu) as well. Thoughts?
Reference: [1] acpi/cpuhp: Allow hiding disabled CPUs for Arm https://jpbrucker.net/git/qemu/commit/?h=cpuhp/devel&id=5f12c581328bb2a8...
- return arch_register_cpu(pr->id);
+}
static int acpi_processor_get_info(struct acpi_device *device) { union acpi_object object = { 0 }; @@ -271,7 +297,7 @@ static int acpi_processor_get_info(struct acpi_device *device) }
if (!invalid_logical_cpuid(pr->id) && cpu_present(pr->id)) {
int ret = arch_register_cpu(pr->id);
int ret = acpi_processor_make_enabled(pr);if (ret) return ret;
@@ -479,6 +505,9 @@ static void acpi_processor_remove(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_X86
2.30.2