s390x: more zpci compat fun

d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made creating the s390 phb dependant on the zpci facility. This broke migration from pre-cpu model machines which was fixed with 8ad9087c4a ("s390x/ccw: create s390 phb for compat reasons as well"). However, that is not enough: Migration from 2.10 with -cpu z13 breaks as well.

Let's create a phb for all pre-2.11 compat machines to fix this. We leave the zpci facility off to avoid a guest-visible change with cpu models on.

Reported-by Christian Borntraeger <[email protected]>

Fixes d32bd032d8 ("s390x/ccw: create s390 phb conditionally")

Signed-off-by Cornelia Huck <[email protected]>

---
 hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
 include/hw/s390x/s390-virtio-ccw.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

hw/s390x/s390-virtio-ccw.c +7 -1

index 1bcb7000ab..981f1c4336 100644
@@ -247,6 +247,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;

@@ -266,7 +268,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()->phb_compat) {
         DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
         object_property_add_child(qdev_get_machine(),
                                   TYPE_S390_PCI_HOST_BRIDGE,

@@ -407,6 +409,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->phb_compat = false;
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
     mc->hot_add_cpu = s390_hot_add_cpu;

@@ -716,6 +719,9 @@ static void ccw_machine_2_10_instance_options(MachineState *machine)
 
 static void ccw_machine_2_10_class_options(MachineClass *mc)
 {
+    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
+    s390mc->phb_compat = pci_available;
     ccw_machine_2_11_class_options(mc);
     SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_10);
 }

include/hw/s390x/s390-virtio-ccw.h +1 -0

index a9a90c2022..fb717afe92 100644
@@ -41,6 +41,7 @@ typedef struct S390CcwMachineClass {
     bool cpu_model_allowed;
     bool css_migration_enabled;
     bool gs_allowed;
+    bool phb_compat;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */

d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made creating the s390 phb dependant on the zpci facility. This broke migration from pre-cpu model machines which was fixed with 8ad9087c4a ("s390x/ccw: create s390 phb for compat reasons as well"). However, that is not enough: Migration from 2.10 with -cpu z13 breaks as well.

Let's create a phb for all pre-2.11 compat machines to fix this. We leave the zpci facility off to avoid a guest-visible change with cpu models on.

Reported-by Christian Borntraeger <[email protected]>

Fixes d32bd032d8 ("s390x/ccw: create s390 phb conditionally")

Signed-off-by Cornelia Huck <[email protected]>

---
 hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
 include/hw/s390x/s390-virtio-ccw.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

hw/s390x/s390-virtio-ccw.c +7 -1

index 1bcb7000ab..981f1c4336 100644
@@ -247,6 +247,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;

@@ -266,7 +268,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()->phb_compat) {
         DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
         object_property_add_child(qdev_get_machine(),
                                   TYPE_S390_PCI_HOST_BRIDGE,

@@ -407,6 +409,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->phb_compat = false;
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
     mc->hot_add_cpu = s390_hot_add_cpu;

@@ -716,6 +719,9 @@ static void ccw_machine_2_10_instance_options(MachineState *machine)
 
 static void ccw_machine_2_10_class_options(MachineClass *mc)
 {
+    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
+    s390mc->phb_compat = pci_available;
     ccw_machine_2_11_class_options(mc);
     SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_10);
 }

include/hw/s390x/s390-virtio-ccw.h +0 -0

index a9a90c2022..fb717afe92 100644

Quoted 65 lines · Cornelia Huck

d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made creating the s390 phb dependant on the zpci facility. This broke migration from pre-cpu model machines which was fixed with 8ad9087c4a ("s390x/ccw: create s390 phb for compat reasons as well"). However, that is not enough: Migration from 2.10 with -cpu z13 breaks as well.

Let's create a phb for all pre-2.11 compat machines to fix this. We leave the zpci facility off to avoid a guest-visible change with cpu models on.

Reported-by Christian Borntraeger <[email protected]>

Fixes d32bd032d8 ("s390x/ccw: create s390 phb conditionally")

Signed-off-by Cornelia Huck <[email protected]>

---
 hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
 include/hw/s390x/s390-virtio-ccw.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

hw/s390x/s390-virtio-ccw.c +7 -1

index 1bcb7000ab..981f1c4336 100644
@@ -247,6 +247,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;

@@ -266,7 +268,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()->phb_compat) {
         DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
         object_property_add_child(qdev_get_machine(),
                                   TYPE_S390_PCI_HOST_BRIDGE,

@@ -407,6 +409,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->phb_compat = false;
     mc->init = ccw_init;
     mc->reset = s390_machine_reset;
     mc->hot_add_cpu = s390_hot_add_cpu;

@@ -716,6 +719,9 @@ static void ccw_machine_2_10_instance_options(MachineState *machine)
 
 static void ccw_machine_2_10_class_options(MachineClass *mc)
 {
+    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
+    s390mc->phb_compat = pci_available;
     ccw_machine_2_11_class_options(mc);
     SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_10);
 }

include/hw/s390x/s390-virtio-ccw.h +0 -0

index a9a90c2022..fb717afe92 100644

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 phb_compat;
 } S390CcwMachineClass;
 
 /* runtime-instrumentation allowed by the machine */

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Quoted 79 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

Quoted 65 lines · Cornelia Huck

d32bd032d8 ("s390x/ccw: create s390 phb conditionally") made creating the s390 phb dependant on the zpci facility. This broke migration from pre-cpu model machines which was fixed with 8ad9087c4a ("s390x/ccw: create s390 phb for compat reasons as well"). However, that is not enough: Migration from 2.10 with -cpu z13 breaks as well.

Let's create a phb for all pre-2.11 compat machines to fix this. We leave the zpci facility off to avoid a guest-visible change with cpu models on.

Reported-by Christian Borntraeger <[email protected]>

Fixes d32bd032d8 ("s390x/ccw: create s390 phb conditionally")

Signed-off-by Cornelia Huck <[email protected]>

---
  hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
  include/hw/s390x/s390-virtio-ccw.h | 1 +
  2 files changed, 8 insertions(+), 1 deletion(-)

hw/s390x/s390-virtio-ccw.c +7 -1

index 1bcb7000ab..981f1c4336 100644
@@ -247,6 +247,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;

@@ -266,7 +268,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()->phb_compat) {
          DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
          object_property_add_child(qdev_get_machine(),
                                    TYPE_S390_PCI_HOST_BRIDGE,

@@ -407,6 +409,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->phb_compat = false;
      mc->init = ccw_init;
      mc->reset = s390_machine_reset;
      mc->hot_add_cpu = s390_hot_add_cpu;

@@ -716,6 +719,9 @@ static void ccw_machine_2_10_instance_options(MachineState *machine)
 
  static void ccw_machine_2_10_class_options(MachineClass *mc)
  {
+    S390CcwMachineClass *s390mc = S390_MACHINE_CLASS(mc);
+
+    s390mc->phb_compat = pci_available;
      ccw_machine_2_11_class_options(mc);
      SET_MACHINE_COMPAT(mc, CCW_COMPAT_2_10);
  }

include/hw/s390x/s390-virtio-ccw.h +0 -0

index a9a90c2022..fb717afe92 100644

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 phb_compat;
  } S390CcwMachineClass;
 
  /* runtime-instrumentation allowed by the machine */

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

Quoted 1 line

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 13 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Quoted 4 lines

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Signature · 3 lines

Thanks,

David

Quoted 41 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 39 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Quoted 3 lines

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Quoted 46 lines · On Wed, 27 Sep 2017 15:28:38 +0100 "Dr. David Alan Gilbert"…

Quoted 44 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 3 lines

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Ugh, that sounds like we need two machine types on s390x as well (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built conditionally. That whole zpci detanglement is looking worse and worse :(

Quoted 49 lines · * Cornelia Huck ([email protected]) wrote:

Quoted 46 lines · On Wed, 27 Sep 2017 15:28:38 +0100 "Dr. David Alan Gilbert"…

Quoted 44 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 8 lines

Quoted 3 lines

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Ugh, that sounds like we need two machine types on s390x as well (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built conditionally. That whole zpci detanglement is looking worse and worse :(

Well fundamentally the migration expects to migrate something into the same shaped hole on the destination; if you've got a lump of PCI config on the source there's got to be somewhere for it to fit on the destination.
Now, if PCI is actually pretty rare; then you might be able to make the host-pci bridge a normal device and not include it in any machine type; that way those who want PCI can just instantiate the host-pci bridge, and those who don't want it just stick with the base machine type.

Quoted 59 lines · On Wed, 27 Sep 2017 15:49:27 +0100 "Dr. David Alan Gilbert"…

Quoted 49 lines · * Cornelia Huck ([email protected]) wrote:

Quoted 46 lines · On Wed, 27 Sep 2017 15:28:38 +0100 "Dr. David Alan Gilbert"…

Quoted 44 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 8 lines

Quoted 3 lines

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Ugh, that sounds like we need two machine types on s390x as well (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built conditionally. That whole zpci detanglement is looking worse and worse :(

Quoted 9 lines

Well fundamentally the migration expects to migrate something into the same shaped hole on the destination; if you've got a lump of PCI config on the source there's got to be somewhere for it to fit on the destination.
Now, if PCI is actually pretty rare; then you might be able to make the host-pci bridge a normal device and not include it in any machine type; that way those who want PCI can just instantiate the host-pci bridge, and those who don't want it just stick with the base machine type.

I fear that ship has already sailed; the s390-ccw-virtio machine type has been instantiating a phb for quite some time, which means we have to drag this on in the compat machines...

Quoted 72 lines · Cornelia Huck

Quoted 59 lines · On Wed, 27 Sep 2017 15:49:27 +0100 "Dr. David Alan Gilbert"…

Quoted 49 lines · * Cornelia Huck ([email protected]) wrote:

Quoted 46 lines · On Wed, 27 Sep 2017 15:28:38 +0100 "Dr. David Alan Gilbert"…

Quoted 44 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 8 lines

Quoted 3 lines

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Ugh, that sounds like we need two machine types on s390x as well (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built conditionally. That whole zpci detanglement is looking worse and worse :(

Quoted 9 lines

Well fundamentally the migration expects to migrate something into the same shaped hole on the destination; if you've got a lump of PCI config on the source there's got to be somewhere for it to fit on the destination.
Now, if PCI is actually pretty rare; then you might be able to make the host-pci bridge a normal device and not include it in any machine type; that way those who want PCI can just instantiate the host-pci bridge, and those who don't want it just stick with the base machine type.

Quoted 3 lines

I fear that ship has already sailed; the s390-ccw-virtio machine type has been instantiating a phb for quite some time, which means we have to drag this on in the compat machines...

In the end that means that you should revert

commit d32bd032d8fde41281aae34c16a4aa97e9acfeac

Author:     Cornelia Huck <[email protected]>
AuthorDate: Thu Jul 6 17:21:52 2017 +0200
Commit:     Cornelia Huck <[email protected]>
CommitDate: Wed Aug 30 18:23:25 2017 +0200

    s390x/ccw: create s390 phb conditionally

to keep s390-ccw-virtio clean proper.

If you want to have PCI disabled, you can do you in a machine like

s390-rhelx.y.z or whatever.

Quoted 89 lines · Christian Borntraeger

Quoted 72 lines · Cornelia Huck

Quoted 59 lines · On Wed, 27 Sep 2017 15:49:27 +0100 "Dr. David Alan Gilbert"…

Quoted 49 lines · * Cornelia Huck ([email protected]) wrote:

Quoted 46 lines · On Wed, 27 Sep 2017 15:28:38 +0100 "Dr. David Alan Gilbert"…

Quoted 44 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 8 lines

Quoted 3 lines

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Ugh, that sounds like we need two machine types on s390x as well (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built conditionally. That whole zpci detanglement is looking worse and worse :(

Quoted 9 lines

Well fundamentally the migration expects to migrate something into the same shaped hole on the destination; if you've got a lump of PCI config on the source there's got to be somewhere for it to fit on the destination.
Now, if PCI is actually pretty rare; then you might be able to make the host-pci bridge a normal device and not include it in any machine type; that way those who want PCI can just instantiate the host-pci bridge, and those who don't want it just stick with the base machine type.

Quoted 3 lines

I fear that ship has already sailed; the s390-ccw-virtio machine type has been instantiating a phb for quite some time, which means we have to drag this on in the compat machines...

In the end that means that you should revert

commit d32bd032d8fde41281aae34c16a4aa97e9acfeac

Author:     Cornelia Huck <[email protected]>
AuthorDate: Thu Jul 6 17:21:52 2017 +0200
Commit:     Cornelia Huck <[email protected]>
CommitDate: Wed Aug 30 18:23:25 2017 +0200

    s390x/ccw: create s390 phb conditionally

Quoted 3 lines

to keep s390-ccw-virtio clean proper.

If you want to have PCI disabled, you can do you in a machine like

too quick ^that^

Quoted 1 line

s390-rhelx.y.z or whatever.

I think we really must revert this commit.

Quoted 93 lines · On Thu, 28 Sep 2017 12:41:54 +0200 Christian Borntraeger…

Quoted 92 lines · Christian Borntraeger

Quoted 72 lines · Cornelia Huck

Quoted 59 lines · On Wed, 27 Sep 2017 15:49:27 +0100 "Dr. David Alan Gilbert"…

Quoted 49 lines · * Cornelia Huck ([email protected]) wrote:

Quoted 46 lines · On Wed, 27 Sep 2017 15:28:38 +0100 "Dr. David Alan Gilbert"…

Quoted 44 lines · * David Hildenbrand ([email protected]) wrote:

Quoted 33 lines · Christian Borntraeger

Quoted 21 lines · Cornelia Huck

Quoted 18 lines · On Wed, 27 Sep 2017 18:25:00 +0800 Yi Min Zhao <[email protected]>…

Quoted 17 lines · Cornelia Huck

Quoted 7 lines · On Tue, 26 Sep 2017 20:40:25 +0200 David Hildenbrand <[email protected]> wrote:

I'd really really really (did I mention really?) favor something like a
dummy device, because we could easily handle the !CONFIG_PCI case then.

All these compat options and conditions will kill us someday... we're
already patching around that whole stuff way too much.

If we ever unconditionally created a device, we should keep doing so.

Yes, that whole thing is horrible, especially interaction with compat machines.

Do you have an idea on how to create such a dummy device (without having to effectively copy a lot of configured-out code)?

Quoted 8 lines

Quoted 4 lines

How about in s390_pcihost_hot_plug() we check s390_has_feat(zpci)? If no zpci feature, we avoid plugging any pci device. Then we could always create phb.
I think pcibus's vmstate is only data to migrate.

That's still problematic if CONFIG_PCI is off. I currently don't have a better idea than either disallowing compat machines on builds without pci, or using a dummy device...

Quoted 6 lines

For this particular case your initial patch might be less problematic than a dummy device, because the code that does the migration is NOT contained in s390 specific code but in common PCI code instead. We would need to keep the dummy device always in a way that it will work with the common PCI code.

Interesting, so how is migration then handled for e.g. x86 or other architectures that can work without CONFIG_PCI? I assume their migration should also break?

Quoted 8 lines

Quoted 3 lines

It's tied to machine-type; the x86 i440fx and q35 machine types have PCI; you can't disable PCI while still having those machine types. (I don't know if you can disable PCI at all on x86)

Ugh, that sounds like we need two machine types on s390x as well (s390x-ccw-virtio and s390x-ccw-virtio-nopci or so), built conditionally. That whole zpci detanglement is looking worse and worse :(

Quoted 9 lines

Well fundamentally the migration expects to migrate something into the same shaped hole on the destination; if you've got a lump of PCI config on the source there's got to be somewhere for it to fit on the destination.
Now, if PCI is actually pretty rare; then you might be able to make the host-pci bridge a normal device and not include it in any machine type; that way those who want PCI can just instantiate the host-pci bridge, and those who don't want it just stick with the base machine type.

Quoted 3 lines

I fear that ship has already sailed; the s390-ccw-virtio machine type has been instantiating a phb for quite some time, which means we have to drag this on in the compat machines...

In the end that means that you should revert

commit d32bd032d8fde41281aae34c16a4aa97e9acfeac

Author:     Cornelia Huck <[email protected]>
AuthorDate: Thu Jul 6 17:21:52 2017 +0200
Commit:     Cornelia Huck <[email protected]>
CommitDate: Wed Aug 30 18:23:25 2017 +0200

    s390x/ccw: create s390 phb conditionally

to keep s390-ccw-virtio clean proper.

If you want to have PCI disabled, you can do you in a machine like

Quoted 5 lines

too quick ^that^

Quoted 1 line

s390-rhelx.y.z or whatever.

I think we really must revert this commit.

A set of non-pci machines looks more attractive to me. I currently have the following (not yet tested):

I see no point if PCI disable machines. There is no non-PCI x86 machine besides the isapc. But this has no version whatsoever so it certainly is not made for being migrated.
As far as I can see and we would make things much more complex.

non-PCI will trigger other issues. e.g. zpci and aen is part of the z14 cpu model since 2.10. Really this is not helping, its making things worse.

If you really want to avoid PCI for whatever redhat is doing, can't you just hide it in a rhel specific machine (which you seem to have anyway for x86 and power)?

Quoted 11 lines

In the end that means that you should revert

commit d32bd032d8fde41281aae34c16a4aa97e9acfeac

Author:     Cornelia Huck <[email protected]>
AuthorDate: Thu Jul 6 17:21:52 2017 +0200
Commit:     Cornelia Huck <[email protected]>
CommitDate: Wed Aug 30 18:23:25 2017 +0200

    s390x/ccw: create s390 phb conditionally

Quoted 3 lines

to keep s390-ccw-virtio clean proper.

If you want to have PCI disabled, you can do you in a machine like

too quick ^that^

Quoted 1 line

s390-rhelx.y.z or whatever.

I think we really must revert this commit.