[Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Cornelia Huck]

Next version, not so many changes from v3.

As you might have guessed, the goals are still the same:
- Being able to disable PCI support in a build completely.
- Properly fencing off PCI if the relevant facility bit is not provided.

Changes v3->v4:
- introduce pci_available boolean
- use pci_available to fence off setting the zcpi facility bit
- collected tags

Branch is still git://github.com/cohuck/qemu no-zpci-cpumodel

Cornelia Huck (9):
  kvm: remove hard dependency on pci
  s390x/pci: add stubs
  s390x: chsc nt2 events are pci-only
  s390x/pci: do not advertise pci on non-pci builds
  s390x/ccw: create s390 phb conditionally
  s390x/sclp: properly guard pci-specific functions
  s390x/pci: fence off instructions for non-pci
  s390x/kvm: msi route fixup for non-pci
  s390x: refine pci dependencies

 accel/kvm/kvm-all.c               |  6 ++--
 default-configs/s390x-softmmu.mak |  2 +-
 hw/pci/pci-stub.c                 | 13 +++++++
 hw/pci/pci.c                      |  2 ++
 hw/s390x/Makefile.objs            |  3 +-
 hw/s390x/s390-pci-bus.c           |  4 +--
 hw/s390x/s390-pci-bus.h           |  4 +--
 hw/s390x/s390-pci-stub.c          | 74 +++++++++++++++++++++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c        | 12 ++++---
 hw/s390x/sclp.c                   | 19 +++++++---
 include/hw/pci/pci.h              |  2 ++
 target/s390x/ioinst.c             | 16 +++++++++
 target/s390x/kvm.c                | 64 +++++++++++++++++++++++++--------
 13 files changed, 189 insertions(+), 32 deletions(-)
 create mode 100644 hw/s390x/s390-pci-stub.c

-- 
2.13.3

[Qemu-devel] [PATCH v4 1/9] kvm: remove hard dependency on pci

[Cornelia Huck]

The msi routing code in kvm calls some pci functions: provide
some stubs to enable builds without pci.

Also, to make this more obvious, guard them via a pci_available boolean
(which also can be reused in other places).

Fixes: e1d4fb2de ("kvm-irqchip: x86: add msi route notify fn")
Fixes: 767a554a0 ("kvm-all: Pass requester ID to MSI routing functions")
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 accel/kvm/kvm-all.c  |  6 +++---
 hw/pci/pci-stub.c    | 13 +++++++++++++
 hw/pci/pci.c         |  2 ++
 include/hw/pci/pci.h |  2 ++
 4 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index 46ce479dc3..f85553a851 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -1248,7 +1248,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
     int virq;
     MSIMessage msg = {0, 0};
 
