Hi Cedric, > Hello Jamin > > On 4/16/24 11:18, Jamin Lin wrote: > > AST2700 interrupt controller(INTC) provides hardware interrupt > > interfaces to interrupt of processors PSP, SSP and TSP. In INTC, each > > interrupt of INT 128 to INT136 combines 32 interrupts. > > > > Introduce a new aspeed_intc class with instance_init and realize handlers. > > > > So far, this model only supports GICINT128 to GICINT136. > > It creates 9 GICINT or-gates to connect 32 interrupts sources from > > GICINT128 to GICINT136 as IRQ GPIO-OUTPUT pins. > > Then, this model registers IRQ handler with its IRQ GPIO-INPUT pins > > which connect to GICINT or-gates. And creates 9 GICINT IRQ GPIO-OUTPUT > > pins which connect to GIC device with GIC IRQ 128 to 136. > > > > If one interrupt source from GICINT128 to GICINT136 set irq, the > > OR-GATE irq callback function is called and set irq to INTC by OR-GATE > > GPIO-OUTPUT pins. Then, the INTC irq callback function is called and > > set irq to GIC by its GICINT IRQ GPIO-OUTPUT pins. Finally, the GIC > > irq callback function is called and set irq to CPUs and CPUs execute > > Interrupt Service Routine (ISR). > > It is difficult to understand what the model does :/ > I noticed serial console stuck if I run the following commands. In this situation, I need to type keyboard any keys to trigger serial interrupt again. The contents of my test script as following. Test.sh ``` while true; do ifconfig ip addr cat /proc/cpuinfo cat /proc/meminfo systemctl status hexdump /dev/mtd0 done ```
I refactor INTC model and believe this issues has been fixed because I run this scripts stress test INTC model over 3 days. Could you please help to review the new changes and give me your suggestions? To make you more understand this model behavior my briefly instruction as following. 1. A source trigger interrupt from a orgate, the aspeed_intc_set_irq is called. In this function, it checks orgate level index from bit 0 to 31 with INTC enable variable to find the valid source interrupt. Then, set source bit 1 to status register. Finally, FW known to run which source ISR by reading status register. https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/drivers/irqchip/irq-aspeed-intc.c#L30 For example: According to the data sheet description, serial console(tty12) is in gic132 at bit18. Therefore, it is in orgate index 4(132-128) and level index 18. s->regs[status_addr] = select; trace_aspeed_intc_debug("trigger source interrupt", s->regs[status_addr]); aspeed_intc_update(s, irq, 1) 2. Once CPU enters ISR mode, FW masks enable register first, clear source bit in status register after it executes source ISR and unmasks enable register. a. Mask https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/drivers/irqchip/irq-aspeed-intc.c#L39 b. After executes source ISR, clear status register at source bit, https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/drivers/irqchip/irq-aspeed-intc.c#L33 c. Unmask https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/drivers/irqchip/irq-aspeed-intc.c#L46 3. If status register is 0, it means ALL source ISR execution are done. If no pending interrupt, it clear gic else set gic to handle source pending interrupts. /* All source ISR execution are done */ if (!s->regs[addr]) { if (s->pending[irq]) { /* * handle pending source interrupt * notify firmware which source interrupt are pending * by setting status register */ s->regs[addr] = s->pending[irq]; s->pending[irq] = 0; trace_aspeed_intc_debug("trigger pending interrupt", s->regs[addr]); aspeed_intc_update(s, irq, 1); } else { /* clear irq */ aspeed_intc_update(s, irq, 0); } } 4. According to the design of INTC controller, it only have Enable register. This register is used for enable source interrupt, mask and unmake source interrupt if executing ISR mode. We don’t know users want to “mask or disable” specific source interrupt because it only have one register. So, I create enable variables to save the source interrupt enable status if enable register has been set to 1. old_enable = s->enable[irq]; s->enable[irq] |= data; /* enable new source interrupt */ if (old_enable != s->enable[irq]) { trace_aspeed_intc_debug("enable new source interrupt", s->enable[irq]); s->regs[addr] = data; return; } If no new bits set to 1, it means mask/unmask behavior. It set mask status in s->mask variables, so aspeed_intc_set_irq function know it is in ISR mode, and all source interrupts are saved to pending variables. After ISR done, this model set gic to handle pending interrupts. /* mask and unmask source interrupt */ change = s->regs[addr] ^ data; trace_aspeed_intc_debug("change bit", change); if (change & data) { s->mask[irq] &= ~change; trace_aspeed_intc_debug("umask", s->mask[irq]); } else { s->mask[irq] |= change; trace_aspeed_intc_debug("mask", s->mask[irq]); } s->regs[addr] = data; My new changes: aspeed_intc.c /* * ASPEED INTC Controller * * Copyright (C) 2024 ASPEED Technology Inc. * * SPDX-License-Identifier: GPL-2.0-or-later */ #include "qemu/osdep.h" #include "hw/intc/aspeed_intc.h" #include "hw/irq.h" #include "migration/vmstate.h" #include "qemu/bitops.h" #include "qemu/log.h" #include "qemu/module.h" #include "hw/intc/arm_gicv3.h" #include "trace.h" #include "hw/registerfields.h" #include "qapi/error.h" /* INTC Registers */ REG32(GICINT128_EN, 0x1000) REG32(GICINT128_STATUS, 0x1004) REG32(GICINT129_EN, 0x1100) REG32(GICINT129_STATUS, 0x1104) REG32(GICINT130_EN, 0x1200) REG32(GICINT130_STATUS, 0x1204) REG32(GICINT131_EN, 0x1300) REG32(GICINT131_STATUS, 0x1304) REG32(GICINT132_EN, 0x1400) REG32(GICINT132_STATUS, 0x1404) REG32(GICINT133_EN, 0x1500) REG32(GICINT133_STATUS, 0x1504) REG32(GICINT134_EN, 0x1600) REG32(GICINT134_STATUS, 0x1604) REG32(GICINT135_EN, 0x1700) REG32(GICINT135_STATUS, 0x1704) REG32(GICINT136_EN, 0x1800) REG32(GICINT136_STATUS, 0x1804) #define GICINT_STATUS_BASE R_GICINT128_STATUS static void aspeed_intc_update(AspeedINTCState *s, int irq, int level) { trace_aspeed_intc_update_irq(irq, level); qemu_set_irq(s->output_pins[irq], level); } /* * The address of GICINT128 to GICINT136 are from 0x1000 to 0x1804. * Utilize "address & 0x0f00" to get the irq and irq output pin index * The value of irq should be 0 to ASPEED_INTC_NR_GICS. * The irq 0 indicates GICINT128, irq 1 indicates GICINT129 and so on. */ static void aspeed_intc_set_irq(void *opaque, int irq, int level) { AspeedINTCState *s = (AspeedINTCState *)opaque; AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s); uint32_t status_addr = GICINT_STATUS_BASE + ((0x100 * irq) >> 2); uint32_t enable = s->enable[irq]; uint32_t select = 0; int i; if (irq > ASPEED_INTC_NR_GICS) { qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n", __func__, irq); return; } trace_aspeed_intc_set_irq(irq, level); if (!level) { return; } for (i = 0; i < aic->num_lines; i++) { if (s->orgate[irq].levels[i]) { if (enable & BIT(i)) { select |= BIT(i); } } } if (!select) { return; } trace_aspeed_intc_debug("select", select); trace_aspeed_intc_debug("mask", s->mask[irq]); trace_aspeed_intc_debug("status", s->regs[status_addr]); if (s->mask[irq] || s->regs[status_addr]) { /* * a. mask is not 0 means in ISR mode * sources interrupt routine are executing. * b. status register value is not 0 means previous * source interrupt does not be executed, yet. * * save source interrupt to pending variable. */ s->pending[irq] |= select; trace_aspeed_intc_debug("pending source interrupt", s->pending[irq]); } else { /* * notify firmware which source interrupt are coming * by setting status register */ s->regs[status_addr] = select; trace_aspeed_intc_debug("trigger source interrupt", s->regs[status_addr]); aspeed_intc_update(s, irq, 1); } } static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int size) { AspeedINTCState *s = ASPEED_INTC(opaque); uint32_t addr = offset >> 2; uint32_t value = 0; if (addr >= ASPEED_INTC_NR_REGS) { qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n", __func__, offset); return 0; } value = s->regs[addr]; trace_aspeed_intc_read(offset, size, value); return value; } static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data, unsigned size) { AspeedINTCState *s = ASPEED_INTC(opaque); uint32_t irq = (offset & 0x0f00) >> 8; uint32_t addr = offset >> 2; uint32_t old_enable; uint32_t change; if (addr >= ASPEED_INTC_NR_REGS) { qemu_log_mask(LOG_GUEST_ERROR, "%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx "\n", __func__, offset); return; } trace_aspeed_intc_write(offset, size, data); switch (addr) { case R_GICINT128_EN: case R_GICINT129_EN: case R_GICINT130_EN: case R_GICINT131_EN: case R_GICINT132_EN: case R_GICINT133_EN: case R_GICINT134_EN: case R_GICINT135_EN: case R_GICINT136_EN: /* * These registers are used for enable sources interrupt and * mask and unmask source interrupt while executing source ISR. */ /* disable all source interrupt */ if (!data && !s->enable[irq]) { s->regs[addr] = data; trace_aspeed_intc_debug("disable all source interrupt", irq); return; } old_enable = s->enable[irq]; s->enable[irq] |= data; /* enable new source interrupt */ if (old_enable != s->enable[irq]) { trace_aspeed_intc_debug("enable new source interrupt", s->enable[irq]); s->regs[addr] = data; return; } /* mask and unmask source interrupt */ change = s->regs[addr] ^ data; trace_aspeed_intc_debug("change bit", change); if (change & data) { s->mask[irq] &= ~change; trace_aspeed_intc_debug("umask", s->mask[irq]); } else { s->mask[irq] |= change; trace_aspeed_intc_debug("mask", s->mask[irq]); } s->regs[addr] = data; break; case R_GICINT128_STATUS: case R_GICINT129_STATUS: case R_GICINT130_STATUS: case R_GICINT131_STATUS: case R_GICINT132_STATUS: case R_GICINT133_STATUS: case R_GICINT134_STATUS: case R_GICINT135_STATUS: case R_GICINT136_STATUS: /* clear status */ s->regs[addr] &= ~data; /* * These status registers are used for notify sources ISR are executed. * If one source ISR is executed, it will clear one bit. * If it clear all bits, it means to initialize this register status * rather than sources ISR are executed. */ if (data == 0xffffffff) { trace_aspeed_intc_debug("clear all source interrupt status", s->regs[addr]); return; } /* All source ISR execution are done */ if (!s->regs[addr]) { if (s->pending[irq]) { /* * handle pending source interrupt * notify firmware which source interrupt are pending * by setting status register */ s->regs[addr] = s->pending[irq]; s->pending[irq] = 0; trace_aspeed_intc_debug("trigger pending interrupt", s->regs[addr]); aspeed_intc_update(s, irq, 1); } else { /* clear irq */ aspeed_intc_update(s, irq, 0); } } break; default: s->regs[addr] = data; break; } return; } static const MemoryRegionOps aspeed_intc_ops = { .read = aspeed_intc_read, .write = aspeed_intc_write, .endianness = DEVICE_LITTLE_ENDIAN, .valid = { .min_access_size = 4, .max_access_size = 4, } }; static void aspeed_intc_instance_init(Object *obj) { AspeedINTCState *s = ASPEED_INTC(obj); AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s); int i; for (i = 0; i < ASPEED_INTC_NR_GICS; i++) { object_initialize_child(obj, "intc-orgate[*]", &s->orgate[i], TYPE_OR_IRQ); object_property_set_int(OBJECT(&s->orgate[i]), "num-lines", aic->num_lines, &error_abort); } } static void aspeed_intc_reset(DeviceState *dev) { AspeedINTCState *s = ASPEED_INTC(dev); memset(s->regs, 0, sizeof(s->regs)); memset(s->enable, 0, sizeof(s->enable)); memset(s->mask, 0, sizeof(s->mask)); memset(s->pending, 0, sizeof(s->pending)); } static void aspeed_intc_realize(DeviceState *dev, Error **errp) { SysBusDevice *sbd = SYS_BUS_DEVICE(dev); AspeedINTCState *s = ASPEED_INTC(dev); int i; memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s, TYPE_ASPEED_INTC ".regs", ASPEED_INTC_NR_REGS << 2); sysbus_init_mmio(sbd, &s->iomem); qdev_init_gpio_in(dev, aspeed_intc_set_irq, ASPEED_INTC_NR_GICS); for (i = 0; i < ASPEED_INTC_NR_GICS; i++) { if (!qdev_realize(DEVICE(&s->orgate[i]), NULL, errp)) { return; } sysbus_init_irq(sbd, &s->output_pins[i]); } } static void aspeed_intc_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); dc->desc = "ASPEED INTC Controller"; dc->realize = aspeed_intc_realize; dc->reset = aspeed_intc_reset; dc->vmsd = NULL; } static const TypeInfo aspeed_intc_info = { .name = TYPE_ASPEED_INTC, .parent = TYPE_SYS_BUS_DEVICE, .instance_init = aspeed_intc_instance_init, .instance_size = sizeof(AspeedINTCState), .class_init = aspeed_intc_class_init, .abstract = true, }; static void aspeed_2700_intc_class_init(ObjectClass *klass, void *data) { DeviceClass *dc = DEVICE_CLASS(klass); AspeedINTCClass *aic = ASPEED_INTC_CLASS(klass); dc->desc = "ASPEED 2700 INTC Controller"; aic->num_lines = 32; } static const TypeInfo aspeed_2700_intc_info = { .name = TYPE_ASPEED_2700_INTC, .parent = TYPE_ASPEED_INTC, .class_init = aspeed_2700_intc_class_init, }; static void aspeed_intc_register_types(void) { type_register_static(&aspeed_intc_info); type_register_static(&aspeed_2700_intc_info); } type_init(aspeed_intc_register_types); aspeed_intc.h /* * ASPEED INTC Controller * * Copyright (C) 2024 ASPEED Technology Inc. * * SPDX-License-Identifier: GPL-2.0-or-later */ #ifndef ASPEED_INTC_H #define ASPEED_INTC_H #include "hw/sysbus.h" #include "qom/object.h" #include "hw/or-irq.h" #define TYPE_ASPEED_INTC "aspeed.intc" #define TYPE_ASPEED_2700_INTC TYPE_ASPEED_INTC "-ast2700" OBJECT_DECLARE_TYPE(AspeedINTCState, AspeedINTCClass, ASPEED_INTC) #define ASPEED_INTC_NR_REGS (0x2000 >> 2) #define ASPEED_INTC_NR_GICS 9 struct AspeedINTCState { /*< private >*/ SysBusDevice parent_obj; DeviceState *gic; /*< public >*/ MemoryRegion iomem; uint32_t regs[ASPEED_INTC_NR_REGS]; OrIRQState orgate[ASPEED_INTC_NR_GICS]; qemu_irq output_pins[ASPEED_INTC_NR_GICS]; uint32_t enable[ASPEED_INTC_NR_GICS]; uint32_t mask[ASPEED_INTC_NR_GICS]; uint32_t pending[ASPEED_INTC_NR_GICS]; }; struct AspeedINTCClass { SysBusDeviceClass parent_class; uint32_t num_lines; }; #endif /* ASPEED_INTC_H */ aspeed_ast27x0.c static qemu_irq aspeed_soc_ast2700_get_irq(AspeedSoCState *s, int dev) { Aspeed27x0SoCState *a = ASPEED27X0_SOC(s); AspeedSoCClass *sc = ASPEED_SOC_GET_CLASS(s); int i; for (i = 0; i < ARRAY_SIZE(aspeed_soc_ast2700_gic_intcmap); i++) { if (sc->irqmap[dev] == aspeed_soc_ast2700_gic_intcmap[i].irq) { assert(aspeed_soc_ast2700_gic_intcmap[i].ptr); return qdev_get_gpio_in(DEVICE(&a->intc.orgate[i]), aspeed_soc_ast2700_gic_intcmap[i].ptr[dev]); } } return qdev_get_gpio_in(a->intc.gic, sc->irqmap[dev]); } static void aspeed_soc_ast2700_init(Object *obj) object_initialize_child(obj, "intc", &a->intc, TYPE_ASPEED_2700_INTC); static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp) /* INTC */ if (!sysbus_realize(SYS_BUS_DEVICE(&a->intc), errp)) { return; } aspeed_mmio_map(s, SYS_BUS_DEVICE(&a->intc), 0, sc->memmap[ASPEED_DEV_INTC]); /* GICINT orgate -> INTC -> GIC */ for (i = 0; i < ASPEED_INTC_NR_GICS; i++) { qdev_connect_gpio_out(DEVICE(&a->intc.orgate[i]), 0, qdev_get_gpio_in(DEVICE(&a->intc), i)); sysbus_connect_irq(SYS_BUS_DEVICE(&a->intc), i, qdev_get_gpio_in(a->intc.gic, aspeed_soc_ast2700_gic_intcmap[i].irq)); } Thanks-Jamin > > Signed-off-by: Troy Lee <troy_...@aspeedtech.com> > > Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com> > > --- > > hw/intc/aspeed_intc.c | 269 > ++++++++++++++++++++++++++++++++++ > > hw/intc/meson.build | 1 + > > hw/intc/trace-events | 6 + > > include/hw/intc/aspeed_intc.h | 35 +++++ > > 4 files changed, 311 insertions(+) > > create mode 100644 hw/intc/aspeed_intc.c > > create mode 100644 include/hw/intc/aspeed_intc.h > > > > diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c new file > > mode 100644 index 0000000000..df31900d56 > > --- /dev/null > > +++ b/hw/intc/aspeed_intc.c > > @@ -0,0 +1,269 @@ > > +/* > > + * ASPEED INTC Controller > > + * > > + * Copyright (C) 2024 ASPEED Technology Inc. > > + * > > + * SPDX-License-Identifier: GPL-2.0-or-later */ > > + > > +#include "qemu/osdep.h" > > +#include "hw/intc/aspeed_intc.h" > > +#include "hw/irq.h" > > +#include "migration/vmstate.h" > > +#include "qemu/bitops.h" > > +#include "qemu/log.h" > > +#include "qemu/module.h" > > +#include "hw/intc/arm_gicv3.h" > > +#include "trace.h" > > +#include "hw/registerfields.h" > > +#include "qapi/error.h" > > + > > +/* INTC Registers */ > > +REG32(GICINT128_EN, 0x1000) > > +REG32(GICINT128_STATUS, 0x1004) > > +REG32(GICINT129_EN, 0x1100) > > +REG32(GICINT129_STATUS, 0x1104) > > +REG32(GICINT130_EN, 0x1200) > > +REG32(GICINT130_STATUS, 0x1204) > > +REG32(GICINT131_EN, 0x1300) > > +REG32(GICINT131_STATUS, 0x1304) > > +REG32(GICINT132_EN, 0x1400) > > +REG32(GICINT132_STATUS, 0x1404) > > +REG32(GICINT133_EN, 0x1500) > > +REG32(GICINT133_STATUS, 0x1504) > > +REG32(GICINT134_EN, 0x1600) > > +REG32(GICINT134_STATUS, 0x1604) > > +REG32(GICINT135_EN, 0x1700) > > +REG32(GICINT135_STATUS, 0x1704) > > +REG32(GICINT136_EN, 0x1800) > > +REG32(GICINT136_STATUS, 0x1804) > > + > > +#define GICINT_EN_BASE R_GICINT128_EN > > + > > +/* > > + * The address of GICINT128 to GICINT136 are from 0x1000 to 0x1804. > > + * Utilize "address & 0x0f00" to get the gicint_out index and > > + * its gic irq. > > + */ > > +static void aspeed_intc_update(AspeedINTCState *s, int irq, int > > +level) { > > + uint32_t gicint_enable_addr = GICINT_EN_BASE + ((0x100 * irq) >> 2); > > + uint32_t gicint_status_addr = gicint_enable_addr + (0x4 >> 2); > > + > > + if (s->trigger[irq]) { > > + if (!level && !s->regs[gicint_status_addr]) { > > + /* clear irq */ > > + trace_aspeed_intc_update_irq(irq, 0); > > + qemu_set_irq(s->gicint_out[irq], 0); > > + s->trigger[irq] = false; > > + } > > + } else { > > + if (s->new_gicint_status[irq]) { > > + /* set irq */ > > + trace_aspeed_intc_update_irq(irq, 1); > > + s->regs[gicint_status_addr] = s->new_gicint_status[irq]; > > + s->new_gicint_status[irq] = 0; > > + qemu_set_irq(s->gicint_out[irq], 1); > > + s->trigger[irq] = true; > > + } > > + } > > I will trust you on this as I don't understand. > > > > +} > > + > > +/* > > + * The value of irq should be 0 to ASPEED_INTC_NR_GICS. > > + * The irq 0 indicates GICINT128, irq 1 indicates GICINT129 and so on. > > + */ > > +static void aspeed_intc_set_irq(void *opaque, int irq, int level) { > > + AspeedINTCState *s = (AspeedINTCState *)opaque; > > + uint32_t gicint_enable_addr = GICINT_EN_BASE + ((0x100 * irq) >> 2); > > + uint32_t enable = s->regs[gicint_enable_addr]; > > + int i; > > + > > + if (irq > ASPEED_INTC_NR_GICS) { > > + qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt > number: %d\n", > > + __func__, irq); > > + return; > > + } > > + > > + trace_aspeed_intc_set_irq(irq, level); > > + > > + for (i = 0; i < 32; i++) { > > + if (s->gicint_orgate[irq].levels[i]) { > > + if (enable & BIT(i)) { > > + s->new_gicint_status[irq] |= BIT(i); > > + } > > + } > > + } > > + > > + aspeed_intc_update(s, irq, level); } > > + > > +static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, > > +unsigned int size) { > > + AspeedINTCState *s = ASPEED_INTC(opaque); > > + uint32_t addr = offset >> 2; > > + uint32_t value = 0; > > + > > + if (addr >= ASPEED_INTC_NR_REGS) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: Out-of-bounds read at offset 0x%" > HWADDR_PRIx "\n", > > + __func__, offset); > > + return 0; > > + } > > + > > + switch (addr) { > > + case R_GICINT128_EN: > > + case R_GICINT129_EN: > > + case R_GICINT130_EN: > > + case R_GICINT131_EN: > > + case R_GICINT132_EN: > > + case R_GICINT133_EN: > > + case R_GICINT134_EN: > > + case R_GICINT135_EN: > > + case R_GICINT136_EN: > > + case R_GICINT128_STATUS: > > + case R_GICINT129_STATUS: > > + case R_GICINT130_STATUS: > > + case R_GICINT131_STATUS: > > + case R_GICINT132_STATUS: > > + case R_GICINT133_STATUS: > > + case R_GICINT134_STATUS: > > + case R_GICINT135_STATUS: > > + case R_GICINT136_STATUS: > > + value = s->regs[addr]; > > + break; > > + default: > > + value = s->regs[addr]; > > + break; > > + } > > + > > + trace_aspeed_intc_read(offset, size, value); > > + > > + return value; > > +} > > + > > +static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data, > > + unsigned size) { > > + AspeedINTCState *s = ASPEED_INTC(opaque); > > + uint32_t irq = (offset & 0x0f00) >> 8; > > + uint32_t addr = offset >> 2; > > + > > + > > + if (addr >= ASPEED_INTC_NR_REGS) { > > + qemu_log_mask(LOG_GUEST_ERROR, > > + "%s: Out-of-bounds write at offset 0x%" > HWADDR_PRIx "\n", > > + __func__, offset); > > + return; > > + } > > + > > + trace_aspeed_intc_write(offset, size, data); > > + > > + switch (addr) { > > + case R_GICINT128_EN: > > + case R_GICINT129_EN: > > + case R_GICINT130_EN: > > + case R_GICINT131_EN: > > + case R_GICINT132_EN: > > + case R_GICINT133_EN: > > + case R_GICINT134_EN: > > + case R_GICINT135_EN: > > + case R_GICINT136_EN: > > + s->regs[addr] = data; > > + break; > > + case R_GICINT128_STATUS: > > + case R_GICINT129_STATUS: > > + case R_GICINT130_STATUS: > > + case R_GICINT131_STATUS: > > + case R_GICINT132_STATUS: > > + case R_GICINT133_STATUS: > > + case R_GICINT134_STATUS: > > + case R_GICINT135_STATUS: > > + case R_GICINT136_STATUS: > > + /* clear status */ > > + s->regs[addr] &= ~data; > > + if (!s->regs[addr]) { > > + aspeed_intc_update(s, irq, 0); > > + } > > + break; > > + default: > > + s->regs[addr] = data; > > + break; > > + } > > + > > + return; > > +} > > + > > +static const MemoryRegionOps aspeed_intc_ops = { > > + .read = aspeed_intc_read, > > + .write = aspeed_intc_write, > > + .endianness = DEVICE_LITTLE_ENDIAN, > > + .valid = { > > + .min_access_size = 4, > > + .max_access_size = 4, > > + } > > +}; > > + > > +static void aspeed_intc_instance_init(Object *obj) { > > + AspeedINTCState *s = ASPEED_INTC(obj); > > + int i; > > + > > + for (i = 0; i < ASPEED_INTC_NR_GICS; i++) { > > + object_initialize_child(obj, "gic-orgate[*]", &s->gicint_orgate[i], > > + TYPE_OR_IRQ); > > + object_property_set_int(OBJECT(&s->gicint_orgate[i]), > "num-lines", > > + 32, &error_abort); > > + } > > +} > > + > > +static void aspeed_intc_reset(DeviceState *dev) { > > + AspeedINTCState *s = ASPEED_INTC(dev); > > + memset(s->regs, 0, sizeof(s->regs)); > > + memset(s->trigger, 0, sizeof(s->trigger)); > > + memset(s->new_gicint_status, 0, sizeof(s->new_gicint_status)); } > > + > > +static void aspeed_intc_realize(DeviceState *dev, Error **errp) { > > + SysBusDevice *sbd = SYS_BUS_DEVICE(dev); > > + AspeedINTCState *s = ASPEED_INTC(dev); > > + int i; > > + > > + memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s, > > + TYPE_ASPEED_INTC ".regs", > > + ASPEED_INTC_NR_REGS << 2); > > + > > + sysbus_init_mmio(sbd, &s->iomem); > > + qdev_init_gpio_in(dev, aspeed_intc_set_irq, ASPEED_INTC_NR_GICS); > > + > > + for (i = 0; i < ASPEED_INTC_NR_GICS; i++) { > > + sysbus_init_irq(sbd, &s->gicint_out[i]); > > + } > > Why aren't the orgates realized here ? > fixed and please see my new changes. > > +} > > + > > +static void aspeed_intc_class_init(ObjectClass *klass, void *data) { > > + DeviceClass *dc = DEVICE_CLASS(klass); > > + > > + dc->realize = aspeed_intc_realize; > > + dc->reset = aspeed_intc_reset; > > + dc->desc = "ASPEED INTC Controller"; > > + dc->vmsd = NULL; > > +} > > + > > +static const TypeInfo aspeed_intc_info = { > > + .name = TYPE_ASPEED_INTC, > > + .parent = TYPE_SYS_BUS_DEVICE, > > + .instance_init = aspeed_intc_instance_init, > > + .instance_size = sizeof(AspeedINTCState), > > + .class_init = aspeed_intc_class_init, }; > > + > > +static void aspeed_intc_register_types(void) { > > + type_register_static(&aspeed_intc_info); > > +} > > + > > +type_init(aspeed_intc_register_types); > > diff --git a/hw/intc/meson.build b/hw/intc/meson.build index > > ed355941d1..f5c574f584 100644 > > --- a/hw/intc/meson.build > > +++ b/hw/intc/meson.build > > @@ -14,6 +14,7 @@ system_ss.add(when: 'CONFIG_ARM_GICV3_TCG', > if_true: files( > > )) > > system_ss.add(when: 'CONFIG_ALLWINNER_A10_PIC', if_true: > files('allwinner-a10-pic.c')) > > system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: > > files('aspeed_vic.c')) > > +system_ss.add(when: 'CONFIG_ASPEED_SOC', if_true: > > +files('aspeed_intc.c')) > > system_ss.add(when: 'CONFIG_ETRAXFS', if_true: files('etraxfs_pic.c')) > > system_ss.add(when: 'CONFIG_EXYNOS4', if_true: files('exynos4210_gic.c', > 'exynos4210_combiner.c')) > > system_ss.add(when: 'CONFIG_GOLDFISH_PIC', if_true: > > files('goldfish_pic.c')) diff --git a/hw/intc/trace-events > > b/hw/intc/trace-events index 1ef29d0256..30b9dd2a96 100644 > > --- a/hw/intc/trace-events > > +++ b/hw/intc/trace-events > > @@ -79,6 +79,12 @@ aspeed_vic_update_fiq(int flags) "Raising FIQ: %d" > > aspeed_vic_update_irq(int flags) "Raising IRQ: %d" > > aspeed_vic_read(uint64_t offset, unsigned size, uint32_t value) "From > 0x%" PRIx64 " of size %u: 0x%" PRIx32 > > aspeed_vic_write(uint64_t offset, unsigned size, uint32_t data) "To > > 0x%" PRIx64 " of size %u: 0x%" PRIx32 > > +# aspeed_intc.c > > +aspeed_intc_read(uint64_t offset, unsigned size, uint32_t value) > > +"From 0x%" PRIx64 " of size %u: 0x%" PRIx32 > > +aspeed_intc_write(uint64_t offset, unsigned size, uint32_t data) "To 0x%" > PRIx64 " of size %u: 0x%" PRIx32 aspeed_intc_set_irq(int irq, int level) "Set > IRQ > %d: %d" > > +aspeed_intc_update_irq(int irq, int level) "Update IRQ: %d: %d" > > +aspeed_intc_debug(uint32_t offset, uint32_t value) "Debug 0x%x: 0x%x" > > > > # arm_gic.c > > gic_enable_irq(int irq) "irq %d enabled" > > diff --git a/include/hw/intc/aspeed_intc.h > > b/include/hw/intc/aspeed_intc.h new file mode 100644 index > > 0000000000..0d2fbbda8f > > --- /dev/null > > +++ b/include/hw/intc/aspeed_intc.h > > @@ -0,0 +1,35 @@ > > +/* > > + * ASPEED INTC Controller > > + * > > + * Copyright (C) 2024 ASPEED Technology Inc. > > + * > > + * SPDX-License-Identifier: GPL-2.0-or-later */ #ifndef > > +ASPEED_INTC_H #define ASPEED_INTC_H > > + > > +#include "hw/sysbus.h" > > +#include "qom/object.h" > > +#include "hw/or-irq.h" > > + > > +#define TYPE_ASPEED_INTC "aspeed.intc" > > +OBJECT_DECLARE_SIMPLE_TYPE(AspeedINTCState, ASPEED_INTC) > > + > > +#define ASPEED_INTC_NR_REGS (0x2000 >> 2) #define > ASPEED_INTC_NR_GICS > > +9 > > + > > +struct AspeedINTCState { > > + /*< private >*/ > > + SysBusDevice parent_obj; > > + DeviceState *gic; > > + > > + /*< public >*/ > > + MemoryRegion iomem; > > + uint32_t regs[ASPEED_INTC_NR_REGS]; > > + OrIRQState gicint_orgate[ASPEED_INTC_NR_GICS]; > > + qemu_irq gicint_out[ASPEED_INTC_NR_GICS]; > > + bool trigger[ASPEED_INTC_NR_GICS]; > > + uint32_t new_gicint_status[ASPEED_INTC_NR_GICS]; > > I don't think the gicint prefix is useful in the names. I am struggling to > catch > what new_gicint_status and trigger really do. > fixed and please see my new changes. Thanks-Jamin > > Thanks, > > C. > > > > > > +}; > > + > > +#endif /* ASPEED_INTC_H */
