Hi all,
Do we have any topic for next open discussion(Feb 28th)?
Thanks:) Joyce
在 2023年2月24日,上午8:00,linaro-open-discussions-request@op-lists.linaro.org 写道:
Send Linaro-open-discussions mailing list submissions to linaro-open-discussions@op-lists.linaro.org
To subscribe or unsubscribe via email, send a message with subject or body 'help' to linaro-open-discussions-request@op-lists.linaro.org
You can reach the person managing the list at linaro-open-discussions-owner@op-lists.linaro.org
When replying, please edit your Subject line so it is more specific than "Re: Contents of Linaro-open-discussions digest..."
Today's Topics:
- Re: [RFC PATCH v0.1 22/25] ACPI: add support to register CPUs based on the _STA enabled bit (Vishnu Pajjuri)
Message: 1 Date: Thu, 23 Feb 2023 11:40:43 +0530 From: Vishnu Pajjuri vishnu@amperemail.onmicrosoft.com Subject: [Linaro-open-discussions] Re: [RFC PATCH v0.1 22/25] ACPI: add support to register CPUs based on the _STA enabled bit To: James Morse james.morse@arm.com, linaro-open-discussions@op-lists.linaro.org Message-ID: 6010811b-6c26-ad52-2a15-f5a55972c108@amperemail.onmicrosoft.com Content-Type: text/plain; charset=UTF-8; format=flowed
Hi James/Salil,
On 31-08-2022 16:39, James Morse wrote:
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(a)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 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 }; @@ -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
Enumeration failure while trying to hot plug vcpus after guest reboot
Following sequence of steps causes the issue
- Boot VM with max 80 vcpus and current 72 vcpus(For example)
- hot plug 3 more vcpus # virsh setvcpus vm2 75 --live
- hot unplug 3 vcpus ( which were added earlier ) # virsh setvcpus vm2 72 --live
- reboot VM # reboot
- Again try to hot plug 3 vcpus -- ERROR # virsh setvcpus vm2 75 --live [ 47.367823] acpi ACPI0007:48: Enumeration failure [ 47.565173] acpi ACPI0007:49: Enumeration failure [ 47.703423] acpi ACPI0007:4a: Enumeration failure
I have tested the above scenario with following git repos
https://github.com/salil-mehta/qemu/tree/virt-cpuhp-armv8/rfc-v1-port2909202...
https://github.com/salil-mehta/linux/tree/virt-cpuhp-arm64/rfc-v2/jmorse-var... https://gitlab.arm.com/linux-arm/linux-jm/-/tree/virtual_cpu_hotplug/rfc/v/0...
I observed the following sequence while doing hotplug and unplug
When we do hotplug, making vcpu as *present*(_STA.Pres=1) and *enabled*(_STA.Enabled=1) When we do hot unplug, we just *disabled*(_STA.Enabled=0) vcpu but not *removed*(_STA.Pres=1)
In such a case CPU shall still be present at the firmware/VMM level and could be kept disabled i.e. _STA.Enable=0 and _STA.Present=1
After reboot, system is trying to enumerate vcpu(as *present*(_STA.Pres=1)) and it is marking flags flags.initialized = true and flags.visited = true
as the vcpu *disabled*(_STA.Enabled=0), vcpu has been made not present and flags flags.initialized and flags.visited were not cleared
And following fix resolves the issue
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 558664d169fc..e93b933cb8a4 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2158,6 +2158,9 @@ static int acpi_scan_attach_handler(struct acpi_device *device) break;
device->handler = NULL;
device->flags.initialized = false;
acpi_device_clear_enumerated(device);
device->flags.power_manageable = 0; if (ret < 0) break; }
_Regards_, -Vishnu.
Subject: Digest Footer
Linaro-open-discussions mailing list -- linaro-open-discussions@op-lists.linaro.org To unsubscribe send an email to linaro-open-discussions-leave@op-lists.linaro.org
End of Linaro-open-discussions Digest, Vol 29, Issue 6
Hi All
If no special topics, I will cancel today’s meeting:)
Thanks:) Joyce
在 2023年2月24日,下午9:03,Joyce Qi joyce.qi@linaro.org 写道:
Hi all,
Do we have any topic for next open discussion(Feb 28th)?
Thanks:) Joyce
在 2023年2月24日,上午8:00,linaro-open-discussions-request@op-lists.linaro.org 写道:
Send Linaro-open-discussions mailing list submissions to linaro-open-discussions@op-lists.linaro.org
To subscribe or unsubscribe via email, send a message with subject or body 'help' to linaro-open-discussions-request@op-lists.linaro.org
You can reach the person managing the list at linaro-open-discussions-owner@op-lists.linaro.org
When replying, please edit your Subject line so it is more specific than "Re: Contents of Linaro-open-discussions digest..."
Today's Topics:
- Re: [RFC PATCH v0.1 22/25] ACPI: add support to register CPUs based on the _STA enabled bit (Vishnu Pajjuri)
Message: 1 Date: Thu, 23 Feb 2023 11:40:43 +0530 From: Vishnu Pajjuri vishnu@amperemail.onmicrosoft.com Subject: [Linaro-open-discussions] Re: [RFC PATCH v0.1 22/25] ACPI: add support to register CPUs based on the _STA enabled bit To: James Morse james.morse@arm.com, linaro-open-discussions@op-lists.linaro.org Message-ID: 6010811b-6c26-ad52-2a15-f5a55972c108@amperemail.onmicrosoft.com Content-Type: text/plain; charset=UTF-8; format=flowed
Hi James/Salil,
On 31-08-2022 16:39, James Morse wrote:
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(a)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 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 }; @@ -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
Enumeration failure while trying to hot plug vcpus after guest reboot
Following sequence of steps causes the issue
- Boot VM with max 80 vcpus and current 72 vcpus(For example)
- hot plug 3 more vcpus # virsh setvcpus vm2 75 --live
- hot unplug 3 vcpus ( which were added earlier ) # virsh setvcpus vm2 72 --live
- reboot VM # reboot
- Again try to hot plug 3 vcpus -- ERROR # virsh setvcpus vm2 75 --live [ 47.367823] acpi ACPI0007:48: Enumeration failure [ 47.565173] acpi ACPI0007:49: Enumeration failure [ 47.703423] acpi ACPI0007:4a: Enumeration failure
I have tested the above scenario with following git repos
https://github.com/salil-mehta/qemu/tree/virt-cpuhp-armv8/rfc-v1-port2909202...
https://github.com/salil-mehta/linux/tree/virt-cpuhp-arm64/rfc-v2/jmorse-var... https://gitlab.arm.com/linux-arm/linux-jm/-/tree/virtual_cpu_hotplug/rfc/v/0...
I observed the following sequence while doing hotplug and unplug
When we do hotplug, making vcpu as *present*(_STA.Pres=1) and *enabled*(_STA.Enabled=1) When we do hot unplug, we just *disabled*(_STA.Enabled=0) vcpu but not *removed*(_STA.Pres=1)
In such a case CPU shall still be present at the firmware/VMM level and could be kept disabled i.e. _STA.Enable=0 and _STA.Present=1
After reboot, system is trying to enumerate vcpu(as *present*(_STA.Pres=1)) and it is marking flags flags.initialized = true and flags.visited = true
as the vcpu *disabled*(_STA.Enabled=0), vcpu has been made not present and flags flags.initialized and flags.visited were not cleared
And following fix resolves the issue
diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c index 558664d169fc..e93b933cb8a4 100644 --- a/drivers/acpi/scan.c +++ b/drivers/acpi/scan.c @@ -2158,6 +2158,9 @@ static int acpi_scan_attach_handler(struct acpi_device *device) break;
device->handler = NULL;
device->flags.initialized = false;
acpi_device_clear_enumerated(device);
device->flags.power_manageable = 0; if (ret < 0) break; }
_Regards_, -Vishnu.
Subject: Digest Footer
Linaro-open-discussions mailing list -- linaro-open-discussions@op-lists.linaro.org To unsubscribe send an email to linaro-open-discussions-leave@op-lists.linaro.org
End of Linaro-open-discussions Digest, Vol 29, Issue 6
linaro-open-discussions@op-lists.linaro.org