Hi Cedric, It is a partial block diagram. Thanks-Jamin
GIC132 ETH1 +-----------+ +-------->+0 3| ETH2 | 4| +-------->+1 5| ETH3 | 6| +-------->+2 19| INTC GIC UART0 | 20| +--------------------------+ +-------->+7 21| | | +--------------+ UART1 | 22| |orgate0 +----> output_pin0+----------->+GIC128 | +-------->+8 23| | | | | UART2 | 24| |orgate1 +----> output_pin1+----------->+GIC129 | +-------->+9 25| | | | | UART3 | 26| |orgate2 +----> output_pin2+----------->+GIC130 | +--------->10 27| | | | | UART5 | 28| |orgate3 +----> output_pin3+----------->+GIC131 | +-------->+11 29| | | | | UART6 | +----------->+orgate4 +----> output_pin4+----------->+GIC132 | +-------->+12 30| | | | | UART7 | 31| |orgate5 +----> output_pin5+----------->+GIC133 | +-------->+13 | | | | | UART8 | OR[0:31] | |orgate6 +----> output_pin6+----------->+GIC134 | ---------->14 | | | | | UART9 | | |orgate7 +----> output_pin7+----------->+GIC135 | --------->+15 | | | | | UART10 | | |orgate8 +----> output_pin8+----------->+GIC136 | --------->+16 | | | +--------------+ UART11 | | +--------------------------+ +-------->+17 | UART12 | | +--------->18 | | | | | | | +-----------+ > 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/ir > qchip/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/ir > qchip/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/ir > qchip/irq-aspeed-intc.c#L33 > c. Unmask > https://github.com/AspeedTech-BMC/linux/blob/aspeed-master-v6.6/drivers/ir > qchip/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 */