Re: [PATCH v2 07/17] hw/misc: Add nr_regs and cold_reset_values to NPCM GCR

[Peter Maydell]

On Thu, 26 Dec 2024 at 08:28, 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.
>
> Signed-off-by: Hao Wu <wuhaotsh@google.com>
> ---
>  hw/misc/npcm_gcr.c         | 24 +++++++++++++++---------
>  include/hw/misc/npcm_gcr.h | 13 +++++++++++--
>  2 files changed, 26 insertions(+), 11 deletions(-)
>



> @@ -156,10 +158,10 @@ 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));
> +    memcpy(s->regs, c->cold_reset_values, c->nr_regs * sizeof(uint32_t));

Previously we had a compile-time assert that we were copying
the right amount of data. Making this a class-specific
thing means we can't assert at compile time any more, but
I think it's still worth a runtime assert that we aren't
copying more data than will fit into s->regs.

Otherwise
Reviewed-by: Peter Maydell <peter.maydell@linaro.org>

thanks
-- PMM