Message ID | 20241112021738.1952851-4-maobibo@loongson.cn |
---|---|
State | New |
Headers | show |
Series | hw/loongarch/virt: Add cpu hotplug support | expand |
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. > > 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") >
On 2024/11/22 下午9:45, Igor Mammedov wrote: > 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) Got it. I have further understanding with CPU object, I do not notice this before. And thanks for your kindly help. Regards Bibo Mao > > 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") >>
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. > >> >> 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. lapic is created when x86 cpu object is realized, there is no lapic on LoongArch. One mechanism need be used to notify irqchip driver to setup interrupt routing for multiple processors when CPU is added. How about adding HOTPLUG interface in irqchip driver, notifying irqchip driver when cpu is added? The sample code is shown at website https://lore.kernel.org/qemu-devel/20241128021024.662057-5-maobibo@loongson.cn/T/#u If so, qdev_connect_gpio_out and cs->cpu_index will be removed, just hotplug_handler_plug() notification to irqchip driver is used such as hotplug_handler_plug(HOTPLUG_HANDLER(lvms->ipi), cs, &local_err); 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") >
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")
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(-)