* [Qemu-devel] [PATCH RFC v2 0/1] s390x: pci compat handling
@ 2017-09-15 16:39 Cornelia Huck
2017-09-15 16:39 ` [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well Cornelia Huck
0 siblings, 1 reply; 7+ messages in thread
From: Cornelia Huck @ 2017-09-15 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic,
Cornelia Huck
As David has identified the reason why only 2.7 and older are affected
(thanks!), we can fix migration of these machines by unconditionally
providing the zpci feature bit if cpu models are off.
Still RFC, as I have only sniff-tested it.
We still need to think about what to do with compat machines on pci-less
builds. Probably we should disable compat machines (at least 2.7 and older)
on those builds. Not a pressing issue, as switching off CONFIG_PCI needs
to be done manually.
Changes RFC -> RFC v2:
- switch from machine class property to enabling zpci bit for non-cpu model
compat machines
Cornelia Huck (1):
s390x/ccw: create s390 phb for compat reasons as well
hw/s390x/s390-virtio-ccw.c | 2 ++
target/s390x/cpu_models.c | 3 +++
2 files changed, 5 insertions(+)
--
2.13.5
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well
2017-09-15 16:39 [Qemu-devel] [PATCH RFC v2 0/1] s390x: pci compat handling Cornelia Huck
@ 2017-09-15 16:39 ` Cornelia Huck
2017-09-18 7:33 ` Christian Borntraeger
0 siblings, 1 reply; 7+ messages in thread
From: Cornelia Huck @ 2017-09-15 16:39 UTC (permalink / raw)
To: qemu-devel
Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic,
Cornelia Huck
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;
+ }
return 0;
}
return test_bit(feat, cpu->model->features);
--
2.13.5
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well
2017-09-15 16:39 ` [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well Cornelia Huck
@ 2017-09-18 7:33 ` Christian Borntraeger
2017-09-18 7:47 ` Christian Borntraeger
2017-09-18 7:48 ` Cornelia Huck
0 siblings, 2 replies; 7+ messages in thread
From: Christian Borntraeger @ 2017-09-18 7:33 UTC (permalink / raw)
To: Cornelia Huck, qemu-devel; +Cc: agraf, thuth, david, pmorel, zyimin, pasic
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);
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well
2017-09-18 7:33 ` Christian Borntraeger
@ 2017-09-18 7:47 ` Christian Borntraeger
2017-09-18 8:07 ` Cornelia Huck
2017-09-18 7:48 ` Cornelia Huck
1 sibling, 1 reply; 7+ messages in thread
From: Christian Borntraeger @ 2017-09-18 7:47 UTC (permalink / raw)
To: Cornelia Huck, qemu-devel; +Cc: agraf, thuth, david, pmorel, zyimin, pasic
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well
2017-09-18 7:33 ` Christian Borntraeger
2017-09-18 7:47 ` Christian Borntraeger
@ 2017-09-18 7:48 ` Cornelia Huck
1 sibling, 0 replies; 7+ messages in thread
From: Cornelia Huck @ 2017-09-18 7:48 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, agraf, thuth, david, pmorel, zyimin, pasic
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);
> >
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well
2017-09-18 7:47 ` Christian Borntraeger
@ 2017-09-18 8:07 ` Cornelia Huck
2017-09-18 8:35 ` Christian Borntraeger
0 siblings, 1 reply; 7+ messages in thread
From: Cornelia Huck @ 2017-09-18 8:07 UTC (permalink / raw)
To: Christian Borntraeger
Cc: qemu-devel, agraf, thuth, david, pmorel, zyimin, pasic
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.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well
2017-09-18 8:07 ` Cornelia Huck
@ 2017-09-18 8:35 ` Christian Borntraeger
0 siblings, 0 replies; 7+ messages in thread
From: Christian Borntraeger @ 2017-09-18 8:35 UTC (permalink / raw)
To: Cornelia Huck; +Cc: qemu-devel, agraf, thuth, david, pmorel, zyimin, pasic
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>
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-09-18 8:36 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-09-15 16:39 [Qemu-devel] [PATCH RFC v2 0/1] s390x: pci compat handling Cornelia Huck
2017-09-15 16:39 ` [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well Cornelia Huck
2017-09-18 7:33 ` Christian Borntraeger
2017-09-18 7:47 ` Christian Borntraeger
2017-09-18 8:07 ` Cornelia Huck
2017-09-18 8:35 ` Christian Borntraeger
2017-09-18 7:48 ` Cornelia Huck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).