[1/1] s390x: create a compat s390 phb for <=2.10

Commit Message

Cornelia Huck Sept. 26, 2017, 4:20 p.m. UTC

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 <[e-mail]>
Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally")
Signed-off-by: Cornelia Huck <[e-mail]>
---
 hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
 include/hw/s390x/s390-virtio-ccw.h | 1 +
 2 files changed, 8 insertions(+), 1 deletion(-)

Comments

Christian Borntraeger Sept. 26, 2017, 5:07 p.m. UTC | #1

On 09/26/2017 06:20 PM, Cornelia Huck wrote:
> 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 <[e-mail]>
> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally")
> Signed-off-by: Cornelia Huck <[e-mail]>

Tested-by: Christian Borntraeger <[e-mail]>

seems to work fine. 
> ---
>  hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
>  include/hw/s390x/s390-virtio-ccw.h | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 1bcb7000ab..981f1c4336 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -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);
>  }
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index a9a90c2022..fb717afe92 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -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 */
>

David Hildenbrand Sept. 26, 2017, 6:40 p.m. UTC | #2

On 26.09.2017 18:20, Cornelia Huck wrote:
> 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 <[e-mail]>
> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally")
> Signed-off-by: Cornelia Huck <[e-mail]>
> ---
>  hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
>  include/hw/s390x/s390-virtio-ccw.h | 1 +
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> index 1bcb7000ab..981f1c4336 100644
> --- a/hw/s390x/s390-virtio-ccw.c
> +++ b/hw/s390x/s390-virtio-ccw.c
> @@ -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);
>  }
> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> index a9a90c2022..fb717afe92 100644
> --- a/include/hw/s390x/s390-virtio-ccw.h
> +++ b/include/hw/s390x/s390-virtio-ccw.h
> @@ -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.

Cornelia Huck Sept. 27, 2017, 9:47 a.m. UTC | #3

On Tue, 26 Sep 2017 20:40:25 +0200
David Hildenbrand <[e-mail]> wrote:

> On 26.09.2017 18:20, Cornelia Huck wrote:
> > 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 <[e-mail]>
> > Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally")
> > Signed-off-by: Cornelia Huck <[e-mail]>
> > ---
> >  hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
> >  include/hw/s390x/s390-virtio-ccw.h | 1 +
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> > 
> > diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
> > index 1bcb7000ab..981f1c4336 100644
> > --- a/hw/s390x/s390-virtio-ccw.c
> > +++ b/hw/s390x/s390-virtio-ccw.c
> > @@ -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);
> >  }
> > diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
> > index a9a90c2022..fb717afe92 100644
> > --- a/include/hw/s390x/s390-virtio-ccw.h
> > +++ b/include/hw/s390x/s390-virtio-ccw.h
> > @@ -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.

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

Yi Min Zhao Sept. 27, 2017, 10:25 a.m. UTC | #4

在 2017/9/27 下午5:47, Cornelia Huck 写道:
> On Tue, 26 Sep 2017 20:40:25 +0200
> David Hildenbrand <[e-mail]> wrote:
>
>> On 26.09.2017 18:20, Cornelia Huck wrote:
>>> 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 <[e-mail]>
>>> Fixes: d32bd032d8 ("s390x/ccw: create s390 phb conditionally")
>>> Signed-off-by: Cornelia Huck <[e-mail]>
>>> ---
>>>   hw/s390x/s390-virtio-ccw.c         | 8 +++++++-
>>>   include/hw/s390x/s390-virtio-ccw.h | 1 +
>>>   2 files changed, 8 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
>>> index 1bcb7000ab..981f1c4336 100644
>>> --- a/hw/s390x/s390-virtio-ccw.c
>>> +++ b/hw/s390x/s390-virtio-ccw.c
>>> @@ -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);
>>>   }
>>> diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
>>> index a9a90c2022..fb717afe92 100644
>>> --- a/include/hw/s390x/s390-virtio-ccw.h
>>> +++ b/include/hw/s390x/s390-virtio-ccw.h
>>> @@ -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.
> 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)?
>
>
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.

Cornelia Huck Sept. 27, 2017, 10:56 a.m. UTC | #5

On Wed, 27 Sep 2017 18:25:00 +0800
Yi Min Zhao <[e-mail]> wrote:

> 在 2017/9/27 下午5:47, Cornelia Huck 写道:
> > On Tue, 26 Sep 2017 20:40:25 +0200
> > David Hildenbrand <[e-mail]> 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)?
> >
> >  
> 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...

Christian Borntraeger Sept. 27, 2017, 10:59 a.m. UTC | #6

On 09/27/2017 12:56 PM, Cornelia Huck wrote:
> On Wed, 27 Sep 2017 18:25:00 +0800
> Yi Min Zhao <[e-mail]> wrote:
> 
>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:
>>> On Tue, 26 Sep 2017 20:40:25 +0200
>>> David Hildenbrand <[e-mail]> 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)?
>>>
>>>  
>> 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.

