Hello Jamin,
On 2/13/25 04:35, Jamin Lin wrote:
According to the AST2700 datasheet, the INTC(CPU DIE) controller has 16KB
(0x4000) of register space, and the INTCIO (I/O DIE) controller has 1KB (0x400)
of register space.
Introduced a new class attribute "mem_size" to set different memory sizes for
the INTC models in AST2700.
Introduced a new class attribute "reg_size" to set different register sizes for
the INTC models in AST2700.
Shouldn't that be multiple patches ?
Signed-off-by: Jamin Lin <jamin_...@aspeedtech.com>
---
hw/intc/aspeed_intc.c | 17 +++++++++++++----
include/hw/intc/aspeed_intc.h | 4 ++++
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 126b711b94..316885a27a 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -117,10 +117,11 @@ static void aspeed_intc_set_irq(void *opaque, int irq,
int level)
static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int
size)
{
AspeedINTCState *s = ASPEED_INTC(opaque);
+ AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
uint32_t addr = offset >> 2;
uint32_t value = 0;
- if (addr >= ASPEED_INTC_NR_REGS) {
Side note, ASPEED_INTC_NR_REGS is defined as
#define ASPEED_INTC_NR_REGS (0x2000 >> 2)
and the register array as:
uint32_t regs[ASPEED_INTC_NR_REGS];
The number of regs looks pretty big for me. Are the registers covering
the whole MMIO aperture ?
+ if (offset >= aic->reg_size) {
This is dead code since the MMIO aperture has the same size. You could
remove the check.
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Out-of-bounds read at offset 0x%" HWADDR_PRIx "\n",
__func__, offset);
@@ -143,7 +144,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset,
uint64_t data,
uint32_t change;
uint32_t irq;
- if (addr >= ASPEED_INTC_NR_REGS) {
+ if (offset >= aic->reg_size) {
qemu_log_mask(LOG_GUEST_ERROR,
"%s: Out-of-bounds write at offset 0x%" HWADDR_PRIx
"\n",
__func__, offset);
@@ -302,10 +303,16 @@ static void aspeed_intc_realize(DeviceState *dev, Error
**errp)
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
int i;
+ memory_region_init(&s->iomem_container, OBJECT(s),
+ TYPE_ASPEED_INTC ".container", aic->mem_size);
+
+ sysbus_init_mmio(sbd, &s->iomem_container);
Why introduce a container ? Do you plan to have multiple sub-regions ?
Thanks,
C.
+
memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
- TYPE_ASPEED_INTC ".regs", ASPEED_INTC_NR_REGS << 2);
+ TYPE_ASPEED_INTC ".regs", aic->reg_size);
+
+ memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
- sysbus_init_mmio(sbd, &s->iomem);
qdev_init_gpio_in(dev, aspeed_intc_set_irq, aic->num_ints);
for (i = 0; i < aic->num_ints; i++) {
@@ -344,6 +351,8 @@ static void aspeed_2700_intc_class_init(ObjectClass *klass,
void *data)
dc->desc = "ASPEED 2700 INTC Controller";
aic->num_lines = 32;
aic->num_ints = 9;
+ aic->mem_size = 0x4000;
+ aic->reg_size = 0x2000;
}
static const TypeInfo aspeed_2700_intc_info = {
diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index 18cb43476c..ecaeb15aea 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -25,6 +25,8 @@ struct AspeedINTCState {
/*< public >*/
MemoryRegion iomem;
+ MemoryRegion iomem_container;
+
uint32_t regs[ASPEED_INTC_NR_REGS];
OrIRQState orgates[ASPEED_INTC_NR_INTS];
qemu_irq output_pins[ASPEED_INTC_NR_INTS];
@@ -39,6 +41,8 @@ struct AspeedINTCClass {
uint32_t num_lines;
uint32_t num_ints;
+ uint64_t mem_size;
+ uint64_t reg_size;
};
#endif /* ASPEED_INTC_H */
