diff mbox series

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

Message ID 20240523230747.45703-26-npiggin@gmail.com
State Not Applicable
Headers show
Series [PULL,01/72] spapr: avoid overhead of finding vhyp class in critical operations | expand

Commit Message

Nicholas Piggin May 23, 2024, 11:06 p.m. UTC 
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(+)

Comments

Fabiano Rosas May 31, 2024, 8:06 p.m. UTC | #1 
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.
Nicholas Piggin June 3, 2024, 4:32 a.m. UTC | #2 
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 mbox series

Patch

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,