2019-06-29 02:45:25

by Xiongfeng Wang

[permalink] [raw]
Subject: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

This patchset mark all the GICC node in MADT as possible CPUs even though it
is disabled. But only those enabled GICC node are marked as present CPUs.
So that kernel will initialize some CPU related data structure in advance before
the CPU is actually hot added into the system. This patchset also implement
'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
needed to enable CPU hotplug.

To support CPU hotplug, we need to add all the possible GICC node in MADT
including those CPUs that are not present but may be hot added later. Those
CPUs are marked as disabled in GICC nodes.

Changelog:

v1 -> v2:
rebase the thrid patch to the lastest kernel

Xiongfeng Wang (3):
ACPI / scan: evaluate _STA for processors declared via ASL Device
statement
arm64: mark all the GICC nodes in MADT as possible cpu
arm64: Add CPU hotplug support

arch/arm64/kernel/acpi.c | 22 ++++++++++++++++++++++
arch/arm64/kernel/setup.c | 19 ++++++++++++++++++-
arch/arm64/kernel/smp.c | 11 +++++------
drivers/acpi/scan.c | 12 ++++++++++++
4 files changed, 57 insertions(+), 7 deletions(-)

--
1.7.12.4


2019-06-29 02:45:35

by Xiongfeng Wang

[permalink] [raw]
Subject: [RFC PATCH v2 1/3] ACPI / scan: evaluate _STA for processors declared via ASL Device statement

When we scan all the acpi namespace node in
acpi_scan_init()->acpi_bus_scan(), we evaluate '_STA' method for processor
type node to determine whether the device is present. But processors can
also be declared via ASL Device statement. ACPI 6.3 spec specifically
says that the Processor statement is deprecated and a Device statement
should be used for processors. In that case, acpi_object_type is
ACPI_TYPE_DEVICE rather than ACPI_TYPE_PROCESSOR.

Current code doesn't evaluate '_STA' for nodes with ACPI_TYPE_DEVICE, and
the device status is set to 'present' as default. This patch get the
device status from '_STA' method for processors declared via ASL Device
statement if it does have a '_STA' method.

Signed-off-by: Xiongfeng Wang <[email protected]>

---
I am not sure if I should set 'type' as ACPI_BUS_TYPE_PROCESSOR rather
than ACPI_BUS_TYPE_DEVICE for processors declared via ASL Device
statement.
---
drivers/acpi/scan.c | 12 ++++++++++++
1 file changed, 12 insertions(+)

diff --git a/drivers/acpi/scan.c b/drivers/acpi/scan.c
index 0e28270..cec43f6 100644
--- a/drivers/acpi/scan.c
+++ b/drivers/acpi/scan.c
@@ -16,6 +16,7 @@
#include <linux/dma-mapping.h>
#include <linux/platform_data/x86/apple.h>

+#include <acpi/processor.h>
#include <asm/pgtable.h>

