On Tue, 12 Nov 2024 10:17:35 +0800
Bibo Mao <maob...@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 <maob...@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")
