All of lore.kernel.org
 help / color / mirror / Atom feed
* [Qemu-devel] [PATCH v5 0/9] zpci detangling
@ 2017-08-23 15:54 Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 1/9] 9pfs: fix dependencies Cornelia Huck
                   ` (9 more replies)
  0 siblings, 10 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, Cornelia Huck

Hopefully should be close now...

v4->v5:
- dropped "s390x/kvm: msi route fixup for non-pci"
- reworked "s390x/sclp: properly guard pci-specific functions": the
  configure/deconfigure stuff is about I/O adapters, we were missing
  checks, and I used an incorrect return code -- please review
- collected more r-bs

Cornelia Huck (9):
  9pfs: fix dependencies
  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: refine pci dependencies

 accel/kvm/kvm-all.c               |  6 ++--
 default-configs/s390x-softmmu.mak |  2 +-
 fsdev/Makefile.objs               |  9 ++---
 hw/9pfs/Makefile.objs             |  2 +-
 hw/Makefile.objs                  |  2 +-
 hw/pci/pci-stub.c                 | 16 ++++++++-
 hw/pci/pci.c                      |  2 ++
 hw/s390x/Makefile.objs            |  3 +-
 hw/s390x/s390-pci-bus.c           | 18 +++-------
 hw/s390x/s390-pci-bus.h           | 12 ++-----
 hw/s390x/s390-pci-stub.c          | 76 +++++++++++++++++++++++++++++++++++++++
 hw/s390x/s390-virtio-ccw.c        | 14 ++++----
 hw/s390x/sclp.c                   | 39 +++++++++++++++++---
 include/hw/pci/pci.h              |  2 ++
 include/hw/s390x/sclp.h           | 17 ++++++---
 target/s390x/ioinst.c             | 16 +++++++++
 target/s390x/kvm.c                | 58 ++++++++++++++++++++++--------
 17 files changed, 227 insertions(+), 67 deletions(-)
 create mode 100644 hw/s390x/s390-pci-stub.c

-- 
2.13.5

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 1/9] 9pfs: fix dependencies
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci Cornelia Huck
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, Cornelia Huck

Nothing in fsdev/ or hw/9pfs/ depends on pci; it should rather depend
on CONFIG_VIRTFS and CONFIG_VIRTIO/CONFIG_XEN only.

Acked-by: Greg Kurz <groug@kaod.org>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 fsdev/Makefile.objs   | 9 +++------
 hw/9pfs/Makefile.objs | 2 +-
 hw/Makefile.objs      | 2 +-
 3 files changed, 5 insertions(+), 8 deletions(-)

diff --git a/fsdev/Makefile.objs b/fsdev/Makefile.objs
index 659df6e187..fb38017c0b 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 9p backend if we also enabled virtio or xen.
+common-obj-$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO),$(CONFIG_XEN))) = qemu-fsdev.o 9p-marshal.o 9p-iov-marshal.o
+common-obj-$(call lnot,$(call land,$(CONFIG_VIRTFS),$(call lor,$(CONFIG_VIRTIO),$(CONFIG_XEN)))) = 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/9pfs/Makefile.objs b/hw/9pfs/Makefile.objs
index cab5e942ed..fd90b62900 100644
--- a/hw/9pfs/Makefile.objs
+++ b/hw/9pfs/Makefile.objs
@@ -7,4 +7,4 @@ common-obj-$(CONFIG_OPEN_BY_HANDLE) +=  9p-handle.o
 common-obj-y += 9p-proxy.o
 common-obj-$(CONFIG_XEN) += xen-9p-backend.o
 
-obj-y += virtio-9p-device.o
+obj-$(CONFIG_VIRTIO) += virtio-9p-device.o
diff --git a/hw/Makefile.objs b/hw/Makefile.objs
index a2c61f6b09..cf4cb2010b 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),$(CONFIG_XEN))) += 9pfs/
 devices-dirs-$(CONFIG_SOFTMMU) += acpi/
 devices-dirs-$(CONFIG_SOFTMMU) += adc/
 devices-dirs-$(CONFIG_SOFTMMU) += audio/
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 1/9] 9pfs: fix dependencies Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-24  1:13   ` Thomas Huth
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs Cornelia Huck
                   ` (7 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, 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")
Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 accel/kvm/kvm-all.c  |  6 +++---
 hw/pci/pci-stub.c    | 16 +++++++++++++++-
 hw/pci/pci.c         |  2 ++
 include/hw/pci/pci.h |  2 ++
 4 files changed, 22 insertions(+), 4 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..2d3b32c6ab 100644
--- a/hw/pci/pci-stub.c
+++ b/hw/pci/pci-stub.c
@@ -23,10 +23,11 @@
 #include "monitor/monitor.h"
 #include "qapi/qmp/qerror.h"
 #include "hw/pci/pci.h"
-#include "qmp-commands.h"
 #include "hw/pci/msi.h"
+#include "qmp-commands.h"
 
 bool msi_nonbroken;
+bool pci_available;
 
 PciInfoList *qmp_query_pci(Error **errp)
 {
@@ -38,3 +39,16 @@ 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)
+{
+    g_assert(false);
+    return (MSIMessage){};
+}
+
+uint16_t pci_requester_id(PCIDevice *dev)
+{
+    g_assert(false);
+    return 0;
+}
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.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 1/9] 9pfs: fix dependencies Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-24  7:38   ` Christian Borntraeger
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 4/9] s390x: chsc nt2 events are pci-only Cornelia Huck
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, 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.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 4/9] s390x: chsc nt2 events are pci-only
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (2 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 5/9] s390x/pci: do not advertise pci on non-pci builds Cornelia Huck
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, Cornelia Huck

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

Reviewed-by: Thomas Huth <thuth@redhat.com>
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.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 5/9] s390x/pci: do not advertise pci on non-pci builds
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (3 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 4/9] s390x: chsc nt2 events are pci-only Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 6/9] s390x/ccw: create s390 phb conditionally Cornelia Huck
                   ` (4 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, Cornelia Huck

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

Reviewed-by: David Hildenbrand <david@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
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.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 6/9] s390x/ccw: create s390 phb conditionally
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (4 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 5/9] s390x/pci: do not advertise pci on non-pci builds Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions Cornelia Huck
                   ` (3 subsequent siblings)
  9 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, 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 | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/hw/s390x/s390-virtio-ccw.c b/hw/s390x/s390-virtio-ccw.c