David Hildenbrand Sept. 27, 2017, 12:21 p.m. UTC | #7

On 27.09.2017 12:59, Christian Borntraeger wrote:
> 
> 
> On 09/27/2017 12:56 PM, Cornelia Huck wrote:
>> On Wed, 27 Sep 2017 18:25:00 +0800
>> Yi Min Zhao <[e-mail]> wrote:
>>
>>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:
>>>> On Tue, 26 Sep 2017 20:40:25 +0200
>>>> David Hildenbrand <[e-mail]> 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)?
>>>>
>>>>  
>>> 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.
> 

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?

Christian Borntraeger Sept. 27, 2017, 12:26 p.m. UTC | #8

On 09/27/2017 02:21 PM, David Hildenbrand wrote:
> On 27.09.2017 12:59, Christian Borntraeger wrote:
>>
>>
>> On 09/27/2017 12:56 PM, Cornelia Huck wrote:
>>> On Wed, 27 Sep 2017 18:25:00 +0800
>>> Yi Min Zhao <[e-mail]> wrote:
>>>
>>>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:
>>>>> On Tue, 26 Sep 2017 20:40:25 +0200
>>>>> David Hildenbrand <[e-mail]> 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)?
>>>>>
>>>>>  
>>>> 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.
>>
> 
> 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?

If you migrate from a QEMU with CONFIG_PCI to a QEMU without CONFIG_PCI 
I assume it will break. But maybe there is really some clever way to
do the right thing. 

PS: Is really anybody disabling CONFIG_PCI and why?

Dr. David Alan Gilbert Sept. 27, 2017, 2:28 p.m. UTC | #9

* David Hildenbrand ([e-mail]) wrote:
> On 27.09.2017 12:59, Christian Borntraeger wrote:
> > 
> > 
> > On 09/27/2017 12:56 PM, Cornelia Huck wrote:
> >> On Wed, 27 Sep 2017 18:25:00 +0800
> >> Yi Min Zhao <[e-mail]> wrote:
> >>
> >>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:
> >>>> On Tue, 26 Sep 2017 20:40:25 +0200
> >>>> David Hildenbrand <[e-mail]> 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)?
> >>>>
> >>>>  
> >>> 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.
> > 
> 
> 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)

Dave

> -- 
> 
> Thanks,
> 
> David
--
Dr. David Alan Gilbert / [e-mail] / Manchester, UK

Cornelia Huck Sept. 27, 2017, 2:46 p.m. UTC | #10

On Wed, 27 Sep 2017 15:28:38 +0100
"Dr. David Alan Gilbert" <[e-mail]> wrote:

> * David Hildenbrand ([e-mail]) wrote:
> > On 27.09.2017 12:59, Christian Borntraeger wrote:  
> > > 
> > > 
> > > On 09/27/2017 12:56 PM, Cornelia Huck wrote:  
> > >> On Wed, 27 Sep 2017 18:25:00 +0800
> > >> Yi Min Zhao <[e-mail]> wrote:
> > >>  
> > >>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:  
> > >>>> On Tue, 26 Sep 2017 20:40:25 +0200
> > >>>> David Hildenbrand <[e-mail]> 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)?
> > >>>>
> > >>>>    
> > >>> 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.
> > >   
> > 
> > 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)

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 :(

Dr. David Alan Gilbert Sept. 27, 2017, 2:49 p.m. UTC | #11

* Cornelia Huck ([e-mail]) wrote:
> On Wed, 27 Sep 2017 15:28:38 +0100
> "Dr. David Alan Gilbert" <[e-mail]> wrote:
> 
> > * David Hildenbrand ([e-mail]) wrote:
> > > On 27.09.2017 12:59, Christian Borntraeger wrote:  
> > > > 
> > > > 
> > > > On 09/27/2017 12:56 PM, Cornelia Huck wrote:  
> > > >> On Wed, 27 Sep 2017 18:25:00 +0800
> > > >> Yi Min Zhao <[e-mail]> wrote:
> > > >>  
> > > >>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:  
> > > >>>> On Tue, 26 Sep 2017 20:40:25 +0200
> > > >>>> David Hildenbrand <[e-mail]> 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)?
> > > >>>>
> > > >>>>    
> > > >>> 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.
> > > >   
> > > 
> > > 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)
> 
> 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.

Dave
--
Dr. David Alan Gilbert / [e-mail] / Manchester, UK

Cornelia Huck Sept. 27, 2017, 3:03 p.m. UTC | #12

On Wed, 27 Sep 2017 15:49:27 +0100
"Dr. David Alan Gilbert" <[e-mail]> wrote:

