Re: [RFC PATCH v7 20/29] hw/loongarch: Add irq hierarchy for the system

[Richard Henderson <richard.henderson@linaro.org>]

On 3/28/22 06:57, Xiaojuan Yang wrote:
> +static struct DeviceState *ipi, *extioi;

Static variables can't be right.
These need to be somewhere else, or ...

> @@ -107,12 +115,101 @@ static void loongarch_cpu_init(LoongArchCPU *la_cpu, int cpu_num)
>                             NULL, "iocsr_misc", IOCSR_MEM_SIZE);
>   
>       memory_region_add_subregion(&env->system_iocsr, 0, &env->iocsr_mem);
> +    /* ipi memory region */
> +    ipi_addr = SMP_IPI_MAILBOX + cpu_num * 0x100;
> +    memory_region_add_subregion(&env->system_iocsr, ipi_addr,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(ipi),
> +                                cpu_num));
> +    /* extioi memory region */
> +    memory_region_add_subregion(&env->system_iocsr, APIC_BASE,
> +                                sysbus_mmio_get_region(SYS_BUS_DEVICE(extioi),
> +                                cpu_num));

... this code needs to be moved somewhere else.

> +static void loongarch_irq_init(LoongArchMachineState *lams,
> +                               DeviceState *ipi, DeviceState *extioi)
> +{
> +    MachineState *ms = MACHINE(lams);
> +    DeviceState *pch_pic, *pch_msi, *cpudev;
> +
> +    SysBusDevice *d;
> +    int cpu, pin, i;
> +
> +    for (cpu = 0; cpu < ms->smp.cpus; cpu++) {
> +        cpudev = DEVICE(qemu_get_cpu(cpu));
> +        /* connect ipi irq to cpu irq */
> +        qdev_connect_gpio_out(ipi, cpu, qdev_get_gpio_in(cpudev, IRQ_IPI));
> +    }
> +
> +    for (i = 0; i < EXTIOI_IRQS; i++) {
> +        sysbus_connect_irq(SYS_BUS_DEVICE(extioi),
> +                           i, qdev_get_gpio_in(extioi, i));
> +    }

Um... connecting extioi to itself?

I think that graphic that you used in the description of patch 15 belongs here as a comment.

> diff --git a/include/hw/pci-host/ls7a.h b/include/hw/pci-host/ls7a.h
> new file mode 100644
> index 0000000000..bf80e99ce1
> --- /dev/null
> +++ b/include/hw/pci-host/ls7a.h
> @@ -0,0 +1,30 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * QEMU LoongArch CPU
> + *
> + * Copyright (c) 2021 Loongson Technology Corporation Limited
> + */
> +
> +#ifndef HW_LS7A_H
> +#define HW_LS7A_H
> +
> +#include "hw/pci/pci.h"
> +#include "hw/pci/pcie_host.h"
> +#include "hw/pci-host/pam.h"
> +#include "qemu/units.h"
> +#include "qemu/range.h"
> +#include "qom/object.h"
> +
> +#define LS7A_PCH_REG_BASE       0x10000000UL
> +#define LS7A_IOAPIC_REG_BASE    (LS7A_PCH_REG_BASE)
> +#define LS7A_PCH_MSI_ADDR_LOW   0x2FF00000UL
> +
> +/*
> + * According to the kernel pch irq start from 64 offset
> + * 0 ~ 16 irqs used for non-pci device while 16 ~ 64 irqs
> + * used for pci device.
> + */
> +#define PCH_PIC_IRQ_OFFSET      64
> +#define LS7A_DEVICE_IRQS        16
> +#define LS7A_PCI_IRQS           48
> +#endif

Why is this file in this patch?
It seems like it should be in either patch 17 or 18?


r~