| Message ID | 20240523230747.45703-26-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/72] spapr: avoid overhead of finding vhyp class in critical operations | expand |
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.
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
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, #endif /* TARGET_PPC64 */ &vmstate_tlb6xx, &vmstate_tlbemb,