index d3d67b8d77..8331c0a275 100644
--- a/hw/s390x/s390-virtio-ccw.c
+++ b/hw/s390x/s390-virtio-ccw.c
@@ -118,12 +118,11 @@ static void ccw_init(MachineState *machine)
 {
     int ret;
     VirtualCssBus *css_bus;
-    DeviceState *dev;
 
     s390_sclp_init();
     s390_memory_init(machine->ram_size);
 
-    /* init CPUs */
+    /* init CPUs (incl. CPU model) early so s390_has_feature() works */
     s390_init_cpus(machine);
 
     s390_flic_init();
@@ -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.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (5 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 6/9] s390x/ccw: create s390 phb conditionally Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-23 16:25   ` Pierre Morel
  2017-08-24  9:29   ` Halil Pasic
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 8/9] s390x/pci: fence off instructions for non-pci Cornelia Huck
                   ` (2 subsequent siblings)
  9 siblings, 2 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, Cornelia Huck

If we do not provide zpci, pci reconfiguration via sclp is not available
either. I/O adapter configuration, however, should always be present.

Rename the values that refer to I/O adapter configuration (instead of only
pci) to make things clearer.

Move length checking of the sccb for I/O adapter configuration into the
common sclp code (out of the pci code). This also fixes an issue that
the pci code would refer to a field in the sccb before checking whether
it was actually long enough.

Check for the adapter type in the sccb and return unrecognized adapter
type if the guest tries to issue I/O adapter configure/deconfigure for
a type other than pci or for pci if the zpci facility is not provided.

Signed-off-by: Cornelia Huck <cohuck@redhat.com>
---
 hw/s390x/s390-pci-bus.c  | 14 ++------------
 hw/s390x/s390-pci-bus.h  |  8 --------
 hw/s390x/s390-pci-stub.c |  2 ++
 hw/s390x/sclp.c          | 39 ++++++++++++++++++++++++++++++++++-----
 include/hw/s390x/sclp.h  | 17 +++++++++++++----
 5 files changed, 51 insertions(+), 29 deletions(-)

diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
index c57f6ebae0..0a31a4ae88 100644
--- a/hw/s390x/s390-pci-bus.c
+++ b/hw/s390x/s390-pci-bus.c
@@ -122,16 +122,11 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid)
 
 void s390_pci_sclp_configure(SCCB *sccb)
 {
-    PciCfgSccb *psccb = (PciCfgSccb *)sccb;
+    IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
     S390PCIBusDevice *pbdev = s390_pci_find_dev_by_fid(s390_get_phb(),
                                                        be32_to_cpu(psccb->aid));
     uint16_t rc;
 
-    if (be16_to_cpu(sccb->h.length) < 16) {
-        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
-        goto out;
-    }
-
     if (!pbdev) {
         DPRINTF("sclp config no dev found\n");
         rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
@@ -155,16 +150,11 @@ out:
 
 void s390_pci_sclp_deconfigure(SCCB *sccb)
 {
-    PciCfgSccb *psccb = (PciCfgSccb *)sccb;
+    IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
     S390PCIBusDevice *pbdev = s390_pci_find_dev_by_fid(s390_get_phb(),
                                                        be32_to_cpu(psccb->aid));
     uint16_t rc;
 
-    if (be16_to_cpu(sccb->h.length) < 16) {
-        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
-        goto out;
-    }
-
     if (!pbdev) {
         DPRINTF("sclp deconfig no dev found\n");
         rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
index 5df6292509..bd636abc28 100644
--- a/hw/s390x/s390-pci-bus.h
+++ b/hw/s390x/s390-pci-bus.h
@@ -244,14 +244,6 @@ typedef struct ChscSeiNt2Res {
     uint8_t ccdf[4016];
 } QEMU_PACKED ChscSeiNt2Res;
 
-typedef struct PciCfgSccb {
-    SCCBHeader header;
-    uint8_t atype;
-    uint8_t reserved1;
-    uint16_t reserved2;
-    uint32_t aid;
-} QEMU_PACKED PciCfgSccb;
-
 typedef struct S390MsixInfo {
     bool available;
     uint8_t table_bar;
diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
index cc7278a865..7a642d376c 100644
--- a/hw/s390x/s390-pci-stub.c
+++ b/hw/s390x/s390-pci-stub.c
@@ -20,10 +20,12 @@ int pci_chsc_sei_nt2_have_event(void)
 /* hw/s390x/sclp.c */
 void s390_pci_sclp_configure(SCCB *sccb)
 {
+    sccb->h.response_code = cpu_to_be16(SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED);
 }
 
 void s390_pci_sclp_deconfigure(SCCB *sccb)
 {
+    sccb->h.response_code = cpu_to_be16(SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED);
 }
 
 /* target/s390x/kvm.c */
diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
index 9253dbbc64..7ae6a0e37a 100644
--- a/hw/s390x/sclp.c
+++ b/hw/s390x/sclp.c
@@ -80,7 +80,7 @@ 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);
+                                        SCLP_HAS_IOA_RECONFIG);
 
     /* Memory Hotplug is only supported for the ccw machine type */
     if (mhd) {
@@ -354,6 +354,35 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
     sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
 }
 
+static void sclp_configure_io_adapter(SCLPDevice *sclp, SCCB *sccb,
+                                      bool configure)
+{
+    int rc;
+
+    if (be16_to_cpu(sccb->h.length) < 16) {
+        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
+        goto out_err;
+    }
+
+    switch (((IoaCfgSccb *)sccb)->atype) {
+    case SCLP_RECONFIG_PCI_ATYPE:
+        if (s390_has_feat(S390_FEAT_ZPCI)) {
+            if (configure) {
+                s390_pci_sclp_configure(sccb);
+            } else {
+                s390_pci_sclp_deconfigure(sccb);
+            }
+            return;
+        }
+        /* fallthrough */
+    default:
+        rc = SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED;
+    }
+
+ out_err:
+    sccb->h.response_code = cpu_to_be16(rc);
+}
+
 static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code)
 {
     SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
@@ -384,11 +413,11 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code)
     case SCLP_UNASSIGN_STORAGE:
         sclp_c->unassign_storage(sclp, sccb);
         break;
-    case SCLP_CMDW_CONFIGURE_PCI:
-        s390_pci_sclp_configure(sccb);
+    case SCLP_CMDW_CONFIGURE_IOA:
+        sclp_configure_io_adapter(sclp, sccb, true);
         break;
-    case SCLP_CMDW_DECONFIGURE_PCI:
-        s390_pci_sclp_deconfigure(sccb);
+    case SCLP_CMDW_DECONFIGURE_IOA:
+        sclp_configure_io_adapter(sclp, sccb, false);
         break;
     default:
         efc->command_handler(ef, sccb, code);
diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
index e71d526605..a72d096081 100644
--- a/include/hw/s390x/sclp.h
+++ b/include/hw/s390x/sclp.h
@@ -44,10 +44,10 @@
 #define SCLP_CMDW_DECONFIGURE_CPU               0x00100001
 
 /* SCLP PCI codes */
-#define SCLP_HAS_PCI_RECONFIG                   0x0000000040000000ULL
-#define SCLP_CMDW_CONFIGURE_PCI                 0x001a0001
-#define SCLP_CMDW_DECONFIGURE_PCI               0x001b0001
-#define SCLP_RECONFIG_PCI_ATPYE                 2
+#define SCLP_HAS_IOA_RECONFIG                   0x0000000040000000ULL
+#define SCLP_CMDW_CONFIGURE_IOA                 0x001a0001
+#define SCLP_CMDW_DECONFIGURE_IOA               0x001b0001
+#define SCLP_RECONFIG_PCI_ATYPE                 2
 
 /* SCLP response codes */
 #define SCLP_RC_NORMAL_READ_COMPLETION          0x0010
@@ -59,6 +59,7 @@
 #define SCLP_RC_INSUFFICIENT_SCCB_LENGTH        0x0300
 #define SCLP_RC_STANDBY_READ_COMPLETION         0x0410
 #define SCLP_RC_ADAPTER_IN_RESERVED_STATE       0x05f0
+#define SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED     0x06f0
 #define SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED       0x09f0
 #define SCLP_RC_INVALID_FUNCTION                0x40f0
 #define SCLP_RC_NO_EVENT_BUFFERS_STORED         0x60f0
@@ -167,6 +168,14 @@ typedef struct AssignStorage {
     uint16_t rn;
 } QEMU_PACKED AssignStorage;
 
+typedef struct IoaCfgSccb {
+    SCCBHeader header;
+    uint8_t atype;
+    uint8_t reserved1;
+    uint16_t reserved2;
+    uint32_t aid;
+} QEMU_PACKED IoaCfgSccb;
+
 typedef struct SCCB {
     SCCBHeader h;
     char data[SCCB_DATA_LEN];
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 8/9] s390x/pci: fence off instructions for non-pci
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (6 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 9/9] s390x: refine pci dependencies Cornelia Huck
  2017-08-24  7:49 ` [Qemu-devel] [PATCH v5 0/9] zpci detangling Christian Borntraeger
  9 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, 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>
Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.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.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [Qemu-devel] [PATCH v5 9/9] s390x: refine pci dependencies
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (7 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 8/9] s390x/pci: fence off instructions for non-pci Cornelia Huck
@ 2017-08-23 15:54 ` Cornelia Huck
  2017-08-24  7:49 ` [Qemu-devel] [PATCH v5 0/9] zpci detangling Christian Borntraeger
  9 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-23 15:54 UTC (permalink / raw)
  To: qemu-devel
  Cc: borntraeger, agraf, thuth, david, pmorel, zyimin, pasic, 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>
