s390x: pci compat handling

RFC PATCH Superseded Cornelia Huck 2 participants 7 messages Sep 15, 2017 – Sep 18, 2017

Share & tools

Cornelia Huck

author

2017-09-15 16:40 UTC #0 lore raw

v2 cover s390x: pci compat handling

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(+)

Signature · 1 line

2.13.5

Cornelia Huck

author

2017-09-15 16:40 UTC #1 lore raw

RFC PATCH v2 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 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 <[email protected]>

---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu_models.c  | 3 +++
 2 files changed, 5 insertions(+)

hw/s390x/s390-virtio-ccw.c +2 -0

index 0471407187..907abc7a32 100644
@@ -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;

target/s390x/cpu_models.c +3 -0

index c295e641e6..5169379db5 100644
@@ -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);

Signature · 1 line

2.13.5

Christian Borntraeger

2017-09-18 07:33 UTC #2 lore raw

Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well

Quoted 32 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 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 <[email protected]>
---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu_models.c  | 3 +++
 2 files changed, 5 insertions(+)
hw/s390x/s390-virtio-ccw.c +2 -0
index 0471407187..907abc7a32 100644
@@ -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;
target/s390x/cpu_models.c +0 -0
index c295e641e6..5169379db5 100644

target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 201

Quoted 6 lines

target/s390x/cpu_models.c +3 -0

@@ -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?

Quoted 4 lines

target/s390x/cpu_models.c +0 -0

         return 0;
     }
     return test_bit(feat, cpu->model->features);

Christian Borntraeger

2017-09-18 07:47 UTC #3 lore raw

Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well

Quoted 37 lines · Christian Borntraeger

Quoted 34 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 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 <[email protected]>
---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu_models.c  | 3 +++
 2 files changed, 5 insertions(+)
hw/s390x/s390-virtio-ccw.c +2 -0
index 0471407187..907abc7a32 100644
@@ -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;
target/s390x/cpu_models.c +0 -0
index c295e641e6..5169379db5 100644
@@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat)
             }
         }

Quoted 9 lines

target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 201
Quoted 4 lines
target/s390x/cpu_models.c +3 -0
 #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.

Cornelia Huck

author

2017-09-18 08:08 UTC #4 lore raw

Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well

Quoted 26 lines · On Mon, 18 Sep 2017 09:47:00 +0200 Christian Borntraeger…

Quoted 25 lines · Christian Borntraeger
Quoted 22 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 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 <[email protected]>
---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu_models.c  | 3 +++
 2 files changed, 5 insertions(+)
hw/s390x/s390-virtio-ccw.c +0 -0
index 0471407187..907abc7a32 100644
@@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name)
     }
 }
 

Quoted 5 lines

Quoted 5 lines
hw/s390x/s390-virtio-ccw.c static void s390_create_virtio_net(BusState *bus, const char *name) · line 276
Quoted 5 lines
hw/s390x/s390-virtio-ccw.c +2 -0
+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...

Quoted 17 lines

Quoted 7 lines
Quoted 7 lines
target/s390x/cpu_models.c +0 -0
index c295e641e6..5169379db5 100644
@@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat)
             }
         }
Quoted 9 lines
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 201
Quoted 4 lines
target/s390x/cpu_models.c +3 -0
 #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?

Quoted 2 lines

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.

Christian Borntraeger

Acked-by

2017-09-18 08:36 UTC #5 lore raw

Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well

Quoted 54 lines · Cornelia Huck

Quoted 26 lines · On Mon, 18 Sep 2017 09:47:00 +0200 Christian Borntraeger…
Quoted 25 lines · Christian Borntraeger
Quoted 22 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 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 <[email protected]>
---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu_models.c  | 3 +++
 2 files changed, 5 insertions(+)
hw/s390x/s390-virtio-ccw.c +0 -0
index 0471407187..907abc7a32 100644
@@ -269,6 +269,8 @@ static void s390_create_virtio_net(BusState *bus, const char *name)
     }
 }
 
Quoted 5 lines
Quoted 5 lines
hw/s390x/s390-virtio-ccw.c static void s390_create_virtio_net(BusState *bus, const char *name) · line 276
Quoted 5 lines
hw/s390x/s390-virtio-ccw.c +2 -0
+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...
Quoted 17 lines
Quoted 16 lines
target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 201
Quoted 11 lines
target/s390x/cpu_models.c +3 -0
index c295e641e6..5169379db5 100644
@@ -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?

Quoted 4 lines

Quoted 2 lines
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 <[email protected]>

Cornelia Huck

author

2017-09-18 07:49 UTC #6 lore raw

Re: [Qemu-devel] [PATCH RFC v2 1/1] s390x/ccw: create s390 phb for compat reasons as well

In reply to Christian Borntraeger

Quoted 35 lines · On Mon, 18 Sep 2017 09:33:23 +0200 Christian Borntraeger…

Quoted 34 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 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 <[email protected]>
---
 hw/s390x/s390-virtio-ccw.c | 2 ++
 target/s390x/cpu_models.c  | 3 +++
 2 files changed, 5 insertions(+)
hw/s390x/s390-virtio-ccw.c +2 -0
index 0471407187..907abc7a32 100644
@@ -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;
target/s390x/cpu_models.c +0 -0
index c295e641e6..5169379db5 100644
@@ -196,6 +196,9 @@ bool s390_has_feat(S390Feat feat)
             }
         }

Quoted 8 lines

target/s390x/cpu_models.c bool s390_has_feat(S390Feat feat) · line 201
Quoted 4 lines
target/s390x/cpu_models.c +3 -0
 #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?

Quoted 7 lines

Quoted 6 lines

target/s390x/cpu_models.c +0 -0

         return 0;
     }
     return test_bit(feat, cpu->model->features);