[Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

[Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

[Tony Krowiak]

This patch series is the QEMU counterpart to the KVM/kernel support for 
guest dedicated crypto adapters. The KVM/kernel model is built on the 
VFIO mediated device framework and provides the infrastructure for 
granting exclusive guest access to crypto devices installed on the linux 
host. This patch series introduces a new QEMU command line option, QEMU 
object model and CPU model features to exploit the KVM/kernel model.

See the detailed specifications for AP virtualization provided by this 
patch set in docs/vfio-ap.txt for a more complete discussion of the 
design introduced by this patch series.

Note: This series implements the minimal viable product (MVP) and does not
      provide support for hot plug/unplug, migration, or VSIE. The goal
      of the MVP model is to solidify a base upon which these additional 
      features will be built.

v3 => v4 Change log:
===================
* Made vfio-ap device unpluggable for now
* Renamed command line CPU model feature for QCI: qci=on -> apqci=on
* Removed call to KVM_S390_VM_CRYPTO_INTERPRET_AP ioctl - ioctl was 
  removed from kernel and AP instruction interpretation is set from the
  VFIO device driver
* Added check to ensure only one vfio-ap device can be configured per 
  guest
* Removed AP instruction interception handlers: AP instructions will be 
  interpreted by default if AP facilities are installed to handle the case
  where feature ap=on and no vfio-ap device is configured for the guest.

Tony Krowiak (5):
  linux-headers: linux header updates for AP support
  s390x/ap: base Adjunct Processor (AP) object
  s390x/cpumodel: Set up CPU model for AP device support
  s390x/vfio: ap: Introduce VFIO AP device
  s390: doc: detailed specifications for AP virtualization

 default-configs/s390x-softmmu.mak |    1 +
 docs/vfio-ap.txt                  |  649 +++++++++++++++++++++++++++++++++++++
 hw/s390x/Makefile.objs            |    1 +
 hw/s390x/ap-device.c              |   39 +++
 hw/vfio/Makefile.objs             |    1 +
 hw/vfio/ap.c                      |  191 +++++++++++
 include/hw/s390x/ap-device.h      |   38 +++
 include/hw/vfio/vfio-common.h     |    1 +
 linux-headers/asm-s390/kvm.h      |    2 +
 linux-headers/linux/vfio.h        |    2 +
 target/s390x/cpu_features.c       |    3 +
 target/s390x/cpu_features_def.h   |    3 +
 target/s390x/cpu_models.c         |    2 +
 target/s390x/gen-features.c       |    3 +
 target/s390x/kvm.c                |    1 +
 ui/keycodemapdb                   |    2 +-
 16 files changed, 938 insertions(+), 1 deletions(-)
 create mode 100644 docs/vfio-ap.txt
 create mode 100644 hw/s390x/ap-device.c
 create mode 100644 hw/vfio/ap.c
 create mode 100644 include/hw/s390x/ap-device.h

[Qemu-devel] [PATCH v4 1/5] linux-headers: linux header updates for AP support

[Tony Krowiak]

Updates the linux header files in preparation for introduction
of the VFIO AP device:

* Added a feature ID to indicate AP facilities are installed

* Added a device attribute to the KVM_S390_VM_CRYPTO group to
  indicate whether AP instructions are to be interpreted

* Added VFIO device information for AP devices

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---
 linux-headers/asm-s390/kvm.h |    2 ++
 linux-headers/linux/vfio.h   |    2 ++
 2 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/linux-headers/asm-s390/kvm.h b/linux-headers/asm-s390/kvm.h
index 11def14..391b250 100644
--- a/linux-headers/asm-s390/kvm.h
+++ b/linux-headers/asm-s390/kvm.h
@@ -130,6 +130,7 @@ struct kvm_s390_vm_cpu_machine {
 #define KVM_S390_VM_CPU_FEAT_PFMFI	11
 #define KVM_S390_VM_CPU_FEAT_SIGPIF	12
 #define KVM_S390_VM_CPU_FEAT_KSS	13
+#define KVM_S390_VM_CPU_FEAT_AP		14
 struct kvm_s390_vm_cpu_feat {
 	__u64 feat[16];
 };
@@ -160,6 +161,7 @@ struct kvm_s390_vm_cpu_subfunc {
 #define KVM_S390_VM_CRYPTO_ENABLE_DEA_KW	1
 #define KVM_S390_VM_CRYPTO_DISABLE_AES_KW	2
 #define KVM_S390_VM_CRYPTO_DISABLE_DEA_KW	3
+#define KVM_S390_VM_CRYPTO_INTERPRET_AP		4
 
 /* kvm attributes for migration mode */
 #define KVM_S390_VM_MIGRATION_STOP	0
diff --git a/linux-headers/linux/vfio.h b/linux-headers/linux/vfio.h
index 3a0a305..1bb9897 100644
--- a/linux-headers/linux/vfio.h
+++ b/linux-headers/linux/vfio.h
@@ -200,6 +200,7 @@ struct vfio_device_info {
 #define VFIO_DEVICE_FLAGS_PLATFORM (1 << 2)	/* vfio-platform device */
 #define VFIO_DEVICE_FLAGS_AMBA  (1 << 3)	/* vfio-amba device */
 #define VFIO_DEVICE_FLAGS_CCW	(1 << 4)	/* vfio-ccw device */
+#define VFIO_DEVICE_FLAGS_AP (1 << 5)		/* vfio-ap device */
 	__u32	num_regions;	/* Max region index + 1 */
 	__u32	num_irqs;	/* Max IRQ index + 1 */
 };
@@ -215,6 +216,7 @@ struct vfio_device_info {
 #define VFIO_DEVICE_API_PLATFORM_STRING		"vfio-platform"
 #define VFIO_DEVICE_API_AMBA_STRING		"vfio-amba"
 #define VFIO_DEVICE_API_CCW_STRING		"vfio-ccw"
+#define VFIO_DEVICE_API_AP_STRING		"vfio-ap"
 
 /**
  * VFIO_DEVICE_GET_REGION_INFO - _IOWR(VFIO_TYPE, VFIO_BASE + 8,
-- 
1.7.1

[Qemu-devel] [PATCH v4 2/5] s390x/ap: base Adjunct Processor (AP) object

[Tony Krowiak]

This patch introduces the base object for an AP device.

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---
 hw/s390x/Makefile.objs       |    1 +
 hw/s390x/ap-device.c         |   39 +++++++++++++++++++++++++++++++++++++++
 include/hw/s390x/ap-device.h |   38 ++++++++++++++++++++++++++++++++++++++
 ui/keycodemapdb              |    2 +-
 4 files changed, 79 insertions(+), 1 deletions(-)
 create mode 100644 hw/s390x/ap-device.c
 create mode 100644 include/hw/s390x/ap-device.h

diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
index dc704b5..3247a07 100644
--- a/hw/s390x/Makefile.objs
+++ b/hw/s390x/Makefile.objs
@@ -17,3 +17,4 @@ obj-y += s390-stattrib.o
 obj-$(CONFIG_KVM) += s390-skeys-kvm.o
 obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
 obj-y += s390-ccw.o
+obj-y += ap-device.o
diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
new file mode 100644
index 0000000..3cd4bae
--- /dev/null
+++ b/hw/s390x/ap-device.c
@@ -0,0 +1,39 @@
+/*
+ * Adjunct Processor (AP) matrix device
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#include "qemu/osdep.h"
+#include "qemu/module.h"
+#include "qapi/error.h"
+#include "hw/qdev.h"
+#include "hw/s390x/ap-device.h"
+
+static void ap_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->desc = "AP device class";
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo ap_device_info = {
+    .name = AP_DEVICE_TYPE,
+    .parent = TYPE_DEVICE,
+    .instance_size = sizeof(APDevice),
+    .class_size = sizeof(APDeviceClass),
+    .class_init = ap_class_init,
+    .abstract = true,
+};
+
+static void ap_device_register(void)
+{
+    type_register_static(&ap_device_info);
+}
+
+type_init(ap_device_register)
diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
new file mode 100644
index 0000000..693df90
--- /dev/null
+++ b/include/hw/s390x/ap-device.h
@@ -0,0 +1,38 @@
+/*
+ * Adjunct Processor (AP) matrix device interfaces
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+#ifndef HW_S390X_AP_DEVICE_H
+#define HW_S390X_AP_DEVICE_H
+
+#define AP_DEVICE_TYPE       "ap-device"
+
+typedef struct APDevice {
+    DeviceState parent_obj;
+} APDevice;
+
+typedef struct APDeviceClass {
+    DeviceClass parent_class;
+} APDeviceClass;
+
+static inline APDevice *to_ap_dev(DeviceState *dev)
+{
+    return container_of(dev, APDevice, parent_obj);
+}
+
+#define AP_DEVICE(obj) \
+    OBJECT_CHECK(APDevice, (obj), AP_DEVICE_TYPE)
+
+#define AP_DEVICE_GET_CLASS(obj) \
+    OBJECT_GET_CLASS(APDeviceClass, (obj), AP_DEVICE_TYPE)
+
+#define AP_DEVICE_CLASS(klass) \
+    OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE)
+
+#endif /* HW_S390X_AP_DEVICE_H */
diff --git a/ui/keycodemapdb b/ui/keycodemapdb
index 6b3d716..16e5b07 160000
--- a/ui/keycodemapdb
+++ b/ui/keycodemapdb
@@ -1 +1 @@
-Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
+Subproject commit 16e5b0787687d8904dad2c026107409eb9bfcb95
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 2/5] s390x/ap: base Adjunct Processor (AP) object

[Pierre Morel]

On 15/04/2018 21:07, Tony Krowiak wrote:
> This patch introduces the base object for an AP device.
>
> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> ---
>   hw/s390x/Makefile.objs       |    1 +
>   hw/s390x/ap-device.c         |   39 +++++++++++++++++++++++++++++++++++++++
>   include/hw/s390x/ap-device.h |   38 ++++++++++++++++++++++++++++++++++++++
>   ui/keycodemapdb              |    2 +-
>   4 files changed, 79 insertions(+), 1 deletions(-)
>   create mode 100644 hw/s390x/ap-device.c
>   create mode 100644 include/hw/s390x/ap-device.h
>
> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
> index dc704b5..3247a07 100644
> --- a/hw/s390x/Makefile.objs
> +++ b/hw/s390x/Makefile.objs
> @@ -17,3 +17,4 @@ obj-y += s390-stattrib.o
>   obj-$(CONFIG_KVM) += s390-skeys-kvm.o
>   obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
>   obj-y += s390-ccw.o
> +obj-y += ap-device.o
> diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
> new file mode 100644
> index 0000000..3cd4bae
> --- /dev/null
> +++ b/hw/s390x/ap-device.c
> @@ -0,0 +1,39 @@
> +/*
> + * Adjunct Processor (AP) matrix device
> + *
> + * Copyright 2018 IBM Corp.
> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +#include "qemu/osdep.h"
> +#include "qemu/module.h"
> +#include "qapi/error.h"
> +#include "hw/qdev.h"
> +#include "hw/s390x/ap-device.h"
> +
> +static void ap_class_init(ObjectClass *klass, void *data)
> +{
> +    DeviceClass *dc = DEVICE_CLASS(klass);
> +
> +    dc->desc = "AP device class";
> +    dc->hotpluggable = false;
> +}
> +
> +static const TypeInfo ap_device_info = {
> +    .name = AP_DEVICE_TYPE,
> +    .parent = TYPE_DEVICE,
> +    .instance_size = sizeof(APDevice),
> +    .class_size = sizeof(APDeviceClass),
> +    .class_init = ap_class_init,
> +    .abstract = true,
> +};
> +
> +static void ap_device_register(void)
> +{
> +    type_register_static(&ap_device_info);
> +}
> +
> +type_init(ap_device_register)
> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
> new file mode 100644
> index 0000000..693df90
> --- /dev/null
> +++ b/include/hw/s390x/ap-device.h
> @@ -0,0 +1,38 @@
> +/*
> + * Adjunct Processor (AP) matrix device interfaces
> + *
> + * Copyright 2018 IBM Corp.
> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +#ifndef HW_S390X_AP_DEVICE_H
> +#define HW_S390X_AP_DEVICE_H
> +
> +#define AP_DEVICE_TYPE       "ap-device"
> +
> +typedef struct APDevice {
> +    DeviceState parent_obj;
> +} APDevice;
> +
> +typedef struct APDeviceClass {
> +    DeviceClass parent_class;
> +} APDeviceClass;
> +
> +static inline APDevice *to_ap_dev(DeviceState *dev)
> +{
> +    return container_of(dev, APDevice, parent_obj);
> +}
> +
> +#define AP_DEVICE(obj) \
> +    OBJECT_CHECK(APDevice, (obj), AP_DEVICE_TYPE)
> +
> +#define AP_DEVICE_GET_CLASS(obj) \
> +    OBJECT_GET_CLASS(APDeviceClass, (obj), AP_DEVICE_TYPE)
> +
> +#define AP_DEVICE_CLASS(klass) \
> +    OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE)
> +
> +#endif /* HW_S390X_AP_DEVICE_H */

until here, LGTM

Does this next hunk belong to the patch?
> diff --git a/ui/keycodemapdb b/ui/keycodemapdb
> index 6b3d716..16e5b07 160000
> --- a/ui/keycodemapdb
> +++ b/ui/keycodemapdb
> @@ -1 +1 @@
> -Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
> +Subproject commit 16e5b0787687d8904dad2c026107409eb9bfcb95


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

Re: [Qemu-devel] [PATCH v4 2/5] s390x/ap: base Adjunct Processor (AP) object

[Tony Krowiak]

On 04/18/2018 05:16 AM, Pierre Morel wrote:
> On 15/04/2018 21:07, Tony Krowiak wrote:
>> This patch introduces the base object for an AP device.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>> ---
>>   hw/s390x/Makefile.objs       |    1 +
>>   hw/s390x/ap-device.c         |   39 
>> +++++++++++++++++++++++++++++++++++++++
>>   include/hw/s390x/ap-device.h |   38 
>> ++++++++++++++++++++++++++++++++++++++
>>   ui/keycodemapdb              |    2 +-
>>   4 files changed, 79 insertions(+), 1 deletions(-)
>>   create mode 100644 hw/s390x/ap-device.c
>>   create mode 100644 include/hw/s390x/ap-device.h
>>
>> diff --git a/hw/s390x/Makefile.objs b/hw/s390x/Makefile.objs
>> index dc704b5..3247a07 100644
>> --- a/hw/s390x/Makefile.objs
>> +++ b/hw/s390x/Makefile.objs
>> @@ -17,3 +17,4 @@ obj-y += s390-stattrib.o
>>   obj-$(CONFIG_KVM) += s390-skeys-kvm.o
>>   obj-$(CONFIG_KVM) += s390-stattrib-kvm.o
>>   obj-y += s390-ccw.o
>> +obj-y += ap-device.o
>> diff --git a/hw/s390x/ap-device.c b/hw/s390x/ap-device.c
>> new file mode 100644
>> index 0000000..3cd4bae
>> --- /dev/null
>> +++ b/hw/s390x/ap-device.c
>> @@ -0,0 +1,39 @@
>> +/*
>> + * Adjunct Processor (AP) matrix device
>> + *
>> + * Copyright 2018 IBM Corp.
>> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 
>> or (at
>> + * your option) any later version. See the COPYING file in the 
>> top-level
>> + * directory.
>> + */
>> +#include "qemu/osdep.h"
>> +#include "qemu/module.h"
>> +#include "qapi/error.h"
>> +#include "hw/qdev.h"
>> +#include "hw/s390x/ap-device.h"
>> +
>> +static void ap_class_init(ObjectClass *klass, void *data)
>> +{
>> +    DeviceClass *dc = DEVICE_CLASS(klass);
>> +
>> +    dc->desc = "AP device class";
>> +    dc->hotpluggable = false;
>> +}
>> +
>> +static const TypeInfo ap_device_info = {
>> +    .name = AP_DEVICE_TYPE,
>> +    .parent = TYPE_DEVICE,
>> +    .instance_size = sizeof(APDevice),
>> +    .class_size = sizeof(APDeviceClass),
>> +    .class_init = ap_class_init,
>> +    .abstract = true,
>> +};
>> +
>> +static void ap_device_register(void)
>> +{
>> +    type_register_static(&ap_device_info);
>> +}
>> +
>> +type_init(ap_device_register)
>> diff --git a/include/hw/s390x/ap-device.h b/include/hw/s390x/ap-device.h
>> new file mode 100644
>> index 0000000..693df90
>> --- /dev/null
>> +++ b/include/hw/s390x/ap-device.h
>> @@ -0,0 +1,38 @@
>> +/*
>> + * Adjunct Processor (AP) matrix device interfaces
>> + *
>> + * Copyright 2018 IBM Corp.
>> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 
>> or (at
>> + * your option) any later version. See the COPYING file in the 
>> top-level
>> + * directory.
>> + */
>> +#ifndef HW_S390X_AP_DEVICE_H
>> +#define HW_S390X_AP_DEVICE_H
>> +
>> +#define AP_DEVICE_TYPE       "ap-device"
>> +
>> +typedef struct APDevice {
>> +    DeviceState parent_obj;
>> +} APDevice;
>> +
>> +typedef struct APDeviceClass {
>> +    DeviceClass parent_class;
>> +} APDeviceClass;
>> +
>> +static inline APDevice *to_ap_dev(DeviceState *dev)
>> +{
>> +    return container_of(dev, APDevice, parent_obj);
>> +}
>> +
>> +#define AP_DEVICE(obj) \
>> +    OBJECT_CHECK(APDevice, (obj), AP_DEVICE_TYPE)
>> +
>> +#define AP_DEVICE_GET_CLASS(obj) \
>> +    OBJECT_GET_CLASS(APDeviceClass, (obj), AP_DEVICE_TYPE)
>> +
>> +#define AP_DEVICE_CLASS(klass) \
>> +    OBJECT_CLASS_CHECK(APDeviceClass, (klass), AP_DEVICE_TYPE)
>> +
>> +#endif /* HW_S390X_AP_DEVICE_H */
>
> until here, LGTM
>
> Does this next hunk belong to the patch?

No it doesn't. I had compile problems due to keycodemap errors which I 
fixed, but I
have no idea how this got in here. There is a v5 coming, I'll make sure 
this
is removed.

>> diff --git a/ui/keycodemapdb b/ui/keycodemapdb
>> index 6b3d716..16e5b07 160000
>> --- a/ui/keycodemapdb
>> +++ b/ui/keycodemapdb
>> @@ -1 +1 @@
>> -Subproject commit 6b3d716e2b6472eb7189d3220552280ef3d832ce
>> +Subproject commit 16e5b0787687d8904dad2c026107409eb9bfcb95
>
>

[Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

A new CPU model feature and two new CPU model facilities are
introduced to support AP devices for a KVM guest.

CPU model features:

1. The KVM_S390_VM_CPU_FEAT_AP CPU model feature indicates that
   AP facilities are installed. This feature will be enabled by
   the kernel only if the AP facilities are installed on the linux
   host. This feature must be turned on from userspace to access
   AP devices from the KVM guest. The QEMU command line to turn
   this feature looks something like this:

	qemu-system-s390x ... -cpu xxx,ap=on

CPU model facilities:

1. The S390_FEAT_AP_QUERY_CONFIG_INFO feature indicates the AP Query
   Configuration Information (QCI) facility is installed. This feature
   will be enabled by the kernel only if the QCI is installed on
   the host.

2. The S390_FEAT_AP_FACILITY_TEST feature indicates that the AP
   Facility Test (APFT) facility is installed. This feature will
   be enabled by the kernel only if the APFT facility is installed
   on the host.

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---
 target/s390x/cpu_features.c     |    3 +++
 target/s390x/cpu_features_def.h |    3 +++
 target/s390x/cpu_models.c       |    2 ++
 target/s390x/gen-features.c     |    3 +++
 target/s390x/kvm.c              |    1 +
 5 files changed, 12 insertions(+), 0 deletions(-)

diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
index 3b9e274..5ee3a2d 100644
--- a/target/s390x/cpu_features.c
+++ b/target/s390x/cpu_features.c
@@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
     FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
     FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
     FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
+    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
     FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
     FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
+    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
     FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
     FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
     FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
@@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
 
     FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
     FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
+    FEAT_INIT_MISC("ap", "AP facilities installed"),
 
     FEAT_INIT("plo-cl", S390_FEAT_TYPE_PLO, 0, "PLO Compare and load (32 bit in general registers)"),
     FEAT_INIT("plo-clg", S390_FEAT_TYPE_PLO, 1, "PLO Compare and load (64 bit in parameter list)"),
diff --git a/target/s390x/cpu_features_def.h b/target/s390x/cpu_features_def.h
index 7c5915c..8998b65 100644
--- a/target/s390x/cpu_features_def.h
+++ b/target/s390x/cpu_features_def.h
@@ -27,8 +27,10 @@ typedef enum {
     S390_FEAT_SENSE_RUNNING_STATUS,
     S390_FEAT_CONDITIONAL_SSKE,
     S390_FEAT_CONFIGURATION_TOPOLOGY,
+    S390_FEAT_AP_QUERY_CONFIG_INFO,
     S390_FEAT_IPTE_RANGE,
     S390_FEAT_NONQ_KEY_SETTING,
+    S390_FEAT_AP_FACILITIES_TEST,
     S390_FEAT_EXTENDED_TRANSLATION_2,
     S390_FEAT_MSA,
     S390_FEAT_LONG_DISPLACEMENT,
@@ -118,6 +120,7 @@ typedef enum {
     /* Misc */
     S390_FEAT_DAT_ENH_2,
     S390_FEAT_CMM,
+    S390_FEAT_AP,
 
     /* PLO */
     S390_FEAT_PLO_CL,
diff --git a/target/s390x/cpu_models.c b/target/s390x/cpu_models.c
index 2741b68..2ab59cd 100644
--- a/target/s390x/cpu_models.c
+++ b/target/s390x/cpu_models.c
@@ -770,6 +770,8 @@ static void check_consistency(const S390CPUModel *model)
         { S390_FEAT_PRNO_TRNG_QRTCR, S390_FEAT_MSA_EXT_5 },
         { S390_FEAT_PRNO_TRNG, S390_FEAT_MSA_EXT_5 },
         { S390_FEAT_SIE_KSS, S390_FEAT_SIE_F2 },
+        { S390_FEAT_AP_QUERY_CONFIG_INFO, S390_FEAT_AP },
+        { S390_FEAT_AP_FACILITIES_TEST, S390_FEAT_AP },
     };
     int i;
 
diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
index 0cdbc15..0d5b0f7 100644
--- a/target/s390x/gen-features.c
+++ b/target/s390x/gen-features.c
@@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
     S390_FEAT_ADAPTER_INT_SUPPRESSION,
     S390_FEAT_EDAT_2,
     S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
+    S390_FEAT_AP_QUERY_CONFIG_INFO,
+    S390_FEAT_AP_FACILITIES_TEST,
+    S390_FEAT_AP,
 };
 
 static uint16_t full_GEN12_GA2[] = {
diff --git a/target/s390x/kvm.c b/target/s390x/kvm.c
index fb59d92..8c251b4 100644
--- a/target/s390x/kvm.c
+++ b/target/s390x/kvm.c
@@ -2100,6 +2100,7 @@ static int kvm_to_feat[][2] = {
     { KVM_S390_VM_CPU_FEAT_PFMFI, S390_FEAT_SIE_PFMFI},
     { KVM_S390_VM_CPU_FEAT_SIGPIF, S390_FEAT_SIE_SIGPIF},
     { KVM_S390_VM_CPU_FEAT_KSS, S390_FEAT_SIE_KSS},
+    { KVM_S390_VM_CPU_FEAT_AP, S390_FEAT_AP},
 };
 
 static int query_cpu_feat(S390FeatBitmap features)
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[David Hildenbrand]

>  
> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> index 0cdbc15..0d5b0f7 100644
> --- a/target/s390x/gen-features.c
> +++ b/target/s390x/gen-features.c
> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
>      S390_FEAT_EDAT_2,
>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
> +    S390_FEAT_AP_FACILITIES_TEST,
> +    S390_FEAT_AP,
>  };
>  

Now I have to ask a very stupid question:

I heard that the execution controls in the SIE block for AP are one of
the oldest ones we have around. How can it be that the AP feature cannot
be used before zEC12?


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/16/2018 11:44 AM, David Hildenbrand wrote:
>>   
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 0cdbc15..0d5b0f7 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>       S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>       S390_FEAT_EDAT_2,
>>       S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>> +    S390_FEAT_AP_FACILITIES_TEST,
>> +    S390_FEAT_AP,
>>   };
>>   
> Now I have to ask a very stupid question:
>
> I heard that the execution controls in the SIE block for AP are one of
> the oldest ones we have around. How can it be that the AP feature cannot
> be used before zEC12?

Its not a stupid question, but I don't have an answer because this was
not a design decision I made. I will consult with the crypto team to
see if I can get you an answer.

>
>

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Christian Borntraeger]

On 04/16/2018 05:44 PM, David Hildenbrand wrote:
> 
>>  
>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>> index 0cdbc15..0d5b0f7 100644
>> --- a/target/s390x/gen-features.c
>> +++ b/target/s390x/gen-features.c
>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>      S390_FEAT_EDAT_2,
>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>> +    S390_FEAT_AP_FACILITIES_TEST,
>> +    S390_FEAT_AP,
>>  };
>>  
> 
> Now I have to ask a very stupid question:
> 
> I heard that the execution controls in the SIE block for AP are one of
> the oldest ones we have around. How can it be that the AP feature cannot
> be used before zEC12?

It was a suggestion from the crypto team due to testability. Nobody has a z196
with the older cards.

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Cornelia Huck]

On Tue, 17 Apr 2018 20:21:31 +0200
Christian Borntraeger <borntraeger@de.ibm.com> wrote:

> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
> >   
> >>  
> >> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
> >> index 0cdbc15..0d5b0f7 100644
> >> --- a/target/s390x/gen-features.c
> >> +++ b/target/s390x/gen-features.c
> >> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
> >>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
> >>      S390_FEAT_EDAT_2,
> >>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
> >> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
> >> +    S390_FEAT_AP_FACILITIES_TEST,
> >> +    S390_FEAT_AP,
> >>  };
> >>    
> > 
> > Now I have to ask a very stupid question:
> > 
> > I heard that the execution controls in the SIE block for AP are one of
> > the oldest ones we have around. How can it be that the AP feature cannot
> > be used before zEC12?  
> 
> It was a suggestion from the crypto team due to testability. Nobody has a z196
> with the older cards.
> 