Reviewed-by: David Hildenbrand <david@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 51191b77df..6ab2bc65ac 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=$(and $(CONFIG_VHOST_USER),$(CONFIG_LINUX))
 CONFIG_VIRTIO=y
 CONFIG_SCLPCONSOLE=y
-- 
2.13.5

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions Cornelia Huck
@ 2017-08-23 16:25   ` Pierre Morel
  2017-08-24  8:31     ` Cornelia Huck
  2017-08-24  9:29   ` Halil Pasic
  1 sibling, 1 reply; 23+ messages in thread
From: Pierre Morel @ 2017-08-23 16:25 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: borntraeger, agraf, thuth, david, zyimin, pasic

On 23/08/2017 17:54, Cornelia Huck wrote:
> If we do not provide zpci, pci reconfiguration via sclp is not available
> either. I/O adapter configuration, however, should always be present.
> 
> Rename the values that refer to I/O adapter configuration (instead of only
> pci) to make things clearer.
> 
> Move length checking of the sccb for I/O adapter configuration into the
> common sclp code (out of the pci code). This also fixes an issue that
> the pci code would refer to a field in the sccb before checking whether
> it was actually long enough.
> 
> Check for the adapter type in the sccb and return unrecognized adapter
> type if the guest tries to issue I/O adapter configure/deconfigure for
> a type other than pci or for pci if the zpci facility is not provided.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>   hw/s390x/s390-pci-bus.c  | 14 ++------------
>   hw/s390x/s390-pci-bus.h  |  8 --------
>   hw/s390x/s390-pci-stub.c |  2 ++
>   hw/s390x/sclp.c          | 39 ++++++++++++++++++++++++++++++++++-----
>   include/hw/s390x/sclp.h  | 17 +++++++++++++----
>   5 files changed, 51 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index c57f6ebae0..0a31a4ae88 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -122,16 +122,11 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid)
> 
>   void s390_pci_sclp_configure(SCCB *sccb)
>   {
> -    PciCfgSccb *psccb = (PciCfgSccb *)sccb;
> +    IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
>       S390PCIBusDevice *pbdev = s390_pci_find_dev_by_fid(s390_get_phb(),
>                                                          be32_to_cpu(psccb->aid));
>       uint16_t rc;
> 
> -    if (be16_to_cpu(sccb->h.length) < 16) {
> -        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
> -        goto out;
> -    }
> -
>       if (!pbdev) {
>           DPRINTF("sclp config no dev found\n");
>           rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
> @@ -155,16 +150,11 @@ out:
> 
>   void s390_pci_sclp_deconfigure(SCCB *sccb)
>   {
> -    PciCfgSccb *psccb = (PciCfgSccb *)sccb;
> +    IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
>       S390PCIBusDevice *pbdev = s390_pci_find_dev_by_fid(s390_get_phb(),
>                                                          be32_to_cpu(psccb->aid));
>       uint16_t rc;
> 
> -    if (be16_to_cpu(sccb->h.length) < 16) {
> -        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
> -        goto out;
> -    }
> -
>       if (!pbdev) {
>           DPRINTF("sclp deconfig no dev found\n");
>           rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
> index 5df6292509..bd636abc28 100644
> --- a/hw/s390x/s390-pci-bus.h
> +++ b/hw/s390x/s390-pci-bus.h
> @@ -244,14 +244,6 @@ typedef struct ChscSeiNt2Res {
>       uint8_t ccdf[4016];
>   } QEMU_PACKED ChscSeiNt2Res;
> 
> -typedef struct PciCfgSccb {
> -    SCCBHeader header;
> -    uint8_t atype;
> -    uint8_t reserved1;
> -    uint16_t reserved2;
> -    uint32_t aid;
> -} QEMU_PACKED PciCfgSccb;
> -
>   typedef struct S390MsixInfo {
>       bool available;
>       uint8_t table_bar;
> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
> index cc7278a865..7a642d376c 100644
> --- a/hw/s390x/s390-pci-stub.c
> +++ b/hw/s390x/s390-pci-stub.c
> @@ -20,10 +20,12 @@ int pci_chsc_sei_nt2_have_event(void)
>   /* hw/s390x/sclp.c */
>   void s390_pci_sclp_configure(SCCB *sccb)
>   {
> +    sccb->h.response_code = cpu_to_be16(SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED);
>   }
> 
>   void s390_pci_sclp_deconfigure(SCCB *sccb)
>   {
> +    sccb->h.response_code = cpu_to_be16(SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED);
>   }
> 
>   /* target/s390x/kvm.c */
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 9253dbbc64..7ae6a0e37a 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -80,7 +80,7 @@ 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);
> +                                        SCLP_HAS_IOA_RECONFIG);
> 
>       /* Memory Hotplug is only supported for the ccw machine type */
>       if (mhd) {
> @@ -354,6 +354,35 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>       sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>   }
> 
> +static void sclp_configure_io_adapter(SCLPDevice *sclp, SCCB *sccb,
> +                                      bool configure)
> +{
> +    int rc;
> +
> +    if (be16_to_cpu(sccb->h.length) < 16) {
> +        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
> +        goto out_err;
> +    }
> +
> +    switch (((IoaCfgSccb *)sccb)->atype) {
> +    case SCLP_RECONFIG_PCI_ATYPE:
> +        if (s390_has_feat(S390_FEAT_ZPCI)) {
> +            if (configure) {
> +                s390_pci_sclp_configure(sccb);
> +            } else {
> +                s390_pci_sclp_deconfigure(sccb);
> +            }
> +            return;
> +        }
> +        /* fallthrough */
> +    default:
> +        rc = SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED;
> +    }
> +
> + out_err:
> +    sccb->h.response_code = cpu_to_be16(rc);
> +}
> +
>   static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code)
>   {
>       SCLPDeviceClass *sclp_c = SCLP_GET_CLASS(sclp);
> @@ -384,11 +413,11 @@ static void sclp_execute(SCLPDevice *sclp, SCCB *sccb, uint32_t code)
>       case SCLP_UNASSIGN_STORAGE:
>           sclp_c->unassign_storage(sclp, sccb);
>           break;
> -    case SCLP_CMDW_CONFIGURE_PCI:
> -        s390_pci_sclp_configure(sccb);
> +    case SCLP_CMDW_CONFIGURE_IOA:
> +        sclp_configure_io_adapter(sclp, sccb, true);
>           break;
> -    case SCLP_CMDW_DECONFIGURE_PCI:
> -        s390_pci_sclp_deconfigure(sccb);
> +    case SCLP_CMDW_DECONFIGURE_IOA:
> +        sclp_configure_io_adapter(sclp, sccb, false);
>           break;
>       default:
>           efc->command_handler(ef, sccb, code);
> diff --git a/include/hw/s390x/sclp.h b/include/hw/s390x/sclp.h
> index e71d526605..a72d096081 100644
> --- a/include/hw/s390x/sclp.h
> +++ b/include/hw/s390x/sclp.h
> @@ -44,10 +44,10 @@
>   #define SCLP_CMDW_DECONFIGURE_CPU               0x00100001
> 
>   /* SCLP PCI codes */
> -#define SCLP_HAS_PCI_RECONFIG                   0x0000000040000000ULL
> -#define SCLP_CMDW_CONFIGURE_PCI                 0x001a0001
> -#define SCLP_CMDW_DECONFIGURE_PCI               0x001b0001
> -#define SCLP_RECONFIG_PCI_ATPYE                 2
> +#define SCLP_HAS_IOA_RECONFIG                   0x0000000040000000ULL
> +#define SCLP_CMDW_CONFIGURE_IOA                 0x001a0001
> +#define SCLP_CMDW_DECONFIGURE_IOA               0x001b0001
> +#define SCLP_RECONFIG_PCI_ATYPE                 2
> 
>   /* SCLP response codes */
>   #define SCLP_RC_NORMAL_READ_COMPLETION          0x0010
> @@ -59,6 +59,7 @@
>   #define SCLP_RC_INSUFFICIENT_SCCB_LENGTH        0x0300
>   #define SCLP_RC_STANDBY_READ_COMPLETION         0x0410
>   #define SCLP_RC_ADAPTER_IN_RESERVED_STATE       0x05f0
> +#define SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED     0x06f0
>   #define SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED       0x09f0
>   #define SCLP_RC_INVALID_FUNCTION                0x40f0
>   #define SCLP_RC_NO_EVENT_BUFFERS_STORED         0x60f0
> @@ -167,6 +168,14 @@ typedef struct AssignStorage {
>       uint16_t rn;
>   } QEMU_PACKED AssignStorage;
> 
> +typedef struct IoaCfgSccb {
> +    SCCBHeader header;
> +    uint8_t atype;
> +    uint8_t reserved1;
> +    uint16_t reserved2;
> +    uint32_t aid;
> +} QEMU_PACKED IoaCfgSccb;
> +
>   typedef struct SCCB {
>       SCCBHeader h;
>       char data[SCCB_DATA_LEN];
> 

LGTM and
I could also test the all serie on the hardware (sure is sure) :)
with VFIO and virtio PCI... so

Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>


-- 
Pierre Morel
Linux/KVM/QEMU in Böblingen - Germany

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci Cornelia Huck
@ 2017-08-24  1:13   ` Thomas Huth
  2017-08-24  8:35     ` Cornelia Huck
  0 siblings, 1 reply; 23+ messages in thread
From: Thomas Huth @ 2017-08-24  1:13 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel
  Cc: borntraeger, agraf, david, pmorel, zyimin, pasic

On 23.08.2017 17:54, Cornelia Huck wrote:
> 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")
> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  accel/kvm/kvm-all.c  |  6 +++---
>  hw/pci/pci-stub.c    | 16 +++++++++++++++-
>  hw/pci/pci.c         |  2 ++
>  include/hw/pci/pci.h |  2 ++
>  4 files changed, 22 insertions(+), 4 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..2d3b32c6ab 100644
> --- a/hw/pci/pci-stub.c
> +++ b/hw/pci/pci-stub.c
> @@ -23,10 +23,11 @@
>  #include "monitor/monitor.h"
>  #include "qapi/qmp/qerror.h"
>  #include "hw/pci/pci.h"
> -#include "qmp-commands.h"
>  #include "hw/pci/msi.h"
> +#include "qmp-commands.h"

Nit: Unnecessary movement of the #include statement.

If you fix that:

Reviewed-by: Thomas Huth <thuth@redhat.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs Cornelia Huck
@ 2017-08-24  7:38   ` Christian Borntraeger
  2017-08-24  7:43     ` Christian Borntraeger
  0 siblings, 1 reply; 23+ messages in thread
From: Christian Borntraeger @ 2017-08-24  7:38 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: agraf, thuth, david, pmorel, zyimin, pasic



On 08/23/2017 05:54 PM, Cornelia Huck wrote:
> 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)
> +{
> +}

shouldnt these function set an error code in the sccb, e.g.
something like

   sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs
  2017-08-24  7:38   ` Christian Borntraeger