-    if (dev) {
+    if (pci_available && dev) {
         msg = pci_get_msi_message(dev, vector);
     }
 
@@ -1271,7 +1271,7 @@ int kvm_irqchip_add_msi_route(KVMState *s, int vector, PCIDevice *dev)
     kroute.u.msi.address_lo = (uint32_t)msg.address;
     kroute.u.msi.address_hi = msg.address >> 32;
     kroute.u.msi.data = le32_to_cpu(msg.data);
-    if (kvm_msi_devid_required()) {
+    if (pci_available && kvm_msi_devid_required()) {
         kroute.flags = KVM_MSI_VALID_DEVID;
         kroute.u.msi.devid = pci_requester_id(dev);
     }
@@ -1309,7 +1309,7 @@ int kvm_irqchip_update_msi_route(KVMState *s, int virq, MSIMessage msg,
     kroute.u.msi.address_lo = (uint32_t)msg.address;
     kroute.u.msi.address_hi = msg.address >> 32;
     kroute.u.msi.data = le32_to_cpu(msg.data);
-    if (kvm_msi_devid_required()) {
+    if (pci_available && kvm_msi_devid_required()) {
         kroute.flags = KVM_MSI_VALID_DEVID;
         kroute.u.msi.devid = pci_requester_id(dev);
     }
diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
index ecad664946..877c5d165e 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -23,10 +23,12 @@
 #include "monitor/monitor.h"
 #include "qapi/qmp/qerror.h"
 #include "hw/pci/pci.h"
+#include "hw/pci/msi.h"
 #include "qmp-commands.h"
 #include "hw/pci/msi.h"
 
 bool msi_nonbroken;
+bool pci_available;
 
 PciInfoList *qmp_query_pci(Error **errp)
 {
@@ -38,3 +40,14 @@ void hmp_pcie_aer_inject_error(Monitor *mon, const QDict *qdict)
 {
     monitor_printf(mon, "PCI devices not supported\n");
 }
+
+/* kvm-all wants this */
+MSIMessage pci_get_msi_message(PCIDevice *dev, int vector)
+{
+    assert(false);
+}
+
+uint16_t pci_requester_id(PCIDevice *dev)
+{
+    assert(false);
+}
diff --git a/hw/pci/pci.c b/hw/pci/pci.c
index 258fbe51e2..26f346db2c 100644
--- a/hw/pci/pci.c
+++ b/hw/pci/pci.c
@@ -49,6 +49,8 @@
 # define PCI_DPRINTF(format, ...)       do { } while (0)
 #endif
 
+bool pci_available = true;
+
 static void pcibus_dev_print(Monitor *mon, DeviceState *dev, int indent);
 static char *pcibus_get_dev_path(DeviceState *dev);
 static char *pcibus_get_fw_dev_path(DeviceState *dev);
diff --git a/include/hw/pci/pci.h b/include/hw/pci/pci.h
index e598b095eb..8bb6449dd7 100644
--- a/include/hw/pci/pci.h
+++ b/include/hw/pci/pci.h
@@ -10,6 +10,8 @@
 
 #include "hw/pci/pcie.h"
 
+extern bool pci_available;
+
 /* PCI bus */
 
 #define PCI_DEVFN(slot, func)   ((((slot) & 0x1f) << 3) | ((func) & 0x07))
-- 
2.13.3

[Qemu-devel] [PATCH v4 2/9] s390x/pci: add stubs

[Cornelia Huck]

Some non-pci code calls into zpci code. Provide some stubs for builds
without pci.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/Makefile.objs   |  3 +-
 hw/s390x/s390-pci-stub.c | 74 ++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 76 insertions(+), 1 deletion(-)
 create mode 100644 hw/s390x/s390-pci-stub.c

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index b2aade2466..7ee19d3abc 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -11,7 +11,8 @@ obj-y += 3270-ccw.o
 obj-y += virtio-ccw.o
 obj-y += css-bridge.o
 obj-y += ccw-device.o
-obj-y += s390-pci-bus.o s390-pci-inst.o
+obj-$(CONFIG_PCI) += s390-pci-bus.o s390-pci-inst.o
+obj-$(call lnot,$(CONFIG_PCI)) += s390-pci-stub.o
 obj-y += s390-skeys.o
 obj-y += s390-stattrib.o
 obj-$(CONFIG_KVM) += s390-skeys-kvm.o
diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
new file mode 100644
index 0000000000..2e7e42a2af
--- /dev/null
+++ b/hw/s390x/s390-pci-stub.c
@@ -0,0 +1,74 @@
+/* stubs for non-pci builds */
+
+#include "qemu/osdep.h"
+#include "qemu-common.h"
+#include "cpu.h"
+#include "s390-pci-inst.h"
+#include "s390-pci-bus.h"
+
+/* target/s390x/ioinst.c */
+int chsc_sei_nt2_get_event(void *res)
+{
+    return 1;
+}
+
+int chsc_sei_nt2_have_event(void)
+{
+    return 0;
+}
+
+/* hw/s390x/sclp.c */
+void s390_pci_sclp_configure(SCCB *sccb)
+{
+}
+
+void s390_pci_sclp_deconfigure(SCCB *sccb)
+{
+}
+
+/* target/s390x/kvm.c */
+int clp_service_call(S390CPU *cpu, uint8_t r2)
+{
+    return -1;
+}
+
+int pcilg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
+{
+    return -1;
+}
+
+int pcistg_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
+{
+    return -1;
+}
+
+int stpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
+{
+    return -1;
+}
+
+int rpcit_service_call(S390CPU *cpu, uint8_t r1, uint8_t r2)
+{
+    return -1;
+}
+
+int pcistb_service_call(S390CPU *cpu, uint8_t r1, uint8_t r3, uint64_t gaddr,
+                        uint8_t ar)
+{
+    return -1;
+}
+
+int mpcifc_service_call(S390CPU *cpu, uint8_t r1, uint64_t fiba, uint8_t ar)
+{
+    return -1;
+}
+
+S390pciState *s390_get_phb(void)
+{
+    return NULL;
+}
+
+S390PCIBusDevice *s390_pci_find_dev_by_idx(S390pciState *s, uint32_t idx)
+{
+    return NULL;
+}
-- 
2.13.3

[Qemu-devel] [PATCH v4 3/9] s390x: chsc nt2 events are pci-only

[Cornelia Huck]

The nt2 event class is pci-only - don't look for events if pci is
not in the active cpu model.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/s390-pci-bus.c  |  4 ++--
 hw/s390x/s390-pci-bus.h  |  4 ++--
 hw/s390x/s390-pci-stub.c |  4 ++--
 target/s390x/ioinst.c    | 16 ++++++++++++++++
 4 files changed, 22 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index 61cfd2138f..c57f6ebae0 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -47,7 +47,7 @@ S390pciState *s390_get_phb(void)
     return phb;
 }
 
-int chsc_sei_nt2_get_event(void *res)
+int pci_chsc_sei_nt2_get_event(void *res)
 {
     ChscSeiNt2Res *nt2_res = (ChscSeiNt2Res *)res;
     PciCcdfAvail *accdf;
@@ -87,7 +87,7 @@ int chsc_sei_nt2_get_event(void *res)
     return rc;
 }
 
-int chsc_sei_nt2_have_event(void)
+int pci_chsc_sei_nt2_have_event(void)
 {
     S390pciState *s = s390_get_phb();
 
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index 67af2c12ff..5df6292509 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -319,8 +319,8 @@ typedef struct S390pciState {
 } S390pciState;
 
 S390pciState *s390_get_phb(void);
-int chsc_sei_nt2_get_event(void *res);
-int chsc_sei_nt2_have_event(void);
+int pci_chsc_sei_nt2_get_event(void *res);
+int pci_chsc_sei_nt2_have_event(void);
 void s390_pci_sclp_configure(SCCB *sccb);
 void s390_pci_sclp_deconfigure(SCCB *sccb);
 void s390_pci_iommu_enable(S390PCIIOMMU *iommu);
diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
index 2e7e42a2af..cc7278a865 100644
--- a/hw/s390x/s390-pci-stub.c
+++ b/hw/s390x/s390-pci-stub.c
@@ -7,12 +7,12 @@
 #include "s390-pci-bus.h"
 
 /* target/s390x/ioinst.c */
-int chsc_sei_nt2_get_event(void *res)
+int pci_chsc_sei_nt2_get_event(void *res)
 {
     return 1;
 }
 
-int chsc_sei_nt2_have_event(void)
+int pci_chsc_sei_nt2_have_event(void)
 {
     return 0;
 }
diff --git a/target/s390x/ioinst.c b/target/s390x/ioinst.c
index 51fbea620d..3fa3301f50 100644
--- a/target/s390x/ioinst.c
+++ b/target/s390x/ioinst.c
@@ -599,6 +599,22 @@ static int chsc_sei_nt0_have_event(void)
     return 0;
 }
 
+static int chsc_sei_nt2_get_event(void *res)
+{
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        return pci_chsc_sei_nt2_get_event(res);
+    }
+    return 1;
+}
+
+static int chsc_sei_nt2_have_event(void)
+{
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        return pci_chsc_sei_nt2_have_event();
+    }
+    return 0;
+}
+
 #define CHSC_SEI_NT0    (1ULL << 63)
 #define CHSC_SEI_NT2    (1ULL << 61)
 static void ioinst_handle_chsc_sei(ChscReq *req, ChscResp *res)
-- 
2.13.3

[Qemu-devel] [PATCH v4 4/9] s390x/pci: do not advertise pci on non-pci builds

[Cornelia Huck]

Only set the zpci feature bit on builds that actually support pci.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/kvm.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index c4c5791d27..bc62bba5b7 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2662,7 +2662,9 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
     }
 
     /* We emulate a zPCI bus and AEN, therefore we don't need HW support */
-    set_bit(S390_FEAT_ZPCI, model->features);
+    if (pci_available) {
+        set_bit(S390_FEAT_ZPCI, model->features);
+    }
     set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features);
 
     if (s390_known_cpu_type(cpu_type)) {
-- 
2.13.3

Re: [Qemu-devel] [PATCH v4 4/9] s390x/pci: do not advertise pci on non-pci builds

[David Hildenbrand]

On 04.08.2017 13:29, Cornelia Huck wrote:
> Only set the zpci feature bit on builds that actually support pci.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  target/s390x/kvm.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index c4c5791d27..bc62bba5b7 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2662,7 +2662,9 @@ void kvm_s390_get_host_cpu_model(S390CPUModel *model, Error **errp)
>      }
>  
>      /* We emulate a zPCI bus and AEN, therefore we don't need HW support */
> -    set_bit(S390_FEAT_ZPCI, model->features);
> +    if (pci_available) {
> +        set_bit(S390_FEAT_ZPCI, model->features);
> +    }
>      set_bit(S390_FEAT_ADAPTER_EVENT_NOTIFICATION, model->features);
>  
>      if (s390_known_cpu_type(cpu_type)) {
> 

Yup, looks good to me.

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

[Qemu-devel] [PATCH v4 5/9] s390x/ccw: create s390 phb conditionally

[Cornelia Huck]

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

Reviewed-by: Thomas Huth <thuth@redhat.com>
Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/s390-virtio-ccw.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

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();
-- 
2.13.3

Re: [Qemu-devel] [PATCH v4 5/9] s390x/ccw: create s390 phb conditionally

[David Hildenbrand]

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 <thuth@redhat.com>
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>

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?

> ---
>  hw/s390x/s390-virtio-ccw.c | 12 +++++++-----
>  1 file changed, 7 insertions(+), 5 deletions(-)
> 
> 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();
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v4 5/9] s390x/ccw: create s390 phb conditionally

