[PATCH v5 06/17] hw/misc: Add nr_regs and cold_reset_values to NPCM GCR

Hao Wu posted 17 patches 3 weeks, 1 day ago
[PATCH v5 06/17] hw/misc: Add nr_regs and cold_reset_values to NPCM GCR
Posted by Hao Wu 3 weeks, 1 day ago
These 2 values are different between NPCM7XX and NPCM8XX
GCRs. So we add them to the class and assign different values
to them.

Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Hao Wu <wuhaotsh@google.com>
---
 hw/misc/npcm_gcr.c         | 27 ++++++++++++++++-----------
 include/hw/misc/npcm_gcr.h | 13 +++++++++++--
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/hw/misc/npcm_gcr.c b/hw/misc/npcm_gcr.c
index 0959f2e5c4..d89e8c2c3b 100644
--- a/hw/misc/npcm_gcr.c
+++ b/hw/misc/npcm_gcr.c
@@ -66,10 +66,9 @@ enum NPCM7xxGCRRegisters {
     NPCM7XX_GCR_SCRPAD          = 0x013c / sizeof(uint32_t),
     NPCM7XX_GCR_USB1PHYCTL,
     NPCM7XX_GCR_USB2PHYCTL,
-    NPCM7XX_GCR_REGS_END,
 };
 
-static const uint32_t cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
+static const uint32_t npcm7xx_cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
     [NPCM7XX_GCR_PDID]          = 0x04a92750,   /* Poleg A1 */
     [NPCM7XX_GCR_MISCPE]        = 0x0000ffff,
     [NPCM7XX_GCR_SPSWC]         = 0x00000003,
@@ -88,8 +87,9 @@ static uint64_t npcm_gcr_read(void *opaque, hwaddr offset, unsigned size)
 {
     uint32_t reg = offset / sizeof(uint32_t);
     NPCMGCRState *s = opaque;
+    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
 
-    if (reg >= NPCM7XX_GCR_NR_REGS) {
+    if (reg >= c->nr_regs) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: offset 0x%04" HWADDR_PRIx " out of range\n",
                       __func__, offset);
@@ -106,11 +106,12 @@ static void npcm_gcr_write(void *opaque, hwaddr offset,
 {
     uint32_t reg = offset / sizeof(uint32_t);
     NPCMGCRState *s = opaque;
+    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
     uint32_t value = v;
 
-    trace_npcm_gcr_write(offset, value);
+    trace_npcm_gcr_write(offset, v);
 
-    if (reg >= NPCM7XX_GCR_NR_REGS) {
+    if (reg >= c->nr_regs) {
         qemu_log_mask(LOG_GUEST_ERROR,
                       "%s: offset 0x%04" HWADDR_PRIx " out of range\n",
                       __func__, offset);
@@ -156,10 +157,12 @@ static const struct MemoryRegionOps npcm_gcr_ops = {
 static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type)
 {
     NPCMGCRState *s = NPCM_GCR(obj);
+    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(obj);
 
-    QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
-
-    memcpy(s->regs, cold_reset_values, sizeof(s->regs));
+    g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values));
+    g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t));
+    memcpy(s->regs, c->cold_reset_values, c->nr_regs * sizeof(uint32_t));
+    /* These 3 registers are at the same location in both 7xx and 8xx. */
     s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
     s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
     s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
@@ -224,7 +227,7 @@ static const VMStateDescription vmstate_npcm_gcr = {
     .version_id = 1,
     .minimum_version_id = 1,
     .fields = (const VMStateField[]) {
-        VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM7XX_GCR_NR_REGS),
+        VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM_GCR_MAX_NR_REGS),
         VMSTATE_END_OF_LIST(),
     },
 };
@@ -238,7 +241,6 @@ static void npcm_gcr_class_init(ObjectClass *klass, void *data)
 {
     DeviceClass *dc = DEVICE_CLASS(klass);
 
-    QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END > NPCM7XX_GCR_NR_REGS);
     dc->realize = npcm_gcr_realize;
     dc->vmsd = &vmstate_npcm_gcr;
 
@@ -247,13 +249,15 @@ static void npcm_gcr_class_init(ObjectClass *klass, void *data)
 
 static void npcm7xx_gcr_class_init(ObjectClass *klass, void *data)
 {
+    NPCMGCRClass *c = NPCM_GCR_CLASS(klass);
     DeviceClass *dc = DEVICE_CLASS(klass);
     ResettableClass *rc = RESETTABLE_CLASS(klass);
 
-    QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END != NPCM7XX_GCR_NR_REGS);
     dc->desc = "NPCM7xx System Global Control Registers";
     rc->phases.enter = npcm7xx_gcr_enter_reset;
 
