| Message ID | 20241029095335.2219343-2-maobibo@loongson.cn (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | hw/loongarch/virt: Add cpu hotplug support | expand |
Hi Bibo, [snip] > +In the CPU topology relationship, When we know the ``socket_id`` ``core_id`` > +and ``thread_id`` of the CPU, we can calculate its ``arch_id``: > + > +``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)`` What's the difference between arch_id and CPU index (CPUState.cpu_index)? > static void virt_init(MachineState *machine) > { > - LoongArchCPU *lacpu; > const char *cpu_model = machine->cpu_type; > MemoryRegion *address_space_mem = get_system_memory(); > LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine); > @@ -1145,7 +1144,7 @@ static void virt_init(MachineState *machine) > hwaddr base, size, ram_size = machine->ram_size; > const CPUArchIdList *possible_cpus; > MachineClass *mc = MACHINE_GET_CLASS(machine); > - CPUState *cpu; > + Object *cpuobj; > > if (!cpu_model) { > cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); > @@ -1164,12 +1163,25 @@ static void virt_init(MachineState *machine) > > /* Init CPUs */ > possible_cpus = mc->possible_cpu_arch_ids(machine); > - for (i = 0; i < possible_cpus->len; i++) { > - cpu = cpu_create(machine->cpu_type); > - cpu->cpu_index = i; > - machine->possible_cpus->cpus[i].cpu = cpu; > - lacpu = LOONGARCH_CPU(cpu); > - lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id; > + for (i = 0; i < machine->smp.cpus; i++) { > + cpuobj = object_new(machine->cpu_type); > + object_property_set_uint(cpuobj, "phy-id", > + possible_cpus->cpus[i].arch_id, NULL); > + /* > + * The CPU in place at the time of machine startup will also enter > + * the CPU hot-plug process when it is created, but at this time, > + * the GED device has not been created, resulting in exit in the CPU > + * hot-plug process, which can avoid the incumbent CPU repeatedly > + * applying for resources. > + * > + * The interrupt resource of the in-place CPU will be requested at > + * the current function call virt_irq_init(). > + * > + * The interrupt resource of the subsequently inserted CPU will be > + * requested in the CPU hot-plug process. > + */ > + qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > + object_unref(cpuobj); You can use qdev_realize_and_unref(). > } > fdt_add_cpu_nodes(lvms); > fdt_add_memory_nodes(machine); > @@ -1286,6 +1298,35 @@ static void virt_initfn(Object *obj) > virt_flash_create(lvms); > } > > +static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo) > +{ > + int arch_id, sock_vcpu_num, core_vcpu_num; > + > + /* > + * calculate total logical cpus across socket/core/thread. > + * For more information on how to calculate the arch_id, > + * you can refer to the CPU Topology chapter of the > + * docs/system/loongarch/virt.rst document. > + */ > + sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores); > + core_vcpu_num = topo->core_id * ms->smp.threads; > + > + /* get vcpu-id(logical cpu index) for this vcpu from this topology */ > + arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id; Looking at the calculations, arch_id looks the same as cpu_index, right? > + assert(arch_id >= 0 && arch_id < ms->possible_cpus->len); > + > + return arch_id; > +} > + > +static void virt_get_topo_from_index(MachineState *ms, > + LoongArchCPUTopo *topo, int index) > +{ > + topo->socket_id = index / (ms->smp.cores * ms->smp.threads); > + topo->core_id = index / ms->smp.threads % ms->smp.cores; > + topo->thread_id = index % ms->smp.threads; > +} > + > static bool memhp_type_supported(DeviceState *dev) > { > /* we only support pc dimm now */ > @@ -1385,8 +1426,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, > > static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) > { > - int n; > + int n, arch_id; > unsigned int max_cpus = ms->smp.max_cpus; > + LoongArchCPUTopo topo; > > if (ms->possible_cpus) { > assert(ms->possible_cpus->len == max_cpus); > @@ -1397,17 +1439,17 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) > sizeof(CPUArchId) * max_cpus); > ms->possible_cpus->len = max_cpus; > for (n = 0; n < ms->possible_cpus->len; n++) { > + virt_get_topo_from_index(ms, &topo, n); > + arch_id = virt_get_arch_id_from_topo(ms, &topo); > + ms->possible_cpus->cpus[n].vcpus_count = ms->smp.threads; In include/hw/boards.h, the doc of CPUArchId said: vcpus_count - number of threads provided by @cpu object And I undersatnd each element in possible_cpus->cpus[] is mapped to a CPU object, so that here vcpus_count should be 1. > ms->possible_cpus->cpus[n].type = ms->cpu_type; > - ms->possible_cpus->cpus[n].arch_id = n; > - > + ms->possible_cpus->cpus[n].arch_id = arch_id; > ms->possible_cpus->cpus[n].props.has_socket_id = true; > - ms->possible_cpus->cpus[n].props.socket_id = > - n / (ms->smp.cores * ms->smp.threads); > + ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id; > ms->possible_cpus->cpus[n].props.has_core_id = true; > - ms->possible_cpus->cpus[n].props.core_id = > - n / ms->smp.threads % ms->smp.cores; > + ms->possible_cpus->cpus[n].props.core_id = topo.core_id; > ms->possible_cpus->cpus[n].props.has_thread_id = true; > - ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads; > + ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id; > } > return ms->possible_cpus; > } > diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c > index 7212fb5f8f..5dfc0d5c43 100644 > --- a/target/loongarch/cpu.c > +++ b/target/loongarch/cpu.c > @@ -16,6 +16,7 @@ > #include "kvm/kvm_loongarch.h" > #include "exec/exec-all.h" > #include "cpu.h" > +#include "hw/qdev-properties.h" > #include "internals.h" > #include "fpu/softfloat-helpers.h" > #include "cpu-csr.h" > @@ -780,6 +781,15 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs) > } > #endif > > +static Property loongarch_cpu_properties[] = { > + DEFINE_PROP_UINT32("phy-id", LoongArchCPU, phy_id, UNSET_PHY_ID), IIUC, the phy_id is kind of like the x86 apic_id, but the field is completely derived from topology, so why do you need to define it as a property and then expose it to the user? Thanks, Zhao
Hi Zhao, Thanks for reviewing the patch. On 2024/10/29 下午9:19, Zhao Liu wrote: > Hi Bibo, > > [snip] > >> +In the CPU topology relationship, When we know the ``socket_id`` ``core_id`` >> +and ``thread_id`` of the CPU, we can calculate its ``arch_id``: >> + >> +``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)`` > > What's the difference between arch_id and CPU index (CPUState.cpu_index)? > >> static void virt_init(MachineState *machine) >> { >> - LoongArchCPU *lacpu; >> const char *cpu_model = machine->cpu_type; >> MemoryRegion *address_space_mem = get_system_memory(); >> LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine); >> @@ -1145,7 +1144,7 @@ static void virt_init(MachineState *machine) >> hwaddr base, size, ram_size = machine->ram_size; >> const CPUArchIdList *possible_cpus; >> MachineClass *mc = MACHINE_GET_CLASS(machine); >> - CPUState *cpu; >> + Object *cpuobj; >> >> if (!cpu_model) { >> cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); >> @@ -1164,12 +1163,25 @@ static void virt_init(MachineState *machine) >> >> /* Init CPUs */ >> possible_cpus = mc->possible_cpu_arch_ids(machine); >> - for (i = 0; i < possible_cpus->len; i++) { >> - cpu = cpu_create(machine->cpu_type); >> - cpu->cpu_index = i; >> - machine->possible_cpus->cpus[i].cpu = cpu; >> - lacpu = LOONGARCH_CPU(cpu); >> - lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id; >> + for (i = 0; i < machine->smp.cpus; i++) { >> + cpuobj = object_new(machine->cpu_type); >> + object_property_set_uint(cpuobj, "phy-id", >> + possible_cpus->cpus[i].arch_id, NULL); >> + /* >> + * The CPU in place at the time of machine startup will also enter >> + * the CPU hot-plug process when it is created, but at this time, >> + * the GED device has not been created, resulting in exit in the CPU >> + * hot-plug process, which can avoid the incumbent CPU repeatedly >> + * applying for resources. >> + * >> + * The interrupt resource of the in-place CPU will be requested at >> + * the current function call virt_irq_init(). >> + * >> + * The interrupt resource of the subsequently inserted CPU will be >> + * requested in the CPU hot-plug process. >> + */ >> + qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); >> + object_unref(cpuobj); > > You can use qdev_realize_and_unref(). sure, will do. > >> } >> fdt_add_cpu_nodes(lvms); >> fdt_add_memory_nodes(machine); >> @@ -1286,6 +1298,35 @@ static void virt_initfn(Object *obj) >> virt_flash_create(lvms); >> } >> >> +static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo) >> +{ >> + int arch_id, sock_vcpu_num, core_vcpu_num; >> + >> + /* >> + * calculate total logical cpus across socket/core/thread. >> + * For more information on how to calculate the arch_id, >> + * you can refer to the CPU Topology chapter of the >> + * docs/system/loongarch/virt.rst document. >> + */ >> + sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores); >> + core_vcpu_num = topo->core_id * ms->smp.threads; >> + >> + /* get vcpu-id(logical cpu index) for this vcpu from this topology */ >> + arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id; > > Looking at the calculations, arch_id looks the same as cpu_index, right? The value of arch_id and cpu_index is the same now, however meaning is different. cpu_index is cpuslot index of possible_cpus array, arch_id is physical id. Now there is no special HW calculation for physical id, value of them is the same. In future if physical id width exceeds 8-bit because extioi only support max 256 cpu routing, its calculation method will change. > >> + assert(arch_id >= 0 && arch_id < ms->possible_cpus->len); >> + >> + return arch_id; >> +} >> + >> +static void virt_get_topo_from_index(MachineState *ms, >> + LoongArchCPUTopo *topo, int index) >> +{ >> + topo->socket_id = index / (ms->smp.cores * ms->smp.threads); >> + topo->core_id = index / ms->smp.threads % ms->smp.cores; >> + topo->thread_id = index % ms->smp.threads; >> +} >> + >> static bool memhp_type_supported(DeviceState *dev) >> { >> /* we only support pc dimm now */ >> @@ -1385,8 +1426,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, >> >> static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) >> { >> - int n; >> + int n, arch_id; >> unsigned int max_cpus = ms->smp.max_cpus; >> + LoongArchCPUTopo topo; >> >> if (ms->possible_cpus) { >> assert(ms->possible_cpus->len == max_cpus); >> @@ -1397,17 +1439,17 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) >> sizeof(CPUArchId) * max_cpus); >> ms->possible_cpus->len = max_cpus; >> for (n = 0; n < ms->possible_cpus->len; n++) { >> + virt_get_topo_from_index(ms, &topo, n); >> + arch_id = virt_get_arch_id_from_topo(ms, &topo); >> + ms->possible_cpus->cpus[n].vcpus_count = ms->smp.threads; > > In include/hw/boards.h, the doc of CPUArchId said: > > vcpus_count - number of threads provided by @cpu object > > And I undersatnd each element in possible_cpus->cpus[] is mapped to a > CPU object, so that here vcpus_count should be 1. yes, it is should be 1, thank for pointing it out > > >> ms->possible_cpus->cpus[n].type = ms->cpu_type; >> - ms->possible_cpus->cpus[n].arch_id = n; >> - >> + ms->possible_cpus->cpus[n].arch_id = arch_id; >> ms->possible_cpus->cpus[n].props.has_socket_id = true; >> - ms->possible_cpus->cpus[n].props.socket_id = >> - n / (ms->smp.cores * ms->smp.threads); >> + ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id; >> ms->possible_cpus->cpus[n].props.has_core_id = true; >> - ms->possible_cpus->cpus[n].props.core_id = >> - n / ms->smp.threads % ms->smp.cores; >> + ms->possible_cpus->cpus[n].props.core_id = topo.core_id; >> ms->possible_cpus->cpus[n].props.has_thread_id = true; >> - ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads; >> + ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id; >> } >> return ms->possible_cpus; >> } >> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c >> index 7212fb5f8f..5dfc0d5c43 100644 >> --- a/target/loongarch/cpu.c >> +++ b/target/loongarch/cpu.c >> @@ -16,6 +16,7 @@ >> #include "kvm/kvm_loongarch.h" >> #include "exec/exec-all.h" >> #include "cpu.h" >> +#include "hw/qdev-properties.h" >> #include "internals.h" >> #include "fpu/softfloat-helpers.h" >> #include "cpu-csr.h" >> @@ -780,6 +781,15 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs) >> } >> #endif >> >> +static Property loongarch_cpu_properties[] = { >> + DEFINE_PROP_UINT32("phy-id", LoongArchCPU, phy_id, UNSET_PHY_ID), > > IIUC, the phy_id is kind of like the x86 apic_id, but the field is completely > derived from topology, so why do you need to define it as a property and then > expose it to the user? The property is wrongly used here. we want to differentiate cold-added cpus and hot-added cpus. phy_id of cold-added cpus can be set during power on, however that of hot-added cpus is default -1. Internal variable phy_id can be set with default value -1 in function loongarch_cpu_init(), property can be removed. Regards Bibo Mao > > Thanks, > Zhao >
On Tue, 29 Oct 2024 21:19:15 +0800 Zhao Liu <zhao1.liu@intel.com> wrote: > Hi Bibo, > > [snip] > > > +In the CPU topology relationship, When we know the ``socket_id`` ``core_id`` > > +and ``thread_id`` of the CPU, we can calculate its ``arch_id``: > > + > > +``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)`` > > What's the difference between arch_id and CPU index (CPUState.cpu_index)? They might be the same but not necessarily. arch_id is unique cpu identifier from architecture point of view (which easily could be sparse and without specific order). (ex: for x86 it's apic_id, for spapr it's core_id) while cpu_index is internal QEMU, that existed before possible_cpus[] and non-sparse range and depends on order of cpus are created. For machines that support possible_cpus[], we override default cpu_index assignment to fit possible_cpus[]. It might be nice to get rid of cpu_index in favor of possible_cpus[], but that would be a lot work probably with no huge benefit when it comes majority of machines that don't need variable cpu count. > > > static void virt_init(MachineState *machine) > > { > > - LoongArchCPU *lacpu; > > const char *cpu_model = machine->cpu_type; > > MemoryRegion *address_space_mem = get_system_memory(); > > LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine); > > @@ -1145,7 +1144,7 @@ static void virt_init(MachineState *machine) > > hwaddr base, size, ram_size = machine->ram_size; > > const CPUArchIdList *possible_cpus; > > MachineClass *mc = MACHINE_GET_CLASS(machine); > > - CPUState *cpu; > > + Object *cpuobj; > > > > if (!cpu_model) { > > cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); > > @@ -1164,12 +1163,25 @@ static void virt_init(MachineState *machine) > > > > /* Init CPUs */ > > possible_cpus = mc->possible_cpu_arch_ids(machine); > > - for (i = 0; i < possible_cpus->len; i++) { > > - cpu = cpu_create(machine->cpu_type); > > - cpu->cpu_index = i; > > - machine->possible_cpus->cpus[i].cpu = cpu; > > - lacpu = LOONGARCH_CPU(cpu); > > - lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id; > > + for (i = 0; i < machine->smp.cpus; i++) { > > + cpuobj = object_new(machine->cpu_type); > > + object_property_set_uint(cpuobj, "phy-id", > > + possible_cpus->cpus[i].arch_id, NULL); > > + /* > > + * The CPU in place at the time of machine startup will also enter > > + * the CPU hot-plug process when it is created, but at this time, > > + * the GED device has not been created, resulting in exit in the CPU > > + * hot-plug process, which can avoid the incumbent CPU repeatedly > > + * applying for resources. > > + * > > + * The interrupt resource of the in-place CPU will be requested at > > + * the current function call virt_irq_init(). > > + * > > + * The interrupt resource of the subsequently inserted CPU will be > > + * requested in the CPU hot-plug process. > > + */ > > + qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > > + object_unref(cpuobj); > > You can use qdev_realize_and_unref(). > > > } > > fdt_add_cpu_nodes(lvms); > > fdt_add_memory_nodes(machine); > > @@ -1286,6 +1298,35 @@ static void virt_initfn(Object *obj) > > virt_flash_create(lvms); > > } > > > > +static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo) > > +{ > > + int arch_id, sock_vcpu_num, core_vcpu_num; > > + > > + /* > > + * calculate total logical cpus across socket/core/thread. > > + * For more information on how to calculate the arch_id, > > + * you can refer to the CPU Topology chapter of the > > + * docs/system/loongarch/virt.rst document. > > + */ > > + sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores); > > + core_vcpu_num = topo->core_id * ms->smp.threads; > > + > > + /* get vcpu-id(logical cpu index) for this vcpu from this topology */ > > + arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id; > > Looking at the calculations, arch_id looks the same as cpu_index, right? > > > + assert(arch_id >= 0 && arch_id < ms->possible_cpus->len); > > + > > + return arch_id; > > +} > > + > > +static void virt_get_topo_from_index(MachineState *ms, > > + LoongArchCPUTopo *topo, int index) > > +{ > > + topo->socket_id = index / (ms->smp.cores * ms->smp.threads); > > + topo->core_id = index / ms->smp.threads % ms->smp.cores; > > + topo->thread_id = index % ms->smp.threads; > > +} > > + > > static bool memhp_type_supported(DeviceState *dev) > > { > > /* we only support pc dimm now */ > > @@ -1385,8 +1426,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, > > > > static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) > > { > > - int n; > > + int n, arch_id; > > unsigned int max_cpus = ms->smp.max_cpus; > > + LoongArchCPUTopo topo; > > > > if (ms->possible_cpus) { > > assert(ms->possible_cpus->len == max_cpus); > > @@ -1397,17 +1439,17 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) > > sizeof(CPUArchId) * max_cpus); > > ms->possible_cpus->len = max_cpus; > > for (n = 0; n < ms->possible_cpus->len; n++) { > > + virt_get_topo_from_index(ms, &topo, n); > > + arch_id = virt_get_arch_id_from_topo(ms, &topo); > > + ms->possible_cpus->cpus[n].vcpus_count = ms->smp.threads; > > In include/hw/boards.h, the doc of CPUArchId said: > > vcpus_count - number of threads provided by @cpu object > > And I undersatnd each element in possible_cpus->cpus[] is mapped to a > CPU object, so that here vcpus_count should be 1. it's arch specific, CPU object in possible_cpus was meant to point to thread/core/..higher layers in future../ For example spapr put's there CPUCore, where vcpus_count can be > 1 That is a bit broken though, since CPUCore is not CPUState by any means, spapr_core_plug() gets away with it only because core_slot->cpu = CPU(dev) CPU() is dumb pointer cast. Ideally CPUArchId::cpu should be (Object*) to accommodate various levels of granularity correctly (with dumb cast above it's just cosmetics though as long as we treat it as Object in non arch specific code). > > ms->possible_cpus->cpus[n].type = ms->cpu_type; > > - ms->possible_cpus->cpus[n].arch_id = n; > > - > > + ms->possible_cpus->cpus[n].arch_id = arch_id; > > ms->possible_cpus->cpus[n].props.has_socket_id = true; > > - ms->possible_cpus->cpus[n].props.socket_id = > > - n / (ms->smp.cores * ms->smp.threads); > > + ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id; > > ms->possible_cpus->cpus[n].props.has_core_id = true; > > - ms->possible_cpus->cpus[n].props.core_id = > > - n / ms->smp.threads % ms->smp.cores; > > + ms->possible_cpus->cpus[n].props.core_id = topo.core_id; > > ms->possible_cpus->cpus[n].props.has_thread_id = true; > > - ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads; > > + ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id; > > } > > return ms->possible_cpus; > > } > > diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c > > index 7212fb5f8f..5dfc0d5c43 100644 > > --- a/target/loongarch/cpu.c > > +++ b/target/loongarch/cpu.c > > @@ -16,6 +16,7 @@ > > #include "kvm/kvm_loongarch.h" > > #include "exec/exec-all.h" > > #include "cpu.h" > > +#include "hw/qdev-properties.h" > > #include "internals.h" > > #include "fpu/softfloat-helpers.h" > > #include "cpu-csr.h" > > @@ -780,6 +781,15 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs) > > } > > #endif > > > > +static Property loongarch_cpu_properties[] = { > > + DEFINE_PROP_UINT32("phy-id", LoongArchCPU, phy_id, UNSET_PHY_ID), > > IIUC, the phy_id is kind of like the x86 apic_id, but the field is completely > derived from topology, so why do you need to define it as a property and then > expose it to the user? for x86 we do expose apic_id as a property as well, partly from historical pov but also it's better to access cpu fields via properties from outside of CPU object than directly poke inside of CPU structure from outside of CPU (especially if it comes to generic code) > > Thanks, > Zhao > >
On Wed, 30 Oct 2024 09:42:10 +0800 maobibo <maobibo@loongson.cn> wrote: > Hi Zhao, > > Thanks for reviewing the patch. > > On 2024/10/29 下午9:19, Zhao Liu wrote: > > Hi Bibo, > > > > [snip] > > > >> +In the CPU topology relationship, When we know the ``socket_id`` ``core_id`` > >> +and ``thread_id`` of the CPU, we can calculate its ``arch_id``: > >> + > >> +``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)`` > > > > What's the difference between arch_id and CPU index (CPUState.cpu_index)? > > > >> static void virt_init(MachineState *machine) > >> { > >> - LoongArchCPU *lacpu; > >> const char *cpu_model = machine->cpu_type; > >> MemoryRegion *address_space_mem = get_system_memory(); > >> LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine); > >> @@ -1145,7 +1144,7 @@ static void virt_init(MachineState *machine) > >> hwaddr base, size, ram_size = machine->ram_size; > >> const CPUArchIdList *possible_cpus; > >> MachineClass *mc = MACHINE_GET_CLASS(machine); > >> - CPUState *cpu; > >> + Object *cpuobj; > >> > >> if (!cpu_model) { > >> cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); > >> @@ -1164,12 +1163,25 @@ static void virt_init(MachineState *machine) > >> > >> /* Init CPUs */ > >> possible_cpus = mc->possible_cpu_arch_ids(machine); > >> - for (i = 0; i < possible_cpus->len; i++) { > >> - cpu = cpu_create(machine->cpu_type); > >> - cpu->cpu_index = i; > >> - machine->possible_cpus->cpus[i].cpu = cpu; > >> - lacpu = LOONGARCH_CPU(cpu); > >> - lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id; > >> + for (i = 0; i < machine->smp.cpus; i++) { > >> + cpuobj = object_new(machine->cpu_type); > >> + object_property_set_uint(cpuobj, "phy-id", > >> + possible_cpus->cpus[i].arch_id, NULL); > >> + /* > >> + * The CPU in place at the time of machine startup will also enter > >> + * the CPU hot-plug process when it is created, but at this time, > >> + * the GED device has not been created, resulting in exit in the CPU > >> + * hot-plug process, which can avoid the incumbent CPU repeatedly > >> + * applying for resources. > >> + * > >> + * The interrupt resource of the in-place CPU will be requested at > >> + * the current function call virt_irq_init(). > >> + * > >> + * The interrupt resource of the subsequently inserted CPU will be > >> + * requested in the CPU hot-plug process. > >> + */ > >> + qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); > >> + object_unref(cpuobj); > > > > You can use qdev_realize_and_unref(). > sure, will do. > > > > >> } > >> fdt_add_cpu_nodes(lvms); > >> fdt_add_memory_nodes(machine); > >> @@ -1286,6 +1298,35 @@ static void virt_initfn(Object *obj) > >> virt_flash_create(lvms); > >> } > >> > >> +static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo) > >> +{ > >> + int arch_id, sock_vcpu_num, core_vcpu_num; > >> + > >> + /* > >> + * calculate total logical cpus across socket/core/thread. > >> + * For more information on how to calculate the arch_id, > >> + * you can refer to the CPU Topology chapter of the > >> + * docs/system/loongarch/virt.rst document. > >> + */ > >> + sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores); > >> + core_vcpu_num = topo->core_id * ms->smp.threads; > >> + > >> + /* get vcpu-id(logical cpu index) for this vcpu from this topology */ > >> + arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id; > > > > Looking at the calculations, arch_id looks the same as cpu_index, right? > The value of arch_id and cpu_index is the same now, however meaning is > different. cpu_index is cpuslot index of possible_cpus array, arch_id is > physical id. > > Now there is no special HW calculation for physical id, value of them is > the same. In future if physical id width exceeds 8-bit because extioi > only support max 256 cpu routing, its calculation method will change. > > > >> + assert(arch_id >= 0 && arch_id < ms->possible_cpus->len); > >> + > >> + return arch_id; > >> +} > >> + > >> +static void virt_get_topo_from_index(MachineState *ms, > >> + LoongArchCPUTopo *topo, int index) > >> +{ > >> + topo->socket_id = index / (ms->smp.cores * ms->smp.threads); > >> + topo->core_id = index / ms->smp.threads % ms->smp.cores; > >> + topo->thread_id = index % ms->smp.threads; > >> +} > >> + > >> static bool memhp_type_supported(DeviceState *dev) > >> { > >> /* we only support pc dimm now */ > >> @@ -1385,8 +1426,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, > >> > >> static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) > >> { > >> - int n; > >> + int n, arch_id; > >> unsigned int max_cpus = ms->smp.max_cpus; > >> + LoongArchCPUTopo topo; > >> > >> if (ms->possible_cpus) { > >> assert(ms->possible_cpus->len == max_cpus); > >> @@ -1397,17 +1439,17 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) > >> sizeof(CPUArchId) * max_cpus); > >> ms->possible_cpus->len = max_cpus; > >> for (n = 0; n < ms->possible_cpus->len; n++) { > >> + virt_get_topo_from_index(ms, &topo, n); > >> + arch_id = virt_get_arch_id_from_topo(ms, &topo); > >> + ms->possible_cpus->cpus[n].vcpus_count = ms->smp.threads; > > > > In include/hw/boards.h, the doc of CPUArchId said: > > > > vcpus_count - number of threads provided by @cpu object > > > > And I undersatnd each element in possible_cpus->cpus[] is mapped to a > > CPU object, so that here vcpus_count should be 1. > yes, it is should be 1, thank for pointing it out > > > > > >> ms->possible_cpus->cpus[n].type = ms->cpu_type; > >> - ms->possible_cpus->cpus[n].arch_id = n; > >> - > >> + ms->possible_cpus->cpus[n].arch_id = arch_id; > >> ms->possible_cpus->cpus[n].props.has_socket_id = true; > >> - ms->possible_cpus->cpus[n].props.socket_id = > >> - n / (ms->smp.cores * ms->smp.threads); > >> + ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id; > >> ms->possible_cpus->cpus[n].props.has_core_id = true; > >> - ms->possible_cpus->cpus[n].props.core_id = > >> - n / ms->smp.threads % ms->smp.cores; > >> + ms->possible_cpus->cpus[n].props.core_id = topo.core_id; > >> ms->possible_cpus->cpus[n].props.has_thread_id = true; > >> - ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads; > >> + ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id; > >> } > >> return ms->possible_cpus; > >> } > >> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c > >> index 7212fb5f8f..5dfc0d5c43 100644 > >> --- a/target/loongarch/cpu.c > >> +++ b/target/loongarch/cpu.c > >> @@ -16,6 +16,7 @@ > >> #include "kvm/kvm_loongarch.h" > >> #include "exec/exec-all.h" > >> #include "cpu.h" > >> +#include "hw/qdev-properties.h" > >> #include "internals.h" > >> #include "fpu/softfloat-helpers.h" > >> #include "cpu-csr.h" > >> @@ -780,6 +781,15 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs) > >> } > >> #endif > >> > >> +static Property loongarch_cpu_properties[] = { > >> + DEFINE_PROP_UINT32("phy-id", LoongArchCPU, phy_id, UNSET_PHY_ID), > > > > IIUC, the phy_id is kind of like the x86 apic_id, but the field is completely > > derived from topology, so why do you need to define it as a property and then > > expose it to the user? > The property is wrongly used here. we want to differentiate cold-added > cpus and hot-added cpus. phy_id of cold-added cpus can be set during > power on, however that of hot-added cpus is default -1. why would you need to differentiate them? > > Internal variable phy_id can be set with default value -1 in function > loongarch_cpu_init(), property can be removed. > > Regards > Bibo Mao > > > > Thanks, > > Zhao > > > >
On 2024/11/6 下午6:41, Igor Mammedov wrote: > On Tue, 29 Oct 2024 21:19:15 +0800 > Zhao Liu <zhao1.liu@intel.com> wrote: > >> Hi Bibo, >> >> [snip] >> >>> +In the CPU topology relationship, When we know the ``socket_id`` ``core_id`` >>> +and ``thread_id`` of the CPU, we can calculate its ``arch_id``: >>> + >>> +``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)`` >> >> What's the difference between arch_id and CPU index (CPUState.cpu_index)? > > They might be the same but not necessarily. > arch_id is unique cpu identifier from architecture point of view > (which easily could be sparse and without specific order). > (ex: for x86 it's apic_id, for spapr it's core_id) > > while cpu_index is internal QEMU, that existed before possible_cpus[] > and non-sparse range and depends on order of cpus are created. > For machines that support possible_cpus[], we override default > cpu_index assignment to fit possible_cpus[]. > > It might be nice to get rid of cpu_index in favor of possible_cpus[], > but that would be a lot work probably with no huge benefit > when it comes majority of machines that don't need variable > cpu count. > >> >>> static void virt_init(MachineState *machine) >>> { >>> - LoongArchCPU *lacpu; >>> const char *cpu_model = machine->cpu_type; >>> MemoryRegion *address_space_mem = get_system_memory(); >>> LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine); >>> @@ -1145,7 +1144,7 @@ static void virt_init(MachineState *machine) >>> hwaddr base, size, ram_size = machine->ram_size; >>> const CPUArchIdList *possible_cpus; >>> MachineClass *mc = MACHINE_GET_CLASS(machine); >>> - CPUState *cpu; >>> + Object *cpuobj; >>> >>> if (!cpu_model) { >>> cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); >>> @@ -1164,12 +1163,25 @@ static void virt_init(MachineState *machine) >>> >>> /* Init CPUs */ >>> possible_cpus = mc->possible_cpu_arch_ids(machine); >>> - for (i = 0; i < possible_cpus->len; i++) { >>> - cpu = cpu_create(machine->cpu_type); >>> - cpu->cpu_index = i; >>> - machine->possible_cpus->cpus[i].cpu = cpu; >>> - lacpu = LOONGARCH_CPU(cpu); >>> - lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id; >>> + for (i = 0; i < machine->smp.cpus; i++) { >>> + cpuobj = object_new(machine->cpu_type); >>> + object_property_set_uint(cpuobj, "phy-id", >>> + possible_cpus->cpus[i].arch_id, NULL); >>> + /* >>> + * The CPU in place at the time of machine startup will also enter >>> + * the CPU hot-plug process when it is created, but at this time, >>> + * the GED device has not been created, resulting in exit in the CPU >>> + * hot-plug process, which can avoid the incumbent CPU repeatedly >>> + * applying for resources. >>> + * >>> + * The interrupt resource of the in-place CPU will be requested at >>> + * the current function call virt_irq_init(). >>> + * >>> + * The interrupt resource of the subsequently inserted CPU will be >>> + * requested in the CPU hot-plug process. >>> + */ >>> + qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); >>> + object_unref(cpuobj); >> >> You can use qdev_realize_and_unref(). >> >>> } >>> fdt_add_cpu_nodes(lvms); >>> fdt_add_memory_nodes(machine); >>> @@ -1286,6 +1298,35 @@ static void virt_initfn(Object *obj) >>> virt_flash_create(lvms); >>> } >>> >>> +static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo) >>> +{ >>> + int arch_id, sock_vcpu_num, core_vcpu_num; >>> + >>> + /* >>> + * calculate total logical cpus across socket/core/thread. >>> + * For more information on how to calculate the arch_id, >>> + * you can refer to the CPU Topology chapter of the >>> + * docs/system/loongarch/virt.rst document. >>> + */ >>> + sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores); >>> + core_vcpu_num = topo->core_id * ms->smp.threads; >>> + >>> + /* get vcpu-id(logical cpu index) for this vcpu from this topology */ >>> + arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id; >> >> Looking at the calculations, arch_id looks the same as cpu_index, right? >> >>> + assert(arch_id >= 0 && arch_id < ms->possible_cpus->len); >>> + >>> + return arch_id; >>> +} >>> + >>> +static void virt_get_topo_from_index(MachineState *ms, >>> + LoongArchCPUTopo *topo, int index) >>> +{ >>> + topo->socket_id = index / (ms->smp.cores * ms->smp.threads); >>> + topo->core_id = index / ms->smp.threads % ms->smp.cores; >>> + topo->thread_id = index % ms->smp.threads; >>> +} >>> + >>> static bool memhp_type_supported(DeviceState *dev) >>> { >>> /* we only support pc dimm now */ >>> @@ -1385,8 +1426,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, >>> >>> static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) >>> { >>> - int n; >>> + int n, arch_id; >>> unsigned int max_cpus = ms->smp.max_cpus; >>> + LoongArchCPUTopo topo; >>> >>> if (ms->possible_cpus) { >>> assert(ms->possible_cpus->len == max_cpus); >>> @@ -1397,17 +1439,17 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) >>> sizeof(CPUArchId) * max_cpus); >>> ms->possible_cpus->len = max_cpus; >>> for (n = 0; n < ms->possible_cpus->len; n++) { >>> + virt_get_topo_from_index(ms, &topo, n); >>> + arch_id = virt_get_arch_id_from_topo(ms, &topo); >>> + ms->possible_cpus->cpus[n].vcpus_count = ms->smp.threads; >> >> In include/hw/boards.h, the doc of CPUArchId said: >> >> vcpus_count - number of threads provided by @cpu object >> >> And I undersatnd each element in possible_cpus->cpus[] is mapped to a >> CPU object, so that here vcpus_count should be 1. > > it's arch specific, CPU object in possible_cpus was meant to point to > thread/core/..higher layers in future../ > > For example spapr put's there CPUCore, where vcpus_count can be > 1 > > That is a bit broken though, since CPUCore is not CPUState by any means, > spapr_core_plug() gets away with it only because > core_slot->cpu = CPU(dev) > CPU() is dumb pointer cast. > > Ideally CPUArchId::cpu should be (Object*) to accommodate various > levels of granularity correctly (with dumb cast above it's just > cosmetics though as long as we treat it as Object in non arch > specific code). > >>> ms->possible_cpus->cpus[n].type = ms->cpu_type; >>> - ms->possible_cpus->cpus[n].arch_id = n; >>> - >>> + ms->possible_cpus->cpus[n].arch_id = arch_id; >>> ms->possible_cpus->cpus[n].props.has_socket_id = true; >>> - ms->possible_cpus->cpus[n].props.socket_id = >>> - n / (ms->smp.cores * ms->smp.threads); >>> + ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id; >>> ms->possible_cpus->cpus[n].props.has_core_id = true; >>> - ms->possible_cpus->cpus[n].props.core_id = >>> - n / ms->smp.threads % ms->smp.cores; >>> + ms->possible_cpus->cpus[n].props.core_id = topo.core_id; >>> ms->possible_cpus->cpus[n].props.has_thread_id = true; >>> - ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads; >>> + ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id; >>> } >>> return ms->possible_cpus; >>> } >>> diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c >>> index 7212fb5f8f..5dfc0d5c43 100644 >>> --- a/target/loongarch/cpu.c >>> +++ b/target/loongarch/cpu.c >>> @@ -16,6 +16,7 @@ >>> #include "kvm/kvm_loongarch.h" >>> #include "exec/exec-all.h" >>> #include "cpu.h" >>> +#include "hw/qdev-properties.h" >>> #include "internals.h" >>> #include "fpu/softfloat-helpers.h" >>> #include "cpu-csr.h" >>> @@ -780,6 +781,15 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs) >>> } >>> #endif >>> >>> +static Property loongarch_cpu_properties[] = { >>> + DEFINE_PROP_UINT32("phy-id", LoongArchCPU, phy_id, UNSET_PHY_ID), >> >> IIUC, the phy_id is kind of like the x86 apic_id, but the field is completely >> derived from topology, so why do you need to define it as a property and then >> expose it to the user? > > for x86 we do expose apic_id as a property as well, partly from historical pov > but also it's better to access cpu fields via properties from outside of > CPU object than directly poke inside of CPU structure from outside of CPU > (especially if it comes to generic code) yes, accessing fields via properties is better than directly poking internal structure elements. Is there internal property for cpu object? so that internal property is invisible for users. There will be problem if user adds CPU object with apic-id property, for example: qemu-system-x86_64 -display none -no-user-config -m 2048 -nodefaults -monitor stdio -machine pc,accel=kvm,usb=off -smp 1,maxcpus=2 -cpu IvyBridge-IBRS -qmp unix:/tmp/qmp-sock,server=on,wait=off (qemu) device_add id=cpu-2,driver=IvyBridge-IBRS-x86_64-cpu,socket-id=0,core-id=1,thread-id=0,apic-id=100 Error: Invalid CPU [socket: 50, die: 0, module: 0, core: 0, thread: 0] with APIC ID 100, valid index range 0:1 (qemu) device_add id=cpu-2,driver=IvyBridge-IBRS-x86_64-cpu,socket-id=0,core-id=1,thread-id=0,apic-id=0 Error: CPU[0] with APIC ID 0 exists Regards Bibo Mao > >> >> Thanks, >> Zhao >> >>
Hi Igor, > > What's the difference between arch_id and CPU index (CPUState.cpu_index)? > > They might be the same but not necessarily. > arch_id is unique cpu identifier from architecture point of view > (which easily could be sparse and without specific order). > (ex: for x86 it's apic_id, for spapr it's core_id) Yes, I was previously puzzled as to why the core_id of spapr is global, which is completely different from the meaning of core_id in x86. Now, your analogy has made it very clear to me. Thanks! > while cpu_index is internal QEMU, that existed before possible_cpus[] > and non-sparse range and depends on order of cpus are created. > For machines that support possible_cpus[], we override default > cpu_index assignment to fit possible_cpus[]. > > It might be nice to get rid of cpu_index in favor of possible_cpus[], > but that would be a lot work probably with no huge benefit > when it comes majority of machines that don't need variable > cpu count. Thank you! Now I see. > > In include/hw/boards.h, the doc of CPUArchId said: > > > > vcpus_count - number of threads provided by @cpu object > > > > And I undersatnd each element in possible_cpus->cpus[] is mapped to a > > CPU object, so that here vcpus_count should be 1. > > it's arch specific, CPU object in possible_cpus was meant to point to > thread/core/..higher layers in future../ > > For example spapr put's there CPUCore, where vcpus_count can be > 1 > > That is a bit broken though, since CPUCore is not CPUState by any means, > spapr_core_plug() gets away with it only because > core_slot->cpu = CPU(dev) > CPU() is dumb pointer cast. Is it also because of architectural reasons that the smallest granularity supported by spapr can only be the core? > Ideally CPUArchId::cpu should be (Object*) to accommodate various > levels of granularity correctly (with dumb cast above it's just > cosmetics though as long as we treat it as Object in non arch > specific code). Thank you. So, I would like to ask, should the elements in possible_cpus be understood as the smallest granularity supported by hotplug? I want to understand that this reason is unrelated to the loongarch patch, instead I mainly want to continue thinking about my previous qom-topo[*] proposal. I remember your hotplug slides also mentioned larger granularity hotplug, which I understand, for example, allows x86 to support core/socket, etc. (this of course requires core/socket object abstraction). If possible_cpus[] only needs to correspond to the smallest granularity topo object, then it matches my qom-topo proposal quite well, essentially mapping one layer of a complete topology tree (which is built from socket to thread, layer by layer) to possible_cpus[] (actually, this is my design: mapping the thread layer of the x86 topology tree to possible_cpus[]. :) ) Although many years have passed, I still believe that larger granularity hotplug is valuable, especially as hardware includes more and more CPUs. [*]: https://lore.kernel.org/qemu-devel/20240919015533.766754-1-zhao1.liu@intel.com/ [snip] > > IIUC, the phy_id is kind of like the x86 apic_id, but the field is completely > > derived from topology, so why do you need to define it as a property and then > > expose it to the user? > > for x86 we do expose apic_id as a property as well, partly from historical pov > but also it's better to access cpu fields via properties from outside of > CPU object than directly poke inside of CPU structure from outside of CPU > (especially if it comes to generic code) Thank you for your guidance. Similar to Bibo’s question, I also wonder if there is the need for a property that won't be exposed to users. Regards, Zhao
diff --git a/docs/system/loongarch/virt.rst b/docs/system/loongarch/virt.rst index 172fba079e..8daf60785f 100644 --- a/docs/system/loongarch/virt.rst +++ b/docs/system/loongarch/virt.rst @@ -28,6 +28,37 @@ The ``qemu-system-loongarch64`` provides emulation for virt machine. You can specify the machine type ``virt`` and cpu type ``la464``. +CPU Topology +------------ + +The ``LA464`` type CPUs have the concept of Socket Core and Thread. + +For example: + +``-smp 1,maxcpus=M,sockets=S,cores=C,threads=T`` + +The above parameters indicate that the machine has a maximum of ``M`` vCPUs and +``S`` sockets, each socket has ``C`` cores, each core has ``T`` threads, +and each thread corresponds to a vCPU. + +Then ``M`` ``S`` ``C`` ``T`` has the following relationship: + +``M = S * C * T`` + +In the CPU topology relationship, When we know the ``socket_id`` ``core_id`` +and ``thread_id`` of the CPU, we can calculate its ``arch_id``: + +``arch_id = (socket_id * S) + (core_id * C) + (thread_id * T)`` + +Similarly, when we know the ``arch_id`` of the CPU, +we can also get its ``socket_id`` ``core_id`` and ``thread_id``: + +``socket_id = arch_id / (C * T)`` + +``core_id = (arch_id / T) % C`` + +``thread_id = arch_id % T`` + Boot options ------------ diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c index 9a635d1d3d..e138dac510 100644 --- a/hw/loongarch/virt.c +++ b/hw/loongarch/virt.c @@ -1137,7 +1137,6 @@ static void fw_cfg_add_memory(MachineState *ms) static void virt_init(MachineState *machine) { - LoongArchCPU *lacpu; const char *cpu_model = machine->cpu_type; MemoryRegion *address_space_mem = get_system_memory(); LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(machine); @@ -1145,7 +1144,7 @@ static void virt_init(MachineState *machine) hwaddr base, size, ram_size = machine->ram_size; const CPUArchIdList *possible_cpus; MachineClass *mc = MACHINE_GET_CLASS(machine); - CPUState *cpu; + Object *cpuobj; if (!cpu_model) { cpu_model = LOONGARCH_CPU_TYPE_NAME("la464"); @@ -1164,12 +1163,25 @@ static void virt_init(MachineState *machine) /* Init CPUs */ possible_cpus = mc->possible_cpu_arch_ids(machine); - for (i = 0; i < possible_cpus->len; i++) { - cpu = cpu_create(machine->cpu_type); - cpu->cpu_index = i; - machine->possible_cpus->cpus[i].cpu = cpu; - lacpu = LOONGARCH_CPU(cpu); - lacpu->phy_id = machine->possible_cpus->cpus[i].arch_id; + for (i = 0; i < machine->smp.cpus; i++) { + cpuobj = object_new(machine->cpu_type); + object_property_set_uint(cpuobj, "phy-id", + possible_cpus->cpus[i].arch_id, NULL); + /* + * The CPU in place at the time of machine startup will also enter + * the CPU hot-plug process when it is created, but at this time, + * the GED device has not been created, resulting in exit in the CPU + * hot-plug process, which can avoid the incumbent CPU repeatedly + * applying for resources. + * + * The interrupt resource of the in-place CPU will be requested at + * the current function call virt_irq_init(). + * + * The interrupt resource of the subsequently inserted CPU will be + * requested in the CPU hot-plug process. + */ + qdev_realize(DEVICE(cpuobj), NULL, &error_fatal); + object_unref(cpuobj); } fdt_add_cpu_nodes(lvms); fdt_add_memory_nodes(machine); @@ -1286,6 +1298,35 @@ static void virt_initfn(Object *obj) virt_flash_create(lvms); } +static int virt_get_arch_id_from_topo(MachineState *ms, LoongArchCPUTopo *topo) +{ + int arch_id, sock_vcpu_num, core_vcpu_num; + + /* + * calculate total logical cpus across socket/core/thread. + * For more information on how to calculate the arch_id, + * you can refer to the CPU Topology chapter of the + * docs/system/loongarch/virt.rst document. + */ + sock_vcpu_num = topo->socket_id * (ms->smp.threads * ms->smp.cores); + core_vcpu_num = topo->core_id * ms->smp.threads; + + /* get vcpu-id(logical cpu index) for this vcpu from this topology */ + arch_id = (sock_vcpu_num + core_vcpu_num) + topo->thread_id; + + assert(arch_id >= 0 && arch_id < ms->possible_cpus->len); + + return arch_id; +} + +static void virt_get_topo_from_index(MachineState *ms, + LoongArchCPUTopo *topo, int index) +{ + topo->socket_id = index / (ms->smp.cores * ms->smp.threads); + topo->core_id = index / ms->smp.threads % ms->smp.cores; + topo->thread_id = index % ms->smp.threads; +} + static bool memhp_type_supported(DeviceState *dev) { /* we only support pc dimm now */ @@ -1385,8 +1426,9 @@ static HotplugHandler *virt_get_hotplug_handler(MachineState *machine, static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) { - int n; + int n, arch_id; unsigned int max_cpus = ms->smp.max_cpus; + LoongArchCPUTopo topo; if (ms->possible_cpus) { assert(ms->possible_cpus->len == max_cpus); @@ -1397,17 +1439,17 @@ static const CPUArchIdList *virt_possible_cpu_arch_ids(MachineState *ms) sizeof(CPUArchId) * max_cpus); ms->possible_cpus->len = max_cpus; for (n = 0; n < ms->possible_cpus->len; n++) { + virt_get_topo_from_index(ms, &topo, n); + arch_id = virt_get_arch_id_from_topo(ms, &topo); + ms->possible_cpus->cpus[n].vcpus_count = ms->smp.threads; ms->possible_cpus->cpus[n].type = ms->cpu_type; - ms->possible_cpus->cpus[n].arch_id = n; - + ms->possible_cpus->cpus[n].arch_id = arch_id; ms->possible_cpus->cpus[n].props.has_socket_id = true; - ms->possible_cpus->cpus[n].props.socket_id = - n / (ms->smp.cores * ms->smp.threads); + ms->possible_cpus->cpus[n].props.socket_id = topo.socket_id; ms->possible_cpus->cpus[n].props.has_core_id = true; - ms->possible_cpus->cpus[n].props.core_id = - n / ms->smp.threads % ms->smp.cores; + ms->possible_cpus->cpus[n].props.core_id = topo.core_id; ms->possible_cpus->cpus[n].props.has_thread_id = true; - ms->possible_cpus->cpus[n].props.thread_id = n % ms->smp.threads; + ms->possible_cpus->cpus[n].props.thread_id = topo.thread_id; } return ms->possible_cpus; } diff --git a/target/loongarch/cpu.c b/target/loongarch/cpu.c index 7212fb5f8f..5dfc0d5c43 100644 --- a/target/loongarch/cpu.c +++ b/target/loongarch/cpu.c @@ -16,6 +16,7 @@ #include "kvm/kvm_loongarch.h" #include "exec/exec-all.h" #include "cpu.h" +#include "hw/qdev-properties.h" #include "internals.h" #include "fpu/softfloat-helpers.h" #include "cpu-csr.h" @@ -780,6 +781,15 @@ static int64_t loongarch_cpu_get_arch_id(CPUState *cs) } #endif +static Property loongarch_cpu_properties[] = { + DEFINE_PROP_UINT32("phy-id", LoongArchCPU, phy_id, UNSET_PHY_ID), + DEFINE_PROP_INT32("socket-id", LoongArchCPU, socket_id, 0), + DEFINE_PROP_INT32("core-id", LoongArchCPU, core_id, 0), + DEFINE_PROP_INT32("thread-id", LoongArchCPU, thread_id, 0), + DEFINE_PROP_INT32("node-id", LoongArchCPU, node_id, CPU_UNSET_NUMA_NODE_ID), + DEFINE_PROP_END_OF_LIST() +}; + static void loongarch_cpu_class_init(ObjectClass *c, void *data) { LoongArchCPUClass *lacc = LOONGARCH_CPU_CLASS(c); @@ -787,6 +797,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data) DeviceClass *dc = DEVICE_CLASS(c); ResettableClass *rc = RESETTABLE_CLASS(c); + device_class_set_props(dc, loongarch_cpu_properties); device_class_set_parent_realize(dc, loongarch_cpu_realizefn, &lacc->parent_realize); resettable_class_set_parent_phases(rc, NULL, loongarch_cpu_reset_hold, NULL, @@ -811,6 +822,7 @@ static void loongarch_cpu_class_init(ObjectClass *c, void *data) #ifdef CONFIG_TCG cc->tcg_ops = &loongarch_tcg_ops; #endif + dc->user_creatable = true; } static const gchar *loongarch32_gdb_arch_name(CPUState *cs) diff --git a/target/loongarch/cpu.h b/target/loongarch/cpu.h index 6c41fafb70..fb50cbbeee 100644 --- a/target/loongarch/cpu.h +++ b/target/loongarch/cpu.h @@ -19,6 +19,12 @@ #include "cpu-csr.h" #include "cpu-qom.h" +/* + * CPU can't have 0xFFFFFFFF physical ID, use that value to distinguish + * that physical ID hasn't been set yet + */ +#define UNSET_PHY_ID 0xFFFFFFFF + #define IOCSRF_TEMP 0 #define IOCSRF_NODECNT 1 #define IOCSRF_MSI 2 @@ -369,6 +375,12 @@ typedef struct CPUArchState { #endif } CPULoongArchState; +typedef struct LoongArchCPUTopo { + int32_t socket_id; /* socket-id of this VCPU */ + int32_t core_id; /* core-id of this VCPU */ + int32_t thread_id; /* thread-id of this VCPU */ +} LoongArchCPUTopo; + /** * LoongArchCPU: * @env: #CPULoongArchState @@ -381,6 +393,10 @@ struct ArchCPU { CPULoongArchState env; QEMUTimer timer; uint32_t phy_id; + int32_t socket_id; /* socket-id of this VCPU */ + int32_t core_id; /* core-id of this VCPU */ + int32_t thread_id; /* thread-id of this VCPU */ + int32_t node_id; /* NUMA node of this VCPU */ /* 'compatible' string for this CPU for Linux device trees */ const char *dtb_compatible; @@ -399,6 +415,7 @@ struct LoongArchCPUClass { CPUClass parent_class; DeviceRealize parent_realize; + DeviceUnrealize parent_unrealize; ResettablePhases parent_phases; };
Add topological relationship for Loongarch VCPU and initialize topology member variables, the topo information includes socket-id, core-id and thread-id. And its physical cpu id is calculated from topo information. Co-developed-by: Xianglai Li <lixianglai@loongson.cn> Signed-off-by: Bibo Mao <maobibo@loongson.cn> --- docs/system/loongarch/virt.rst | 31 ++++++++++++++ hw/loongarch/virt.c | 74 ++++++++++++++++++++++++++-------- target/loongarch/cpu.c | 12 ++++++ target/loongarch/cpu.h | 17 ++++++++ 4 files changed, 118 insertions(+), 16 deletions(-)