> * Cornelia Huck ([e-mail]) wrote:
> > On Wed, 27 Sep 2017 15:28:38 +0100
> > "Dr. David Alan Gilbert" <[e-mail]> wrote:
> >   
> > > * David Hildenbrand ([e-mail]) wrote:  
> > > > On 27.09.2017 12:59, Christian Borntraeger wrote:    
> > > > > 
> > > > > 
> > > > > On 09/27/2017 12:56 PM, Cornelia Huck wrote:    
> > > > >> On Wed, 27 Sep 2017 18:25:00 +0800
> > > > >> Yi Min Zhao <[e-mail]> wrote:
> > > > >>    
> > > > >>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:    
> > > > >>>> On Tue, 26 Sep 2017 20:40:25 +0200
> > > > >>>> David Hildenbrand <[e-mail]> 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)?
> > > > >>>>
> > > > >>>>      
> > > > >>> 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.
> > > > >     
> > > > 
> > > > 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)  
> > 
> > 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.

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...

Christian Borntraeger Sept. 28, 2017, 10:34 a.m. UTC | #13

On 09/27/2017 05:03 PM, Cornelia Huck wrote:
> On Wed, 27 Sep 2017 15:49:27 +0100
> "Dr. David Alan Gilbert" <[e-mail]> wrote:
> 
>> * Cornelia Huck ([e-mail]) wrote:
>>> On Wed, 27 Sep 2017 15:28:38 +0100
>>> "Dr. David Alan Gilbert" <[e-mail]> wrote:
>>>   
>>>> * David Hildenbrand ([e-mail]) wrote:  
>>>>> On 27.09.2017 12:59, Christian Borntraeger wrote:    
>>>>>>
>>>>>>
>>>>>> On 09/27/2017 12:56 PM, Cornelia Huck wrote:    
>>>>>>> On Wed, 27 Sep 2017 18:25:00 +0800
>>>>>>> Yi Min Zhao <[e-mail]> wrote:
>>>>>>>    
>>>>>>>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:    
>>>>>>>>> On Tue, 26 Sep 2017 20:40:25 +0200
>>>>>>>>> David Hildenbrand <[e-mail]> 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)?
>>>>>>>>>
>>>>>>>>>      
>>>>>>>> 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.
>>>>>>     
>>>>>
>>>>> 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)  
>>>
>>> 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.
> 
> 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 <[e-mail]>
AuthorDate: Thu Jul 6 17:21:52 2017 +0200
Commit:     Cornelia Huck <[e-mail]>
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.

Christian Borntraeger Sept. 28, 2017, 10:41 a.m. UTC | #14

On 09/28/2017 12:34 PM, Christian Borntraeger wrote:
> 
> 
> On 09/27/2017 05:03 PM, Cornelia Huck wrote:
>> On Wed, 27 Sep 2017 15:49:27 +0100
>> "Dr. David Alan Gilbert" <[e-mail]> wrote:
>>
>>> * Cornelia Huck ([e-mail]) wrote:
>>>> On Wed, 27 Sep 2017 15:28:38 +0100
>>>> "Dr. David Alan Gilbert" <[e-mail]> wrote:
>>>>   
>>>>> * David Hildenbrand ([e-mail]) wrote:  
>>>>>> On 27.09.2017 12:59, Christian Borntraeger wrote:    
>>>>>>>
>>>>>>>
>>>>>>> On 09/27/2017 12:56 PM, Cornelia Huck wrote:    
>>>>>>>> On Wed, 27 Sep 2017 18:25:00 +0800
>>>>>>>> Yi Min Zhao <[e-mail]> wrote:
>>>>>>>>    
>>>>>>>>> 在 2017/9/27 下午5:47, Cornelia Huck 写道:    
>>>>>>>>>> On Tue, 26 Sep 2017 20:40:25 +0200
>>>>>>>>>> David Hildenbrand <[e-mail]> 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)?
>>>>>>>>>>
>>>>>>>>>>      
>>>>>>>>> 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.
>>>>>>>     
>>>>>>
>>>>>> 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)  
>>>>
>>>> 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.
>>
>> 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 <[e-mail]>
> AuthorDate: Thu Jul 6 17:21:52 2017 +0200
> Commit:     Cornelia Huck <[e-mail]>
> 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 
too quick                                     ^that^					 
> s390-rhelx.y.z or whatever. 

I think we really must revert this commit.

David Hildenbrand Sept. 28, 2017, 12:33 p.m. UTC | #15

>> In the end that means that you should revert
>>
>> commit d32bd032d8fde41281aae34c16a4aa97e9acfeac
>> Author:     Cornelia Huck <[e-mail]>
>> AuthorDate: Thu Jul 6 17:21:52 2017 +0200
>> Commit:     Cornelia Huck <[e-mail]>
>> 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 
> too quick                                     ^that^					 
>> s390-rhelx.y.z or whatever. 
> 
> I think we really must revert this commit. 
> 

I tend to agree.

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1bcb7000ab..981f1c4336 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -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);
 }
diff --git a/include/hw/s390x/s390-virtio-ccw.h b/include/hw/s390x/s390-virtio-ccw.h
index a9a90c2022..fb717afe92 100644
--- a/include/hw/s390x/s390-virtio-ccw.h
+++ b/include/hw/s390x/s390-virtio-ccw.h
@@ -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 */