| Message ID | 20220425091027.2877892-34-yangxiaojuan@loongson.cn (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Add LoongArch softmmu support | expand |
On 4/25/22 02:10, Xiaojuan Yang wrote: > +static void loongarch_msi_mem_write(void *opaque, hwaddr addr, > + uint64_t val, unsigned size) > +{ > + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); > + int irq_num = val & 0xff; > + > + trace_loongarch_msi_set_irq(irq_num); > + qemu_set_irq(s->pch_msi_irq[irq_num - PCH_PIC_IRQ_NUM], 1); > +} This needs to be bounded properly -- irq_num >= PCH_PIC_IRQ_NUM -- since 'val' is under the control of the guest. > +static void pch_msi_irq_handler(void *opaque, int irq, int level) > +{ > + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); > + > + qemu_set_irq(s->pch_msi_irq[irq], level); > +} You should be able to connect the gpio lines directly, rather than having a pass-through function like this. I think this is sysbus_pass_irq. r~
On 27/04/2022 03:01, Richard Henderson wrote: > On 4/25/22 02:10, Xiaojuan Yang wrote: >> +static void loongarch_msi_mem_write(void *opaque, hwaddr addr, >> + uint64_t val, unsigned size) >> +{ >> + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); >> + int irq_num = val & 0xff; >> + >> + trace_loongarch_msi_set_irq(irq_num); >> + qemu_set_irq(s->pch_msi_irq[irq_num - PCH_PIC_IRQ_NUM], 1); >> +} > > This needs to be bounded properly -- irq_num >= PCH_PIC_IRQ_NUM -- since 'val' is > under the control of the guest. > > >> +static void pch_msi_irq_handler(void *opaque, int irq, int level) >> +{ >> + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); >> + >> + qemu_set_irq(s->pch_msi_irq[irq], level); >> +} > > You should be able to connect the gpio lines directly, rather than having a > pass-through function like this. I think this is sysbus_pass_irq. The *_pass() functions don't work for this case because they directly alias the gpios onto another device i.e. an input still remains an input on the target device, rather than becoming an output. So I think this is still required here. However your comment above about PCH_PIC_IRQ_NUM has made me realise that it is possible to improve the wiring of the IRQs here, so I'll follow up with replies on a few more patches separately. ATB, Mark.
On 25/04/2022 10:10, Xiaojuan Yang wrote: > This patch realize PCH-MSI interrupt controller. > > Signed-off-by: Xiaojuan Yang <yangxiaojuan@loongson.cn> > Signed-off-by: Song Gao <gaosong@loongson.cn> > Reviewed-by: Richard Henderson <richard.henderson@linaro.org> > --- > hw/intc/Kconfig | 5 ++ > hw/intc/loongarch_pch_msi.c | 75 +++++++++++++++++++++++++++++ > hw/intc/meson.build | 1 + > hw/intc/trace-events | 3 ++ > hw/loongarch/Kconfig | 1 + > include/hw/intc/loongarch_pch_msi.h | 20 ++++++++ > 6 files changed, 105 insertions(+) > create mode 100644 hw/intc/loongarch_pch_msi.c > create mode 100644 include/hw/intc/loongarch_pch_msi.h > > diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig > index 1fbba2e728..71c04c328e 100644 > --- a/hw/intc/Kconfig > +++ b/hw/intc/Kconfig > @@ -91,3 +91,8 @@ config LOONGARCH_IPI > config LOONGARCH_PCH_PIC > bool > select UNIMP > + > +config LOONGARCH_PCH_MSI > + select MSI_NONBROKEN > + bool > + select UNIMP > diff --git a/hw/intc/loongarch_pch_msi.c b/hw/intc/loongarch_pch_msi.c > new file mode 100644 > index 0000000000..57a894f3e5 > --- /dev/null > +++ b/hw/intc/loongarch_pch_msi.c > @@ -0,0 +1,75 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * QEMU Loongson 7A1000 msi interrupt controller. > + * > + * Copyright (C) 2021 Loongson Technology Corporation Limited > + */ > + > +#include "qemu/osdep.h" > +#include "hw/sysbus.h" > +#include "hw/irq.h" > +#include "hw/intc/loongarch_pch_msi.h" > +#include "hw/intc/loongarch_pch_pic.h" > +#include "hw/pci/msi.h" > +#include "hw/misc/unimp.h" > +#include "migration/vmstate.h" > +#include "trace.h" > + > +static uint64_t loongarch_msi_mem_read(void *opaque, hwaddr addr, unsigned size) > +{ > + return 0; > +} > + > +static void loongarch_msi_mem_write(void *opaque, hwaddr addr, > + uint64_t val, unsigned size) > +{ > + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); > + int irq_num = val & 0xff; > + > + trace_loongarch_msi_set_irq(irq_num); > + qemu_set_irq(s->pch_msi_irq[irq_num - PCH_PIC_IRQ_NUM], 1); > +} This really looks wrong to me: the PCI-MSI device has PCH_PIC_IRQ_NUM IRQs which are wired up to EIOINTC with an offset of PCH_MSI_IRQ_START already. What you're saying here is that you're manually setting the PCI-MSI IRQs using their EIOINTC IRQ number instead of their input IRQ number? I think what you would want here is just: assert(irq_num < PCH_MSI_IRQ_NUM); trace_loongarch_msi_set_irq(irq_num); qemu_set_irq(s->pch_msi_irq[irq_num], 1); Otherwise the PCH-MSI device has awareness of IRQ hierarchy outside of itself. Presumably this is some kind of testing interface? > +static const MemoryRegionOps loongarch_pch_msi_ops = { > + .read = loongarch_msi_mem_read, > + .write = loongarch_msi_mem_write, > + .endianness = DEVICE_LITTLE_ENDIAN, > +}; > + > +static void pch_msi_irq_handler(void *opaque, int irq, int level) > +{ > + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); > + > + qemu_set_irq(s->pch_msi_irq[irq], level); > +} > + > +static void loongarch_pch_msi_init(Object *obj) > +{ > + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(obj); > + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); > + int i; > + > + memory_region_init_io(&s->msi_mmio, obj, &loongarch_pch_msi_ops, > + s, TYPE_LOONGARCH_PCH_MSI, 0x8); > + sysbus_init_mmio(sbd, &s->msi_mmio); > + msi_nonbroken = true; > + > + for (i = 0; i < PCH_MSI_IRQ_NUM; i++) { > + sysbus_init_irq(sbd, &s->pch_msi_irq[i]); > + } > + qdev_init_gpio_in(DEVICE(obj), pch_msi_irq_handler, PCH_MSI_IRQ_NUM); > +} Again I'd simply use GPIOs instead of sysbus_init_irq() here, similar to the example given in my reply to the PCH-PIC patch. > +static const TypeInfo loongarch_pch_msi_info = { > + .name = TYPE_LOONGARCH_PCH_MSI, > + .parent = TYPE_SYS_BUS_DEVICE, > + .instance_size = sizeof(LoongArchPCHMSI), > + .instance_init = loongarch_pch_msi_init, > +}; > + > +static void loongarch_pch_msi_register_types(void) > +{ > + type_register_static(&loongarch_pch_msi_info); > +} > + > +type_init(loongarch_pch_msi_register_types) > diff --git a/hw/intc/meson.build b/hw/intc/meson.build > index 960ce81a92..77a30cec33 100644 > --- a/hw/intc/meson.build > +++ b/hw/intc/meson.build > @@ -64,3 +64,4 @@ specific_ss.add(when: 'CONFIG_GOLDFISH_PIC', if_true: files('goldfish_pic.c')) > specific_ss.add(when: 'CONFIG_M68K_IRQC', if_true: files('m68k_irqc.c')) > specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: files('loongarch_ipi.c')) > specific_ss.add(when: 'CONFIG_LOONGARCH_PCH_PIC', if_true: files('loongarch_pch_pic.c')) > +specific_ss.add(when: 'CONFIG_LOONGARCH_PCH_MSI', if_true: files('loongarch_pch_msi.c')) > diff --git a/hw/intc/trace-events b/hw/intc/trace-events > index 1f343676ee..8bcc1b6992 100644 > --- a/hw/intc/trace-events > +++ b/hw/intc/trace-events > @@ -300,3 +300,6 @@ loongarch_pch_pic_high_readw(unsigned size, uint32_t addr, unsigned long val) "s > loongarch_pch_pic_high_writew(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 > loongarch_pch_pic_readb(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 > loongarch_pch_pic_writeb(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 > + > +# loongarch_pch_msi.c > +loongarch_msi_set_irq(int irq_num) "set msi irq %d" > diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig > index 2df45f7e8f..d814fc6103 100644 > --- a/hw/loongarch/Kconfig > +++ b/hw/loongarch/Kconfig > @@ -4,3 +4,4 @@ config LOONGARCH_VIRT > select PCI_EXPRESS_GENERIC_BRIDGE > select LOONGARCH_IPI > select LOONGARCH_PCH_PIC > + select LOONGARCH_PCH_MSI > diff --git a/include/hw/intc/loongarch_pch_msi.h b/include/hw/intc/loongarch_pch_msi.h > new file mode 100644 > index 0000000000..f668bfca7a > --- /dev/null > +++ b/include/hw/intc/loongarch_pch_msi.h > @@ -0,0 +1,20 @@ > +/* SPDX-License-Identifier: GPL-2.0-or-later */ > +/* > + * LoongArch 7A1000 I/O interrupt controller definitions > + * > + * Copyright (C) 2021 Loongson Technology Corporation Limited > + */ > + > +#define TYPE_LOONGARCH_PCH_MSI "loongarch_pch_msi" > +OBJECT_DECLARE_SIMPLE_TYPE(LoongArchPCHMSI, LOONGARCH_PCH_MSI) > + > +/* Msi irq start start from 64 to 255 */ > +#define PCH_MSI_IRQ_START 64 > +#define PCH_MSI_IRQ_END 255 > +#define PCH_MSI_IRQ_NUM 192 > + > +struct LoongArchPCHMSI { > + SysBusDevice parent_obj; > + qemu_irq pch_msi_irq[PCH_MSI_IRQ_NUM]; > + MemoryRegion msi_mmio; > +}; ATB, Mark.
diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig index 1fbba2e728..71c04c328e 100644 --- a/hw/intc/Kconfig +++ b/hw/intc/Kconfig @@ -91,3 +91,8 @@ config LOONGARCH_IPI config LOONGARCH_PCH_PIC bool select UNIMP + +config LOONGARCH_PCH_MSI + select MSI_NONBROKEN + bool + select UNIMP diff --git a/hw/intc/loongarch_pch_msi.c b/hw/intc/loongarch_pch_msi.c new file mode 100644 index 0000000000..57a894f3e5 --- /dev/null +++ b/hw/intc/loongarch_pch_msi.c @@ -0,0 +1,75 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * QEMU Loongson 7A1000 msi interrupt controller. + * + * Copyright (C) 2021 Loongson Technology Corporation Limited + */ + +#include "qemu/osdep.h" +#include "hw/sysbus.h" +#include "hw/irq.h" +#include "hw/intc/loongarch_pch_msi.h" +#include "hw/intc/loongarch_pch_pic.h" +#include "hw/pci/msi.h" +#include "hw/misc/unimp.h" +#include "migration/vmstate.h" +#include "trace.h" + +static uint64_t loongarch_msi_mem_read(void *opaque, hwaddr addr, unsigned size) +{ + return 0; +} + +static void loongarch_msi_mem_write(void *opaque, hwaddr addr, + uint64_t val, unsigned size) +{ + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); + int irq_num = val & 0xff; + + trace_loongarch_msi_set_irq(irq_num); + qemu_set_irq(s->pch_msi_irq[irq_num - PCH_PIC_IRQ_NUM], 1); +} + +static const MemoryRegionOps loongarch_pch_msi_ops = { + .read = loongarch_msi_mem_read, + .write = loongarch_msi_mem_write, + .endianness = DEVICE_LITTLE_ENDIAN, +}; + +static void pch_msi_irq_handler(void *opaque, int irq, int level) +{ + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(opaque); + + qemu_set_irq(s->pch_msi_irq[irq], level); +} + +static void loongarch_pch_msi_init(Object *obj) +{ + LoongArchPCHMSI *s = LOONGARCH_PCH_MSI(obj); + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + int i; + + memory_region_init_io(&s->msi_mmio, obj, &loongarch_pch_msi_ops, + s, TYPE_LOONGARCH_PCH_MSI, 0x8); + sysbus_init_mmio(sbd, &s->msi_mmio); + msi_nonbroken = true; + + for (i = 0; i < PCH_MSI_IRQ_NUM; i++) { + sysbus_init_irq(sbd, &s->pch_msi_irq[i]); + } + qdev_init_gpio_in(DEVICE(obj), pch_msi_irq_handler, PCH_MSI_IRQ_NUM); +} + +static const TypeInfo loongarch_pch_msi_info = { + .name = TYPE_LOONGARCH_PCH_MSI, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(LoongArchPCHMSI), + .instance_init = loongarch_pch_msi_init, +}; + +static void loongarch_pch_msi_register_types(void) +{ + type_register_static(&loongarch_pch_msi_info); +} + +type_init(loongarch_pch_msi_register_types) diff --git a/hw/intc/meson.build b/hw/intc/meson.build index 960ce81a92..77a30cec33 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -64,3 +64,4 @@ specific_ss.add(when: 'CONFIG_GOLDFISH_PIC', if_true: files('goldfish_pic.c')) specific_ss.add(when: 'CONFIG_M68K_IRQC', if_true: files('m68k_irqc.c')) specific_ss.add(when: 'CONFIG_LOONGARCH_IPI', if_true: files('loongarch_ipi.c')) specific_ss.add(when: 'CONFIG_LOONGARCH_PCH_PIC', if_true: files('loongarch_pch_pic.c')) +specific_ss.add(when: 'CONFIG_LOONGARCH_PCH_MSI', if_true: files('loongarch_pch_msi.c')) diff --git a/hw/intc/trace-events b/hw/intc/trace-events index 1f343676ee..8bcc1b6992 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -300,3 +300,6 @@ loongarch_pch_pic_high_readw(unsigned size, uint32_t addr, unsigned long val) "s loongarch_pch_pic_high_writew(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 loongarch_pch_pic_readb(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 loongarch_pch_pic_writeb(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 + +# loongarch_pch_msi.c +loongarch_msi_set_irq(int irq_num) "set msi irq %d" diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig index 2df45f7e8f..d814fc6103 100644 --- a/hw/loongarch/Kconfig +++ b/hw/loongarch/Kconfig @@ -4,3 +4,4 @@ config LOONGARCH_VIRT select PCI_EXPRESS_GENERIC_BRIDGE select LOONGARCH_IPI select LOONGARCH_PCH_PIC + select LOONGARCH_PCH_MSI diff --git a/include/hw/intc/loongarch_pch_msi.h b/include/hw/intc/loongarch_pch_msi.h new file mode 100644 index 0000000000..f668bfca7a --- /dev/null +++ b/include/hw/intc/loongarch_pch_msi.h @@ -0,0 +1,20 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * LoongArch 7A1000 I/O interrupt controller definitions + * + * Copyright (C) 2021 Loongson Technology Corporation Limited + */ + +#define TYPE_LOONGARCH_PCH_MSI "loongarch_pch_msi" +OBJECT_DECLARE_SIMPLE_TYPE(LoongArchPCHMSI, LOONGARCH_PCH_MSI) + +/* Msi irq start start from 64 to 255 */ +#define PCH_MSI_IRQ_START 64 +#define PCH_MSI_IRQ_END 255 +#define PCH_MSI_IRQ_NUM 192 + +struct LoongArchPCHMSI { + SysBusDevice parent_obj; + qemu_irq pch_msi_irq[PCH_MSI_IRQ_NUM]; + MemoryRegion msi_mmio; +};