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