@ 2017-08-24  7:43     ` Christian Borntraeger
  2017-08-24  9:09       ` Cornelia Huck
  0 siblings, 1 reply; 23+ messages in thread
From: Christian Borntraeger @ 2017-08-24  7:43 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: thuth, zyimin, david, pmorel, agraf, pasic



On 08/24/2017 09:38 AM, Christian Borntraeger wrote:
> 
> 
> On 08/23/2017 05:54 PM, Cornelia Huck wrote:
>> 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)
>> +{
>> +}
> 
> shouldnt these function set an error code in the sccb, e.g.
> something like
> 
>    sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> 
> 
> 

Oh you have something like that in patch 7. Maybe move?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/9] zpci detangling
  2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
                   ` (8 preceding siblings ...)
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 9/9] s390x: refine pci dependencies Cornelia Huck
@ 2017-08-24  7:49 ` Christian Borntraeger
  2017-08-24 11:12   ` Cornelia Huck
  9 siblings, 1 reply; 23+ messages in thread
From: Christian Borntraeger @ 2017-08-24  7:49 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel; +Cc: agraf, thuth, david, pmorel, zyimin, pasic

As there are only minor things left, 
whole series

Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>


On 08/23/2017 05:54 PM, Cornelia Huck wrote:
> Hopefully should be close now...
> 
> v4->v5:
> - dropped "s390x/kvm: msi route fixup for non-pci"
> - reworked "s390x/sclp: properly guard pci-specific functions": the
>   configure/deconfigure stuff is about I/O adapters, we were missing
>   checks, and I used an incorrect return code -- please review
> - collected more r-bs
> 
> Cornelia Huck (9):
>   9pfs: fix dependencies
>   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: refine pci dependencies
> 
>  accel/kvm/kvm-all.c               |  6 ++--
>  default-configs/s390x-softmmu.mak |  2 +-
>  fsdev/Makefile.objs               |  9 ++---
>  hw/9pfs/Makefile.objs             |  2 +-
>  hw/Makefile.objs                  |  2 +-
>  hw/pci/pci-stub.c                 | 16 ++++++++-
>  hw/pci/pci.c                      |  2 ++
>  hw/s390x/Makefile.objs            |  3 +-
>  hw/s390x/s390-pci-bus.c           | 18 +++-------
>  hw/s390x/s390-pci-bus.h           | 12 ++-----
>  hw/s390x/s390-pci-stub.c          | 76 +++++++++++++++++++++++++++++++++++++++
>  hw/s390x/s390-virtio-ccw.c        | 14 ++++----
>  hw/s390x/sclp.c                   | 39 +++++++++++++++++---
>  include/hw/pci/pci.h              |  2 ++
>  include/hw/s390x/sclp.h           | 17 ++++++---
>  target/s390x/ioinst.c             | 16 +++++++++
>  target/s390x/kvm.c                | 58 ++++++++++++++++++++++--------
>  17 files changed, 227 insertions(+), 67 deletions(-)
>  create mode 100644 hw/s390x/s390-pci-stub.c
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions
  2017-08-23 16:25   ` Pierre Morel
