s390x: pci compat handling
cover s390x: pci compat handling
While playing around with compat machines a bit, I noticed that my zpci detanglement patches broke migration from 2.7 to current master (2.8 or newer are fine, which is why I did not notice that before.)
qemu 2.7 seems to create a savevm that a s390-next (or master) build without the s390 phb chokes on:
qemu-system-s390x: Unknown savevm section or instance 'PCIBUS' 0
Creating the s390 phb for compat machines seems to cure this; still RFC for the following reasons:
- I'm not sure what we're supposed to do on builds without pci. Fail creating the compat machines? Do we need a new set of _NOPCI compat machines for that?
- I don't understand why 2.7 fails, but 2.8 and later are fine. At least, I was not able to spot which commit changed the behaviour here... and I really want to understand this. Pointers welcome.
- I have not tested it extensively yet.
Patch is against s390-next.
Cornelia Huck (1):
s390x/ccw: create s390 phb for compat reasons as well
hw/s390x/s390-virtio-ccw.c | 6 +++++- include/hw/s390x/s390-virtio-ccw.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)
Signature · 1 line
2.13.5
RFC PATCH 1/1 s390x/ccw: create s390 phb for compat reasons as well
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 some old machines.
Create the s390 phb if we need it for compat reasons, even if we don't provide the zpci facility.
--- hw/s390x/s390-virtio-ccw.c | 6 +++++- include/hw/s390x/s390-virtio-ccw.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)
hw/s390x/s390-virtio-ccw.c +5 -1
@@ -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;
@@ -288,7 +290,7 @@ static void ccw_init(MachineState *machine) machine->initrd_filename,"s390-ccw.img" , "s390-netboot.img", true); -if (s390_has_feat (S390_FEAT_ZPCI )) { +if (s390_has_feat (S390_FEAT_ZPCI ) ||get_machine_class ()->pci_compat) {DeviceState *dev =qdev_create (NULL ,TYPE_S390_PCI_HOST_BRIDGE ); object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
@@ -429,6 +431,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->cpu_model_allowed = true; s390mc->css_migration_enabled = true; s390mc->gs_allowed = true; + s390mc->pci_compat = false; mc->init = ccw_init; mc->reset = s390_machine_reset; mc->hot_add_cpu = s390_hot_add_cpu;
@@ -784,6 +787,7 @@ static void ccw_machine_2_7_class_options(MachineClass *mc)S390CcwMachineClass *s390mc =S390_MACHINE_CLASS (mc); s390mc->cpu_model_allowed = false; + s390mc->pci_compat = pci_available;ccw_machine_2_8_class_options (mc);SET_MACHINE_COMPAT (mc,CCW_COMPAT_2_7 ); }
include/hw/s390x/s390-virtio-ccw.h +1 -0
@@ -41,6 +41,7 @@ typedef struct S390CcwMachineClass {bool cpu_model_allowed;bool css_migration_enabled;bool gs_allowed; +bool pci_compat; } S390CcwMachineClass;/* runtime-instrumentation allowed by the machine */
Signature · 1 line
2.13.5
Re: [Qemu-devel] [PATCH RFC 1/1] s390x/ccw: create s390 phb for compat reasons as well
Quoted 59 lines · 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 some old machines.Create the s390 phb if we need it for compat reasons, even if we don't provide the zpci facility.
Fixes d32bd032d8 ("s390x/ccw: create s390 phb conditionally")Signed-off-by Cornelia Huck <[email protected]>--- hw/s390x/s390-virtio-ccw.c | 6 +++++- include/hw/s390x/s390-virtio-ccw.h | 1 + 2 files changed, 6 insertions(+), 1 deletion(-)hw/s390x/s390-virtio-ccw.c +5
-1@@ -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;@@ -288,7 +290,7 @@ static void ccw_init(MachineState *machine) machine->initrd_filename,"s390-ccw.img" , "s390-netboot.img", true); -if (s390_has_feat (S390_FEAT_ZPCI )) { +if (s390_has_feat (S390_FEAT_ZPCI ) ||get_machine_class ()->pci_compat) {DeviceState *dev =qdev_create (NULL ,TYPE_S390_PCI_HOST_BRIDGE ); object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,@@ -429,6 +431,7 @@ static void ccw_machine_class_init(ObjectClass *oc, void *data) s390mc->cpu_model_allowed = true; s390mc->css_migration_enabled = true; s390mc->gs_allowed = true; + s390mc->pci_compat = false; mc->init = ccw_init; mc->reset = s390_machine_reset; mc->hot_add_cpu = s390_hot_add_cpu;@@ -784,6 +787,7 @@ static void ccw_machine_2_7_class_options(MachineClass *mc)S390CcwMachineClass *s390mc =S390_MACHINE_CLASS (mc); s390mc->cpu_model_allowed = false; + s390mc->pci_compat = pci_available;ccw_machine_2_8_class_options (mc);SET_MACHINE_COMPAT (mc,CCW_COMPAT_2_7 ); }include/hw/s390x/s390-virtio-ccw.h +0
-0
include/hw/s390x/s390-virtio-ccw.h typedef struct S390CcwMachineClass { · line 47
Quoted 7 lines
include/hw/s390x/s390-virtio-ccw.h +1
-0@@ -41,6 +41,7 @@ typedef struct S390CcwMachineClass {bool cpu_model_allowed;bool css_migration_enabled;bool gs_allowed; +bool pci_compat; } S390CcwMachineClass;/* runtime-instrumentation allowed by the machine */
As an alternative, simply
target/s390x/cpu_models.c +2 -0
@@ -195,6 +195,8 @@ bool s390_has_feat(S390Feat feat) return true; } } +if (feat ==S390_FEAT_ZPCI ) +return true;#endif return 0 ; }
(this is also the way we handle other features without cpu model support -especially if cpu model support is disabled for older machines)
Signature · 3 lines
Thanks, David
Re: [Qemu-devel] [PATCH RFC 1/1] s390x/ccw: create s390 phb for compat reasons as well
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 199
Quoted 12 lines · On Fri, 15 Sep 2017 13:57:40 +0200 David Hildenbrand <[email protected]> wrote:
As an alternative, simply
target/s390x/cpu_models.c +2
-0@@ -195,6 +195,8 @@ bool s390_has_feat(S390Feat feat)return true; } } +if (feat ==S390_FEAT_ZPCI ) +return true;
Move that out of the CONFIG_KVM #ifdef?
(Also, we still have the issue with pci support :/ - depend on pci_available?)
Quoted 7 lines
target/s390x/cpu_models.c +0
-0#endif return 0 ; }(this is also the way we handle other features without cpu model support -especially if cpu model support is disabled for older machines)
Now that you mention cpu models (which are disabled on old machines), the failure starts to look plausible :)
Re: [Qemu-devel] [PATCH RFC 1/1] s390x/ccw: create s390 phb for compat reasons as well
Quoted 5 lines · Cornelia Huck
Quoted 2 lines · On Fri, 15 Sep 2017 13:57:40 +0200 David Hildenbrand <[email protected]> wrote:
As an alternative, simply
Quoted 13 lines
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 199
Quoted 10 lines
target/s390x/cpu_models.c +2
-0@@ -195,6 +195,8 @@ bool s390_has_feat(S390Feat feat)return true; } } +if (feat ==S390_FEAT_ZPCI ) +return true;Move that out of the
CONFIG_KVM#ifdef?
Should not make a difference I think, cpu models are not shielded off for TCG I think (maybe we should fix that, but doesn't look that easy - they have no host model which would allow cpu->model = NULL).
Quoted 2 lines
(Also, we still have the issue with pci support :/ - depend on
pci_available?)
hm ....
Quoted 12 lines
Quoted 8 lines
target/s390x/cpu_models.c +0
-0#endif return 0 ; }(this is also the way we handle other features without cpu model support -especially if cpu model support is disabled for older machines)
Now that you mention cpu models (which are disabled on old machines), the failure starts to look plausible :)
Signature · 3 lines
Thanks, David
Re: [Qemu-devel] [PATCH RFC 1/1] s390x/ccw: create s390 phb for compat reasons as well
Quoted 20 lines · On Fri, 15 Sep 2017 15:09:02 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 5 lines · Cornelia Huck
Quoted 2 lines · On Fri, 15 Sep 2017 13:57:40 +0200 David Hildenbrand <[email protected]> wrote:
As an alternative, simply
Quoted 13 lines
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 199
Quoted 10 lines
target/s390x/cpu_models.c +2
-0@@ -195,6 +195,8 @@ bool s390_has_feat(S390Feat feat)return true; } } +if (feat ==S390_FEAT_ZPCI ) +return true;Move that out of the
CONFIG_KVM#ifdef?
Quoted 3 lines
Should not make a difference I think, cpu models are not shielded off for TCG I think (maybe we should fix that, but doesn't look that easy - they have no host model which would allow
cpu->model= NULL).
It just looks odd to have it inside the define...
Quoted 5 lines
Quoted 3 lines
(Also, we still have the issue with pci support :/ - depend on
pci_available?)hm ....
Should we maybe disallow compat machines if CONFIG_PCI is off, and think about something for the 2.11 machine? Otherwise, I cannot see that end well :/
But I'll just draw up a v2 for now.
Re: [Qemu-devel] [PATCH RFC 1/1] s390x/ccw: create s390 phb for compat reasons as well
Quoted 23 lines · Cornelia Huck
Quoted 20 lines · On Fri, 15 Sep 2017 15:09:02 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 18 lines · Cornelia Huck
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 199
Quoted 12 lines · On Fri, 15 Sep 2017 13:57:40 +0200 David Hildenbrand <[email protected]> wrote:
As an alternative, simply
target/s390x/cpu_models.c +2
-0@@ -195,6 +195,8 @@ bool s390_has_feat(S390Feat feat)return true; } } +if (feat ==S390_FEAT_ZPCI ) +return true;Move that out of the
CONFIG_KVM#ifdef?
Quoted 5 lines
Quoted 3 lines
Should not make a difference I think, cpu models are not shielded off for TCG I think (maybe we should fix that, but doesn't look that easy - they have no host model which would allow
cpu->model= NULL).It just looks odd to have it inside the define...
sure, you can then also just move it into kvm_enable() next to the other ones.
Quoted 10 lines
Quoted 6 lines
Quoted 4 lines
(Also, we still have the issue with pci support :/ - depend on
pci_available?)hm ....
Should we maybe disallow compat machines if
CONFIG_PCIis off, and think about something for the 2.11 machine? Otherwise, I cannot see that end well :/
If they required PCI, than that is the only way to do it.
Quoted 3 lines
But I'll just draw up a v2 for now.
Signature · 3 lines
Thanks, David
Re: [Qemu-devel] [PATCH RFC 1/1] s390x/ccw: create s390 phb for compat reasons as well
Quoted 38 lines · David Hildenbrand
Quoted 23 lines · Cornelia Huck
Quoted 20 lines · On Fri, 15 Sep 2017 15:09:02 +0200 David Hildenbrand <[email protected]> wrote:
Quoted 18 lines · Cornelia Huck
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 199
Quoted 12 lines · On Fri, 15 Sep 2017 13:57:40 +0200 David Hildenbrand <[email protected]> wrote:
As an alternative, simply
target/s390x/cpu_models.c +2
-0@@ -195,6 +195,8 @@ bool s390_has_feat(S390Feat feat)return true; } } +if (feat ==S390_FEAT_ZPCI ) +return true;Move that out of the
CONFIG_KVM#ifdef?Quoted 5 lines
Quoted 3 lines
Should not make a difference I think, cpu models are not shielded off for TCG I think (maybe we should fix that, but doesn't look that easy - they have no host model which would allow
cpu->model= NULL).It just looks odd to have it inside the define...
sure, you can then also just move it into
kvm_enable()next to the other ones.Quoted 5 lines
Quoted 5 lines
Quoted 4 lines
(Also, we still have the issue with pci support :/ - depend on
pci_available?)
Quoted 11 lines
Quoted 5 lines
Quoted 1 line
hm ....
Should we maybe disallow compat machines if
CONFIG_PCIis off, and think about something for the 2.11 machine? Otherwise, I cannot see that end well :/If they required PCI, than that is the only way to do it.
Quoted 3 lines
But I'll just draw up a v2 for now.
Sorry for not contributing anything here. I don't have the whole thing in my head, so I can't quite follow without digging into this.
I don't think I will be able to join today -- maybe I could have a look next week .
Halil