diff mbox series

[v3,5/5] hw/loongarch/virt: Enable cpu hotplug feature on virt machine

Message ID 20241104063435.4130262-6-maobibo@loongson.cn (mailing list archive)
State New
Headers show
Series hw/loongarch/virt: Add cpu hotplug support | expand

Commit Message

Bibo Mao Nov. 4, 2024, 6:34 a.m. UTC
On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For
hot-added CPUs after power on, interrupt pin of extioi and ipi interrupt
controller need connect to pins of new CPU.

Also change num-cpu property of extioi and ipi from smp.cpus to
smp.max_cpus

Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
 hw/loongarch/virt.c         | 57 +++++++++++++++++++++++++++++++++----
 include/hw/loongarch/virt.h |  2 ++
 2 files changed, 54 insertions(+), 5 deletions(-)

Comments

Igor Mammedov Nov. 5, 2024, 1:58 p.m. UTC | #1
On Mon,  4 Nov 2024 14:34:35 +0800
Bibo Mao <maobibo@loongson.cn> wrote:

> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For
> hot-added CPUs after power on, interrupt pin of extioi and ipi interrupt
> controller need connect to pins of new CPU.
> 
> Also change num-cpu property of extioi and ipi from smp.cpus to
> smp.max_cpus
> 
> Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
>  hw/loongarch/virt.c         | 57 +++++++++++++++++++++++++++++++++----
>  include/hw/loongarch/virt.h |  2 ++
>  2 files changed, 54 insertions(+), 5 deletions(-)
> 
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index e73f689c83..6673493109 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -891,8 +891,9 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>  
>      /* Create IPI device */
>      ipi = qdev_new(TYPE_LOONGARCH_IPI);
> -    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.cpus);
> +    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.max_cpus);
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
> +    lvms->ipi = ipi;
>  
>      /* IPI iocsr memory region */
>      memory_region_add_subregion(&lvms->system_iocsr, SMP_IPI_MAILBOX,
> @@ -905,11 +906,12 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>  
>      /* Create EXTIOI device */
>      extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
> -    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
> +    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.max_cpus);
>      if (virt_is_veiointc_enabled(lvms)) {
>          qdev_prop_set_bit(extioi, "has-virtualization-extension", true);
>      }
>      sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
> +    lvms->extioi = extioi;
>      memory_region_add_subregion(&lvms->system_iocsr, APIC_BASE,
>                      sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 0));
>      if (virt_is_veiointc_enabled(lvms)) {
> @@ -1403,8 +1405,40 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
>       }
>  
>      if (cpu->phy_id == UNSET_PHY_ID) {
> -        error_setg(&local_err, "CPU hotplug not supported");
> -        goto out;
> +        if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
> +            error_setg(&local_err,
> +                       "Invalid thread-id %u specified, must be in range 1:%u",
> +                       cpu->thread_id, ms->smp.threads - 1);
> +            goto out;
> +        }
> +
> +        if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
> +            error_setg(&local_err,
> +                       "Invalid core-id %u specified, must be in range 1:%u",
> +                       cpu->core_id, ms->smp.cores - 1);
> +            goto out;
> +        }
> +
> +        if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
> +            error_setg(&local_err,
> +                       "Invalid socket-id %u specified, must be in range 1:%u",
> +                       cpu->socket_id, ms->smp.sockets - 1);
> +            goto out;
> +        }
> +
> +        topo.socket_id = cpu->socket_id;
> +        topo.core_id = cpu->core_id;
> +        topo.thread_id = cpu->thread_id;
> +        arch_id =  virt_get_arch_id_from_topo(ms, &topo);
> +        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
> +        if (CPU(cpu_slot->cpu)) {
> +            error_setg(&local_err,
> +                       "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
> +                       cs->cpu_index, cpu->socket_id, cpu->core_id,
> +                       cpu->thread_id, cpu_slot->arch_id);
> +            goto out;
> +        }
> +        cpu->phy_id = arch_id;
>      } else {
>          /*
>           * For cold-add cpu, topo property is not set. And only physical id
> @@ -1484,10 +1518,22 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>                                  DeviceState *dev, Error **errp)
>  {
>      CPUArchId *cpu_slot;
> +    Error *local_err = NULL;
>      LoongArchCPU *cpu = LOONGARCH_CPU(dev);
> +    MachineState *ms = MACHINE(hotplug_dev);
>      LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
>  
> -    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
> +    if (lvms->acpi_ged) {
> +        /* Connect irq to cpu, including ipi and extioi irqchip */

> +        virt_init_cpu_irq(ms, CPU(cpu));

why it depends on GED?
Can't you just call it unconditionally here for both hotplugged and coldplugged CPUs?
/and drop this call from for() loop of coldplug CPUs/

> +        hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    cpu_slot = virt_find_cpu_slot(ms, cpu->phy_id, NULL);
>      cpu_slot->cpu = CPU(dev);
>      return;
>  }
> @@ -1671,6 +1717,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>      mc->numa_mem_supported = true;
>      mc->auto_enable_numa_with_memhp = true;
>      mc->auto_enable_numa_with_memdev = true;
> +    mc->has_hotpluggable_cpus = true;
>      mc->get_hotplug_handler = virt_get_hotplug_handler;
>      mc->default_nic = "virtio-net-pci";
>      hc->plug = virt_device_plug_cb;
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 861034d614..79a85723c9 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -61,6 +61,8 @@ struct LoongArchVirtMachineState {
>      MemoryRegion iocsr_mem;
>      AddressSpace as_iocsr;
>      struct loongarch_boot_info bootinfo;
> +    DeviceState *ipi;
> +    DeviceState *extioi;
>  };
>  
>  #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
Bibo Mao Nov. 6, 2024, 2:03 a.m. UTC | #2
On 2024/11/5 下午9:58, Igor Mammedov wrote:
> On Mon,  4 Nov 2024 14:34:35 +0800
> Bibo Mao <maobibo@loongson.cn> wrote:
> 
>> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For
>> hot-added CPUs after power on, interrupt pin of extioi and ipi interrupt
>> controller need connect to pins of new CPU.
>>
>> Also change num-cpu property of extioi and ipi from smp.cpus to
>> smp.max_cpus
>>
>> Co-developed-by: Xianglai Li <lixianglai@loongson.cn>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>>   hw/loongarch/virt.c         | 57 +++++++++++++++++++++++++++++++++----
>>   include/hw/loongarch/virt.h |  2 ++
>>   2 files changed, 54 insertions(+), 5 deletions(-)
>>
>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>> index e73f689c83..6673493109 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -891,8 +891,9 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>>   
>>       /* Create IPI device */
>>       ipi = qdev_new(TYPE_LOONGARCH_IPI);
>> -    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.cpus);
>> +    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.max_cpus);
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
>> +    lvms->ipi = ipi;
>>   
>>       /* IPI iocsr memory region */
>>       memory_region_add_subregion(&lvms->system_iocsr, SMP_IPI_MAILBOX,
>> @@ -905,11 +906,12 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>>   
>>       /* Create EXTIOI device */
>>       extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
>> -    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
>> +    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.max_cpus);
>>       if (virt_is_veiointc_enabled(lvms)) {
>>           qdev_prop_set_bit(extioi, "has-virtualization-extension", true);
>>       }
>>       sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
>> +    lvms->extioi = extioi;
>>       memory_region_add_subregion(&lvms->system_iocsr, APIC_BASE,
>>                       sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 0));
>>       if (virt_is_veiointc_enabled(lvms)) {
>> @@ -1403,8 +1405,40 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
>>        }
>>   
>>       if (cpu->phy_id == UNSET_PHY_ID) {
>> -        error_setg(&local_err, "CPU hotplug not supported");
>> -        goto out;
>> +        if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
>> +            error_setg(&local_err,
>> +                       "Invalid thread-id %u specified, must be in range 1:%u",
>> +                       cpu->thread_id, ms->smp.threads - 1);
>> +            goto out;
>> +        }
>> +
>> +        if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
>> +            error_setg(&local_err,
>> +                       "Invalid core-id %u specified, must be in range 1:%u",
>> +                       cpu->core_id, ms->smp.cores - 1);
>> +            goto out;
>> +        }
>> +
>> +        if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
>> +            error_setg(&local_err,
>> +                       "Invalid socket-id %u specified, must be in range 1:%u",
>> +                       cpu->socket_id, ms->smp.sockets - 1);
>> +            goto out;
>> +        }
>> +
>> +        topo.socket_id = cpu->socket_id;
>> +        topo.core_id = cpu->core_id;
>> +        topo.thread_id = cpu->thread_id;
>> +        arch_id =  virt_get_arch_id_from_topo(ms, &topo);
>> +        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
>> +        if (CPU(cpu_slot->cpu)) {
>> +            error_setg(&local_err,
>> +                       "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
>> +                       cs->cpu_index, cpu->socket_id, cpu->core_id,
>> +                       cpu->thread_id, cpu_slot->arch_id);
>> +            goto out;
>> +        }
>> +        cpu->phy_id = arch_id;
>>       } else {
>>           /*
>>            * For cold-add cpu, topo property is not set. And only physical id
>> @@ -1484,10 +1518,22 @@ static void virt_cpu_plug(HotplugHandler *hotplug_dev,
>>                                   DeviceState *dev, Error **errp)
>>   {
>>       CPUArchId *cpu_slot;
>> +    Error *local_err = NULL;
>>       LoongArchCPU *cpu = LOONGARCH_CPU(dev);
>> +    MachineState *ms = MACHINE(hotplug_dev);
>>       LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
>>   
>> -    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
>> +    if (lvms->acpi_ged) {
>> +        /* Connect irq to cpu, including ipi and extioi irqchip */
> 
>> +        virt_init_cpu_irq(ms, CPU(cpu));
> 
> why it depends on GED?
> Can't you just call it unconditionally here for both hotplugged and coldplugged CPUs?
> /and drop this call from for() loop of coldplug CPUs/
yes, will do in this way, then it will be unified for hotplugged and 
coldplugged CPUs.

CPU objects need be created after interrupt controller, also it can be 
created after acpi ged device. From the code function 
hotplug_handler_plug() will check PHASE_MACHINE_READY and hotplugged 
state, and do nothing for coldplugged CPUs.

And thanks for your guidance, will send the patch in next round.

Regards
Bibo Mao
> 
>> +        hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
>> +        if (local_err) {
>> +            error_propagate(errp, local_err);
>> +            return;
>> +        }
>> +    }
>> +
>> +    cpu_slot = virt_find_cpu_slot(ms, cpu->phy_id, NULL);
>>       cpu_slot->cpu = CPU(dev);
>>       return;
>>   }
>> @@ -1671,6 +1717,7 @@ static void virt_class_init(ObjectClass *oc, void *data)
>>       mc->numa_mem_supported = true;
>>       mc->auto_enable_numa_with_memhp = true;
>>       mc->auto_enable_numa_with_memdev = true;
>> +    mc->has_hotpluggable_cpus = true;
>>       mc->get_hotplug_handler = virt_get_hotplug_handler;
>>       mc->default_nic = "virtio-net-pci";
>>       hc->plug = virt_device_plug_cb;
>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>> index 861034d614..79a85723c9 100644
>> --- a/include/hw/loongarch/virt.h
>> +++ b/include/hw/loongarch/virt.h
>> @@ -61,6 +61,8 @@ struct LoongArchVirtMachineState {
>>       MemoryRegion iocsr_mem;
>>       AddressSpace as_iocsr;
>>       struct loongarch_boot_info bootinfo;
>> +    DeviceState *ipi;
>> +    DeviceState *extioi;
>>   };
>>   
>>   #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")
>
diff mbox series

