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 <[email protected]>
> > > Signed-off-by: Jamin Lin <[email protected]>
> > > ---
> > > 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 */
