On 2/13/25 04:35, Jamin Lin wrote:
> Currently, these trace events only refer to INTC. To simplify the INTC model,
> both INTC(CPU Die) and INTCIO(IO Die) will share the same helper functions.
>
> However, it is difficult to recognize whether these trace events are comes from
> INTC or INTCIO. 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>
Reviewed-by: Cédric Le Goater <clg@redhat.com>
Thanks,
C.
> ---
> 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 8f9fa97acc..91d8edb261 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;
>
> @@ -201,7 +205,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
> @@ -210,11 +214,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);
> }
> }
> @@ -224,6 +228,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;
>
> @@ -235,7 +240,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;
> }
> @@ -245,6 +250,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) {
> @@ -254,7 +260,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"