[v4,5/9] s390x/ccw: create s390 phb conditionally

Commit Message

Cornelia Huck Aug. 4, 2017, 11:29 a.m. UTC

Don't create the s390 pci host bridge if we do not provide the zpci
facility.

Reviewed-by: Thomas Huth <[e-mail]>
Acked-by: Christian Borntraeger <[e-mail]>
Signed-off-by: Cornelia Huck <[e-mail]>
---
 hw/s390x/s390-virtio-ccw.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

Comments

Cornelia Huck Aug. 7, 2017, 10 a.m. UTC | #1

On Fri, 4 Aug 2017 15:20:26 +0200
David Hildenbrand <[e-mail]> wrote:

> On 04.08.2017 13:29, Cornelia Huck wrote:
> > Don't create the s390 pci host bridge if we do not provide the zpci
> > facility.
> > 
> > Reviewed-by: Thomas Huth <[e-mail]>
> > Acked-by: Christian Borntraeger <[e-mail]>
> > Signed-off-by: Cornelia Huck <[e-mail]>  
> 
> This works, because s390_init_cpus(machine) (which sets up features) is
> called before adding the bus. Mind to add a comment so nobody tries to
> reshuffle these functions?

This is already needed by the flic setup (which also relies on the
features being set up). We really want the cpu setup as early as
possible.

Do you have a good comment handy? :)

David Hildenbrand Aug. 7, 2017, 10:21 a.m. UTC | #2

On 07.08.2017 12:00, Cornelia Huck wrote:
> On Fri, 4 Aug 2017 15:20:26 +0200
> David Hildenbrand <[e-mail]> wrote:
> 
>> On 04.08.2017 13:29, Cornelia Huck wrote:
>>> Don't create the s390 pci host bridge if we do not provide the zpci
>>> facility.
>>>
>>> Reviewed-by: Thomas Huth <[e-mail]>
>>> Acked-by: Christian Borntraeger <[e-mail]>
>>> Signed-off-by: Cornelia Huck <[e-mail]>  
>>
>> This works, because s390_init_cpus(machine) (which sets up features) is
>> called before adding the bus. Mind to add a comment so nobody tries to
>> reshuffle these functions?
> 
> This is already needed by the flic setup (which also relies on the
> features being set up). We really want the cpu setup as early as
> possible.
> 
> Do you have a good comment handy? :)
> 

/* init CPUs (incl. CPU model) early so s390_has_feature() works */

Cornelia Huck Aug. 7, 2017, 11:11 a.m. UTC | #3

On Mon, 7 Aug 2017 12:21:30 +0200
David Hildenbrand <[e-mail]> wrote:

> On 07.08.2017 12:00, Cornelia Huck wrote:
> > On Fri, 4 Aug 2017 15:20:26 +0200
> > David Hildenbrand <[e-mail]> wrote:
> >   
> >> On 04.08.2017 13:29, Cornelia Huck wrote:  
> >>> Don't create the s390 pci host bridge if we do not provide the zpci
> >>> facility.
> >>>
> >>> Reviewed-by: Thomas Huth <[e-mail]>
> >>> Acked-by: Christian Borntraeger <[e-mail]>
> >>> Signed-off-by: Cornelia Huck <[e-mail]>    
> >>
> >> This works, because s390_init_cpus(machine) (which sets up features) is
> >> called before adding the bus. Mind to add a comment so nobody tries to
> >> reshuffle these functions?  
> > 
> > This is already needed by the flic setup (which also relies on the
> > features being set up). We really want the cpu setup as early as
> > possible.
> > 
> > Do you have a good comment handy? :)
> >   
> 
> /* init CPUs (incl. CPU model) early so s390_has_feature() works */
> 

Thanks, I'll go with that.

Patch

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index 1c7af39ce6..8be4a541c1 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -118,7 +118,6 @@  static void ccw_init(MachineState *machine)
 {
     int ret;
     VirtualCssBus *css_bus;
-    DeviceState *dev;
 
     s390_sclp_init();
     s390_memory_init(machine->ram_size);
@@ -134,10 +133,13 @@  static void ccw_init(MachineState *machine)
                       machine->initrd_filename, "s390-ccw.img",
                       "s390-netboot.img", true);
 
-    dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
-    object_property_add_child(qdev_get_machine(), TYPE_S390_PCI_HOST_BRIDGE,
-                              OBJECT(dev), NULL);
-    qdev_init_nofail(dev);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        DeviceState *dev = qdev_create(NULL, TYPE_S390_PCI_HOST_BRIDGE);
+        object_property_add_child(qdev_get_machine(),
+                                  TYPE_S390_PCI_HOST_BRIDGE,
+                                  OBJECT(dev), NULL);
+        qdev_init_nofail(dev);
+    }
 
     /* register hypercalls */
     virtio_ccw_register_hcalls();