Re: [PATCH v2 03/10] macfb: increase number of registers saved in MacfbState

[Peter Maydell]

On Wed, 2 Mar 2022 at 21:31, Mark Cave-Ayland
<mark.cave-ayland@ilande.co.uk> wrote:
>
> The MacOS toolbox ROM accesses a number of addresses between 0x0 and 0x200 
> during
> initialisation and resolution changes. Whilst the function of many of these
> registers is unknown, it is worth the minimal cost of saving these extra 
> values as
> part of migration to help future-proof the migration stream for the q800 
> machine
> as it starts to stabilise.
>
> Signed-off-by: Mark Cave-Ayland <mark.cave-ayland@ilande.co.uk>
> ---
>  hw/display/macfb.c         | 8 ++++++++
>  include/hw/display/macfb.h | 3 ++-
>  2 files changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/macfb.c b/hw/display/macfb.c
> index fb54b460c1..dfdae90144 100644
> --- a/hw/display/macfb.c
> +++ b/hw/display/macfb.c
> @@ -537,6 +537,10 @@ static uint64_t macfb_ctrl_read(void *opaque,
>      case DAFB_MODE_SENSE:
>          val = macfb_sense_read(s);
>          break;
> +    default:
> +        if (addr < MACFB_CTRL_TOPADDR) {
> +            val = s->regs[addr >> 2];
> +        }
>      }
>
>      trace_macfb_ctrl_read(addr, val, size);
> @@ -592,6 +596,10 @@ static void macfb_ctrl_write(void *opaque,
>              macfb_invalidate_display(s);
>          }
>          break;
> +    default:
> +        if (addr < MACFB_CTRL_TOPADDR) {
> +            s->regs[addr >> 2] = val;
> +        }
>      }
>
>      trace_macfb_ctrl_write(addr, val, size);
> diff --git a/include/hw/display/macfb.h b/include/hw/display/macfb.h
> index 6d9f0f7869..55a50d3fb0 100644
> --- a/include/hw/display/macfb.h
> +++ b/include/hw/display/macfb.h
> @@ -48,7 +48,8 @@ typedef struct MacFbMode {
>      uint32_t offset;
>  } MacFbMode;
>
> -#define MACFB_NUM_REGS      8
> +#define MACFB_CTRL_TOPADDR  0x200
> +#define MACFB_NUM_REGS      (MACFB_CTRL_TOPADDR / sizeof(uint32_t))

You should either bump the vmstate_macfb version numbers here,
or at least note in the commit message that although it's a
migration break we know nobody's migrating this device because
of the bug we just fixed in the previous commit.

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

thanks
-- PMM