[Cornelia Huck]

On Fri, 4 Aug 2017 15:20:26 +0200
David Hildenbrand <david@redhat.com> 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 <thuth@redhat.com>
> > Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>  
> 
> 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? :)

Re: [Qemu-devel] [PATCH v4 5/9] s390x/ccw: create s390 phb conditionally

[David Hildenbrand]

On 07.08.2017 12:00, Cornelia Huck wrote:
> On Fri, 4 Aug 2017 15:20:26 +0200
> David Hildenbrand <david@redhat.com> 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 <thuth@redhat.com>
>>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>  
>>
>> 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,

David

Re: [Qemu-devel] [PATCH v4 5/9] s390x/ccw: create s390 phb conditionally

[Cornelia Huck]

On Mon, 7 Aug 2017 12:21:30 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 07.08.2017 12:00, Cornelia Huck wrote:
> > On Fri, 4 Aug 2017 15:20:26 +0200
> > David Hildenbrand <david@redhat.com> 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 <thuth@redhat.com>
> >>> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>
> >>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>    
> >>
> >> 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.

[Qemu-devel] [PATCH v4 6/9] s390x/sclp: properly guard pci-specific functions

[Cornelia Huck]

If we do not provide zpci, pci reconfiguration via sclp is not available
either. Don't indicate it in the sclp facilities and return an invalid
command if the guest tries to issue pci configure/deconfigure.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/sclp.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 9253dbbc64..d0104cd784 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -59,6 +59,7 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
     int rnsize, rnmax;
     int slots = MIN(machine->ram_slots, s390_get_memslot_count(kvm_state));
     IplParameterBlock *ipib = s390_ipl_get_iplb();
+    uint64_t sclp_facilities = SCLP_HAS_CPU_INFO;
 
     CPU_FOREACH(cpu) {
         cpu_count++;
@@ -79,8 +80,10 @@ static void read_SCP_info(SCLPDevice *sclp, SCCB *sccb)
 
     prepare_cpu_entries(sclp, read_info->entries, cpu_count);
 
-    read_info->facilities = cpu_to_be64(SCLP_HAS_CPU_INFO |
-                                        SCLP_HAS_PCI_RECONFIG);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        sclp_facilities |= SCLP_HAS_PCI_RECONFIG;
+    }
+    read_info->facilities = cpu_to_be64(sclp_facilities);
 
     /* Memory Hotplug is only supported for the ccw machine type */
     if (mhd) {
@@ -385,10 +388,18 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code)
         sclp_c->unassign_storage(sclp, sccb);
         break;
     case SCLP_CMDW_CONFIGURE_PCI:
-        s390_pci_sclp_configure(sccb);
+        if (s390_has_feat(S390_FEAT_ZPCI)) {
+            s390_pci_sclp_configure(sccb);
+        } else {
+            sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+        }
         break;
     case SCLP_CMDW_DECONFIGURE_PCI:
-        s390_pci_sclp_deconfigure(sccb);
+        if (s390_has_feat(S390_FEAT_ZPCI)) {
+            s390_pci_sclp_deconfigure(sccb);
+        } else {
+            sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
+        }
         break;
     default:
         efc->command_handler(ef, sccb, code);
-- 
2.13.3

[Qemu-devel] [PATCH v4 7/9] s390x/pci: fence off instructions for non-pci

[Cornelia Huck]

If a guest running on a machine without zpci issues a pci instruction,
throw them an exception.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/kvm.c | 54 +++++++++++++++++++++++++++++++++++++++++-------------
 1 file changed, 41 insertions(+), 13 deletions(-)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index bc62bba5b7..9de165d8b1 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -1191,7 +1191,11 @@ static int kvm_clp_service_call(S390CPU *cpu, struct kvm_run *run)
 {
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
-    return clp_service_call(cpu, r2);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        return clp_service_call(cpu, r2);
+    } else {
+        return -1;
+    }
 }
 
 static int kvm_pcilg_service_call(S390CPU *cpu, struct kvm_run *run)