Patch

diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index e73f689c83..6673493109 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -891,8 +891,9 @@  static void virt_irq_init(LoongArchVirtMachineState *lvms)
 
     /* Create IPI device */
     ipi = qdev_new(TYPE_LOONGARCH_IPI);
-    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.cpus);
+    qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.max_cpus);
     sysbus_realize_and_unref(SYS_BUS_DEVICE(ipi), &error_fatal);
+    lvms->ipi = ipi;
 
     /* IPI iocsr memory region */
     memory_region_add_subregion(&lvms->system_iocsr, SMP_IPI_MAILBOX,
@@ -905,11 +906,12 @@  static void virt_irq_init(LoongArchVirtMachineState *lvms)
 
     /* Create EXTIOI device */
     extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
-    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
+    qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.max_cpus);
     if (virt_is_veiointc_enabled(lvms)) {
         qdev_prop_set_bit(extioi, "has-virtualization-extension", true);
     }
     sysbus_realize_and_unref(SYS_BUS_DEVICE(extioi), &error_fatal);
+    lvms->extioi = extioi;
     memory_region_add_subregion(&lvms->system_iocsr, APIC_BASE,
                     sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 0));
     if (virt_is_veiointc_enabled(lvms)) {
@@ -1403,8 +1405,40 @@  static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev,
      }
 
     if (cpu->phy_id == UNSET_PHY_ID) {
-        error_setg(&local_err, "CPU hotplug not supported");
-        goto out;
+        if ((cpu->thread_id < 0) || (cpu->thread_id >= ms->smp.threads)) {
+            error_setg(&local_err,
+                       "Invalid thread-id %u specified, must be in range 1:%u",
+                       cpu->thread_id, ms->smp.threads - 1);
+            goto out;
+        }
+
+        if ((cpu->core_id < 0) || (cpu->core_id >= ms->smp.cores)) {
+            error_setg(&local_err,
+                       "Invalid core-id %u specified, must be in range 1:%u",
+                       cpu->core_id, ms->smp.cores - 1);
+            goto out;
+        }
+
+        if ((cpu->socket_id < 0) || (cpu->socket_id >= ms->smp.sockets)) {
+            error_setg(&local_err,
+                       "Invalid socket-id %u specified, must be in range 1:%u",
+                       cpu->socket_id, ms->smp.sockets - 1);
+            goto out;
+        }
+
+        topo.socket_id = cpu->socket_id;
+        topo.core_id = cpu->core_id;
+        topo.thread_id = cpu->thread_id;
+        arch_id =  virt_get_arch_id_from_topo(ms, &topo);
+        cpu_slot = virt_find_cpu_slot(ms, arch_id, &index);
+        if (CPU(cpu_slot->cpu)) {
+            error_setg(&local_err,
+                       "cpu(id%d=%d:%d:%d) with arch-id %" PRIu64 " exists",
+                       cs->cpu_index, cpu->socket_id, cpu->core_id,
+                       cpu->thread_id, cpu_slot->arch_id);
+            goto out;
+        }
+        cpu->phy_id = arch_id;
     } else {
         /*
          * For cold-add cpu, topo property is not set. And only physical id
@@ -1484,10 +1518,22 @@  static void virt_cpu_plug(HotplugHandler *hotplug_dev,
                                 DeviceState *dev, Error **errp)
 {
     CPUArchId *cpu_slot;
+    Error *local_err = NULL;
     LoongArchCPU *cpu = LOONGARCH_CPU(dev);
+    MachineState *ms = MACHINE(hotplug_dev);
     LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev);
 
-    cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL);
+    if (lvms->acpi_ged) {
+        /* Connect irq to cpu, including ipi and extioi irqchip */
+        virt_init_cpu_irq(ms, CPU(cpu));
+        hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err);
+        if (local_err) {
+            error_propagate(errp, local_err);
+            return;
+        }
+    }
+
+    cpu_slot = virt_find_cpu_slot(ms, cpu->phy_id, NULL);
     cpu_slot->cpu = CPU(dev);
     return;
 }
@@ -1671,6 +1717,7 @@  static void virt_class_init(ObjectClass *oc, void *data)
     mc->numa_mem_supported = true;
     mc->auto_enable_numa_with_memhp = true;
     mc->auto_enable_numa_with_memdev = true;
+    mc->has_hotpluggable_cpus = true;
     mc->get_hotplug_handler = virt_get_hotplug_handler;
     mc->default_nic = "virtio-net-pci";
     hc->plug = virt_device_plug_cb;
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 861034d614..79a85723c9 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -61,6 +61,8 @@  struct LoongArchVirtMachineState {
     MemoryRegion iocsr_mem;
     AddressSpace as_iocsr;
     struct loongarch_boot_info bootinfo;
+    DeviceState *ipi;
+    DeviceState *extioi;
 };
 
 #define TYPE_LOONGARCH_VIRT_MACHINE  MACHINE_TYPE_NAME("virt")