| Message ID | 20170915163952.32147-2-cohuck@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | s390x: pci compat handling | expand |
On 09/15/2017 06:39 PM, 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. > > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > --- > hw/s390x/s390-virtio-ccw.c | 2 ++ > target/s390x/cpu_models.c | 3 +++ > 2 files changed, 5 insertions(+) > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > index 0471407187..907abc7a32 100644 > --- a/hw/s390x/s390-virtio-ccw.c > +++ b/hw/s390x/s390-virtio-ccw.c > @@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name) > } > } > > +static S390CcwMachineClass *get_machine_class(void); > + > static void ccw_init(MachineState *machine) > { > int ret; > 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; > + } Shouldnt that be depend on the machine being 2.7? I mean unless I misread the context of this patch, you hard enable the PCI facility bit for all machines and make all s390_has_feat(S390_FEAT_ZPCI) useless? > return 0; > } > return test_bit(feat, cpu->model->features); >
On 09/18/2017 09:33 AM, Christian Borntraeger wrote: > > > On 09/15/2017 06:39 PM, 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. >> >> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >> --- >> hw/s390x/s390-virtio-ccw.c | 2 ++ >> target/s390x/cpu_models.c | 3 +++ >> 2 files changed, 5 insertions(+) >> >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >> index 0471407187..907abc7a32 100644 >> --- a/hw/s390x/s390-virtio-ccw.c >> +++ b/hw/s390x/s390-virtio-ccw.c >> @@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name) >> } >> } >> >> +static S390CcwMachineClass *get_machine_class(void); >> + >> static void ccw_init(MachineState *machine) >> { >> int ret; >> 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; >> + } > > Shouldnt that be depend on the machine being 2.7? I mean unless I misread > the context of this patch, you hard enable the PCI facility bit for all > machines and make all s390_has_feat(S390_FEAT_ZPCI) useless? > Sorry, I had to lookup again the original code. So this is inside !cpu->model. Yes makes sense then.
On Mon, 18 Sep 2017 09:33:23 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 09/15/2017 06:39 PM, 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. > > > > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > > Signed-off-by: Cornelia Huck <cohuck@redhat.com> > > --- > > hw/s390x/s390-virtio-ccw.c | 2 ++ > > target/s390x/cpu_models.c | 3 +++ > > 2 files changed, 5 insertions(+) > > > > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > > index 0471407187..907abc7a32 100644 > > --- a/hw/s390x/s390-virtio-ccw.c > > +++ b/hw/s390x/s390-virtio-ccw.c > > @@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name) > > } > > } > > > > +static S390CcwMachineClass *get_machine_class(void); > > + > > static void ccw_init(MachineState *machine) > > { > > int ret; > > 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; > > + } > > Shouldnt that be depend on the machine being 2.7? I mean unless I misread > the context of this patch, you hard enable the PCI facility bit for all > machines and make all s390_has_feat(S390_FEAT_ZPCI) useless? That's for machines without cpu model, which implies 2.7 or earlier, no? > > > > return 0; > > } > > return test_bit(feat, cpu->model->features); > > >
On Mon, 18 Sep 2017 09:47:00 +0200 Christian Borntraeger <borntraeger@de.ibm.com> wrote: > On 09/18/2017 09:33 AM, Christian Borntraeger wrote: > > > > > > On 09/15/2017 06:39 PM, 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. > >> > >> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") > >> Signed-off-by: Cornelia Huck <cohuck@redhat.com> > >> --- > >> hw/s390x/s390-virtio-ccw.c | 2 ++ > >> target/s390x/cpu_models.c | 3 +++ > >> 2 files changed, 5 insertions(+) > >> > >> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c > >> index 0471407187..907abc7a32 100644 > >> --- a/hw/s390x/s390-virtio-ccw.c > >> +++ b/hw/s390x/s390-virtio-ccw.c > >> @@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name) > >> } > >> } > >> > >> +static S390CcwMachineClass *get_machine_class(void); > >> + > >> static void ccw_init(MachineState *machine) > >> { > >> int ret; This hunk is from the previous version and should not be in there... > >> 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; > >> + } > > > > Shouldnt that be depend on the machine being 2.7? I mean unless I misread > > the context of this patch, you hard enable the PCI facility bit for all > > machines and make all s390_has_feat(S390_FEAT_ZPCI) useless? > > > > Sorry, I had to lookup again the original code. So this is inside !cpu->model. > Yes makes sense then. Yes, this is hard to figure out from the context alone.
On 09/18/2017 10:07 AM, Cornelia Huck wrote: > On Mon, 18 Sep 2017 09:47:00 +0200 > Christian Borntraeger <borntraeger@de.ibm.com> wrote: > >> On 09/18/2017 09:33 AM, Christian Borntraeger wrote: >>> >>> >>> On 09/15/2017 06:39 PM, 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. >>>> >>>> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") >>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com> >>>> --- >>>> hw/s390x/s390-virtio-ccw.c | 2 ++ >>>> target/s390x/cpu_models.c | 3 +++ >>>> 2 files changed, 5 insertions(+) >>>> >>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c >>>> index 0471407187..907abc7a32 100644 >>>> --- a/hw/s390x/s390-virtio-ccw.c >>>> +++ b/hw/s390x/s390-virtio-ccw.c >>>> @@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name) >>>> } >>>> } >>>> >>>> +static S390CcwMachineClass *get_machine_class(void); >>>> + >>>> static void ccw_init(MachineState *machine) >>>> { >>>> int ret; > > This hunk is from the previous version and should not be in there... > >>>> 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; >>>> + } >>> >>> Shouldnt that be depend on the machine being 2.7? I mean unless I misread >>> the context of this patch, you hard enable the PCI facility bit for all >>> machines and make all s390_has_feat(S390_FEAT_ZPCI) useless? >>> >> >> Sorry, I had to lookup again the original code. So this is inside !cpu->model. >> Yes makes sense then. > > Yes, this is hard to figure out from the context alone. So with an improved patch description (explain the fix better) Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c index 0471407187..907abc7a32 100644 --- a/hw/s390x/s390-virtio-ccw.c +++ b/hw/s390x/s390-virtio-ccw.c @@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name) } } +static S390CcwMachineClass *get_machine_class(void); + static void ccw_init(MachineState *machine) { int ret; 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. Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally") Signed-off-by: Cornelia Huck <cohuck@redhat.com> --- hw/s390x/s390-virtio-ccw.c | 2 ++ target/s390x/cpu_models.c | 3 +++ 2 files changed, 5 insertions(+)