| Message ID | 20170918085542.13265-1-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [v3] s390x/ccw: create s390 phb for compat reasons as well | expand |
On 09/18/2017 10:55 AM, Cornelia Huck wrote: > d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made > registering the s390 pci host bridge conditional on presense > of the zpci facility bit. Sadly, that breaks migration from > machines that did not use the cpu model (2.7 and previous). > > Create the s390 phb for pre-cpu model machines as well: We can > tweak s390_has_feat() to always indicate the zpci facility bit > when no cpu model is available (on 2.7 and previous compat machines). Looks better now. Thanks > > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > > v2->v3: > - no longer RFC (I tested a bit more) > - removed unrelated hunk > - more verbose patch description > > I'll wait a bit for more acks/reviews and will probably send a pull > request for s390x tomorrow or so before the amount of queued patches > gets out of hand... I have a small amount of patches pending, but then I got sick last week so I still have to sort out things. Lets do the pull request and we can then queue the other patches after they have received proper review. > > --- > target/s390x/cpu_models.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > index c295e641e6..5169379db5 100644 > --- a/target/s390x/cpu_models.c > +++ b/target/s390x/cpu_models.c > @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat) > } > } > #endif > + if (feat == S390_FEAT_ZPCI) { > + return true; > + } > return 0; > } > return test_bit(feat, cpu->model->features); >
On Mon, 18 Sep 2017 10:58:47 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 09/18/2017 10:55 AM, Cornelia Huck wrote: > > d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made > > registering the s390 pci host bridge conditional on presense > > of the zpci facility bit. Sadly, that breaks migration from > > machines that did not use the cpu model (2.7 and previous). > > > > Create the s390 phb for pre-cpu model machines as well: We can > > tweak s390_has_feat() to always indicate the zpci facility bit > > when no cpu model is available (on 2.7 and previous compat machines). > > Looks better now. Thanks > > > > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > > > v2->v3: > > - no longer RFC (I tested a bit more) > > - removed unrelated hunk > > - more verbose patch description > > > > I'll wait a bit for more acks/reviews and will probably send a pull > > request for s390x tomorrow or so before the amount of queued patches > > gets out of hand... > > I have a small amount of patches pending, but then I got sick last week > so I still have to sort out things. Lets do the pull request and we can > then queue the other patches after they have received proper review. Sounds like a plan.
On 18.09.2017 10:55, Cornelia Huck wrote: > d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made > registering the s390 pci host bridge conditional on presense > of the zpci facility bit. Sadly, that breaks migration from > machines that did not use the cpu model (2.7 and previous). > > Create the s390 phb for pre-cpu model machines as well: We can > tweak s390_has_feat() to always indicate the zpci facility bit > when no cpu model is available (on 2.7 and previous compat machines). > > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > > v2->v3: > - no longer RFC (I tested a bit more) > - removed unrelated hunk > - more verbose patch description > > I'll wait a bit for more acks/reviews and will probably send a pull > request for s390x tomorrow or so before the amount of queued patches > gets out of hand... > > --- > target/s390x/cpu_models.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > index c295e641e6..5169379db5 100644 > --- a/target/s390x/cpu_models.c > +++ b/target/s390x/cpu_models.c > @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat) > } > } > #endif > + if (feat == S390_FEAT_ZPCI) { > + return true; > + } > return 0; > } > return test_bit(feat, cpu->model->features); > 1. cpu->model will always be set for QEMU, so you can move that into the ifdef, next do the other checks. You can even send a cleanup to remove the if (kvm_enabled()) check. 2. "return pci_available" ?
On Mon, 18 Sep 2017 14:03:20 +0200 David Hildenbrand <david@redhat.com> wrote: > On 18.09.2017 10:55, Cornelia Huck wrote: > > d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made > > registering the s390 pci host bridge conditional on presense > > of the zpci facility bit. Sadly, that breaks migration from > > machines that did not use the cpu model (2.7 and previous). > > > > Create the s390 phb for pre-cpu model machines as well: We can > > tweak s390_has_feat() to always indicate the zpci facility bit > > when no cpu model is available (on 2.7 and previous compat machines). > > > > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > > > v2->v3: > > - no longer RFC (I tested a bit more) > > - removed unrelated hunk > > - more verbose patch description > > > > I'll wait a bit for more acks/reviews and will probably send a pull > > request for s390x tomorrow or so before the amount of queued patches > > gets out of hand... > > > > --- > > target/s390x/cpu_models.c | 3 +++ > > 1 file changed, 3 insertions(+) > > > > diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > > index c295e641e6..5169379db5 100644 > > --- a/target/s390x/cpu_models.c > > +++ b/target/s390x/cpu_models.c > > @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat) > > } > > } > > #endif > > + if (feat == S390_FEAT_ZPCI) { > > + return true; > > + } > > return 0; > > } > > return test_bit(feat, cpu->model->features); > > > > 1. cpu->model will always be set for QEMU, so you can move that into the > ifdef, next do the other checks. You can even send a cleanup to remove > the if (kvm_enabled()) check. I prefer it the way it is now. There's nothing kvm specific about that bit, and cpu->model always being set is not really obvious. > > 2. "return pci_available" ? I thought about that as well. I think, however, that a better way is just to disable compat machines that rely on pci being available. As you can turn off pci only manually, I'd like to defer this and first get this out of the door, as it fixes an existing problem.
On 18.09.2017 14:11, Cornelia Huck wrote: > On Mon, 18 Sep 2017 14:03:20 +0200 > David Hildenbrand <david@redhat.com> wrote: > >> On 18.09.2017 10:55, Cornelia Huck wrote: >>> d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made >>> registering the s390 pci host bridge conditional on presense >>> of the zpci facility bit. Sadly, that breaks migration from >>> machines that did not use the cpu model (2.7 and previous). >>> >>> Create the s390 phb for pre-cpu model machines as well: We can >>> tweak s390_has_feat() to always indicate the zpci facility bit >>> when no cpu model is available (on 2.7 and previous compat machines). >>> >>> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") >>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>> --- >>> >>> v2->v3: >>> - no longer RFC (I tested a bit more) >>> - removed unrelated hunk >>> - more verbose patch description >>> >>> I'll wait a bit for more acks/reviews and will probably send a pull >>> request for s390x tomorrow or so before the amount of queued patches >>> gets out of hand... >>> >>> --- >>> target/s390x/cpu_models.c | 3 +++ >>> 1 file changed, 3 insertions(+) >>> >>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c >>> index c295e641e6..5169379db5 100644 >>> --- a/target/s390x/cpu_models.c >>> +++ b/target/s390x/cpu_models.c >>> @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat) >>> } >>> } >>> #endif >>> + if (feat == S390_FEAT_ZPCI) { >>> + return true; >>> + } >>> return 0; >>> } >>> return test_bit(feat, cpu->model->features); >>> >> >> 1. cpu->model will always be set for QEMU, so you can move that into the >> ifdef, next do the other checks. You can even send a cleanup to remove >> the if (kvm_enabled()) check. > > I prefer it the way it is now. There's nothing kvm specific about that > bit, and cpu->model always being set is not really obvious. > I's already kvm specific as this can never happen with TCG :) But whatever you prefer. This is good enough to fix the problem.
On Mon, 18 Sep 2017 14:13:07 +0200 David Hildenbrand <david@redhat.com> wrote: > On 18.09.2017 14:11, Cornelia Huck wrote: > > On Mon, 18 Sep 2017 14:03:20 +0200 > > David Hildenbrand <david@redhat.com> wrote: > > > >> On 18.09.2017 10:55, Cornelia Huck wrote: > >>> d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made > >>> registering the s390 pci host bridge conditional on presense > >>> of the zpci facility bit. Sadly, that breaks migration from > >>> machines that did not use the cpu model (2.7 and previous). > >>> > >>> Create the s390 phb for pre-cpu model machines as well: We can > >>> tweak s390_has_feat() to always indicate the zpci facility bit > >>> when no cpu model is available (on 2.7 and previous compat machines). > >>> > >>> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > >>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> > >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> > >>> --- > >>> > >>> v2->v3: > >>> - no longer RFC (I tested a bit more) > >>> - removed unrelated hunk > >>> - more verbose patch description > >>> > >>> I'll wait a bit for more acks/reviews and will probably send a pull > >>> request for s390x tomorrow or so before the amount of queued patches > >>> gets out of hand... > >>> > >>> --- > >>> target/s390x/cpu_models.c | 3 +++ > >>> 1 file changed, 3 insertions(+) > >>> > >>> diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c > >>> index c295e641e6..5169379db5 100644 > >>> --- a/target/s390x/cpu_models.c > >>> +++ b/target/s390x/cpu_models.c > >>> @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat) > >>> } > >>> } > >>> #endif > >>> + if (feat == S390_FEAT_ZPCI) { > >>> + return true; > >>> + } > >>> return 0; > >>> } > >>> return test_bit(feat, cpu->model->features); > >>> > >> > >> 1. cpu->model will always be set for QEMU, so you can move that into the > >> ifdef, next do the other checks. You can even send a cleanup to remove > >> the if (kvm_enabled()) check. > > > > I prefer it the way it is now. There's nothing kvm specific about that > > bit, and cpu->model always being set is not really obvious. > > > > I's already kvm specific as this can never happen with TCG :) > > But whatever you prefer. This is good enough to fix the problem. Sure, we can revisit that one later. I plan to send a pull request tomorrow.
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c index c295e641e6..5169379db5 100644 --- a/target/s390x/cpu_models.c +++ b/target/s390x/cpu_models.c @@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat) } } #endif + if (feat == S390_FEAT_ZPCI) { + return true; + } return 0; } return test_bit(feat, cpu->model->features);
d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made registering the s390 pci host bridge conditional on presense of the zpci facility bit. Sadly, that breaks migration from machines that did not use the cpu model (2.7 and previous). Create the s390 phb for pre-cpu model machines as well: We can tweak s390_has_feat() to always indicate the zpci facility bit when no cpu model is available (on 2.7 and previous compat machines). Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") Acked-by: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- v2->v3: - no longer RFC (I tested a bit more) - removed unrelated hunk - more verbose patch description I'll wait a bit for more acks/reviews and will probably send a pull request for s390x tomorrow or so before the amount of queued patches gets out of hand... --- target/s390x/cpu_models.c | 3 +++ 1 file changed, 3 insertions(+)