#include "internal.h"
@@ -1687,6 +1688,7 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
{
acpi_status status;
acpi_object_type acpi_type;
+ struct acpi_device_info *info;

status = acpi_get_type(handle, &acpi_type);
if (ACPI_FAILURE(status))
@@ -1699,6 +1701,16 @@ static int acpi_bus_type_and_status(acpi_handle handle, int *type,
return -ENODEV;

*type = ACPI_BUS_TYPE_DEVICE;
+
+ status = acpi_get_object_info(handle, &info);
+ if (ACPI_SUCCESS(status) && info->valid & ACPI_VALID_HID &&
+ !strcmp(info->hardware_id.string,
+ ACPI_PROCESSOR_DEVICE_HID)) {
+ status = acpi_bus_get_status_handle(handle, sta);
+ if (ACPI_SUCCESS(status))
+ break;
+ }
+
/*
* acpi_add_single_object updates this once we've an acpi_device
* so that acpi_bus_get_status' quirk handling can be used.
--
1.7.12.4

2019-06-29 02:45:52

by Xiongfeng Wang

[permalink] [raw]
Subject: [RFC PATCH v2 3/3] arm64: Add CPU hotplug support

To support CPU hotplug, we need to implement 'acpi_(un)map_cpu()' and
'arch_(un)register_cpu()' for ARM64. These functions are called in
'acpi_processor_hotadd_init()/acpi_processor_remove()' when the CPU is hot
added into or hot removed from the system.

Signed-off-by: Xiongfeng Wang <[email protected]>
---
arch/arm64/kernel/acpi.c | 22 ++++++++++++++++++++++
arch/arm64/kernel/setup.c | 17 +++++++++++++++++
2 files changed, 39 insertions(+)

diff --git a/arch/arm64/kernel/acpi.c b/arch/arm64/kernel/acpi.c
index 2804330..57835fa 100644
--- a/arch/arm64/kernel/acpi.c
+++ b/arch/arm64/kernel/acpi.c
@@ -25,6 +25,7 @@
#include <linux/serial_core.h>

#include <acpi/ghes.h>
+#include <acpi/processor.h>
#include <asm/cputype.h>
#include <asm/cpu_ops.h>
#include <asm/daifflags.h>
@@ -284,3 +285,24 @@ int apei_claim_sea(struct pt_regs *regs)

return err;
}
+
+int acpi_map_cpu(acpi_handle handle, phys_cpuid_t physid, u32 acpi_id,
+ int *pcpu)
+{
+ int cpu;
+
+ cpu = acpi_map_cpuid(physid, acpi_id);
+ *pcpu = cpu;
+ set_cpu_present(cpu, true);
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_map_cpu);
+
+int acpi_unmap_cpu(int cpu)
+{
+ set_cpu_present(cpu, false);
+
+ return 0;
+}
+EXPORT_SYMBOL(acpi_unmap_cpu);
diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 7f4d12a..f2a881e 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -398,3 +398,20 @@ static int __init register_kernel_offset_dumper(void)
return 0;
}
__initcall(register_kernel_offset_dumper);
+
+int arch_register_cpu(int num)
+{
+ struct cpu *cpu = &per_cpu(cpu_data.cpu, num);
+
+ cpu->hotpluggable = 1;
+ return register_cpu(cpu, num);
+}
+EXPORT_SYMBOL(arch_register_cpu);
+
+void arch_unregister_cpu(int num)
+{
+ struct cpu *cpu = &per_cpu(cpu_data.cpu, num);
+
+ unregister_cpu(cpu);
+}
+EXPORT_SYMBOL(arch_unregister_cpu);
--
1.7.12.4

2019-06-29 02:46:38

by Xiongfeng Wang

[permalink] [raw]
Subject: [RFC PATCH v2 2/3] arm64: mark all the GICC nodes in MADT as possible cpu

We set 'cpu_possible_mask' based on the enabled GICC node in MADT. If
the GICC node is disabled, we will skip initializing the kernel data
structure for that CPU.

To support CPU hotplug, we need to initialize some CPU related data
structure in advance. This patch mark all the GICC nodes as possible CPU
and only these enabled GICC nodes as present CPU.

Signed-off-by: Xiongfeng Wang <[email protected]>
---
arch/arm64/kernel/setup.c | 2 +-
arch/arm64/kernel/smp.c | 11 +++++------
2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
index 7e541f9..7f4d12a 100644
--- a/arch/arm64/kernel/setup.c
+++ b/arch/arm64/kernel/setup.c
@@ -359,7 +359,7 @@ static int __init topology_init(void)
for_each_online_node(i)
register_one_node(i);

- for_each_possible_cpu(i) {
+ for_each_online_cpu(i) {
struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
cpu->hotpluggable = 1;
register_cpu(cpu, i);
diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
index 6dcf960..6d9983c 100644
--- a/arch/arm64/kernel/smp.c
+++ b/arch/arm64/kernel/smp.c
@@ -525,16 +525,14 @@ struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu)
{
u64 hwid = processor->arm_mpidr;

- if (!(processor->flags & ACPI_MADT_ENABLED)) {
- pr_debug("skipping disabled CPU entry with 0x%llx MPIDR\n", hwid);
- return;
- }
-
if (hwid & ~MPIDR_HWID_BITMASK || hwid == INVALID_HWID) {
pr_err("skipping CPU entry with invalid MPIDR 0x%llx\n", hwid);
return;
}

+ if (!(processor->flags & ACPI_MADT_ENABLED))
+ pr_debug("disabled CPU entry with 0x%llx MPIDR\n", hwid);
+
if (is_mpidr_duplicate(cpu_count, hwid)) {
pr_err("duplicate CPU MPIDR 0x%llx in MADT\n", hwid);
return;
@@ -755,7 +753,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
if (err)
continue;

- set_cpu_present(cpu, true);
+ if ((cpu_madt_gicc[cpu].flags & ACPI_MADT_ENABLED))
+ set_cpu_present(cpu, true);
numa_store_cpu_info(cpu);
}
}
--
1.7.12.4

2019-06-29 03:27:35

by Xiongfeng Wang

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64


My test process is as follows:

1. Use the following command to boot a Guest OS with 8 CPUs

qemu-system-aarch64 -machine virt -cpu cortex-a57 -machine type=virt \
-nographic -m 2048 -smp cpus=8,sockets=2,cores=4 \
-numa node,mem=1024,cpus=0-3 -numa node,mem=1024,cpus=4-7 \
-kernel $IMAGE_FILE -bios /home/wxf/QEMU_EFI.fd \
--initrd $INITRDFS_FILE \
--append "root=/dev/ram rdinit=/linuxrc earlycon=pl011,0x9000000 acpi.debug_layer=0xffffffff acpi.debug_level=0x2" \
--fsdev local,id=kmod_dev,path=$VIRFS,security_model=none \
-device virtio-9p-device,fsdev=kmod_dev,mount_tag=kmod_mount

2. Extract the ACPI table DSDT and APIC. Mark CPU6 as disabled in APIC.
Add '_STA' and '_EJ0' method for CPU6 in DSDT.

Modified tables are attached as attachment. I referred to the following two
documents to override the ACPI tables.
Documentation/admin-guide/acpi/initrd_table_override.rst
Documentation/admin-guide/acpi/dsdt-override.rst

3. Add a procfs interface to sent a event to a namespace node in DSDT.

It is used to emulate the SCI interrupt. It is based on https://lwn.net/Articles/84089/
The patch is attached.

4. Use the following commands to test hot-add and hot-remove

a. echo "C006 1" > /proc/acpi/sci/notify
b. echo 1 > /sys/devices/system/cpu/cpu6/online
c. use 'taskset' to run an application on CPU6
d. echo 0 > /sys/devices/system/cpu/cpu6/online
e. echo "C006 3" > /proc/acpi/sci/notify


Thanks,
Xiongfeng Wang

On 2019/6/29 10:42, Xiongfeng Wang wrote:
> This patchset mark all the GICC node in MADT as possible CPUs even though it
> is disabled. But only those enabled GICC node are marked as present CPUs.
> So that kernel will initialize some CPU related data structure in advance before
> the CPU is actually hot added into the system. This patchset also implement
> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
> needed to enable CPU hotplug.
>
> To support CPU hotplug, we need to add all the possible GICC node in MADT
> including those CPUs that are not present but may be hot added later. Those
> CPUs are marked as disabled in GICC nodes.
>
> Changelog:
>
> v1 -> v2:
> rebase the thrid patch to the lastest kernel
>
> Xiongfeng Wang (3):
> ACPI / scan: evaluate _STA for processors declared via ASL Device
> statement
> arm64: mark all the GICC nodes in MADT as possible cpu
> arm64: Add CPU hotplug support
>
> arch/arm64/kernel/acpi.c | 22 ++++++++++++++++++++++
> arch/arm64/kernel/setup.c | 19 ++++++++++++++++++-
> arch/arm64/kernel/smp.c | 11 +++++------
> drivers/acpi/scan.c | 12 ++++++++++++
> 4 files changed, 57 insertions(+), 7 deletions(-)
>


Attachments:
0001-SCI-interrupt-emulation.patch (5.95 kB)
APIC.dsl (14.91 kB)
DSDT.dsl (219.68 kB)
Download all attachments

2019-07-04 06:47:15

by Jia He

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/3] arm64: mark all the GICC nodes in MADT as possible cpu


On 2019/6/29 10:42, Xiongfeng Wang wrote:
> We set 'cpu_possible_mask' based on the enabled GICC node in MADT. If
> the GICC node is disabled, we will skip initializing the kernel data
> structure for that CPU.
>
> To support CPU hotplug, we need to initialize some CPU related data
> structure in advance. This patch mark all the GICC nodes as possible CPU
> and only these enabled GICC nodes as present CPU.
>
> Signed-off-by: Xiongfeng Wang <[email protected]>
> ---
> arch/arm64/kernel/setup.c | 2 +-
> arch/arm64/kernel/smp.c | 11 +++++------
> 2 files changed, 6 insertions(+), 7 deletions(-)
>
> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
> index 7e541f9..7f4d12a 100644
> --- a/arch/arm64/kernel/setup.c
> +++ b/arch/arm64/kernel/setup.c
> @@ -359,7 +359,7 @@ static int __init topology_init(void)
> for_each_online_node(i)
> register_one_node(i);
>
> - for_each_possible_cpu(i) {
> + for_each_online_cpu(i) {

Have you considered the case in non-acpi mode? and setting "maxcpus=n" in host
kernel boot

parameters?

---
Cheers,
Justin (Jia He)


> struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
> cpu->hotpluggable = 1;
> register_cpu(cpu, i);
> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
> index 6dcf960..6d9983c 100644
> --- a/arch/arm64/kernel/smp.c
> +++ b/arch/arm64/kernel/smp.c
> @@ -525,16 +525,14 @@ struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu)
> {
> u64 hwid = processor->arm_mpidr;
>
> - if (!(processor->flags & ACPI_MADT_ENABLED)) {
> - pr_debug("skipping disabled CPU entry with 0x%llx MPIDR\n", hwid);
> - return;
> - }
> -
> if (hwid & ~MPIDR_HWID_BITMASK || hwid == INVALID_HWID) {
> pr_err("skipping CPU entry with invalid MPIDR 0x%llx\n", hwid);
> return;
> }
>
> + if (!(processor->flags & ACPI_MADT_ENABLED))
> + pr_debug("disabled CPU entry with 0x%llx MPIDR\n", hwid);
> +
> if (is_mpidr_duplicate(cpu_count, hwid)) {
> pr_err("duplicate CPU MPIDR 0x%llx in MADT\n", hwid);
> return;
> @@ -755,7 +753,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
> if (err)
> continue;
>
> - set_cpu_present(cpu, true);
> + if ((cpu_madt_gicc[cpu].flags & ACPI_MADT_ENABLED))
> + set_cpu_present(cpu, true);
> numa_store_cpu_info(cpu);
> }
> }

--

2019-07-04 08:21:07

by Xiongfeng Wang

[permalink] [raw]
Subject: Re: [RFC PATCH v2 2/3] arm64: mark all the GICC nodes in MADT as possible cpu



On 2019/7/4 14:46, Jia He wrote:
>
> On 2019/6/29 10:42, Xiongfeng Wang wrote:
>> We set 'cpu_possible_mask' based on the enabled GICC node in MADT. If
>> the GICC node is disabled, we will skip initializing the kernel data
>> structure for that CPU.
>>
>> To support CPU hotplug, we need to initialize some CPU related data
>> structure in advance. This patch mark all the GICC nodes as possible CPU
>> and only these enabled GICC nodes as present CPU.
>>
>> Signed-off-by: Xiongfeng Wang <[email protected]>
>> ---
>>   arch/arm64/kernel/setup.c |  2 +-
>>   arch/arm64/kernel/smp.c   | 11 +++++------
>>   2 files changed, 6 insertions(+), 7 deletions(-)
>>
>> diff --git a/arch/arm64/kernel/setup.c b/arch/arm64/kernel/setup.c
>> index 7e541f9..7f4d12a 100644
>> --- a/arch/arm64/kernel/setup.c
>> +++ b/arch/arm64/kernel/setup.c
>> @@ -359,7 +359,7 @@ static int __init topology_init(void)
>>       for_each_online_node(i)
>>           register_one_node(i);
>>   -    for_each_possible_cpu(i) {
>> +    for_each_online_cpu(i) {
>
> Have you considered the case in non-acpi mode? and setting "maxcpus=n" in host kernel boot
>
> parameters?

Thanks for your mention. I haven't considered non-acpi mode. I should add ACPI check in
'smp_prepare_cpus()'.
'maxcpus' is check when we actually online the CPU, so I think it is not influenced.

Thanks,
Xiongfeng

>
> ---
> Cheers,
> Justin (Jia He)
>
>
>>           struct cpu *cpu = &per_cpu(cpu_data.cpu, i);
>>           cpu->hotpluggable = 1;
>>           register_cpu(cpu, i);
>> diff --git a/arch/arm64/kernel/smp.c b/arch/arm64/kernel/smp.c
>> index 6dcf960..6d9983c 100644
>> --- a/arch/arm64/kernel/smp.c
>> +++ b/arch/arm64/kernel/smp.c
>> @@ -525,16 +525,14 @@ struct acpi_madt_generic_interrupt *acpi_cpu_get_madt_gicc(int cpu)
>>   {
>>       u64 hwid = processor->arm_mpidr;
>>   -    if (!(processor->flags & ACPI_MADT_ENABLED)) {
>> -        pr_debug("skipping disabled CPU entry with 0x%llx MPIDR\n", hwid);
>> -        return;
>> -    }
>> -
>>       if (hwid & ~MPIDR_HWID_BITMASK || hwid == INVALID_HWID) {
>>           pr_err("skipping CPU entry with invalid MPIDR 0x%llx\n", hwid);
>>           return;
>>       }
>>   +    if (!(processor->flags & ACPI_MADT_ENABLED))
>> +        pr_debug("disabled CPU entry with 0x%llx MPIDR\n", hwid);
>> +
>>       if (is_mpidr_duplicate(cpu_count, hwid)) {
>>           pr_err("duplicate CPU MPIDR 0x%llx in MADT\n", hwid);
>>           return;
>> @@ -755,7 +753,8 @@ void __init smp_prepare_cpus(unsigned int max_cpus)
>>           if (err)
>>               continue;
>>   -        set_cpu_present(cpu, true);
>> +        if ((cpu_madt_gicc[cpu].flags & ACPI_MADT_ENABLED))
>> +            set_cpu_present(cpu, true);
>>           numa_store_cpu_info(cpu);
>>       }
>>   }
>

2019-07-05 10:13:00

by James Morse

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

Hi guys,

(CC: +kvmarm list)

On 29/06/2019 03:42, Xiongfeng Wang wrote:
> This patchset mark all the GICC node in MADT as possible CPUs even though it
> is disabled. But only those enabled GICC node are marked as present CPUs.
> So that kernel will initialize some CPU related data structure in advance before
> the CPU is actually hot added into the system. This patchset also implement
> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
> needed to enable CPU hotplug.
>
> To support CPU hotplug, we need to add all the possible GICC node in MADT
> including those CPUs that are not present but may be hot added later. Those
> CPUs are marked as disabled in GICC nodes.

... what do you need this for?

(The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
the platform, we usually mean taking CPUs online/offline for power management. e.g.
cpuhp_offline_cpu_device())

It looks like you're adding support for hot-adding a new package/die to the platform ...
but only for virtualisation.

I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
these vcpu exist before you can enter the guest for the first time. You can't create them
late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
problem?

If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
the guest brings the vcpu online, which would solve that problem, and save the host
resources for the thread too. (and its acpi/dt agnostic)

I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
the vcpu online later. The only real difference seems to be moving the can-be-online
policy into the hypervisor/VMM...


I think physical package/die hotadd is a much bigger, uglier problem than doing the same
under virtualisation. Its best to do this on real hardware first so we don't miss
something. (cpu-topology, numa, memory, errata, timers?)
I'm worried that doing virtualisation first means the firmware-requirements for physical
hotadd stuff is "whatever Qemu does".


Thanks,

James

2019-07-09 19:20:25

by Maran Wilson

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

On 7/5/2019 3:12 AM, James Morse wrote:
> Hi guys,
>
> (CC: +kvmarm list)
>
> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>> is disabled. But only those enabled GICC node are marked as present CPUs.
>> So that kernel will initialize some CPU related data structure in advance before
>> the CPU is actually hot added into the system. This patchset also implement
>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>> needed to enable CPU hotplug.
>>
>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>> including those CPUs that are not present but may be hot added later. Those
>> CPUs are marked as disabled in GICC nodes.
> ... what do you need this for?
>
> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
> the platform, we usually mean taking CPUs online/offline for power management. e.g.
> cpuhp_offline_cpu_device())
>
> It looks like you're adding support for hot-adding a new package/die to the platform ...
> but only for virtualisation.
>
> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
> these vcpu exist before you can enter the guest for the first time. You can't create them
> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
> problem?
>
> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
> the guest brings the vcpu online, which would solve that problem, and save the host
> resources for the thread too. (and its acpi/dt agnostic)
>
> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
> the vcpu online later. The only real difference seems to be moving the can-be-online
> policy into the hypervisor/VMM...

Isn't that an important distinction from a cloud service provider's
perspective?

As far as I understand it, you also need CPU hotplug capabilities to
support things like Kata runtime under Kubernetes. i.e. when
implementing your containers in the form of light weight VMs for the
additional security ... and the orchestration layer cannot determine
ahead of time how much CPU/memory resources are going to be needed to
run the pod(s).

Thanks,
-Maran

>
> I think physical package/die hotadd is a much bigger, uglier problem than doing the same
> under virtualisation. Its best to do this on real hardware first so we don't miss
> something. (cpu-topology, numa, memory, errata, timers?)
> I'm worried that doing virtualisation first means the firmware-requirements for physical
> hotadd stuff is "whatever Qemu does".
>
>
> Thanks,
>
> James
> _______________________________________________
> kvmarm mailing list
> [email protected]
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

2019-07-10 09:16:34

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

On 09/07/2019 20:06, Maran Wilson wrote:
> On 7/5/2019 3:12 AM, James Morse wrote:
>> Hi guys,
>>
>> (CC: +kvmarm list)
>>
>> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>>> is disabled. But only those enabled GICC node are marked as present CPUs.
>>> So that kernel will initialize some CPU related data structure in advance before
>>> the CPU is actually hot added into the system. This patchset also implement
>>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>>> needed to enable CPU hotplug.
>>>
>>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>>> including those CPUs that are not present but may be hot added later. Those
>>> CPUs are marked as disabled in GICC nodes.
>> ... what do you need this for?
>>
>> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
>> the platform, we usually mean taking CPUs online/offline for power management. e.g.
>> cpuhp_offline_cpu_device())
>>
>> It looks like you're adding support for hot-adding a new package/die to the platform ...
>> but only for virtualisation.
>>
>> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
>> these vcpu exist before you can enter the guest for the first time. You can't create them
>> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
>> problem?
>>
>> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
>> the guest brings the vcpu online, which would solve that problem, and save the host
>> resources for the thread too. (and its acpi/dt agnostic)
>>
>> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
>> the vcpu online later. The only real difference seems to be moving the can-be-online
>> policy into the hypervisor/VMM...
>
> Isn't that an important distinction from a cloud service provider's
> perspective?
>
> As far as I understand it, you also need CPU hotplug capabilities to
> support things like Kata runtime under Kubernetes. i.e. when
> implementing your containers in the form of light weight VMs for the
> additional security ... and the orchestration layer cannot determine
> ahead of time how much CPU/memory resources are going to be needed to
> run the pod(s).

Why would it be any different? You can pre-allocate your vcpus, leave
them parked until some external agent decides to signal the container
that it it can use another bunch of CPUs. At that point, the container
must actively boot these vcpus (they aren't going to come up by magic).

Given that you must have sized your virtual platform to deal with the
maximum set of resources you anticipate (think of the GIC
redistributors, for example), I really wonder what you gain here.

>
> Thanks,
> -Maran
>
>>
>> I think physical package/die hotadd is a much bigger, uglier problem than doing the same
>> under virtualisation. Its best to do this on real hardware first so we don't miss
>> something. (cpu-topology, numa, memory, errata, timers?)
>> I'm worried that doing virtualisation first means the firmware-requirements for physical
>> hotadd stuff is "whatever Qemu does".

For sure, I want to model the virtualization side after the actual HW,
and not the other way around. Live reconfiguration of the interrupt
topology (and thus the whole memory map) will certainly be challenging.

Thanks,

M.
--
Jazz is not dead. It just smells funny...

2019-07-10 16:10:20

by Maran Wilson

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

On 7/10/2019 2:15 AM, Marc Zyngier wrote:
> On 09/07/2019 20:06, Maran Wilson wrote:
>> On 7/5/2019 3:12 AM, James Morse wrote:
>>> Hi guys,
>>>
>>> (CC: +kvmarm list)
>>>
>>> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>>>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>>>> is disabled. But only those enabled GICC node are marked as present CPUs.
>>>> So that kernel will initialize some CPU related data structure in advance before
>>>> the CPU is actually hot added into the system. This patchset also implement
>>>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>>>> needed to enable CPU hotplug.
>>>>
>>>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>>>> including those CPUs that are not present but may be hot added later. Those
>>>> CPUs are marked as disabled in GICC nodes.
>>> ... what do you need this for?
>>>
>>> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
>>> the platform, we usually mean taking CPUs online/offline for power management. e.g.
>>> cpuhp_offline_cpu_device())
>>>
>>> It looks like you're adding support for hot-adding a new package/die to the platform ...
>>> but only for virtualisation.
>>>
>>> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
>>> these vcpu exist before you can enter the guest for the first time. You can't create them
>>> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
>>> problem?
>>>
>>> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
>>> the guest brings the vcpu online, which would solve that problem, and save the host
>>> resources for the thread too. (and its acpi/dt agnostic)
>>>
>>> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
>>> the vcpu online later. The only real difference seems to be moving the can-be-online
>>> policy into the hypervisor/VMM...
>> Isn't that an important distinction from a cloud service provider's
>> perspective?
>>
>> As far as I understand it, you also need CPU hotplug capabilities to
>> support things like Kata runtime under Kubernetes. i.e. when
>> implementing your containers in the form of light weight VMs for the
>> additional security ... and the orchestration layer cannot determine
>> ahead of time how much CPU/memory resources are going to be needed to
>> run the pod(s).
> Why would it be any different? You can pre-allocate your vcpus, leave
> them parked until some external agent decides to signal the container
> that it it can use another bunch of CPUs. At that point, the container
> must actively boot these vcpus (they aren't going to come up by magic).
>
> Given that you must have sized your virtual platform to deal with the
> maximum set of resources you anticipate (think of the GIC
> redistributors, for example), I really wonder what you gain here.

Maybe I'm not following the alternative proposal completely, but
wouldn't a guest VM (who happens to be in control of its OS) be able to
add/online vCPU resources without approval from the VMM this way?

Thanks,
-Maran

>> Thanks,
>> -Maran
>>
>>> I think physical package/die hotadd is a much bigger, uglier problem than doing the same
>>> under virtualisation. Its best to do this on real hardware first so we don't miss
>>> something. (cpu-topology, numa, memory, errata, timers?)
>>> I'm worried that doing virtualisation first means the firmware-requirements for physical
>>> hotadd stuff is "whatever Qemu does".
> For sure, I want to model the virtualization side after the actual HW,
> and not the other way around. Live reconfiguration of the interrupt
> topology (and thus the whole memory map) will certainly be challenging.
>
> Thanks,
>
> M.

2019-07-15 13:43:56

by James Morse

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

Hi Maran,

On 10/07/2019 17:05, Maran Wilson wrote:
> On 7/10/2019 2:15 AM, Marc Zyngier wrote:
>> On 09/07/2019 20:06, Maran Wilson wrote:
>>> On 7/5/2019 3:12 AM, James Morse wrote:
>>>> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>>>>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>>>>> is disabled. But only those enabled GICC node are marked as present CPUs.
>>>>> So that kernel will initialize some CPU related data structure in advance before
>>>>> the CPU is actually hot added into the system. This patchset also implement
>>>>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>>>>> needed to enable CPU hotplug.
>>>>>
>>>>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>>>>> including those CPUs that are not present but may be hot added later. Those
>>>>> CPUs are marked as disabled in GICC nodes.
>>>> ... what do you need this for?
>>>>
>>>> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
>>>> the platform, we usually mean taking CPUs online/offline for power management. e.g.
>>>> cpuhp_offline_cpu_device())
>>>>
>>>> It looks like you're adding support for hot-adding a new package/die to the platform ...
>>>> but only for virtualisation.
>>>>
>>>> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
>>>> these vcpu exist before you can enter the guest for the first time. You can't create them
>>>> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
>>>> problem?
>>>>
>>>> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
>>>> the guest brings the vcpu online, which would solve that problem, and save the host
>>>> resources for the thread too. (and its acpi/dt agnostic)
>>>>
>>>> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
>>>> the vcpu online later. The only real difference seems to be moving the can-be-online
>>>> policy into the hypervisor/VMM...

>>> Isn't that an important distinction from a cloud service provider's
>>> perspective?

Host cpu-time is. Describing this as guest vcpu's is a bit weird.

I'd expect the statement be something like "you're paying for 50% of one Xeon v-whatever".
It shouldn't make a difference if I run 8 vcpus or 2, the amount of cpu-time would still
be constrained by the cloud provider.


>>> As far as I understand it, you also need CPU hotplug capabilities to
>>> support things like Kata runtime under Kubernetes. i.e. when
>>> implementing your containers in the form of light weight VMs for the
>>> additional security ... and the orchestration layer cannot determine
>>> ahead of time how much CPU/memory resources are going to be needed to
>>> run the pod(s).

>> Why would it be any different? You can pre-allocate your vcpus, leave
>> them parked until some external agent decides to signal the container
>> that it it can use another bunch of CPUs. At that point, the container
>> must actively boot these vcpus (they aren't going to come up by magic).
>>
>> Given that you must have sized your virtual platform to deal with the
>> maximum set of resources you anticipate (think of the GIC
>> redistributors, for example), I really wonder what you gain here.

> Maybe I'm not following the alternative proposal completely, but wouldn't a guest VM (who
> happens to be in control of its OS) be able to add/online vCPU resources without approval
> from the VMM this way?

The in-kernel PSCI implementation will allow all CPUs to be online/offline. If we moved
that support to the VMM, it could apply some policy as to whether a cpu-online call
succeeds or fails.


Thanks,

James

2019-07-16 07:53:52

by Xiongfeng Wang

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64



On 2019/7/5 18:12, James Morse wrote:
> Hi guys,
>
> (CC: +kvmarm list)
>
> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>> is disabled. But only those enabled GICC node are marked as present CPUs.
>> So that kernel will initialize some CPU related data structure in advance before
>> the CPU is actually hot added into the system. This patchset also implement
>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>> needed to enable CPU hotplug.
>>
>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>> including those CPUs that are not present but may be hot added later. Those
>> CPUs are marked as disabled in GICC nodes.
>
> ... what do you need this for?
>
> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
> the platform, we usually mean taking CPUs online/offline for power management. e.g.
> cpuhp_offline_cpu_device())
>
> It looks like you're adding support for hot-adding a new package/die to the platform ...
> but only for virtualisation.

I read the GIC driver these days. It is a lot of work to configure the GIC at runtime,
and this patchset doesn't support this.
Actually, my original idea is hot-adding cores to the platform, and it is only for virtualisation.
These cores need to be on the same physical package. The GIC is initialized when
the kernel boots and GICR is initialized when the core is hot-added and brought up.

>
> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
> these vcpu exist before you can enter the guest for the first time. You can't create them
> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
> problem?
>
> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
> the guest brings the vcpu online, which would solve that problem, and save the host
> resources for the thread too. (and its acpi/dt agnostic)
>
> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
> the vcpu online later. The only real difference seems to be moving the can-be-online
> policy into the hypervisor/VMM...
>
>
> I think physical package/die hotadd is a much bigger, uglier problem than doing the same
> under virtualisation. Its best to do this on real hardware first so we don't miss
> something. (cpu-topology, numa, memory, errata, timers?)
> I'm worried that doing virtualisation first means the firmware-requirements for physical
> hotadd stuff is "whatever Qemu does".
>
>
> Thanks,
>
> James
>
> .
>

2019-07-16 08:01:26

by Jia He

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

Hi Marc

On 2019/7/10 17:15, Marc Zyngier wrote:
> On 09/07/2019 20:06, Maran Wilson wrote:
>> On 7/5/2019 3:12 AM, James Morse wrote:
>>> Hi guys,
>>>
>>> (CC: +kvmarm list)
>>>
>>> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>>>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>>>> is disabled. But only those enabled GICC node are marked as present CPUs.
>>>> So that kernel will initialize some CPU related data structure in advance before
>>>> the CPU is actually hot added into the system. This patchset also implement
>>>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>>>> needed to enable CPU hotplug.
>>>>
>>>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>>>> including those CPUs that are not present but may be hot added later. Those
>>>> CPUs are marked as disabled in GICC nodes.
>>> ... what do you need this for?
>>>
>>> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
>>> the platform, we usually mean taking CPUs online/offline for power management. e.g.
>>> cpuhp_offline_cpu_device())
>>>
>>> It looks like you're adding support for hot-adding a new package/die to the platform ...
>>> but only for virtualisation.
>>>
>>> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
>>> these vcpu exist before you can enter the guest for the first time. You can't create them
>>> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
>>> problem?
>>>
>>> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
>>> the guest brings the vcpu online, which would solve that problem, and save the host
>>> resources for the thread too. (and its acpi/dt agnostic)
>>>
>>> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
>>> the vcpu online later. The only real difference seems to be moving the can-be-online
>>> policy into the hypervisor/VMM...
>> Isn't that an important distinction from a cloud service provider's
>> perspective?
>>
>> As far as I understand it, you also need CPU hotplug capabilities to
>> support things like Kata runtime under Kubernetes. i.e. when
>> implementing your containers in the form of light weight VMs for the
>> additional security ... and the orchestration layer cannot determine
>> ahead of time how much CPU/memory resources are going to be needed to
>> run the pod(s).
> Why would it be any different? You can pre-allocate your vcpus, leave
> them parked until some external agent decides to signal the container
> that it it can use another bunch of CPUs. At that point, the container
> must actively boot these vcpus (they aren't going to come up by magic).
>
> Given that you must have sized your virtual platform to deal with the
> maximum set of resources you anticipate (think of the GIC
> redistributors, for example), I really wonder what you gain here.
I agree with your point in GIC aspect. It will mess up things if it makes

GIC resource hotpluggable in qemu. But it also would be better that vmm

only startup limited vcpu thread resource.

How about:

1. qemu only starts only N vcpu thread (-smp N, maxcpus=M)

2. qemu reserves the GIC resource with maxium M vcpu number

3. when qmp cmd cpu hotplug-add is triggerred, send a GED event to guest kernel

4. guest kernel recv it and trigger the acpi plug process.

Currently ACPI_CPU_HOTPLUG is enabled for Kconfig but completely not workable.


---
Cheers,
Jia

2019-07-16 08:33:40

by Marc Zyngier

[permalink] [raw]
Subject: Re: [RFC PATCH v2 0/3] Support CPU hotplug for ARM64

Hi Jia,

On 16/07/2019 08:59, Jia He wrote:
> Hi Marc
>
> On 2019/7/10 17:15, Marc Zyngier wrote:
>> On 09/07/2019 20:06, Maran Wilson wrote:
>>> On 7/5/2019 3:12 AM, James Morse wrote:
>>>> Hi guys,
>>>>
>>>> (CC: +kvmarm list)
>>>>
>>>> On 29/06/2019 03:42, Xiongfeng Wang wrote:
>>>>> This patchset mark all the GICC node in MADT as possible CPUs even though it
>>>>> is disabled. But only those enabled GICC node are marked as present CPUs.
>>>>> So that kernel will initialize some CPU related data structure in advance before
>>>>> the CPU is actually hot added into the system. This patchset also implement
>>>>> 'acpi_(un)map_cpu()' and 'arch_(un)register_cpu()' for ARM64. These functions are
>>>>> needed to enable CPU hotplug.
>>>>>
>>>>> To support CPU hotplug, we need to add all the possible GICC node in MADT
>>>>> including those CPUs that are not present but may be hot added later. Those
>>>>> CPUs are marked as disabled in GICC nodes.
>>>> ... what do you need this for?
>>>>
>>>> (The term cpu-hotplug in the arm world almost never means hot-adding a new package/die to
>>>> the platform, we usually mean taking CPUs online/offline for power management. e.g.
>>>> cpuhp_offline_cpu_device())
>>>>
>>>> It looks like you're adding support for hot-adding a new package/die to the platform ...
>>>> but only for virtualisation.
>>>>
>>>> I don't see why this is needed for virtualisation. The in-kernel irqchip needs to know
>>>> these vcpu exist before you can enter the guest for the first time. You can't create them
>>>> late. At best you're saving the host scheduling a vcpu that is offline. Is this really a
>>>> problem?
>>>>
>>>> If we moved PSCI support to user-space, you could avoid creating host vcpu threads until
>>>> the guest brings the vcpu online, which would solve that problem, and save the host
>>>> resources for the thread too. (and its acpi/dt agnostic)
>>>>
>>>> I don't see the difference here between booting the guest with 'maxcpus=1', and bringing
>>>> the vcpu online later. The only real difference seems to be moving the can-be-online
>>>> policy into the hypervisor/VMM...
>>> Isn't that an important distinction from a cloud service provider's
>>> perspective?
>>>
>>> As far as I understand it, you also need CPU hotplug capabilities to
>>> support things like Kata runtime under Kubernetes. i.e. when
>>> implementing your containers in the form of light weight VMs for the
>>> additional security ... and the orchestration layer cannot determine
>>> ahead of time how much CPU/memory resources are going to be needed to
>>> run the pod(s).
>> Why would it be any different? You can pre-allocate your vcpus, leave
>> them parked until some external agent decides to signal the container
>> that it it can use another bunch of CPUs. At that point, the container
>> must actively boot these vcpus (they aren't going to come up by magic).
>>
>> Given that you must have sized your virtual platform to deal with the
>> maximum set of resources you anticipate (think of the GIC
>> redistributors, for example), I really wonder what you gain here.
> I agree with your point in GIC aspect. It will mess up things if it makes
>
> GIC resource hotpluggable in qemu.

It is far worse than just a mess. You'd need to come up with a way to
place your redistributors in memory, and tell the running guest where
these redistributors are. Currently, there is no method to describe such
changes to the address space, and I certainly don't want QEMU to invent
one. This needs to be modeled after what would happen on real HW.

> But it also would be better that vmm
>
> only startup limited vcpu thread resource.
>
> How about:
>
> 1. qemu only starts only N vcpu thread (-smp N, maxcpus=M)
>
> 2. qemu reserves the GIC resource with maxium M vcpu number

Note that this implies actually initializing M vcpus in the VM. You may
not have created the corresponding (M - N) threads, but the vcpus will
exist. Can you please quantify how much you'd save by doing that?

> 3. when qmp cmd cpu hotplug-add is triggerred, send a GED event to guest kernel
>
> 4. guest kernel recv it and trigger the acpi plug process.
>
> Currently ACPI_CPU_HOTPLUG is enabled for Kconfig but completely not workable.

Well, there so far *zero* CPU_HOTPLUG in the arm64 kernel other than
getting CPUs in and out of PSCI.

Thanks,

M.
--
Jazz is not dead. It just smells funny...