[PATCH v6 04/29] hw/intc/aspeed: Support setting different register size

Jamin Lin via posted 29 patches 11 months ago
[PATCH v6 04/29] hw/intc/aspeed: Support setting different register size
Posted by Jamin Lin via 11 months ago
Currently, the size of the regs array is 0x2000, which is too large. So far,
it only use GICINT128 - GICINT134, and the offsets from 0 to 0x1000 are unused.
To save code size, introduce a new class attribute "reg_size" to set the
different register sizes for the INTC models in AST2700 and add a regs
sub-region in the memory container.

Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
---
 include/hw/intc/aspeed_intc.h |  2 +-
 hw/intc/aspeed_intc.c         | 22 +++++-----------------
 2 files changed, 6 insertions(+), 18 deletions(-)

diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index 47ea0520b5..ec4936b3f4 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -16,7 +16,6 @@
 #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_INTS 9
 
 struct AspeedINTCState {
@@ -42,6 +41,7 @@ struct AspeedINTCClass {
     uint32_t num_lines;
     uint32_t num_ints;
     uint64_t mem_size;
+    uint64_t nr_regs;
 };
 
 #endif /* ASPEED_INTC_H */
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 558901570f..134922e46f 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -120,13 +120,6 @@ static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int size)
     uint32_t reg = offset >> 2;
     uint32_t value = 0;
 
-    if (reg >= 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[reg];
     trace_aspeed_intc_read(offset, size, value);
 
@@ -143,13 +136,6 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
     uint32_t change;
     uint32_t irq;
 
-    if (reg >= 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 (reg) {
@@ -288,8 +274,9 @@ static void aspeed_intc_instance_init(Object *obj)
 static void aspeed_intc_reset(DeviceState *dev)
 {
     AspeedINTCState *s = ASPEED_INTC(dev);
+    AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
 
-    memset(s->regs, 0, ASPEED_INTC_NR_REGS << 2);
+    memset(s->regs, 0, aic->nr_regs << 2);
     memset(s->enable, 0, sizeof(s->enable));
     memset(s->mask, 0, sizeof(s->mask));
     memset(s->pending, 0, sizeof(s->pending));
@@ -307,9 +294,9 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
 
     sysbus_init_mmio(sbd, &s->iomem_container);
 
-    s->regs = g_new(uint32_t, ASPEED_INTC_NR_REGS);
+    s->regs = g_new(uint32_t, aic->nr_regs);
     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->nr_regs << 2);
 
     memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
 
@@ -361,6 +348,7 @@ static void aspeed_2700_intc_class_init(ObjectClass *klass, void *data)
     aic->num_lines = 32;
     aic->num_ints = 9;
     aic->mem_size = 0x4000;
+    aic->nr_regs = 0x2000 >> 2;
 }
 
 static const TypeInfo aspeed_2700_intc_info = {
-- 
2.43.0
Re: [PATCH v6 04/29] hw/intc/aspeed: Support setting different register size
Posted by Cédric Le Goater 11 months ago
On 3/7/25 04:59, Jamin Lin wrote:
> Currently, the size of the regs array is 0x2000, which is too large. So far,
> it only use GICINT128 - GICINT134, and the offsets from 0 to 0x1000 are unused.
> To save code size, introduce a new class attribute "reg_size" to set the
> different register sizes for the INTC models in AST2700 and add a regs
> sub-region in the memory container.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>


Reviewed-by: Cédric Le Goater <clg@redhat.com>

Thanks,

C.


> ---
>   include/hw/intc/aspeed_intc.h |  2 +-
>   hw/intc/aspeed_intc.c         | 22 +++++-----------------
>   2 files changed, 6 insertions(+), 18 deletions(-)
> 
> diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
> index 47ea0520b5..ec4936b3f4 100644
> --- a/include/hw/intc/aspeed_intc.h
> +++ b/include/hw/intc/aspeed_intc.h
> @@ -16,7 +16,6 @@
>   #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_INTS 9
>   
>   struct AspeedINTCState {
> @@ -42,6 +41,7 @@ struct AspeedINTCClass {
>       uint32_t num_lines;
>       uint32_t num_ints;
>       uint64_t mem_size;
> +    uint64_t nr_regs;
>   };
>   
>   #endif /* ASPEED_INTC_H */
> diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
> index 558901570f..134922e46f 100644
> --- a/hw/intc/aspeed_intc.c
> +++ b/hw/intc/aspeed_intc.c
> @@ -120,13 +120,6 @@ static uint64_t aspeed_intc_read(void *opaque, hwaddr offset, unsigned int size)
>       uint32_t reg = offset >> 2;
>       uint32_t value = 0;
>   
> -    if (reg >= 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[reg];
>       trace_aspeed_intc_read(offset, size, value);
>   
> @@ -143,13 +136,6 @@ static void aspeed_intc_write(void *opaque, hwaddr offset, uint64_t data,
>       uint32_t change;
>       uint32_t irq;
>   
> -    if (reg >= 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 (reg) {
> @@ -288,8 +274,9 @@ static void aspeed_intc_instance_init(Object *obj)
>   static void aspeed_intc_reset(DeviceState *dev)
>   {
>       AspeedINTCState *s = ASPEED_INTC(dev);
> +    AspeedINTCClass *aic = ASPEED_INTC_GET_CLASS(s);
>   
> -    memset(s->regs, 0, ASPEED_INTC_NR_REGS << 2);
> +    memset(s->regs, 0, aic->nr_regs << 2);
>       memset(s->enable, 0, sizeof(s->enable));
>       memset(s->mask, 0, sizeof(s->mask));
>       memset(s->pending, 0, sizeof(s->pending));
> @@ -307,9 +294,9 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
>   
>       sysbus_init_mmio(sbd, &s->iomem_container);
>   
> -    s->regs = g_new(uint32_t, ASPEED_INTC_NR_REGS);
> +    s->regs = g_new(uint32_t, aic->nr_regs);
>       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->nr_regs << 2);
>   
>       memory_region_add_subregion(&s->iomem_container, 0x0, &s->iomem);
>   
> @@ -361,6 +348,7 @@ static void aspeed_2700_intc_class_init(ObjectClass *klass, void *data)
>       aic->num_lines = 32;
>       aic->num_ints = 9;
>       aic->mem_size = 0x4000;
> +    aic->nr_regs = 0x2000 >> 2;
>   }
>   
>   static const TypeInfo aspeed_2700_intc_info = {