Might be worth adding a note to that respect?

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[David Hildenbrand]

On 18.04.2018 09:40, Cornelia Huck wrote:
> On Tue, 17 Apr 2018 20:21:31 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
> 
>> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
>>>   
>>>>  
>>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>>> index 0cdbc15..0d5b0f7 100644
>>>> --- a/target/s390x/gen-features.c
>>>> +++ b/target/s390x/gen-features.c
>>>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>>>      S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>>>      S390_FEAT_EDAT_2,
>>>>      S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>>>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>>>> +    S390_FEAT_AP_FACILITIES_TEST,
>>>> +    S390_FEAT_AP,
>>>>  };
>>>>    
>>>
>>> Now I have to ask a very stupid question:
>>>
>>> I heard that the execution controls in the SIE block for AP are one of
>>> the oldest ones we have around. How can it be that the AP feature cannot
>>> be used before zEC12?  
>>
>> It was a suggestion from the crypto team due to testability. Nobody has a z196
>> with the older cards.
>>
> 
> Might be worth adding a note to that respect?
> 

We used to have the CPU model stick as close as possible to the real CPU
models.

Support statements should cover in specific products what is expected to
work and what not.

So is there any real (!support statement / !testability) reason to not
allow this feature on older CPU models that also had support for it?

-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/18/2018 04:59 AM, David Hildenbrand wrote:
> On 18.04.2018 09:40, Cornelia Huck wrote:
>> On Tue, 17 Apr 2018 20:21:31 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
>>>>    
>>>>>   
>>>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>>>> index 0cdbc15..0d5b0f7 100644
>>>>> --- a/target/s390x/gen-features.c
>>>>> +++ b/target/s390x/gen-features.c
>>>>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>>>>       S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>>>>       S390_FEAT_EDAT_2,
>>>>>       S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>>>>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>>>>> +    S390_FEAT_AP_FACILITIES_TEST,
>>>>> +    S390_FEAT_AP,
>>>>>   };
>>>>>     
>>>> Now I have to ask a very stupid question:
>>>>
>>>> I heard that the execution controls in the SIE block for AP are one of
>>>> the oldest ones we have around. How can it be that the AP feature cannot
>>>> be used before zEC12?
>>> It was a suggestion from the crypto team due to testability. Nobody has a z196
>>> with the older cards.
>>>
>> Might be worth adding a note to that respect?
>>
> We used to have the CPU model stick as close as possible to the real CPU
> models.
>
> Support statements should cover in specific products what is expected to
> work and what not.
>
> So is there any real (!support statement / !testability) reason to not
> allow this feature on older CPU models that also had support for it?

