On 22/12/2021 02:38, yangxiaojuan wrote:
Hi, MarkOn 12/18/2021 08:33 AM, Mark Cave-Ayland wrote:On 04/12/2021 12:07, Xiaojuan Yang wrote:This patch realize the PCH-PIC interrupt controller. Signed-off-by: Xiaojuan Yang <yangxiaoj...@loongson.cn> Signed-off-by: Song Gao <gaos...@loongson.cn> --- hw/intc/Kconfig | 4 + hw/intc/loongarch_pch_pic.c | 357 ++++++++++++++++++++++++++++ hw/intc/meson.build | 1 + hw/intc/trace-events | 5 + hw/loongarch/Kconfig | 1 + include/hw/intc/loongarch_pch_pic.h | 61 +++++ 6 files changed, 429 insertions(+) create mode 100644 hw/intc/loongarch_pch_pic.c create mode 100644 include/hw/intc/loongarch_pch_pic.h diff --git a/hw/intc/Kconfig b/hw/intc/Kconfig index 511dcac537..96da13ad1d 100644 --- a/hw/intc/Kconfig +++ b/hw/intc/Kconfig @@ -76,3 +76,7 @@ config M68K_IRQC config LOONGARCH_IPI bool + +config LOONGARCH_PCH_PIC + bool + select UNIMP diff --git a/hw/intc/loongarch_pch_pic.c b/hw/intc/loongarch_pch_pic.c new file mode 100644 index 0000000000..2ede29ceb0 --- /dev/null +++ b/hw/intc/loongarch_pch_pic.c @@ -0,0 +1,357 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * QEMU Loongson 7A1000 I/O interrupt controller. + * + * Copyright (C) 2021 Loongson Technology Corporation Limited + */ + +#include "qemu/osdep.h" +#include "hw/sysbus.h" +#include "hw/loongarch/loongarch.h" +#include "hw/irq.h" +#include "hw/intc/loongarch_pch_pic.h" +#include "migration/vmstate.h" +#include "trace.h" + +#define for_each_set_bit(bit, addr, size) \ + for ((bit) = find_first_bit((addr), (size)); \ + (bit) < (size); \ + (bit) = find_next_bit((addr), (size), (bit) + 1)) + +static void pch_pic_update_irq(loongarch_pch_pic *s, uint64_t mask, int level) +{ + int i; + uint64_t val; + val = mask & s->intirr & (~s->int_mask); + + for_each_set_bit(i, &val, 64) { + if (level == 1) { + if ((s->intisr & (0x1ULL << i)) == 0) { + s->intisr |= 1ULL << i; + qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], 1); + } + } else if (level == 0) { + if (s->intisr & (0x1ULL << i)) { + s->intisr &= ~(0x1ULL << i); + qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], 0); + } + } + } +}The normal pattern would be to use something like: for (i = 0; i < 64; i++) { if (level) { s->intisr |= 1ULL << i; } else { s->intisr &= ~(0x1ULL << i); } qemu_set_irq(s->parent_irq[s->htmsi_vector[i]], level); } Why is it necessary to check the previous value of (s->intisr & (0x1ULL << i)) here?Here check the previous value to avoid Unnecessary write. It seems make things more complicated. I will modify
In general a *_update_irq() function should be fine to propagate the IRQ up to the parent directly: I think this is fine in this case because you are directly manipulating the parent_irq elements rather than using e.g. a priority encoder within this device to raise an IRQ to the CPU. I'm presuming this final prioritisation and delivery is done elsewhere?
+static void pch_pic_irq_handler(void *opaque, int irq, int level) +{ + loongarch_pch_pic *s = LOONGARCH_PCH_PIC(opaque); + + assert(irq < PCH_PIC_IRQ_NUM); + uint64_t mask = 1ULL << irq; + + trace_pch_pic_irq_handler(s->intedge, irq, level); + + if (s->intedge & mask) { + /* Edge triggered */ + if (level) { + if ((s->last_intirr & mask) == 0) { + s->intirr |= mask; + } + s->last_intirr |= mask; + } else { + s->last_intirr &= ~mask; + } + } else { + /* Level triggered */ + if (level) { + s->intirr |= mask; + s->last_intirr |= mask; + } else { + s->intirr &= ~mask; + s->last_intirr &= ~mask; + } + + } + pch_pic_update_irq(s, mask, level); +} + +static uint64_t loongarch_pch_pic_reg_read(void *opaque, hwaddr addr, + unsigned size) +{ + loongarch_pch_pic *s = LOONGARCH_PCH_PIC(opaque); + uint64_t val = 0; + uint32_t offset = addr & 0xfff; + int64_t offset_tmp; + + if (size == 8) { + switch (offset) { + case PCH_PIC_INT_ID_OFFSET: + val = (PCH_PIC_INT_ID_NUM << 32) | PCH_PIC_INT_ID_VAL; + break; + case PCH_PIC_INT_MASK_OFFSET: + val = s->int_mask; + break; + case PCH_PIC_INT_STATUS_OFFSET: + val = s->intisr & (~s->int_mask); + break; + case PCH_PIC_INT_EDGE_OFFSET: + val = s->intedge; + break; + case PCH_PIC_INT_POL_OFFSET: + val = s->int_polarity; + break; + case PCH_PIC_HTMSI_EN_OFFSET...PCH_PIC_HTMSI_EN_END: + val = s->htmsi_en; + break; + case PCH_PIC_AUTO_CTRL0_OFFSET...PCH_PIC_AUTO_CTRL0_END: + case PCH_PIC_AUTO_CTRL1_OFFSET...PCH_PIC_AUTO_CTRL1_END: + break; + default: + break; + } + } else if (size == 4) { + switch (offset) { + case PCH_PIC_INT_ID_OFFSET: + val = PCH_PIC_INT_ID_VAL; + break; + case PCH_PIC_INT_ID_OFFSET + 4: + val = PCH_PIC_INT_ID_NUM; + break; + case PCH_PIC_INT_MASK_OFFSET...PCH_PIC_INT_MASK_END: + val = ldl_p((void *)&s->int_mask + + (offset - PCH_PIC_INT_MASK_OFFSET)); + break; + case PCH_PIC_INT_STATUS_OFFSET...PCH_PIC_INT_STATUS_END: + val = ldl_p((void *)&s->intisr + + (offset - PCH_PIC_INT_STATUS_OFFSET)) & (~s->int_mask); + break; + case PCH_PIC_INT_EDGE_OFFSET...PCH_PIC_INT_EDGE_END: + val = ldl_p((void *)&s->intedge + + (offset - PCH_PIC_INT_EDGE_OFFSET)); + break; + case PCH_PIC_INT_POL_OFFSET...PCH_PIC_INT_POL_END: + val = ldl_p((void *)&s->int_polarity + + (offset - PCH_PIC_INT_POL_OFFSET)); + break; + case PCH_PIC_HTMSI_EN_OFFSET...PCH_PIC_HTMSI_EN_END: + val = ldl_p((void *)&s->htmsi_en + + (offset - PCH_PIC_HTMSI_EN_OFFSET)); + break; + case PCH_PIC_AUTO_CTRL0_OFFSET...PCH_PIC_AUTO_CTRL0_END: + case PCH_PIC_AUTO_CTRL1_OFFSET...PCH_PIC_AUTO_CTRL1_END: + break; + default: + break; + } + } else if (size == 1) { + if (offset >= PCH_PIC_HTMSI_VEC_OFFSET) { + offset_tmp = offset - PCH_PIC_HTMSI_VEC_OFFSET; + if (offset_tmp >= 0 && offset_tmp < 64) { + val = s->htmsi_vector[offset_tmp]; + } + } else if (offset >= PCH_PIC_ROUTE_ENTRY_OFFSET) { + offset_tmp = offset - PCH_PIC_ROUTE_ENTRY_OFFSET; + if (offset_tmp >= 0 && offset_tmp < 64) { + val = s->route_entry[offset_tmp]; + } + } + } + + trace_loongarch_pch_pic_read(size, (uint32_t)addr, val); + return val; +} + +static void loongarch_pch_pic_reg_write(void *opaque, hwaddr addr, + uint64_t data, unsigned size) +{ + loongarch_pch_pic *s = LOONGARCH_PCH_PIC(opaque);Perhaps use LoongArchPCHPIC for the camel case version?OK, all interrupt controller will use the camel case.+ int32_t offset_tmp; + uint32_t offset, old; + offset = addr & 0xfff; + + trace_loongarch_pch_pic_write(size, (uint32_t)addr, data); + + if (size == 8) { + switch (offset) { + case PCH_PIC_INT_MASK_OFFSET: + old = s->int_mask; + s->int_mask = data; + if (old & ~data) { + pch_pic_update_irq(s, (old & ~data), 1); + } else if (~old & data) { + pch_pic_update_irq(s, (~old & data), 0); + } + break; + case PCH_PIC_INT_STATUS_OFFSET: + s->intisr = data; + break; + case PCH_PIC_INT_EDGE_OFFSET: + s->intedge = data; + break; + case PCH_PIC_INT_CLEAR_OFFSET: + s->intirr &= (~(data & s->intedge)); + pch_pic_update_irq(s, data, 0); + s->intisr &= (~data); + break; + case PCH_PIC_INT_POL_OFFSET: + s->int_polarity = data; + break; + case PCH_PIC_HTMSI_EN_OFFSET: + s->htmsi_en = data; + break; + case PCH_PIC_AUTO_CTRL0_OFFSET: + case PCH_PIC_AUTO_CTRL1_OFFSET: + break; + default: + break; + } + } else if (size == 4) { + switch (offset) { + case PCH_PIC_INT_MASK_OFFSET...PCH_PIC_INT_MASK_END: + offset -= PCH_PIC_INT_MASK_OFFSET; + old = ldl_p((void *)&s->int_mask + offset); + stl_p((void *)&s->int_mask + offset, data); + + if (old & ~data) { + pch_pic_update_irq(s, (old & ~data), 1); + } else if (~old & data) { + pch_pic_update_irq(s, (~old & data), 0); + } + break; + case PCH_PIC_INT_STATUS_OFFSET...PCH_PIC_INT_STATUS_END: + stl_p((void *)&s->intisr + (offset - PCH_PIC_INT_STATUS_OFFSET), + data); + break; + case PCH_PIC_INT_EDGE_OFFSET...PCH_PIC_INT_EDGE_END: + stl_p((void *)&s->intedge + (offset - PCH_PIC_INT_EDGE_OFFSET), + data); + break; + case PCH_PIC_INT_CLEAR_OFFSET...PCH_PIC_INT_CLEAR_END: + old = s->intirr & (~(data & s->intedge)); + stl_p((void *)&s->intirr + (offset - PCH_PIC_INT_CLEAR_OFFSET), + old); + pch_pic_update_irq(s, data, 0); + old = s->intisr & (~data); + stl_p((void *)&s->intisr + (offset - PCH_PIC_INT_CLEAR_OFFSET), + old); + break; + case PCH_PIC_INT_POL_OFFSET...PCH_PIC_INT_POL_END: + stl_p((void *)&s->int_polarity + (offset - PCH_PIC_INT_POL_OFFSET), + data); + break; + case PCH_PIC_HTMSI_EN_OFFSET...PCH_PIC_HTMSI_EN_END: + stl_p((void *)&s->htmsi_en + (offset - PCH_PIC_HTMSI_EN_OFFSET), + data); + break; + case PCH_PIC_AUTO_CTRL0_OFFSET...PCH_PIC_AUTO_CTRL0_END: + case PCH_PIC_AUTO_CTRL1_OFFSET...PCH_PIC_AUTO_CTRL1_END: + break; + default: + break; + } + } else if (size == 1) { + if (offset >= PCH_PIC_HTMSI_VEC_OFFSET) { + offset_tmp = offset - PCH_PIC_HTMSI_VEC_OFFSET; + if (offset_tmp >= 0 && offset_tmp < 64) { + s->htmsi_vector[offset_tmp] = (uint8_t)(data & 0xff); + } + } else if (offset >= PCH_PIC_ROUTE_ENTRY_OFFSET) { + offset_tmp = offset - PCH_PIC_ROUTE_ENTRY_OFFSET; + if (offset_tmp >= 0 && offset_tmp < 64) { + s->route_entry[offset_tmp] = (uint8_t)(data & 0xff); + } + } + } +} + +static const MemoryRegionOps loongarch_pch_pic_ops = { + .read = loongarch_pch_pic_reg_read, + .write = loongarch_pch_pic_reg_write, + .valid = { + .min_access_size = 1, + .max_access_size = 8, + }, + .impl = { + .min_access_size = 1, + .max_access_size = 8, + }, + .endianness = DEVICE_LITTLE_ENDIAN, +};It feels like this should be split into 2 separate memory regions: as per before set .min_access_size = 4 for PCH_PIC_INT_MASK_OFFSET ... PCH_PIC_AUTO_CTRL1_END and use uint32_t for the underlying types in loongarch_pch_pic. This leaves a separate memory region for accesses > PCH_PIC_HTMSI_VEC_OFFSET which can be set with .min_access_size = 1 which maps nicely to the underlying uint8_t types. Which means you should be able to access the arrays directly instead of having to use ldl_p() and stl_p().yeah, here the PCH_PIC_HTMSI_VEC_OFFSET region is right in the middle, I want to use a separate memory region to overlap the pch-pic region, is it ok to do like this? Thanks
That should be fine. I'd also use a single container memory region so that you only have to expose a single memory region with sysbus_init_mmio() as below:
+static void loongarch_pch_pic_reset(DeviceState *d)+{ + loongarch_pch_pic *s = LOONGARCH_PCH_PIC(d); + int i; + + s->int_mask = -1ULL; + s->htmsi_en = 0x0; + s->intedge = 0x0; + s->intclr = 0x0; + s->auto_crtl0 = 0x0; + s->auto_crtl1 = 0x0; + for (i = 0; i < 64; i++) { + s->route_entry[i] = 0x1; + s->htmsi_vector[i] = 0x0; + } + s->intirr = 0x0; + s->intisr = 0x0; + s->last_intirr = 0x0; + s->int_polarity = 0x0; +} + +static void loongarch_pch_pic_init(Object *obj) +{ + loongarch_pch_pic *s = LOONGARCH_PCH_PIC(obj); + SysBusDevice *sbd = SYS_BUS_DEVICE(obj); + int i; + + memory_region_init_io(&s->iomem, obj, &loongarch_pch_pic_ops, + s, TYPE_LOONGARCH_PCH_PIC, 0x1000); + sysbus_init_mmio(sbd, &s->iomem);
Please adjust the example below for your particular case replacing the sizes as appropriate but I would expect it to look something like:
memory_region_init(&s->picmem, obj, TYPE_LOONGARCH_PCH_PIC, 0x1000);
memory_region_init_io(&s->picintmem, obj, &loongarch_pch_pic_int_ops,
s, "pic.int", 0xaaaa);
memory_region_add_subregion(&s->picmem, INT_OFFSET, &s->picintmem);
memory_region_init_io(&s->picintmem, obj, &loongarch_pch_pic_htmsi_ops,
s, "pic.htmsi", 0xbbbb);
memory_region_add_subregion(&s->picmem, HTMSI_OFFSET, &s->picintmem);
memory_region_init_io(&s->picroutemem, obj, &loongarch_pch_pic_route_ops,
s, "pic.route", 0xcccc);
memory_region_add_subregion(&s->picmem, ROUTE_OFFSET, &s->picroutemem);
sysbus_init_mmio(sbd, &s->picmem);
You can then configure each of the *_ops memory regions with the correct .min.access
and .max.access sizes accordingly, and each region will show up separately in the
output of "info mtree".
+ for (i = 0; i < PCH_PIC_IRQ_NUM; i++) { + sysbus_init_irq(sbd, &s->parent_irq[i]); + } + qdev_init_gpio_in(DEVICE(obj), pch_pic_irq_handler, PCH_PIC_IRQ_NUM); +} + +static const VMStateDescription vmstate_loongarch_pch_pic = { + .name = TYPE_LOONGARCH_PCH_PIC, + .version_id = 1, + .minimum_version_id = 1, + .fields = (VMStateField[]) { + VMSTATE_UINT64(int_mask, loongarch_pch_pic), + VMSTATE_UINT64(htmsi_en, loongarch_pch_pic), + VMSTATE_UINT64(intedge, loongarch_pch_pic), + VMSTATE_UINT64(intclr, loongarch_pch_pic), + VMSTATE_UINT64(auto_crtl0, loongarch_pch_pic), + VMSTATE_UINT64(auto_crtl1, loongarch_pch_pic), + VMSTATE_UINT8_ARRAY(route_entry, loongarch_pch_pic, 64), + VMSTATE_UINT8_ARRAY(htmsi_vector, loongarch_pch_pic, 64), + VMSTATE_UINT64(last_intirr, loongarch_pch_pic), + VMSTATE_UINT64(intirr, loongarch_pch_pic), + VMSTATE_UINT64(intisr, loongarch_pch_pic), + VMSTATE_UINT64(int_polarity, loongarch_pch_pic), + VMSTATE_END_OF_LIST() + } +}; + +static void loongarch_pch_pic_class_init(ObjectClass *klass, void *data) +{ + DeviceClass *dc = DEVICE_CLASS(klass); + + dc->reset = loongarch_pch_pic_reset; + dc->vmsd = &vmstate_loongarch_pch_pic; +} + +static const TypeInfo loongarch_pch_pic_info = { + .name = TYPE_LOONGARCH_PCH_PIC, + .parent = TYPE_SYS_BUS_DEVICE, + .instance_size = sizeof(loongarch_pch_pic), + .instance_init = loongarch_pch_pic_init, + .class_init = loongarch_pch_pic_class_init, +}; + +static void loongarch_pch_pic_register_types(void) +{ + type_register_static(&loongarch_pch_pic_info); +} + +type_init(loongarch_pch_pic_register_types) diff --git a/hw/intc/meson.build b/hw/intc/meson.build index 51f0c3988a..33ba63266e 100644 --- a/hw/intc/meson.build +++ b/hw/intc/meson.build @@ -58,3 +58,4 @@ specific_ss.add(when: ['CONFIG_KVM', 'CONFIG_XIVE'], 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')) diff --git a/hw/intc/trace-events b/hw/intc/trace-events index 124608e51f..52fedf82be 100644 --- a/hw/intc/trace-events +++ b/hw/intc/trace-events @@ -250,3 +250,8 @@ sh_intc_set(int id, int enable) "setting interrupt group %d to %d" # loongarch_ipi.c loongarch_ipi_read(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%"PRIx64 loongarch_ipi_write(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%"PRIx64 + +# loongarch_pch_pic.c +pch_pic_irq_handler(uint32_t edge, int irq, int level) "edge 0x%02x irq %d level %d" +loongarch_pch_pic_read(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 +loongarch_pch_pic_write(unsigned size, uint32_t addr, unsigned long val) "size: %u addr: 0x%"PRIx32 "val: 0x%" PRIx64 diff --git a/hw/loongarch/Kconfig b/hw/loongarch/Kconfig index 1591574397..c2b8046b94 100644 --- a/hw/loongarch/Kconfig +++ b/hw/loongarch/Kconfig @@ -2,3 +2,4 @@ config LOONGSON3_LS7A bool select PCI_EXPRESS_7A select LOONGARCH_IPI + select LOONGARCH_PCH_PIC diff --git a/include/hw/intc/loongarch_pch_pic.h b/include/hw/intc/loongarch_pch_pic.h new file mode 100644 index 0000000000..bc04ed28ef --- /dev/null +++ b/include/hw/intc/loongarch_pch_pic.h @@ -0,0 +1,61 @@ +/* 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_PIC "loongarch_pch_pic" +DECLARE_INSTANCE_CHECKER(struct loongarch_pch_pic, LOONGARCH_PCH_PIC, + TYPE_LOONGARCH_PCH_PIC) + +#define PCH_PIC_IRQ_START 0 +#define PCH_PIC_IRQ_END 63 +#define PCH_PIC_IRQ_NUM 64 +#define PCH_PIC_INT_ID_VAL 0x7000000UL +#define PCH_PIC_INT_ID_NUM 0x3f0001UL + +#define PCH_PIC_INT_ID_OFFSET 0x00 +#define PCH_PIC_INT_ID_END 0x07 +#define PCH_PIC_INT_MASK_OFFSET 0x20 +#define PCH_PIC_INT_MASK_END 0x27 +#define PCH_PIC_HTMSI_EN_OFFSET 0x40 +#define PCH_PIC_HTMSI_EN_END 0x47 +#define PCH_PIC_INT_EDGE_OFFSET 0x60 +#define PCH_PIC_INT_EDGE_END 0x67 +#define PCH_PIC_INT_CLEAR_OFFSET 0x80 +#define PCH_PIC_INT_CLEAR_END 0x87 +#define PCH_PIC_AUTO_CTRL0_OFFSET 0xc0 +#define PCH_PIC_AUTO_CTRL0_END 0xc7 +#define PCH_PIC_AUTO_CTRL1_OFFSET 0xe0 +#define PCH_PIC_AUTO_CTRL1_END 0xe8 +#define PCH_PIC_ROUTE_ENTRY_OFFSET 0x100 +#define PCH_PIC_ROUTE_ENTRY_END 0x13f +#define PCH_PIC_HTMSI_VEC_OFFSET 0x200 +#define PCH_PIC_HTMSI_VEC_END 0x23f +#define PCH_PIC_INT_STATUS_OFFSET 0x3a0 +#define PCH_PIC_INT_STATUS_END 0x3a7 +#define PCH_PIC_INT_POL_OFFSET 0x3e0 +#define PCH_PIC_INT_POL_END 0x3e7 + +typedef struct loongarch_pch_pic { + SysBusDevice parent_obj; + qemu_irq parent_irq[64]; + uint64_t int_mask; /*0x020 interrupt mask register*/ + uint64_t htmsi_en;/*0x040 1=msi*/ + uint64_t intedge; /*0x060 edge=1 level =0*/ + uint64_t intclr; /*0x080 for clean edge int,set 1 clean,set 0 is noused*/ + uint64_t auto_crtl0; /*0x0c0*/ + uint64_t auto_crtl1; /*0x0e0*/ + uint8_t route_entry[64]; /*0x100 - 0x138*/ + uint8_t htmsi_vector[64]; /*0x200 - 0x238*/ + uint64_t last_intirr; /* edge detection */ + uint64_t intirr; /* 0x380 interrupt request register */ + uint64_t intisr; /* 0x3a0 interrupt service register */ + /* + * 0x3e0 interrupt level polarity selection + * register 0 for high level trigger + */ + uint64_t int_polarity; + MemoryRegion iomem; +} loongarch_pch_pic;
ATB, Mark.
