| Message ID | 20241112021738.1952851-7-maobibo@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | hw/loongarch/virt: Add cpu hotplug support | expand |
On Tue, 12 Nov 2024 10:17:38 +0800 Bibo Mao <maobibo@loongson.cn> wrote: > On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For > hot-added CPUs, there is socket-id/core-id/thread-id property set, > arch_id can be caculated from these properties. So that cpu slot can be > searched from its arch_id. > > 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 | 68 +++++++++++++++++++++++++++++++++++++++------ > 1 file changed, 59 insertions(+), 9 deletions(-) > > diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c > index b49b15c0f6..5f81673368 100644 > --- a/hw/loongarch/virt.c > +++ b/hw/loongarch/virt.c > @@ -890,7 +890,7 @@ 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; > > @@ -905,7 +905,7 @@ 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); > } > @@ -1369,11 +1369,15 @@ static void virt_get_topo_from_index(MachineState *ms, > } > > /* Find cpu slot in machine->possible_cpus by arch_id */ > -static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id) > +static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id, int *index) > { > int n; > for (n = 0; n < ms->possible_cpus->len; n++) { > if (ms->possible_cpus->cpus[n].arch_id == arch_id) { > + if (index) { > + *index = n; > + } > + > return &ms->possible_cpus->cpus[n]; > } > } > @@ -1386,10 +1390,12 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, > { > LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev); > MachineState *ms = MACHINE(OBJECT(hotplug_dev)); > + CPUState *cs = CPU(dev); > LoongArchCPU *cpu = LOONGARCH_CPU(dev); > CPUArchId *cpu_slot; > Error *local_err = NULL; > - int arch_id; > + LoongArchCPUTopo topo; > + int arch_id, index; > > /* sanity check the cpu */ > if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) { > @@ -1408,12 +1414,45 @@ 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; > + cs->cpu_index = index; this whole branch applies to cold-plugged CPUs as well, especially if both (hot/cold plugged CPUs are getting wired with help of pre_plug) So this hunk should be introduced somewhere earlier in series, and than I'd likely won't need (cpu->phy_id == UNSET_PHY_ID) check to begin with. the only difference vs cold-plug would be need to call acpi_ged plug handler, like you are dong below in virt_cpu_plug > } else { > /* For cold-add cpu, find cpu slot from arch_id */ > arch_id = cpu->phy_id; > - cpu_slot = virt_find_cpu_slot(ms, arch_id); > + cpu_slot = virt_find_cpu_slot(ms, arch_id, NULL); > } > > numa_cpu_pre_plug(cpu_slot, dev, &local_err); > @@ -1468,7 +1507,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, > return; > } > > - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); > + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); > cpu_slot->cpu = NULL; > return; > } > @@ -1477,14 +1516,24 @@ 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); > > /* Connect irq to cpu, including ipi and extioi irqchip */ > virt_init_cpu_irq(ms, CPU(cpu)); > - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); > + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); > cpu_slot->cpu = CPU(dev); > + > + if (lvms->acpi_ged) { Why do you need check, can machine be created without acpi_ged? > + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err); > + if (local_err) { > + error_propagate(errp, local_err); > + return; > + } > + } > + > return; > } > > @@ -1667,6 +1716,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;
On 2024/11/19 上午1:03, Igor Mammedov wrote: > On Tue, 12 Nov 2024 10:17:38 +0800 > Bibo Mao <maobibo@loongson.cn> wrote: > >> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For >> hot-added CPUs, there is socket-id/core-id/thread-id property set, >> arch_id can be caculated from these properties. So that cpu slot can be >> searched from its arch_id. >> >> 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 | 68 +++++++++++++++++++++++++++++++++++++++------ >> 1 file changed, 59 insertions(+), 9 deletions(-) >> >> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c >> index b49b15c0f6..5f81673368 100644 >> --- a/hw/loongarch/virt.c >> +++ b/hw/loongarch/virt.c >> @@ -890,7 +890,7 @@ 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; >> >> @@ -905,7 +905,7 @@ 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); >> } >> @@ -1369,11 +1369,15 @@ static void virt_get_topo_from_index(MachineState *ms, >> } >> >> /* Find cpu slot in machine->possible_cpus by arch_id */ >> -static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id) >> +static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id, int *index) >> { >> int n; >> for (n = 0; n < ms->possible_cpus->len; n++) { >> if (ms->possible_cpus->cpus[n].arch_id == arch_id) { >> + if (index) { >> + *index = n; >> + } >> + >> return &ms->possible_cpus->cpus[n]; >> } >> } >> @@ -1386,10 +1390,12 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, >> { >> LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev); >> MachineState *ms = MACHINE(OBJECT(hotplug_dev)); >> + CPUState *cs = CPU(dev); >> LoongArchCPU *cpu = LOONGARCH_CPU(dev); >> CPUArchId *cpu_slot; >> Error *local_err = NULL; >> - int arch_id; >> + LoongArchCPUTopo topo; >> + int arch_id, index; >> >> /* sanity check the cpu */ >> if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) { >> @@ -1408,12 +1414,45 @@ 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; >> + cs->cpu_index = index; > this whole branch applies to cold-plugged CPUs as well, especially > if both (hot/cold plugged CPUs are getting wired with help of pre_plug) > So this hunk should be introduced somewhere earlier in series, > and than I'd likely won't need (cpu->phy_id == UNSET_PHY_ID) check to begin with. > > the only difference vs cold-plug would be need to call acpi_ged plug handler, > like you are dong below in virt_cpu_plug Sure, will check acpi_ged plug handler for cold-plug/hot-plug CPU. > >> } else { >> /* For cold-add cpu, find cpu slot from arch_id */ >> arch_id = cpu->phy_id; >> - cpu_slot = virt_find_cpu_slot(ms, arch_id); >> + cpu_slot = virt_find_cpu_slot(ms, arch_id, NULL); >> } >> >> numa_cpu_pre_plug(cpu_slot, dev, &local_err); >> @@ -1468,7 +1507,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, >> return; >> } >> >> - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); >> + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); >> cpu_slot->cpu = NULL; >> return; >> } >> @@ -1477,14 +1516,24 @@ 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); >> >> /* Connect irq to cpu, including ipi and extioi irqchip */ >> virt_init_cpu_irq(ms, CPU(cpu)); >> - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); >> + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); >> cpu_slot->cpu = CPU(dev); >> + >> + if (lvms->acpi_ged) { > Why do you need check, can machine be created without acpi_ged? There is no NULL check with macro HOTPLUG_HANDLER() for cold-plug cpu. Now machine is created with acpi_ged always, in later will add noapic option support. Regards Bibo Mao > >> + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err); >> + if (local_err) { >> + error_propagate(errp, local_err); >> + return; >> + } >> + } >> + >> return; >> } >> >> @@ -1667,6 +1716,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;
On Tue, 19 Nov 2024 18:18:27 +0800 bibo mao <maobibo@loongson.cn> wrote: > On 2024/11/19 上午1:03, Igor Mammedov wrote: > > On Tue, 12 Nov 2024 10:17:38 +0800 > > Bibo Mao <maobibo@loongson.cn> wrote: > > > >> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For > >> hot-added CPUs, there is socket-id/core-id/thread-id property set, > >> arch_id can be caculated from these properties. So that cpu slot can be > >> searched from its arch_id. > >> > >> 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 | 68 +++++++++++++++++++++++++++++++++++++++------ > >> 1 file changed, 59 insertions(+), 9 deletions(-) > >> > >> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c > >> index b49b15c0f6..5f81673368 100644 > >> --- a/hw/loongarch/virt.c > >> +++ b/hw/loongarch/virt.c > >> @@ -890,7 +890,7 @@ 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; > >> > >> @@ -905,7 +905,7 @@ 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); > >> } > >> @@ -1369,11 +1369,15 @@ static void virt_get_topo_from_index(MachineState *ms, > >> } > >> > >> /* Find cpu slot in machine->possible_cpus by arch_id */ > >> -static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id) > >> +static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id, int *index) > >> { > >> int n; > >> for (n = 0; n < ms->possible_cpus->len; n++) { > >> if (ms->possible_cpus->cpus[n].arch_id == arch_id) { > >> + if (index) { > >> + *index = n; > >> + } > >> + > >> return &ms->possible_cpus->cpus[n]; > >> } > >> } > >> @@ -1386,10 +1390,12 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, > >> { > >> LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev); > >> MachineState *ms = MACHINE(OBJECT(hotplug_dev)); > >> + CPUState *cs = CPU(dev); > >> LoongArchCPU *cpu = LOONGARCH_CPU(dev); > >> CPUArchId *cpu_slot; > >> Error *local_err = NULL; > >> - int arch_id; > >> + LoongArchCPUTopo topo; > >> + int arch_id, index; > >> > >> /* sanity check the cpu */ > >> if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) { > >> @@ -1408,12 +1414,45 @@ 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; > >> + cs->cpu_index = index; > > this whole branch applies to cold-plugged CPUs as well, especially > > if both (hot/cold plugged CPUs are getting wired with help of pre_plug) > > So this hunk should be introduced somewhere earlier in series, > > and than I'd likely won't need (cpu->phy_id == UNSET_PHY_ID) check to begin with. > > > > the only difference vs cold-plug would be need to call acpi_ged plug handler, > > like you are dong below in virt_cpu_plug > Sure, will check acpi_ged plug handler for cold-plug/hot-plug CPU. > > > > >> } else { > >> /* For cold-add cpu, find cpu slot from arch_id */ > >> arch_id = cpu->phy_id; > >> - cpu_slot = virt_find_cpu_slot(ms, arch_id); > >> + cpu_slot = virt_find_cpu_slot(ms, arch_id, NULL); > >> } > >> > >> numa_cpu_pre_plug(cpu_slot, dev, &local_err); > >> @@ -1468,7 +1507,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, > >> return; > >> } > >> > >> - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); > >> + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); > >> cpu_slot->cpu = NULL; > >> return; > >> } > >> @@ -1477,14 +1516,24 @@ 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); > >> > >> /* Connect irq to cpu, including ipi and extioi irqchip */ > >> virt_init_cpu_irq(ms, CPU(cpu)); > >> - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); > >> + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); > >> cpu_slot->cpu = CPU(dev); > >> + > >> + if (lvms->acpi_ged) { > > Why do you need check, can machine be created without acpi_ged? > There is no NULL check with macro HOTPLUG_HANDLER() for cold-plug cpu. > Now machine is created with acpi_ged always, in later will add noapic > option support. you've probably meant '-noacpi', anyways right now acpi_ged is always present, so make this patch unconditional. If later on you find a use-case for '-noacpi' and add it, then introduce condition at that time. > > Regards > Bibo Mao > > > > >> + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err); > >> + if (local_err) { > >> + error_propagate(errp, local_err); > >> + return; > >> + } > >> + } > >> + > >> return; > >> } > >> > >> @@ -1667,6 +1716,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; >
On 2024/11/22 下午9:50, Igor Mammedov wrote: > On Tue, 19 Nov 2024 18:18:27 +0800 > bibo mao <maobibo@loongson.cn> wrote: > >> On 2024/11/19 上午1:03, Igor Mammedov wrote: >>> On Tue, 12 Nov 2024 10:17:38 +0800 >>> Bibo Mao <maobibo@loongson.cn> wrote: >>> >>>> On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For >>>> hot-added CPUs, there is socket-id/core-id/thread-id property set, >>>> arch_id can be caculated from these properties. So that cpu slot can be >>>> searched from its arch_id. >>>> >>>> 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 | 68 +++++++++++++++++++++++++++++++++++++++------ >>>> 1 file changed, 59 insertions(+), 9 deletions(-) >>>> >>>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c >>>> index b49b15c0f6..5f81673368 100644 >>>> --- a/hw/loongarch/virt.c >>>> +++ b/hw/loongarch/virt.c >>>> @@ -890,7 +890,7 @@ 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; >>>> >>>> @@ -905,7 +905,7 @@ 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); >>>> } >>>> @@ -1369,11 +1369,15 @@ static void virt_get_topo_from_index(MachineState *ms, >>>> } >>>> >>>> /* Find cpu slot in machine->possible_cpus by arch_id */ >>>> -static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id) >>>> +static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id, int *index) >>>> { >>>> int n; >>>> for (n = 0; n < ms->possible_cpus->len; n++) { >>>> if (ms->possible_cpus->cpus[n].arch_id == arch_id) { >>>> + if (index) { >>>> + *index = n; >>>> + } >>>> + >>>> return &ms->possible_cpus->cpus[n]; >>>> } >>>> } >>>> @@ -1386,10 +1390,12 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, >>>> { >>>> LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev); >>>> MachineState *ms = MACHINE(OBJECT(hotplug_dev)); >>>> + CPUState *cs = CPU(dev); >>>> LoongArchCPU *cpu = LOONGARCH_CPU(dev); >>>> CPUArchId *cpu_slot; >>>> Error *local_err = NULL; >>>> - int arch_id; >>>> + LoongArchCPUTopo topo; >>>> + int arch_id, index; >>>> >>>> /* sanity check the cpu */ >>>> if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) { >>>> @@ -1408,12 +1414,45 @@ 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; >>>> + cs->cpu_index = index; >>> this whole branch applies to cold-plugged CPUs as well, especially >>> if both (hot/cold plugged CPUs are getting wired with help of pre_plug) >>> So this hunk should be introduced somewhere earlier in series, >>> and than I'd likely won't need (cpu->phy_id == UNSET_PHY_ID) check to begin with. >>> >>> the only difference vs cold-plug would be need to call acpi_ged plug handler, >>> like you are dong below in virt_cpu_plug >> Sure, will check acpi_ged plug handler for cold-plug/hot-plug CPU. >> >>> >>>> } else { >>>> /* For cold-add cpu, find cpu slot from arch_id */ >>>> arch_id = cpu->phy_id; >>>> - cpu_slot = virt_find_cpu_slot(ms, arch_id); >>>> + cpu_slot = virt_find_cpu_slot(ms, arch_id, NULL); >>>> } >>>> >>>> numa_cpu_pre_plug(cpu_slot, dev, &local_err); >>>> @@ -1468,7 +1507,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, >>>> return; >>>> } >>>> >>>> - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); >>>> + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); >>>> cpu_slot->cpu = NULL; >>>> return; >>>> } >>>> @@ -1477,14 +1516,24 @@ 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); >>>> >>>> /* Connect irq to cpu, including ipi and extioi irqchip */ >>>> virt_init_cpu_irq(ms, CPU(cpu)); >>>> - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); >>>> + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); >>>> cpu_slot->cpu = CPU(dev); >>>> + >>>> + if (lvms->acpi_ged) { >>> Why do you need check, can machine be created without acpi_ged? >> There is no NULL check with macro HOTPLUG_HANDLER() for cold-plug cpu. >> Now machine is created with acpi_ged always, in later will add noapic >> option support. > > you've probably meant '-noacpi', > anyways right now acpi_ged is always present, so make this patch unconditional. Sure, will do in this way. Regards Bibo Mao > > If later on you find a use-case for '-noacpi' and add it, > then introduce condition at that time. > >> >> Regards >> Bibo Mao >> >>> >>>> + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err); >>>> + if (local_err) { >>>> + error_propagate(errp, local_err); >>>> + return; >>>> + } >>>> + } >>>> + >>>> return; >>>> } >>>> >>>> @@ -1667,6 +1716,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/hw/loongarch/virt.c b/hw/loongarch/virt.c index b49b15c0f6..5f81673368 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -890,7 +890,7 @@ 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; @@ -905,7 +905,7 @@ 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); } @@ -1369,11 +1369,15 @@ static void virt_get_topo_from_index(MachineState *ms, } /* Find cpu slot in machine->possible_cpus by arch_id */ -static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id) +static CPUArchId *virt_find_cpu_slot(MachineState *ms, int arch_id, int *index) { int n; for (n = 0; n < ms->possible_cpus->len; n++) { if (ms->possible_cpus->cpus[n].arch_id == arch_id) { + if (index) { + *index = n; + } + return &ms->possible_cpus->cpus[n]; } } @@ -1386,10 +1390,12 @@ static void virt_cpu_pre_plug(HotplugHandler *hotplug_dev, { LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(hotplug_dev); MachineState *ms = MACHINE(OBJECT(hotplug_dev)); + CPUState *cs = CPU(dev); LoongArchCPU *cpu = LOONGARCH_CPU(dev); CPUArchId *cpu_slot; Error *local_err = NULL; - int arch_id; + LoongArchCPUTopo topo; + int arch_id, index; /* sanity check the cpu */ if (!object_dynamic_cast(OBJECT(cpu), ms->cpu_type)) { @@ -1408,12 +1414,45 @@ 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; + cs->cpu_index = index; } else { /* For cold-add cpu, find cpu slot from arch_id */ arch_id = cpu->phy_id; - cpu_slot = virt_find_cpu_slot(ms, arch_id); + cpu_slot = virt_find_cpu_slot(ms, arch_id, NULL); } numa_cpu_pre_plug(cpu_slot, dev, &local_err); @@ -1468,7 +1507,7 @@ static void virt_cpu_unplug(HotplugHandler *hotplug_dev, return; } - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); cpu_slot->cpu = NULL; return; } @@ -1477,14 +1516,24 @@ 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); /* Connect irq to cpu, including ipi and extioi irqchip */ virt_init_cpu_irq(ms, CPU(cpu)); - cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id); + cpu_slot = virt_find_cpu_slot(MACHINE(lvms), cpu->phy_id, NULL); cpu_slot->cpu = CPU(dev); + + if (lvms->acpi_ged) { + hotplug_handler_plug(HOTPLUG_HANDLER(lvms->acpi_ged), dev, &local_err); + if (local_err) { + error_propagate(errp, local_err); + return; + } + } + return; } @@ -1667,6 +1716,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;
On virt machine, enable CPU hotplug feature has_hotpluggable_cpus. For hot-added CPUs, there is socket-id/core-id/thread-id property set, arch_id can be caculated from these properties. So that cpu slot can be searched from its arch_id. 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 | 68 +++++++++++++++++++++++++++++++++++++++------ 1 file changed, 59 insertions(+), 9 deletions(-)