1. Patchew
  2. QEMU
  3. [PATCH v5 00/29] Support AST2700 A1
  4. View patch

[PATCH v5 03/29] hw/intc/aspeed: Introduce dynamic allocation for regs array

Jamin Lin via posted 29 patches 3 weeks, 6 days ago
There is a newer version of this series
[PATCH v5 03/29] hw/intc/aspeed: Introduce dynamic allocation for regs array
Posted by Jamin Lin via 3 weeks, 6 days ago 
Currently, the size of the "regs" array is 0x2000, which is too large. To save
code size and avoid mapping large unused gaps, will update it to only map the
useful set of registers. This update will support multiple sub-regions with
different sizes.

To address the redundant size issue, replace the static "regs" array with a
dynamically allocated "regs" memory.

Introduce a new "aspeed_intc_unrealize" function to free the allocated "regs"
memory.

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

diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
index 03324f05ab..47ea0520b5 100644
--- a/include/hw/intc/aspeed_intc.h
+++ b/include/hw/intc/aspeed_intc.h
@@ -27,7 +27,7 @@ struct AspeedINTCState {
     MemoryRegion iomem;
     MemoryRegion iomem_container;
 
-    uint32_t regs[ASPEED_INTC_NR_REGS];
+    uint32_t *regs;
     OrIRQState orgates[ASPEED_INTC_NR_INTS];
     qemu_irq output_pins[ASPEED_INTC_NR_INTS];
 
diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
index 465f41e4fd..feb2c52441 100644
--- a/hw/intc/aspeed_intc.c
+++ b/hw/intc/aspeed_intc.c
@@ -289,7 +289,7 @@ static void aspeed_intc_reset(DeviceState *dev)
 {
     AspeedINTCState *s = ASPEED_INTC(dev);
 
-    memset(s->regs, 0, sizeof(s->regs));
+    memset(s->regs, 0, ASPEED_INTC_NR_REGS);
     memset(s->enable, 0, sizeof(s->enable));
     memset(s->mask, 0, sizeof(s->mask));
     memset(s->pending, 0, sizeof(s->pending));
@@ -307,6 +307,7 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
 
     sysbus_init_mmio(sbd, &s->iomem_container);
 
+    s->regs = g_malloc0(ASPEED_INTC_NR_REGS);
     memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
                           TYPE_ASPEED_INTC ".regs", ASPEED_INTC_NR_REGS << 2);
 
@@ -322,12 +323,21 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
     }
 }
 
+static void aspeed_intc_unrealize(DeviceState *dev)
+{
+    AspeedINTCState *s = ASPEED_INTC(dev);
+
+    g_free(s->regs);
+    s->regs = NULL;
+}
+
 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->unrealize = aspeed_intc_unrealize;
     device_class_set_legacy_reset(dc, aspeed_intc_reset);
     dc->vmsd = NULL;
 }
-- 
2.43.0
Re: [PATCH v5 03/29] hw/intc/aspeed: Introduce dynamic allocation for regs array
Posted by Cédric Le Goater 3 weeks, 5 days ago 
On 3/6/25 11:38, Jamin Lin wrote:
> Currently, the size of the "regs" array is 0x2000, which is too large. To save
> code size and avoid mapping large unused gaps, will update it to only map the
> useful set of registers. This update will support multiple sub-regions with
> different sizes.
> 
> To address the redundant size issue, replace the static "regs" array with a
> dynamically allocated "regs" memory.
> 
> Introduce a new "aspeed_intc_unrealize" function to free the allocated "regs"
> memory.
> 
> Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> ---
>   include/hw/intc/aspeed_intc.h |  2 +-
>   hw/intc/aspeed_intc.c         | 12 +++++++++++-
>   2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/include/hw/intc/aspeed_intc.h b/include/hw/intc/aspeed_intc.h
> index 03324f05ab..47ea0520b5 100644
> --- a/include/hw/intc/aspeed_intc.h
> +++ b/include/hw/intc/aspeed_intc.h
> @@ -27,7 +27,7 @@ struct AspeedINTCState {
>       MemoryRegion iomem;
>       MemoryRegion iomem_container;
>   
> -    uint32_t regs[ASPEED_INTC_NR_REGS];
> +    uint32_t *regs;
>       OrIRQState orgates[ASPEED_INTC_NR_INTS];
>       qemu_irq output_pins[ASPEED_INTC_NR_INTS];
>   
> diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c
> index 465f41e4fd..feb2c52441 100644
> --- a/hw/intc/aspeed_intc.c
> +++ b/hw/intc/aspeed_intc.c
> @@ -289,7 +289,7 @@ static void aspeed_intc_reset(DeviceState *dev)
>   {
>       AspeedINTCState *s = ASPEED_INTC(dev);
>   
> -    memset(s->regs, 0, sizeof(s->regs));
> +    memset(s->regs, 0, ASPEED_INTC_NR_REGS);

this is not the same size. s->regs is larger than ASPEED_INTC_NR_REGS.

>       memset(s->enable, 0, sizeof(s->enable));
>       memset(s->mask, 0, sizeof(s->mask));
>       memset(s->pending, 0, sizeof(s->pending));
> @@ -307,6 +307,7 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
>   
>       sysbus_init_mmio(sbd, &s->iomem_container);
>   
> +    s->regs = g_malloc0(ASPEED_INTC_NR_REGS);

please use g_new(....)


Thanks,

C.


>       memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops, s,
>                             TYPE_ASPEED_INTC ".regs", ASPEED_INTC_NR_REGS << 2);
>   
> @@ -322,12 +323,21 @@ static void aspeed_intc_realize(DeviceState *dev, Error **errp)
>       }
>   }
>   
> +static void aspeed_intc_unrealize(DeviceState *dev)
> +{
> +    AspeedINTCState *s = ASPEED_INTC(dev);
> +
> +    g_free(s->regs);
> +    s->regs = NULL;
> +}
> +
>   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->unrealize = aspeed_intc_unrealize;
>       device_class_set_legacy_reset(dc, aspeed_intc_reset);
>       dc->vmsd = NULL;
>   }
RE: [PATCH v5 03/29] hw/intc/aspeed: Introduce dynamic allocation for regs array
Posted by Jamin Lin 3 weeks, 5 days ago 
Hi Cedric

