Here generic function virt_init_cpu_irq() is added to init interrupt
pin of CPU object, IPI and extioi interrupt controllers are connected
to interrupt pin of CPU object.
The generic function can be used to both cold-plug and hot-plug CPUs.
Signed-off-by: Bibo Mao <maobibo@loongson.cn>
---
hw/loongarch/virt.c | 78 ++++++++++++++++++++++++-------------
include/hw/loongarch/virt.h | 2 +
2 files changed, 53 insertions(+), 27 deletions(-)
diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
index b6b616d278..621380e2b3 100644
--- a/hw/loongarch/virt.c
+++ b/hw/loongarch/virt.c
@@ -58,6 +58,20 @@ static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
return true;
}
+static CPUState *virt_get_cpu(MachineState *ms, int index)
+{
+ MachineClass *mc = MACHINE_GET_CLASS(ms);
+ const CPUArchIdList *possible_cpus;
+
+ /* Init CPUs */
+ possible_cpus = mc->possible_cpu_arch_ids(ms);
+ if (index < 0 || index >= possible_cpus->len) {
+ return NULL;
+ }
+
+ return possible_cpus->cpus[index].cpu;
+}
+
static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
void *opaque, Error **errp)
{
@@ -365,7 +379,7 @@ static void create_fdt(LoongArchVirtMachineState *lvms)
static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
{
int num;
- const MachineState *ms = MACHINE(lvms);
+ MachineState *ms = MACHINE(lvms);
int smp_cpus = ms->smp.cpus;
qemu_fdt_add_subnode(ms->fdt, "/cpus");
@@ -375,7 +389,7 @@ static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
/* cpu nodes */
for (num = smp_cpus - 1; num >= 0; num--) {
char *nodename = g_strdup_printf("/cpus/cpu@%d", num);
- LoongArchCPU *cpu = LOONGARCH_CPU(qemu_get_cpu(num));
+ LoongArchCPU *cpu = LOONGARCH_CPU(virt_get_cpu(ms, num));
CPUState *cs = CPU(cpu);
qemu_fdt_add_subnode(ms->fdt, nodename);
@@ -783,16 +797,42 @@ static void virt_devices_init(DeviceState *pch_pic,
lvms->platform_bus_dev = create_platform_bus(pch_pic);
}
+static void virt_init_cpu_irq(MachineState *ms, CPUState *cs)
+{
+ LoongArchCPU *cpu = LOONGARCH_CPU(cs);
+ CPULoongArchState *env;
+ LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(ms);
+ int pin;
+
+ if (!lvms->ipi || !lvms->extioi) {
+ return;
+ }
+
+ env = &(cpu->env);
+ env->address_space_iocsr = &lvms->as_iocsr;
+ env->ipistate = lvms->ipi;
+ /* connect ipi irq to cpu irq, logic cpu index used here */
+ qdev_connect_gpio_out(lvms->ipi, cs->cpu_index,
+ qdev_get_gpio_in(DEVICE(cs), IRQ_IPI));
+
+ /*
+ * connect ext irq to the cpu irq
+ * cpu_pin[9:2] <= intc_pin[7:0]
+ */
+ for (pin = 0; pin < LS3A_INTC_IP; pin++) {
+ qdev_connect_gpio_out(lvms->extioi, cs->cpu_index * LS3A_INTC_IP + pin,
+ qdev_get_gpio_in(DEVICE(cs), pin + 2));
+ }
+}
+
static void virt_irq_init(LoongArchVirtMachineState *lvms)
{
MachineState *ms = MACHINE(lvms);
- DeviceState *pch_pic, *pch_msi, *cpudev;
+ DeviceState *pch_pic, *pch_msi;
DeviceState *ipi, *extioi;
SysBusDevice *d;
- LoongArchCPU *lacpu;
- CPULoongArchState *env;
CPUState *cpu_state;
- int cpu, pin, i, start, num;
+ int cpu, i, start, num;
uint32_t cpuintc_phandle, eiointc_phandle, pch_pic_phandle, pch_msi_phandle;
/*
@@ -843,6 +883,7 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
ipi = qdev_new(TYPE_LOONGARCH_IPI);
qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.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,
@@ -853,18 +894,6 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
/* Add cpu interrupt-controller */
fdt_add_cpuic_node(lvms, &cpuintc_phandle);
- for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
- cpu_state = qemu_get_cpu(cpu);
- cpudev = DEVICE(cpu_state);
- lacpu = LOONGARCH_CPU(cpu_state);
- env = &(lacpu->env);
- env->address_space_iocsr = &lvms->as_iocsr;
-
- /* connect ipi irq to cpu irq */
- qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
- env->ipistate = ipi;
- }
-
/* Create EXTIOI device */
extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
@@ -872,6 +901,7 @@ static void virt_irq_init(LoongArchVirtMachineState *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)) {
@@ -879,16 +909,10 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 1));
}
- /*
- * connect ext irq to the cpu irq
- * cpu_pin[9:2] <= intc_pin[7:0]
- */
+ /* Connect irq to cpu, including ipi and extioi irqchip */
for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
- cpudev = DEVICE(qemu_get_cpu(cpu));
- for (pin = 0; pin < LS3A_INTC_IP; pin++) {
- qdev_connect_gpio_out(extioi, (cpu * 8 + pin),
- qdev_get_gpio_in(cpudev, pin + 2));
- }
+ cpu_state = virt_get_cpu(ms, cpu);
+ virt_init_cpu_irq(ms, cpu_state);
}
/* Add Extend I/O Interrupt Controller node */
diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
index 9ba47793ef..260e6bd7cf 100644
--- a/include/hw/loongarch/virt.h
+++ b/include/hw/loongarch/virt.h
@@ -60,6 +60,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")
--
2.39.3On Tue, 12 Nov 2024 10:17:35 +0800
Bibo Mao <maobibo@loongson.cn> wrote:
> Here generic function virt_init_cpu_irq() is added to init interrupt
> pin of CPU object, IPI and extioi interrupt controllers are connected
> to interrupt pin of CPU object.
>
> The generic function can be used to both cold-plug and hot-plug CPUs.
this patch is heavily depends on cpu_index and specific order CPUs
are created.
>
> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> ---
> hw/loongarch/virt.c | 78 ++++++++++++++++++++++++-------------
> include/hw/loongarch/virt.h | 2 +
> 2 files changed, 53 insertions(+), 27 deletions(-)
>
> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> index b6b616d278..621380e2b3 100644
> --- a/hw/loongarch/virt.c
> +++ b/hw/loongarch/virt.c
> @@ -58,6 +58,20 @@ static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
> return true;
> }
>
> +static CPUState *virt_get_cpu(MachineState *ms, int index)
> +{
> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> + const CPUArchIdList *possible_cpus;
> +
> + /* Init CPUs */
> + possible_cpus = mc->possible_cpu_arch_ids(ms);
> + if (index < 0 || index >= possible_cpus->len) {
> + return NULL;
> + }
> +
> + return possible_cpus->cpus[index].cpu;
> +}
instead of adding this helper I'd suggest to try reusing
virt_find_cpu_slot() added in previous patch.
> +
> static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
> void *opaque, Error **errp)
> {
> @@ -365,7 +379,7 @@ static void create_fdt(LoongArchVirtMachineState *lvms)
> static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
> {
> int num;
> - const MachineState *ms = MACHINE(lvms);
> + MachineState *ms = MACHINE(lvms);
> int smp_cpus = ms->smp.cpus;
>
> qemu_fdt_add_subnode(ms->fdt, "/cpus");
> @@ -375,7 +389,7 @@ static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
> /* cpu nodes */
> for (num = smp_cpus - 1; num >= 0; num--) {
loops based on smp_cpus become broken as soon as you have
'-smp x, -device your-cpu,...
since it doesn't take in account '-device' created CPUs.
You likely need to replace such loops to iterate over possible_cpus
(in a separate patch please)
> char *nodename = g_strdup_printf("/cpus/cpu@%d", num);
> - LoongArchCPU *cpu = LOONGARCH_CPU(qemu_get_cpu(num));
> + LoongArchCPU *cpu = LOONGARCH_CPU(virt_get_cpu(ms, num));
> CPUState *cs = CPU(cpu);
>
> qemu_fdt_add_subnode(ms->fdt, nodename);
> @@ -783,16 +797,42 @@ static void virt_devices_init(DeviceState *pch_pic,
> lvms->platform_bus_dev = create_platform_bus(pch_pic);
> }
>
> +static void virt_init_cpu_irq(MachineState *ms, CPUState *cs)
> +{
> + LoongArchCPU *cpu = LOONGARCH_CPU(cs);
> + CPULoongArchState *env;
> + LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(ms);
> + int pin;
> +
> + if (!lvms->ipi || !lvms->extioi) {
> + return;
> + }
> +
> + env = &(cpu->env);
> + env->address_space_iocsr = &lvms->as_iocsr;
> + env->ipistate = lvms->ipi;
> + /* connect ipi irq to cpu irq, logic cpu index used here */
> + qdev_connect_gpio_out(lvms->ipi, cs->cpu_index,
I'd try to avoid using cpu_index (basically internal CPU detail) when
wiring components together. It would be better to implement this the way
the real hw does it.
> + qdev_get_gpio_in(DEVICE(cs), IRQ_IPI));
> +
> + /*
> + * connect ext irq to the cpu irq
> + * cpu_pin[9:2] <= intc_pin[7:0]
> + */
> + for (pin = 0; pin < LS3A_INTC_IP; pin++) {
> + qdev_connect_gpio_out(lvms->extioi, cs->cpu_index * LS3A_INTC_IP + pin,
> + qdev_get_gpio_in(DEVICE(cs), pin + 2));
> + }
> +}
> +
> static void virt_irq_init(LoongArchVirtMachineState *lvms)
> {
> MachineState *ms = MACHINE(lvms);
> - DeviceState *pch_pic, *pch_msi, *cpudev;
> + DeviceState *pch_pic, *pch_msi;
> DeviceState *ipi, *extioi;
> SysBusDevice *d;
> - LoongArchCPU *lacpu;
> - CPULoongArchState *env;
> CPUState *cpu_state;
> - int cpu, pin, i, start, num;
> + int cpu, i, start, num;
> uint32_t cpuintc_phandle, eiointc_phandle, pch_pic_phandle, pch_msi_phandle;
>
> /*
> @@ -843,6 +883,7 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
> ipi = qdev_new(TYPE_LOONGARCH_IPI);
> qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.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,
> @@ -853,18 +894,6 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
> /* Add cpu interrupt-controller */
> fdt_add_cpuic_node(lvms, &cpuintc_phandle);
>
> - for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> - cpu_state = qemu_get_cpu(cpu);
> - cpudev = DEVICE(cpu_state);
> - lacpu = LOONGARCH_CPU(cpu_state);
> - env = &(lacpu->env);
> - env->address_space_iocsr = &lvms->as_iocsr;
> -
> - /* connect ipi irq to cpu irq */
> - qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
> - env->ipistate = ipi;
> - }
> -
> /* Create EXTIOI device */
> extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
> qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
> @@ -872,6 +901,7 @@ static void virt_irq_init(LoongArchVirtMachineState *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)) {
> @@ -879,16 +909,10 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
> sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 1));
> }
>
> - /*
> - * connect ext irq to the cpu irq
> - * cpu_pin[9:2] <= intc_pin[7:0]
> - */
> + /* Connect irq to cpu, including ipi and extioi irqchip */
> for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> - cpudev = DEVICE(qemu_get_cpu(cpu));
> - for (pin = 0; pin < LS3A_INTC_IP; pin++) {
> - qdev_connect_gpio_out(extioi, (cpu * 8 + pin),
> - qdev_get_gpio_in(cpudev, pin + 2));
> - }
> + cpu_state = virt_get_cpu(ms, cpu);
> + virt_init_cpu_irq(ms, cpu_state);
> }
>
> /* Add Extend I/O Interrupt Controller node */
> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> index 9ba47793ef..260e6bd7cf 100644
> --- a/include/hw/loongarch/virt.h
> +++ b/include/hw/loongarch/virt.h
> @@ -60,6 +60,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")
On 2024/11/19 上午12:43, Igor Mammedov wrote:
> On Tue, 12 Nov 2024 10:17:35 +0800
> Bibo Mao <maobibo@loongson.cn> wrote:
>
>> Here generic function virt_init_cpu_irq() is added to init interrupt
>> pin of CPU object, IPI and extioi interrupt controllers are connected
>> to interrupt pin of CPU object.
>>
>> The generic function can be used to both cold-plug and hot-plug CPUs.
>
> this patch is heavily depends on cpu_index and specific order CPUs
> are created.
yes, that is actually one problem with heavy dependency, I will try to
remove the dependency.
>
>>
>> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
>> ---
>> hw/loongarch/virt.c | 78 ++++++++++++++++++++++++-------------
>> include/hw/loongarch/virt.h | 2 +
>> 2 files changed, 53 insertions(+), 27 deletions(-)
>>
>> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
>> index b6b616d278..621380e2b3 100644
>> --- a/hw/loongarch/virt.c
>> +++ b/hw/loongarch/virt.c
>> @@ -58,6 +58,20 @@ static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
>> return true;
>> }
>>
>> +static CPUState *virt_get_cpu(MachineState *ms, int index)
>> +{
>> + MachineClass *mc = MACHINE_GET_CLASS(ms);
>> + const CPUArchIdList *possible_cpus;
>> +
>> + /* Init CPUs */
>> + possible_cpus = mc->possible_cpu_arch_ids(ms);
>> + if (index < 0 || index >= possible_cpus->len) {
>> + return NULL;
>> + }
>> +
>> + return possible_cpus->cpus[index].cpu;
>> +}
>
> instead of adding this helper I'd suggest to try reusing
> virt_find_cpu_slot() added in previous patch.
>
>> +
>> static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
>> void *opaque, Error **errp)
>> {
>> @@ -365,7 +379,7 @@ static void create_fdt(LoongArchVirtMachineState *lvms)
>> static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
>> {
>> int num;
>> - const MachineState *ms = MACHINE(lvms);
>> + MachineState *ms = MACHINE(lvms);
>> int smp_cpus = ms->smp.cpus;
>>
>> qemu_fdt_add_subnode(ms->fdt, "/cpus");
>> @@ -375,7 +389,7 @@ static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
>> /* cpu nodes */
>> for (num = smp_cpus - 1; num >= 0; num--) {
>
> loops based on smp_cpus become broken as soon as you have
> '-smp x, -device your-cpu,...
> since it doesn't take in account '-device' created CPUs.
> You likely need to replace such loops to iterate over possible_cpus
> (in a separate patch please)
yes, will do. possible_cpus can be used and virt_get_cpu() is unnecessary.
Interesting, I never create cpu like the method like this, will try this.
'-smp x, -device your-cpu,...'
>
>> char *nodename = g_strdup_printf("/cpus/cpu@%d", num);
>> - LoongArchCPU *cpu = LOONGARCH_CPU(qemu_get_cpu(num));
>> + LoongArchCPU *cpu = LOONGARCH_CPU(virt_get_cpu(ms, num));
>> CPUState *cs = CPU(cpu);
>>
>> qemu_fdt_add_subnode(ms->fdt, nodename);
>> @@ -783,16 +797,42 @@ static void virt_devices_init(DeviceState *pch_pic,
>> lvms->platform_bus_dev = create_platform_bus(pch_pic);
>> }
>>
>> +static void virt_init_cpu_irq(MachineState *ms, CPUState *cs)
>> +{
>> + LoongArchCPU *cpu = LOONGARCH_CPU(cs);
>> + CPULoongArchState *env;
>> + LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(ms);
>> + int pin;
>> +
>> + if (!lvms->ipi || !lvms->extioi) {
>> + return;
>> + }
>> +
>> + env = &(cpu->env);
>> + env->address_space_iocsr = &lvms->as_iocsr;
>> + env->ipistate = lvms->ipi;
>> + /* connect ipi irq to cpu irq, logic cpu index used here */
>> + qdev_connect_gpio_out(lvms->ipi, cs->cpu_index,
> I'd try to avoid using cpu_index (basically internal CPU detail) when
> wiring components together. It would be better to implement this the way
> the real hw does it.
yes, will try to remove this and ipi device realize funciton. When ipi
device is created, it will search possible_cpus and connect to interrupt
pin of supported CPU.
The real hw is same with Interrupt Pin method :(, and there is no
apic-bus or Processor System Bus like x86.
Regards
Bibo Mao
>
>
>> + qdev_get_gpio_in(DEVICE(cs), IRQ_IPI));
>> +
>> + /*
>> + * connect ext irq to the cpu irq
>> + * cpu_pin[9:2] <= intc_pin[7:0]
>> + */
>> + for (pin = 0; pin < LS3A_INTC_IP; pin++) {
>> + qdev_connect_gpio_out(lvms->extioi, cs->cpu_index * LS3A_INTC_IP + pin,
>> + qdev_get_gpio_in(DEVICE(cs), pin + 2));
>> + }
>> +}
>> +
>> static void virt_irq_init(LoongArchVirtMachineState *lvms)
>> {
>> MachineState *ms = MACHINE(lvms);
>> - DeviceState *pch_pic, *pch_msi, *cpudev;
>> + DeviceState *pch_pic, *pch_msi;
>> DeviceState *ipi, *extioi;
>> SysBusDevice *d;
>> - LoongArchCPU *lacpu;
>> - CPULoongArchState *env;
>> CPUState *cpu_state;
>> - int cpu, pin, i, start, num;
>> + int cpu, i, start, num;
>> uint32_t cpuintc_phandle, eiointc_phandle, pch_pic_phandle, pch_msi_phandle;
>>
>> /*
>> @@ -843,6 +883,7 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>> ipi = qdev_new(TYPE_LOONGARCH_IPI);
>> qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.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,
>> @@ -853,18 +894,6 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>> /* Add cpu interrupt-controller */
>> fdt_add_cpuic_node(lvms, &cpuintc_phandle);
>>
>> - for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
>> - cpu_state = qemu_get_cpu(cpu);
>> - cpudev = DEVICE(cpu_state);
>> - lacpu = LOONGARCH_CPU(cpu_state);
>> - env = &(lacpu->env);
>> - env->address_space_iocsr = &lvms->as_iocsr;
>> -
>> - /* connect ipi irq to cpu irq */
>> - qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
>> - env->ipistate = ipi;
>> - }
>> -
>> /* Create EXTIOI device */
>> extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
>> qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
>> @@ -872,6 +901,7 @@ static void virt_irq_init(LoongArchVirtMachineState *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)) {
>> @@ -879,16 +909,10 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
>> sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 1));
>> }
>>
>> - /*
>> - * connect ext irq to the cpu irq
>> - * cpu_pin[9:2] <= intc_pin[7:0]
>> - */
>> + /* Connect irq to cpu, including ipi and extioi irqchip */
>> for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
>> - cpudev = DEVICE(qemu_get_cpu(cpu));
>> - for (pin = 0; pin < LS3A_INTC_IP; pin++) {
>> - qdev_connect_gpio_out(extioi, (cpu * 8 + pin),
>> - qdev_get_gpio_in(cpudev, pin + 2));
>> - }
>> + cpu_state = virt_get_cpu(ms, cpu);
>> + virt_init_cpu_irq(ms, cpu_state);
>> }
>>
>> /* Add Extend I/O Interrupt Controller node */
>> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
>> index 9ba47793ef..260e6bd7cf 100644
>> --- a/include/hw/loongarch/virt.h
>> +++ b/include/hw/loongarch/virt.h
>> @@ -60,6 +60,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")
On Tue, 19 Nov 2024 18:02:54 +0800
bibo mao <maobibo@loongson.cn> wrote:
> On 2024/11/19 上午12:43, Igor Mammedov wrote:
> > On Tue, 12 Nov 2024 10:17:35 +0800
> > Bibo Mao <maobibo@loongson.cn> wrote:
> >
> >> Here generic function virt_init_cpu_irq() is added to init interrupt
> >> pin of CPU object, IPI and extioi interrupt controllers are connected
> >> to interrupt pin of CPU object.
> >>
> >> The generic function can be used to both cold-plug and hot-plug CPUs.
> >
> > this patch is heavily depends on cpu_index and specific order CPUs
> > are created.
> yes, that is actually one problem with heavy dependency, I will try to
> remove the dependency.
> >
> >>
> >> Signed-off-by: Bibo Mao <maobibo@loongson.cn>
> >> ---
> >> hw/loongarch/virt.c | 78 ++++++++++++++++++++++++-------------
> >> include/hw/loongarch/virt.h | 2 +
> >> 2 files changed, 53 insertions(+), 27 deletions(-)
> >>
> >> diff --git a/hw/loongarch/virt.c b/hw/loongarch/virt.c
> >> index b6b616d278..621380e2b3 100644
> >> --- a/hw/loongarch/virt.c
> >> +++ b/hw/loongarch/virt.c
> >> @@ -58,6 +58,20 @@ static bool virt_is_veiointc_enabled(LoongArchVirtMachineState *lvms)
> >> return true;
> >> }
> >>
> >> +static CPUState *virt_get_cpu(MachineState *ms, int index)
> >> +{
> >> + MachineClass *mc = MACHINE_GET_CLASS(ms);
> >> + const CPUArchIdList *possible_cpus;
> >> +
> >> + /* Init CPUs */
> >> + possible_cpus = mc->possible_cpu_arch_ids(ms);
> >> + if (index < 0 || index >= possible_cpus->len) {
> >> + return NULL;
> >> + }
> >> +
> >> + return possible_cpus->cpus[index].cpu;
> >> +}
> >
> > instead of adding this helper I'd suggest to try reusing
> > virt_find_cpu_slot() added in previous patch.
> >
> >> +
> >> static void virt_get_veiointc(Object *obj, Visitor *v, const char *name,
> >> void *opaque, Error **errp)
> >> {
> >> @@ -365,7 +379,7 @@ static void create_fdt(LoongArchVirtMachineState *lvms)
> >> static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
> >> {
> >> int num;
> >> - const MachineState *ms = MACHINE(lvms);
> >> + MachineState *ms = MACHINE(lvms);
> >> int smp_cpus = ms->smp.cpus;
> >>
> >> qemu_fdt_add_subnode(ms->fdt, "/cpus");
> >> @@ -375,7 +389,7 @@ static void fdt_add_cpu_nodes(const LoongArchVirtMachineState *lvms)
> >> /* cpu nodes */
> >> for (num = smp_cpus - 1; num >= 0; num--) {
> >
> > loops based on smp_cpus become broken as soon as you have
> > '-smp x, -device your-cpu,...
> > since it doesn't take in account '-device' created CPUs.
> > You likely need to replace such loops to iterate over possible_cpus
> > (in a separate patch please)
> yes, will do. possible_cpus can be used and virt_get_cpu() is unnecessary.
>
> Interesting, I never create cpu like the method like this, will try this.
> '-smp x, -device your-cpu,...'
that's how target VM could be starred with if cpu were hotpluged on
migration source side.
'-smp x' basically shortcut to series of '-device cpu-foo',
with the only big difference is that the later is created after machine_init
while '-smp x' CPUs are created at machine_init time.
That's the reason to I'm pushing you to move all CPU wiring to plug handlers,
so eventually you would end up with only way of adding CPUs, regardless of
what creates them (-smp or -device/device_add)
Ideally/if possible you should be able to start VM with '-smp 0, -device cpu-foo'
> >
> >> char *nodename = g_strdup_printf("/cpus/cpu@%d", num);
> >> - LoongArchCPU *cpu = LOONGARCH_CPU(qemu_get_cpu(num));
> >> + LoongArchCPU *cpu = LOONGARCH_CPU(virt_get_cpu(ms, num));
> >> CPUState *cs = CPU(cpu);
> >>
> >> qemu_fdt_add_subnode(ms->fdt, nodename);
> >> @@ -783,16 +797,42 @@ static void virt_devices_init(DeviceState *pch_pic,
> >> lvms->platform_bus_dev = create_platform_bus(pch_pic);
> >> }
> >>
> >> +static void virt_init_cpu_irq(MachineState *ms, CPUState *cs)
> >> +{
> >> + LoongArchCPU *cpu = LOONGARCH_CPU(cs);
> >> + CPULoongArchState *env;
> >> + LoongArchVirtMachineState *lvms = LOONGARCH_VIRT_MACHINE(ms);
> >> + int pin;
> >> +
> >> + if (!lvms->ipi || !lvms->extioi) {
> >> + return;
> >> + }
> >> +
> >> + env = &(cpu->env);
> >> + env->address_space_iocsr = &lvms->as_iocsr;
> >> + env->ipistate = lvms->ipi;
> >> + /* connect ipi irq to cpu irq, logic cpu index used here */
> >> + qdev_connect_gpio_out(lvms->ipi, cs->cpu_index,
> > I'd try to avoid using cpu_index (basically internal CPU detail) when
> > wiring components together. It would be better to implement this the way
> > the real hw does it.
> yes, will try to remove this and ipi device realize funciton. When ipi
> device is created, it will search possible_cpus and connect to interrupt
> pin of supported CPU.
>
> The real hw is same with Interrupt Pin method :(, and there is no
> apic-bus or Processor System Bus like x86.
>
> Regards
> Bibo Mao
> >
> >
> >> + qdev_get_gpio_in(DEVICE(cs), IRQ_IPI));
> >> +
> >> + /*
> >> + * connect ext irq to the cpu irq
> >> + * cpu_pin[9:2] <= intc_pin[7:0]
> >> + */
> >> + for (pin = 0; pin < LS3A_INTC_IP; pin++) {
> >> + qdev_connect_gpio_out(lvms->extioi, cs->cpu_index * LS3A_INTC_IP + pin,
> >> + qdev_get_gpio_in(DEVICE(cs), pin + 2));
> >> + }
> >> +}
> >> +
> >> static void virt_irq_init(LoongArchVirtMachineState *lvms)
> >> {
> >> MachineState *ms = MACHINE(lvms);
> >> - DeviceState *pch_pic, *pch_msi, *cpudev;
> >> + DeviceState *pch_pic, *pch_msi;
> >> DeviceState *ipi, *extioi;
> >> SysBusDevice *d;
> >> - LoongArchCPU *lacpu;
> >> - CPULoongArchState *env;
> >> CPUState *cpu_state;
> >> - int cpu, pin, i, start, num;
> >> + int cpu, i, start, num;
> >> uint32_t cpuintc_phandle, eiointc_phandle, pch_pic_phandle, pch_msi_phandle;
> >>
> >> /*
> >> @@ -843,6 +883,7 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
> >> ipi = qdev_new(TYPE_LOONGARCH_IPI);
> >> qdev_prop_set_uint32(ipi, "num-cpu", ms->smp.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,
> >> @@ -853,18 +894,6 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
> >> /* Add cpu interrupt-controller */
> >> fdt_add_cpuic_node(lvms, &cpuintc_phandle);
> >>
> >> - for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> >> - cpu_state = qemu_get_cpu(cpu);
> >> - cpudev = DEVICE(cpu_state);
> >> - lacpu = LOONGARCH_CPU(cpu_state);
> >> - env = &(lacpu->env);
> >> - env->address_space_iocsr = &lvms->as_iocsr;
> >> -
> >> - /* connect ipi irq to cpu irq */
> >> - qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
> >> - env->ipistate = ipi;
> >> - }
> >> -
> >> /* Create EXTIOI device */
> >> extioi = qdev_new(TYPE_LOONGARCH_EXTIOI);
> >> qdev_prop_set_uint32(extioi, "num-cpu", ms->smp.cpus);
> >> @@ -872,6 +901,7 @@ static void virt_irq_init(LoongArchVirtMachineState *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)) {
> >> @@ -879,16 +909,10 @@ static void virt_irq_init(LoongArchVirtMachineState *lvms)
> >> sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi), 1));
> >> }
> >>
> >> - /*
> >> - * connect ext irq to the cpu irq
> >> - * cpu_pin[9:2] <= intc_pin[7:0]
> >> - */
> >> + /* Connect irq to cpu, including ipi and extioi irqchip */
> >> for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> >> - cpudev = DEVICE(qemu_get_cpu(cpu));
> >> - for (pin = 0; pin < LS3A_INTC_IP; pin++) {
> >> - qdev_connect_gpio_out(extioi, (cpu * 8 + pin),
> >> - qdev_get_gpio_in(cpudev, pin + 2));
> >> - }
> >> + cpu_state = virt_get_cpu(ms, cpu);
> >> + virt_init_cpu_irq(ms, cpu_state);
> >> }
> >>
> >> /* Add Extend I/O Interrupt Controller node */
> >> diff --git a/include/hw/loongarch/virt.h b/include/hw/loongarch/virt.h
> >> index 9ba47793ef..260e6bd7cf 100644
> >> --- a/include/hw/loongarch/virt.h
> >> +++ b/include/hw/loongarch/virt.h
> >> @@ -60,6 +60,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")
>
© 2016 - 2024 Red Hat, Inc.