[libvirt] [PATCH 00/13] PCI passthrough support on s390

The PCI representation in QEMU has recently been extended for S390 allowing configuration of zPCI attributes like uid (user-defined identifier) and fid (PCI function identifier). The details can be found here: https://lists.gnu.org/archive/html/qemu-devel/2016-06/msg07262.html To support the new zPCI feature of the S390 platform, two new XML attributes, @uid and @fid, are introduced for device addresses of type 'pci', i.e.: <hostdev mode='subsystem' type='pci'> <driver name='vfio'/> <source> <address domain='0x0001' bus='0x00' slot='0x00' function='0x0'/> </source> <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' uid='0x0003' fid='0x00000027'/> </hostdev> uid and fid are optional attributes. If they are defined by the user, unique values within the guest domain must be used. If they are not specified and the architecture requires them, they are automatically generated with non-conflicting values. Current implementation is the most seamless one for the user as it unites the address specific data of a PCI device on one XML element. It could accommodate both specifying our special parameters (uid and fid) and re-using standard statements (domain, bus, slot and function) for PCI devices. User can still specify bus/slot/function for the virtualized PCI devices in the XML. Thus uid/fid act as an extension to the PCI address and are stored in a new structure 'virZPCIDeviceAddress' which is a member of common PCI Address structure. Additionally, two hashtables are used for assignment and reservation of uid/fid. In support of extending the PCI address, a new PCI address extension flag is introduced. This extension flag allows is not only dedicated for the S390 platform but also other architectures needing certain extensions to PCI address space. Yi Min Zhao (13): conf: Add definitions for 'uid' and 'fid' PCI address attributes qemu: Introduce zPCI capability conf: Introduce a new PCI address extension flag qemu: Enable PCI multi bus for S390 guests qemu: Auto add pci-root for s390/s390x guests qemu: Generate and use zPCI device in QEMU command line qemu: Add hotpluging support for PCI devices on S390 guests conf: Introduce parser, formatter for uid and fid conf: Introduce address caching for PCI extensions conf: Allocate/release 'uid' and 'fid' in PCI address tests: Add new tests for zPCI docs: Add 'uid' and 'fid' information news: Update news for PCI address extension attributes docs/formatdomain.html.in | 9 +- docs/news.xml | 11 + docs/schemas/basictypes.rng | 28 ++ docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.c | 74 +++++ src/conf/device_conf.h | 1 + src/conf/domain_addr.c | 346 +++++++++++++++++++++ src/conf/domain_addr.h | 29 ++ src/conf/domain_conf.c | 4 + src/libvirt_private.syms | 4 + src/qemu/qemu_capabilities.c | 5 + src/qemu/qemu_capabilities.h | 1 + src/qemu/qemu_command.c | 104 +++++++ src/qemu/qemu_command.h | 4 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_domain_address.c | 183 ++++++++++- src/qemu/qemu_hotplug.c | 175 ++++++++++- src/util/virpci.h | 13 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 27 ++ tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml | 17 + .../hostdev-vfio-zpci-autogenerate.args | 24 ++ .../hostdev-vfio-zpci-autogenerate.xml | 18 ++ .../hostdev-vfio-zpci-boundaries.args | 27 ++ .../hostdev-vfio-zpci-boundaries.xml | 26 ++ .../hostdev-vfio-zpci-multidomain-many.args | 38 +++ .../hostdev-vfio-zpci-multidomain-many.xml | 67 ++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 24 ++ tests/qemuxml2argvdata/hostdev-vfio-zpci.xml | 19 ++ tests/qemuxml2argvtest.c | 21 ++ tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml | 29 ++ tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml | 30 ++ tests/qemuxml2xmltest.c | 3 + 38 files changed, 1347 insertions(+), 22 deletions(-) create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml -- 2.16.3

