| Message ID | 20190522044600.16534-22-david@gibson.dropbear.id.au |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/38] target/ppc/kvm: Fix trace typo | expand |
On Wed, 22 May 2019 14:45:43 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > spapr machine capabilities are supposed to be sent in the migration stream > so that we can sanity check the source and destination have compatible > configuration. Unfortunately, when we added the hpt-max-page-size > capability, we forgot to add it to the migration state. This means that we > can generate spurious warnings when both ends are configured for large > pages, or potentially fail to warn if the source is configured for huge > pages, but the destination is not. > > Fixes: 2309832afda "spapr: Maximum (HPT) pagesize property" > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- Huh... we discussed that it was breaking backward migration: https://lists.gnu.org/archive/html/qemu-ppc/2019-05/msg00330.html So I'm a bit surprised to see this in the PR... is it intentional ? > hw/ppc/spapr.c | 1 + > hw/ppc/spapr_caps.c | 1 + > include/hw/ppc/spapr.h | 1 + > 3 files changed, 3 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 8580a8dc67..bcae30ad26 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -2125,6 +2125,7 @@ static const VMStateDescription vmstate_spapr = { > &vmstate_spapr_cap_cfpc, > &vmstate_spapr_cap_sbbc, > &vmstate_spapr_cap_ibs, > + &vmstate_spapr_cap_hpt_maxpagesize, > &vmstate_spapr_irq_map, > &vmstate_spapr_cap_nested_kvm_hv, > &vmstate_spapr_dtb, > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > index 9b1c10baa6..658eb15a14 100644 > --- a/hw/ppc/spapr_caps.c > +++ b/hw/ppc/spapr_caps.c > @@ -703,6 +703,7 @@ SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP); > SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC); > SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC); > SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS); > +SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); > SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); > SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); > SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > index 7e32f309c2..9fc91c8f5e 100644 > --- a/include/hw/ppc/spapr.h > +++ b/include/hw/ppc/spapr.h > @@ -849,6 +849,7 @@ extern const VMStateDescription vmstate_spapr_cap_dfp; > extern const VMStateDescription vmstate_spapr_cap_cfpc; > extern const VMStateDescription vmstate_spapr_cap_sbbc; > extern const VMStateDescription vmstate_spapr_cap_ibs; > +extern const VMStateDescription vmstate_spapr_cap_hpt_maxpagesize; > extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv; > extern const VMStateDescription vmstate_spapr_cap_large_decr; > extern const VMStateDescription vmstate_spapr_cap_ccf_assist;
On Wed, May 22, 2019 at 09:58:29AM +0200, Greg Kurz wrote: > On Wed, 22 May 2019 14:45:43 +1000 > David Gibson <david@gibson.dropbear.id.au> wrote: > > > spapr machine capabilities are supposed to be sent in the migration stream > > so that we can sanity check the source and destination have compatible > > configuration. Unfortunately, when we added the hpt-max-page-size > > capability, we forgot to add it to the migration state. This means that we > > can generate spurious warnings when both ends are configured for large > > pages, or potentially fail to warn if the source is configured for huge > > pages, but the destination is not. > > > > Fixes: 2309832afda "spapr: Maximum (HPT) pagesize property" > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > --- > > Huh... we discussed that it was breaking backward migration: > > https://lists.gnu.org/archive/html/qemu-ppc/2019-05/msg00330.html > > So I'm a bit surprised to see this in the PR... is it intentional ? Sod, no, I forgot to remove it from my tree. Having been through the test cycle, I'd prefer not to hold up the PR for this - as long as we fix it before the release we should be ok. > > > hw/ppc/spapr.c | 1 + > > hw/ppc/spapr_caps.c | 1 + > > include/hw/ppc/spapr.h | 1 + > > 3 files changed, 3 insertions(+) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index 8580a8dc67..bcae30ad26 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > > @@ -2125,6 +2125,7 @@ static const VMStateDescription vmstate_spapr = { > > &vmstate_spapr_cap_cfpc, > > &vmstate_spapr_cap_sbbc, > > &vmstate_spapr_cap_ibs, > > + &vmstate_spapr_cap_hpt_maxpagesize, > > &vmstate_spapr_irq_map, > > &vmstate_spapr_cap_nested_kvm_hv, > > &vmstate_spapr_dtb, > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > > index 9b1c10baa6..658eb15a14 100644 > > --- a/hw/ppc/spapr_caps.c > > +++ b/hw/ppc/spapr_caps.c > > @@ -703,6 +703,7 @@ SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP); > > SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC); > > SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC); > > SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS); > > +SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); > > SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); > > SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); > > SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index 7e32f309c2..9fc91c8f5e 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -849,6 +849,7 @@ extern const VMStateDescription vmstate_spapr_cap_dfp; > > extern const VMStateDescription vmstate_spapr_cap_cfpc; > > extern const VMStateDescription vmstate_spapr_cap_sbbc; > > extern const VMStateDescription vmstate_spapr_cap_ibs; > > +extern const VMStateDescription vmstate_spapr_cap_hpt_maxpagesize; > > extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv; > > extern const VMStateDescription vmstate_spapr_cap_large_decr; > > extern const VMStateDescription vmstate_spapr_cap_ccf_assist; >
On Wed, 22 May 2019 21:10:35 +1000 David Gibson <david@gibson.dropbear.id.au> wrote: > On Wed, May 22, 2019 at 09:58:29AM +0200, Greg Kurz wrote: > > On Wed, 22 May 2019 14:45:43 +1000 > > David Gibson <david@gibson.dropbear.id.au> wrote: > > > > > spapr machine capabilities are supposed to be sent in the migration stream > > > so that we can sanity check the source and destination have compatible > > > configuration. Unfortunately, when we added the hpt-max-page-size > > > capability, we forgot to add it to the migration state. This means that we > > > can generate spurious warnings when both ends are configured for large > > > pages, or potentially fail to warn if the source is configured for huge > > > pages, but the destination is not. > > > > > > Fixes: 2309832afda "spapr: Maximum (HPT) pagesize property" > > > > > > Signed-off-by: David Gibson <david@gibson.dropbear.id.au> > > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > > > --- > > > > Huh... we discussed that it was breaking backward migration: > > > > https://lists.gnu.org/archive/html/qemu-ppc/2019-05/msg00330.html > > > > So I'm a bit surprised to see this in the PR... is it intentional ? > > Sod, no, I forgot to remove it from my tree. > > Having been through the test cycle, I'd prefer not to hold up the PR > for this - as long as we fix it before the release we should be ok. > Fair enough, I'll re-post my fix proposal in a proper patch ASAP. > > > > > hw/ppc/spapr.c | 1 + > > > hw/ppc/spapr_caps.c | 1 + > > > include/hw/ppc/spapr.h | 1 + > > > 3 files changed, 3 insertions(+) > > > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > > index 8580a8dc67..bcae30ad26 100644 > > > --- a/hw/ppc/spapr.c > > > +++ b/hw/ppc/spapr.c > > > @@ -2125,6 +2125,7 @@ static const VMStateDescription vmstate_spapr = { > > > &vmstate_spapr_cap_cfpc, > > > &vmstate_spapr_cap_sbbc, > > > &vmstate_spapr_cap_ibs, > > > + &vmstate_spapr_cap_hpt_maxpagesize, > > > &vmstate_spapr_irq_map, > > > &vmstate_spapr_cap_nested_kvm_hv, > > > &vmstate_spapr_dtb, > > > diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c > > > index 9b1c10baa6..658eb15a14 100644 > > > --- a/hw/ppc/spapr_caps.c > > > +++ b/hw/ppc/spapr_caps.c > > > @@ -703,6 +703,7 @@ SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP); > > > SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC); > > > SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC); > > > SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS); > > > +SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); > > > SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); > > > SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); > > > SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); > > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > > index 7e32f309c2..9fc91c8f5e 100644 > > > --- a/include/hw/ppc/spapr.h > > > +++ b/include/hw/ppc/spapr.h > > > @@ -849,6 +849,7 @@ extern const VMStateDescription vmstate_spapr_cap_dfp; > > > extern const VMStateDescription vmstate_spapr_cap_cfpc; > > > extern const VMStateDescription vmstate_spapr_cap_sbbc; > > > extern const VMStateDescription vmstate_spapr_cap_ibs; > > > +extern const VMStateDescription vmstate_spapr_cap_hpt_maxpagesize; > > > extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv; > > > extern const VMStateDescription vmstate_spapr_cap_large_decr; > > > extern const VMStateDescription vmstate_spapr_cap_ccf_assist; > > >
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 8580a8dc67..bcae30ad26 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -2125,6 +2125,7 @@ static const VMStateDescription vmstate_spapr = { &vmstate_spapr_cap_cfpc, &vmstate_spapr_cap_sbbc, &vmstate_spapr_cap_ibs, + &vmstate_spapr_cap_hpt_maxpagesize, &vmstate_spapr_irq_map, &vmstate_spapr_cap_nested_kvm_hv, &vmstate_spapr_dtb, diff --git a/hw/ppc/spapr_caps.c b/hw/ppc/spapr_caps.c index 9b1c10baa6..658eb15a14 100644 --- a/hw/ppc/spapr_caps.c +++ b/hw/ppc/spapr_caps.c @@ -703,6 +703,7 @@ SPAPR_CAP_MIG_STATE(dfp, SPAPR_CAP_DFP); SPAPR_CAP_MIG_STATE(cfpc, SPAPR_CAP_CFPC); SPAPR_CAP_MIG_STATE(sbbc, SPAPR_CAP_SBBC); SPAPR_CAP_MIG_STATE(ibs, SPAPR_CAP_IBS); +SPAPR_CAP_MIG_STATE(hpt_maxpagesize, SPAPR_CAP_HPT_MAXPAGESIZE); SPAPR_CAP_MIG_STATE(nested_kvm_hv, SPAPR_CAP_NESTED_KVM_HV); SPAPR_CAP_MIG_STATE(large_decr, SPAPR_CAP_LARGE_DECREMENTER); SPAPR_CAP_MIG_STATE(ccf_assist, SPAPR_CAP_CCF_ASSIST); diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h index 7e32f309c2..9fc91c8f5e 100644 --- a/include/hw/ppc/spapr.h +++ b/include/hw/ppc/spapr.h @@ -849,6 +849,7 @@ extern const VMStateDescription vmstate_spapr_cap_dfp; extern const VMStateDescription vmstate_spapr_cap_cfpc; extern const VMStateDescription vmstate_spapr_cap_sbbc; extern const VMStateDescription vmstate_spapr_cap_ibs; +extern const VMStateDescription vmstate_spapr_cap_hpt_maxpagesize; extern const VMStateDescription vmstate_spapr_cap_nested_kvm_hv; extern const VMStateDescription vmstate_spapr_cap_large_decr; extern const VMStateDescription vmstate_spapr_cap_ccf_assist;