Re: [PULL 25/72] Adds migration support for Branch History Rolling Buffer (BHRB) internal state.

[Nicholas Piggin]

On Sat Jun 1, 2024 at 6:06 AM AEST, Fabiano Rosas wrote:
> Nicholas Piggin <npiggin@gmail.com> writes:
>
> > From: Glenn Miles <milesg@linux.vnet.ibm.com>
> >
> > Reviewed-by: Nicholas Piggin <npiggin@gmail.com>
> > Signed-off-by: Glenn Miles <milesg@linux.vnet.ibm.com>
> > Signed-off-by: Nicholas Piggin <npiggin@gmail.com>
> > ---
> >  target/ppc/machine.c | 21 +++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/target/ppc/machine.c b/target/ppc/machine.c
> > index 6b6c31d903..731dd8df35 100644
> > --- a/target/ppc/machine.c
> > +++ b/target/ppc/machine.c
> > @@ -711,6 +711,26 @@ static const VMStateDescription vmstate_reservation = {
> >      }
> >  };
> >  
> > +#ifdef TARGET_PPC64
> > +static bool bhrb_needed(void *opaque)
> > +{
> > +    PowerPCCPU *cpu = opaque;
> > +    return (cpu->env.flags & POWERPC_FLAG_BHRB) != 0;
> > +}
> > +
> > +static const VMStateDescription vmstate_bhrb = {
> > +    .name = "cpu/bhrb",
> > +    .version_id = 1,
> > +    .minimum_version_id = 1,
> > +    .needed = bhrb_needed,
> > +    .fields = (VMStateField[]) {
> > +        VMSTATE_UINTTL(env.bhrb_offset, PowerPCCPU),
> > +        VMSTATE_UINT64_ARRAY(env.bhrb, PowerPCCPU, BHRB_MAX_NUM_ENTRIES),
> > +        VMSTATE_END_OF_LIST()
> > +    }
> > +};
> > +#endif
> > +
> >  const VMStateDescription vmstate_ppc_cpu = {
> >      .name = "cpu",
> >      .version_id = 5,
> > @@ -756,6 +776,7 @@ const VMStateDescription vmstate_ppc_cpu = {
> >  #ifdef TARGET_PPC64
> >          &vmstate_tm,
> >          &vmstate_slb,
> > +        &vmstate_bhrb,
>
> Running some tests now that Nick re-enabled ppc for migration tests, I
> see that this new state breaks backward migrations:
>
> $ QTEST_TRACE="vmstate_*" \
>   QTEST_QEMU_BINARY_DST=../build-previous/qemu-system-ppc64 \
>   QTEST_QEMU_BINARY=./qemu-system-ppc64 \
>   ./tests/qtest/migration-test -p /ppc64/migration/precopy/tcp/plain
> ...
> vmstate_load_state_field cpu/slb:env.slb
> vmstate_n_elems env.slb: 64
> vmstate_subsection_load cpu/slb
> vmstate_subsection_load_bad cpu/slb: cpu/bhrb/(prefix)
> vmstate_load_state_end cpu/slb end/0
> vmstate_subsection_load_bad cpu: cpu/bhrb/(lookup)
> qemu-system-ppc64: error while loading state for instance 0x0 of device 'cpu'
> vmstate_downtime_checkpoint dst-precopy-loadvm-completed
> qemu-system-ppc64: load of migration failed: No such file or directory
>
> If you want to support backwards migration, then this needs to be
> fixed. Otherwise we can ignore it.


Hey Fabiano,

Thanks for picking that up, I missed it.

I think we could just say it's not needed, or only needed in
case we are doing replay. I will work on a fix.

Thanks,
Nick