@ 2017-08-24  8:31     ` Cornelia Huck
  0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-24  8:31 UTC (permalink / raw)
  To: Pierre Morel; +Cc: qemu-devel, borntraeger, agraf, thuth, david, zyimin, pasic

On Wed, 23 Aug 2017 18:25:05 +0200
Pierre Morel <pmorel@linux.vnet.ibm.com> wrote:

> On 23/08/2017 17:54, Cornelia Huck wrote:
> > If we do not provide zpci, pci reconfiguration via sclp is not available
> > either. I/O adapter configuration, however, should always be present.
> > 
> > Rename the values that refer to I/O adapter configuration (instead of only
> > pci) to make things clearer.
> > 
> > Move length checking of the sccb for I/O adapter configuration into the
> > common sclp code (out of the pci code). This also fixes an issue that
> > the pci code would refer to a field in the sccb before checking whether
> > it was actually long enough.
> > 
> > Check for the adapter type in the sccb and return unrecognized adapter
> > type if the guest tries to issue I/O adapter configure/deconfigure for
> > a type other than pci or for pci if the zpci facility is not provided.
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >   hw/s390x/s390-pci-bus.c  | 14 ++------------
> >   hw/s390x/s390-pci-bus.h  |  8 --------
> >   hw/s390x/s390-pci-stub.c |  2 ++
> >   hw/s390x/sclp.c          | 39 ++++++++++++++++++++++++++++++++++-----
> >   include/hw/s390x/sclp.h  | 17 +++++++++++++----
> >   5 files changed, 51 insertions(+), 29 deletions(-)

> LGTM and
> I could also test the all serie on the hardware (sure is sure) :)
> with VFIO and virtio PCI... so
> 
> Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>

Cool, thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci
  2017-08-24  1:13   ` Thomas Huth