From: Yi Min Zhao <zyimin@linux.ibm.com> Add zPCI definitions in preparation of extending the PCI address with parameters uid (user-defined identifier) and fid (PCI function identifier). Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/util/virpci.h | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/util/virpci.h b/src/util/virpci.h index 794b7e59db..250e9278ba 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -36,12 +36,22 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr; +typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; +struct _virZPCIDeviceAddress { + unsigned int zpcifid; + unsigned int zpciuid; + bool fid_assigned; + bool uid_assigned; +}; + struct _virPCIDeviceAddress { unsigned int domain; unsigned int bus; unsigned int slot; unsigned int function; int multi; /* virTristateSwitch */ + virZPCIDeviceAddressPtr zpci; }; typedef enum { -- 2.16.3

On Thu, May 24, 2018 at 02:24:26PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Add zPCI definitions in preparation of extending the PCI address with parameters uid (user-defined identifier) and fid (PCI function identifier).
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/util/virpci.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/util/virpci.h b/src/util/virpci.h index 794b7e59db..250e9278ba 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -36,12 +36,22 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr;
+typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; +struct _virZPCIDeviceAddress { + unsigned int zpcifid; + unsigned int zpciuid;
zpci_fid and zpci_uid, please
+ bool fid_assigned; + bool uid_assigned; +}; +
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano

在 2018/6/2 下午10:12, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:26PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Add zPCI definitions in preparation of extending the PCI address with parameters uid (user-defined identifier) and fid (PCI function identifier).
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/util/virpci.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/util/virpci.h b/src/util/virpci.h index 794b7e59db..250e9278ba 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -36,12 +36,22 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr;
+typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; +struct _virZPCIDeviceAddress { + unsigned int zpcifid; + unsigned int zpciuid;
zpci_fid and zpci_uid, please OK. Thanks!
+ bool fid_assigned; + bool uid_assigned; +}; +
Reviewed-by: Ján Tomko <jtomko@redhat.com>
Jano

On 05/24/2018 08:24 AM, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Add zPCI definitions in preparation of extending the PCI address with parameters uid (user-defined identifier) and fid (PCI function identifier).
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/util/virpci.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/util/virpci.h b/src/util/virpci.h index 794b7e59db..250e9278ba 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -36,12 +36,22 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr;
+typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; +struct _virZPCIDeviceAddress { + unsigned int zpcifid; + unsigned int zpciuid; + bool fid_assigned; + bool uid_assigned; +}; + struct _virPCIDeviceAddress { unsigned int domain; unsigned int bus; unsigned int slot; unsigned int function; int multi; /* virTristateSwitch */ + virZPCIDeviceAddressPtr zpci;
Why does this need to be a pointer to a "virZPCIDeviceAddress"? The struct itself is a fixed size, and it already has bools to indicate whether or not each of the data attributes has been set. That's just extra memory management churn for no useful purpose.
};
typedef enum {

Laine Stump <laine@laine.org> [2018-06-07, 09:43PM -0400]:
On 05/24/2018 08:24 AM, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Add zPCI definitions in preparation of extending the PCI address with parameters uid (user-defined identifier) and fid (PCI function identifier).
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/util/virpci.h | 10 ++++++++++ 1 file changed, 10 insertions(+)
diff --git a/src/util/virpci.h b/src/util/virpci.h index 794b7e59db..250e9278ba 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -36,12 +36,22 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr;
+typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; +typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; +struct _virZPCIDeviceAddress { + unsigned int zpcifid; + unsigned int zpciuid; + bool fid_assigned; + bool uid_assigned; +}; + struct _virPCIDeviceAddress { unsigned int domain; unsigned int bus; unsigned int slot; unsigned int function; int multi; /* virTristateSwitch */ + virZPCIDeviceAddressPtr zpci;
Why does this need to be a pointer to a "virZPCIDeviceAddress"? The struct itself is a fixed size, and it already has bools to indicate whether or not each of the data attributes has been set. That's just extra memory management churn for no useful purpose.
Yeah, we do encode a lot of state with this member. IIRC, if the zpci pointer is allocated, we generally do have zpci support available. The booleans refer to if an actual zpci address has been generated by the PCI address generation code. This only happens when the pointer is allocated. Another way would be to check the zpci capability.
};
typedef enum {
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- IBM Systems Linux on Z & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 ------------------------------------------------------------------------ Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Yi Min Zhao <zyimin@linux.ibm.com> Let's introduce zPCI capability. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + 8 files changed, 9 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8a63db5f4f..e6fd7ee468 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -489,6 +489,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "screendump_device", "hda-output", "blockdev-del", + "zpci", ); @@ -1121,6 +1122,7 @@ struct virQEMUCapsStringFlags virQEMUCapsObjectTypes[] = { { "virtual-css-bridge", QEMU_CAPS_CCW }, { "vfio-ccw", QEMU_CAPS_DEVICE_VFIO_CCW }, { "hda-output", QEMU_CAPS_HDA_OUTPUT }, + { "zpci", QEMU_CAPS_DEVICE_ZPCI }, }; static struct virQEMUCapsStringFlags virQEMUCapsDevicePropsVirtioBalloon[] = { diff --git a/src/qemu/qemu_capabilities.h b/src/qemu/qemu_capabilities.h index 3e120e64c0..2610597929 100644 --- a/src/qemu/qemu_capabilities.h +++ b/src/qemu/qemu_capabilities.h @@ -473,6 +473,7 @@ typedef enum { /* virQEMUCapsFlags grouping marker for syntax-check */ QEMU_CAPS_SCREENDUMP_DEVICE, /* screendump command accepts device & head */ QEMU_CAPS_HDA_OUTPUT, /* -device hda-output */ QEMU_CAPS_BLOCKDEV_DEL, /* blockdev-del is supported */ + QEMU_CAPS_DEVICE_ZPCI, /* -device zpci */ QEMU_CAPS_LAST /* this must always be the last item */ } virQEMUCapsFlags; diff --git a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml index 68006c6fa0..89674f2625 100644 --- a/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml @@ -118,6 +118,7 @@ <flag name='virtual-css-bridge'/> <flag name='sdl-gl'/> <flag name='blockdev-del'/> + <flag name='zpci'/> <version>2010000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>303434</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml index cb11562e39..a1c5aae165 100644 --- a/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml @@ -124,6 +124,7 @@ <flag name='virtual-css-bridge'/> <flag name='sdl-gl'/> <flag name='blockdev-del'/> + <flag name='zpci'/> <version>2011000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>342166</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml index df0f6e4eba..48df8d9e11 100644 --- a/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml @@ -130,6 +130,7 @@ <flag name='sdl-gl'/> <flag name='screendump_device'/> <flag name='blockdev-del'/> + <flag name='zpci'/> <version>2012000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>371055</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml index 49b3cb2dfa..e8b3b034aa 100644 --- a/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml @@ -107,6 +107,7 @@ <flag name='nbd-tls'/> <flag name='virtual-css-bridge'/> <flag name='sdl-gl'/> + <flag name='zpci'/> <version>2007000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>216840</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml index 985a4114c5..01cc0472f0 100644 --- a/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml @@ -109,6 +109,7 @@ <flag name='nbd-tls'/> <flag name='virtual-css-bridge'/> <flag name='sdl-gl'/> + <flag name='zpci'/> <version>2007093</version> <kvmVersion>0</kvmVersion> <microcodeVersion>241741</microcodeVersion> diff --git a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml index b216c69906..cd72530a6c 100644 --- a/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml +++ b/tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml @@ -113,6 +113,7 @@ <flag name='virtual-css-bridge'/> <flag name='sdl-gl'/> <flag name='blockdev-del'/> + <flag name='zpci'/> <version>2009000</version> <kvmVersion>0</kvmVersion> <microcodeVersion>265159</microcodeVersion> -- 2.16.3

On Thursday, 24 May 2018 14:24:27 CEST Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Let's introduce zPCI capability.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + 8 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8a63db5f4f..e6fd7ee468 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -489,6 +489,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "screendump_device", "hda-output", "blockdev-del", + "zpci",
Wrong indentation here. -- Pino Toscano

On 5/25/2018 6:58 PM, Pino Toscano wrote:
On Thursday, 24 May 2018 14:24:27 CEST Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Let's introduce zPCI capability.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 2 ++ src/qemu/qemu_capabilities.h | 1 + tests/qemucapabilitiesdata/caps_2.10.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.11.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.12.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.7.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.8.0.s390x.xml | 1 + tests/qemucapabilitiesdata/caps_2.9.0.s390x.xml | 1 + 8 files changed, 9 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 8a63db5f4f..e6fd7ee468 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -489,6 +489,7 @@ VIR_ENUM_IMPL(virQEMUCaps, QEMU_CAPS_LAST, "screendump_device", "hda-output", "blockdev-del", + "zpci", Wrong indentation here.
Ok, will revise.

From: Yi Min Zhao <zyimin@linux.ibm.com> This patch introduces a new attribute PCI address extension flag to deal with the extension PCI attributes such as 'uid' and 'fid' on the S390 platform. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/device_conf.h | 1 + src/conf/domain_addr.h | 5 ++ src/qemu/qemu_domain_address.c | 137 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 2 deletions(-) diff --git a/src/conf/device_conf.h b/src/conf/device_conf.h index a31ce9c376..6f926dff1d 100644 --- a/src/conf/device_conf.h +++ b/src/conf/device_conf.h @@ -164,6 +164,7 @@ struct _virDomainDeviceInfo { * assignment, never saved and never reported. */ int pciConnectFlags; /* enum virDomainPCIConnectFlags */ + int pciAddressExtFlags; /* enum virDomainPCIAddressExtensionFlags */ char *loadparm; /* PCI devices will only be automatically placed on a PCI bus diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 3236b7d6de..1d1837bd23 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -29,6 +29,11 @@ # define VIR_PCI_ADDRESS_SLOT_LAST 31 # define VIR_PCI_ADDRESS_FUNCTION_LAST 7 +typedef enum { + VIR_PCI_ADDRESS_EXTENSION_NONE = 0, /* no extension */ + VIR_PCI_ADDRESS_EXTENSION_ZPCI = 1 << 0, /* zpci support */ +} virDomainPCIAddressExtensionFlags; + typedef enum { VIR_PCI_CONNECT_HOTPLUGGABLE = 1 << 0, /* is hotplug needed/supported */ diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b7c82cb6f1..adce399be6 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -502,6 +502,60 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, } +static bool +qemuDomainDeviceSupportZPCI(virQEMUCapsPtr qemuCaps, + virDomainDeviceDefPtr device) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) + return false; + + switch ((virDomainDeviceType) device->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_CHR: + return false; + + case VIR_DOMAIN_DEVICE_NONE: + case VIR_DOMAIN_DEVICE_DISK: + case VIR_DOMAIN_DEVICE_LEASE: + case VIR_DOMAIN_DEVICE_FS: + case VIR_DOMAIN_DEVICE_NET: + case VIR_DOMAIN_DEVICE_INPUT: + case VIR_DOMAIN_DEVICE_SOUND: + case VIR_DOMAIN_DEVICE_VIDEO: + case VIR_DOMAIN_DEVICE_HOSTDEV: + case VIR_DOMAIN_DEVICE_WATCHDOG: + case VIR_DOMAIN_DEVICE_GRAPHICS: + case VIR_DOMAIN_DEVICE_HUB: + case VIR_DOMAIN_DEVICE_REDIRDEV: + case VIR_DOMAIN_DEVICE_SMARTCARD: + case VIR_DOMAIN_DEVICE_MEMBALLOON: + case VIR_DOMAIN_DEVICE_NVRAM: + case VIR_DOMAIN_DEVICE_RNG: + case VIR_DOMAIN_DEVICE_SHMEM: + case VIR_DOMAIN_DEVICE_TPM: + case VIR_DOMAIN_DEVICE_PANIC: + case VIR_DOMAIN_DEVICE_MEMORY: + case VIR_DOMAIN_DEVICE_IOMMU: + case VIR_DOMAIN_DEVICE_LAST: + break; + } + return true; +} + + +static virDomainPCIAddressExtensionFlags +qemuDomainDeviceCalculatePCIAddressExtensionFlags(virQEMUCapsPtr qemuCaps, + virDomainDeviceDefPtr dev) +{ + virDomainPCIAddressExtensionFlags extFlags = 0; + + if (qemuDomainDeviceSupportZPCI(qemuCaps, dev)) + extFlags |= VIR_PCI_ADDRESS_EXTENSION_ZPCI; + + return extFlags; +} + + /** * qemuDomainDeviceCalculatePCIConnectFlags: * @@ -980,6 +1034,55 @@ qemuDomainFillAllPCIConnectFlags(virDomainDefPtr def, } +/** + * qemuDomainFillDevicePCIExtensionFlagsIter: + * + * @def: the entire DomainDef + * @dev: The device to be checked + * @info: virDomainDeviceInfo within the device + * @opaque: qemu capabilities + * + * Sets the pciAddressExtFlags for a single device's info. Has properly + * formatted arguments to be called by virDomainDeviceInfoIterate(). + * + * Always returns 0 - there is no failure. + */ +static int +qemuDomainFillDevicePCIExtensionFlagsIter(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr dev, + virDomainDeviceInfoPtr info, + void *opaque) +{ + virQEMUCapsPtr qemuCaps = opaque; + + info->pciAddressExtFlags + = qemuDomainDeviceCalculatePCIAddressExtensionFlags(qemuCaps, dev); + return 0; +} + + +/** + * qemuDomainFillAllPCIExtensionFlags: + * + * @def: the entire DomainDef + * @qemuCaps: as you'd expect + * + * Set the info->pciAddressExtFlags for all devices in the domain. + * + * Returns 0 on success or -1 on failure (the only possibility of + * failure would be some internal problem with + * virDomainDeviceInfoIterate()) + */ +static int +qemuDomainFillAllPCIExtensionFlags(virDomainDefPtr def, + virQEMUCapsPtr qemuCaps) +{ + return virDomainDeviceInfoIterate(def, + qemuDomainFillDevicePCIExtensionFlagsIter, + qemuCaps); +} + + /** * qemuDomainFindUnusedIsolationGroupIter: * @def: domain definition @@ -1254,6 +1357,29 @@ qemuDomainFillDevicePCIConnectFlags(virDomainDefPtr def, } +/** + * qemuDomainFillDevicePCIExtensionFlags: + * + * @dev: The device to be checked + * @qemuCaps: as you'd expect + * + * Set the info->pciAddressExtFlags for a single device. + * + * No return value. + */ +static void +qemuDomainFillDevicePCIExtensionFlags(virDomainDeviceDefPtr dev, + virQEMUCapsPtr qemuCaps) +{ + virDomainDeviceInfoPtr info = virDomainDeviceGetInfo(dev); + + if (info) { + info->pciAddressExtFlags + = qemuDomainDeviceCalculatePCIAddressExtensionFlags(qemuCaps, dev); + } +} + + static int qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, virDomainDeviceInfoPtr dev) @@ -2380,6 +2506,9 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (qemuDomainFillAllPCIConnectFlags(def, qemuCaps, driver) < 0) goto cleanup; + if (qemuDomainFillAllPCIExtensionFlags(def, qemuCaps) < 0) + goto cleanup; + if (qemuDomainSetupIsolationGroups(def) < 0) goto cleanup; @@ -2415,7 +2544,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, */ virDomainDeviceInfo info = { .pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCI_DEVICE) + VIR_PCI_CONNECT_TYPE_PCI_DEVICE), + .pciAddressExtFlags = VIR_PCI_ADDRESS_EXTENSION_NONE }; bool buses_reserved = true; @@ -2452,7 +2582,8 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, qemuDomainHasPCIeRoot(def)) { virDomainDeviceInfo info = { .pciConnectFlags = (VIR_PCI_CONNECT_HOTPLUGGABLE | - VIR_PCI_CONNECT_TYPE_PCIE_DEVICE) + VIR_PCI_CONNECT_TYPE_PCIE_DEVICE), + .pciAddressExtFlags = VIR_PCI_ADDRESS_EXTENSION_NONE }; /* if there isn't an empty pcie-root-port, this will @@ -2969,6 +3100,8 @@ qemuDomainEnsurePCIAddress(virDomainObjPtr obj, qemuDomainFillDevicePCIConnectFlags(obj->def, dev, priv->qemuCaps, driver); + qemuDomainFillDevicePCIExtensionFlags(dev, priv->qemuCaps); + return virDomainPCIAddressEnsureAddr(priv->pciaddrs, info, info->pciConnectFlags); } -- 2.16.3

On Thu, May 24, 2018 at 02:24:28PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces a new attribute PCI address extension flag to deal with the extension PCI attributes such as 'uid' and 'fid' on the S390 platform.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/device_conf.h | 1 + src/conf/domain_addr.h | 5 ++ src/qemu/qemu_domain_address.c | 137 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b7c82cb6f1..adce399be6 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -502,6 +502,60 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, }
+static bool +qemuDomainDeviceSupportZPCI(virQEMUCapsPtr qemuCaps, + virDomainDeviceDefPtr device) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) + return false;
There's no need to propagate qemuCaps all the way here, if we don't have QEMU_CAPS_DEVICE_ZPCI, we don't care which devices support it.
+ + switch ((virDomainDeviceType) device->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_CHR: + return false;
Are these going to support it later? How will we detect that? Jano

在 2018/6/2 下午10:15, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:28PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces a new attribute PCI address extension flag to deal with the extension PCI attributes such as 'uid' and 'fid' on the S390 platform.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/device_conf.h | 1 + src/conf/domain_addr.h | 5 ++ src/qemu/qemu_domain_address.c | 137 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b7c82cb6f1..adce399be6 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -502,6 +502,60 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, }
+static bool +qemuDomainDeviceSupportZPCI(virQEMUCapsPtr qemuCaps, + virDomainDeviceDefPtr device) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) + return false;
There's no need to propagate qemuCaps all the way here, if we don't have QEMU_CAPS_DEVICE_ZPCI, we don't care which devices support it.
For hotplug case, qemuDomainEnsurePCIAddress() should be called. Then if we don't have QEMU_CAPS_DEVICE_ZPCI, we should not set zPCI extension flag. Except checking qemuCaps, I don't know there's any other way to check zPCI support. If we don't check zPCI cap, it would return true for those supported device types, and then zPCI device would be generated but qemu binary doesn't support that.
+ + switch ((virDomainDeviceType) device->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_CHR: + return false;
Are these going to support it later? How will we detect that? For now though, we won't support these in future. Actually I don't know how to detect that via qemu interface. But anyway, we could easily switch on support if any of them is supported on S390.
Jano

On Mon, Jun 04, 2018 at 03:52:31PM +0800, Yi Min Zhao wrote:
在 2018/6/2 下午10:15, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:28PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces a new attribute PCI address extension flag to deal with the extension PCI attributes such as 'uid' and 'fid' on the S390 platform.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/device_conf.h | 1 + src/conf/domain_addr.h | 5 ++ src/qemu/qemu_domain_address.c | 137 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b7c82cb6f1..adce399be6 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -502,6 +502,60 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, }
+static bool +qemuDomainDeviceSupportZPCI(virQEMUCapsPtr qemuCaps, + virDomainDeviceDefPtr device) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) + return false;
There's no need to propagate qemuCaps all the way here, if we don't have QEMU_CAPS_DEVICE_ZPCI, we don't care which devices support it.
For hotplug case, qemuDomainEnsurePCIAddress() should be called. Then if we don't have QEMU_CAPS_DEVICE_ZPCI, we should not set zPCI extension flag. Except checking qemuCaps, I don't know there's any other way to check zPCI support. If we don't check zPCI cap, it would return true for those supported device types, and then zPCI device would be generated but qemu binary doesn't support that.
I meant checking the capability earlier and not even calling the qemuDomainDeviceSupportZPCI if the capability is not there.
+ + switch ((virDomainDeviceType) device->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_CHR: + return false;
Are these going to support it later? How will we detect that? For now though, we won't support these in future. Actually I don't know how to detect that via qemu interface. But anyway, we could easily switch on support if any of them is supported on S390.
The last two sentences contradict each other - if we cannot easily probe it and the devices do not support it now, by turning on the support later we will break the usage of libvirt with QEMUs where these devices did not support it yet. Jano

在 2018/6/4 下午10:08, Ján Tomko 写道:
On Mon, Jun 04, 2018 at 03:52:31PM +0800, Yi Min Zhao wrote:
在 2018/6/2 下午10:15, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:28PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces a new attribute PCI address extension flag to deal with the extension PCI attributes such as 'uid' and 'fid' on the S390 platform.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/device_conf.h | 1 + src/conf/domain_addr.h | 5 ++ src/qemu/qemu_domain_address.c | 137 ++++++++++++++++++++++++++++++++++++++++- 3 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index b7c82cb6f1..adce399be6 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -502,6 +502,60 @@ qemuDomainAssignARMVirtioMMIOAddresses(virDomainDefPtr def, }
+static bool +qemuDomainDeviceSupportZPCI(virQEMUCapsPtr qemuCaps, + virDomainDeviceDefPtr device) +{ + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) + return false;
There's no need to propagate qemuCaps all the way here, if we don't have QEMU_CAPS_DEVICE_ZPCI, we don't care which devices support it.
For hotplug case, qemuDomainEnsurePCIAddress() should be called. Then if we don't have QEMU_CAPS_DEVICE_ZPCI, we should not set zPCI extension flag. Except checking qemuCaps, I don't know there's any other way to check zPCI support. If we don't check zPCI cap, it would return true for those supported device types, and then zPCI device would be generated but qemu binary doesn't support that.
I meant checking the capability earlier and not even calling the qemuDomainDeviceSupportZPCI if the capability is not there. OK.
+ + switch ((virDomainDeviceType) device->type) { + case VIR_DOMAIN_DEVICE_CONTROLLER: + case VIR_DOMAIN_DEVICE_CHR: + return false;
Are these going to support it later? How will we detect that? For now though, we won't support these in future. Actually I don't know how to detect that via qemu interface. But anyway, we could easily switch on support if any of them is supported on S390.
The last two sentences contradict each other - if we cannot easily probe it and the devices do not support it now, by turning on the support later we will break the usage of libvirt with QEMUs where these devices did not support it yet. Yes, that is a problem. But there's no proper way to probe which device supports zPCI. There's a problem confusing me. To what degree we consider it is supported. For example, a pci device could be plugged to s390 phb but it can't be used in guest OS.
Jano

From: Yi Min Zhao <zyimin@linux.ibm.com> QEMU on s390 supports PCI multibus since forever. But zPCI, as extension of PCI device on s390, is the significant capability. Only when zPCI capability is existing, we consider QEMU supports PCI multibus properly. So let enable PCI multibus only if zPCI is supported. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e6fd7ee468..3d4f64ca09 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1716,6 +1716,9 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; } + if (ARCH_IS_S390(def->os.arch)) + return virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI); + /* If 'virt' supports PCI, it supports multibus. * No extra conditions here for simplicity. */ -- 2.16.3

On Thu, May 24, 2018 at 02:24:29PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
QEMU on s390 supports PCI multibus since forever. But zPCI, as extension of PCI device on s390, is the significant capability. Only when zPCI capability is existing, we consider QEMU supports PCI multibus properly. So let enable PCI multibus only if zPCI is supported.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e6fd7ee468..3d4f64ca09 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1716,6 +1716,9 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; }
+ if (ARCH_IS_S390(def->os.arch))
This includes S390 as well as S390X, do we care about that distinction? Jano

在 2018/6/2 下午10:16, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:29PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
QEMU on s390 supports PCI multibus since forever. But zPCI, as extension of PCI device on s390, is the significant capability. Only when zPCI capability is existing, we consider QEMU supports PCI multibus properly. So let enable PCI multibus only if zPCI is supported.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e6fd7ee468..3d4f64ca09 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1716,6 +1716,9 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; }
+ if (ARCH_IS_S390(def->os.arch))
This includes S390 as well as S390X, do we care about that distinction? No need, at least from my point of view.
Jano