I'll discuss it with the team and get back to you.

>

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/18/2018 04:59 AM, David Hildenbrand wrote:
> On 18.04.2018 09:40, Cornelia Huck wrote:
>> On Tue, 17 Apr 2018 20:21:31 +0200
>> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>>
>>> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
>>>>    
>>>>>   
>>>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>>>> index 0cdbc15..0d5b0f7 100644
>>>>> --- a/target/s390x/gen-features.c
>>>>> +++ b/target/s390x/gen-features.c
>>>>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>>>>       S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>>>>       S390_FEAT_EDAT_2,
>>>>>       S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>>>>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>>>>> +    S390_FEAT_AP_FACILITIES_TEST,
>>>>> +    S390_FEAT_AP,
>>>>>   };
>>>>>     
>>>> Now I have to ask a very stupid question:
>>>>
>>>> I heard that the execution controls in the SIE block for AP are one of
>>>> the oldest ones we have around. How can it be that the AP feature cannot
>>>> be used before zEC12?
>>> It was a suggestion from the crypto team due to testability. Nobody has a z196
>>> with the older cards.
>>>
>> Might be worth adding a note to that respect?
>>
> We used to have the CPU model stick as close as possible to the real CPU
> models.
>
> Support statements should cover in specific products what is expected to
> work and what not.
>
> So is there any real (!support statement / !testability) reason to not
> allow this feature on older CPU models that also had support for it?

This is a business decision based on the following factors:
1. We do not have access to the older cards/systems for testing of the 
older devices.
2. Pre-CEX2 cards are no longer supported in the kernel with others to 
follow.
3. Keep the code base as small as possible and ensure it can be 
adequately tested.
4. Business investment priorities - i.e., $$$$'s.

>

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/18/2018 03:40 AM, Cornelia Huck wrote:
> On Tue, 17 Apr 2018 20:21:31 +0200
> Christian Borntraeger <borntraeger@de.ibm.com> wrote:
>
>> On 04/16/2018 05:44 PM, David Hildenbrand wrote:
>>>    
>>>>   
>>>> diff --git a/target/s390x/gen-features.c b/target/s390x/gen-features.c
>>>> index 0cdbc15..0d5b0f7 100644
>>>> --- a/target/s390x/gen-features.c
>>>> +++ b/target/s390x/gen-features.c
>>>> @@ -447,6 +447,9 @@ static uint16_t full_GEN12_GA1[] = {
>>>>       S390_FEAT_ADAPTER_INT_SUPPRESSION,
>>>>       S390_FEAT_EDAT_2,
>>>>       S390_FEAT_SIDE_EFFECT_ACCESS_ESOP2,
>>>> +    S390_FEAT_AP_QUERY_CONFIG_INFO,
>>>> +    S390_FEAT_AP_FACILITIES_TEST,
>>>> +    S390_FEAT_AP,
>>>>   };
>>>>     
>>> Now I have to ask a very stupid question:
>>>
>>> I heard that the execution controls in the SIE block for AP are one of
>>> the oldest ones we have around. How can it be that the AP feature cannot
>>> be used before zEC12?
>> It was a suggestion from the crypto team due to testability. Nobody has a z196
>> with the older cards.
>>
> Might be worth adding a note to that respect?

I will.

>

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Halil Pasic]

On 04/15/2018 09:07 PM, Tony Krowiak wrote:
> A new CPU model feature and two new CPU model facilities are
> introduced to support AP devices for a KVM guest.
[..]
> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
> index 3b9e274..5ee3a2d 100644
> --- a/target/s390x/cpu_features.c
> +++ b/target/s390x/cpu_features.c
> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>       FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>       FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>       FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),

Why did you change this form qci to apqci. Too may people found the
qci good?

>       FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>       FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>       FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>       FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>       FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
> 
>       FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>       FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
> +    FEAT_INIT_MISC("ap", "AP facilities installed"),

Why plural ('facilities')? Would not s/facilities/instructions be more end-user
friendly?

Regards,
Halil

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[David Hildenbrand]

On 18.04.2018 12:55, Halil Pasic wrote:
> 
> 
> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>> A new CPU model feature and two new CPU model facilities are
>> introduced to support AP devices for a KVM guest.
> [..]
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 3b9e274..5ee3a2d 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>       FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>>       FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>>       FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
> 
> Why did you change this form qci to apqci. Too may people found the
> qci good?

If the facility is called "Query AP Configuration facility" it should be
qapc

Where does the term "qci" come from ?

> 
>>       FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>>       FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>>       FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>>       FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>>       FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>
>>       FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>       FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
> 
> Why plural ('facilities')? Would not s/facilities/instructions be more end-user
> friendly?
> 
> Regards,
> Halil
> 


-- 

Thanks,

David / dhildenb

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Pierre Morel]

On 18/04/2018 13:03, David Hildenbrand wrote:
> On 18.04.2018 12:55, Halil Pasic wrote:
>>
>> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>>> A new CPU model feature and two new CPU model facilities are
>>> introduced to support AP devices for a KVM guest.
>> [..]
>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>> index 3b9e274..5ee3a2d 100644
>>> --- a/target/s390x/cpu_features.c
>>> +++ b/target/s390x/cpu_features.c
>>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>>        FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>>>        FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>>>        FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
>>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
>> Why did you change this form qci to apqci. Too may people found the
>> qci good?
> If the facility is called "Query AP Configuration facility" it should be
> qapc
>
> Where does the term "qci" come from ?

Query Configuration Information

:)


>
>>>        FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>>>        FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
>>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>>>        FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>>>        FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>>>        FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
>>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>>
>>>        FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>>        FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>> Why plural ('facilities')? Would not s/facilities/instructions be more end-user
>> friendly?
>>
>> Regards,
>> Halil
>>
>

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

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/18/2018 07:03 AM, David Hildenbrand wrote:
> On 18.04.2018 12:55, Halil Pasic wrote:
>>
>> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>>> A new CPU model feature and two new CPU model facilities are
>>> introduced to support AP devices for a KVM guest.
>> [..]
>>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>>> index 3b9e274..5ee3a2d 100644
>>> --- a/target/s390x/cpu_features.c
>>> +++ b/target/s390x/cpu_features.c
>>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>>        FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status facility"),
>>>        FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE facility"),
>>>        FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, "Configuration-topology facility"),
>>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
>> Why did you change this form qci to apqci. Too may people found the
>> qci good?
> If the facility is called "Query AP Configuration facility" it should be
> qapc
>
> Where does the term "qci" come from ?

It comes from the name of the function Query AP Configuration 
Information which
is identified by the initials QCI.

>
>>>        FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range facility"),
>>>        FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing key-setting facility"),
>>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),
>>>        FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, "Extended-translation facility 2"),
>>>        FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, "Message-security-assist facility (excluding subfunctions)"),
>>>        FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement facility"),
>>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>>
>>>        FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>>        FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>> Why plural ('facilities')? Would not s/facilities/instructions be more end-user
>> friendly?
>>
>> Regards,
>> Halil
>>
>

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/18/2018 06:55 AM, Halil Pasic wrote:
>
>
> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>> A new CPU model feature and two new CPU model facilities are
>> introduced to support AP devices for a KVM guest.
> [..]
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 3b9e274..5ee3a2d 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>       FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status 
>> facility"),
>>       FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE 
>> facility"),
>>       FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, 
>> "Configuration-topology facility"),
>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP 
>> Configuration facility"),
>
> Why did you change this form qci to apqci. Too may people found the
> qci good?
>
>>       FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range 
>> facility"),
>>       FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing 
>> key-setting facility"),
>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor 
>> Facilities Test facility"),
>>       FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, 
>> "Extended-translation facility 2"),
>>       FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, 
>> "Message-security-assist facility (excluding subfunctions)"),
>>       FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement 
>> facility"),
>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>
>>       FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>       FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>
> Why plural ('facilities')? Would not s/facilities/instructions be more 
> end-user
> friendly?

It's a matter of opinion. I prefer facilities because AP is comprised of 
not only
instructions, but also AP processors.

>
>
> Regards,
> Halil

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Halil Pasic]

On 04/22/2018 05:43 PM, Tony Krowiak wrote:
>>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>>
>> Why plural ('facilities')? Would not s/facilities/instructions be more end-user
>> friendly?
> 
> It's a matter of opinion. I prefer facilities because AP is comprised of not only
> instructions, but also AP processors.

Please elaborate. You mean processors like AP cards? If yes what if the matrix is
empty (e.g. the state we decided is the default when no further action is taken
(assign queues to the vfio-ap kernel driver, set up an mdev, -device vfio-ap on
qemu cmd line))?

I just wanted to point out that this plural is very vague. Not speaking about that
QCI should be an AP facility too, but is not included in this 'features' and is not
covered by this cpu model feature. It has it's own cpu model feature and even a
dedicated STFLE bit.

Regards,
Halil

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/22/2018 12:01 PM, Halil Pasic wrote:
>
>
> On 04/22/2018 05:43 PM, Tony Krowiak wrote:
>>>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>>>
>>> Why plural ('facilities')? Would not s/facilities/instructions be 
>>> more end-user
>>> friendly?
>>
>> It's a matter of opinion. I prefer facilities because AP is comprised 
>> of not only
>> instructions, but also AP processors.
>
> Please elaborate. You mean processors like AP cards? If yes what if 
> the matrix is
> empty (e.g. the state we decided is the default when no further action 
> is taken
> (assign queues to the vfio-ap kernel driver, set up an mdev, -device 
> vfio-ap on
> qemu cmd line))?

I don't want to get into a debate about this. If it means that much to 
you, I'll
go ahead and change it.

>
>
> I just wanted to point out that this plural is very vague. Not 
> speaking about that
> QCI should be an AP facility too, but is not included in this 
> 'features' and is not
> covered by this cpu model feature. It has it's own cpu model feature 
> and even a
> dedicated STFLE bit.

Uncle .... I got it.

>
>
> Regards,
> Halil

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/22/2018 12:01 PM, Halil Pasic wrote:
>
>
> On 04/22/2018 05:43 PM, Tony Krowiak wrote:
>>>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>>>
>>> Why plural ('facilities')? Would not s/facilities/instructions be 
>>> more end-user
>>> friendly?
>>
>> It's a matter of opinion. I prefer facilities because AP is comprised 
>> of not only
>> instructions, but also AP processors.
>
> Please elaborate. You mean processors like AP cards? If yes what if 
> the matrix is
> empty (e.g. the state we decided is the default when no further action 
> is taken
> (assign queues to the vfio-ap kernel driver, set up an mdev, -device 
> vfio-ap on
> qemu cmd line))?
>
> I just wanted to point out that this plural is very vague. Not 
> speaking about that
> QCI should be an AP facility too, but is not included in this 
> 'features' and is not
> covered by this cpu model feature. It has it's own cpu model feature 
> and even a
> dedicated STFLE bit.

FYI, I decided to take your suggestion and go with 'AP instructions 
installed'.

>
>
> Regards,
> Halil

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Tony Krowiak]