@ 2017-08-24  8:35     ` Cornelia Huck
  0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-24  8:35 UTC (permalink / raw)
  To: Thomas Huth; +Cc: qemu-devel, borntraeger, agraf, david, pmorel, zyimin, pasic

On Thu, 24 Aug 2017 03:13:46 +0200
Thomas Huth <thuth@redhat.com> wrote:

> On 23.08.2017 17:54, Cornelia Huck wrote:
> > 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")
> > Reviewed-by: Pierre Morel <pmorel@linux.vnet.ibm.com>
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  accel/kvm/kvm-all.c  |  6 +++---
> >  hw/pci/pci-stub.c    | 16 +++++++++++++++-
> >  hw/pci/pci.c         |  2 ++
> >  include/hw/pci/pci.h |  2 ++
> >  4 files changed, 22 insertions(+), 4 deletions(-)

> > diff --git a/hw/pci/pci-stub.c b/hw/pci/pci-stub.c
> > index ecad664946..2d3b32c6ab 100644
> > --- a/hw/pci/pci-stub.c
> > +++ b/hw/pci/pci-stub.c
> > @@ -23,10 +23,11 @@
> >  #include "monitor/monitor.h"
> >  #include "qapi/qmp/qerror.h"
> >  #include "hw/pci/pci.h"
> > -#include "qmp-commands.h"
> >  #include "hw/pci/msi.h"
> > +#include "qmp-commands.h"  
> 
> Nit: Unnecessary movement of the #include statement.

Seems to have slipped in during the various rebases. Fixed.

> 
> If you fix that:
> 
> Reviewed-by: Thomas Huth <thuth@redhat.com>

Thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs
  2017-08-24  7:43     ` Christian Borntraeger
@ 2017-08-24  9:09       ` Cornelia Huck
  2017-08-24  9:50         ` Halil Pasic
  0 siblings, 1 reply; 23+ messages in thread
From: Cornelia Huck @ 2017-08-24  9:09 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: qemu-devel, thuth, zyimin, david, pmorel, agraf, pasic

On Thu, 24 Aug 2017 09:43:48 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 08/24/2017 09:38 AM, Christian Borntraeger wrote:
> > 
> > 
> > On 08/23/2017 05:54 PM, Cornelia Huck wrote:  
> >> 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

> >> +/* hw/s390x/sclp.c */
> >> +void s390_pci_sclp_configure(SCCB *sccb)
> >> +{
> >> +}
> >> +
> >> +void s390_pci_sclp_deconfigure(SCCB *sccb)
> >> +{
> >> +}  
> > 
> > shouldnt these function set an error code in the sccb, e.g.
> > something like
> > 
> >    sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> > 
> > 
> >   
> 
> Oh you have something like that in patch 7. Maybe move?