On Mon, Jun 04, 2018 at 01:57:04PM +0800, Yi Min Zhao wrote:
在 2018/6/2 下午10:16, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:29PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
QEMU on s390 supports PCI multibus since forever. But zPCI, as extension of PCI device on s390, is the significant capability. Only when zPCI capability is existing, we consider QEMU supports PCI multibus properly. So let enable PCI multibus only if zPCI is supported.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e6fd7ee468..3d4f64ca09 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1716,6 +1716,9 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; }
+ if (ARCH_IS_S390(def->os.arch))
This includes S390 as well as S390X, do we care about that distinction? No need, at least from my point of view.
On second thought, if this is the only way to get PCI to work on S390, this particular function should not be capability-based but always return true - it merely says whether to use pci.0 vs. pci for the 0th bus, not whether PCI is actually supported or not. Jano

在 2018/6/4 下午10:10, Ján Tomko 写道:
On Mon, Jun 04, 2018 at 01:57:04PM +0800, Yi Min Zhao wrote:
在 2018/6/2 下午10:16, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:29PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
QEMU on s390 supports PCI multibus since forever. But zPCI, as extension of PCI device on s390, is the significant capability. Only when zPCI capability is existing, we consider QEMU supports PCI multibus properly. So let enable PCI multibus only if zPCI is supported.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_capabilities.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index e6fd7ee468..3d4f64ca09 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -1716,6 +1716,9 @@ bool virQEMUCapsHasPCIMultiBus(virQEMUCapsPtr qemuCaps, return false; }
+ if (ARCH_IS_S390(def->os.arch))
This includes S390 as well as S390X, do we care about that distinction? No need, at least from my point of view.
On second thought, if this is the only way to get PCI to work on S390, this particular function should not be capability-based but always return true - it merely says whether to use pci.0 vs. pci for the 0th bus, not whether PCI is actually supported or not.
Jano Yes, I agree with you. Only pci.0 is supported on s390 throughout.

From: Yi Min Zhao <zyimin@linux.ibm.com> The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee676a2789..05136540aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3068,6 +3068,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; + addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI); break; case VIR_ARCH_SPARC: -- 2.16.3

On Thu, May 24, 2018 at 02:24:30PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee676a2789..05136540aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3068,6 +3068,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; + addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI);
So from a certain QEMU version all the S390 machine types have an implicit PCI root? Is migration a thing on S390? Jano
break;
case VIR_ARCH_SPARC: -- 2.16.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

在 2018/6/2 下午10:18, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:30PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee676a2789..05136540aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3068,6 +3068,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; + addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI);
So from a certain QEMU version all the S390 machine types have an implicit PCI root? If zPCI exists, there must be pci root.
Is migration a thing on S390? I'm not clear with your said. Could you please explain more?
Jano
break;
case VIR_ARCH_SPARC: -- 2.16.3
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On Mon, Jun 04, 2018 at 02:05:17PM +0800, Yi Min Zhao wrote:
在 2018/6/2 下午10:18, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:30PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee676a2789..05136540aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3068,6 +3068,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; + addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI);
So from a certain QEMU version all the S390 machine types have an implicit PCI root? If zPCI exists, there must be pci root.
Is migration a thing on S390? I'm not clear with your said. Could you please explain more?
Well from this code it seems from a certain version, QEMU added a new implicit device for all machine types, without letting libvirt have control over it, which is strange. So if we have a machine started by older libvirt on older QEMU, can we do a live migration to a newer libvirt+QEMU with the zPCI functionality? Or did live migration never work on S390? Jano

On Mon, 4 Jun 2018 16:17:52 +0200 Ján Tomko <jtomko@redhat.com> wrote:
On Mon, Jun 04, 2018 at 02:05:17PM +0800, Yi Min Zhao wrote:
在 2018/6/2 下午10:18, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:30PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
The pci-root depends on zpci capability. So autogenerate pci-root if zpci exists.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_domain.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index ee676a2789..05136540aa 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -3068,6 +3068,7 @@ qemuDomainDefAddDefaultDevices(virDomainDefPtr def, case VIR_ARCH_S390X: addDefaultUSB = false; addPanicDevice = true; + addPCIRoot = virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI);
So from a certain QEMU version all the S390 machine types have an implicit PCI root? If zPCI exists, there must be pci root.
Is migration a thing on S390? I'm not clear with your said. Could you please explain more?
Well from this code it seems from a certain version, QEMU added a new implicit device for all machine types, without letting libvirt have control over it, which is strange.
So if we have a machine started by older libvirt on older QEMU, can we do a live migration to a newer libvirt+QEMU with the zPCI functionality? Or did live migration never work on S390?
[warning, perspective of a non-libvirt qemu developer ahead] Sadly, zpci support in qemu took quite some time from "we've committed a prototype that works for some of us with hacked-up setups" to "should be usable". - The s390 phb device has been created unconditionally since 2.3. We tried to make it conditional later, but could not find a way to do so without breaking migration. - Compat machines for s390 only go back to 2.4, so this hopefully should not hurt migration. - The zpci device (which seems to be what the zpci capability refers to, if I read patch 2 correctly) has only existed since 2.7, so any qemu supporting the zpci device also has the s390 phb present, which seems to be in line with what this patch does. - You'll also need the zpci cpu feature bit set to have something that is actually usable by a guest. That was added in 2.10.