On 04/18/2018 06:55 AM, Halil Pasic wrote:
>
>
> On 04/15/2018 09:07 PM, Tony Krowiak wrote:
>> A new CPU model feature and two new CPU model facilities are
>> introduced to support AP devices for a KVM guest.
> [..]
>> diff --git a/target/s390x/cpu_features.c b/target/s390x/cpu_features.c
>> index 3b9e274..5ee3a2d 100644
>> --- a/target/s390x/cpu_features.c
>> +++ b/target/s390x/cpu_features.c
>> @@ -40,8 +40,10 @@ static const S390FeatDef s390_features[] = {
>>       FEAT_INIT("srs", S390_FEAT_TYPE_STFL, 9, "Sense-running-status 
>> facility"),
>>       FEAT_INIT("csske", S390_FEAT_TYPE_STFL, 10, "Conditional-SSKE 
>> facility"),
>>       FEAT_INIT("ctop", S390_FEAT_TYPE_STFL, 11, 
>> "Configuration-topology facility"),
>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP 
>> Configuration facility"),
>
> Why did you change this form qci to apqci. Too may people found the
> qci good?

I changed it by request and I thought it made sense to do so. Maybe we 
should just
take a vote.

>
>
>>       FEAT_INIT("ipter", S390_FEAT_TYPE_STFL, 13, "IPTE-range 
>> facility"),
>>       FEAT_INIT("nonqks", S390_FEAT_TYPE_STFL, 14, "Nonquiescing 
>> key-setting facility"),
>> +    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor 
>> Facilities Test facility"),
>>       FEAT_INIT("etf2", S390_FEAT_TYPE_STFL, 16, 
>> "Extended-translation facility 2"),
>>       FEAT_INIT("msa-base", S390_FEAT_TYPE_STFL, 17, 
>> "Message-security-assist facility (excluding subfunctions)"),
>>       FEAT_INIT("ldisp", S390_FEAT_TYPE_STFL, 18, "Long-displacement 
>> facility"),
>> @@ -129,6 +131,7 @@ static const S390FeatDef s390_features[] = {
>>
>>       FEAT_INIT_MISC("dateh2", "DAT-enhancement facility 2"),
>>       FEAT_INIT_MISC("cmm", "Collaborative-memory-management facility"),
>> +    FEAT_INIT_MISC("ap", "AP facilities installed"),
>
> Why plural ('facilities')? Would not s/facilities/instructions be more 
> end-user
> friendly?
>
> Regards,
> Halil

Re: [Qemu-devel] [PATCH v4 3/5] s390x/cpumodel: Set up CPU model for AP device support

[Halil Pasic]

On 04/22/2018 05:52 PM, Tony Krowiak wrote:
>>> +    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),
>>
>> Why did you change this form qci to apqci. Too may people found the
>> qci good?
> 
> I changed it by request and I thought it made sense to do so. Maybe we should just
> take a vote.

Sorry I've missed that request. Could you give me a link into the mail archive?
I would like to examine the discussion.

Thanks,
Halil

[Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Tony Krowiak]

Introduces a VFIO based AP device. The device is defined via
the QEMU command line by specifying:

    -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>

There may be only one vfio-ap device configured for a guest.

The mediated matrix device is created by the VFIO AP device
driver by writing a UUID to a sysfs attribute file (see
docs/vfio-ap.txt). The mediated matrix device will be named
after the UUID. Symbolic links to the $uuid are created in
many places, so the path to the mediated matrix device $uuid
can be specified in any of the following ways:

/sys/devices/vfio_ap/matrix/$uuid
/sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
/sys/bus/mdev/devices/$uuid
/sys/bus/mdev/drivers/vfio_mdev/$uuid

When the vfio-ap device is realized, it acquires and opens the
VFIO iommu group to which the mediated matrix device is
bound. This causes a VFIO group notification event to be
signaled. The vfio_ap device driver's group notification
handler will get called at which time the device driver
will configure the the AP devices to which the guest will
be granted access.

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---
 default-configs/s390x-softmmu.mak |    1 +
 hw/vfio/Makefile.objs             |    1 +
 hw/vfio/ap.c                      |  191 +++++++++++++++++++++++++++++++++++++
 include/hw/vfio/vfio-common.h     |    1 +
 4 files changed, 194 insertions(+), 0 deletions(-)
 create mode 100644 hw/vfio/ap.c

diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
index 2f4bfe7..0b784b6 100644
--- a/default-configs/s390x-softmmu.mak
+++ b/default-configs/s390x-softmmu.mak
@@ -9,3 +9,4 @@ CONFIG_S390_FLIC=y
 CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
 CONFIG_VFIO_CCW=$(CONFIG_LINUX)
 CONFIG_WDT_DIAG288=y
+CONFIG_VFIO_AP=$(CONFIG_LINUX)
diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
index a2e7a0a..8b3f664 100644
--- a/hw/vfio/Makefile.objs
+++ b/hw/vfio/Makefile.objs
@@ -6,4 +6,5 @@ obj-$(CONFIG_SOFTMMU) += platform.o
 obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
 obj-$(CONFIG_VFIO_AMD_XGBE) += amd-xgbe.o
 obj-$(CONFIG_SOFTMMU) += spapr.o
+obj-$(CONFIG_VFIO_AP) += ap.o
 endif
diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
new file mode 100644
index 0000000..7b48a3a
--- /dev/null
+++ b/hw/vfio/ap.c
@@ -0,0 +1,191 @@
+/*
+ * VFIO based AP matrix device assignment
+ *
+ * Copyright 2018 IBM Corp.
+ * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or (at
+ * your option) any later version. See the COPYING file in the top-level
+ * directory.
+ */
+
+#include <linux/vfio.h>
+#include <sys/ioctl.h>
+#include "qemu/osdep.h"
+#include "qapi/error.h"
+#include "hw/sysbus.h"
+#include "hw/vfio/vfio.h"
+#include "hw/vfio/vfio-common.h"
+#include "hw/s390x/ap-device.h"
+#include "qemu/error-report.h"
+#include "qemu/queue.h"
+#include "qemu/option.h"
+#include "qemu/config-file.h"
+#include "cpu.h"
+#include "kvm_s390x.h"
+#include "sysemu/sysemu.h"
+
+#define VFIO_AP_DEVICE_TYPE      "vfio-ap"
+
+typedef struct VFIOAPDevice {
+    APDevice apdev;
+    VFIODevice vdev;
+    QTAILQ_ENTRY(VFIOAPDevice) sibling;
+} VFIOAPDevice;
+
+VFIOAPDevice *vfio_apdev;
+
+static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
+{
+    vdev->needs_reset = false;
+}
+
+/*
+ * We don't need vfio_hot_reset_multi and vfio_eoi operations for
+ * vfio-ap-matrix device now.
+ */
+struct VFIODeviceOps vfio_ap_ops = {
+    .vfio_compute_needs_reset = vfio_ap_compute_needs_reset,
+};
+
+static void vfio_put_device(VFIOAPDevice *vapdev)
+{
+    g_free(vapdev->vdev.name);
+    vfio_put_base_device(&vapdev->vdev);
+}
+
+static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
+{
+    char *tmp, group_path[PATH_MAX];
+    ssize_t len;
+    int groupid;
+
+    tmp = g_strdup_printf("%s/iommu_group", vapdev->vdev.sysfsdev);
+    len = readlink(tmp, group_path, sizeof(group_path));
+    g_free(tmp);
+
+    if (len <= 0 || len >= sizeof(group_path)) {
+        error_setg(errp, "%s: no iommu_group found for %s",
+                   VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev);
+        return NULL;
+    }
+
+    group_path[len] = 0;
+
+    if (sscanf(basename(group_path), "%d", &groupid) != 1) {
+        error_setg(errp, "vfio: failed to read %s", group_path);
+        return NULL;
+    }
+
+    return vfio_get_group(groupid, &address_space_memory, errp);
+}
+
+static void vfio_ap_realize(DeviceState *dev, Error **errp)
+{
+    VFIODevice *vbasedev;
+    VFIOGroup *vfio_group;
+    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
+    char *mdevid;
+    Error *local_err = NULL;
+    int ret;
+
+    /*
+     * Since a guest's matrix is configured in its entirety by the mediated
+     * matrix device and hot plug is not currently supported, there is no
+     * need to have more than one vfio-ap device. Check if a vfio-ap device
+     * has already been defined.
+     */
+    if (vfio_apdev) {
+        error_setg(&local_err, "Only one %s device is allowed",
+                   VFIO_AP_DEVICE_TYPE);
+        goto out_err;
+    }
+
+    if (!s390_has_feat(S390_FEAT_AP)) {
+        error_setg(&local_err, "AP support not enabled");
+        goto out_err;
+    }
+
+    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
+
+    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
+    if (!vfio_group) {
+        goto out_err;
+    }
+
+    vfio_apdev->vdev.ops = &vfio_ap_ops;
+    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
+    mdevid = basename(vfio_apdev->vdev.sysfsdev);
+    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
+    vfio_apdev->vdev.dev = dev;
+    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {
+        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
+            error_setg(&local_err,
+                       "%s: AP device %s has already been realized",
+                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
+            goto out_device_err;
+        }
+    }
+
+    ret = vfio_get_device(vfio_group, mdevid, &vfio_apdev->vdev, &local_err);
+    if (ret) {
+        goto out_device_err;
+    }
+
+    return;
+
+
+out_device_err:
+    vfio_put_group(vfio_group);
+out_err:
+    vfio_apdev = NULL;
+    error_propagate(errp, local_err);
+}
+
+static void vfio_ap_unrealize(DeviceState *dev, Error **errp)
+{
+    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
+    VFIOAPDevice *vapdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
+    VFIOGroup *group = vapdev->vdev.group;
+
+    vfio_put_device(vapdev);
+    vfio_put_group(group);
+    vfio_apdev = NULL;
+}
+
+static Property vfio_ap_properties[] = {
+    DEFINE_PROP_STRING("sysfsdev", VFIOAPDevice, vdev.sysfsdev),
+    DEFINE_PROP_END_OF_LIST(),
+};
+
+static const VMStateDescription vfio_ap_vmstate = {
+    .name = VFIO_AP_DEVICE_TYPE,
+    .unmigratable = 1,
+};
+
+static void vfio_ap_class_init(ObjectClass *klass, void *data)
+{
+    DeviceClass *dc = DEVICE_CLASS(klass);
+
+    dc->props = vfio_ap_properties;
+    dc->vmsd = &vfio_ap_vmstate;
+    dc->desc = "VFIO-based AP device assignment";
+    dc->realize = vfio_ap_realize;
+    dc->unrealize = vfio_ap_unrealize;
+    dc->hotpluggable = false;
+}
+
+static const TypeInfo vfio_ap_info = {
+    .name = VFIO_AP_DEVICE_TYPE,
+    .parent = AP_DEVICE_TYPE,
+    .instance_size = sizeof(VFIOAPDevice),
+    .class_init = vfio_ap_class_init,
+};
+
+static void vfio_ap_type_init(void)
+{
+    type_register_static(&vfio_ap_info);
+    vfio_apdev = NULL;
+}
+
+type_init(vfio_ap_type_init)
diff --git a/include/hw/vfio/vfio-common.h b/include/hw/vfio/vfio-common.h
index d936014..f29df6e 100644
--- a/include/hw/vfio/vfio-common.h
+++ b/include/hw/vfio/vfio-common.h
@@ -47,6 +47,7 @@ enum {
     VFIO_DEVICE_TYPE_PCI = 0,
     VFIO_DEVICE_TYPE_PLATFORM = 1,
     VFIO_DEVICE_TYPE_CCW = 2,
+    VFIO_DEVICE_TYPE_AP = 3,
 };
 
 typedef struct VFIOMmap {
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Pierre Morel]

On 15/04/2018 21:07, Tony Krowiak wrote:
> Introduces a VFIO based AP device. The device is defined via
> the QEMU command line by specifying:
>
>      -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
>
> There may be only one vfio-ap device configured for a guest.
>
> The mediated matrix device is created by the VFIO AP device
> driver by writing a UUID to a sysfs attribute file (see
> docs/vfio-ap.txt). The mediated matrix device will be named
> after the UUID. Symbolic links to the $uuid are created in
> many places, so the path to the mediated matrix device $uuid
> can be specified in any of the following ways:
>
> /sys/devices/vfio_ap/matrix/$uuid
> /sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
> /sys/bus/mdev/devices/$uuid
> /sys/bus/mdev/drivers/vfio_mdev/$uuid
>
> When the vfio-ap device is realized, it acquires and opens the
> VFIO iommu group to which the mediated matrix device is
> bound. This causes a VFIO group notification event to be
> signaled. The vfio_ap device driver's group notification
> handler will get called at which time the device driver
> will configure the the AP devices to which the guest will
> be granted access.
>
> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> ---
>   default-configs/s390x-softmmu.mak |    1 +
>   hw/vfio/Makefile.objs             |    1 +
>   hw/vfio/ap.c                      |  191 +++++++++++++++++++++++++++++++++++++
>   include/hw/vfio/vfio-common.h     |    1 +
>   4 files changed, 194 insertions(+), 0 deletions(-)
>   create mode 100644 hw/vfio/ap.c
>
> diff --git a/default-configs/s390x-softmmu.mak b/default-configs/s390x-softmmu.mak
> index 2f4bfe7..0b784b6 100644
> --- a/default-configs/s390x-softmmu.mak
> +++ b/default-configs/s390x-softmmu.mak
> @@ -9,3 +9,4 @@ CONFIG_S390_FLIC=y
>   CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
>   CONFIG_VFIO_CCW=$(CONFIG_LINUX)
>   CONFIG_WDT_DIAG288=y
> +CONFIG_VFIO_AP=$(CONFIG_LINUX)
> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
> index a2e7a0a..8b3f664 100644
> --- a/hw/vfio/Makefile.objs
> +++ b/hw/vfio/Makefile.objs
> @@ -6,4 +6,5 @@ obj-$(CONFIG_SOFTMMU) += platform.o
>   obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
>   obj-$(CONFIG_VFIO_AMD_XGBE) += amd-xgbe.o
>   obj-$(CONFIG_SOFTMMU) += spapr.o
> +obj-$(CONFIG_VFIO_AP) += ap.o
>   endif
> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
> new file mode 100644
> index 0000000..7b48a3a
> --- /dev/null
> +++ b/hw/vfio/ap.c
> @@ -0,0 +1,191 @@
> +/*
> + * VFIO based AP matrix device assignment
> + *
> + * Copyright 2018 IBM Corp.
> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> + *
> + * This work is licensed under the terms of the GNU GPL, version 2 or (at
> + * your option) any later version. See the COPYING file in the top-level
> + * directory.
> + */
> +
> +#include <linux/vfio.h>
> +#include <sys/ioctl.h>
> +#include "qemu/osdep.h"
> +#include "qapi/error.h"
> +#include "hw/sysbus.h"
> +#include "hw/vfio/vfio.h"
> +#include "hw/vfio/vfio-common.h"
> +#include "hw/s390x/ap-device.h"
> +#include "qemu/error-report.h"
> +#include "qemu/queue.h"
> +#include "qemu/option.h"
> +#include "qemu/config-file.h"
> +#include "cpu.h"
> +#include "kvm_s390x.h"
> +#include "sysemu/sysemu.h"
> +
> +#define VFIO_AP_DEVICE_TYPE      "vfio-ap"
> +
> +typedef struct VFIOAPDevice {
> +    APDevice apdev;
> +    VFIODevice vdev;
> +    QTAILQ_ENTRY(VFIOAPDevice) sibling;
> +} VFIOAPDevice;
> +
> +VFIOAPDevice *vfio_apdev;
> +
> +static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
> +{
> +    vdev->needs_reset = false;
> +}
> +
> +/*
> + * We don't need vfio_hot_reset_multi and vfio_eoi operations for
> + * vfio-ap-matrix device now.
> + */
> +struct VFIODeviceOps vfio_ap_ops = {
> +    .vfio_compute_needs_reset = vfio_ap_compute_needs_reset,
> +};
> +
> +static void vfio_put_device(VFIOAPDevice *vapdev)
> +{
> +    g_free(vapdev->vdev.name);
> +    vfio_put_base_device(&vapdev->vdev);
> +}
> +
> +static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
> +{
> +    char *tmp, group_path[PATH_MAX];
> +    ssize_t len;
> +    int groupid;
> +
> +    tmp = g_strdup_printf("%s/iommu_group", vapdev->vdev.sysfsdev);
> +    len = readlink(tmp, group_path, sizeof(group_path));
> +    g_free(tmp);
> +
> +    if (len <= 0 || len >= sizeof(group_path)) {
> +        error_setg(errp, "%s: no iommu_group found for %s",
> +                   VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev);
> +        return NULL;
> +    }
> +
> +    group_path[len] = 0;
> +
> +    if (sscanf(basename(group_path), "%d", &groupid) != 1) {
> +        error_setg(errp, "vfio: failed to read %s", group_path);
> +        return NULL;
> +    }
> +
> +    return vfio_get_group(groupid, &address_space_memory, errp);
> +}
> +
> +static void vfio_ap_realize(DeviceState *dev, Error **errp)
> +{
> +    VFIODevice *vbasedev;
> +    VFIOGroup *vfio_group;
> +    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
> +    char *mdevid;
> +    Error *local_err = NULL;
> +    int ret;
> +
> +    /*
> +     * Since a guest's matrix is configured in its entirety by the mediated
> +     * matrix device and hot plug is not currently supported, there is no
> +     * need to have more than one vfio-ap device. Check if a vfio-ap device
> +     * has already been defined.
> +     */
> +    if (vfio_apdev) {
> +        error_setg(&local_err, "Only one %s device is allowed",
> +                   VFIO_AP_DEVICE_TYPE);
> +        goto out_err;
> +    }
> +
> +    if (!s390_has_feat(S390_FEAT_AP)) {
> +        error_setg(&local_err, "AP support not enabled");
> +        goto out_err;
> +    }
> +
> +    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
> +
> +    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
> +    if (!vfio_group) {
> +        goto out_err;
> +    }
> +
> +    vfio_apdev->vdev.ops = &vfio_ap_ops;
> +    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
> +    mdevid = basename(vfio_apdev->vdev.sysfsdev);
> +    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
> +    vfio_apdev->vdev.dev = dev;
> +    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {

can this happen if you have only one device ?

> +        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
> +            error_setg(&local_err,
> +                       "%s: AP device %s has already been realized",
> +                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
> +            goto out_device_err;
> +        }
> +    }
> +
> +
...snip...

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

Re: [Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Tony Krowiak]

On 04/18/2018 05:11 AM, Pierre Morel wrote:
> On 15/04/2018 21:07, Tony Krowiak wrote:
>> Introduces a VFIO based AP device. The device is defined via
>> the QEMU command line by specifying:
>>
>>      -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
>>
>> There may be only one vfio-ap device configured for a guest.
>>
>> The mediated matrix device is created by the VFIO AP device
>> driver by writing a UUID to a sysfs attribute file (see
>> docs/vfio-ap.txt). The mediated matrix device will be named
>> after the UUID. Symbolic links to the $uuid are created in
>> many places, so the path to the mediated matrix device $uuid
>> can be specified in any of the following ways:
>>
>> /sys/devices/vfio_ap/matrix/$uuid
>> /sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid 
>>
>> /sys/bus/mdev/devices/$uuid
>> /sys/bus/mdev/drivers/vfio_mdev/$uuid
>>
>> When the vfio-ap device is realized, it acquires and opens the
>> VFIO iommu group to which the mediated matrix device is
>> bound. This causes a VFIO group notification event to be
>> signaled. The vfio_ap device driver's group notification
>> handler will get called at which time the device driver
>> will configure the the AP devices to which the guest will
>> be granted access.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>> ---
>>   default-configs/s390x-softmmu.mak |    1 +
>>   hw/vfio/Makefile.objs             |    1 +
>>   hw/vfio/ap.c                      |  191 
>> +++++++++++++++++++++++++++++++++++++
>>   include/hw/vfio/vfio-common.h     |    1 +
>>   4 files changed, 194 insertions(+), 0 deletions(-)
>>   create mode 100644 hw/vfio/ap.c
>>
>> diff --git a/default-configs/s390x-softmmu.mak 
>> b/default-configs/s390x-softmmu.mak
>> index 2f4bfe7..0b784b6 100644
>> --- a/default-configs/s390x-softmmu.mak
>> +++ b/default-configs/s390x-softmmu.mak
>> @@ -9,3 +9,4 @@ CONFIG_S390_FLIC=y
>>   CONFIG_S390_FLIC_KVM=$(CONFIG_KVM)
>>   CONFIG_VFIO_CCW=$(CONFIG_LINUX)
>>   CONFIG_WDT_DIAG288=y
>> +CONFIG_VFIO_AP=$(CONFIG_LINUX)
>> diff --git a/hw/vfio/Makefile.objs b/hw/vfio/Makefile.objs
>> index a2e7a0a..8b3f664 100644
>> --- a/hw/vfio/Makefile.objs
>> +++ b/hw/vfio/Makefile.objs
>> @@ -6,4 +6,5 @@ obj-$(CONFIG_SOFTMMU) += platform.o
>>   obj-$(CONFIG_VFIO_XGMAC) += calxeda-xgmac.o
>>   obj-$(CONFIG_VFIO_AMD_XGBE) += amd-xgbe.o
>>   obj-$(CONFIG_SOFTMMU) += spapr.o
>> +obj-$(CONFIG_VFIO_AP) += ap.o
>>   endif
>> diff --git a/hw/vfio/ap.c b/hw/vfio/ap.c
>> new file mode 100644
>> index 0000000..7b48a3a
>> --- /dev/null
>> +++ b/hw/vfio/ap.c
>> @@ -0,0 +1,191 @@
>> +/*
>> + * VFIO based AP matrix device assignment
>> + *
>> + * Copyright 2018 IBM Corp.
>> + * Author(s): Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>> + *
>> + * This work is licensed under the terms of the GNU GPL, version 2 
>> or (at
>> + * your option) any later version. See the COPYING file in the 
>> top-level
>> + * directory.
>> + */
>> +
>> +#include <linux/vfio.h>
>> +#include <sys/ioctl.h>
>> +#include "qemu/osdep.h"
>> +#include "qapi/error.h"
>> +#include "hw/sysbus.h"
>> +#include "hw/vfio/vfio.h"
>> +#include "hw/vfio/vfio-common.h"
>> +#include "hw/s390x/ap-device.h"
>> +#include "qemu/error-report.h"
>> +#include "qemu/queue.h"
>> +#include "qemu/option.h"
>> +#include "qemu/config-file.h"
>> +#include "cpu.h"
>> +#include "kvm_s390x.h"
>> +#include "sysemu/sysemu.h"
>> +
>> +#define VFIO_AP_DEVICE_TYPE      "vfio-ap"
>> +
>> +typedef struct VFIOAPDevice {
>> +    APDevice apdev;
>> +    VFIODevice vdev;
>> +    QTAILQ_ENTRY(VFIOAPDevice) sibling;
>> +} VFIOAPDevice;
>> +
>> +VFIOAPDevice *vfio_apdev;
>> +
>> +static void vfio_ap_compute_needs_reset(VFIODevice *vdev)
>> +{
>> +    vdev->needs_reset = false;
>> +}
>> +
>> +/*
>> + * We don't need vfio_hot_reset_multi and vfio_eoi operations for
>> + * vfio-ap-matrix device now.
>> + */
>> +struct VFIODeviceOps vfio_ap_ops = {
>> +    .vfio_compute_needs_reset = vfio_ap_compute_needs_reset,
>> +};
>> +
>> +static void vfio_put_device(VFIOAPDevice *vapdev)
>> +{
>> +    g_free(vapdev->vdev.name);
>> +    vfio_put_base_device(&vapdev->vdev);
>> +}
>> +
>> +static VFIOGroup *vfio_ap_get_group(VFIOAPDevice *vapdev, Error **errp)
>> +{
>> +    char *tmp, group_path[PATH_MAX];
>> +    ssize_t len;
>> +    int groupid;
>> +
>> +    tmp = g_strdup_printf("%s/iommu_group", vapdev->vdev.sysfsdev);
>> +    len = readlink(tmp, group_path, sizeof(group_path));
>> +    g_free(tmp);
>> +
>> +    if (len <= 0 || len >= sizeof(group_path)) {
>> +        error_setg(errp, "%s: no iommu_group found for %s",
>> +                   VFIO_AP_DEVICE_TYPE, vapdev->vdev.sysfsdev);
>> +        return NULL;
>> +    }
>> +
>> +    group_path[len] = 0;
>> +
>> +    if (sscanf(basename(group_path), "%d", &groupid) != 1) {
>> +        error_setg(errp, "vfio: failed to read %s", group_path);
>> +        return NULL;
>> +    }
>> +
>> +    return vfio_get_group(groupid, &address_space_memory, errp);
>> +}
>> +
>> +static void vfio_ap_realize(DeviceState *dev, Error **errp)
>> +{
>> +    VFIODevice *vbasedev;
>> +    VFIOGroup *vfio_group;
>> +    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
>> +    char *mdevid;
>> +    Error *local_err = NULL;
>> +    int ret;
>> +
>> +    /*
>> +     * Since a guest's matrix is configured in its entirety by the 
>> mediated
>> +     * matrix device and hot plug is not currently supported, there 
>> is no
>> +     * need to have more than one vfio-ap device. Check if a vfio-ap 
>> device
>> +     * has already been defined.
>> +     */
>> +    if (vfio_apdev) {
>> +        error_setg(&local_err, "Only one %s device is allowed",
>> +                   VFIO_AP_DEVICE_TYPE);
>> +        goto out_err;
>> +    }
>> +
>> +    if (!s390_has_feat(S390_FEAT_AP)) {
>> +        error_setg(&local_err, "AP support not enabled");
>> +        goto out_err;
>> +    }
>> +
>> +    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
>> +
>> +    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
>> +    if (!vfio_group) {
>> +        goto out_err;
>> +    }
>> +
>> +    vfio_apdev->vdev.ops = &vfio_ap_ops;
>> +    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
>> +    mdevid = basename(vfio_apdev->vdev.sysfsdev);
>> +    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
>> +    vfio_apdev->vdev.dev = dev;
>> +    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {
>
> can this happen if you have only one device ?

No, it is a remnant from previous versions when we didn't enforce
the single device requirement. I can remove it.

>
>
>> +        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
>> +            error_setg(&local_err,
>> +                       "%s: AP device %s has already been realized",
>> +                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
>> +            goto out_device_err;
>> +        }
>> +    }
>> +
>> +
> ...snip...
>

Re: [Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Cornelia Huck]

On Sun, 15 Apr 2018 15:07:23 -0400
Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:

> Introduces a VFIO based AP device. The device is defined via
> the QEMU command line by specifying:
> 
>     -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
> 
> There may be only one vfio-ap device configured for a guest.
> 
> The mediated matrix device is created by the VFIO AP device
> driver by writing a UUID to a sysfs attribute file (see
> docs/vfio-ap.txt). The mediated matrix device will be named
> after the UUID. Symbolic links to the $uuid are created in
> many places, so the path to the mediated matrix device $uuid
> can be specified in any of the following ways:
> 
> /sys/devices/vfio_ap/matrix/$uuid
> /sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
> /sys/bus/mdev/devices/$uuid
> /sys/bus/mdev/drivers/vfio_mdev/$uuid
> 
> When the vfio-ap device is realized, it acquires and opens the
> VFIO iommu group to which the mediated matrix device is
> bound. This causes a VFIO group notification event to be
> signaled. The vfio_ap device driver's group notification
> handler will get called at which time the device driver
> will configure the the AP devices to which the guest will
> be granted access.
> 
> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> ---
>  default-configs/s390x-softmmu.mak |    1 +
>  hw/vfio/Makefile.objs             |    1 +
>  hw/vfio/ap.c                      |  191 +++++++++++++++++++++++++++++++++++++
>  include/hw/vfio/vfio-common.h     |    1 +
>  4 files changed, 194 insertions(+), 0 deletions(-)
>  create mode 100644 hw/vfio/ap.c

> +static void vfio_ap_realize(DeviceState *dev, Error **errp)
> +{
> +    VFIODevice *vbasedev;
> +    VFIOGroup *vfio_group;
> +    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
> +    char *mdevid;
> +    Error *local_err = NULL;
> +    int ret;
> +
> +    /*
> +     * Since a guest's matrix is configured in its entirety by the mediated
> +     * matrix device and hot plug is not currently supported, there is no
> +     * need to have more than one vfio-ap device. Check if a vfio-ap device
> +     * has already been defined.
> +     */
> +    if (vfio_apdev) {
> +        error_setg(&local_err, "Only one %s device is allowed",
> +                   VFIO_AP_DEVICE_TYPE);
> +        goto out_err;
> +    }
> +
> +    if (!s390_has_feat(S390_FEAT_AP)) {
> +        error_setg(&local_err, "AP support not enabled");
> +        goto out_err;
> +    }
> +
> +    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
> +
> +    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
> +    if (!vfio_group) {
> +        goto out_err;
> +    }
> +
> +    vfio_apdev->vdev.ops = &vfio_ap_ops;
> +    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
> +    mdevid = basename(vfio_apdev->vdev.sysfsdev);
> +    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
> +    vfio_apdev->vdev.dev = dev;
> +    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {
> +        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
> +            error_setg(&local_err,
> +                       "%s: AP device %s has already been realized",
> +                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
> +            goto out_device_err;
> +        }
> +    }
> +
> +    ret = vfio_get_device(vfio_group, mdevid, &vfio_apdev->vdev, &local_err);
> +    if (ret) {
> +        goto out_device_err;
> +    }

Don't you need a put somewhere to avoid memory leaks?

> +
> +    return;
> +
> +
> +out_device_err:
> +    vfio_put_group(vfio_group);
> +out_err:
> +    vfio_apdev = NULL;
> +    error_propagate(errp, local_err);
> +}

Re: [Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Tony Krowiak]

On 04/19/2018 08:03 AM, Cornelia Huck wrote:
> On Sun, 15 Apr 2018 15:07:23 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>
>> Introduces a VFIO based AP device. The device is defined via
>> the QEMU command line by specifying:
>>
>>      -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
>>
>> There may be only one vfio-ap device configured for a guest.
>>
>> The mediated matrix device is created by the VFIO AP device
>> driver by writing a UUID to a sysfs attribute file (see
>> docs/vfio-ap.txt). The mediated matrix device will be named
>> after the UUID. Symbolic links to the $uuid are created in
>> many places, so the path to the mediated matrix device $uuid
>> can be specified in any of the following ways:
>>
>> /sys/devices/vfio_ap/matrix/$uuid
>> /sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
>> /sys/bus/mdev/devices/$uuid
>> /sys/bus/mdev/drivers/vfio_mdev/$uuid
>>
>> When the vfio-ap device is realized, it acquires and opens the
>> VFIO iommu group to which the mediated matrix device is
>> bound. This causes a VFIO group notification event to be
>> signaled. The vfio_ap device driver's group notification
>> handler will get called at which time the device driver
>> will configure the the AP devices to which the guest will
>> be granted access.
>>
>> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>> ---
>>   default-configs/s390x-softmmu.mak |    1 +
>>   hw/vfio/Makefile.objs             |    1 +
>>   hw/vfio/ap.c                      |  191 +++++++++++++++++++++++++++++++++++++
>>   include/hw/vfio/vfio-common.h     |    1 +
>>   4 files changed, 194 insertions(+), 0 deletions(-)
>>   create mode 100644 hw/vfio/ap.c
>> +static void vfio_ap_realize(DeviceState *dev, Error **errp)
>> +{
>> +    VFIODevice *vbasedev;
>> +    VFIOGroup *vfio_group;
>> +    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
>> +    char *mdevid;
>> +    Error *local_err = NULL;
>> +    int ret;
>> +
>> +    /*
>> +     * Since a guest's matrix is configured in its entirety by the mediated
>> +     * matrix device and hot plug is not currently supported, there is no
>> +     * need to have more than one vfio-ap device. Check if a vfio-ap device
>> +     * has already been defined.
>> +     */
>> +    if (vfio_apdev) {
>> +        error_setg(&local_err, "Only one %s device is allowed",
>> +                   VFIO_AP_DEVICE_TYPE);
>> +        goto out_err;
>> +    }
>> +
>> +    if (!s390_has_feat(S390_FEAT_AP)) {
>> +        error_setg(&local_err, "AP support not enabled");
>> +        goto out_err;
>> +    }
>> +
>> +    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
>> +
>> +    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
>> +    if (!vfio_group) {
>> +        goto out_err;
>> +    }
>> +
>> +    vfio_apdev->vdev.ops = &vfio_ap_ops;
>> +    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
>> +    mdevid = basename(vfio_apdev->vdev.sysfsdev);
>> +    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
>> +    vfio_apdev->vdev.dev = dev;
>> +    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {
>> +        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
>> +            error_setg(&local_err,
>> +                       "%s: AP device %s has already been realized",
>> +                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
>> +            goto out_device_err;
>> +        }
>> +    }
>> +
>> +    ret = vfio_get_device(vfio_group, mdevid, &vfio_apdev->vdev, &local_err);
>> +    if (ret) {
>> +        goto out_device_err;
>> +    }
> Don't you need a put somewhere to avoid memory leaks?

There is a call to vfio_put_device in the unrealize function.

>
>> +
>> +    return;
>> +
>> +
>> +out_device_err:
>> +    vfio_put_group(vfio_group);
>> +out_err:
>> +    vfio_apdev = NULL;
>> +    error_propagate(errp, local_err);
>> +}

Re: [Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Cornelia Huck]

On Sun, 22 Apr 2018 12:05:44 -0400
Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:

> On 04/19/2018 08:03 AM, Cornelia Huck wrote:
> > On Sun, 15 Apr 2018 15:07:23 -0400
> > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> >  
> >> Introduces a VFIO based AP device. The device is defined via
> >> the QEMU command line by specifying:
> >>
> >>      -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
> >>
> >> There may be only one vfio-ap device configured for a guest.
> >>
> >> The mediated matrix device is created by the VFIO AP device
> >> driver by writing a UUID to a sysfs attribute file (see
> >> docs/vfio-ap.txt). The mediated matrix device will be named
> >> after the UUID. Symbolic links to the $uuid are created in
> >> many places, so the path to the mediated matrix device $uuid
> >> can be specified in any of the following ways:
> >>
> >> /sys/devices/vfio_ap/matrix/$uuid
> >> /sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
> >> /sys/bus/mdev/devices/$uuid
> >> /sys/bus/mdev/drivers/vfio_mdev/$uuid
> >>
> >> When the vfio-ap device is realized, it acquires and opens the
> >> VFIO iommu group to which the mediated matrix device is
> >> bound. This causes a VFIO group notification event to be
> >> signaled. The vfio_ap device driver's group notification
> >> handler will get called at which time the device driver
> >> will configure the the AP devices to which the guest will
> >> be granted access.
> >>
> >> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
> >> ---
> >>   default-configs/s390x-softmmu.mak |    1 +
> >>   hw/vfio/Makefile.objs             |    1 +
> >>   hw/vfio/ap.c                      |  191 +++++++++++++++++++++++++++++++++++++
> >>   include/hw/vfio/vfio-common.h     |    1 +
> >>   4 files changed, 194 insertions(+), 0 deletions(-)
> >>   create mode 100644 hw/vfio/ap.c
> >> +static void vfio_ap_realize(DeviceState *dev, Error **errp)
> >> +{
> >> +    VFIODevice *vbasedev;
> >> +    VFIOGroup *vfio_group;
> >> +    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
> >> +    char *mdevid;
> >> +    Error *local_err = NULL;
> >> +    int ret;
> >> +
> >> +    /*
> >> +     * Since a guest's matrix is configured in its entirety by the mediated
> >> +     * matrix device and hot plug is not currently supported, there is no
> >> +     * need to have more than one vfio-ap device. Check if a vfio-ap device
> >> +     * has already been defined.
> >> +     */
> >> +    if (vfio_apdev) {
> >> +        error_setg(&local_err, "Only one %s device is allowed",
> >> +                   VFIO_AP_DEVICE_TYPE);
> >> +        goto out_err;
> >> +    }
> >> +
> >> +    if (!s390_has_feat(S390_FEAT_AP)) {
> >> +        error_setg(&local_err, "AP support not enabled");
> >> +        goto out_err;
> >> +    }
> >> +
> >> +    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
> >> +
> >> +    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
> >> +    if (!vfio_group) {
> >> +        goto out_err;
> >> +    }
> >> +
> >> +    vfio_apdev->vdev.ops = &vfio_ap_ops;
> >> +    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
> >> +    mdevid = basename(vfio_apdev->vdev.sysfsdev);
> >> +    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
> >> +    vfio_apdev->vdev.dev = dev;
> >> +    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {
> >> +        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
> >> +            error_setg(&local_err,
> >> +                       "%s: AP device %s has already been realized",
> >> +                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
> >> +            goto out_device_err;
> >> +        }
> >> +    }
> >> +
> >> +    ret = vfio_get_device(vfio_group, mdevid, &vfio_apdev->vdev, &local_err);
> >> +    if (ret) {
> >> +        goto out_device_err;
> >> +    }  
> > Don't you need a put somewhere to avoid memory leaks?  
> 
> There is a call to vfio_put_device in the unrealize function.

I don't think unrealize is called if realize failed, so you need to
clean up in the error cases?

> 
> >  
> >> +
> >> +    return;
> >> +
> >> +
> >> +out_device_err:
> >> +    vfio_put_group(vfio_group);
> >> +out_err:
> >> +    vfio_apdev = NULL;
> >> +    error_propagate(errp, local_err);
> >> +}  
> 
> 

Re: [Qemu-devel] [PATCH v4 4/5] s390x/vfio: ap: Introduce VFIO AP device

[Tony Krowiak]

On 04/23/2018 03:00 AM, Cornelia Huck wrote:
> On Sun, 22 Apr 2018 12:05:44 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>
>> On 04/19/2018 08:03 AM, Cornelia Huck wrote:
>>> On Sun, 15 Apr 2018 15:07:23 -0400
>>> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>>>   
>>>> Introduces a VFIO based AP device. The device is defined via
>>>> the QEMU command line by specifying:
>>>>
>>>>       -device vfio-ap,sysfsdev=<path-to-mediated-matrix-device>
>>>>
>>>> There may be only one vfio-ap device configured for a guest.
>>>>
>>>> The mediated matrix device is created by the VFIO AP device
>>>> driver by writing a UUID to a sysfs attribute file (see
>>>> docs/vfio-ap.txt). The mediated matrix device will be named
>>>> after the UUID. Symbolic links to the $uuid are created in
>>>> many places, so the path to the mediated matrix device $uuid
>>>> can be specified in any of the following ways:
>>>>
>>>> /sys/devices/vfio_ap/matrix/$uuid
>>>> /sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
>>>> /sys/bus/mdev/devices/$uuid
>>>> /sys/bus/mdev/drivers/vfio_mdev/$uuid
>>>>
>>>> When the vfio-ap device is realized, it acquires and opens the
>>>> VFIO iommu group to which the mediated matrix device is
>>>> bound. This causes a VFIO group notification event to be
>>>> signaled. The vfio_ap device driver's group notification
>>>> handler will get called at which time the device driver
>>>> will configure the the AP devices to which the guest will
>>>> be granted access.
>>>>
>>>> Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
>>>> ---
>>>>    default-configs/s390x-softmmu.mak |    1 +
>>>>    hw/vfio/Makefile.objs             |    1 +
>>>>    hw/vfio/ap.c                      |  191 +++++++++++++++++++++++++++++++++++++
>>>>    include/hw/vfio/vfio-common.h     |    1 +
>>>>    4 files changed, 194 insertions(+), 0 deletions(-)
>>>>    create mode 100644 hw/vfio/ap.c
>>>> +static void vfio_ap_realize(DeviceState *dev, Error **errp)
>>>> +{
>>>> +    VFIODevice *vbasedev;
>>>> +    VFIOGroup *vfio_group;
>>>> +    APDevice *apdev = DO_UPCAST(APDevice, parent_obj, dev);
>>>> +    char *mdevid;
>>>> +    Error *local_err = NULL;
>>>> +    int ret;
>>>> +
>>>> +    /*
>>>> +     * Since a guest's matrix is configured in its entirety by the mediated
>>>> +     * matrix device and hot plug is not currently supported, there is no
>>>> +     * need to have more than one vfio-ap device. Check if a vfio-ap device
>>>> +     * has already been defined.
>>>> +     */
>>>> +    if (vfio_apdev) {
>>>> +        error_setg(&local_err, "Only one %s device is allowed",
>>>> +                   VFIO_AP_DEVICE_TYPE);
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    if (!s390_has_feat(S390_FEAT_AP)) {
>>>> +        error_setg(&local_err, "AP support not enabled");
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    vfio_apdev = DO_UPCAST(VFIOAPDevice, apdev, apdev);
>>>> +
>>>> +    vfio_group = vfio_ap_get_group(vfio_apdev, &local_err);
>>>> +    if (!vfio_group) {
>>>> +        goto out_err;
>>>> +    }
>>>> +
>>>> +    vfio_apdev->vdev.ops = &vfio_ap_ops;
>>>> +    vfio_apdev->vdev.type = VFIO_DEVICE_TYPE_AP;
>>>> +    mdevid = basename(vfio_apdev->vdev.sysfsdev);
>>>> +    vfio_apdev->vdev.name = g_strdup_printf("%s", mdevid);
>>>> +    vfio_apdev->vdev.dev = dev;
>>>> +    QLIST_FOREACH(vbasedev, &vfio_group->device_list, next) {
>>>> +        if (strcmp(vbasedev->name, vfio_apdev->vdev.name) == 0) {
>>>> +            error_setg(&local_err,
>>>> +                       "%s: AP device %s has already been realized",
>>>> +                       VFIO_AP_DEVICE_TYPE, vfio_apdev->vdev.name);
>>>> +            goto out_device_err;
>>>> +        }
>>>> +    }
>>>> +
>>>> +    ret = vfio_get_device(vfio_group, mdevid, &vfio_apdev->vdev, &local_err);
>>>> +    if (ret) {
>>>> +        goto out_device_err;
>>>> +    }
>>> Don't you need a put somewhere to avoid memory leaks?
>> There is a call to vfio_put_device in the unrealize function.
> I don't think unrealize is called if realize failed, so you need to
> clean up in the error cases?

Very true. I  misinterpreted your point. Yes, I need to clean up
if realize fails.

>
>>>   
>>>> +
>>>> +    return;
>>>> +
>>>> +
>>>> +out_device_err:
>>>> +    vfio_put_group(vfio_group);
>>>> +out_err:
>>>> +    vfio_apdev = NULL;
>>>> +    error_propagate(errp, local_err);
>>>> +}
>>

[Qemu-devel] [PATCH v4 5/5] s390: doc: detailed specifications for AP virtualization

[Tony Krowiak]

This patch provides documentation describing the AP architecture and
design concepts behind the virtualization of AP devices. It also
includes an example of how to configure AP devices for exclusive
use of KVM guests.

Signed-off-by: Tony Krowiak <akrowiak@linux.vnet.ibm.com>
---
 docs/vfio-ap.txt |  649 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 649 insertions(+), 0 deletions(-)
 create mode 100644 docs/vfio-ap.txt

diff --git a/docs/vfio-ap.txt b/docs/vfio-ap.txt
new file mode 100644
index 0000000..e0d826c
--- /dev/null
+++ b/docs/vfio-ap.txt
@@ -0,0 +1,649 @@
+Adjunct Processor (AP) Device
+=============================
+
+Contents:
+=========
+* Introduction
+* AP Architectural Overview
+* Start Interpretive Execution (SIE) Instruction
+* AP Matrix Configuration on Linux Host
+* AP Matrix Configuration for a Linux Guest
+* Starting a Linux Guest Configured with an AP Matrix
+* Example: Configure AP Matrices for Two Linux Guests
+
+Introduction:
+============
+The IBM Adjunct Processor (AP) Cryptographic Facility is comprised
+of three AP instructions and from 1 to 256 PCIe cryptographic adapter cards.
+These AP devices provide cryptographic functions to all CPUs assigned to a
+linux system running in an IBM Z system LPAR.
+
+On s390x, AP adapter cards are exposed via the AP bus. This document
+describes how those cards may be made available to KVM guests using the
+VFIO mediated device framework.
+
+AP Architectural Overview:
+=========================
+In order understand the terminology used in the rest of this document, let's
+start with some definitions:
+
+* AP adapter
+
+  An AP adapter is an IBM Z adapter card that can perform cryptographic
+  functions. There can be from 0 to 256 adapters assigned to an LPAR. Adapters
+  assigned to the LPAR in which a linux host is running will be available to
+  the linux host. Each adapter is identified by a number from 0 to 255. When
+  installed, an AP adapter is accessed by AP instructions executed by any CPU.
+
+* AP domain
+
+  An adapter is partitioned into domains. Each domain can be thought of as
+  a set of hardware registers for processing AP instructions. An adapter can
+  hold up to 256 domains. Each domain is identified by a number from 0 to 255.
+  Domains can be further classified into two types:
+
+    * Usage domains are domains that can be accessed directly to process AP
+      commands
+
+    * Control domains are domains that are accessed indirectly by AP
+      commands sent to a usage domain to control or change the domain, for
+      example; to set a secure private key for the domain.
+
+  The AP usage and control domains are assigned to a given LPAR via the system's
+  Activation Profile which can be edited via the HMC. When the system is IPL'd,
+  the AP bus module is loaded and detects the AP usage and control domains
+  assigned to the LPAR. The domain number of each usage domain will be coupled
+  with the adapter number of each AP adapter assigned to the LPAR to identify
+  the AP queues (see AP Queue section below). The domain number of each control
+  domain will be represented in a bitmask and stored in a sysfs file
+  /sys/bus/ap/ap_control_domain_mask created by the bus. The bits in the mask,
+  from most to least significant bit, correspond to domains 0-255.
+
+  A domain may be assigned to a system as both a usage and control domain, or
+  as a control domain only. Consequently, all domains assigned as both a usage
+  and control domain can both process AP commands as well as be changed by an AP
+  command sent to any usage domain assigned to the same system. Domains assigned
+  only as control domains can not process AP commands but can be changed by AP
+  commands sent to any usage domain assigned to the system.
+
+* AP Queue
+
+  An AP queue is the means by which an AP command-request message is sent to an
+  AP usage domain inside a specific AP. An AP queue is identified by a tuple
+  comprised of an AP adapter ID (APID) and an AP queue index (APQI). The
+  APQI corresponds to a given usage domain number within the adapter. This tuple
+  forms an AP Queue Number (APQN) uniquely identifying an AP queue. AP
+  instructions include a field containing the APQN to identify the AP queue to
+  which the AP command-request message is to be sent for processing.
+
+* AP Instructions:
+
+  There are three AP instructions:
+
+  * NQAP: to enqueue an AP command-request message to a queue
+  * DQAP: to dequeue an AP command-reply message from a queue
+  * PQAP: to administer the queues
+
+Start Interpretive Execution (SIE) Instruction
+==============================================
+A KVM guest is started by executing the Start Interpretive Execution (SIE)
+instruction. The SIE state description is a control block that contains the
+state information for a KVM guest and is supplied as input to the SIE
+instruction. The SIE state description contains a field that references
+a Crypto Control Block (CRYCB). The CRYCB contains three fields to identify the
+adapters, usage domains and control domains assigned to the KVM guest:
+
+* The AP Mask (APM) field is a bit mask that identifies the AP adapters assigned
+  to the KVM guest. Each bit in the mask, from most significant to least
+  significant bit, corresponds to an APID from 0-255. If a bit is set, the
+  corresponding adapter is valid for use by the KVM guest.
+
+* The AP Queue Mask (AQM) field is a bit mask identifying the AP usage domains
+  assigned to the KVM guest. Each bit in the mask, from most significant to
+  least significant bit, corresponds to an AP queue index (APQI) from 0-255. If
+  a bit is set, the corresponding queue is valid for use by the KVM guest.
+
+* The AP Domain Mask field is a bit mask that identifies the AP control domains
+  assigned to the KVM guest. The ADM bit mask controls which domains can be
+  changed by an AP command-request message sent to a usage domain from the
+  guest. Each bit in the mask, from least significant to most significant bit,
+  corresponds to a domain from 0-255. If a bit is set, the corresponding domain
+  can be modified by an AP command-request message sent to a usage domain
+  configured for the KVM guest.
+
+If you recall from the description of an AP Queue, AP instructions include
+an APQN to identify the AP adapter and AP queue to which an AP command-request
+message is to be sent (NQAP and PQAP instructions), or from which a
+command-reply message is to be received (DQAP instruction). The validity of an
+APQN is defined by the matrix calculated from the APM and AQM; it is the
+cross product of all assigned adapter numbers (APM) with all assigned queue
+indexes (AQM). For example, if adapters 1 and 2 and usage domains 5 and 6 are
+assigned to a guest, the APQNs (1,5), (1,6), (2,5) and (2,6) will be valid for
+the guest.
+
+The APQNs can provide secure key functionality - i.e., a private key is stored
+on the adapter card for each of its domains - so each APQN must be assigned to
+at most one guest or the linux host.
+
+   Example 1: Valid configuration:
+   ------------------------------
+   Guest1: adapters 1,2  domains 5,6
+   Guest2: adapter  1,2  domain 7
+
+   This is valid because both guests have a unique set of APQNs: Guest1 has
+   APQNs (1,5), (1,6), (2,5) and (2,6); Guest2 has APQNs (1,7) and (2,7).
+
+   Example 2: Invalid configuration:
+   --------------------------------
+   Guest1: adapters 1,2  domains 5,6
+   Guest2: adapter  1    domains 6,7
+
+   This is an invalid configuration because both guests have access to
+   APQN (1,6).
+
+AP device Configuration on Linux Host:
+=====================================
+A linux system is a guest of the LPAR in which it is running and has access to
+the AP resources configured for the LPAR. The LPAR's AP matrix is
+configured using the 'Customize/Delete Activation Profiles' dialog from the HMC.
+This dialog displays the activation profiles configured for the linux system.
+Selecting the specific activation profile to be edited and clicking the
+'Customize Profile' button will open the 'Customize Image Profiles' dialog.
+Selecting the 'Crypto' link in the tree view on the left hand side of the dialog
+will display the AP matrix configuration in the right hand panel. There, one can
+assign AP adapters - called Cryptos - and domains to the LPAR. When the linux
+system is started using this activation profile, it will have access to the
+matrix of AP adapters and domains configured via the activation profile.
+
+When the linux system is started, the AP adapter devices will be connected to
+the AP bus and the following AP matrix interfaces will be created in sysfs:
+
+/sys/bus/ap
+... [devices]
+...... xx.yyyy
+...... ...
+...... cardxx
+...... ...
+
+Where:
+    cardxx     is adapter number xx (in hex)
+    yyyy       is a usage domain number yyyy (in hex)
+....xx.yyyy    is APQN (xx,yyyy)
+
+For example, if AP adapters 5 and 6 and domains 4 and 71 (0x47) are configured
+for the LPAR, the sysfs representation on the linux system would look like this:
+
+/sys/bus/ap
+... [devices]
+...... 05.0004
+...... 05.0047
+...... 06.0004
+...... 06.0047
+...... card05
+...... card06
+
+There will also be AP device drivers created to control each type of AP matrix
+interface available to the IBM Z system:
+
+/sys/bus/ap
+... [drivers]
+...... [cex2acard]        for Crypto Express 2/3 accelerator cards
+...... [cex2aqueue]       for AP queues served by Crypto Express 2/3
+                          accelerator cards
+...... [cex4card]         for Crypto Express 4/5/6 accelerator and coprocessor
+                          cards
+...... [cex4queue]        for AP queues served by Crypto Express 4/5/6
+                          accelerator and coprocessor cards
+...... [pcixcccard]       for Crypto Express 2/3 coprocessor cards
+...... [pcixccqueue]      for AP queues served by Crypto Express 2/3
+                          coprocessor cards
+
+Links to the AP interfaces controlled by each AP device driver will be created
+in the device driver's sysfs directory. For example, if AP adapter 5 and domains
+4 and 71 (0x47) are assigned to the LPAR and adapter 5 is a CEX5 card, the
+following links will be created in the CEX5 drivers' sysfs directories:
+
+/sys/bus/ap
+... [drivers]
+...... [cex4card]
+......... [card05]
+...... [cex4queue]
+......... [05.0004]
+......... [05.0047]
+
+AP Matrix Configuration for a Linux Guest:
+=========================================
+In order to configure the AP matrix for a guest, the adapters, usage domains
+and control domains to be used by the guest must be assigned to the guest. This
+section describes how to configure a guest's AP matrix.
+
+The kernel interfaces for configuring an AP matrix for a linux guest are built
+on the VFIO mediated device framework and are provided by the vfio_ap
+kernel module. By default, the vfio_ap module is a loadable module, The
+dependency chain for the vfio_ap module is:
+* vfio
+* mdev
+* vfio_mdev
+* vfio_ap
+
+When installed, the vfio_ap module is initialized. During module initialization,
+a vfio_ap driver is created and registered with the AP bus creating the
+following sysfs interfaces:
+
+ /sys/bus/ap/drivers/
+...[vfio_ap]
+...... bind
+...... unbind
+
+The vfio_ap device driver will create a 'matrix' device to hold the APQNs
+reserved for exclusive use by KVM guests:
+
+/sys/devices/
+... [vfio_ap]
+......[matrix] symlink to the matrix device directory
+
+The vfio_ap device driver serves several purposes:
+1. Provides an interface for securing APQNs preventing their use by the host
+   linux system and reserving their use by one or more guests.
+2. Creates the sysfs interfaces for configuring an AP matrix for a linux guest.
+
+Securing APQNs
+--------------
+   An APQN is reserved by unbinding an AP queue device AP bus device driver and
+   binding it to the vfio_ap device driver. For example, suppose we want to
+   secure APQN (05,0004). Assuming that the AP adapter card 5 is a CEX5
+   coprocessor card:
+
+   echo 05.0004 > /sys/bus/ap/drivers/cex4queue/unbind
+   echo 05.0004 > /sys/bus/ap/drivers/vfio_ap/bind
+
+   This action will store the APQN in the /sys/devices/vfio_ap/matrix device
+   which makes it available for use by a linux guest.
+
+Configuring an AP matrix for a linux guest.
+------------------------------------------
+These sysfs interfaces are built on the VFIO mediated device framework. To
+configure an AP matrix for a guest, a mediated matrix device must first be
+created for the /sys/devices/vfio_ap/matrix device. The sysfs interfaceAPQI corresponding to
+for creating a mediated matrix device is in:
+
+/sys/devices
+... [vfio_ap]
+......[matrix]
+......... [mdev_supported_types]
+............ [vfio_ap-passthrough]
+............... create
+............... [devices]
+
+A mediated AP matrix device is created by writing a UUID to the attribute
+file named 'create', for example:
+
+   uuidgen > create
+
+When a mediated AP matrix device is created, a sysfs directory named after
+the UUID:
+
+/sys/devices
+... [vfio_ap]
+......[matrix]
+......... [mdev_supported_types]
+............ [vfio_ap-passthrough]
+............... create
+............... [devices]
+.................. [$uuid]
+
+There will also be three sets of attribute files created in the mediated
+matrix device's sysfs directory to configure an AP matrix for the
+KVM guest:
+
+/sys/devices
+... [vfio_ap]
+......[matrix]
+......... [mdev_supported_types]
+............ [vfio_ap-passthrough]
+............... create
+............... [devices]
+.................. [$uuid]
+..................... assign_adapter
+..................... assign_control_domain
+..................... assign_domain
+..................... matrix
+..................... unassign_adapter
+..................... unassign_control_domain
+..................... unassign_domain
+
+assign_adapter
+   To assign an AP adapter to the mediated matrix device, its APID is written
+   'assign_adapter' file. This may be done multiple times to assign more than
+   one adapter. The APID may be specified using conventional semantics
+   as a decimal, hexidecimal, or octal number. For example, to assign adapters
+   4, 5 and 16 to mediated matrix device $uuid in decimal, hexidecimal and octal
+   respectively:
+
+       echo 4 > assign_adapter
+       echo 0x5 > assign_adapter
+       echo 020 > assign_adapter
+
+unassign_adapter
+   To unassign an AP adapter, its APID is written to the 'unassign_adapter'
+   file. This may also be done multiple times to unassign more than one adapter.
+
+assign_domain
+   To assign a usage domain, the APQI corresponding to the domain number is
+   written into the 'assign_domain' file. This may be done multiple times to
+   assign more than one usage domain. The APQI may be specified using
+   conventional semantics as a decimal, hexidecimal, or octal number. For
+   example, to assign usage domains 4, 8, and 71 to mediated matrix device
+   $uuid in decimal, hexidecimal and octal respectively:
+
+      echo 4 > assign_domain
+      echo 0x8 > assign_domain
+      echo 0107 > assign_domain
+
+unassign_domain
+   To unassign a usage domain, the APQI corresponding to the domain number is
+   written into the 'unassign_domain' file. This may be done multiple times to
+   unassign more than one usage domain.
+
+assign_control_domain
+   To assign a control domain, the domain number is written into the
+   'assign_control_domain' file. This may be done multiple times to
+   assign more than one control domain. The domain number may be specified using
+   conventional semantics as a decimal, hexidecimal, or octal number. For
+   example, to assign  control domains 4, 8, and 71 to mediated matrix device
+   $uuid in decimal, hexidecimal and octal respectively:
+
+      echo 4 > assign_domain
+      echo 0x8 > assign_domain
+      echo 0107 > assign_domain
+
+unassign_control_domain
+   To unassign a control domain, the domain number is written into the
+   'unassign_domain' file. This may be done multiple times to unassign more than
+   one control domain.
+
+Notes:
+* Hot plug/unplug is not currently supported for mediated AP matrix devices,
+  so the AP matrix resulting from assignment and/or unassignment of AP
+  adapters, usage domains and control domains to a mediated AP matrix device
+  while the guest is running will not take affect until the linux guest is
+  rebooted.
+* For the initial implementation, all usage domains configured for a KVM guest
+  will also be implicitly assigned as control domains also, to there is no
+  need to assign control domains that are assigned as usage domains. This could
+  change in future releases.
+
+Starting a Linux Guest Configured with an AP Matrix:
+===================================================
+In addition to providing the sysfs interfaces for configuring the AP matrix for
+a linux guest, a mediated matrix device also acts as a communication pathway
+between QEMU and the vfio_ap device driver. To gain access to the
+device driver, the following option must be specified on the QEMU command line:
+
+   -device vfio_ap,sysfsdev=$path-to-mdev
+
+The sysfsdev parameter specifies the path to the mediated matrix device.
+There are a number of ways to specify this path:
+
+/sys/devices/vfio_ap/matrix/$uuid
+/sys/bus/mdev/devices/$uuid
+/sys/bus/mdev/drivers/vfio_mdev/$uuid
+/sys/devices/vfio_ap/matrix/mdev_supported_types/vfio_ap-passthrough/devices/$uuid
+
+When the linux guest is subsequently started, the guest will open the mediated
+matrix device's file descriptor to get information about the mediated matrix
+device. The vfio_ap device driver will update the APM, AQM, and ADM fields in the
+guest's CRYCB with the adapter, usage domain and control domains assigned to
+via the mediated matrix device's sysfs attribute files. Programs running on the
+linux guest will then:
+
+1. Have direct access to the APQNs derived from the intersection of the AP
+   adapter and usage domain numbers specified in the APM and AQM respectively
+
+2. Have authorization to process AP commands to change - e.g., store a new
+   secure key - a control domain identified in an AP instruction sent to a valid
+   APQN.
+
+CPU model features:
+
+Three CPU model features are available for controlling guest access to AP
+facilities:
+
+1. AP facilities feature
+
+   The AP facilities feature indicates that AP facilities are installed on the
+   guest. This feature will be enabled by the kernel only if the AP facilities
+   are installed on the host system. It will be turned on automatically for
+   guests started with CPU model zEC12 or newer. The feature is s390-specific
+   and is represented as a parameter of the -cpu option on the QEMU command
+   line:
+
+      qemu-system-s390x -cpu $model,ap=on|off
+
+      Where:
+
+         $model is the CPU model defined for the guest (defaults to the model of
+                the host system if not specified).
+
+         ap=on|off indicates whether AP facilities are installed (on) or not
+                   (off). The default for CPU models zEC12 or newer
+                   is ap=on. AP facilities must be installed a vfio-ap device
+                   (-device vfio-ap,sysfsdev=$path) is configured for the
+                   guest or the guest will fail to start.
+
+2. Query Configuration Information (QCI) facility
+
+   The QCI facility is used by the AP bus running on the guest to query the
+   configuration of the AP facilities. This facility will be enabled by
+   the kernel only if the QCI facility is installed on the host system. It will
+   be turned on automatically for guests started with CPU model zEC12 or newer.
+   The feature is s390-specific and is represented as a parameter of the -cpu
+   option on the QEMU command line:
+
+      qemu-system-s390x -cpu $model,apqci=on|off
+
+      Where:
+
+         $model is the CPU model defined for the guest
+
+         apqci=on|off indicates whether the QCI facility is installed (on) or
+                      not (off). The default for CPU models zEC12 or newer
+                      is apqci=on; for older models, QCI will not be installed.
+
+                      If QCI is installed (apqci=on) but AP facilities are not
+                      (ap=off), an error message will be logged, but the guest
+                      will be allowed to start. It makes no sense to have QCI
+                      installed if the AP facilities are not; this is considered
+                      an invalid configuration.
+
+                      If the QCI facility is not installed, APQNs with an APQI
+                      greater than 15 will not be detected by the AP bus
+                      running on the guest.
+
+3. Adjunct Process Facility Test (APFT) facility
+
+   The APFT facility is used by the AP bus running on the guest to test the
+   AP facilities available for a given AP queue. This facility will be enabled
+   by the kernel only if the APFT facility is installed on the host system. It
+   will be turned on automatically for guests started with CPU model zEC12 or
+   newer. The feature is s390-specific and is represented as a parameter of the
+   -cpu option on the QEMU command line:
+
+      qemu-system-s390x -cpu $model,apft=on|off
+
+      Where:
+
+         $model is the CPU model defined for the guest (defaults to the model of
+                the host system if not specified).
+
+         apft=on|off indicates whether the APFT facility is installed (on) or
+                     not (off). The default for CPU models zEC12 and
+                     newer is apft=on for older models, APFT will not be
+                     installed.
+
+                     If APFT is installed (apft=on) but AP facilities are not
+                     (ap=off), an error message will be logged, but the guest
+                     will be allowed to start. It makes no sense to have APFT
+                     installed if the AP facilities are not; this is considered
+                     an invalid configuration.
+
+                     It also makes no sense to turn APFT off because the AP bus
+                     running on the guest will not detect CEX4 and newer devices
+                     without it. Since only CEX4 and newer devices are supported
+                     for guest usage, no AP devices can be made accessible to a
+                     guest started without APFT installed.
+
+Example: Configure AP Matrixes for Two Linux Guests:
+===================================================
+Let's now provide an example to illustrate how KVM guests may be given
+access to AP facilities. For this example, we will show how to configure
+two guests such that executing the lszcrypt command on the guests would
+look like this:
+
+Guest1
+------
+CARD.DOMAIN TYPE  MODE
+------------------------------
+05          CEX5C CCA-Coproc
+05.0004     CEX5C CCA-Coproc
+05.00ab     CEX5C CCA-Coproc
+06          CEX5A Accelerator
+06.0004     CEX5A Accelerator
+06.00ab     CEX5C CCA-Coproc
+
+Guest2
+------
+CARD.DOMAIN TYPE  MODE
+------------------------------
+05          CEX5A Accelerator
+05.0047     CEX5A Accelerator
+05.00ff     CEX5A Accelerator
+
+These are the steps for configuring the Guest1 and Guest2:
+
+1. The first thing that needs to be done is to secure the AP queues to be
+   used by the two guests so that the host can not access them. This is done
+   by unbinding each AP Queue device from its respective AP driver. In our
+   example, these queues are bound to the cex4queue driver. This would be
+   the sysfs location of these devices:
+
+   /sys/bus/ap
+   --- [drivers]
+   ------ [cex4queue]
+   --------- [05.0004]
+   --------- [05.0047]
+   --------------------- control_domains
+   --------------------- domains
+   --------- [05.00ab]
+   --------- [05.00ff]
+   --------- [06.0004]
+   --------- [06.00ab]
+   --------- unbind
+
+   To unbind AP queue 05.0004 from the cex4queue device driver:
+
+    echo 05.0004 > unbind
+
+   This must also be done for AP queues 05.00ab, 05.0047, 05.00ff, 06.0004,
+   and 06.00ab.
+
+2. The next step is to reserve the queues for use by the two KVM guests.
+   This is accomplished by binding them to the VFIO AP device driver.
+   This is the sysfs location of the VFIO AP device driver:
+
+   /sys/bus/ap
+   ---[drivers]
+   ------ [vfio_ap]
+   ---------- bind
+
+   To bind queue 05.0004 to the vfio_ap driver:
+
+    echo 05.0004 > bind
+
+   This must also be done for AP queues 05.00ab, 05.0047, 05.00ff, 06.0004,
+   and 06.00ab.
+
+3. Create the mediated devices needed to configure the AP matrixes for the
+   two guests and to provide an interface to the vfio_ap driver for
+   use by the guests:
+
+   /sys/devices/
+   --- [vfio_ap]
+   ------ [matrix] (this is the matrix device)
+   --------- [mdev_supported_types]
+   ------------ [vfio_ap-passthrough] (passthrough mediated matrix device type)
+   --------------- create
+   --------------- [devices]
+
+   To create the mediated devices for the two guests:
+
+    uuidgen > create
+    uuidgen > create
+
+   This will create two mediated devices in the [devices] subdirectory named
+   with the UUID written to the create attribute file. We call them $uuid1
+   and $uuid2:
+
+   /sys/devices/
+   --- [vfio_ap]
+   ------ [matrix]
+   --------- [mdev_supported_types]
+   ------------ [vfio_ap-passthrough]
+   --------------- [devices]
+   ------------------ [$uuid1]
+   --------------------- assign_adapter
+   --------------------- assign_control_domain
+   --------------------- assign_domain
+   --------------------- matrix
+   --------------------- unassign_adapter
+   --------------------- unassign_control_domain
+   --------------------- unassign_domain
+
+   ------------------ [$uuid2]
+   --------------------- assign_adapter
+   --------------------- assign_control_domain
+   --------------------- assign_domain
+   --------------------- matrix
+   --------------------- unassign_adapter
+   --------------------- unassign_control_domain
+   --------------------- unassign_domain
+
+4. The administrator now needs to configure the matrixes for mediated
+   devices $uuid1 (for Guest1) and $uuid2 (for Guest2).
+
+   This is how the matrix is configured for Guest1:
+
+   cd $uuid1
+   echo 5 > assign_adapter
+   echo 6 > assign_adapter
+   echo 4 > assign_domain
+   echo 0xab > assign_domain
+
+   For this implementation, all usage domains - i.e., domains assigned
+   via the assign_domain attribute file - will also be configured in the ADM
+   field of the KVM guest's CRYCB, so there is no need to assign control
+   domains here unless you want to assign control domains that are not
+   assigned as usage domains.
+
+   If a mistake is made configuring an adapter, domain or control domain,
+   you can use the unassign_xxx files to unassign the adapter, domain or
+   control domain.
+
+   To display the matrix configuration for Guest1:
+
+   cat matrix
+
+   This is how the matrix is configured for Guest2:
+
+   cd $uuid2
+   echo 5 > assign_adapter
+   echo 0x47 > assign_domain
+   echo 0xff > assign_domain
+
+5. Start Guest1
+
+   /usr/bin/qemu-system-s390x ... -device vfio-ap,sysfsdev=/sys/devices/vfio_ap/matrix/$uuid1 ...
+
+6. Start Guest2
+
+   /usr/bin/qemu-system-s390x ... -device vfio-ap,sysfsdev=/sys/devices/vfio_ap/matrix/$uuid2 ...
-- 
1.7.1

Re: [Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

[no-reply]

Hi,

This series seems to have some coding style problems. See output below for
more information:

Type: series
Message-id: 1523819244-29954-1-git-send-email-akrowiak@linux.vnet.ibm.com
Subject: [Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]               patchew/1523819244-29954-1-git-send-email-akrowiak@linux.vnet.ibm.com -> patchew/1523819244-29954-1-git-send-email-akrowiak@linux.vnet.ibm.com
Switched to a new branch 'test'
05c25cd0f2 s390: doc: detailed specifications for AP virtualization
543e5cbc46 s390x/vfio: ap: Introduce VFIO AP device
994913441f s390x/cpumodel: Set up CPU model for AP device support
e7f90b1258 s390x/ap: base Adjunct Processor (AP) object
13602d236e linux-headers: linux header updates for AP support

=== OUTPUT BEGIN ===
Checking PATCH 1/5: linux-headers: linux header updates for AP support...
Checking PATCH 2/5: s390x/ap: base Adjunct Processor (AP) object...
Checking PATCH 3/5: s390x/cpumodel: Set up CPU model for AP device support...
WARNING: line over 80 characters
#43: FILE: target/s390x/cpu_features.c:43:
+    FEAT_INIT("apqci", S390_FEAT_TYPE_STFL, 12, "Query AP Configuration facility"),

ERROR: line over 90 characters
#46: FILE: target/s390x/cpu_features.c:46:
+    FEAT_INIT("apft", S390_FEAT_TYPE_STFL, 15, "Adjunct Processor Facilities Test facility"),

total: 1 errors, 1 warnings, 58 lines checked

Your patch has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.

Checking PATCH 4/5: s390x/vfio: ap: Introduce VFIO AP device...
Checking PATCH 5/5: s390: doc: detailed specifications for AP virtualization...
=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

Re: [Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

[Cornelia Huck]

On Sun, 15 Apr 2018 15:07:19 -0400
Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:

>  default-configs/s390x-softmmu.mak |    1 +
>  docs/vfio-ap.txt                  |  649 +++++++++++++++++++++++++++++++++++++
>  hw/s390x/Makefile.objs            |    1 +
>  hw/s390x/ap-device.c              |   39 +++
>  hw/vfio/Makefile.objs             |    1 +
>  hw/vfio/ap.c                      |  191 +++++++++++
>  include/hw/s390x/ap-device.h      |   38 +++
>  include/hw/vfio/vfio-common.h     |    1 +
>  linux-headers/asm-s390/kvm.h      |    2 +
>  linux-headers/linux/vfio.h        |    2 +
>  target/s390x/cpu_features.c       |    3 +
>  target/s390x/cpu_features_def.h   |    3 +
>  target/s390x/cpu_models.c         |    2 +
>  target/s390x/gen-features.c       |    3 +
>  target/s390x/kvm.c                |    1 +
>  ui/keycodemapdb                   |    2 +-
>  16 files changed, 938 insertions(+), 1 deletions(-)
>  create mode 100644 docs/vfio-ap.txt
>  create mode 100644 hw/s390x/ap-device.c
>  create mode 100644 hw/vfio/ap.c
>  create mode 100644 include/hw/s390x/ap-device.h

Can we please get a MAINTAINERS entry for ap? While I'm obviously happy
to take things through the s390x tree, ap is not something I can easily
review (or even test), so I'd like to see an entry with dedicated (IBM)
maintainers.

Also, hw/vfio/ap.c and docs/vfio-ap.c should be added to the generic
s390 section.

Re: [Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

[Tony Krowiak]

On 04/19/2018 07:51 AM, Cornelia Huck wrote:
> On Sun, 15 Apr 2018 15:07:19 -0400
> Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
>
>>   default-configs/s390x-softmmu.mak |    1 +
>>   docs/vfio-ap.txt                  |  649 +++++++++++++++++++++++++++++++++++++
>>   hw/s390x/Makefile.objs            |    1 +
>>   hw/s390x/ap-device.c              |   39 +++
>>   hw/vfio/Makefile.objs             |    1 +
>>   hw/vfio/ap.c                      |  191 +++++++++++
>>   include/hw/s390x/ap-device.h      |   38 +++
>>   include/hw/vfio/vfio-common.h     |    1 +
>>   linux-headers/asm-s390/kvm.h      |    2 +
>>   linux-headers/linux/vfio.h        |    2 +
>>   target/s390x/cpu_features.c       |    3 +
>>   target/s390x/cpu_features_def.h   |    3 +
>>   target/s390x/cpu_models.c         |    2 +
>>   target/s390x/gen-features.c       |    3 +
>>   target/s390x/kvm.c                |    1 +
>>   ui/keycodemapdb                   |    2 +-
>>   16 files changed, 938 insertions(+), 1 deletions(-)
>>   create mode 100644 docs/vfio-ap.txt
>>   create mode 100644 hw/s390x/ap-device.c
>>   create mode 100644 hw/vfio/ap.c
>>   create mode 100644 include/hw/s390x/ap-device.h
> Can we please get a MAINTAINERS entry for ap? While I'm obviously happy
> to take things through the s390x tree, ap is not something I can easily
> review (or even test), so I'd like to see an entry with dedicated (IBM)
> maintainers.

Will do .... who would you like to see in this list?

>
> Also, hw/vfio/ap.c and docs/vfio-ap.c should be added to the generic
> s390 section.

Okay.

>

Re: [Qemu-devel] [PATCH v4 0/5] s390x: vfio-ap: guest dedicated crypto adapters

[Cornelia Huck]

On Sun, 22 Apr 2018 12:17:22 -0400
Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:

> On 04/19/2018 07:51 AM, Cornelia Huck wrote:
> > On Sun, 15 Apr 2018 15:07:19 -0400
> > Tony Krowiak <akrowiak@linux.vnet.ibm.com> wrote:
> >  
> >>   default-configs/s390x-softmmu.mak |    1 +
> >>   docs/vfio-ap.txt                  |  649 +++++++++++++++++++++++++++++++++++++
> >>   hw/s390x/Makefile.objs            |    1 +
> >>   hw/s390x/ap-device.c              |   39 +++
> >>   hw/vfio/Makefile.objs             |    1 +
> >>   hw/vfio/ap.c                      |  191 +++++++++++
> >>   include/hw/s390x/ap-device.h      |   38 +++
> >>   include/hw/vfio/vfio-common.h     |    1 +
> >>   linux-headers/asm-s390/kvm.h      |    2 +
> >>   linux-headers/linux/vfio.h        |    2 +
> >>   target/s390x/cpu_features.c       |    3 +
> >>   target/s390x/cpu_features_def.h   |    3 +
> >>   target/s390x/cpu_models.c         |    2 +
> >>   target/s390x/gen-features.c       |    3 +
> >>   target/s390x/kvm.c                |    1 +
> >>   ui/keycodemapdb                   |    2 +-
> >>   16 files changed, 938 insertions(+), 1 deletions(-)
> >>   create mode 100644 docs/vfio-ap.txt
> >>   create mode 100644 hw/s390x/ap-device.c
> >>   create mode 100644 hw/vfio/ap.c
> >>   create mode 100644 include/hw/s390x/ap-device.h  
> > Can we please get a MAINTAINERS entry for ap? While I'm obviously happy
> > to take things through the s390x tree, ap is not something I can easily
> > review (or even test), so I'd like to see an entry with dedicated (IBM)
> > maintainers.  
> 
> Will do .... who would you like to see in this list?

Whoever feels up to the task and has access to the documentation :)

> 
> >
> > Also, hw/vfio/ap.c and docs/vfio-ap.c should be added to the generic
> > s390 section.  
> 
> Okay.
> 
> >  
>