@@ -1199,7 +1203,11 @@ static int kvm_pcilg_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r1 = (run->s390_sieic.ipb & 0x00f00000) >> 20;
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
-    return pcilg_service_call(cpu, r1, r2);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        return pcilg_service_call(cpu, r1, r2);
+    } else {
+        return -1;
+    }
 }
 
 static int kvm_pcistg_service_call(S390CPU *cpu, struct kvm_run *run)
@@ -1207,7 +1215,11 @@ static int kvm_pcistg_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r1 = (run->s390_sieic.ipb & 0x00f00000) >> 20;
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
-    return pcistg_service_call(cpu, r1, r2);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        return pcistg_service_call(cpu, r1, r2);
+    } else {
+        return -1;
+    }
 }
 
 static int kvm_stpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
@@ -1216,10 +1228,14 @@ static int kvm_stpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
     uint64_t fiba;
     uint8_t ar;
 
-    cpu_synchronize_state(CPU(cpu));
-    fiba = get_base_disp_rxy(cpu, run, &ar);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        cpu_synchronize_state(CPU(cpu));
+        fiba = get_base_disp_rxy(cpu, run, &ar);
 
-    return stpcifc_service_call(cpu, r1, fiba, ar);
+        return stpcifc_service_call(cpu, r1, fiba, ar);
+    } else {
+        return -1;
+    }
 }
 
 static int kvm_sic_service_call(S390CPU *cpu, struct kvm_run *run)
@@ -1247,7 +1263,11 @@ static int kvm_rpcit_service_call(S390CPU *cpu, struct kvm_run *run)
     uint8_t r1 = (run->s390_sieic.ipb & 0x00f00000) >> 20;
     uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
 
-    return rpcit_service_call(cpu, r1, r2);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        return rpcit_service_call(cpu, r1, r2);
+    } else {
+        return -1;
+    }
 }
 
 static int kvm_pcistb_service_call(S390CPU *cpu, struct kvm_run *run)
@@ -1257,10 +1277,14 @@ static int kvm_pcistb_service_call(S390CPU *cpu, struct kvm_run *run)
     uint64_t gaddr;
     uint8_t ar;
 
-    cpu_synchronize_state(CPU(cpu));
-    gaddr = get_base_disp_rsy(cpu, run, &ar);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        cpu_synchronize_state(CPU(cpu));
+        gaddr = get_base_disp_rsy(cpu, run, &ar);
 
-    return pcistb_service_call(cpu, r1, r3, gaddr, ar);
+        return pcistb_service_call(cpu, r1, r3, gaddr, ar);
+    } else {
+        return -1;
+    }
 }
 
 static int kvm_mpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
@@ -1269,10 +1293,14 @@ static int kvm_mpcifc_service_call(S390CPU *cpu, struct kvm_run *run)
     uint64_t fiba;
     uint8_t ar;
 
-    cpu_synchronize_state(CPU(cpu));
-    fiba = get_base_disp_rxy(cpu, run, &ar);
+    if (s390_has_feat(S390_FEAT_ZPCI)) {
+        cpu_synchronize_state(CPU(cpu));
+        fiba = get_base_disp_rxy(cpu, run, &ar);
 
-    return mpcifc_service_call(cpu, r1, fiba, ar);
+        return mpcifc_service_call(cpu, r1, fiba, ar);
+    } else {
+        return -1;
+    }
 }
 
 static int handle_b9(S390CPU *cpu, struct kvm_run *run, uint8_t ipa1)
-- 
2.13.3

Re: [Qemu-devel] [PATCH v4 7/9] s390x/pci: fence off instructions for non-pci

[David Hildenbrand]

On 04.08.2017 13:29, Cornelia Huck wrote:
> If a guest running on a machine without zpci issues a pci instruction,
> throw them an exception.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  target/s390x/kvm.c | 54 +++++++++++++++++++++++++++++++++++++++++-------------
>  1 file changed, 41 insertions(+), 13 deletions(-)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index bc62bba5b7..9de165d8b1 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -1191,7 +1191,11 @@ static int kvm_clp_service_call(S390CPU *cpu, struct kvm_run *run)
>  {
>      uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
>  
> -    return clp_service_call(cpu, r2);
> +    if (s390_has_feat(S390_FEAT_ZPCI)) {
> +        return clp_service_call(cpu, r2);
> +    } else {
> +        return -1;
> +    }

I am a fan of dropping these else case and returning directly. But that
is just my opinion.

(applies to all changes in this patch)


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v4 7/9] s390x/pci: fence off instructions for non-pci

[Cornelia Huck]

On Fri, 4 Aug 2017 15:17:25 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 04.08.2017 13:29, Cornelia Huck wrote:
> > If a guest running on a machine without zpci issues a pci instruction,
> > throw them an exception.
> > 
> > Reviewed-by: Thomas Huth <thuth@redhat.com>
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  target/s390x/kvm.c | 54 +++++++++++++++++++++++++++++++++++++++++-------------
> >  1 file changed, 41 insertions(+), 13 deletions(-)
> > 
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index bc62bba5b7..9de165d8b1 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -1191,7 +1191,11 @@ static int kvm_clp_service_call(S390CPU *cpu, struct kvm_run *run)
> >  {
> >      uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
> >  
> > -    return clp_service_call(cpu, r2);
> > +    if (s390_has_feat(S390_FEAT_ZPCI)) {
> > +        return clp_service_call(cpu, r2);
> > +    } else {
> > +        return -1;
> > +    }  
> 
> I am a fan of dropping these else case and returning directly. But that
> is just my opinion.
> 
> (applies to all changes in this patch)
> 
> 

You are not the first to say that :)

I do prefer this way around, though, and if there aren't strong
objections, I'll keep it like this.

Re: [Qemu-devel] [PATCH v4 7/9] s390x/pci: fence off instructions for non-pci

[David Hildenbrand]