> Subject: Re: [PATCH v5 03/29] hw/intc/aspeed: Introduce dynamic allocation
> for regs array
> 
> On 3/6/25 11:38, Jamin Lin wrote:
> > Currently, the size of the "regs" array is 0x2000, which is too large.
> > To save code size and avoid mapping large unused gaps, will update it
> > to only map the useful set of registers. This update will support
> > multiple sub-regions with different sizes.
> >
> > To address the redundant size issue, replace the static "regs" array
> > with a dynamically allocated "regs" memory.
> >
> > Introduce a new "aspeed_intc_unrealize" function to free the allocated "regs"
> > memory.
> >
> > Signed-off-by: Jamin Lin <jamin_lin@aspeedtech.com>
> > ---
> >   include/hw/intc/aspeed_intc.h |  2 +-
> >   hw/intc/aspeed_intc.c         | 12 +++++++++++-
> >   2 files changed, 12 insertions(+), 2 deletions(-)
> >
> > diff --git a/include/hw/intc/aspeed_intc.h
> > b/include/hw/intc/aspeed_intc.h index 03324f05ab..47ea0520b5 100644
> > --- a/include/hw/intc/aspeed_intc.h
> > +++ b/include/hw/intc/aspeed_intc.h
> > @@ -27,7 +27,7 @@ struct AspeedINTCState {
> >       MemoryRegion iomem;
> >       MemoryRegion iomem_container;
> >
> > -    uint32_t regs[ASPEED_INTC_NR_REGS];
> > +    uint32_t *regs;
> >       OrIRQState orgates[ASPEED_INTC_NR_INTS];
> >       qemu_irq output_pins[ASPEED_INTC_NR_INTS];
> >
> > diff --git a/hw/intc/aspeed_intc.c b/hw/intc/aspeed_intc.c index
> > 465f41e4fd..feb2c52441 100644
> > --- a/hw/intc/aspeed_intc.c
> > +++ b/hw/intc/aspeed_intc.c
> > @@ -289,7 +289,7 @@ static void aspeed_intc_reset(DeviceState *dev)
> >   {
> >       AspeedINTCState *s = ASPEED_INTC(dev);
> >
> > -    memset(s->regs, 0, sizeof(s->regs));
> > +    memset(s->regs, 0, ASPEED_INTC_NR_REGS);
> 
> this is not the same size. s->regs is larger than ASPEED_INTC_NR_REGS.
Will fix it.
> 
> >       memset(s->enable, 0, sizeof(s->enable));
> >       memset(s->mask, 0, sizeof(s->mask));
> >       memset(s->pending, 0, sizeof(s->pending)); @@ -307,6 +307,7 @@
> > static void aspeed_intc_realize(DeviceState *dev, Error **errp)
> >
> >       sysbus_init_mmio(sbd, &s->iomem_container);
> >
> > +    s->regs = g_malloc0(ASPEED_INTC_NR_REGS);
> 
> please use g_new(....)

Will do

Thanks for review and suggestion.
Jamin
> 
> 
> Thanks,
> 
> C.
> 
> 
> >       memory_region_init_io(&s->iomem, OBJECT(s), &aspeed_intc_ops,
> s,
> >                             TYPE_ASPEED_INTC ".regs",
> > ASPEED_INTC_NR_REGS << 2);
> >
> > @@ -322,12 +323,21 @@ static void aspeed_intc_realize(DeviceState *dev,
> Error **errp)
> >       }
> >   }
> >
> > +static void aspeed_intc_unrealize(DeviceState *dev) {
> > +    AspeedINTCState *s = ASPEED_INTC(dev);
> > +
> > +    g_free(s->regs);
> > +    s->regs = NULL;
> > +}
> > +
> >   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->unrealize = aspeed_intc_unrealize;
> >       device_class_set_legacy_reset(dc, aspeed_intc_reset);
> >       dc->vmsd = NULL;
> >   }

Patchew 2.1-devel-b67e4c2

© 2016 - 2025 Red Hat, Inc.