* [PATCH v2 1/6] hw/intc/aspeed: Support setting different memory and register size
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
@ 2025-02-06 9:52 ` Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 2/6] hw/intc/aspeed: Introduce helper functions for enable and status registers Jamin Lin via
` (5 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Jamin Lin via @ 2025-02-06 9:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, yunlin.tang
According to the AST2700 datasheet, the INTC (CPU DIE) controller has 16KB
(0x4000) of register space, and the INTC_IO (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.
Signed-off-by: Jamin Lin <jamin_lin@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) {
+ if (offset >= aic->reg_size) {
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);
+
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 */
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 2/6] hw/intc/aspeed: Introduce helper functions for enable and status registers
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 1/6] hw/intc/aspeed: Support setting different memory and register size Jamin Lin via
@ 2025-02-06 9:52 ` Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 3/6] hw/intc/aspeed: Add object type name to trace events for better debugging Jamin Lin via
` (4 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Jamin Lin via @ 2025-02-06 9:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, yunlin.tang
The behavior of the enable and status registers is almost identical between
INTC(CPU Die) and INTC_IO(IO Die). To reduce duplicated code, adds
"aspeed_intc_enable_handler" functions to handle enable register write
behavior and "aspeed_intc_status_handler" functions to handle status
register write behavior.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/intc/aspeed_intc.c | 185 +++++++++++++++++++++++-------------------
1 file changed, 103 insertions(+), 82 deletions(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 316885a27a..8b1f83c878 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -114,6 +114,107 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
}
}
+static void aspeed_intc_enable_handler(AspeedINTCState *s, hwaddr offset,
+ uint64_t data)
+{
+ AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ uint32_t addr = offset >> 2;
+ uint32_t old_enable;
+ uint32_t change;
+ uint32_t irq;
+
+ irq = (offset & 0x0f00) >> 8;
+
+ if (irq >= aic->num_ints) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
+ __func__, irq);
+ return;
+ }
+
+ /*
+ * The enable registers are used to enable source interrupts.
+ * They also handle masking and unmasking of source interrupts
+ * during the execution of the source ISR.
+ */
+
+ /* disable all source interrupt */
+ if (!data && !s->enable[irq]) {
+ s->regs[addr] = data;
+ return;
+ }
+
+ old_enable = s->enable[irq];
+ s->enable[irq] |= data;
+
+ /* enable new source interrupt */
+ if (old_enable != s->enable[irq]) {
+ trace_aspeed_intc_enable(s->enable[irq]);
+ s->regs[addr] = data;
+ return;
+ }
+
+ /* mask and unmask source interrupt */
+ change = s->regs[addr] ^ data;
+ if (change & data) {
+ s->mask[irq] &= ~change;
+ trace_aspeed_intc_unmask(change, s->mask[irq]);
+ } else {
+ s->mask[irq] |= change;
+ trace_aspeed_intc_mask(change, s->mask[irq]);
+ }
+
+ s->regs[addr] = data;
+}
+
+static void aspeed_intc_status_handler(AspeedINTCState *s, hwaddr offset,
+ uint64_t data)
+{
+ AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ uint32_t addr = offset >> 2;
+ uint32_t irq;
+
+ irq = (offset & 0x0f00) >> 8;
+
+ if (irq >= aic->num_ints) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
+ __func__, irq);
+ return;
+ }
+
+ /* 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) {
+ return;
+ }
+
+ /* All source ISR execution are done */
+ if (!s->regs[addr]) {
+ trace_aspeed_intc_all_isr_done(irq);
+ 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_trigger_irq(irq, s->regs[addr]);
+ aspeed_intc_update(s, irq, 1);
+ } else {
+ /* clear irq */
+ trace_aspeed_intc_clear_irq(irq, 0);
+ aspeed_intc_update(s, irq, 0);
+ }
+ }
+}
+
static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int size)
{
AspeedINTCState *s = ASPEED_INTC(opaque);
@@ -140,9 +241,6 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
AspeedINTCState *s = ASPEED_INTC(opaque);
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
uint32_t addr = offset >> 2;
- uint32_t old_enable;
- uint32_t change;
- uint32_t irq;
if (offset >= aic->reg_size) {
qemu_log_mask(LOG_GUEST_ERROR,
@@ -163,45 +261,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
case R_GICINT134_EN:
case R_GICINT135_EN:
case R_GICINT136_EN:
- irq = (offset & 0x0f00) >> 8;
-
- if (irq >= aic->num_ints) {
- qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
- __func__, irq);
- return;
- }
-
- /*
- * 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;
- return;
- }
-
- old_enable = s->enable[irq];
- s->enable[irq] |= data;
-
- /* enable new source interrupt */
- if (old_enable != s->enable[irq]) {
- trace_aspeed_intc_enable(s->enable[irq]);
- s->regs[addr] = data;
- return;
- }
-
- /* mask and unmask source interrupt */
- change = s->regs[addr] ^ data;
- if (change & data) {
- s->mask[irq] &= ~change;
- trace_aspeed_intc_unmask(change, s->mask[irq]);
- } else {
- s->mask[irq] |= change;
- trace_aspeed_intc_mask(change, s->mask[irq]);
- }
- s->regs[addr] = data;
+ aspeed_intc_enable_handler(s, offset, data);
break;
case R_GICINT128_STATUS:
case R_GICINT129_STATUS:
@@ -212,46 +272,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
case R_GICINT134_STATUS:
case R_GICINT135_STATUS:
case R_GICINT136_STATUS:
- irq = (offset & 0x0f00) >> 8;
-
- if (irq >= aic->num_ints) {
- qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
- __func__, irq);
- return;
- }
-
- /* 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) {
- return;
- }
-
- /* All source ISR execution are done */
- if (!s->regs[addr]) {
- trace_aspeed_intc_all_isr_done(irq);
- 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_trigger_irq(irq, s->regs[addr]);
- aspeed_intc_update(s, irq, 1);
- } else {
- /* clear irq */
- trace_aspeed_intc_clear_irq(irq, 0);
- aspeed_intc_update(s, irq, 0);
- }
- }
+ aspeed_intc_status_handler(s, offset, data);
break;
default:
s->regs[addr] = data;
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 3/6] hw/intc/aspeed: Add object type name to trace events for better debugging
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 1/6] hw/intc/aspeed: Support setting different memory and register size Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 2/6] hw/intc/aspeed: Introduce helper functions for enable and status registers Jamin Lin via
@ 2025-02-06 9:52 ` Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 4/6] hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0 Jamin Lin via
` (3 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Jamin Lin via @ 2025-02-06 9:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, yunlin.tang
Currently, these trace events only refer to INTC. To simplify the INTC model,
both INTC(CPU Die) and INTC_IO(IO Die) will share the same helper functions.
However, it is difficult to recognize whether these trace events are comes from
INTC or INTC_IO. To make these trace events more readable, adds object type name
to the INTC trace events.
Update trace events to include the "name" field for better identification.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/intc/aspeed_intc.c | 32 +++++++++++++++++++-------------
hw/intc/trace-events | 24 ++++++++++++------------
2 files changed, 31 insertions(+), 25 deletions(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 8b1f83c878..e1e4a9fe59 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -39,6 +39,7 @@ REG32(GICINT136_STATUS, 0x1804)
static void aspeed_intc_update(AspeedINTCState *s, int irq, int level)
{
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ const char *name = object_get_typename(OBJECT(s));
if (irq >= aic->num_ints) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: Invalid interrupt number: %d\n",
@@ -46,7 +47,7 @@ static void aspeed_intc_update(AspeedINTCState *s, int irq, int level)
return;
}
- trace_aspeed_intc_update_irq(irq, level);
+ trace_aspeed_intc_update_irq(name, irq, level);
qemu_set_irq(s->output_pins[irq], level);
}
@@ -60,6 +61,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
{
AspeedINTCState *s = (AspeedINTCState *)opaque;
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ const char *name = object_get_typename(OBJECT(s));
uint32_t status_addr = GICINT_STATUS_BASE + ((0x100 * irq) >> 2);
uint32_t select = 0;
uint32_t enable;
@@ -71,7 +73,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
return;
}
- trace_aspeed_intc_set_irq(irq, level);
+ trace_aspeed_intc_set_irq(name, irq, level);
enable = s->enable[irq];
if (!level) {
@@ -90,7 +92,7 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
return;
}
- trace_aspeed_intc_select(select);
+ trace_aspeed_intc_select(name, select);
if (s->mask[irq] || s->regs[status_addr]) {
/*
@@ -102,14 +104,14 @@ static void aspeed_intc_set_irq(void *opaque, int irq, int level)
* save source interrupt to pending variable.
*/
s->pending[irq] |= select;
- trace_aspeed_intc_pending_irq(irq, s->pending[irq]);
+ trace_aspeed_intc_pending_irq(name, irq, s->pending[irq]);
} else {
/*
* notify firmware which source interrupt are coming
* by setting status register
*/
s->regs[status_addr] = select;
- trace_aspeed_intc_trigger_irq(irq, s->regs[status_addr]);
+ trace_aspeed_intc_trigger_irq(name, irq, s->regs[status_addr]);
aspeed_intc_update(s, irq, 1);
}
}
@@ -118,6 +120,7 @@ static void aspeed_intc_enable_handler(AspeedINTCState *s, hwaddr offset,
uint64_t data)
{
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ const char *name = object_get_typename(OBJECT(s));
uint32_t addr = offset >> 2;
uint32_t old_enable;
uint32_t change;
@@ -148,7 +151,7 @@ static void aspeed_intc_enable_handler(AspeedINTCState *s, hwaddr offset,
/* enable new source interrupt */
if (old_enable != s->enable[irq]) {
- trace_aspeed_intc_enable(s->enable[irq]);
+ trace_aspeed_intc_enable(name, s->enable[irq]);
s->regs[addr] = data;
return;
}
@@ -157,10 +160,10 @@ static void aspeed_intc_enable_handler(AspeedINTCState *s, hwaddr offset,
change = s->regs[addr] ^ data;
if (change & data) {
s->mask[irq] &= ~change;
- trace_aspeed_intc_unmask(change, s->mask[irq]);
+ trace_aspeed_intc_unmask(name, change, s->mask[irq]);
} else {
s->mask[irq] |= change;
- trace_aspeed_intc_mask(change, s->mask[irq]);
+ trace_aspeed_intc_mask(name, change, s->mask[irq]);
}
s->regs[addr] = data;
@@ -170,6 +173,7 @@ static void aspeed_intc_status_handler(AspeedINTCState *s, hwaddr offset,
uint64_t data)
{
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ const char *name = object_get_typename(OBJECT(s));
uint32_t addr = offset >> 2;
uint32_t irq;
@@ -196,7 +200,7 @@ static void aspeed_intc_status_handler(AspeedINTCState *s, hwaddr offset,
/* All source ISR execution are done */
if (!s->regs[addr]) {
- trace_aspeed_intc_all_isr_done(irq);
+ trace_aspeed_intc_all_isr_done(name, irq);
if (s->pending[irq]) {
/*
* handle pending source interrupt
@@ -205,11 +209,11 @@ static void aspeed_intc_status_handler(AspeedINTCState *s, hwaddr offset,
*/
s->regs[addr] = s->pending[irq];
s->pending[irq] = 0;
- trace_aspeed_intc_trigger_irq(irq, s->regs[addr]);
+ trace_aspeed_intc_trigger_irq(name, irq, s->regs[addr]);
aspeed_intc_update(s, irq, 1);
} else {
/* clear irq */
- trace_aspeed_intc_clear_irq(irq, 0);
+ trace_aspeed_intc_clear_irq(name, irq, 0);
aspeed_intc_update(s, irq, 0);
}
}
@@ -219,6 +223,7 @@ 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);
+ const char *name = object_get_typename(OBJECT(s));
uint32_t addr = offset >> 2;
uint32_t value = 0;
@@ -230,7 +235,7 @@ static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int size)
}
value = s->regs[addr];
- trace_aspeed_intc_read(offset, size, value);
+ trace_aspeed_intc_read(name, offset, size, value);
return value;
}
@@ -240,6 +245,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
{
AspeedINTCState *s = ASPEED_INTC(opaque);
AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
+ const char *name = object_get_typename(OBJECT(s));
uint32_t addr = offset >> 2;
if (offset >= aic->reg_size) {
@@ -249,7 +255,7 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
return;
}
- trace_aspeed_intc_write(offset, size, data);
+ trace_aspeed_intc_write(name, offset, size, data);
switch (addr) {
case R_GICINT128_EN:
diff --git a/hw/intc/trace-events b/hw/intc/trace-events
index 3dcf147198..e9ca34755e 100644
--- a/hw/intc/trace-events
+++ b/hw/intc/trace-events
@@ -80,18 +80,18 @@ 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_clear_irq(int irq, int level) "Clear IRQ %d: %d"
-aspeed_intc_update_irq(int irq, int level) "Update IRQ: %d: %d"
-aspeed_intc_pending_irq(int irq, uint32_t value) "Pending IRQ: %d: 0x%x"
-aspeed_intc_trigger_irq(int irq, uint32_t value) "Trigger IRQ: %d: 0x%x"
-aspeed_intc_all_isr_done(int irq) "All source ISR execution are done: %d"
-aspeed_intc_enable(uint32_t value) "Enable: 0x%x"
-aspeed_intc_select(uint32_t value) "Select: 0x%x"
-aspeed_intc_mask(uint32_t change, uint32_t value) "Mask: 0x%x: 0x%x"
-aspeed_intc_unmask(uint32_t change, uint32_t value) "UnMask: 0x%x: 0x%x"
+aspeed_intc_read(const char *s, uint64_t offset, unsigned size, uint32_t value) "%s: From 0x%" PRIx64 " of size %u: 0x%" PRIx32
+aspeed_intc_write(const char *s, uint64_t offset, unsigned size, uint32_t data) "%s: To 0x%" PRIx64 " of size %u: 0x%" PRIx32
+aspeed_intc_set_irq(const char *s, int irq, int level) "%s: Set IRQ %d: %d"
+aspeed_intc_clear_irq(const char *s, int irq, int level) "%s: Clear IRQ %d: %d"
+aspeed_intc_update_irq(const char *s, int irq, int level) "%s: Update IRQ: %d: %d"
+aspeed_intc_pending_irq(const char *s, int irq, uint32_t value) "%s: Pending IRQ: %d: 0x%x"
+aspeed_intc_trigger_irq(const char *s, int irq, uint32_t value) "%s: Trigger IRQ: %d: 0x%x"
+aspeed_intc_all_isr_done(const char *s, int irq) "%s: All source ISR execution are done: %d"
+aspeed_intc_enable(const char *s, uint32_t value) "%s: Enable: 0x%x"
+aspeed_intc_select(const char *s, uint32_t value) "%s: Select: 0x%x"
+aspeed_intc_mask(const char *s, uint32_t change, uint32_t value) "%s: Mask: 0x%x: 0x%x"
+aspeed_intc_unmask(const char *s, uint32_t change, uint32_t value) "%s: UnMask: 0x%x: 0x%x"
# arm_gic.c
gic_enable_irq(int irq) "irq %d enabled"
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 4/6] hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
` (2 preceding siblings ...)
2025-02-06 9:52 ` [PATCH v2 3/6] hw/intc/aspeed: Add object type name to trace events for better debugging Jamin Lin via
@ 2025-02-06 9:52 ` Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number Jamin Lin via
` (2 subsequent siblings)
6 siblings, 0 replies; 9+ messages in thread
From: Jamin Lin via @ 2025-02-06 9:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, yunlin.tang
Currently, AST2700 SoC only supports A0. To support AST2700 A1, rename its IRQ
table and machine name.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/arm/aspeed.c | 8 ++++----
hw/arm/aspeed_ast27x0.c | 8 ++++----
2 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/hw/arm/aspeed.c b/hw/arm/aspeed.c
index d9418e2b9f..6ddfdbdeba 100644
--- a/hw/arm/aspeed.c
+++ b/hw/arm/aspeed.c
@@ -1654,12 +1654,12 @@ static void ast2700_evb_i2c_init(AspeedMachineState *bmc)
TYPE_TMP105, 0x4d);
}
-static void aspeed_machine_ast2700_evb_class_init(ObjectClass *oc, void *data)
+static void aspeed_machine_ast2700a0_evb_class_init(ObjectClass *oc, void *data)
{
MachineClass *mc = MACHINE_CLASS(oc);
AspeedMachineClass *amc = ASPEED_MACHINE_CLASS(oc);
- mc->desc = "Aspeed AST2700 EVB (Cortex-A35)";
+ mc->desc = "Aspeed AST2700 A0 EVB (Cortex-A35)";
amc->soc_name = "ast2700-a0";
amc->hw_strap1 = AST2700_EVB_HW_STRAP1;
amc->hw_strap2 = AST2700_EVB_HW_STRAP2;
@@ -1795,9 +1795,9 @@ static const TypeInfo aspeed_machine_types[] = {
.class_init = aspeed_minibmc_machine_ast1030_evb_class_init,
#ifdef TARGET_AARCH64
}, {
- .name = MACHINE_TYPE_NAME("ast2700-evb"),
+ .name = MACHINE_TYPE_NAME("ast2700a0-evb"),
.parent = TYPE_ASPEED_MACHINE,
- .class_init = aspeed_machine_ast2700_evb_class_init,
+ .class_init = aspeed_machine_ast2700a0_evb_class_init,
#endif
}, {
.name = TYPE_ASPEED_MACHINE,
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 4114e15ddd..39567fcab9 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -72,7 +72,7 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
#define AST2700_MAX_IRQ 256
/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
-static const int aspeed_soc_ast2700_irqmap[] = {
+static const int aspeed_soc_ast2700a0_irqmap[] = {
[ASPEED_DEV_UART0] = 132,
[ASPEED_DEV_UART1] = 132,
[ASPEED_DEV_UART2] = 132,
@@ -740,7 +740,7 @@ static void aspeed_soc_ast2700_realize(DeviceState *dev, Error **errp)
create_unimplemented_device("ast2700.io", 0x0, 0x4000000);
}
-static void aspeed_soc_ast2700_class_init(ObjectClass *oc, void *data)
+static void aspeed_soc_ast2700a0_class_init(ObjectClass *oc, void *data)
{
static const char * const valid_cpu_types[] = {
ARM_CPU_TYPE_NAME("cortex-a35"),
@@ -763,7 +763,7 @@ static void aspeed_soc_ast2700_class_init(ObjectClass *oc, void *data)
sc->uarts_num = 13;
sc->num_cpus = 4;
sc->uarts_base = ASPEED_DEV_UART0;
- sc->irqmap = aspeed_soc_ast2700_irqmap;
+ sc->irqmap = aspeed_soc_ast2700a0_irqmap;
sc->memmap = aspeed_soc_ast2700_memmap;
sc->get_irq = aspeed_soc_ast2700_get_irq;
}
@@ -778,7 +778,7 @@ static const TypeInfo aspeed_soc_ast27x0_types[] = {
.name = "ast2700-a0",
.parent = TYPE_ASPEED27X0_SOC,
.instance_init = aspeed_soc_ast2700_init,
- .class_init = aspeed_soc_ast2700_class_init,
+ .class_init = aspeed_soc_ast2700a0_class_init,
},
};
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
` (3 preceding siblings ...)
2025-02-06 9:52 ` [PATCH v2 4/6] hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0 Jamin Lin via
@ 2025-02-06 9:52 ` Jamin Lin via
2025-02-06 9:52 ` [PATCH v2 6/6] hw/intc/aspeed: Support different memory region ops Jamin Lin via
2025-02-10 1:46 ` [PATCH v2 0/6] INTC model cleanup Jamin Lin
6 siblings, 0 replies; 9+ messages in thread
From: Jamin Lin via @ 2025-02-06 9:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, yunlin.tang
To improve readability, sort the IRQ table by IRQ number.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/arm/aspeed_ast27x0.c | 50 ++++++++++++++++++++---------------------
1 file changed, 25 insertions(+), 25 deletions(-)
diff --git a/hw/arm/aspeed_ast27x0.c b/hw/arm/aspeed_ast27x0.c
index 39567fcab9..6a8487fee6 100644
--- a/hw/arm/aspeed_ast27x0.c
+++ b/hw/arm/aspeed_ast27x0.c
@@ -73,27 +73,13 @@ static const hwaddr aspeed_soc_ast2700_memmap[] = {
/* Shared Peripheral Interrupt values below are offset by -32 from datasheet */
static const int aspeed_soc_ast2700a0_irqmap[] = {
- [ASPEED_DEV_UART0] = 132,
- [ASPEED_DEV_UART1] = 132,
- [ASPEED_DEV_UART2] = 132,
- [ASPEED_DEV_UART3] = 132,
- [ASPEED_DEV_UART4] = 8,
- [ASPEED_DEV_UART5] = 132,
- [ASPEED_DEV_UART6] = 132,
- [ASPEED_DEV_UART7] = 132,
- [ASPEED_DEV_UART8] = 132,
- [ASPEED_DEV_UART9] = 132,
- [ASPEED_DEV_UART10] = 132,
- [ASPEED_DEV_UART11] = 132,
- [ASPEED_DEV_UART12] = 132,
- [ASPEED_DEV_FMC] = 131,
[ASPEED_DEV_SDMC] = 0,
- [ASPEED_DEV_SCU] = 12,
- [ASPEED_DEV_ADC] = 130,
+ [ASPEED_DEV_HACE] = 4,
[ASPEED_DEV_XDMA] = 5,
- [ASPEED_DEV_EMMC] = 15,
- [ASPEED_DEV_GPIO] = 130,
+ [ASPEED_DEV_UART4] = 8,
+ [ASPEED_DEV_SCU] = 12,
[ASPEED_DEV_RTC] = 13,
+ [ASPEED_DEV_EMMC] = 15,
[ASPEED_DEV_TIMER1] = 16,
[ASPEED_DEV_TIMER2] = 17,
[ASPEED_DEV_TIMER3] = 18,
@@ -102,19 +88,33 @@ static const int aspeed_soc_ast2700a0_irqmap[] = {
[ASPEED_DEV_TIMER6] = 21,
[ASPEED_DEV_TIMER7] = 22,
[ASPEED_DEV_TIMER8] = 23,
- [ASPEED_DEV_WDT] = 131,
- [ASPEED_DEV_PWM] = 131,
+ [ASPEED_DEV_DP] = 28,
[ASPEED_DEV_LPC] = 128,
[ASPEED_DEV_IBT] = 128,
+ [ASPEED_DEV_KCS] = 128,
+ [ASPEED_DEV_ADC] = 130,
+ [ASPEED_DEV_GPIO] = 130,
[ASPEED_DEV_I2C] = 130,
- [ASPEED_DEV_PECI] = 133,
+ [ASPEED_DEV_FMC] = 131,
+ [ASPEED_DEV_WDT] = 131,
+ [ASPEED_DEV_PWM] = 131,
+ [ASPEED_DEV_I3C] = 131,
+ [ASPEED_DEV_UART0] = 132,
+ [ASPEED_DEV_UART1] = 132,
+ [ASPEED_DEV_UART2] = 132,
+ [ASPEED_DEV_UART3] = 132,
+ [ASPEED_DEV_UART5] = 132,
+ [ASPEED_DEV_UART6] = 132,
+ [ASPEED_DEV_UART7] = 132,
+ [ASPEED_DEV_UART8] = 132,
+ [ASPEED_DEV_UART9] = 132,
+ [ASPEED_DEV_UART10] = 132,
+ [ASPEED_DEV_UART11] = 132,
+ [ASPEED_DEV_UART12] = 132,
[ASPEED_DEV_ETH1] = 132,
[ASPEED_DEV_ETH2] = 132,
[ASPEED_DEV_ETH3] = 132,
- [ASPEED_DEV_HACE] = 4,
- [ASPEED_DEV_KCS] = 128,
- [ASPEED_DEV_DP] = 28,
- [ASPEED_DEV_I3C] = 131,
+ [ASPEED_DEV_PECI] = 133,
[ASPEED_DEV_SDHCI] = 133,
};
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* [PATCH v2 6/6] hw/intc/aspeed: Support different memory region ops
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
` (4 preceding siblings ...)
2025-02-06 9:52 ` [PATCH v2 5/6] hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number Jamin Lin via
@ 2025-02-06 9:52 ` Jamin Lin via
2025-02-10 1:46 ` [PATCH v2 0/6] INTC model cleanup Jamin Lin
6 siblings, 0 replies; 9+ messages in thread
From: Jamin Lin via @ 2025-02-06 9:52 UTC (permalink / raw)
To: Cédric Le Goater, Peter Maydell, Steven Lee, Troy Lee,
Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: jamin_lin, troy_lee, yunlin.tang
The previous implementation set the "aspeed_intc_ops" struct, containing read
and write callbacks, to be used when I/O is performed on the INTC region.
Both "aspeed_intc_read" and "aspeed_intc_write" callback functions were used
for INTC (CPU Die).
To support the INTC_IO (IO Die) model, introduces a new "reg_ops" class
attribute. This allows setting different memory region operations to support
different INTC models.
Will introduce "aspeed_intc_io_read" and "aspeed_intc_io_write" callback
functions are used for INTC_IO.
Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
hw/intc/aspeed_intc.c | 5 ++++-
include/hw/intc/aspeed_intc.h | 1 +
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index e1e4a9fe59..e0e843201a 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -335,7 +335,7 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
sysbus_init_mmio(sbd, &s->iomem_container);
- memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
+ memory_region_init_io(&s->iomem, OBJECT(s), aic->reg_ops, s,
TYPE_ASPEED_INTC ".regs", aic->reg_size);
memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
@@ -353,11 +353,14 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
static void aspeed_intc_class_init(ObjectClass *klass, void *data)
{
DeviceClass *dc = DEVICE_CLASS(klass);
+ AspeedINTCClass *aic = ASPEED_INTC_CLASS(klass);
dc->desc = "ASPEED INTC Controller";
dc->realize = aspeed_intc_realize;
device_class_set_legacy_reset(dc, aspeed_intc_reset);
dc->vmsd = NULL;
+
+ aic->reg_ops = &aspeed_intc_ops;
}
static const TypeInfo aspeed_intc_info = {
diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index ecaeb15aea..749d7c55be 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -43,6 +43,7 @@ struct AspeedINTCClass {
uint32_t num_ints;
uint64_t mem_size;
uint64_t reg_size;
+ const MemoryRegionOps *reg_ops;
};
#endif /* ASPEED_INTC_H */
--
2.34.1
^ permalink raw reply related [flat|nested] 9+ messages in thread* RE: [PATCH v2 0/6] INTC model cleanup
2025-02-06 9:52 [PATCH v2 0/6] INTC model cleanup Jamin Lin via
` (5 preceding siblings ...)
2025-02-06 9:52 ` [PATCH v2 6/6] hw/intc/aspeed: Support different memory region ops Jamin Lin via
@ 2025-02-10 1:46 ` Jamin Lin
2025-02-10 7:23 ` Cédric Le Goater
6 siblings, 1 reply; 9+ messages in thread
From: Jamin Lin @ 2025-02-10 1:46 UTC (permalink / raw)
To: Jamin Lin, Cédric Le Goater, Peter Maydell, Steven Lee,
Troy Lee, Andrew Jeffery, Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: Troy Lee, Yunlin Tang
Hi Cedric, Andrew
> -----Original Message-----
> From: Jamin Lin <jamin_lin@aspeedtech.com>
> Sent: Thursday, February 6, 2025 5:53 PM
> To: Cédric Le Goater <clg@kaod.org>; Peter Maydell
> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
> Lee <leetroy@gmail.com>; Andrew Jeffery <andrew@codeconstruct.com.au>;
> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
> <qemu-arm@nongnu.org>; open list:All patches CC here
> <qemu-devel@nongnu.org>
> Cc: Jamin Lin <jamin_lin@aspeedtech.com>; Troy Lee
> <troy_lee@aspeedtech.com>; Yunlin Tang <yunlin.tang@aspeedtech.com>
> Subject: [PATCH v2 0/6] INTC model cleanup
>
> v2:
> To streamline the review process, split the following patch series into
> three parts.
>
> https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.246
> 5942-1-jamin_lin@aspeedtech.com/
> This patch series focuses on cleaning up the INTC model to
> facilitate future support for the INTC_IO model.
>
> Jamin Lin (6):
> hw/intc/aspeed: Support setting different memory and register size
> hw/intc/aspeed: Introduce helper functions for enable and status
> registers
> hw/intc/aspeed: Add object type name to trace events for better
> debugging
> hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
> hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
> hw/intc/aspeed: Support different memory region ops
>
> hw/arm/aspeed.c | 8 +-
> hw/arm/aspeed_ast27x0.c | 58 ++++-----
> hw/intc/aspeed_intc.c | 227 ++++++++++++++++++++--------------
> hw/intc/trace-events | 24 ++--
> include/hw/intc/aspeed_intc.h | 5 +
> 5 files changed, 183 insertions(+), 139 deletions(-)
>
Please ignore the v2 patch.
I will resend the v3 patch, as I am retaining the INTC naming and introducing a new INTC_IO model to support the AST2700 A1.
So, I think I don't need to split 3 part patches to support AST2700 A1.
Sorry for your inconvenience.
Do you have any suggestion or concern?
Jamin
> --
> 2.34.1
^ permalink raw reply [flat|nested] 9+ messages in thread* Re: [PATCH v2 0/6] INTC model cleanup
2025-02-10 1:46 ` [PATCH v2 0/6] INTC model cleanup Jamin Lin
@ 2025-02-10 7:23 ` Cédric Le Goater
0 siblings, 0 replies; 9+ messages in thread
From: Cédric Le Goater @ 2025-02-10 7:23 UTC (permalink / raw)
To: Jamin Lin, Peter Maydell, Steven Lee, Troy Lee, Andrew Jeffery,
Joel Stanley, open list:ASPEED BMCs,
open list:All patches CC here
Cc: Troy Lee, Yunlin Tang
Hello Jamin,
On 2/10/25 02:46, Jamin Lin wrote:
> Hi Cedric, Andrew
>
>> -----Original Message-----
>> From: Jamin Lin <jamin_lin@aspeedtech.com>
>> Sent: Thursday, February 6, 2025 5:53 PM
>> To: Cédric Le Goater <clg@kaod.org>; Peter Maydell
>> <peter.maydell@linaro.org>; Steven Lee <steven_lee@aspeedtech.com>; Troy
>> Lee <leetroy@gmail.com>; Andrew Jeffery <andrew@codeconstruct.com.au>;
>> Joel Stanley <joel@jms.id.au>; open list:ASPEED BMCs
>> <qemu-arm@nongnu.org>; open list:All patches CC here
>> <qemu-devel@nongnu.org>
>> Cc: Jamin Lin <jamin_lin@aspeedtech.com>; Troy Lee
>> <troy_lee@aspeedtech.com>; Yunlin Tang <yunlin.tang@aspeedtech.com>
>> Subject: [PATCH v2 0/6] INTC model cleanup
>>
>> v2:
>> To streamline the review process, split the following patch series into
>> three parts.
>>
>> https://patchwork.kernel.org/project/qemu-devel/cover/20250121070424.246
>> 5942-1-jamin_lin@aspeedtech.com/
>> This patch series focuses on cleaning up the INTC model to
>> facilitate future support for the INTC_IO model.
>>
>> Jamin Lin (6):
>> hw/intc/aspeed: Support setting different memory and register size
>> hw/intc/aspeed: Introduce helper functions for enable and status
>> registers
>> hw/intc/aspeed: Add object type name to trace events for better
>> debugging
>> hw/arm/aspeed: Rename IRQ table and machine name for AST2700 A0
>> hw/arm/aspeed_ast27x0: Sort the IRQ table by IRQ number
>> hw/intc/aspeed: Support different memory region ops
>>
>> hw/arm/aspeed.c | 8 +-
>> hw/arm/aspeed_ast27x0.c | 58 ++++-----
>> hw/intc/aspeed_intc.c | 227 ++++++++++++++++++++--------------
>> hw/intc/trace-events | 24 ++--
>> include/hw/intc/aspeed_intc.h | 5 +
>> 5 files changed, 183 insertions(+), 139 deletions(-)
>>
>
> Please ignore the v2 patch.
> I will resend the v3 patch, as I am retaining the INTC naming and introducing a new INTC_IO model to support the AST2700 A1.
> So, I think I don't need to split 3 part patches to support AST2700 A1.
> Sorry for your inconvenience.
> Do you have any suggestion or concern?
none. Please resend as well as the hace series if possible. these 2
series are easy to review and merge.
Thanks,
C.
^ permalink raw reply [flat|nested] 9+ messages in thread