On Thu, 3 Mar 2022 at 17:44, Mark Cave-Ayland
<mark.cave-ayl...@ilande.co.uk> wrote:
>
> On 03/03/2022 15:25, Peter Maydell wrote:
>
> > On Wed, 2 Mar 2022 at 21:31, Mark Cave-Ayland
> > <mark.cave-ayl...@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-ayl...@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.mayd...@linaro.org>
>
> I can do this if you like, although until the last 2 patches anything that is
> using
> the disk will fail, and that's just about everything because DMA requests
> require
> guest support to move the data from the ESP to the CPU.
>
> In terms of the q800 machine there is an implicit assumption that there will
> be more
> migration breaks to come, mainly because there are new devices to be added to
> the
> q800 machine in my outstanding MacOS patches that will break migration again
> once. So
> until these are finally merged I don't think it's worth trying to stabilise
> the
> migration stream.
Yeah, fair enough; just put a note in the commit message to
that effect. (Mostly bumping the migration version is about making
the error message nicer if somebody does do a mismatched save/load.)
thanks
-- PMM