+    c->nr_regs = NPCM7XX_GCR_NR_REGS;
+    c->cold_reset_values = npcm7xx_cold_reset_values;
 }
 
 static const TypeInfo npcm_gcr_info[] = {
@@ -262,6 +266,7 @@ static const TypeInfo npcm_gcr_info[] = {
         .parent             = TYPE_SYS_BUS_DEVICE,
         .instance_size      = sizeof(NPCMGCRState),
         .instance_init      = npcm_gcr_init,
+        .class_size         = sizeof(NPCMGCRClass),
         .class_init         = npcm_gcr_class_init,
         .abstract           = true,
     },
diff --git a/include/hw/misc/npcm_gcr.h b/include/hw/misc/npcm_gcr.h
index 6d3d00d260..9af24e5cdc 100644
--- a/include/hw/misc/npcm_gcr.h
+++ b/include/hw/misc/npcm_gcr.h
@@ -18,6 +18,7 @@
 
 #include "exec/memory.h"
 #include "hw/sysbus.h"
+#include "qom/object.h"
 
 /*
  * NPCM7XX PWRON STRAP bit fields
@@ -53,6 +54,7 @@
  * Number of registers in our device state structure. Don't change this without
  * incrementing the version_id in the vmstate.
  */
+#define NPCM_GCR_MAX_NR_REGS NPCM7XX_GCR_NR_REGS
 #define NPCM7XX_GCR_NR_REGS (0x148 / sizeof(uint32_t))
 
 typedef struct NPCMGCRState {
@@ -60,15 +62,22 @@ typedef struct NPCMGCRState {
 
     MemoryRegion iomem;
 
-    uint32_t regs[NPCM7XX_GCR_NR_REGS];
+    uint32_t regs[NPCM_GCR_MAX_NR_REGS];
 
     uint32_t reset_pwron;
     uint32_t reset_mdlr;
     uint32_t reset_intcr3;
 } NPCMGCRState;
 
+typedef struct NPCMGCRClass {
+    SysBusDeviceClass parent;
+
+    size_t nr_regs;
+    const uint32_t *cold_reset_values;
+} NPCMGCRClass;
+
 #define TYPE_NPCM_GCR "npcm-gcr"
 #define TYPE_NPCM7XX_GCR "npcm7xx-gcr"
-OBJECT_DECLARE_SIMPLE_TYPE(NPCMGCRState, NPCM_GCR)
+OBJECT_DECLARE_TYPE(NPCMGCRState, NPCMGCRClass, NPCM_GCR)
 
 #endif /* NPCM_GCR_H */
-- 
2.48.1.601.g30ceb7b040-goog
Re: [PATCH v5 06/17] hw/misc: Add nr_regs and cold_reset_values to NPCM GCR
Posted by Peter Maydell 2 weeks, 2 days ago
On Wed, 19 Feb 2025 at 18:46, Hao Wu <wuhaotsh@google.com> wrote:
>
> These 2 values are different between NPCM7XX and NPCM8XX
> GCRs. So we add them to the class and assign different values
> to them.
>
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>


> @@ -156,10 +157,12 @@ static const struct MemoryRegionOps npcm_gcr_ops = {
>  static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type)
>  {
>      NPCMGCRState *s = NPCM_GCR(obj);
> +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(obj);
>
> -    QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
> -
> -    memcpy(s->regs, cold_reset_values, sizeof(s->regs));
> +    g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values));
> +    g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t));
> +    memcpy(s->regs, c->cold_reset_values, c->nr_regs * sizeof(uint32_t));

I looked again at this code after seeing the fix to the
similar code in npcm_clk that Pierrick just sent, and this
one looks broken in a different way:

c->cold_reset_values is a pointer, not an array, so
  g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values))
is asserting that the s->regs[] array is bigger than the size
of a pointer (8 bytes), which probably isn't what you meant.

Other than that, this is now the same as the fixed npcm_clk code,
except that we could do the same as that does and use a local
variable for sizeof_regs:

    size_t sizeof_regs = c->nr_regs * sizeof(uint32_t);
    g_assert(sizeof(s->regs) >= sizeof_regs);
    memcpy(s->regs, c->cold_reset_values, sizeof_regs);