Does not really change anything in practice, but I can move it.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions
  2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions Cornelia Huck
  2017-08-23 16:25   ` Pierre Morel
@ 2017-08-24  9:29   ` Halil Pasic
  2017-08-24  9:33     ` Cornelia Huck
  1 sibling, 1 reply; 23+ messages in thread
From: Halil Pasic @ 2017-08-24  9:29 UTC (permalink / raw)
  To: Cornelia Huck, qemu-devel
  Cc: thuth, zyimin, david, pmorel, agraf, borntraeger



On 08/23/2017 05:54 PM, Cornelia Huck wrote:
> If we do not provide zpci, pci reconfiguration via sclp is not available
> either. I/O adapter configuration, however, should always be present.
> 
> Rename the values that refer to I/O adapter configuration (instead of only
> pci) to make things clearer.
> 
> Move length checking of the sccb for I/O adapter configuration into the
> common sclp code (out of the pci code). This also fixes an issue that
> the pci code would refer to a field in the sccb before checking whether
> it was actually long enough.
> 
> Check for the adapter type in the sccb and return unrecognized adapter
> type if the guest tries to issue I/O adapter configure/deconfigure for
> a type other than pci or for pci if the zpci facility is not provided.
> 
> Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> ---
>  hw/s390x/s390-pci-bus.c  | 14 ++------------
>  hw/s390x/s390-pci-bus.h  |  8 --------
>  hw/s390x/s390-pci-stub.c |  2 ++
>  hw/s390x/sclp.c          | 39 ++++++++++++++++++++++++++++++++++-----
>  include/hw/s390x/sclp.h  | 17 +++++++++++++----
>  5 files changed, 51 insertions(+), 29 deletions(-)
> 
> diff --git a/hw/s390x/s390-pci-bus.c b/hw/s390x/s390-pci-bus.c
> index c57f6ebae0..0a31a4ae88 100644
> --- a/hw/s390x/s390-pci-bus.c
> +++ b/hw/s390x/s390-pci-bus.c
> @@ -122,16 +122,11 @@ S390PCIBusDevice *s390_pci_find_dev_by_fid(S390pciState *s, uint32_t fid)
> 
>  void s390_pci_sclp_configure(SCCB *sccb)
>  {
> -    PciCfgSccb *psccb = (PciCfgSccb *)sccb;
> +    IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
>      S390PCIBusDevice *pbdev = s390_pci_find_dev_by_fid(s390_get_phb(),
>                                                         be32_to_cpu(psccb->aid));
>      uint16_t rc;
> 
> -    if (be16_to_cpu(sccb->h.length) < 16) {
> -        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
> -        goto out;
> -    }
> -
>      if (!pbdev) {
>          DPRINTF("sclp config no dev found\n");
>          rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
> @@ -155,16 +150,11 @@ out:
> 
>  void s390_pci_sclp_deconfigure(SCCB *sccb)
>  {
> -    PciCfgSccb *psccb = (PciCfgSccb *)sccb;
> +    IoaCfgSccb *psccb = (IoaCfgSccb *)sccb;
>      S390PCIBusDevice *pbdev = s390_pci_find_dev_by_fid(s390_get_phb(),
>                                                         be32_to_cpu(psccb->aid));
>      uint16_t rc;
> 
> -    if (be16_to_cpu(sccb->h.length) < 16) {
> -        rc = SCLP_RC_INSUFFICIENT_SCCB_LENGTH;
> -        goto out;
> -    }
> -
>      if (!pbdev) {
>          DPRINTF("sclp deconfig no dev found\n");
>          rc = SCLP_RC_ADAPTER_ID_NOT_RECOGNIZED;
> diff --git a/hw/s390x/s390-pci-bus.h b/hw/s390x/s390-pci-bus.h
> index 5df6292509..bd636abc28 100644
> --- a/hw/s390x/s390-pci-bus.h
> +++ b/hw/s390x/s390-pci-bus.h
> @@ -244,14 +244,6 @@ typedef struct ChscSeiNt2Res {
>      uint8_t ccdf[4016];
>  } QEMU_PACKED ChscSeiNt2Res;
> 
> -typedef struct PciCfgSccb {
> -    SCCBHeader header;
> -    uint8_t atype;
> -    uint8_t reserved1;
> -    uint16_t reserved2;
> -    uint32_t aid;
> -} QEMU_PACKED PciCfgSccb;
> -
>  typedef struct S390MsixInfo {
>      bool available;
>      uint8_t table_bar;
> diff --git a/hw/s390x/s390-pci-stub.c b/hw/s390x/s390-pci-stub.c
> index cc7278a865..7a642d376c 100644
> --- a/hw/s390x/s390-pci-stub.c
> +++ b/hw/s390x/s390-pci-stub.c
> @@ -20,10 +20,12 @@ int pci_chsc_sei_nt2_have_event(void)
>  /* hw/s390x/sclp.c */
>  void s390_pci_sclp_configure(SCCB *sccb)
>  {
> +    sccb->h.response_code = cpu_to_be16(SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED);
>  }
> 
>  void s390_pci_sclp_deconfigure(SCCB *sccb)
>  {
> +    sccb->h.response_code = cpu_to_be16(SCLP_RC_ADAPTER_TYPE_NOT_RECOGNIZED);
>  }
> 
>  /* target/s390x/kvm.c */
> diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> index 9253dbbc64..7ae6a0e37a 100644
> --- a/hw/s390x/sclp.c
> +++ b/hw/s390x/sclp.c
> @@ -80,7 +80,7 @@ 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);
> +                                        SCLP_HAS_IOA_RECONFIG);
> 
>      /* Memory Hotplug is only supported for the ccw machine type */
>      if (mhd) {
> @@ -354,6 +354,35 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
>      sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
>  }
> 
> +static void sclp_configure_io_adapter(SCLPDevice *sclp, SCCB *sccb,
> +                                      bool configure)

Just an idea. This could be called reconfigure instead
of configure (that's sclp_reconfigure_io_adapter) as the
facility providing both conf and deconf is called reconf.

It ain't important. Patch looks good!

Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions
  2017-08-24  9:29   ` Halil Pasic
@ 2017-08-24  9:33     ` Cornelia Huck
  0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-24  9:33 UTC (permalink / raw)
  To: Halil Pasic; +Cc: qemu-devel, thuth, zyimin, david, pmorel, agraf, borntraeger

On Thu, 24 Aug 2017 11:29:47 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> On 08/23/2017 05:54 PM, Cornelia Huck wrote:
> > If we do not provide zpci, pci reconfiguration via sclp is not available
> > either. I/O adapter configuration, however, should always be present.
> > 
> > Rename the values that refer to I/O adapter configuration (instead of only
> > pci) to make things clearer.
> > 
> > Move length checking of the sccb for I/O adapter configuration into the
> > common sclp code (out of the pci code). This also fixes an issue that
> > the pci code would refer to a field in the sccb before checking whether
> > it was actually long enough.
> > 
> > Check for the adapter type in the sccb and return unrecognized adapter
> > type if the guest tries to issue I/O adapter configure/deconfigure for
> > a type other than pci or for pci if the zpci facility is not provided.
> > 
> > Signed-off-by: Cornelia Huck <cohuck@redhat.com>
> > ---
> >  hw/s390x/s390-pci-bus.c  | 14 ++------------
> >  hw/s390x/s390-pci-bus.h  |  8 --------
> >  hw/s390x/s390-pci-stub.c |  2 ++
> >  hw/s390x/sclp.c          | 39 ++++++++++++++++++++++++++++++++++-----
> >  include/hw/s390x/sclp.h  | 17 +++++++++++++----
> >  5 files changed, 51 insertions(+), 29 deletions(-)

> > diff --git a/hw/s390x/sclp.c b/hw/s390x/sclp.c
> > index 9253dbbc64..7ae6a0e37a 100644
> > --- a/hw/s390x/sclp.c
> > +++ b/hw/s390x/sclp.c
> > @@ -80,7 +80,7 @@ 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);
> > +                                        SCLP_HAS_IOA_RECONFIG);
> > 
> >      /* Memory Hotplug is only supported for the ccw machine type */
> >      if (mhd) {
> > @@ -354,6 +354,35 @@ static void sclp_read_cpu_info(SCLPDevice *sclp, SCCB *sccb)
> >      sccb->h.response_code = cpu_to_be16(SCLP_RC_NORMAL_READ_COMPLETION);
> >  }
> > 
> > +static void sclp_configure_io_adapter(SCLPDevice *sclp, SCCB *sccb,
> > +                                      bool configure)  
> 
> Just an idea. This could be called reconfigure instead
> of configure (that's sclp_reconfigure_io_adapter) as the
> facility providing both conf and deconf is called reconf.
> 
> It ain't important. Patch looks good!

So I'll just keep it as-is :)

> 
> Reviewed-by: Halil Pasic <pasic@linux.vnet.ibm.com>

Thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs
  2017-08-24  9:09       ` Cornelia Huck