On 07.08.2017 11:52, Cornelia Huck wrote:
> On Fri, 4 Aug 2017 15:17:25 +0200
> David Hildenbrand <david@redhat.com> wrote:
> 
>> On 04.08.2017 13:29, Cornelia Huck wrote:
>>> If a guest running on a machine without zpci issues a pci instruction,
>>> throw them an exception.
>>>
>>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
>>> ---
>>>  target/s390x/kvm.c | 54 +++++++++++++++++++++++++++++++++++++++++-------------
>>>  1 file changed, 41 insertions(+), 13 deletions(-)
>>>
>>> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
>>> index bc62bba5b7..9de165d8b1 100644
>>> --- a/target/s390x/kvm.c
>>> +++ b/target/s390x/kvm.c
>>> @@ -1191,7 +1191,11 @@ static int kvm_clp_service_call(S390CPU *cpu, struct kvm_run *run)
>>>  {
>>>      uint8_t r2 = (run->s390_sieic.ipb & 0x000f0000) >> 16;
>>>  
>>> -    return clp_service_call(cpu, r2);
>>> +    if (s390_has_feat(S390_FEAT_ZPCI)) {
>>> +        return clp_service_call(cpu, r2);
>>> +    } else {
>>> +        return -1;
>>> +    }  
>>
>> I am a fan of dropping these else case and returning directly. But that
>> is just my opinion.
>>
>> (applies to all changes in this patch)
>>
>>
> 
> You are not the first to say that :)

so ... at least 2 vs. 1? ;)

Nevermind, doesn't really matter.

> 
> I do prefer this way around, though, and if there aren't strong
> objections, I'll keep it like this.
> 


-- 

Thanks,

David

[Qemu-devel] [PATCH v4 8/9] s390x/kvm: msi route fixup for non-pci

[Cornelia Huck]

If we don't provide pci, we cannot have a pci device for which we
have to translate to adapter routes: just return -ENODEV.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 target/s390x/kvm.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index 9de165d8b1..d8db1cbf6e 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2424,6 +2424,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
     uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
     uint32_t vec = data & ZPCI_MSI_VEC_MASK;
 