But also, in this device (unlike npcm_clk) we have separate
reset method functions for npcm7xx and npcm8xx. So we don't
really need the s->cold_reset_values pointer at all, because
we could make npcm7xx_gcr_enter_reset() directly copy from
npcm7xx_cold_reset_values[] and similarly fro the npcm8xx function.

I think we should at least delete the assert() that isn't
doing what it looks like it's doing; I'll leave it to you
whether you want to also do one of the other two suggestions.

thanks
-- PMM
Re: [PATCH v5 06/17] hw/misc: Add nr_regs and cold_reset_values to NPCM GCR
Posted by Pierrick Bouvier 2 weeks, 3 days ago
Hello,

This patch introduces a buffer-overflow, now reported by address sanitizer.

I sent a patch:
https://lore.kernel.org/qemu-devel/20250224205053.104959-1-
pierrick.bouvier@linaro.org/T/#u

You're welcome to review it, or fix the problem differently if there is 
a better approach.

Regards,
Pierrick

On 2/19/25 10:45, Hao Wu wrote:
> These 2 values are different between NPCM7XX and NPCM8XX
> GCRs. So we add them to the class and assign different values
> to them.
> 
> Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>   hw/misc/npcm_gcr.c         | 27 ++++++++++++++++-----------
>   include/hw/misc/npcm_gcr.h | 13 +++++++++++--
>   2 files changed, 27 insertions(+), 13 deletions(-)
> 
> diff --git a/hw/misc/npcm_gcr.c b/hw/misc/npcm_gcr.c
> index 0959f2e5c4..d89e8c2c3b 100644
> --- a/hw/misc/npcm_gcr.c
> +++ b/hw/misc/npcm_gcr.c
> @@ -66,10 +66,9 @@ enum NPCM7xxGCRRegisters {
>       NPCM7XX_GCR_SCRPAD          = 0x013c / sizeof(uint32_t),
>       NPCM7XX_GCR_USB1PHYCTL,
>       NPCM7XX_GCR_USB2PHYCTL,
> -    NPCM7XX_GCR_REGS_END,
>   };
>   
> -static const uint32_t cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
> +static const uint32_t npcm7xx_cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
>       [NPCM7XX_GCR_PDID]          = 0x04a92750,   /* Poleg A1 */
>       [NPCM7XX_GCR_MISCPE]        = 0x0000ffff,
>       [NPCM7XX_GCR_SPSWC]         = 0x00000003,
> @@ -88,8 +87,9 @@ static uint64_t npcm_gcr_read(void *opaque, hwaddr offset, unsigned size)
>   {
>       uint32_t reg = offset / sizeof(uint32_t);
>       NPCMGCRState *s = opaque;
> +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
>   
> -    if (reg >= NPCM7XX_GCR_NR_REGS) {
> +    if (reg >= c->nr_regs) {
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: offset 0x%04" HWADDR_PRIx " out of range\n",
>                         __func__, offset);
> @@ -106,11 +106,12 @@ static void npcm_gcr_write(void *opaque, hwaddr offset,
>   {
>       uint32_t reg = offset / sizeof(uint32_t);
>       NPCMGCRState *s = opaque;
> +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
>       uint32_t value = v;
>   
> -    trace_npcm_gcr_write(offset, value);
> +    trace_npcm_gcr_write(offset, v);
>   
> -    if (reg >= NPCM7XX_GCR_NR_REGS) {
> +    if (reg >= c->nr_regs) {
>           qemu_log_mask(LOG_GUEST_ERROR,
>                         "%s: offset 0x%04" HWADDR_PRIx " out of range\n",
>                         __func__, offset);
> @@ -156,10 +157,12 @@ static const struct MemoryRegionOps npcm_gcr_ops = {
>   static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type)
>   {
>       NPCMGCRState *s = NPCM_GCR(obj);
> +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(obj);
>   
> -    QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
> -
> -    memcpy(s->regs, cold_reset_values, sizeof(s->regs));
> +    g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values));
> +    g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t));
> +    memcpy(s->regs, c->cold_reset_values, c->nr_regs * sizeof(uint32_t));
> +    /* These 3 registers are at the same location in both 7xx and 8xx. */
>       s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
>       s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
>       s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
> @@ -224,7 +227,7 @@ static const VMStateDescription vmstate_npcm_gcr = {
>       .version_id = 1,
>       .minimum_version_id = 1,
>       .fields = (const VMStateField[]) {
> -        VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM7XX_GCR_NR_REGS),
> +        VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM_GCR_MAX_NR_REGS),
>           VMSTATE_END_OF_LIST(),
>       },
>   };
> @@ -238,7 +241,6 @@ static void npcm_gcr_class_init(ObjectClass *klass, void *data)
>   {
>       DeviceClass *dc = DEVICE_CLASS(klass);
>   
> -    QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END > NPCM7XX_GCR_NR_REGS);
>       dc->realize = npcm_gcr_realize;
>       dc->vmsd = &vmstate_npcm_gcr;
>   
> @@ -247,13 +249,15 @@ static void npcm_gcr_class_init(ObjectClass *klass, void *data)
>   
>   static void npcm7xx_gcr_class_init(ObjectClass *klass, void *data)
>   {
> +    NPCMGCRClass *c = NPCM_GCR_CLASS(klass);
>       DeviceClass *dc = DEVICE_CLASS(klass);
>       ResettableClass *rc = RESETTABLE_CLASS(klass);
>   
> -    QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END != NPCM7XX_GCR_NR_REGS);
>       dc->desc = "NPCM7xx System Global Control Registers";
>       rc->phases.enter = npcm7xx_gcr_enter_reset;
>   
> +    c->nr_regs = NPCM7XX_GCR_NR_REGS;
> +    c->cold_reset_values = npcm7xx_cold_reset_values;
>   }
>   
>   static const TypeInfo npcm_gcr_info[] = {
> @@ -262,6 +266,7 @@ static const TypeInfo npcm_gcr_info[] = {
>           .parent             = TYPE_SYS_BUS_DEVICE,
>           .instance_size      = sizeof(NPCMGCRState),
>           .instance_init      = npcm_gcr_init,
> +        .class_size         = sizeof(NPCMGCRClass),
>           .class_init         = npcm_gcr_class_init,
>           .abstract           = true,
>       },
> diff --git a/include/hw/misc/npcm_gcr.h b/include/hw/misc/npcm_gcr.h
> index 6d3d00d260..9af24e5cdc 100644
> --- a/include/hw/misc/npcm_gcr.h
> +++ b/include/hw/misc/npcm_gcr.h
> @@ -18,6 +18,7 @@
>   
>   #include "exec/memory.h"
>   #include "hw/sysbus.h"
> +#include "qom/object.h"
>   
>   /*
>    * NPCM7XX PWRON STRAP bit fields
> @@ -53,6 +54,7 @@
>    * Number of registers in our device state structure. Don't change this without
>    * incrementing the version_id in the vmstate.
>    */
> +#define NPCM_GCR_MAX_NR_REGS NPCM7XX_GCR_NR_REGS
>   #define NPCM7XX_GCR_NR_REGS (0x148 / sizeof(uint32_t))
>   
>   typedef struct NPCMGCRState {
> @@ -60,15 +62,22 @@ typedef struct NPCMGCRState {
>   
>       MemoryRegion iomem;
>   
> -    uint32_t regs[NPCM7XX_GCR_NR_REGS];
> +    uint32_t regs[NPCM_GCR_MAX_NR_REGS];
>   
>       uint32_t reset_pwron;
>       uint32_t reset_mdlr;
>       uint32_t reset_intcr3;
>   } NPCMGCRState;
>   
> +typedef struct NPCMGCRClass {
> +    SysBusDeviceClass parent;
> +
> +    size_t nr_regs;
> +    const uint32_t *cold_reset_values;
> +} NPCMGCRClass;
> +
>   #define TYPE_NPCM_GCR "npcm-gcr"
>   #define TYPE_NPCM7XX_GCR "npcm7xx-gcr"
> -OBJECT_DECLARE_SIMPLE_TYPE(NPCMGCRState, NPCM_GCR)
> +OBJECT_DECLARE_TYPE(NPCMGCRState, NPCMGCRClass, NPCM_GCR)
>   
>   #endif /* NPCM_GCR_H */
Re: [PATCH v5 06/17] hw/misc: Add nr_regs and cold_reset_values to NPCM GCR
Posted by Hao Wu 2 weeks, 3 days ago
Thanks! I can review the patch.

On Mon, Feb 24, 2025 at 12:52 PM Pierrick Bouvier <
pierrick.bouvier@linaro.org> wrote:

> Hello,
>
> This patch introduces a buffer-overflow, now reported by address sanitizer.
>
> I sent a patch:
> https://lore.kernel.org/qemu-devel/20250224205053.104959-1-
> pierrick.bouvier@linaro.org/T/#u
>
> You're welcome to review it, or fix the problem differently if there is
> a better approach.
>
> Regards,
> Pierrick
>
> On 2/19/25 10:45, Hao Wu wrote:
> > These 2 values are different between NPCM7XX and NPCM8XX
> > GCRs. So we add them to the class and assign different values
> > to them.
> >
> > Reviewed-by: Peter Maydell <peter.maydell@linaro.org>
> > Signed-off-by: Hao Wu <wuhaotsh@google.com>
> > ---
> >   hw/misc/npcm_gcr.c         | 27 ++++++++++++++++-----------
> >   include/hw/misc/npcm_gcr.h | 13 +++++++++++--
> >   2 files changed, 27 insertions(+), 13 deletions(-)
> >
> > diff --git a/hw/misc/npcm_gcr.c b/hw/misc/npcm_gcr.c
> > index 0959f2e5c4..d89e8c2c3b 100644
> > --- a/hw/misc/npcm_gcr.c
> > +++ b/hw/misc/npcm_gcr.c
> > @@ -66,10 +66,9 @@ enum NPCM7xxGCRRegisters {
> >       NPCM7XX_GCR_SCRPAD          = 0x013c / sizeof(uint32_t),
> >       NPCM7XX_GCR_USB1PHYCTL,
> >       NPCM7XX_GCR_USB2PHYCTL,
> > -    NPCM7XX_GCR_REGS_END,
> >   };
> >
> > -static const uint32_t cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
> > +static const uint32_t npcm7xx_cold_reset_values[NPCM7XX_GCR_NR_REGS] = {
> >       [NPCM7XX_GCR_PDID]          = 0x04a92750,   /* Poleg A1 */
> >       [NPCM7XX_GCR_MISCPE]        = 0x0000ffff,
> >       [NPCM7XX_GCR_SPSWC]         = 0x00000003,
> > @@ -88,8 +87,9 @@ static uint64_t npcm_gcr_read(void *opaque, hwaddr
> offset, unsigned size)
> >   {
> >       uint32_t reg = offset / sizeof(uint32_t);
> >       NPCMGCRState *s = opaque;
> > +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
> >
> > -    if (reg >= NPCM7XX_GCR_NR_REGS) {
> > +    if (reg >= c->nr_regs) {
> >           qemu_log_mask(LOG_GUEST_ERROR,
> >                         "%s: offset 0x%04" HWADDR_PRIx " out of range\n",
> >                         __func__, offset);
> > @@ -106,11 +106,12 @@ static void npcm_gcr_write(void *opaque, hwaddr
> offset,
> >   {
> >       uint32_t reg = offset / sizeof(uint32_t);
> >       NPCMGCRState *s = opaque;
> > +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(s);
> >       uint32_t value = v;
> >
> > -    trace_npcm_gcr_write(offset, value);
> > +    trace_npcm_gcr_write(offset, v);
> >
> > -    if (reg >= NPCM7XX_GCR_NR_REGS) {
> > +    if (reg >= c->nr_regs) {
> >           qemu_log_mask(LOG_GUEST_ERROR,
> >                         "%s: offset 0x%04" HWADDR_PRIx " out of range\n",
> >                         __func__, offset);
> > @@ -156,10 +157,12 @@ static const struct MemoryRegionOps npcm_gcr_ops =
> {
> >   static void npcm7xx_gcr_enter_reset(Object *obj, ResetType type)
> >   {
> >       NPCMGCRState *s = NPCM_GCR(obj);
> > +    NPCMGCRClass *c = NPCM_GCR_GET_CLASS(obj);
> >
> > -    QEMU_BUILD_BUG_ON(sizeof(s->regs) != sizeof(cold_reset_values));
> > -
> > -    memcpy(s->regs, cold_reset_values, sizeof(s->regs));
> > +    g_assert(sizeof(s->regs) >= sizeof(c->cold_reset_values));
> > +    g_assert(sizeof(s->regs) >= c->nr_regs * sizeof(uint32_t));
> > +    memcpy(s->regs, c->cold_reset_values, c->nr_regs *
> sizeof(uint32_t));
> > +    /* These 3 registers are at the same location in both 7xx and 8xx.
> */
> >       s->regs[NPCM7XX_GCR_PWRON] = s->reset_pwron;
> >       s->regs[NPCM7XX_GCR_MDLR] = s->reset_mdlr;
> >       s->regs[NPCM7XX_GCR_INTCR3] = s->reset_intcr3;
> > @@ -224,7 +227,7 @@ static const VMStateDescription vmstate_npcm_gcr = {
> >       .version_id = 1,
> >       .minimum_version_id = 1,
> >       .fields = (const VMStateField[]) {
> > -        VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM7XX_GCR_NR_REGS),
> > +        VMSTATE_UINT32_ARRAY(regs, NPCMGCRState, NPCM_GCR_MAX_NR_REGS),
> >           VMSTATE_END_OF_LIST(),
> >       },
> >   };
> > @@ -238,7 +241,6 @@ static void npcm_gcr_class_init(ObjectClass *klass,
> void *data)
> >   {
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> >
> > -    QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END > NPCM7XX_GCR_NR_REGS);
> >       dc->realize = npcm_gcr_realize;
> >       dc->vmsd = &vmstate_npcm_gcr;
> >
> > @@ -247,13 +249,15 @@ static void npcm_gcr_class_init(ObjectClass
> *klass, void *data)
> >
> >   static void npcm7xx_gcr_class_init(ObjectClass *klass, void *data)
> >   {
> > +    NPCMGCRClass *c = NPCM_GCR_CLASS(klass);
> >       DeviceClass *dc = DEVICE_CLASS(klass);
> >       ResettableClass *rc = RESETTABLE_CLASS(klass);
> >
> > -    QEMU_BUILD_BUG_ON(NPCM7XX_GCR_REGS_END != NPCM7XX_GCR_NR_REGS);
> >       dc->desc = "NPCM7xx System Global Control Registers";
> >       rc->phases.enter = npcm7xx_gcr_enter_reset;
> >
> > +    c->nr_regs = NPCM7XX_GCR_NR_REGS;
> > +    c->cold_reset_values = npcm7xx_cold_reset_values;
> >   }
> >
> >   static const TypeInfo npcm_gcr_info[] = {
> > @@ -262,6 +266,7 @@ static const TypeInfo npcm_gcr_info[] = {
> >           .parent             = TYPE_SYS_BUS_DEVICE,
> >           .instance_size      = sizeof(NPCMGCRState),
> >           .instance_init      = npcm_gcr_init,
> > +        .class_size         = sizeof(NPCMGCRClass),
> >           .class_init         = npcm_gcr_class_init,
> >           .abstract           = true,
> >       },
> > diff --git a/include/hw/misc/npcm_gcr.h b/include/hw/misc/npcm_gcr.h
> > index 6d3d00d260..9af24e5cdc 100644
> > --- a/include/hw/misc/npcm_gcr.h
> > +++ b/include/hw/misc/npcm_gcr.h
> > @@ -18,6 +18,7 @@
> >
> >   #include "exec/memory.h"
> >   #include "hw/sysbus.h"
> > +#include "qom/object.h"
> >
> >   /*
> >    * NPCM7XX PWRON STRAP bit fields
> > @@ -53,6 +54,7 @@
> >    * Number of registers in our device state structure. Don't change
> this without
> >    * incrementing the version_id in the vmstate.
> >    */
> > +#define NPCM_GCR_MAX_NR_REGS NPCM7XX_GCR_NR_REGS
> >   #define NPCM7XX_GCR_NR_REGS (0x148 / sizeof(uint32_t))
> >
> >   typedef struct NPCMGCRState {
> > @@ -60,15 +62,22 @@ typedef struct NPCMGCRState {
> >
> >       MemoryRegion iomem;
> >
> > -    uint32_t regs[NPCM7XX_GCR_NR_REGS];
> > +    uint32_t regs[NPCM_GCR_MAX_NR_REGS];
> >
> >       uint32_t reset_pwron;
> >       uint32_t reset_mdlr;
> >       uint32_t reset_intcr3;
> >   } NPCMGCRState;
> >
> > +typedef struct NPCMGCRClass {
> > +    SysBusDeviceClass parent;
> > +
> > +    size_t nr_regs;
> > +    const uint32_t *cold_reset_values;
> > +} NPCMGCRClass;
> > +
> >   #define TYPE_NPCM_GCR "npcm-gcr"
> >   #define TYPE_NPCM7XX_GCR "npcm7xx-gcr"
> > -OBJECT_DECLARE_SIMPLE_TYPE(NPCMGCRState, NPCM_GCR)
> > +OBJECT_DECLARE_TYPE(NPCMGCRState, NPCMGCRClass, NPCM_GCR)
> >
> >   #endif /* NPCM_GCR_H */
>
>