From: Yi Min Zhao <zyimin@linux.ibm.com> Add new functions to generate zPCI command string and append it to QEMU command line. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 ++ 2 files changed, 108 insertions(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index cb397c7558..ad10846d73 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -2167,6 +2167,68 @@ qemuBuildDriveDevStr(const virDomainDef *def, return NULL; } +char * +qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev) +{ + virBuffer buf = VIR_BUFFER_INITIALIZER; + + virBufferAddLit(&buf, "zpci"); + virBufferAsprintf(&buf, ",uid=%u", dev->addr.pci.zpci->zpciuid); + virBufferAsprintf(&buf, ",fid=%u", dev->addr.pci.zpci->zpcifid); + virBufferAsprintf(&buf, ",target=%s", dev->alias); + virBufferAsprintf(&buf, ",id=zpci%u", dev->addr.pci.zpci->zpciuid); + + if (virBufferCheckError(&buf) < 0) { + virBufferFreeAndReset(&buf); + return NULL; + } + + return virBufferContentAndReset(&buf); +} + +bool +qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info) +{ + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && + ((info->pciAddressExtFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) || + info->addr.pci.zpci)) + return true; + + return false; +} + +static int +qemuAppendZPCIDevStr(virCommandPtr cmd, + virDomainDeviceInfoPtr dev) +{ + char *devstr = NULL; + + virCommandAddArg(cmd, "-device"); + if (!(devstr = qemuBuildZPCIDevStr(dev))) + return -1; + + virCommandAddArg(cmd, devstr); + + VIR_FREE(devstr); + return 0; +} + +static int +qemuBuildExtensionCommandLine(virCommandPtr cmd, + virQEMUCapsPtr qemuCaps, + virDomainDeviceInfoPtr dev) +{ + if (qemuCheckDeviceIsZPCI(dev)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support zpci devices")); + return -1; + } + return qemuAppendZPCIDevStr(cmd, dev); + } + + return 0; +} static int qemuBulildFloppyCommandLineOptions(virCommandPtr cmd, @@ -2311,6 +2373,9 @@ qemuBuildDiskDriveCommandLine(virCommandPtr cmd, bootindex) < 0) return -1; } else { + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &disk->info) < 0) + return -1; + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildDriveDevStr(def, disk, bootindex, @@ -2443,6 +2508,9 @@ qemuBuildFSDevCommandLine(virCommandPtr cmd, virCommandAddArg(cmd, optstr); VIR_FREE(optstr); + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &fs->info) < 0) + return -1; + virCommandAddArg(cmd, "-device"); if (!(optstr = qemuBuildFSDevStr(def, fs, qemuCaps))) return -1; @@ -3745,6 +3813,9 @@ qemuBuildWatchdogCommandLine(virCommandPtr cmd, if (!def->watchdog) return 0; + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &def->watchdog->info) < 0) + return -1; + virCommandAddArg(cmd, "-device"); optstr = qemuBuildWatchdogDevStr(def, watchdog, qemuCaps); @@ -3829,6 +3900,9 @@ qemuBuildMemballoonCommandLine(virCommandPtr cmd, if (qemuBuildVirtioOptionsStr(&buf, def->memballoon->virtio, qemuCaps) < 0) goto error; + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &def->memballoon->info) < 0) + goto error; + virCommandAddArg(cmd, "-device"); virCommandAddArgBuffer(cmd, &buf); return 0; @@ -4051,6 +4125,9 @@ qemuBuildInputCommandLine(virCommandPtr cmd, virDomainInputDefPtr input = def->inputs[i]; char *devstr = NULL; + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &input->info) < 0) + return -1; + if (qemuBuildInputDevStr(&devstr, def, input, qemuCaps) < 0) return -1; @@ -4192,6 +4269,9 @@ qemuBuildSoundCommandLine(virCommandPtr cmd, if (sound->model == VIR_DOMAIN_SOUND_MODEL_PCSPK) { virCommandAddArgList(cmd, "-soundhw", "pcspk", NULL); } else { + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &sound->info) < 0) + return -1; + virCommandAddArg(cmd, "-device"); if (!(str = qemuBuildSoundDevStr(def, sound, qemuCaps))) return -1; @@ -4428,6 +4508,8 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, if (video->primary) { if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_VIDEO_PRIMARY)) { + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &def->videos[i]->info) < 0) + return -1; virCommandAddArg(cmd, "-device"); if (!(str = qemuBuildDeviceVideoStr(def, video, qemuCaps))) @@ -4440,6 +4522,9 @@ qemuBuildVideoCommandLine(virCommandPtr cmd, return -1; } } else { + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &def->videos[i]->info) < 0) + return -1; + virCommandAddArg(cmd, "-device"); if (!(str = qemuBuildDeviceVideoStr(def, video, qemuCaps))) @@ -5209,6 +5294,10 @@ qemuBuildHostdevCommandLine(virCommandPtr cmd, VIR_COMMAND_PASS_FD_CLOSE_PARENT); } } + + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, hostdev->info) < 0) + return -1; + virCommandAddArg(cmd, "-device"); devstr = qemuBuildPCIHostdevDevStr(def, hostdev, bootIndex, configfd_name, qemuCaps); @@ -5673,6 +5762,9 @@ qemuBuildRNGCommandLine(virLogManagerPtr logManager, VIR_FREE(tmp); /* add the device */ + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &rng->info) < 0) + return -1; + if (!(tmp = qemuBuildRNGDevStr(def, rng, qemuCaps))) return -1; virCommandAddArgList(cmd, "-device", tmp, NULL); @@ -8109,6 +8201,9 @@ qemuBuildVhostuserCommandLine(virQEMUDriverPtr driver, virCommandAddArg(cmd, netdev); VIR_FREE(netdev); + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &net->info) < 0) + goto error; + if (!(nic = qemuBuildNicDevStr(def, net, -1, bootindex, queues, qemuCaps))) { virReportError(VIR_ERR_INTERNAL_ERROR, @@ -8400,6 +8495,9 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, goto cleanup; virCommandAddArgList(cmd, "-netdev", host, NULL); + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &net->info) < 0) + goto cleanup; + if (!(nic = qemuBuildNicDevStr(def, net, vlan, bootindex, vhostfdSize, qemuCaps))) goto cleanup; @@ -8857,6 +8955,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, switch ((virDomainShmemModel)shmem->model) { case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM: + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &shmem->info) < 0) + return -1; + devstr = qemuBuildShmemDevLegacyStr(def, shmem, qemuCaps); break; @@ -8869,6 +8970,9 @@ qemuBuildShmemCommandLine(virLogManagerPtr logManager, ATTRIBUTE_FALLTHROUGH; case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM_DOORBELL: + if (qemuBuildExtensionCommandLine(cmd, qemuCaps, &shmem->info) < 0) + return -1; + devstr = qemuBuildShmemDevStr(def, shmem, qemuCaps); break; diff --git a/src/qemu/qemu_command.h b/src/qemu/qemu_command.h index bbbf152660..7100af477f 100644 --- a/src/qemu/qemu_command.h +++ b/src/qemu/qemu_command.h @@ -172,6 +172,10 @@ char *qemuBuildRedirdevDevStr(const virDomainDef *def, virDomainRedirdevDefPtr dev, virQEMUCapsPtr qemuCaps); +char *qemuBuildZPCIDevStr(virDomainDeviceInfoPtr dev); + +bool qemuCheckDeviceIsZPCI(virDomainDeviceInfoPtr info); + int qemuNetworkPrepareDevices(virDomainDefPtr def); int qemuGetDriveSourceString(virStorageSourcePtr src, -- 2.16.3

On Thu, May 24, 2018 at 02:24:31PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Add new functions to generate zPCI command string and append it to QEMU command line.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 ++ 2 files changed, 108 insertions(+)
+static int +qemuBuildExtensionCommandLine(virCommandPtr cmd, + virQEMUCapsPtr qemuCaps, + virDomainDeviceInfoPtr dev) +{ + if (qemuCheckDeviceIsZPCI(dev)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support zpci devices")); + return -1; + }
Even though we have a lot of pre-existing code, qemuBuild* should not be reporting errors based on missing caps. That belongs in Validate. But it's not necessary here, because we would not have set VIR_PCI_ADDRESS_EXTENSION_ZPCI otherwise, right? Jano
+ return qemuAppendZPCIDevStr(cmd, dev); + } + + return 0; +}
static int qemuBulildFloppyCommandLineOptions(virCommandPtr cmd,

在 2018/6/2 下午10:22, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:31PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
Add new functions to generate zPCI command string and append it to QEMU command line.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_command.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++ src/qemu/qemu_command.h | 4 ++ 2 files changed, 108 insertions(+)
+static int +qemuBuildExtensionCommandLine(virCommandPtr cmd, + virQEMUCapsPtr qemuCaps, + virDomainDeviceInfoPtr dev) +{ + if (qemuCheckDeviceIsZPCI(dev)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support zpci devices")); + return -1; + }
Even though we have a lot of pre-existing code, qemuBuild* should not be reporting errors based on missing caps. That belongs in Validate.
But it's not necessary here, because we would not have set VIR_PCI_ADDRESS_EXTENSION_ZPCI otherwise, right? Yes, sounds reasonable. Until now, as my test, it's doable. But I think we need more test. If it has no problem, I will change this as your comment in next version. Thanks for your comment!
Jano
+ return qemuAppendZPCIDevStr(cmd, dev); + } + + return 0; +}
static int qemuBulildFloppyCommandLineOptions(virCommandPtr cmd,

From: Yi Min Zhao <zyimin@linux.ibm.com> This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 175 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 158 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b35594be5f..c64b4fa722 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -193,6 +193,84 @@ qemuDomainDelDiskSrcTLSObject(virQEMUDriverPtr driver, } +static int +qemuDomainAttachZPCIDevice(qemuMonitorPtr mon, + virDomainDeviceInfoPtr info) +{ + int ret = -1; + char *devstr_zpci = NULL; + + if (!(devstr_zpci = qemuBuildZPCIDevStr(info))) + goto cleanup; + + if (qemuMonitorAddDevice(mon, devstr_zpci) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(devstr_zpci); + return ret; +} + + +static int +qemuDomainDetachZPCIDevice(qemuMonitorPtr mon, + virDomainDeviceInfoPtr info) +{ + char *zpciAlias = NULL; + int ret = -1; + + if (virAsprintf(&zpciAlias, "zpci%d", info->addr.pci.zpci->zpciuid) < 0) + goto cleanup; + + if (qemuMonitorDelDevice(mon, zpciAlias) < 0) + goto cleanup; + + ret = 0; + + cleanup: + VIR_FREE(zpciAlias); + return ret; +} + + +static int +qemuDomainAttachExtensionDevice(qemuMonitorPtr mon, + virDomainDeviceInfoPtr info, + virQEMUCapsPtr qemuCaps) +{ + if (qemuCheckDeviceIsZPCI(info)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support zpci devices")); + return -1; + } + return qemuDomainAttachZPCIDevice(mon, info); + } + + return 0; +} + + +static int +qemuDomainDetachExtensionDevice(qemuMonitorPtr mon, + virDomainDeviceInfoPtr info, + virQEMUCapsPtr qemuCaps) +{ + if (qemuCheckDeviceIsZPCI(info)) { + if (!virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("This QEMU doesn't support zpci devices")); + return -1; + } + return qemuDomainDetachZPCIDevice(mon, info); + } + + return 0; +} + + static int qemuHotplugWaitForTrayEject(virQEMUDriverPtr driver, virDomainObjPtr vm, @@ -521,9 +599,16 @@ qemuDomainAttachDiskGeneric(virQEMUDriverPtr driver, goto exit_monitor; driveAdded = true; - if (qemuMonitorAddDevice(priv->mon, devstr) < 0) + if (qemuDomainAttachExtensionDevice(priv->mon, &disk->info, + priv->qemuCaps) < 0) goto exit_monitor; + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &disk->info, + priv->qemuCaps)); + goto exit_monitor; + } + if (qemuDomainObjExitMonitor(driver, vm) < 0) { ret = -2; goto error; @@ -1088,17 +1173,19 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, } if (qemuDomainIsS390CCW(vm->def) && - virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) { - net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; - if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) - goto cleanup; - if (virDomainCCWAddressAssign(&net->info, ccwaddrs, - !net->info.addr.ccw.assigned) < 0) + net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) { + net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; + if (!(ccwaddrs = qemuDomainCCWAddrSetCreateFromDomain(vm->def))) + goto cleanup; + if (virDomainCCWAddressAssign(&net->info, ccwaddrs, + !net->info.addr.ccw.assigned) < 0) + goto cleanup; + } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-s390 net device cannot be hotplugged.")); goto cleanup; - } else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio-s390 net device cannot be hotplugged.")); - goto cleanup; + } } else if (qemuDomainEnsurePCIAddress(vm, &dev, driver) < 0) { goto cleanup; } @@ -1158,7 +1245,17 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, goto try_remove; qemuDomainObjEnterMonitor(driver, vm); + + if (qemuDomainAttachExtensionDevice(priv->mon, &net->info, + priv->qemuCaps) < 0) { + ignore_value(qemuDomainObjExitMonitor(driver, vm)); + virDomainAuditNet(vm, NULL, net, "attach", false); + goto try_remove; + } + if (qemuMonitorAddDevice(priv->mon, nicstr) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &net->info, + priv->qemuCaps)); ignore_value(qemuDomainObjExitMonitor(driver, vm)); virDomainAuditNet(vm, NULL, net, "attach", false); goto try_remove; @@ -1374,8 +1471,17 @@ qemuDomainAttachHostPCIDevice(virQEMUDriverPtr driver, goto error; qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, - configfd, configfd_name); + + if ((ret = qemuDomainAttachExtensionDevice(priv->mon, hostdev->info, + priv->qemuCaps)) < 0) + goto exit_monitor; + + if ((ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, + configfd, configfd_name)) < 0) + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, hostdev->info, + priv->qemuCaps)); + + exit_monitor: if (qemuDomainObjExitMonitor(driver, vm) < 0) goto error; @@ -2052,8 +2158,15 @@ qemuDomainAttachRNGDevice(virQEMUDriverPtr driver, goto exit_monitor; objAdded = true; - if (qemuMonitorAddDevice(priv->mon, devstr) < 0) + if (qemuDomainAttachExtensionDevice(priv->mon, &rng->info, + priv->qemuCaps) < 0) + goto exit_monitor; + + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &rng->info, + priv->qemuCaps)); goto exit_monitor; + } if (qemuDomainObjExitMonitor(driver, vm) < 0) { releaseaddr = false; @@ -2555,8 +2668,18 @@ qemuDomainAttachSCSIVHostDevice(virQEMUDriverPtr driver, qemuDomainObjEnterMonitor(driver, vm); - ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, vhostfd, vhostfdName); + if ((ret = qemuDomainAttachExtensionDevice(priv->mon, hostdev->info, + priv->qemuCaps)) < 0) + goto exit_monitor; + + if ((ret = qemuMonitorAddDeviceWithFd(priv->mon, devstr, vhostfd, + vhostfdName)) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, hostdev->info, + priv->qemuCaps)); + goto exit_monitor; + } + exit_monitor: if (qemuDomainObjExitMonitor(driver, vm) < 0 || ret < 0) goto audit; @@ -2800,8 +2923,15 @@ qemuDomainAttachShmemDevice(virQEMUDriverPtr driver, release_backing = true; - if (qemuMonitorAddDevice(priv->mon, shmstr) < 0) + if (qemuDomainAttachExtensionDevice(priv->mon, &shmem->info, + priv->qemuCaps) < 0) + goto exit_monitor; + + if (qemuMonitorAddDevice(priv->mon, shmstr) < 0) { + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &shmem->info, + priv->qemuCaps)); goto exit_monitor; + } if (qemuDomainObjExitMonitor(driver, vm) < 0) { release_address = false; @@ -2974,8 +3104,19 @@ qemuDomainAttachInputDevice(virQEMUDriverPtr driver, goto cleanup; qemuDomainObjEnterMonitor(driver, vm); - if (qemuMonitorAddDevice(priv->mon, devstr) < 0) + + if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) { + if (qemuDomainAttachExtensionDevice(priv->mon, &input->info, + priv->qemuCaps) < 0) + goto exit_monitor; + } + + if (qemuMonitorAddDevice(priv->mon, devstr) < 0) { + if (input->bus == VIR_DOMAIN_INPUT_BUS_VIRTIO) + ignore_value(qemuDomainDetachExtensionDevice(priv->mon, &input->info, + priv->qemuCaps)); goto exit_monitor; + } if (qemuDomainObjExitMonitor(driver, vm) < 0) { releaseaddr = false; -- 2.16.3

On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device
Hm, how did you arrive at this list? Is it 'anything that uses msi-x'?
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/qemu/qemu_hotplug.c | 175 +++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 158 insertions(+), 17 deletions(-)

Cornelia Huck <cohuck@redhat.com> [2018-05-24, 06:25PM +0200]:
On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device
Hm, how did you arrive at this list? Is it 'anything that uses msi-x'?
I guess it's just any device that libvirt actually supports hotplug for, with a few exceptions (cf. patch 3 and 6). Not familiar wuth msi-x.

On 5/25/2018 6:22 PM, Bjoern Walk wrote:
Cornelia Huck <cohuck@redhat.com> [2018-05-24, 06:25PM +0200]:
On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device Hm, how did you arrive at this list? Is it 'anything that uses msi-x'? I guess it's just any device that libvirt actually supports hotplug for, with a few exceptions (cf. patch 3 and 6). Not familiar wuth msi-x.
The list should be the devices that support MSI-X and have realized the hotplug in qemu, we support the hotplug for them in the libvirt. So the list will be updated to: virtio-blk-pci virtio-net-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci SCSIVhost device As the model of Shmem is not support in qemu, the rng device doesn't support MSI-X. So remove them. In addition, the bug found need to be fixed for the hotplug of tablet, in the virDomainDeviceIsUSB() if ((t == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) || (t == VIR_DOMAIN_DEVICE_INPUT && dev->data.input->type == VIR_DOMAIN_INPUT_BUS_USB) || ---------->dev->data.input->bus == VIR_DOMAIN_INPUT_BUS_USB (t == VIR_DOMAIN_DEVICE_HOSTDEV && I will send the fix if this bug is agreed.

On Mon, 28 May 2018 16:26:57 +0800 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
On 5/25/2018 6:22 PM, Bjoern Walk wrote:
Cornelia Huck <cohuck@redhat.com> [2018-05-24, 06:25PM +0200]:
On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device Hm, how did you arrive at this list? Is it 'anything that uses msi-x'? I guess it's just any device that libvirt actually supports hotplug for, with a few exceptions (cf. patch 3 and 6). Not familiar wuth msi-x.
The list should be the devices that support MSI-X and have realized the hotplug in qemu, we support the hotplug for them in the libvirt. So the list will be updated to:
virtio-blk-pci virtio-net-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci SCSIVhost device
Ok, that makes sense. I also checked that libvirt only allows setting the vectors property for virtio-serial and shmem (vectors == 0 turns msi-x off), so that should be fine.
As the model of Shmem is not support in qemu, the rng device doesn't support MSI-X. So remove them.
So, should qemu add support for msi-x with virtio-rng-pci in the future, we'd want to update libvirt as well, correct?
In addition, the bug found need to be fixed for the hotplug of tablet, in the virDomainDeviceIsUSB() if ((t == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) || (t == VIR_DOMAIN_DEVICE_INPUT && dev->data.input->type == VIR_DOMAIN_INPUT_BUS_USB) || ---------->dev->data.input->bus == VIR_DOMAIN_INPUT_BUS_USB (t == VIR_DOMAIN_DEVICE_HOSTDEV && I will send the fix if this bug is agreed.

在 2018/5/29 下午5:04, Cornelia Huck 写道:
On Mon, 28 May 2018 16:26:57 +0800 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
On 5/25/2018 6:22 PM, Bjoern Walk wrote:
Cornelia Huck <cohuck@redhat.com> [2018-05-24, 06:25PM +0200]:
On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device Hm, how did you arrive at this list? Is it 'anything that uses msi-x'? I guess it's just any device that libvirt actually supports hotplug for, with a few exceptions (cf. patch 3 and 6). Not familiar wuth msi-x. The list should be the devices that support MSI-X and have realized the hotplug in qemu, we support the hotplug for them in the libvirt. So the list will be updated to:
virtio-blk-pci virtio-net-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci SCSIVhost device Ok, that makes sense.
I also checked that libvirt only allows setting the vectors property for virtio-serial and shmem (vectors == 0 turns msi-x off), so that should be fine.
Thanks for your check!
As the model of Shmem is not support in qemu, the rng device doesn't support MSI-X. So remove them. So, should qemu add support for msi-x with virtio-rng-pci in the future, we'd want to update libvirt as well, correct?
This is a little bit hard to answer. Our patch adds the code calling the function which is to attach zpci address in rng attachment function. As the logic in libvirt, zpci could be generated for rng device. But it actually can't be supported in s390 qemu binary. If it is being plugged, the error should be reported. Thus, if we remove the attachement code for zpci in qemuDomainAttachRNGDevice(), we should update libvirt when rng supports msix in qemu. If we keep it here, then update for libvirt is not needed.
In addition, the bug found need to be fixed for the hotplug of tablet, in the virDomainDeviceIsUSB() if ((t == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) || (t == VIR_DOMAIN_DEVICE_INPUT && dev->data.input->type == VIR_DOMAIN_INPUT_BUS_USB) || ---------->dev->data.input->bus == VIR_DOMAIN_INPUT_BUS_USB (t == VIR_DOMAIN_DEVICE_HOSTDEV && I will send the fix if this bug is agreed.

On Fri, 1 Jun 2018 12:43:09 +0800 Yi Min Zhao <zyimin@linux.ibm.com> wrote:
在 2018/5/29 下午5:04, Cornelia Huck 写道:
On Mon, 28 May 2018 16:26:57 +0800 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
As the model of Shmem is not support in qemu, the rng device doesn't support MSI-X. So remove them. So, should qemu add support for msi-x with virtio-rng-pci in the future, we'd want to update libvirt as well, correct? This is a little bit hard to answer. Our patch adds the code calling the function which is to attach zpci address in rng attachment function. As the logic in libvirt, zpci could be generated for rng device. But it actually can't be supported in s390 qemu binary. If it is being plugged, the error should be reported.
Thus, if we remove the attachement code for zpci in qemuDomainAttachRNGDevice(), we should update libvirt when rng supports msix in qemu. If we keep it here, then update for libvirt is not needed.
General question to the libvirt folks: is it better to just assume that qemu can handle a device and deal with failure if it turns out that it doesn't work after all, or to proactively fence off devices we assume to not work? [Making virtio-rng-pci use msi-x in qemu does not look too hard to do.]

在 2018/5/29 下午5:04, Cornelia Huck 写道:
On Mon, 28 May 2018 16:26:57 +0800 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
On 5/25/2018 6:22 PM, Bjoern Walk wrote:
Cornelia Huck <cohuck@redhat.com> [2018-05-24, 06:25PM +0200]:
On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device Hm, how did you arrive at this list? Is it 'anything that uses msi-x'? I guess it's just any device that libvirt actually supports hotplug for, with a few exceptions (cf. patch 3 and 6). Not familiar wuth msi-x. The list should be the devices that support MSI-X and have realized the hotplug in qemu, we support the hotplug for them in the libvirt. So the list will be updated to:
virtio-blk-pci virtio-net-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci SCSIVhost device Ok, that makes sense.
I also checked that libvirt only allows setting the vectors property for virtio-serial and shmem (vectors == 0 turns msi-x off), so that should be fine.
As the model of Shmem is not support in qemu, the rng device doesn't support MSI-X. So remove them. So, should qemu add support for msi-x with virtio-rng-pci in the future, we'd want to update libvirt as well, correct?
In addition, the bug found need to be fixed for the hotplug of tablet, in the virDomainDeviceIsUSB() if ((t == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) || (t == VIR_DOMAIN_DEVICE_INPUT && dev->data.input->type == VIR_DOMAIN_INPUT_BUS_USB) || ---------->dev->data.input->bus == VIR_DOMAIN_INPUT_BUS_USB (t == VIR_DOMAIN_DEVICE_HOSTDEV && I will send the fix if this bug is agreed.
I have not received my response mail until now....What's wrong?

On 05/29/2018 05:04 AM, Cornelia Huck wrote:
On Mon, 28 May 2018 16:26:57 +0800 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
On 5/25/2018 6:22 PM, Bjoern Walk wrote:
Cornelia Huck <cohuck@redhat.com> [2018-05-24, 06:25PM +0200]:
On Thu, 24 May 2018 14:24:32 +0200 Xiao Feng Ren <renxiaof@linux.vnet.ibm.com> wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This commit adds hotplug support for PCI devices on S390 guests. There's no need to implement hot unplug for zPCI as QEMU implements an unplug callback which will unplug both PCI and zPCI device in a cascaded way. Currently, the following PCI devices are supported: virtio-blk-pci virtio-net-pci virtio-rng-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci Shmem device SCSIVhost device Hm, how did you arrive at this list? Is it 'anything that uses msi-x'? I guess it's just any device that libvirt actually supports hotplug for, with a few exceptions (cf. patch 3 and 6). Not familiar wuth msi-x.
Note that, lacking specific information to the contrary, libvirt should be assuming that every device supports hotplug and should attempt to hotplug the device when requested. If the device can't be hotplugged, then qemu will return an error, and libvirt will fail the operation accordingly. (No, this doesn't match libvirt behavior - hotplug for some devices simply isn't implemented, but it should be.)
The list should be the devices that support MSI-X and have realized the hotplug in qemu, we support the hotplug for them in the libvirt. So the list will be updated to:
virtio-blk-pci virtio-net-pci virtio-input-host-pci virtio-keyboard-pci virtio-mouse-pci virtio-tablet-pci vfio-pci SCSIVhost device Ok, that makes sense.
I also checked that libvirt only allows setting the vectors property for virtio-serial and shmem (vectors == 0 turns msi-x off), so that should be fine.
As the model of Shmem is not support in qemu, the rng device doesn't support MSI-X. So remove them. So, should qemu add support for msi-x with virtio-rng-pci in the future, we'd want to update libvirt as well, correct?
In addition, the bug found need to be fixed for the hotplug of tablet, in the virDomainDeviceIsUSB() if ((t == VIR_DOMAIN_DEVICE_DISK && dev->data.disk->bus == VIR_DOMAIN_DISK_BUS_USB) || (t == VIR_DOMAIN_DEVICE_INPUT && dev->data.input->type == VIR_DOMAIN_INPUT_BUS_USB) || ---------->dev->data.input->bus == VIR_DOMAIN_INPUT_BUS_USB (t == VIR_DOMAIN_DEVICE_HOSTDEV && I will send the fix if this bug is agreed.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

From: Yi Min Zhao <zyimin@linux.ibm.com> This patch introduces new XML parser/formatter functions. Uid is 16-bit and non-zero. Fid is 32-bit. They are added as two new attributes of PCI address, and parsed/formatted along with PCI address parser/formatter. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/schemas/basictypes.rng | 28 ++++++++++++++++ docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.c | 74 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.c | 3 ++ src/conf/domain_conf.c | 4 +++ src/util/virpci.h | 3 ++ 6 files changed, 113 insertions(+) diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1a18cd31b1..8050a3ebc4 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -111,6 +111,34 @@ </attribute> </optional> </define> + <define name="zpciaddress"> + <optional> + <attribute name="uid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="unsignedInt"> + <param name="minInclusive">1</param> + <param name="maxInclusive">65535</param> + </data> + </choice> + </attribute> + </optional> + <optional> + <attribute name="fid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param> + </data> + <data type="unsignedLong"> + <param name="minInclusive">0</param> + <param name="maxInclusive">4294967295</param> + </data> + </choice> + </attribute> + </optional> + </define> <!-- a 6 byte MAC address in ASCII-hex format, eg "12:34:56:78:9A:BC" --> <!-- The lowest bit of the 1st byte is the "multicast" bit. a --> diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng index f16e157397..6ab92cfde4 100644 --- a/docs/schemas/domaincommon.rng +++ b/docs/schemas/domaincommon.rng @@ -5046,6 +5046,7 @@ <value>pci</value> </attribute> <ref name="pciaddress"/> + <ref name="zpciaddress"/> </group> <group> <attribute name="type"> diff --git a/src/conf/device_conf.c b/src/conf/device_conf.c index d69f94fadf..0df1afa2af 100644 --- a/src/conf/device_conf.c +++ b/src/conf/device_conf.c @@ -32,6 +32,74 @@ #define VIR_FROM_THIS VIR_FROM_DEVICE +static int +virZPCIDeviceAddressIsValid(virZPCIDeviceAddressPtr zpci) +{ + if (!zpci->uid_assigned) + return 1; + + if (zpci->zpciuid > VIR_DOMAIN_DEVICE_ZPCI_MAX_UID || + zpci->zpciuid == 0) { + virReportError(VIR_ERR_XML_ERROR, + _("Invalid PCI address uid='0x%x', " + "must be > 0x0 and <= 0x%x"), + zpci->zpciuid, + VIR_DOMAIN_DEVICE_ZPCI_MAX_UID); + return 0; + } + + return 1; +} + +static int +virZPCIDeviceAddressParseXML(xmlNodePtr node, + virPCIDeviceAddressPtr addr) +{ + char *uid, *fid; + int ret = -1; + virZPCIDeviceAddressPtr def = NULL; + + if (VIR_ALLOC(def) < 0) + return -1; + + uid = virXMLPropString(node, "uid"); + fid = virXMLPropString(node, "fid"); + + if (uid) { + if (virStrToLong_uip(uid, NULL, 0, &def->zpciuid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'uid' attribute")); + goto cleanup; + } + def->uid_assigned = true; + } + + if (fid) { + if (virStrToLong_uip(fid, NULL, 0, &def->zpcifid) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Cannot parse <address> 'fid' attribute")); + goto cleanup; + } + def->fid_assigned = true; + } + + if (uid || fid) { + if (!virZPCIDeviceAddressIsValid(def)) + goto cleanup; + + addr->zpci = def; + def = NULL; + } + + ret = 0; + + cleanup: + VIR_FREE(uid); + VIR_FREE(fid); + VIR_FREE(def); + return ret; +} + int virDomainDeviceInfoCopy(virDomainDeviceInfoPtr dst, virDomainDeviceInfoPtr src) @@ -57,6 +125,8 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + VIR_FREE(info->addr.pci.zpci); memset(&info->addr, 0, sizeof(info->addr)); info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; VIR_FREE(info->romfile); @@ -187,6 +257,7 @@ int virPCIDeviceAddressIsValid(virPCIDeviceAddressPtr addr, "one of domain, bus, or slot must be > 0")); return 0; } + return 1; } @@ -245,6 +316,9 @@ virPCIDeviceAddressParseXML(xmlNodePtr node, if (!virPCIDeviceAddressIsEmpty(addr) && !virPCIDeviceAddressIsValid(addr, true)) goto cleanup; + if (virZPCIDeviceAddressParseXML(node, addr) < 0) + goto cleanup; + ret = 0; cleanup: diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 8964973e03..e685b9ccf7 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -959,6 +959,9 @@ virDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, dev->isolationGroup, function) < 0) return -1; + if (dev->pciAddressExtFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) + addr.zpci = dev->addr.pci.zpci; + if (virDomainPCIAddressReserveAddrInternal(addrs, &addr, flags, dev->isolationGroup, false) < 0) return -1; diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 2253098090..83ef5d4a4f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6171,6 +6171,10 @@ virDomainDeviceInfoFormat(virBufferPtr buf, info->addr.pci.slot, info->addr.pci.function); } + if (info->addr.pci.zpci) { + virBufferAsprintf(buf, " uid='0x%.4x'", info->addr.pci.zpci->zpciuid); + virBufferAsprintf(buf, " fid='0x%.8x'", info->addr.pci.zpci->zpcifid); + } if (info->addr.pci.multi) { virBufferAsprintf(buf, " multifunction='%s'", virTristateSwitchTypeToString(info->addr.pci.multi)); diff --git a/src/util/virpci.h b/src/util/virpci.h index 250e9278ba..d948a16cb8 100644 --- a/src/util/virpci.h +++ b/src/util/virpci.h @@ -36,6 +36,9 @@ typedef virPCIDeviceAddress *virPCIDeviceAddressPtr; typedef struct _virPCIDeviceList virPCIDeviceList; typedef virPCIDeviceList *virPCIDeviceListPtr; +# define VIR_DOMAIN_DEVICE_ZPCI_MAX_UID 0xFFFF +# define VIR_DOMAIN_DEVICE_ZPCI_MAX_FID 0xFFFFFFFF + typedef struct _virZPCIDeviceAddress virZPCIDeviceAddress; typedef virZPCIDeviceAddress *virZPCIDeviceAddressPtr; struct _virZPCIDeviceAddress { -- 2.16.3

On Thursday, 24 May 2018 14:24:33 CEST Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces new XML parser/formatter functions. Uid is 16-bit and non-zero. Fid is 32-bit. They are added as two new attributes of PCI address, and parsed/formatted along with PCI address parser/formatter.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/schemas/basictypes.rng | 28 ++++++++++++++++ docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.c | 74 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.c | 3 ++ src/conf/domain_conf.c | 4 +++ src/util/virpci.h | 3 ++ 6 files changed, 113 insertions(+)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1a18cd31b1..8050a3ebc4 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -111,6 +111,34 @@ </attribute> </optional> </define> + <define name="zpciaddress"> + <optional> + <attribute name="uid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="unsignedInt"> + <param name="minInclusive">1</param> + <param name="maxInclusive">65535</param> + </data> + </choice> + </attribute>
This seems to be the "uint16" type already.
+ </optional> + <optional> + <attribute name="fid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param> + </data> + <data type="unsignedLong"> + <param name="minInclusive">0</param> + <param name="maxInclusive">4294967295</param> + </data> + </choice> + </attribute>
This could be a new "uint32" type, changing the 0x prefix as non-optional (otherwise the value "10" can be both valid as decimal and hexadeciaml).
@@ -57,6 +125,8 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + VIR_FREE(info->addr.pci.zpci);
VIR_FREE should be safe to use on a NULL pointer, so just call it directly without checking the type.
memset(&info->addr, 0, sizeof(info->addr)); info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; VIR_FREE(info->romfile); @@ -187,6 +257,7 @@ int virPCIDeviceAddressIsValid(virPCIDeviceAddressPtr addr, "one of domain, bus, or slot must be > 0")); return 0; } + return 1; }
Extra change. -- Pino Toscano

Pino Toscano <ptoscano@redhat.com> [2018-05-25, 01:05PM +0200]:
On Thursday, 24 May 2018 14:24:33 CEST Xiao Feng Ren wrote:
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1a18cd31b1..8050a3ebc4 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -111,6 +111,34 @@ </attribute> </optional> </define> + <define name="zpciaddress"> + <optional> + <attribute name="uid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="unsignedInt"> + <param name="minInclusive">1</param> + <param name="maxInclusive">65535</param> + </data> + </choice> + </attribute>
This seems to be the "uint16" type already.
uint16 has '0' inclusive which we do not technically allow, although we couldn't come up with a nice enough regex to also exlude it there.
+ </optional> + <optional> + <attribute name="fid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param> + </data> + <data type="unsignedLong"> + <param name="minInclusive">0</param> + <param name="maxInclusive">4294967295</param> + </data> + </choice> + </attribute>
This could be a new "uint32" type, changing the 0x prefix as non-optional (otherwise the value "10" can be both valid as decimal and hexadeciaml).
Here it would be fine.
@@ -57,6 +125,8 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + VIR_FREE(info->addr.pci.zpci);
VIR_FREE should be safe to use on a NULL pointer, so just call it directly without checking the type.
But if the union info->addr is not of type PCI then the pointer could hold arbitrary value, right? Which is not the case currently, because the zpci field is far enough in the structure to be always 0 set, but we shouldn't rely on this IMHO. -- IBM Systems Linux on Z & Virtualization Development ------------------------------------------------------------------------ IBM Deutschland Research & Development GmbH Schönaicher Str. 220, 71032 Böblingen Phone: +49 7031 16 1819 ------------------------------------------------------------------------ Vorsitzende des Aufsichtsrats: Martina Koederitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

On 05/25/2018 07:05 AM, Pino Toscano wrote:
On Thursday, 24 May 2018 14:24:33 CEST Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces new XML parser/formatter functions. Uid is 16-bit and non-zero. Fid is 32-bit. They are added as two new attributes of PCI address, and parsed/formatted along with PCI address parser/formatter.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/schemas/basictypes.rng | 28 ++++++++++++++++ docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.c | 74 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.c | 3 ++ src/conf/domain_conf.c | 4 +++ src/util/virpci.h | 3 ++ 6 files changed, 113 insertions(+)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1a18cd31b1..8050a3ebc4 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -111,6 +111,34 @@ </attribute> </optional> </define> + <define name="zpciaddress"> + <optional> + <attribute name="uid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="unsignedInt"> + <param name="minInclusive">1</param> + <param name="maxInclusive">65535</param> + </data> + </choice> + </attribute> This seems to be the "uint16" type already.
+ </optional> + <optional> + <attribute name="fid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param> + </data> + <data type="unsignedLong"> + <param name="minInclusive">0</param> + <param name="maxInclusive">4294967295</param> + </data> + </choice> + </attribute> This could be a new "uint32" type, changing the 0x prefix as non-optional (otherwise the value "10" can be both valid as decimal and hexadeciaml).
My personal opinion - if numbers without a leading 0x are consistently not interpreted as hexadecimal, then there is no ambiguity and no confusion. If there's a leading 0x, then it is hexadecimal. No leading 0x, it's decimal. Period.
@@ -57,6 +125,8 @@ void virDomainDeviceInfoClear(virDomainDeviceInfoPtr info) { VIR_FREE(info->alias); + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) + VIR_FREE(info->addr.pci.zpci); VIR_FREE should be safe to use on a NULL pointer, so just call it directly without checking the type.
But this code isn't checking for a NULL pointer. It's checking to see which member of the union is being used - there may be a different address type that uses the same region of memory as something that isn't a pointer, and calling VIR_FREE would lead to dereferencing a bad pointer.
memset(&info->addr, 0, sizeof(info->addr)); info->type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE; VIR_FREE(info->romfile); @@ -187,6 +257,7 @@ int virPCIDeviceAddressIsValid(virPCIDeviceAddressPtr addr, "one of domain, bus, or slot must be > 0")); return 0; } + return 1; }
Extra change.
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list

On 06/08/2018 03:40 AM, Laine Stump wrote:
On 05/25/2018 07:05 AM, Pino Toscano wrote:
On Thursday, 24 May 2018 14:24:33 CEST Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch introduces new XML parser/formatter functions. Uid is 16-bit and non-zero. Fid is 32-bit. They are added as two new attributes of PCI address, and parsed/formatted along with PCI address parser/formatter.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- docs/schemas/basictypes.rng | 28 ++++++++++++++++ docs/schemas/domaincommon.rng | 1 + src/conf/device_conf.c | 74 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.c | 3 ++ src/conf/domain_conf.c | 4 +++ src/util/virpci.h | 3 ++ 6 files changed, 113 insertions(+)
diff --git a/docs/schemas/basictypes.rng b/docs/schemas/basictypes.rng index 1a18cd31b1..8050a3ebc4 100644 --- a/docs/schemas/basictypes.rng +++ b/docs/schemas/basictypes.rng @@ -111,6 +111,34 @@ </attribute> </optional> </define> + <define name="zpciaddress"> + <optional> + <attribute name="uid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,4}</param> + </data> + <data type="unsignedInt"> + <param name="minInclusive">1</param> + <param name="maxInclusive">65535</param> + </data> + </choice> + </attribute> This seems to be the "uint16" type already.
+ </optional> + <optional> + <attribute name="fid"> + <choice> + <data type="string"> + <param name="pattern">(0x)?[0-9a-fA-F]{1,8}</param> + </data> + <data type="unsignedLong"> + <param name="minInclusive">0</param> + <param name="maxInclusive">4294967295</param> + </data> + </choice> + </attribute> This could be a new "uint32" type, changing the 0x prefix as non-optional (otherwise the value "10" can be both valid as decimal and hexadeciaml).
My personal opinion - if numbers without a leading 0x are consistently not interpreted as hexadecimal, then there is no ambiguity and no confusion. If there's a leading 0x, then it is hexadecimal. No leading 0x, it's decimal. Period. Laine, so are you saying the pattern for type string should for uid and fid start with 0x instead of (0x)?. That would certainly work. The difference is than the error message if e.g. for uid a user specifies 0. The schema error is than error: XML document failed to validate against schema: Unable to validate doc against /usr/share/libvirt/schemas/domain.rng Extra element devices in interleave Element domain failed to validate content
and with the optional 0x the value 0 would bypass the schema and the checks in the code reports the user friendly error error: XML error: Invalid PCI address uid='0x0', must be > 0x0 and <= 0xffff I agree that the later is really tricking the schema but with usability in mind (you hopefully agree that the schema error message su(&s ) I definitely vote for the later solution.... :-( -- Mit freundlichen Grüßen/Kind regards Boris Fiuczynski IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martina Köderitz Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

From: Yi Min Zhao <zyimin@linux.ibm.com> This patch provides a caching mechanism for the device address extensions uid and fid on S390. For efficient sparse address allocation, we introduce two hash tables for uid/fid which hold the address set information per domain. Also in order to improve performance of searching available value, we introduce our own callbacks for the two hashtables. In this way, uid/fid is saved in hash key and hash value could be any non-NULL pointer due to no operation on hash value. That is also the reason why we don't introduce hash value free callback. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.vnet.ibm.com> --- src/conf/domain_addr.c | 79 ++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 9 +++++ src/libvirt_private.syms | 1 + src/qemu/qemu_domain_address.c | 5 +++ 4 files changed, 94 insertions(+) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index e685b9ccf7..37a3645c5d 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -27,11 +27,23 @@ #include "virlog.h" #include "virstring.h" #include "domain_addr.h" +#include "virhashcode.h" #define VIR_FROM_THIS VIR_FROM_DOMAIN VIR_LOG_INIT("conf.domain_addr"); +static void +virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs) +{ + if (!addrs || !addrs->zpciIds) + return; + + virHashFree(addrs->zpciIds->uids); + virHashFree(addrs->zpciIds->fids); + VIR_FREE(addrs->zpciIds); +} + virDomainPCIConnectFlags virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model) { @@ -741,6 +753,72 @@ virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, addrs->buses[addr->bus].slot[addr->slot].functions &= ~(1 << addr->function); } + +static uint32_t virZPCIAddrCode(const void *name, uint32_t seed) +{ + unsigned int value = *((unsigned int *)name); + return virHashCodeGen(&value, sizeof(value), seed); +} + + +static bool virZPCIAddrEqual(const void *namea, const void *nameb) +{ + return *((unsigned int *)namea) == *((unsigned int *)nameb); +} + + +static void *virZPCIAddrCopy(const void *name) +{ + unsigned int *copy; + + if (VIR_ALLOC(copy) < 0) + return NULL; + + *copy = *((unsigned int *)name); + return (void *)copy; +} + + +static void virZPCIAddrKeyFree(void *name) +{ + VIR_FREE(name); +} + + +int +virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs, + virDomainPCIAddressExtensionFlags extFlags) +{ + if (extFlags == VIR_PCI_ADDRESS_EXTENSION_ZPCI) { + if (addrs->zpciIds) + return 0; + + if (VIR_ALLOC(addrs->zpciIds) < 0) + return -1; + + if (!(addrs->zpciIds->uids = virHashCreateFull(10, NULL, + virZPCIAddrCode, + virZPCIAddrEqual, + virZPCIAddrCopy, + virZPCIAddrKeyFree))) + goto error; + + if (!(addrs->zpciIds->fids = virHashCreateFull(10, NULL, + virZPCIAddrCode, + virZPCIAddrEqual, + virZPCIAddrCopy, + virZPCIAddrKeyFree))) + goto error; + } + + return 0; + + error: + virDomainPCIAddressSetExtensionFree(addrs); + return -1; +} + + virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses) { @@ -767,6 +845,7 @@ virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs) if (!addrs) return; + virDomainPCIAddressSetExtensionFree(addrs); VIR_FREE(addrs->buses); VIR_FREE(addrs); } diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index 1d1837bd23..f95d56c0a0 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -116,6 +116,11 @@ typedef struct { } virDomainPCIAddressBus; typedef virDomainPCIAddressBus *virDomainPCIAddressBusPtr; +typedef struct { + virHashTablePtr uids, fids; +} virDomainZPCIAddressIds; +typedef virDomainZPCIAddressIds *virDomainZPCIAddressIdsPtr; + struct _virDomainPCIAddressSet { virDomainPCIAddressBus *buses; size_t nbuses; @@ -125,6 +130,7 @@ struct _virDomainPCIAddressSet { bool areMultipleRootsSupported; /* If true, the guest can use the pcie-to-pci-bridge controller */ bool isPCIeToPCIBridgeSupported; + virDomainZPCIAddressIdsPtr zpciIds; }; typedef struct _virDomainPCIAddressSet virDomainPCIAddressSet; typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; @@ -132,6 +138,9 @@ typedef virDomainPCIAddressSet *virDomainPCIAddressSetPtr; char *virDomainPCIAddressAsString(virPCIDeviceAddressPtr addr) ATTRIBUTE_NONNULL(1); +int virDomainPCIAddressSetExtensionAlloc(virDomainPCIAddressSetPtr addrs, + virDomainPCIAddressExtensionFlags extFlags); + virDomainPCIAddressSetPtr virDomainPCIAddressSetAlloc(unsigned int nbuses); void virDomainPCIAddressSetFree(virDomainPCIAddressSetPtr addrs); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index a97b7fe223..2be1f8788a 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -119,6 +119,7 @@ virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextAddr; virDomainPCIAddressSetAllMulti; virDomainPCIAddressSetAlloc; +virDomainPCIAddressSetExtensionAlloc; virDomainPCIAddressSetFree; virDomainPCIAddressSlotInUse; virDomainPCIAddressValidate; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index adce399be6..392723d616 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1498,6 +1498,11 @@ qemuDomainPCIAddressSetCreate(virDomainDefPtr def, addrs->dryRun = dryRun; + /* create zpci address set for s390 domain */ + if (virQEMUCapsGet(qemuCaps, QEMU_CAPS_DEVICE_ZPCI) && + virDomainPCIAddressSetExtensionAlloc(addrs, VIR_PCI_ADDRESS_EXTENSION_ZPCI)) + goto error; + /* pSeries domains support multiple pci-root controllers */ if (qemuDomainIsPSeries(def)) addrs->areMultipleRootsSupported = true; -- 2.16.3

From: Yi Min Zhao <zyimin@linux.ibm.com> This patch adds new functions for reservation, assignment and release to handle the uid/fid. If the uid/fid is defined in the domain XML, they will be reserved directly in collecting phase. If any of them is not defined, we will find out an available value for it from zPCI address hashtable, and reserve it. For hotplug case, there might be or not zPCI definition. So allocate and reserve uid/fid for undefined case. Assign if needed and reserve uid/fid for defined case. If the user define zPCI extension address but zPCI capability doesn't exist, an error will be reported. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> --- src/conf/domain_addr.c | 264 +++++++++++++++++++++++++++++++++++++++++ src/conf/domain_addr.h | 15 +++ src/libvirt_private.syms | 3 + src/qemu/qemu_domain_address.c | 41 ++++++- 4 files changed, 321 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_addr.c b/src/conf/domain_addr.c index 37a3645c5d..e8b7d06609 100644 --- a/src/conf/domain_addr.c +++ b/src/conf/domain_addr.c @@ -33,6 +33,147 @@ VIR_LOG_INIT("conf.domain_addr"); +static int +virDomainZPCIAddressReserveId(virHashTablePtr set, unsigned int id, + const char *name) +{ + if (virHashLookup(set, &id)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("zPCI %s %u is already reserved"), name, id); + return -1; + } + + if (virHashAddEntry(set, &id, (void*)1) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Failed to reserve %s %u"), name, id); + return -1; + } + + return 0; +} + +static int +virDomainZPCIAddressReserveUid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) +{ + return virDomainZPCIAddressReserveId(set, addr->zpciuid, "uid"); +} + +static int +virDomainZPCIAddressReserveFid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) +{ + return virDomainZPCIAddressReserveId(set, addr->zpcifid, "fid"); +} + + +static bool +virDomainZPCIAddressAssignId(virHashTablePtr set, unsigned int *id, + unsigned int min, unsigned int max, + const char *name) +{ + while (virHashLookup(set, &min)) { + if (min == max) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("There is no more free %s."), name); + return false; + } + ++min; + } + *id = min; + + return true; +} + +static int +virDomainZPCIAddressAssignUid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) +{ + if (addr->uid_assigned) + return 0; + + addr->uid_assigned = virDomainZPCIAddressAssignId(set, &addr->zpciuid, 1, + VIR_DOMAIN_DEVICE_ZPCI_MAX_UID, "uid"); + return addr->uid_assigned ? 0 : -1; +} + +static int +virDomainZPCIAddressAssignFid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) +{ + if (addr->fid_assigned) + return 0; + + addr->fid_assigned = virDomainZPCIAddressAssignId(set, &addr->zpcifid, 0, + VIR_DOMAIN_DEVICE_ZPCI_MAX_FID, "fid"); + return addr->fid_assigned ? 0 : -1; +} + + +static void +virDomainZPCIAddressReleaseUid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) +{ + if (!addr->uid_assigned) + return; + + if (virHashRemoveEntry(set, &addr->zpciuid)) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Release uid %u failed"), addr->zpciuid); + + addr->uid_assigned = false; +} + +static void +virDomainZPCIAddressReleaseFid(virHashTablePtr set, virZPCIDeviceAddressPtr addr) +{ + if (!addr->fid_assigned) + return; + + if (virHashRemoveEntry(set, &addr->zpcifid)) + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Release fid %u failed"), addr->zpcifid); + + addr->fid_assigned = false; +} + + +static void +virDomainZPCIAddressReleaseIds(virDomainZPCIAddressIdsPtr zpciIds, + virPCIDeviceAddressPtr addr) +{ + if (!zpciIds || !addr->zpci) + return; + + virDomainZPCIAddressReleaseUid(zpciIds->uids, addr->zpci); + + virDomainZPCIAddressReleaseFid(zpciIds->fids, addr->zpci); + + VIR_FREE(addr->zpci); +} + + +static int +virDomainZPCIAddressReserveNextUid(virHashTablePtr uids, + virZPCIDeviceAddressPtr zpci) +{ + if (virDomainZPCIAddressAssignUid(uids, zpci) < 0) + return -1; + + if (virDomainZPCIAddressReserveUid(uids, zpci) < 0) + return -1; + + return 0; +} + +static int +virDomainZPCIAddressReserveNextFid(virHashTablePtr fids, + virZPCIDeviceAddressPtr zpci) +{ + if (virDomainZPCIAddressAssignFid(fids, zpci) < 0) + return -1; + + if (virDomainZPCIAddressReserveFid(fids, zpci) < 0) + return -1; + + return 0; +} + static void virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs) { @@ -44,6 +185,116 @@ virDomainPCIAddressSetExtensionFree(virDomainPCIAddressSetPtr addrs) VIR_FREE(addrs->zpciIds); } +static int +virDomainZPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr) +{ + if (!addr->zpci) + return 0; + + if (addr->zpci->uid_assigned && + virDomainZPCIAddressReserveUid(addrs->zpciIds->uids, addr->zpci)) + return -1; + + if (addr->zpci->fid_assigned && + virDomainZPCIAddressReserveFid(addrs->zpciIds->fids, addr->zpci)) { + virDomainZPCIAddressReleaseUid(addrs->zpciIds->uids, addr->zpci); + return -1; + } + + return 0; +} + +static int +virDomainZPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, + virZPCIDeviceAddressPtr zpci) +{ + if (!zpci->uid_assigned && + virDomainZPCIAddressReserveNextUid(addrs->zpciIds->uids, zpci)) + return -1; + + if (!zpci->fid_assigned && + virDomainZPCIAddressReserveNextFid(addrs->zpciIds->fids, zpci)) { + virDomainZPCIAddressReleaseUid(addrs->zpciIds->uids, zpci); + return -1; + } + + return 0; +} + + +int +virDomainPCIAddressExtensionReserveAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr, + virDomainPCIAddressExtensionFlags extFlags) +{ + if (extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) { + /* Reserve uid/fid to ZPCI device which has defined uid/fid + * in the domain. + */ + if (virDomainZPCIAddressReserveAddr(addrs, addr) < 0) + return -1; + } + + return 0; +} + + +int +virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr dev, + virDomainPCIAddressExtensionFlags extFlags) +{ + if (extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI) { + /* Assign and reserve uid/fid to ZPCI device which has not defined uid/fid + * in the domain. + */ + virZPCIDeviceAddress zpci = { 0 }; + + if (dev->zpci) + zpci = *dev->zpci; + + if (virDomainZPCIAddressReserveNextAddr(addrs, &zpci) < 0) + return -1; + + if (!addrs->dryRun) { + if (!dev->zpci && VIR_ALLOC(dev->zpci) < 0) + return -1; + *dev->zpci = zpci; + } + } + + return 0; +} + +static int +virDomainPCIAddressEnsureExtensionAddr(virDomainPCIAddressSetPtr addrs, + virDomainDeviceInfoPtr dev) +{ + virZPCIDeviceAddressPtr zpci = dev->addr.pci.zpci; + + if (zpci && !dev->pciAddressExtFlags) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("zPCI is not supported.")); + return -1; + } + + if (!zpci) { + return virDomainPCIAddressExtensionReserveNextAddr(addrs, &dev->addr.pci, + dev->pciAddressExtFlags); + } else { + if (virDomainZPCIAddressAssignUid(addrs->zpciIds->uids, zpci)) + return -1; + + if (virDomainZPCIAddressAssignFid(addrs->zpciIds->fids, zpci)) + return -1; + + if (virDomainZPCIAddressReserveAddr(addrs, &dev->addr.pci)) + return -1; + } + + return 0; +} + virDomainPCIConnectFlags virDomainPCIControllerModelToConnectType(virDomainControllerModelPCI model) { @@ -740,12 +991,25 @@ virDomainPCIAddressEnsureAddr(virDomainPCIAddressSetPtr addrs, ret = virDomainPCIAddressReserveNextAddr(addrs, dev, flags, -1); } + if (virDomainPCIAddressEnsureExtensionAddr(addrs, dev) < 0) + goto cleanup; + cleanup: VIR_FREE(addrStr); return ret; } +void +virDomainPCIAddressExtensionReleaseAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr, + int extFlags) +{ + if ((extFlags & VIR_PCI_ADDRESS_EXTENSION_ZPCI)) + virDomainZPCIAddressReleaseIds(addrs->zpciIds, addr); +} + + void virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) diff --git a/src/conf/domain_addr.h b/src/conf/domain_addr.h index f95d56c0a0..86d54e38f2 100644 --- a/src/conf/domain_addr.h +++ b/src/conf/domain_addr.h @@ -167,6 +167,16 @@ bool virDomainPCIAddressSlotInUse(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +int virDomainPCIAddressExtensionReserveAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr, + virDomainPCIAddressExtensionFlags extFlags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int virDomainPCIAddressExtensionReserveNextAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr, + virDomainPCIAddressExtensionFlags extFlags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + int virDomainPCIAddressReserveAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr, virDomainPCIConnectFlags flags, @@ -188,6 +198,11 @@ void virDomainPCIAddressReleaseAddr(virDomainPCIAddressSetPtr addrs, virPCIDeviceAddressPtr addr) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +void virDomainPCIAddressExtensionReleaseAddr(virDomainPCIAddressSetPtr addrs, + virPCIDeviceAddressPtr addr, + int extFlags) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + void virDomainPCIAddressSetAllMulti(virDomainDefPtr def) ATTRIBUTE_NONNULL(1); diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 2be1f8788a..372f43c6d3 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -114,6 +114,9 @@ virDomainPCIAddressAsString; virDomainPCIAddressBusIsFullyReserved; virDomainPCIAddressBusSetModel; virDomainPCIAddressEnsureAddr; +virDomainPCIAddressExtensionReleaseAddr; +virDomainPCIAddressExtensionReserveAddr; +virDomainPCIAddressExtensionReserveNextAddr; virDomainPCIAddressReleaseAddr; virDomainPCIAddressReserveAddr; virDomainPCIAddressReserveNextAddr; diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 392723d616..f186691949 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -1389,6 +1389,24 @@ qemuDomainPCIAddressReserveNextAddr(virDomainPCIAddressSetPtr addrs, } +static int +qemuDomainAssignPCIAddressExtension(virDomainDefPtr def ATTRIBUTE_UNUSED, + virDomainDeviceDefPtr device ATTRIBUTE_UNUSED, + virDomainDeviceInfoPtr info, + void *opaque) +{ + virDomainPCIAddressSetPtr addrs = opaque; + virPCIDeviceAddressPtr addr = &info->addr.pci; + virDomainPCIAddressExtensionFlags extFlags = info->pciAddressExtFlags; + + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) { + if (virDomainPCIAddressExtensionReserveNextAddr(addrs, addr, extFlags) < 0) + return -1; + } + + return 0; +} + static int qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, virDomainDeviceDefPtr device, @@ -1406,7 +1424,12 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, * parent, and will have its address collected during the scan * of the parent's device type. */ - return 0; + if (info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI || + info->type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) + return virDomainPCIAddressExtensionReserveAddr(addrs, addr, + info->pciAddressExtFlags); + else + return 0; } /* If we get to here, the device has a PCI address assigned in the @@ -1477,6 +1500,11 @@ qemuDomainCollectPCIAddress(virDomainDefPtr def ATTRIBUTE_UNUSED, goto cleanup; } + if (virDomainPCIAddressExtensionReserveAddr(addrs, addr, + info->pciAddressExtFlags) < 0) { + goto cleanup; + } + ret = 0; cleanup: return ret; @@ -2568,6 +2596,9 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup; + if (virDomainDeviceInfoIterate(def, qemuDomainAssignPCIAddressExtension, addrs) < 0) + goto cleanup; + /* Only for *new* domains with pcie-root (and no other * manually specified PCI controllers in the definition): If, * after assigning addresses/reserving slots for all devices, @@ -2662,6 +2693,9 @@ qemuDomainAssignPCIAddresses(virDomainDefPtr def, if (qemuDomainAssignDevicePCISlots(def, qemuCaps, addrs) < 0) goto cleanup; + if (virDomainDeviceInfoIterate(def, qemuDomainAssignPCIAddressExtension, addrs) < 0) + goto cleanup; + /* set multi attribute for devices at function 0 of * any slot that has multiple functions in use */ @@ -3121,8 +3155,11 @@ qemuDomainReleaseDeviceAddress(virDomainObjPtr vm, if (!devstr) devstr = info->alias; - if (virDeviceInfoPCIAddressPresent(info)) + if (virDeviceInfoPCIAddressPresent(info)) { virDomainPCIAddressReleaseAddr(priv->pciaddrs, &info->addr.pci); + virDomainPCIAddressExtensionReleaseAddr(priv->pciaddrs, &info->addr.pci, + info->pciAddressExtFlags); + } if (virDomainUSBAddressRelease(priv->usbaddrs, info) < 0) VIR_WARN("Unable to release USB address on %s", NULLSTR(devstr)); -- 2.16.3

From: Yi Min Zhao <zyimin@linux.ibm.com> This patch adds new test cases for zPCI when 'uid' and 'fid' are defined with different conditions in XML. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 27 +++++++++ tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml | 17 ++++++ .../hostdev-vfio-zpci-autogenerate.args | 24 ++++++++ .../hostdev-vfio-zpci-autogenerate.xml | 18 ++++++ .../hostdev-vfio-zpci-boundaries.args | 27 +++++++++ .../hostdev-vfio-zpci-boundaries.xml | 26 +++++++++ .../hostdev-vfio-zpci-multidomain-many.args | 38 ++++++++++++ .../hostdev-vfio-zpci-multidomain-many.xml | 67 ++++++++++++++++++++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 24 ++++++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.xml | 19 ++++++ tests/qemuxml2argvtest.c | 21 +++++++ tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml | 29 ++++++++++ tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml | 30 ++++++++++ tests/qemuxml2xmltest.c | 3 + 14 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml diff --git a/tests/qemuxml2argvdata/disk-virtio-s390-zpci.args b/tests/qemuxml2argvdata/disk-virtio-s390-zpci.args new file mode 100644 index 0000000000..ffb5d1597e --- /dev/null +++ b/tests/qemuxml2argvdata/disk-virtio-s390-zpci.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-boot c \ +-drive file=/dev/HostVG/QEMUGuest1,format=raw,if=none,id=drive-virtio-disk0 \ +-device zpci,uid=25,fid=31,target=virtio-disk0,id=zpci25 \ +-device virtio-blk-pci,bus=pci.0,addr=0x8,drive=drive-virtio-disk0,\ +id=virtio-disk0 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0000 diff --git a/tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml b/tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml new file mode 100644 index 0000000000..7887b97b2c --- /dev/null +++ b/tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml @@ -0,0 +1,17 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219136</memory> + <vcpu>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0' uid='0x0019' fid='0x0000001f'/> + </disk> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args b/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args new file mode 100644 index 0000000000..d5e10dbb0c --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-device zpci,uid=1,fid=0,target=hostdev0,id=zpci1 \ +-device vfio-pci,host=00:00.0,id=hostdev0,bus=pci.0,addr=0x1 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0000 diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml b/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml new file mode 100644 index 0000000000..36161006ab --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml @@ -0,0 +1,18 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci'/> + </hostdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args b/tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args new file mode 100644 index 0000000000..e296bf7792 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args @@ -0,0 +1,27 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-device pci-bridge,chassis_nr=1,id=pci.1,bus=pci.0,addr=0x1 \ +-device zpci,uid=65535,fid=4294967295,target=hostdev0,id=zpci65535 \ +-device vfio-pci,host=ffff:00:00.0,id=hostdev0,bus=pci.1,addr=0x1f \ +-device zpci,uid=1,fid=0,target=hostdev1,id=zpci1 \ +-device vfio-pci,host=00:00.0,id=hostdev1,bus=pci.0,addr=0x2 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0000 diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml b/tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml new file mode 100644 index 0000000000..779eb12ac2 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml @@ -0,0 +1,26 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0xffff' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x01' slot='0x1f' function='0x0' fid='0xffffffff' uid='0xffff'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' fid='0x00000000' uid='0x0001'/> + </hostdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args b/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args new file mode 100644 index 0000000000..e703f8b2e0 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args @@ -0,0 +1,38 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-device zpci,uid=35,fid=63,target=hostdev0,id=zpci35 \ +-device vfio-pci,host=0001:00:00.0,id=hostdev0,bus=pci.0,addr=0x3 \ +-device zpci,uid=53,fid=104,target=hostdev1,id=zpci53 \ +-device vfio-pci,host=0002:00:00.0,id=hostdev1,bus=pci.0,addr=0x1 \ +-device zpci,uid=1,fid=0,target=hostdev2,id=zpci1 \ +-device vfio-pci,host=0003:00:00.0,id=hostdev2,bus=pci.0,addr=0x2 \ +-device zpci,uid=2,fid=1,target=hostdev3,id=zpci2 \ +-device vfio-pci,host=0004:00:00.0,id=hostdev3,bus=pci.0,addr=0x5 \ +-device zpci,uid=83,fid=2,target=hostdev4,id=zpci83 \ +-device vfio-pci,host=0005:00:00.0,id=hostdev4,bus=pci.0,addr=0x7 \ +-device zpci,uid=3,fid=114,target=hostdev5,id=zpci3 \ +-device vfio-pci,host=0006:00:00.0,id=hostdev5,bus=pci.0,addr=0x9 \ +-device zpci,uid=23,fid=3,target=hostdev6,id=zpci23 \ +-device vfio-pci,host=0007:00:00.0,id=hostdev6,bus=pci.0,addr=0x4 \ +-device zpci,uid=4,fid=40,target=hostdev7,id=zpci4 \ +-device vfio-pci,host=0008:00:00.0,id=hostdev7,bus=pci.0,addr=0x6 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0000 diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml b/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml new file mode 100644 index 0000000000..e69714fc9c --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml @@ -0,0 +1,67 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0001' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x03' function='0x0' uid='0x0023' fid='0x0000003f'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0002' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' uid='0x0035' fid='0x00000068'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0003' bus='0x00' slot='0x00' function='0x0'/> + </source> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0004' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x05' function='0x0'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0005' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x07' function='0x0' uid='0x0053'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0006' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x09' function='0x0' fid='0x00000072'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0007' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' uid='0x0017'/> + </hostdev> + <hostdev mode='subsystem' type='pci'> + <driver name='vfio'/> + <source> + <address domain='0x0008' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' fid='0x00000028'/> + </hostdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci.args b/tests/qemuxml2argvdata/hostdev-vfio-zpci.args new file mode 100644 index 0000000000..1f9f1c688a --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci.args @@ -0,0 +1,24 @@ +LC_ALL=C \ +PATH=/bin \ +HOME=/home/test \ +USER=test \ +LOGNAME=test \ +QEMU_AUDIO_DRV=none \ +/usr/bin/qemu-system-s390x \ +-name QEMUGuest1 \ +-S \ +-machine s390-ccw-virtio,accel=tcg,usb=off,dump-guest-core=off \ +-m 214 \ +-smp 1,sockets=1,cores=1,threads=1 \ +-uuid c7a5fdbd-edaf-9455-926a-d65c16db1809 \ +-display none \ +-no-user-config \ +-nodefaults \ +-chardev socket,id=charmonitor,path=/tmp/lib/domain--1-QEMUGuest1/monitor.sock,\ +server,nowait \ +-mon chardev=charmonitor,id=monitor,mode=control \ +-rtc base=utc \ +-no-shutdown \ +-device zpci,uid=25,fid=31,target=hostdev0,id=zpci25 \ +-device vfio-pci,host=00:00.0,id=hostdev0,bus=pci.0,addr=0x8 \ +-device virtio-balloon-ccw,id=balloon0,devno=fe.0.0000 diff --git a/tests/qemuxml2argvdata/hostdev-vfio-zpci.xml b/tests/qemuxml2argvdata/hostdev-vfio-zpci.xml new file mode 100644 index 0000000000..cde333e220 --- /dev/null +++ b/tests/qemuxml2argvdata/hostdev-vfio-zpci.xml @@ -0,0 +1,19 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory>219100</memory> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + </os> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0' uid='0x0019' fid='0x0000001f'/> + </hostdev> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1d023129ac..80cfec0714 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -976,6 +976,8 @@ mymain(void) QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); DO_TEST("disk-virtio-scsi-ccw", QEMU_CAPS_VIRTIO_SCSI, QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); + DO_TEST("disk-virtio-s390-zpci", QEMU_CAPS_DEVICE_ZPCI, + QEMU_CAPS_CCW, QEMU_CAPS_VIRTIO_S390); DO_TEST("disk-order", QEMU_CAPS_DRIVE_BOOT, QEMU_CAPS_VIRTIO_BLK_SCSI); DO_TEST("disk-virtio-drive-queues", @@ -1542,6 +1544,25 @@ mymain(void) QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST_PARSE_ERROR("hostdev-mdev-invalid-target-address", QEMU_CAPS_DEVICE_VFIO_PCI); + DO_TEST("hostdev-vfio-zpci", + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_CCW, + QEMU_CAPS_VIRTIO_S390, QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_DEVICE_ZPCI); + DO_TEST("hostdev-vfio-zpci-multidomain-many", + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_CCW, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI); + DO_TEST("hostdev-vfio-zpci-autogenerate", + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_CCW, QEMU_CAPS_DEVICE_VFIO_PCI, + QEMU_CAPS_DEVICE_ZPCI); + DO_TEST("hostdev-vfio-zpci-boundaries", + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_CCW, QEMU_CAPS_DEVICE_PCI_BRIDGE, + QEMU_CAPS_DEVICE_VFIO_PCI, QEMU_CAPS_DEVICE_ZPCI); + DO_TEST_FAILURE("hostdev-vfio-zpci", + QEMU_CAPS_BOOTINDEX, QEMU_CAPS_VIRTIO_S390, + QEMU_CAPS_CCW, QEMU_CAPS_DEVICE_VFIO_PCI); DO_TEST("pci-rom", NONE); DO_TEST("pci-rom-disabled", NONE); DO_TEST("pci-rom-disabled-invalid", NONE); diff --git a/tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml b/tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml new file mode 100644 index 0000000000..2e658807e9 --- /dev/null +++ b/tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml @@ -0,0 +1,29 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219136</memory> + <currentMemory unit='KiB'>219136</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <disk type='block' device='disk'> + <driver name='qemu' type='raw'/> + <source dev='/dev/HostVG/QEMUGuest1'/> + <target dev='hda' bus='virtio'/> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0' uid='0x0019' fid='0x0000001f'/> + </disk> + <controller type='pci' index='0' model='pci-root'/> + <memballoon model='virtio'> + <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x0' uid='0x0001' fid='0x00000000'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml b/tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml new file mode 100644 index 0000000000..1f48c44e30 --- /dev/null +++ b/tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml @@ -0,0 +1,30 @@ +<domain type='qemu'> + <name>QEMUGuest1</name> + <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid> + <memory unit='KiB'>219100</memory> + <currentMemory unit='KiB'>219100</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='s390x' machine='s390-ccw-virtio'>hvm</type> + <boot dev='hd'/> + </os> + <clock offset='utc'/> + <on_poweroff>destroy</on_poweroff> + <on_reboot>restart</on_reboot> + <on_crash>destroy</on_crash> + <devices> + <emulator>/usr/bin/qemu-system-s390x</emulator> + <controller type='pci' index='0' model='pci-root'/> + <hostdev mode='subsystem' type='pci' managed='no'> + <driver name='vfio'/> + <source> + <address domain='0x0000' bus='0x00' slot='0x00' function='0x0'/> + </source> + <address type='pci' domain='0x0000' bus='0x00' slot='0x08' function='0x0' uid='0x0019' fid='0x0000001f'/> + </hostdev> + <memballoon model='virtio'> + <address type='ccw' cssid='0xfe' ssid='0x0' devno='0x0000'/> + </memballoon> + <panic model='s390'/> + </devices> +</domain> diff --git a/tests/qemuxml2xmltest.c b/tests/qemuxml2xmltest.c index e31d8212fe..2c7f52a17b 100644 --- a/tests/qemuxml2xmltest.c +++ b/tests/qemuxml2xmltest.c @@ -395,6 +395,8 @@ mymain(void) QEMU_CAPS_VIRTIO_SCSI); DO_TEST("disk-virtio-scsi-ioeventfd", QEMU_CAPS_VIRTIO_SCSI); + DO_TEST("disk-virtio-s390-zpci", + QEMU_CAPS_DEVICE_ZPCI); DO_TEST("disk-scsi-megasas", QEMU_CAPS_SCSI_MEGASAS); DO_TEST("disk-scsi-mptsas1068", @@ -474,6 +476,7 @@ mymain(void) DO_TEST("hostdev-usb-address", NONE); DO_TEST("hostdev-pci-address", NONE); DO_TEST("hostdev-vfio", NONE); + DO_TEST("hostdev-vfio-zpci", QEMU_CAPS_DEVICE_ZPCI, QEMU_CAPS_CCW); DO_TEST("hostdev-mdev-precreated", NONE); DO_TEST("pci-rom", NONE); DO_TEST("pci-rom-disabled", NONE); -- 2.16.3

On Thu, May 24, 2018 at 02:24:36PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch adds new test cases for zPCI when 'uid' and 'fid' are defined with different conditions in XML.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 27 +++++++++ tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml | 17 ++++++ .../hostdev-vfio-zpci-autogenerate.args | 24 ++++++++ .../hostdev-vfio-zpci-autogenerate.xml | 18 ++++++ .../hostdev-vfio-zpci-boundaries.args | 27 +++++++++ .../hostdev-vfio-zpci-boundaries.xml | 26 +++++++++ .../hostdev-vfio-zpci-multidomain-many.args | 38 ++++++++++++ .../hostdev-vfio-zpci-multidomain-many.xml | 67 ++++++++++++++++++++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 24 ++++++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.xml | 19 ++++++ tests/qemuxml2argvtest.c | 21 +++++++
These should be squashed into the first commit that formats the command line
tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml | 29 ++++++++++ tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml | 30 ++++++++++ tests/qemuxml2xmltest.c | 3 +
And these should be introduced along with the XML parser/formatter. That way the tests demonstrate if the subsequent code changes change the tests as well or not. Jano
14 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml

在 2018/6/2 下午10:26, Ján Tomko 写道:
On Thu, May 24, 2018 at 02:24:36PM +0200, Xiao Feng Ren wrote:
From: Yi Min Zhao <zyimin@linux.ibm.com>
This patch adds new test cases for zPCI when 'uid' and 'fid' are defined with different conditions in XML.
Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Bjoern Walk <bwalk@linux.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.vnet.ibm.com> --- tests/qemuxml2argvdata/disk-virtio-s390-zpci.args | 27 +++++++++ tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml | 17 ++++++ .../hostdev-vfio-zpci-autogenerate.args | 24 ++++++++ .../hostdev-vfio-zpci-autogenerate.xml | 18 ++++++ .../hostdev-vfio-zpci-boundaries.args | 27 +++++++++ .../hostdev-vfio-zpci-boundaries.xml | 26 +++++++++ .../hostdev-vfio-zpci-multidomain-many.args | 38 ++++++++++++ .../hostdev-vfio-zpci-multidomain-many.xml | 67 ++++++++++++++++++++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.args | 24 ++++++++ tests/qemuxml2argvdata/hostdev-vfio-zpci.xml | 19 ++++++ tests/qemuxml2argvtest.c | 21 +++++++
These should be squashed into the first commit that formats the command line
tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml | 29 ++++++++++ tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml | 30 ++++++++++ tests/qemuxml2xmltest.c | 3 +
And these should be introduced along with the XML parser/formatter.
That way the tests demonstrate if the subsequent code changes change the tests as well or not.
Jano
OK. Will move test code to the corresponding patches.
14 files changed, 370 insertions(+) create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.args create mode 100644 tests/qemuxml2argvdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-autogenerate.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-boundaries.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci-multidomain-many.xml create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.args create mode 100644 tests/qemuxml2argvdata/hostdev-vfio-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/disk-virtio-s390-zpci.xml create mode 100644 tests/qemuxml2xmloutdata/hostdev-vfio-zpci.xml

From: Yi Min Zhao <zyimin@linux.ibm.com> Update 'Device address' section to describe the 'uid' and 'fid' attributes. Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- docs/formatdomain.html.in | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/formatdomain.html.in b/docs/formatdomain.html.in index 665d0f2529..e1dcefe1a7 100644 --- a/docs/formatdomain.html.in +++ b/docs/formatdomain.html.in @@ -3716,7 +3716,14 @@ (<span class="since">since 0.9.7, requires QEMU 0.13</span>). <code>multifunction</code> defaults to 'off', but should be set to 'on' for function 0 of a slot that will - have multiple functions used.<br/> + have multiple functions used. (<span class="since">Since 4.4.0 + </span>), PCI address extensions depending on the architecture + are supported. E.g. for S390 guests, PCI addresses have + additional attributes: <code>uid</code> (a hex value between + 0x1 and 0xffff, inclusive), and <code>fid</code> (a hex value + between 0x0 and 0xffffffff, inclusive), which are used by PCI + devices on S390 for User-defined Identifiers and Function + Identifiers.<br/> <span class="since">Since 1.3.5</span>, some hypervisor drivers may accept an <code><address type='pci'/></code> element with no other attributes as an explicit request to -- 2.16.3

From: Yi Min Zhao <zyimin@linux.ibm.com> Signed-off-by: Yi Min Zhao <zyimin@linux.ibm.com> Reviewed-by: Boris Fiuczynski <fiuczy@linux.vnet.ibm.com> Reviewed-by: Stefan Zimmermann <stzi@linux.ibm.com> --- docs/news.xml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/docs/news.xml b/docs/news.xml index 76d1613d35..cfd355778a 100644 --- a/docs/news.xml +++ b/docs/news.xml @@ -64,6 +64,17 @@ TLS environment which is setup for the migration connection. </description> </change> + <change> + <summary> + qemu: Added support for PCI device passthrough on S390 + </summary> + <description> + The new zPCI attributes uid (user-defined identifier) + and fid (PCI function identifier) of the S390 platform + extend the PCI device address to support the PCI + passthrough on S390. + </description> + </change> </section> <section title="Improvements"> <change> -- 2.16.3
participants (8)
-
Bjoern Walk
-
Boris Fiuczynski
-
Cornelia Huck
-
Ján Tomko
-
Laine Stump
-
Pino Toscano
-
Xiao Feng Ren
-
Yi Min Zhao