+    if (!s390_has_feat(S390_FEAT_ZPCI)) {
+        /* How can we get here without pci enabled? */
+        g_assert(false);
+        return -ENODEV;
+    }
+
     pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
     if (!pbdev) {
         DPRINTF("add_msi_route no dev\n");
-- 
2.13.3

Re: [Qemu-devel] [PATCH v4 8/9] s390x/kvm: msi route fixup for non-pci

[David Hildenbrand]

On 04.08.2017 13:29, Cornelia Huck wrote:
> If we don't provide pci, we cannot have a pci device for which we
> have to translate to adapter routes: just return -ENODEV.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  target/s390x/kvm.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> index 9de165d8b1..d8db1cbf6e 100644
> --- a/target/s390x/kvm.c
> +++ b/target/s390x/kvm.c
> @@ -2424,6 +2424,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
>      uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
>      uint32_t vec = data & ZPCI_MSI_VEC_MASK;
>  
> +    if (!s390_has_feat(S390_FEAT_ZPCI)) {
> +        /* How can we get here without pci enabled? */
> +        g_assert(false);
> +        return -ENODEV;
> +    }
> +

/* How can we get here without pci enabled? */
g_assert(s390_has_feat(S390_FEAT_ZPCI));

?

>      pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
>      if (!pbdev) {
>          DPRINTF("add_msi_route no dev\n");
> 


-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v4 8/9] s390x/kvm: msi route fixup for non-pci

[Cornelia Huck]

On Fri, 4 Aug 2017 15:18:14 +0200
David Hildenbrand <david@redhat.com> wrote:

> On 04.08.2017 13:29, Cornelia Huck wrote:
> > If we don't provide pci, we cannot have a pci device for which we
> > have to translate to adapter routes: just return -ENODEV.
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  target/s390x/kvm.c | 6 ++++++
> >  1 file changed, 6 insertions(+)
> > 
> > diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
> > index 9de165d8b1..d8db1cbf6e 100644
> > --- a/target/s390x/kvm.c
> > +++ b/target/s390x/kvm.c
> > @@ -2424,6 +2424,12 @@ int kvm_arch_fixup_msi_route(struct kvm_irq_routing_entry *route,
> >      uint32_t idx = data >> ZPCI_MSI_VEC_BITS;
> >      uint32_t vec = data & ZPCI_MSI_VEC_MASK;
> >  
> > +    if (!s390_has_feat(S390_FEAT_ZPCI)) {
> > +        /* How can we get here without pci enabled? */
> > +        g_assert(false);
> > +        return -ENODEV;
> > +    }
> > +  
> 
> /* How can we get here without pci enabled? */
> g_assert(s390_has_feat(S390_FEAT_ZPCI));
> 
> ?

What I don't like about this is that it implicitly relies on
s390_pci_find_dev_by_idx() doing the correct thing if asserts are off -
and it is non-obvious that we'll get pbdev == NULL in that case.

> 
> >      pbdev = s390_pci_find_dev_by_idx(s390_get_phb(), idx);
> >      if (!pbdev) {
> >          DPRINTF("add_msi_route no dev\n");
> >   
> 
> 

[Qemu-devel] [PATCH v4 9/9] s390x: refine pci dependencies

[Cornelia Huck]

VIRTIO_PCI should properly depend on CONFIG_PCI.
With this change, we can switch off pci for s390x by removing
'CONFIG_PCI=y' from the default config.

Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 default-configs/s390x-softmmu.mak | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
index b227a36179..7adaa4a4fe 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -1,5 +1,5 @@
 CONFIG_PCI=y
-CONFIG_VIRTIO_PCI=y
+CONFIG_VIRTIO_PCI=$(CONFIG_PCI)
 CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
 CONFIG_VIRTIO=y
 CONFIG_SCLPCONSOLE=y
-- 
2.13.3

Re: [Qemu-devel] [PATCH v4 9/9] s390x: refine pci dependencies

[David Hildenbrand]

On 04.08.2017 13:29, Cornelia Huck wrote:
> VIRTIO_PCI should properly depend on CONFIG_PCI.
> With this change, we can switch off pci for s390x by removing
> 'CONFIG_PCI=y' from the default config.
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  default-configs/s390x-softmmu.mak | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> index b227a36179..7adaa4a4fe 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -1,5 +1,5 @@
>  CONFIG_PCI=y
> -CONFIG_VIRTIO_PCI=y
> +CONFIG_VIRTIO_PCI=$(CONFIG_PCI)
>  CONFIG_VHOST_USER_SCSI=$(CONFIG_LINUX)
>  CONFIG_VIRTIO=y
>  CONFIG_SCLPCONSOLE=y
> 

Reviewed-by: David Hildenbrand <david@redhat.com>

-- 

Thanks,

David

Re: [Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Cornelia Huck]

On Fri,  4 Aug 2017 13:29:37 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> Next version, not so many changes from v3.
> 
> As you might have guessed, the goals are still the same:
> - Being able to disable PCI support in a build completely.
> - Properly fencing off PCI if the relevant facility bit is not provided.
> 
> Changes v3->v4:
> - introduce pci_available boolean
> - use pci_available to fence off setting the zcpi facility bit
> - collected tags
> 
> Branch is still git://github.com/cohuck/qemu no-zpci-cpumodel

make check on a build with pci disabled revealed an interesting
inconsistency: We create a virtio-9p-ccw device, but the base
virtio-9p-device is in code that is not built for !pci.

If I remove the pci dependency for hw/9pfs/ and fsdev/, things look
fine (at least on s390x). We probably need a different dependency,
though.

virtio-9p maintainers, any suggestions?

Re: [Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Cornelia Huck]

On Fri, 4 Aug 2017 16:59:34 +0200
Cornelia Huck <cohuck@redhat.com> wrote:

> On Fri,  4 Aug 2017 13:29:37 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
> > Next version, not so many changes from v3.
> > 
> > As you might have guessed, the goals are still the same:
> > - Being able to disable PCI support in a build completely.
> > - Properly fencing off PCI if the relevant facility bit is not provided.
> > 
> > Changes v3->v4:
> > - introduce pci_available boolean
> > - use pci_available to fence off setting the zcpi facility bit
> > - collected tags
> > 
> > Branch is still git://github.com/cohuck/qemu no-zpci-cpumodel  
> 
> make check on a build with pci disabled revealed an interesting
> inconsistency: We create a virtio-9p-ccw device, but the base
> virtio-9p-device is in code that is not built for !pci.
> 
> If I remove the pci dependency for hw/9pfs/ and fsdev/, things look
> fine (at least on s390x). We probably need a different dependency,
> though.
> 
> virtio-9p maintainers, any suggestions?

I have the patch below, which is ugly, but seems to work for me. Better
ideas welcome :)

From 0ba6427e7ac7cef38b487d28c9dce653d8cb9a71 Mon Sep 17 00:00:00 2001
From: Cornelia Huck <cohuck@redhat.com>
Date: Tue, 8 Aug 2017 11:03:38 +0200
Subject: [PATCH] 9pfs: fix dependencies

Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
on CONFIG_VIRTFS and on the presence of an appropriate virtio transport
device.

Let's introduce CONFIG_VIRTIO_CCW to cover s390x and check for
CONFIG_VIRTFS && (CONFIG_VIRTIO_PCI || CONFIG_VIRTIO_CCW).

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 default-configs/s390x-softmmu.mak | 1 +
 fsdev/Makefile.objs               | 9 +++------
 hw/Makefile.objs                  | 2 +-
 3 files changed, 5 insertions(+), 7 deletions(-)

diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
index 6ab2bc65ac..7f15ab68b1 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
 CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
 CONFIG_VFIO_CCW=$(CONFIG_LINUX)
 CONFIG_WDT_DIAG288=y
+CONFIG_VIRTIO_CCW=y
diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 659df6e187..10d8caa291 100644
--- a/fsdev/Makefile.objs
+++ b/fsdev/Makefile.objs
@@ -1,10 +1,7 @@
-ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
 # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
-# only pull in the actual virtio-9p device if we also enabled virtio.
-common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
-else
-common-obj-y = qemu-fsdev-dummy.o
-endif
+# only pull in the actual virtio-9p device if we also enabled a virtio backend.
+common-obj-$(call land, $(CONFIG_VIRTFS),$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
+common-obj-$(call lnot, $(call land, $(CONFIG_VIRTFS),$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
 common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
 
 # Toplevel always builds this; targets without virtio will put it in
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index a2c61f6b09..10942fe0b4 100644
--- a/hw/Makefile.objs
+++ b/hw/Makefile.objs
@@ -1,4 +1,4 @@
-devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
+devices-dirs-$(call land, $(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
 devices-dirs-$(CONFIG_SOFTMMU) += acpi/
 devices-dirs-$(CONFIG_SOFTMMU) += adc/
 devices-dirs-$(CONFIG_SOFTMMU) += audio/
-- 
2.13.4

Re: [Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Thomas Huth]

On 08.08.2017 11:15, Cornelia Huck wrote:
> On Fri, 4 Aug 2017 16:59:34 +0200
> Cornelia Huck <cohuck@redhat.com> wrote:
> 
>> On Fri,  4 Aug 2017 13:29:37 +0200
>> Cornelia Huck <cohuck@redhat.com> wrote:
>>
>>> Next version, not so many changes from v3.
>>>
>>> As you might have guessed, the goals are still the same:
>>> - Being able to disable PCI support in a build completely.
>>> - Properly fencing off PCI if the relevant facility bit is not provided.
>>>
>>> Changes v3->v4:
>>> - introduce pci_available boolean
>>> - use pci_available to fence off setting the zcpi facility bit
>>> - collected tags
>>>
>>> Branch is still git://github.com/cohuck/qemu no-zpci-cpumodel  
>>
>> make check on a build with pci disabled revealed an interesting
>> inconsistency: We create a virtio-9p-ccw device, but the base
>> virtio-9p-device is in code that is not built for !pci.
>>
>> If I remove the pci dependency for hw/9pfs/ and fsdev/, things look
>> fine (at least on s390x). We probably need a different dependency,
>> though.
>>
>> virtio-9p maintainers, any suggestions?
> 
> I have the patch below, which is ugly, but seems to work for me. Better
> ideas welcome :)

I haven't tried whether it works, but you could maybe change the define
of CONFIG_VIRTFS instead:

diff --git a/configure b/configure
index dd73cce..64d21f6 100755
--- a/configure
+++ b/configure
@@ -5771,7 +5771,7 @@ if test "$libattr" = "yes" ; then
   echo "CONFIG_LIBATTR=y" >> $config_host_mak
 fi
 if test "$virtfs" = "yes" ; then
-  echo "CONFIG_VIRTFS=y" >> $config_host_mak
+  echo 'CONFIG_VIRTFS=$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)' >> $config_host_mak
 fi
 if test "$vhost_scsi" = "yes" ; then
   echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak

... I think that should simplify the other statements quite a bit since
you then only have to test CONFIG_VIRTFS in the other locations?

 Thomas


> From 0ba6427e7ac7cef38b487d28c9dce653d8cb9a71 Mon Sep 17 00:00:00 2001
> From: Cornelia Huck <cohuck@redhat.com>
> Date: Tue, 8 Aug 2017 11:03:38 +0200
> Subject: [PATCH] 9pfs: fix dependencies
> 
> Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
> on CONFIG_VIRTFS and on the presence of an appropriate virtio transport
> device.
> 
> Let's introduce CONFIG_VIRTIO_CCW to cover s390x and check for
> CONFIG_VIRTFS && (CONFIG_VIRTIO_PCI || CONFIG_VIRTIO_CCW).
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  default-configs/s390x-softmmu.mak | 1 +
>  fsdev/Makefile.objs               | 9 +++------
>  hw/Makefile.objs                  | 2 +-
>  3 files changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> index 6ab2bc65ac..7f15ab68b1 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
>  CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
>  CONFIG_VFIO_CCW=$(CONFIG_LINUX)
>  CONFIG_WDT_DIAG288=y
> +CONFIG_VIRTIO_CCW=y
> diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> index 659df6e187..10d8caa291 100644
> --- a/fsdev/Makefile.objs
> +++ b/fsdev/Makefile.objs
> @@ -1,10 +1,7 @@
> -ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
>  # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> -# only pull in the actual virtio-9p device if we also enabled virtio.
> -common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> -else
> -common-obj-y = qemu-fsdev-dummy.o
> -endif
> +# only pull in the actual virtio-9p device if we also enabled a virtio backend.
> +common-obj-$(call land, $(CONFIG_VIRTFS),$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> +common-obj-$(call lnot, $(call land, $(CONFIG_VIRTFS),$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
>  common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
>  
>  # Toplevel always builds this; targets without virtio will put it in
> diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> index a2c61f6b09..10942fe0b4 100644
> --- a/hw/Makefile.objs
> +++ b/hw/Makefile.objs
> @@ -1,4 +1,4 @@
> -devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
> +devices-dirs-$(call land, $(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
>  devices-dirs-$(CONFIG_SOFTMMU) += acpi/
>  devices-dirs-$(CONFIG_SOFTMMU) += adc/
>  devices-dirs-$(CONFIG_SOFTMMU) += audio/
> 

Re: [Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Cornelia Huck]

On Tue, 8 Aug 2017 11:29:50 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 08.08.2017 11:15, Cornelia Huck wrote:
> > On Fri, 4 Aug 2017 16:59:34 +0200
> > Cornelia Huck <cohuck@redhat.com> wrote:
> >   
> >> On Fri,  4 Aug 2017 13:29:37 +0200
> >> Cornelia Huck <cohuck@redhat.com> wrote:
> >>  
> >>> Next version, not so many changes from v3.
> >>>
> >>> As you might have guessed, the goals are still the same:
> >>> - Being able to disable PCI support in a build completely.
> >>> - Properly fencing off PCI if the relevant facility bit is not provided.
> >>>
> >>> Changes v3->v4:
> >>> - introduce pci_available boolean
> >>> - use pci_available to fence off setting the zcpi facility bit
> >>> - collected tags
> >>>
> >>> Branch is still git://github.com/cohuck/qemu no-zpci-cpumodel    
> >>
> >> make check on a build with pci disabled revealed an interesting
> >> inconsistency: We create a virtio-9p-ccw device, but the base
> >> virtio-9p-device is in code that is not built for !pci.
> >>
> >> If I remove the pci dependency for hw/9pfs/ and fsdev/, things look
> >> fine (at least on s390x). We probably need a different dependency,
> >> though.
> >>
> >> virtio-9p maintainers, any suggestions?  
> > 
> > I have the patch below, which is ugly, but seems to work for me. Better
> > ideas welcome :)  
> 
> I haven't tried whether it works, but you could maybe change the define
> of CONFIG_VIRTFS instead:
> 
> diff --git a/configure b/configure
> index dd73cce..64d21f6 100755
> --- a/configure
> +++ b/configure
> @@ -5771,7 +5771,7 @@ if test "$libattr" = "yes" ; then
>    echo "CONFIG_LIBATTR=y" >> $config_host_mak
>  fi
>  if test "$virtfs" = "yes" ; then
> -  echo "CONFIG_VIRTFS=y" >> $config_host_mak
> +  echo 'CONFIG_VIRTFS=$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)' >> $config_host_mak
>  fi
>  if test "$vhost_scsi" = "yes" ; then
>    echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
> 
> ... I think that should simplify the other statements quite a bit since
> you then only have to test CONFIG_VIRTFS in the other locations?
> 
>  Thomas

Would be a simplification if it worked, yes; not sure whether we should
change the semantic of --enable-virtfs to error out if we don't have
either virtio-pci or virtio-ccw?

> 
> 
> > From 0ba6427e7ac7cef38b487d28c9dce653d8cb9a71 Mon Sep 17 00:00:00 2001
> > From: Cornelia Huck <cohuck@redhat.com>
> > Date: Tue, 8 Aug 2017 11:03:38 +0200
> > Subject: [PATCH] 9pfs: fix dependencies
> > 
> > Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
> > on CONFIG_VIRTFS and on the presence of an appropriate virtio transport
> > device.
> > 
> > Let's introduce CONFIG_VIRTIO_CCW to cover s390x and check for
> > CONFIG_VIRTFS && (CONFIG_VIRTIO_PCI || CONFIG_VIRTIO_CCW).
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  default-configs/s390x-softmmu.mak | 1 +
> >  fsdev/Makefile.objs               | 9 +++------
> >  hw/Makefile.objs                  | 2 +-
> >  3 files changed, 5 insertions(+), 7 deletions(-)
> > 
> > diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> > index 6ab2bc65ac..7f15ab68b1 100644
> > --- a/default-configs/s390x-softmmu.mak
> > +++ b/default-configs/s390x-softmmu.mak
> > @@ -8,3 +8,4 @@ CONFIG_S390_FLIC=y
> >  CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
> >  CONFIG_VFIO_CCW=$(CONFIG_LINUX)
> >  CONFIG_WDT_DIAG288=y
> > +CONFIG_VIRTIO_CCW=y
> > diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
> > index 659df6e187..10d8caa291 100644
> > --- a/fsdev/Makefile.objs
> > +++ b/fsdev/Makefile.objs
> > @@ -1,10 +1,7 @@
> > -ifeq ($(CONFIG_VIRTIO)$(CONFIG_VIRTFS)$(CONFIG_PCI),yyy)
> >  # Lots of the fsdev/9pcode is pulled in by vl.c via qemu_fsdev_add.
> > -# only pull in the actual virtio-9p device if we also enabled virtio.
> > -common-obj-y = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> > -else
> > -common-obj-y = qemu-fsdev-dummy.o
> > -endif
> > +# only pull in the actual virtio-9p device if we also enabled a virtio backend.
> > +common-obj-$(call land, $(CONFIG_VIRTFS),$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))= qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
> > +common-obj-$(call lnot, $(call land, $(CONFIG_VIRTFS),$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)))) = qemu-fsdev-dummy.o
> >  common-obj-y += qemu-fsdev-opts.o qemu-fsdev-throttle.o
> >  
> >  # Toplevel always builds this; targets without virtio will put it in
> > diff --git a/hw/Makefile.objs b/hw/Makefile.objs
> > index a2c61f6b09..10942fe0b4 100644
> > --- a/hw/Makefile.objs
> > +++ b/hw/Makefile.objs
> > @@ -1,4 +1,4 @@
> > -devices-dirs-$(call land, $(CONFIG_VIRTIO),$(call land,$(CONFIG_VIRTFS),$(CONFIG_PCI))) += 9pfs/
> > +devices-dirs-$(call land, $(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW))) += 9pfs/
> >  devices-dirs-$(CONFIG_SOFTMMU) += acpi/
> >  devices-dirs-$(CONFIG_SOFTMMU) += adc/
> >  devices-dirs-$(CONFIG_SOFTMMU) += audio/
> >   
> 

Re: [Qemu-devel] [PATCH v4 0/9] s390x: zPCI detangling

[Thomas Huth]

On 08.08.2017 11:46, Cornelia Huck wrote:
> On Tue, 8 Aug 2017 11:29:50 +0200
> Thomas Huth <thuth@redhat.com> wrote:
> 
>> On 08.08.2017 11:15, Cornelia Huck wrote:
>>> On Fri, 4 Aug 2017 16:59:34 +0200
>>> Cornelia Huck <cohuck@redhat.com> wrote:
>>>   
>>>> On Fri,  4 Aug 2017 13:29:37 +0200
>>>> Cornelia Huck <cohuck@redhat.com> wrote:
>>>>  
>>>>> Next version, not so many changes from v3.
>>>>>
>>>>> As you might have guessed, the goals are still the same:
>>>>> - Being able to disable PCI support in a build completely.
>>>>> - Properly fencing off PCI if the relevant facility bit is not provided.
>>>>>
>>>>> Changes v3->v4:
>>>>> - introduce pci_available boolean
>>>>> - use pci_available to fence off setting the zcpi facility bit
>>>>> - collected tags
>>>>>
>>>>> Branch is still git://github.com/cohuck/qemu no-zpci-cpumodel    
>>>>
>>>> make check on a build with pci disabled revealed an interesting
>>>> inconsistency: We create a virtio-9p-ccw device, but the base
>>>> virtio-9p-device is in code that is not built for !pci.
>>>>
>>>> If I remove the pci dependency for hw/9pfs/ and fsdev/, things look
>>>> fine (at least on s390x). We probably need a different dependency,
>>>> though.
>>>>
>>>> virtio-9p maintainers, any suggestions?  
>>>
>>> I have the patch below, which is ugly, but seems to work for me. Better
>>> ideas welcome :)  
>>
>> I haven't tried whether it works, but you could maybe change the define
>> of CONFIG_VIRTFS instead:
>>
>> diff --git a/configure b/configure
>> index dd73cce..64d21f6 100755
>> --- a/configure
>> +++ b/configure
>> @@ -5771,7 +5771,7 @@ if test "$libattr" = "yes" ; then
>>    echo "CONFIG_LIBATTR=y" >> $config_host_mak
>>  fi
>>  if test "$virtfs" = "yes" ; then
>> -  echo "CONFIG_VIRTFS=y" >> $config_host_mak
>> +  echo 'CONFIG_VIRTFS=$(call lor, $(CONFIG_VIRTIO_PCI),$(CONFIG_VIRTIO_CCW)' >> $config_host_mak
>>  fi
>>  if test "$vhost_scsi" = "yes" ; then
>>    echo "CONFIG_VHOST_SCSI=y" >> $config_host_mak
>>
>> ... I think that should simplify the other statements quite a bit since
>> you then only have to test CONFIG_VIRTFS in the other locations?
>>
>>  Thomas
> 
> Would be a simplification if it worked, yes; not sure whether we should
> change the semantic of --enable-virtfs to error out if we don't have
> either virtio-pci or virtio-ccw?

I don't think that this would work: configure is run once for all
targets, but the CONFIG_PCI and CONFIG_CCW settings are only valid for
individual targets, so you can not use the value of these config
variables during "configure" yet.

 Thomas