@ 2017-08-24  9:50         ` Halil Pasic
  2017-08-24  9:58           ` Cornelia Huck
  0 siblings, 1 reply; 23+ messages in thread
From: Halil Pasic @ 2017-08-24  9:50 UTC (permalink / raw)
  To: Cornelia Huck, Christian Borntraeger
  Cc: thuth, zyimin, david, pmorel, agraf, qemu-devel



On 08/24/2017 11:09 AM, Cornelia Huck wrote:
> On Thu, 24 Aug 2017 09:43:48 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 08/24/2017 09:38 AM, Christian Borntraeger wrote:
>>>
>>>
>>> On 08/23/2017 05:54 PM, Cornelia Huck wrote:  
>>>> 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
> 
>>>> +/* hw/s390x/sclp.c */
>>>> +void s390_pci_sclp_configure(SCCB *sccb)
>>>> +{
>>>> +}
>>>> +
>>>> +void s390_pci_sclp_deconfigure(SCCB *sccb)
>>>> +{
>>>> +}  
>>>
>>> shouldnt these function set an error code in the sccb, e.g.
>>> something like
>>>
>>>    sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
>>>
>>>
>>>   
>>
>> Oh you have something like that in patch 7. Maybe move?
> 
> Does not really change anything in practice, but I can move it.
> 

You mean these stubs are not supposed to be reachable and are just
for making the linker happy, or? If that's the case I would prefer
having that expressed by something like assert(false) or even 
#define NOT_REACHABLE assert(false).

Otherwise patch looks good, but I did not a full review on it,
so let's try this:
Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs
  2017-08-24  9:50         ` Halil Pasic
@ 2017-08-24  9:58           ` Cornelia Huck
  0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-24  9:58 UTC (permalink / raw)
  To: Halil Pasic
  Cc: Christian Borntraeger, thuth, zyimin, david, pmorel, agraf, qemu-devel

On Thu, 24 Aug 2017 11:50:54 +0200
Halil Pasic <pasic@linux.vnet.ibm.com> wrote:

> On 08/24/2017 11:09 AM, Cornelia Huck wrote:
> > On Thu, 24 Aug 2017 09:43:48 +0200
> > Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> >   
> >> On 08/24/2017 09:38 AM, Christian Borntraeger wrote:  
> >>>
> >>>
> >>> On 08/23/2017 05:54 PM, Cornelia Huck wrote:    
> >>>> 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  
> >   
> >>>> +/* hw/s390x/sclp.c */
> >>>> +void s390_pci_sclp_configure(SCCB *sccb)
> >>>> +{
> >>>> +}
> >>>> +
> >>>> +void s390_pci_sclp_deconfigure(SCCB *sccb)
> >>>> +{
> >>>> +}    
> >>>
> >>> shouldnt these function set an error code in the sccb, e.g.
> >>> something like
> >>>
> >>>    sccb->h.response_code = cpu_to_be16(SCLP_RC_INVALID_SCLP_COMMAND);
> >>>
> >>>
> >>>     
> >>
> >> Oh you have something like that in patch 7. Maybe move?  
> > 
> > Does not really change anything in practice, but I can move it.
> >   
> 
> You mean these stubs are not supposed to be reachable and are just
> for making the linker happy, or? If that's the case I would prefer
> having that expressed by something like assert(false) or even 
> #define NOT_REACHABLE assert(false).

Without the last patch that makes virtio-pci depend on pci for s390x,
you can't compile without pci anyway ;)

> 
> Otherwise patch looks good, but I did not a full review on it,
> so let's try this:
> Acked-by: Halil Pasic <pasic@linux.vnet.ibm.com>
> 

Thanks!

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [Qemu-devel] [PATCH v5 0/9] zpci detangling
  2017-08-24  7:49 ` [Qemu-devel] [PATCH v5 0/9] zpci detangling Christian Borntraeger
@ 2017-08-24 11:12   ` Cornelia Huck
  0 siblings, 0 replies; 23+ messages in thread
From: Cornelia Huck @ 2017-08-24 11:12 UTC (permalink / raw)
  To: Christian Borntraeger
  Cc: qemu-devel, agraf, thuth, david, pmorel, zyimin, pasic

On Thu, 24 Aug 2017 09:49:18 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> As there are only minor things left, 
> whole series
> 
> Acked-by: Christian Borntraeger <borntraeger@de.ibm.com>

Thanks.

I've applied this to s390-next.

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2017-08-24 11:12 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-23 15:54 [Qemu-devel] [PATCH v5 0/9] zpci detangling Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 1/9] 9pfs: fix dependencies Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 2/9] kvm: remove hard dependency on pci Cornelia Huck
2017-08-24  1:13   ` Thomas Huth
2017-08-24  8:35     ` Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 3/9] s390x/pci: add stubs Cornelia Huck
2017-08-24  7:38   ` Christian Borntraeger
2017-08-24  7:43     ` Christian Borntraeger
2017-08-24  9:09       ` Cornelia Huck
2017-08-24  9:50         ` Halil Pasic
2017-08-24  9:58           ` Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 4/9] s390x: chsc nt2 events are pci-only Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 5/9] s390x/pci: do not advertise pci on non-pci builds Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 6/9] s390x/ccw: create s390 phb conditionally Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 7/9] s390x/sclp: properly guard pci-specific functions Cornelia Huck
2017-08-23 16:25   ` Pierre Morel
2017-08-24  8:31     ` Cornelia Huck
2017-08-24  9:29   ` Halil Pasic
2017-08-24  9:33     ` Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 8/9] s390x/pci: fence off instructions for non-pci Cornelia Huck
2017-08-23 15:54 ` [Qemu-devel] [PATCH v5 9/9] s390x: refine pci dependencies Cornelia Huck
2017-08-24  7:49 ` [Qemu-devel] [PATCH v5 0/9] zpci detangling Christian Borntraeger
2017-08-24 11:12   ` Cornelia Huck

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.