Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-04-03 Thread Tony Krowiak 

On 04/03/2018 06:57 AM, Cornelia Huck wrote:

On Wed, 14 Mar 2018 14:25:45 -0400
Tony Krowiak  wrote:


Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
use by KVM guests. This is accomplished by unbinding the
devices to be reserved for guest usage from the default AP
device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
interfaces required to create one or more VFIO mediated
devices each of which will be used to configure the AP
matrix for a guest and serve as a file descriptor
for facilitating communication between QEMU and the
VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
   and newer) AP queue devices. The probe and remove callbacks
   will be provided to support the binding/unbinding of
   AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
   the APQNs of the AP devices bound to the VFIO
   AP device driver and serves as the parent of the
   mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
  MAINTAINERS   |2 +
  arch/s390/Kconfig |   11 +++
  drivers/s390/crypto/Makefile  |4 +
  drivers/s390/crypto/vfio_ap_drv.c |  135 +
  drivers/s390/crypto/vfio_ap_private.h |   22 ++
  include/uapi/linux/vfio.h |2 +
  6 files changed, 176 insertions(+), 0 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
  create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
b/drivers/s390/crypto/vfio_ap_drv.c
new file mode 100644
index 000..459e595
--- /dev/null
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -0,0 +1,135 @@
+/*
+ * VFIO based AP device driver
+ *
+ * Copyright IBM Corp. 2017

Update to 2018?

Okay, will do.



+ *
+ * Author(s): Tony Krowiak 
+ */
+
+#include 
+#include 
+#include 
+
+#include "vfio_ap_private.h"
+
+#define VFIO_AP_ROOT_NAME "vfio_ap"
+#define VFIO_AP_DEV_TYPE_NAME "ap_matrix"
+#define VFIO_AP_DEV_NAME "matrix"
+
+MODULE_AUTHOR("IBM Corporation");
+MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017");
+MODULE_LICENSE("GPL v2");
+
+static struct device *vfio_ap_root_device;
+
+static struct ap_driver vfio_ap_drv;
+
+static struct ap_matrix *ap_matrix;
+
+static struct device_type vfio_ap_dev_type = {
+   .name = VFIO_AP_DEV_TYPE_NAME,
+};
+
+/* Only type 10 adapters (CEX4 and later) are supported
+ * by the AP matrix device driver
+ */
+static struct ap_device_id ap_queue_ids[] = {
+   { .dev_type = AP_DEVICE_TYPE_CEX4,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX5,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX6,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { /* end of sibling */ },
+};
+
+MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
+
+static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
+{
+   return 0;
+}
+
+static void vfio_ap_matrix_dev_release(struct device *dev)
+{
+   struct ap_matrix *ap_matrix = dev_get_drvdata(dev);
+
+   kfree(ap_matrix);
+}
+
+static int vfio_ap_matrix_dev_create(void)
+{
+   int ret;
+
+   vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME);
+
+   ret = IS_ERR(vfio_ap_root_device);
+   if (ret) {

Minor nit: I'd contract that to

if (IS_ERR(vfio_ap_root_device)) {

(you're writing ret in any case)

Okay, will do.



+   ret = PTR_ERR(vfio_ap_root_device);
+   goto done;
+   }
+
+   ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
+   if (!ap_matrix) {
+   ret = -ENOMEM;
+   goto matrix_alloc_err;
+   }
+
+   ap_matrix->device.type = _ap_dev_type;
+   dev_set_name(_matrix->device, "%s", VFIO_AP_DEV_NAME);
+   ap_matrix->device.parent = vfio_ap_root_device;
+   ap_matrix->device.release = vfio_ap_matrix_dev_release;
+   ap_matrix->device.driver = _ap_drv.driver;
+
+   ret = device_register(_matrix->device);
+   if (ret)
+   goto matrix_reg_err;
+
+   goto done;
+
+matrix_reg_err:
+   put_device(_matrix->device);
+   kfree(ap_matrix);

The kfree() is wrong: If you called device_register for the embedded
struct device, this needs to be handled via the ->release callback
exclusively (IOW, the put_device() is enough and the kfree needs to go).

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-04-03 Thread Tony Krowiak 

On 04/03/2018 06:57 AM, Cornelia Huck wrote:

On Wed, 14 Mar 2018 14:25:45 -0400
Tony Krowiak  wrote:


Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
use by KVM guests. This is accomplished by unbinding the
devices to be reserved for guest usage from the default AP
device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
interfaces required to create one or more VFIO mediated
devices each of which will be used to configure the AP
matrix for a guest and serve as a file descriptor
for facilitating communication between QEMU and the
VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
   and newer) AP queue devices. The probe and remove callbacks
   will be provided to support the binding/unbinding of
   AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
   the APQNs of the AP devices bound to the VFIO
   AP device driver and serves as the parent of the
   mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
  MAINTAINERS   |2 +
  arch/s390/Kconfig |   11 +++
  drivers/s390/crypto/Makefile  |4 +
  drivers/s390/crypto/vfio_ap_drv.c |  135 +
  drivers/s390/crypto/vfio_ap_private.h |   22 ++
  include/uapi/linux/vfio.h |2 +
  6 files changed, 176 insertions(+), 0 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
  create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
b/drivers/s390/crypto/vfio_ap_drv.c
new file mode 100644
index 000..459e595
--- /dev/null
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -0,0 +1,135 @@
+/*
+ * VFIO based AP device driver
+ *
+ * Copyright IBM Corp. 2017

Update to 2018?

Okay, will do.



+ *
+ * Author(s): Tony Krowiak 
+ */
+
+#include 
+#include 
+#include 
+
+#include "vfio_ap_private.h"
+
+#define VFIO_AP_ROOT_NAME "vfio_ap"
+#define VFIO_AP_DEV_TYPE_NAME "ap_matrix"
+#define VFIO_AP_DEV_NAME "matrix"
+
+MODULE_AUTHOR("IBM Corporation");
+MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017");
+MODULE_LICENSE("GPL v2");
+
+static struct device *vfio_ap_root_device;
+
+static struct ap_driver vfio_ap_drv;
+
+static struct ap_matrix *ap_matrix;
+
+static struct device_type vfio_ap_dev_type = {
+   .name = VFIO_AP_DEV_TYPE_NAME,
+};
+
+/* Only type 10 adapters (CEX4 and later) are supported
+ * by the AP matrix device driver
+ */
+static struct ap_device_id ap_queue_ids[] = {
+   { .dev_type = AP_DEVICE_TYPE_CEX4,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX5,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX6,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { /* end of sibling */ },
+};
+
+MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
+
+static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
+{
+   return 0;
+}
+
+static void vfio_ap_matrix_dev_release(struct device *dev)
+{
+   struct ap_matrix *ap_matrix = dev_get_drvdata(dev);
+
+   kfree(ap_matrix);
+}
+
+static int vfio_ap_matrix_dev_create(void)
+{
+   int ret;
+
+   vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME);
+
+   ret = IS_ERR(vfio_ap_root_device);
+   if (ret) {

Minor nit: I'd contract that to

if (IS_ERR(vfio_ap_root_device)) {

(you're writing ret in any case)

Okay, will do.



+   ret = PTR_ERR(vfio_ap_root_device);
+   goto done;
+   }
+
+   ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
+   if (!ap_matrix) {
+   ret = -ENOMEM;
+   goto matrix_alloc_err;
+   }
+
+   ap_matrix->device.type = _ap_dev_type;
+   dev_set_name(_matrix->device, "%s", VFIO_AP_DEV_NAME);
+   ap_matrix->device.parent = vfio_ap_root_device;
+   ap_matrix->device.release = vfio_ap_matrix_dev_release;
+   ap_matrix->device.driver = _ap_drv.driver;
+
+   ret = device_register(_matrix->device);
+   if (ret)
+   goto matrix_reg_err;
+
+   goto done;
+
+matrix_reg_err:
+   put_device(_matrix->device);
+   kfree(ap_matrix);

The kfree() is wrong: If you called device_register for the embedded
struct device, this needs to be handled via the ->release callback
exclusively (IOW, the put_device() is enough and the kfree needs to go).

Ah yes, I see that. I will fix it.



+
+matrix_alloc_err:
+

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-04-03 Thread Cornelia Huck 
On Wed, 14 Mar 2018 14:25:45 -0400
Tony Krowiak  wrote:

> Introduces a new AP device driver. This device driver
> is built on the VFIO mediated device framework. The framework
> provides sysfs interfaces that facilitate passthrough
> access by guests to devices installed on the linux host.
> 
> The VFIO AP device driver will serve two purposes:
> 
> 1. Provide the interfaces to reserve AP devices for exclusive
>use by KVM guests. This is accomplished by unbinding the
>devices to be reserved for guest usage from the default AP
>device driver and binding them to the VFIO AP device driver.
> 
> 2. Implements the functions, callbacks and sysfs attribute
>interfaces required to create one or more VFIO mediated
>devices each of which will be used to configure the AP
>matrix for a guest and serve as a file descriptor
>for facilitating communication between QEMU and the
>VFIO AP device driver.
> 
> When the VFIO AP device driver is initialized:
> 
> * It registers with the AP bus for control of type 10 (CEX4
>   and newer) AP queue devices. The probe and remove callbacks
>   will be provided to support the binding/unbinding of
>   AP queue devices to/from the VFIO AP device driver.
> 
> * Creates a /sys/devices/vfio-ap/matrix device to hold
>   the APQNs of the AP devices bound to the VFIO
>   AP device driver and serves as the parent of the
>   mediated devices created for each guest.
> 
> Signed-off-by: Tony Krowiak 
> ---
>  MAINTAINERS   |2 +
>  arch/s390/Kconfig |   11 +++
>  drivers/s390/crypto/Makefile  |4 +
>  drivers/s390/crypto/vfio_ap_drv.c |  135 
> +
>  drivers/s390/crypto/vfio_ap_private.h |   22 ++
>  include/uapi/linux/vfio.h |2 +
>  6 files changed, 176 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
>  create mode 100644 drivers/s390/crypto/vfio_ap_private.h
> 

> diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
> b/drivers/s390/crypto/vfio_ap_drv.c
> new file mode 100644
> index 000..459e595
> --- /dev/null
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -0,0 +1,135 @@
> +/*
> + * VFIO based AP device driver
> + *
> + * Copyright IBM Corp. 2017

Update to 2018?

> + *
> + * Author(s): Tony Krowiak 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "vfio_ap_private.h"
> +
> +#define VFIO_AP_ROOT_NAME "vfio_ap"
> +#define VFIO_AP_DEV_TYPE_NAME "ap_matrix"
> +#define VFIO_AP_DEV_NAME "matrix"
> +
> +MODULE_AUTHOR("IBM Corporation");
> +MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017");
> +MODULE_LICENSE("GPL v2");
> +
> +static struct device *vfio_ap_root_device;
> +
> +static struct ap_driver vfio_ap_drv;
> +
> +static struct ap_matrix *ap_matrix;
> +
> +static struct device_type vfio_ap_dev_type = {
> + .name = VFIO_AP_DEV_TYPE_NAME,
> +};
> +
> +/* Only type 10 adapters (CEX4 and later) are supported
> + * by the AP matrix device driver
> + */
> +static struct ap_device_id ap_queue_ids[] = {
> + { .dev_type = AP_DEVICE_TYPE_CEX4,
> +   .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
> + { .dev_type = AP_DEVICE_TYPE_CEX5,
> +   .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
> + { .dev_type = AP_DEVICE_TYPE_CEX6,
> +   .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
> + { /* end of sibling */ },
> +};
> +
> +MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
> +
> +static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
> +{
> + return 0;
> +}
> +
> +static void vfio_ap_matrix_dev_release(struct device *dev)
> +{
> + struct ap_matrix *ap_matrix = dev_get_drvdata(dev);
> +
> + kfree(ap_matrix);
> +}
> +
> +static int vfio_ap_matrix_dev_create(void)
> +{
> + int ret;
> +
> + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME);
> +
> + ret = IS_ERR(vfio_ap_root_device);
> + if (ret) {

Minor nit: I'd contract that to

if (IS_ERR(vfio_ap_root_device)) {

(you're writing ret in any case)

> + ret = PTR_ERR(vfio_ap_root_device);
> + goto done;
> + }
> +
> + ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
> + if (!ap_matrix) {
> + ret = -ENOMEM;
> + goto matrix_alloc_err;
> + }
> +
> + ap_matrix->device.type = _ap_dev_type;
> + dev_set_name(_matrix->device, "%s", VFIO_AP_DEV_NAME);
> + ap_matrix->device.parent = vfio_ap_root_device;
> + ap_matrix->device.release = vfio_ap_matrix_dev_release;
> + ap_matrix->device.driver = _ap_drv.driver;
> +
> + ret = device_register(_matrix->device);
> + if (ret)
> + goto matrix_reg_err;
> +
> + goto done;
> +
> +matrix_reg_err:
> + put_device(_matrix->device);
> + kfree(ap_matrix);

The kfree() is wrong: If you called device_register for the embedded
struct device, this needs

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-04-03 Thread Cornelia Huck 
On Wed, 14 Mar 2018 14:25:45 -0400
Tony Krowiak  wrote:

> Introduces a new AP device driver. This device driver
> is built on the VFIO mediated device framework. The framework
> provides sysfs interfaces that facilitate passthrough
> access by guests to devices installed on the linux host.
> 
> The VFIO AP device driver will serve two purposes:
> 
> 1. Provide the interfaces to reserve AP devices for exclusive
>use by KVM guests. This is accomplished by unbinding the
>devices to be reserved for guest usage from the default AP
>device driver and binding them to the VFIO AP device driver.
> 
> 2. Implements the functions, callbacks and sysfs attribute
>interfaces required to create one or more VFIO mediated
>devices each of which will be used to configure the AP
>matrix for a guest and serve as a file descriptor
>for facilitating communication between QEMU and the
>VFIO AP device driver.
> 
> When the VFIO AP device driver is initialized:
> 
> * It registers with the AP bus for control of type 10 (CEX4
>   and newer) AP queue devices. The probe and remove callbacks
>   will be provided to support the binding/unbinding of
>   AP queue devices to/from the VFIO AP device driver.
> 
> * Creates a /sys/devices/vfio-ap/matrix device to hold
>   the APQNs of the AP devices bound to the VFIO
>   AP device driver and serves as the parent of the
>   mediated devices created for each guest.
> 
> Signed-off-by: Tony Krowiak 
> ---
>  MAINTAINERS   |2 +
>  arch/s390/Kconfig |   11 +++
>  drivers/s390/crypto/Makefile  |4 +
>  drivers/s390/crypto/vfio_ap_drv.c |  135 
> +
>  drivers/s390/crypto/vfio_ap_private.h |   22 ++
>  include/uapi/linux/vfio.h |2 +
>  6 files changed, 176 insertions(+), 0 deletions(-)
>  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
>  create mode 100644 drivers/s390/crypto/vfio_ap_private.h
> 

> diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
> b/drivers/s390/crypto/vfio_ap_drv.c
> new file mode 100644
> index 000..459e595
> --- /dev/null
> +++ b/drivers/s390/crypto/vfio_ap_drv.c
> @@ -0,0 +1,135 @@
> +/*
> + * VFIO based AP device driver
> + *
> + * Copyright IBM Corp. 2017

Update to 2018?

> + *
> + * Author(s): Tony Krowiak 
> + */
> +
> +#include 
> +#include 
> +#include 
> +
> +#include "vfio_ap_private.h"
> +
> +#define VFIO_AP_ROOT_NAME "vfio_ap"
> +#define VFIO_AP_DEV_TYPE_NAME "ap_matrix"
> +#define VFIO_AP_DEV_NAME "matrix"
> +
> +MODULE_AUTHOR("IBM Corporation");
> +MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017");
> +MODULE_LICENSE("GPL v2");
> +
> +static struct device *vfio_ap_root_device;
> +
> +static struct ap_driver vfio_ap_drv;
> +
> +static struct ap_matrix *ap_matrix;
> +
> +static struct device_type vfio_ap_dev_type = {
> + .name = VFIO_AP_DEV_TYPE_NAME,
> +};
> +
> +/* Only type 10 adapters (CEX4 and later) are supported
> + * by the AP matrix device driver
> + */
> +static struct ap_device_id ap_queue_ids[] = {
> + { .dev_type = AP_DEVICE_TYPE_CEX4,
> +   .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
> + { .dev_type = AP_DEVICE_TYPE_CEX5,
> +   .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
> + { .dev_type = AP_DEVICE_TYPE_CEX6,
> +   .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
> + { /* end of sibling */ },
> +};
> +
> +MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
> +
> +static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
> +{
> + return 0;
> +}
> +
> +static void vfio_ap_matrix_dev_release(struct device *dev)
> +{
> + struct ap_matrix *ap_matrix = dev_get_drvdata(dev);
> +
> + kfree(ap_matrix);
> +}
> +
> +static int vfio_ap_matrix_dev_create(void)
> +{
> + int ret;
> +
> + vfio_ap_root_device = root_device_register(VFIO_AP_ROOT_NAME);
> +
> + ret = IS_ERR(vfio_ap_root_device);
> + if (ret) {

Minor nit: I'd contract that to

if (IS_ERR(vfio_ap_root_device)) {

(you're writing ret in any case)

> + ret = PTR_ERR(vfio_ap_root_device);
> + goto done;
> + }
> +
> + ap_matrix = kzalloc(sizeof(*ap_matrix), GFP_KERNEL);
> + if (!ap_matrix) {
> + ret = -ENOMEM;
> + goto matrix_alloc_err;
> + }
> +
> + ap_matrix->device.type = _ap_dev_type;
> + dev_set_name(_matrix->device, "%s", VFIO_AP_DEV_NAME);
> + ap_matrix->device.parent = vfio_ap_root_device;
> + ap_matrix->device.release = vfio_ap_matrix_dev_release;
> + ap_matrix->device.driver = _ap_drv.driver;
> +
> + ret = device_register(_matrix->device);
> + if (ret)
> + goto matrix_reg_err;
> +
> + goto done;
> +
> +matrix_reg_err:
> + put_device(_matrix->device);
> + kfree(ap_matrix);

The kfree() is wrong: If you called device_register for the embedded
struct device, this needs to be handled via the ->release callback
exclusively (IOW, the put_device() is enough

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-04-03 Thread Cornelia Huck 
On Tue, 27 Mar 2018 16:45:02 +0200
Pierre Morel  wrote:

> On 27/03/2018 13:17, Cornelia Huck wrote:
> > On Thu, 15 Mar 2018 13:25:25 -0400
> > Tony Krowiak  wrote:
> >  
> >> On 03/15/2018 09:25 AM, Pierre Morel wrote:  
> >>> On 14/03/2018 19:25, Tony Krowiak wrote:  
>  +config VFIO_AP
>  +def_tristate m  
> >>> not sure it must be module by default.
> >>> I would not set it by default.  
> >> Connie also asked about this in the last review, so I will go ahead
> >> and change it.  
> >>> 
>  +prompt "VFIO support for AP devices"
>  +depends on ZCRYPT && VFIO_MDEV_DEVICE  
> >>> VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
> >>> and has no use case by its own. If it is set it is obviously because some
> >>> mediated device drivers needs it.
> >>> while ZCRYPT is a Z feature which may be set without VFIO_AP.
> >>>
> >>> So you need:
> >>>
> >>> config VFIO_AP
> >>>  def_tristate n
> >>>  prompt "VFIO support for AP devices"
> >>>  depends on ZCRYPT
> >>>  select VFIO_MDEV
> >>>  select VFIO_MDEV_DEVICE
> >>> ...  
> >> I was thinking the same just yesterday and I agree, this makes sense.  
> > OTOH, nobody else seems to do a select on these symbols so far.
> >
> > If you decide to go that route, you'll also need to depend on VFIO  
> 
> I think a select is better (again).
> 
> > (otherwise you could end up selecting symbols with unmet dependencies).
> > All in all, I prefer the 'depends' approach.
> >  
> Why do you prefer this approach?

Hm, I thought I had already written a mail, but apparently I didn't
 
> I can tell you why I prefer a mixed approach:
> 
> We have two tools, depends and select.
> 
> It seems to me that depends should be used for things we can not choose 
> to be there or not, but things that just are there, like hardware 
> dependencies. For example MMU, CPU type, CRYPTO hardware...
> 
> Select on the other hand is useful to choose things that we need like 
> libraries, VFIO, VIRTIO, crypto libraries etc.
> 
> Using this policy is clear and makes easy to choose functionalities and 
> get the utilities automatically.
> 
> On the other hand, only using depends makes things to hide the 
> functionalities behind the utilities.

My view is the following:
- select is useful for library functionality or for enabling
  architecture-specific optimizations (the HAVE_xxx symbols),
  especially things you don't want the user to deal with. If you select
  something, you need to take care of any dependencies yourself.
- depends is useful for more complex dependencies, and especially
  things you don't want automagically enabled. [In modern menuconfig,
  it is easy to figure out any missing dependencies for a config option
  anyway.]

The mdev infrastructure is too complex to be considered a simple
library IMO (cf. the missing VFIO dependency).

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-04-03 Thread Cornelia Huck 
On Tue, 27 Mar 2018 16:45:02 +0200
Pierre Morel  wrote:

> On 27/03/2018 13:17, Cornelia Huck wrote:
> > On Thu, 15 Mar 2018 13:25:25 -0400
> > Tony Krowiak  wrote:
> >  
> >> On 03/15/2018 09:25 AM, Pierre Morel wrote:  
> >>> On 14/03/2018 19:25, Tony Krowiak wrote:  
>  +config VFIO_AP
>  +def_tristate m  
> >>> not sure it must be module by default.
> >>> I would not set it by default.  
> >> Connie also asked about this in the last review, so I will go ahead
> >> and change it.  
> >>> 
>  +prompt "VFIO support for AP devices"
>  +depends on ZCRYPT && VFIO_MDEV_DEVICE  
> >>> VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
> >>> and has no use case by its own. If it is set it is obviously because some
> >>> mediated device drivers needs it.
> >>> while ZCRYPT is a Z feature which may be set without VFIO_AP.
> >>>
> >>> So you need:
> >>>
> >>> config VFIO_AP
> >>>  def_tristate n
> >>>  prompt "VFIO support for AP devices"
> >>>  depends on ZCRYPT
> >>>  select VFIO_MDEV
> >>>  select VFIO_MDEV_DEVICE
> >>> ...  
> >> I was thinking the same just yesterday and I agree, this makes sense.  
> > OTOH, nobody else seems to do a select on these symbols so far.
> >
> > If you decide to go that route, you'll also need to depend on VFIO  
> 
> I think a select is better (again).
> 
> > (otherwise you could end up selecting symbols with unmet dependencies).
> > All in all, I prefer the 'depends' approach.
> >  
> Why do you prefer this approach?

Hm, I thought I had already written a mail, but apparently I didn't
 
> I can tell you why I prefer a mixed approach:
> 
> We have two tools, depends and select.
> 
> It seems to me that depends should be used for things we can not choose 
> to be there or not, but things that just are there, like hardware 
> dependencies. For example MMU, CPU type, CRYPTO hardware...
> 
> Select on the other hand is useful to choose things that we need like 
> libraries, VFIO, VIRTIO, crypto libraries etc.
> 
> Using this policy is clear and makes easy to choose functionalities and 
> get the utilities automatically.
> 
> On the other hand, only using depends makes things to hide the 
> functionalities behind the utilities.

My view is the following:
- select is useful for library functionality or for enabling
  architecture-specific optimizations (the HAVE_xxx symbols),
  especially things you don't want the user to deal with. If you select
  something, you need to take care of any dependencies yourself.
- depends is useful for more complex dependencies, and especially
  things you don't want automagically enabled. [In modern menuconfig,
  it is easy to figure out any missing dependencies for a config option
  anyway.]

The mdev infrastructure is too complex to be considered a simple
library IMO (cf. the missing VFIO dependency).

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-27 Thread Pierre Morel 

On 27/03/2018 13:17, Cornelia Huck wrote:

On Thu, 15 Mar 2018 13:25:25 -0400
Tony Krowiak  wrote:


On 03/15/2018 09:25 AM, Pierre Morel wrote:

On 14/03/2018 19:25, Tony Krowiak wrote:

+config VFIO_AP
+def_tristate m

not sure it must be module by default.
I would not set it by default.

Connie also asked about this in the last review, so I will go ahead
and change it.
  

+prompt "VFIO support for AP devices"
+depends on ZCRYPT && VFIO_MDEV_DEVICE

VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
and has no use case by its own. If it is set it is obviously because some
mediated device drivers needs it.
while ZCRYPT is a Z feature which may be set without VFIO_AP.

So you need:

config VFIO_AP
 def_tristate n
 prompt "VFIO support for AP devices"
 depends on ZCRYPT
 select VFIO_MDEV
 select VFIO_MDEV_DEVICE
...

I was thinking the same just yesterday and I agree, this makes sense.

OTOH, nobody else seems to do a select on these symbols so far.

If you decide to go that route, you'll also need to depend on VFIO


I think a select is better (again).


(otherwise you could end up selecting symbols with unmet dependencies).
All in all, I prefer the 'depends' approach.


Why do you prefer this approach?


I can tell you why I prefer a mixed approach:

We have two tools, depends and select.

It seems to me that depends should be used for things we can not choose 
to be there or not, but things that just are there, like hardware 
dependencies. For example MMU, CPU type, CRYPTO hardware...


Select on the other hand is useful to choose things that we need like 
libraries, VFIO, VIRTIO, crypto libraries etc.


Using this policy is clear and makes easy to choose functionalities and 
get the utilities automatically.


On the other hand, only using depends makes things to hide the 
functionalities behind the utilities.



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

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-27 Thread Pierre Morel 

On 27/03/2018 13:17, Cornelia Huck wrote:

On Thu, 15 Mar 2018 13:25:25 -0400
Tony Krowiak  wrote:


On 03/15/2018 09:25 AM, Pierre Morel wrote:

On 14/03/2018 19:25, Tony Krowiak wrote:

+config VFIO_AP
+def_tristate m

not sure it must be module by default.
I would not set it by default.

Connie also asked about this in the last review, so I will go ahead
and change it.
  

+prompt "VFIO support for AP devices"
+depends on ZCRYPT && VFIO_MDEV_DEVICE

VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
and has no use case by its own. If it is set it is obviously because some
mediated device drivers needs it.
while ZCRYPT is a Z feature which may be set without VFIO_AP.

So you need:

config VFIO_AP
 def_tristate n
 prompt "VFIO support for AP devices"
 depends on ZCRYPT
 select VFIO_MDEV
 select VFIO_MDEV_DEVICE
...

I was thinking the same just yesterday and I agree, this makes sense.

OTOH, nobody else seems to do a select on these symbols so far.

If you decide to go that route, you'll also need to depend on VFIO


I think a select is better (again).


(otherwise you could end up selecting symbols with unmet dependencies).
All in all, I prefer the 'depends' approach.


Why do you prefer this approach?


I can tell you why I prefer a mixed approach:

We have two tools, depends and select.

It seems to me that depends should be used for things we can not choose 
to be there or not, but things that just are there, like hardware 
dependencies. For example MMU, CPU type, CRYPTO hardware...


Select on the other hand is useful to choose things that we need like 
libraries, VFIO, VIRTIO, crypto libraries etc.


Using this policy is clear and makes easy to choose functionalities and 
get the utilities automatically.


On the other hand, only using depends makes things to hide the 
functionalities behind the utilities.



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

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-27 Thread Cornelia Huck 
On Thu, 15 Mar 2018 13:25:25 -0400
Tony Krowiak  wrote:

> On 03/15/2018 09:25 AM, Pierre Morel wrote:
> > On 14/03/2018 19:25, Tony Krowiak wrote:  

> >> +config VFIO_AP
> >> +def_tristate m  
> > not sure it must be module by default.
> > I would not set it by default.  
> Connie also asked about this in the last review, so I will go ahead
> and change it.
> >  
> >> +prompt "VFIO support for AP devices"
> >> +depends on ZCRYPT && VFIO_MDEV_DEVICE  
> >
> > VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
> > and has no use case by its own. If it is set it is obviously because some
> > mediated device drivers needs it.
> > while ZCRYPT is a Z feature which may be set without VFIO_AP.
> >
> > So you need:
> >
> > config VFIO_AP
> > def_tristate n
> > prompt "VFIO support for AP devices"
> > depends on ZCRYPT
> > select VFIO_MDEV
> > select VFIO_MDEV_DEVICE
> > ...  
> I was thinking the same just yesterday and I agree, this makes sense.

OTOH, nobody else seems to do a select on these symbols so far.

If you decide to go that route, you'll also need to depend on VFIO
(otherwise you could end up selecting symbols with unmet dependencies).
All in all, I prefer the 'depends' approach.

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-27 Thread Cornelia Huck 
On Thu, 15 Mar 2018 13:25:25 -0400
Tony Krowiak  wrote:

> On 03/15/2018 09:25 AM, Pierre Morel wrote:
> > On 14/03/2018 19:25, Tony Krowiak wrote:  

> >> +config VFIO_AP
> >> +def_tristate m  
> > not sure it must be module by default.
> > I would not set it by default.  
> Connie also asked about this in the last review, so I will go ahead
> and change it.
> >  
> >> +prompt "VFIO support for AP devices"
> >> +depends on ZCRYPT && VFIO_MDEV_DEVICE  
> >
> > VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
> > and has no use case by its own. If it is set it is obviously because some
> > mediated device drivers needs it.
> > while ZCRYPT is a Z feature which may be set without VFIO_AP.
> >
> > So you need:
> >
> > config VFIO_AP
> > def_tristate n
> > prompt "VFIO support for AP devices"
> > depends on ZCRYPT
> > select VFIO_MDEV
> > select VFIO_MDEV_DEVICE
> > ...  
> I was thinking the same just yesterday and I agree, this makes sense.

OTOH, nobody else seems to do a select on these symbols so far.

If you decide to go that route, you'll also need to depend on VFIO
(otherwise you could end up selecting symbols with unmet dependencies).
All in all, I prefer the 'depends' approach.

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-15 Thread Tony Krowiak 

On 03/15/2018 09:25 AM, Pierre Morel wrote:

On 14/03/2018 19:25, Tony Krowiak wrote:

Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
use by KVM guests. This is accomplished by unbinding the
devices to be reserved for guest usage from the default AP
device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
interfaces required to create one or more VFIO mediated
devices each of which will be used to configure the AP
matrix for a guest and serve as a file descriptor
for facilitating communication between QEMU and the
VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
   and newer) AP queue devices. The probe and remove callbacks
   will be provided to support the binding/unbinding of
   AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
   the APQNs of the AP devices bound to the VFIO
   AP device driver and serves as the parent of the
   mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
  MAINTAINERS   |2 +
  arch/s390/Kconfig |   11 +++
  drivers/s390/crypto/Makefile  |4 +
  drivers/s390/crypto/vfio_ap_drv.c |  135 
+

  drivers/s390/crypto/vfio_ap_private.h |   22 ++
  include/uapi/linux/vfio.h |2 +
  6 files changed, 176 insertions(+), 0 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
  create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 72742d5..f129253 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11884,6 +11884,8 @@ W: 
http://www.ibm.com/developerworks/linux/linux390/

  S:Supported
  F:arch/s390/include/asm/kvm/kvm-ap.h
  F:arch/s390/kvm/kvm-ap.c
+F:drivers/s390/crypto/vfio_ap_drv.c
+F:drivers/s390/crypto/vfio_ap_private.h

  S390 ZFCP DRIVER
  M:Steffen Maier 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index cbe1d97..58509db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -771,6 +771,17 @@ config VFIO_CCW
To compile this driver as a module, choose M here: the
module will be called vfio_ccw.

+config VFIO_AP
+def_tristate m

not sure it must be module by default.
I would not set it by default.

Connie also asked about this in the last review, so I will go ahead
and change it.



+prompt "VFIO support for AP devices"
+depends on ZCRYPT && VFIO_MDEV_DEVICE


VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
and has no use case by its own. If it is set it is obviously because some
mediated device drivers needs it.
while ZCRYPT is a Z feature which may be set without VFIO_AP.

So you need:

config VFIO_AP
def_tristate n
prompt "VFIO support for AP devices"
depends on ZCRYPT
select VFIO_MDEV
select VFIO_MDEV_DEVICE
...

I was thinking the same just yesterday and I agree, this makes sense.




Pierre

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-15 Thread Tony Krowiak 

On 03/15/2018 09:25 AM, Pierre Morel wrote:

On 14/03/2018 19:25, Tony Krowiak wrote:

Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
use by KVM guests. This is accomplished by unbinding the
devices to be reserved for guest usage from the default AP
device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
interfaces required to create one or more VFIO mediated
devices each of which will be used to configure the AP
matrix for a guest and serve as a file descriptor
for facilitating communication between QEMU and the
VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
   and newer) AP queue devices. The probe and remove callbacks
   will be provided to support the binding/unbinding of
   AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
   the APQNs of the AP devices bound to the VFIO
   AP device driver and serves as the parent of the
   mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
  MAINTAINERS   |2 +
  arch/s390/Kconfig |   11 +++
  drivers/s390/crypto/Makefile  |4 +
  drivers/s390/crypto/vfio_ap_drv.c |  135 
+

  drivers/s390/crypto/vfio_ap_private.h |   22 ++
  include/uapi/linux/vfio.h |2 +
  6 files changed, 176 insertions(+), 0 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
  create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 72742d5..f129253 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11884,6 +11884,8 @@ W: 
http://www.ibm.com/developerworks/linux/linux390/

  S:Supported
  F:arch/s390/include/asm/kvm/kvm-ap.h
  F:arch/s390/kvm/kvm-ap.c
+F:drivers/s390/crypto/vfio_ap_drv.c
+F:drivers/s390/crypto/vfio_ap_private.h

  S390 ZFCP DRIVER
  M:Steffen Maier 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index cbe1d97..58509db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -771,6 +771,17 @@ config VFIO_CCW
To compile this driver as a module, choose M here: the
module will be called vfio_ccw.

+config VFIO_AP
+def_tristate m

not sure it must be module by default.
I would not set it by default.

Connie also asked about this in the last review, so I will go ahead
and change it.



+prompt "VFIO support for AP devices"
+depends on ZCRYPT && VFIO_MDEV_DEVICE


VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
and has no use case by its own. If it is set it is obviously because some
mediated device drivers needs it.
while ZCRYPT is a Z feature which may be set without VFIO_AP.

So you need:

config VFIO_AP
def_tristate n
prompt "VFIO support for AP devices"
depends on ZCRYPT
select VFIO_MDEV
select VFIO_MDEV_DEVICE
...

I was thinking the same just yesterday and I agree, this makes sense.




Pierre

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-15 Thread Pierre Morel 

On 14/03/2018 19:25, Tony Krowiak wrote:

Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
use by KVM guests. This is accomplished by unbinding the
devices to be reserved for guest usage from the default AP
device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
interfaces required to create one or more VFIO mediated
devices each of which will be used to configure the AP
matrix for a guest and serve as a file descriptor
for facilitating communication between QEMU and the
VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
   and newer) AP queue devices. The probe and remove callbacks
   will be provided to support the binding/unbinding of
   AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
   the APQNs of the AP devices bound to the VFIO
   AP device driver and serves as the parent of the
   mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
  MAINTAINERS   |2 +
  arch/s390/Kconfig |   11 +++
  drivers/s390/crypto/Makefile  |4 +
  drivers/s390/crypto/vfio_ap_drv.c |  135 +
  drivers/s390/crypto/vfio_ap_private.h |   22 ++
  include/uapi/linux/vfio.h |2 +
  6 files changed, 176 insertions(+), 0 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
  create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 72742d5..f129253 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11884,6 +11884,8 @@ W:  
http://www.ibm.com/developerworks/linux/linux390/
  S:Supported
  F:arch/s390/include/asm/kvm/kvm-ap.h
  F:arch/s390/kvm/kvm-ap.c
+F: drivers/s390/crypto/vfio_ap_drv.c
+F: drivers/s390/crypto/vfio_ap_private.h

  S390 ZFCP DRIVER
  M:Steffen Maier 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index cbe1d97..58509db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -771,6 +771,17 @@ config VFIO_CCW
  To compile this driver as a module, choose M here: the
  module will be called vfio_ccw.

+config VFIO_AP
+   def_tristate m

not sure it must be module by default.
I would not set it by default.

+   prompt "VFIO support for AP devices"
+   depends on ZCRYPT && VFIO_MDEV_DEVICE


VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
and has no use case by its own. If it is set it is obviously because some
mediated device drivers needs it.
while ZCRYPT is a Z feature which may be set without VFIO_AP.

So you need:

config VFIO_AP
    def_tristate n
    prompt "VFIO support for AP devices"
    depends on ZCRYPT
    select VFIO_MDEV
    select VFIO_MDEV_DEVICE
...


Pierre

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

Re: [PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-15 Thread Pierre Morel 

On 14/03/2018 19:25, Tony Krowiak wrote:

Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
use by KVM guests. This is accomplished by unbinding the
devices to be reserved for guest usage from the default AP
device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
interfaces required to create one or more VFIO mediated
devices each of which will be used to configure the AP
matrix for a guest and serve as a file descriptor
for facilitating communication between QEMU and the
VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
   and newer) AP queue devices. The probe and remove callbacks
   will be provided to support the binding/unbinding of
   AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
   the APQNs of the AP devices bound to the VFIO
   AP device driver and serves as the parent of the
   mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
  MAINTAINERS   |2 +
  arch/s390/Kconfig |   11 +++
  drivers/s390/crypto/Makefile  |4 +
  drivers/s390/crypto/vfio_ap_drv.c |  135 +
  drivers/s390/crypto/vfio_ap_private.h |   22 ++
  include/uapi/linux/vfio.h |2 +
  6 files changed, 176 insertions(+), 0 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
  create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 72742d5..f129253 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11884,6 +11884,8 @@ W:  
http://www.ibm.com/developerworks/linux/linux390/
  S:Supported
  F:arch/s390/include/asm/kvm/kvm-ap.h
  F:arch/s390/kvm/kvm-ap.c
+F: drivers/s390/crypto/vfio_ap_drv.c
+F: drivers/s390/crypto/vfio_ap_private.h

  S390 ZFCP DRIVER
  M:Steffen Maier 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index cbe1d97..58509db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -771,6 +771,17 @@ config VFIO_CCW
  To compile this driver as a module, choose M here: the
  module will be called vfio_ccw.

+config VFIO_AP
+   def_tristate m

not sure it must be module by default.
I would not set it by default.

+   prompt "VFIO support for AP devices"
+   depends on ZCRYPT && VFIO_MDEV_DEVICE


VFIO_MDEV_DEVICE is a general feature *needed* by VFIO_AP
and has no use case by its own. If it is set it is obviously because some
mediated device drivers needs it.
while ZCRYPT is a Z feature which may be set without VFIO_AP.

So you need:

config VFIO_AP
    def_tristate n
    prompt "VFIO support for AP devices"
    depends on ZCRYPT
    select VFIO_MDEV
    select VFIO_MDEV_DEVICE
...


Pierre

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

[PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-14 Thread Tony Krowiak 
Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
   use by KVM guests. This is accomplished by unbinding the
   devices to be reserved for guest usage from the default AP
   device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
   interfaces required to create one or more VFIO mediated
   devices each of which will be used to configure the AP
   matrix for a guest and serve as a file descriptor
   for facilitating communication between QEMU and the
   VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
  and newer) AP queue devices. The probe and remove callbacks
  will be provided to support the binding/unbinding of
  AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
  the APQNs of the AP devices bound to the VFIO
  AP device driver and serves as the parent of the
  mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
 MAINTAINERS   |2 +
 arch/s390/Kconfig |   11 +++
 drivers/s390/crypto/Makefile  |4 +
 drivers/s390/crypto/vfio_ap_drv.c |  135 +
 drivers/s390/crypto/vfio_ap_private.h |   22 ++
 include/uapi/linux/vfio.h |2 +
 6 files changed, 176 insertions(+), 0 deletions(-)
 create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
 create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 72742d5..f129253 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11884,6 +11884,8 @@ W:  
http://www.ibm.com/developerworks/linux/linux390/
 S: Supported
 F: arch/s390/include/asm/kvm/kvm-ap.h
 F: arch/s390/kvm/kvm-ap.c
+F: drivers/s390/crypto/vfio_ap_drv.c
+F: drivers/s390/crypto/vfio_ap_private.h
 
 S390 ZFCP DRIVER
 M: Steffen Maier 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index cbe1d97..58509db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -771,6 +771,17 @@ config VFIO_CCW
  To compile this driver as a module, choose M here: the
  module will be called vfio_ccw.
 
+config VFIO_AP
+   def_tristate m
+   prompt "VFIO support for AP devices"
+   depends on ZCRYPT && VFIO_MDEV_DEVICE
+   help
+   This driver grants access to Adjunct Processor (AP) devices
+   via the VFIO mediated device interface.
+
+   To compile this driver as a module, choose M here: the module
+   will be called vfio_ap.
+
 endmenu
 
 menu "Dump support"
diff --git a/drivers/s390/crypto/Makefile b/drivers/s390/crypto/Makefile
index b59af54..48e466e 100644
--- a/drivers/s390/crypto/Makefile
+++ b/drivers/s390/crypto/Makefile
@@ -15,3 +15,7 @@ obj-$(CONFIG_ZCRYPT) += zcrypt_pcixcc.o zcrypt_cex2a.o 
zcrypt_cex4.o
 # pkey kernel module
 pkey-objs := pkey_api.o
 obj-$(CONFIG_PKEY) += pkey.o
+
+# adjunct processor matrix
+vfio_ap-objs := vfio_ap_drv.o
+obj-$(CONFIG_VFIO_AP) += vfio_ap.o
diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
b/drivers/s390/crypto/vfio_ap_drv.c
new file mode 100644
index 000..459e595
--- /dev/null
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -0,0 +1,135 @@
+/*
+ * VFIO based AP device driver
+ *
+ * Copyright IBM Corp. 2017
+ *
+ * Author(s): Tony Krowiak 
+ */
+
+#include 
+#include 
+#include 
+
+#include "vfio_ap_private.h"
+
+#define VFIO_AP_ROOT_NAME "vfio_ap"
+#define VFIO_AP_DEV_TYPE_NAME "ap_matrix"
+#define VFIO_AP_DEV_NAME "matrix"
+
+MODULE_AUTHOR("IBM Corporation");
+MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017");
+MODULE_LICENSE("GPL v2");
+
+static struct device *vfio_ap_root_device;
+
+static struct ap_driver vfio_ap_drv;
+
+static struct ap_matrix *ap_matrix;
+
+static struct device_type vfio_ap_dev_type = {
+   .name = VFIO_AP_DEV_TYPE_NAME,
+};
+
+/* Only type 10 adapters (CEX4 and later) are supported
+ * by the AP matrix device driver
+ */
+static struct ap_device_id ap_queue_ids[] = {
+   { .dev_type = AP_DEVICE_TYPE_CEX4,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX5,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX6,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { /* end of sibling */ },
+};
+
+MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
+
+static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
+{
+   return 0;
+}
+
+static void vfio_ap_matrix_dev_release(struct device

[PATCH v3 05/14] s390: vfio-ap: base implementation of VFIO AP device driver

2018-03-14 Thread Tony Krowiak 
Introduces a new AP device driver. This device driver
is built on the VFIO mediated device framework. The framework
provides sysfs interfaces that facilitate passthrough
access by guests to devices installed on the linux host.

The VFIO AP device driver will serve two purposes:

1. Provide the interfaces to reserve AP devices for exclusive
   use by KVM guests. This is accomplished by unbinding the
   devices to be reserved for guest usage from the default AP
   device driver and binding them to the VFIO AP device driver.

2. Implements the functions, callbacks and sysfs attribute
   interfaces required to create one or more VFIO mediated
   devices each of which will be used to configure the AP
   matrix for a guest and serve as a file descriptor
   for facilitating communication between QEMU and the
   VFIO AP device driver.

When the VFIO AP device driver is initialized:

* It registers with the AP bus for control of type 10 (CEX4
  and newer) AP queue devices. The probe and remove callbacks
  will be provided to support the binding/unbinding of
  AP queue devices to/from the VFIO AP device driver.

* Creates a /sys/devices/vfio-ap/matrix device to hold
  the APQNs of the AP devices bound to the VFIO
  AP device driver and serves as the parent of the
  mediated devices created for each guest.

Signed-off-by: Tony Krowiak 
---
 MAINTAINERS   |2 +
 arch/s390/Kconfig |   11 +++
 drivers/s390/crypto/Makefile  |4 +
 drivers/s390/crypto/vfio_ap_drv.c |  135 +
 drivers/s390/crypto/vfio_ap_private.h |   22 ++
 include/uapi/linux/vfio.h |2 +
 6 files changed, 176 insertions(+), 0 deletions(-)
 create mode 100644 drivers/s390/crypto/vfio_ap_drv.c
 create mode 100644 drivers/s390/crypto/vfio_ap_private.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 72742d5..f129253 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -11884,6 +11884,8 @@ W:  
http://www.ibm.com/developerworks/linux/linux390/
 S: Supported
 F: arch/s390/include/asm/kvm/kvm-ap.h
 F: arch/s390/kvm/kvm-ap.c
+F: drivers/s390/crypto/vfio_ap_drv.c
+F: drivers/s390/crypto/vfio_ap_private.h
 
 S390 ZFCP DRIVER
 M: Steffen Maier 
diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index cbe1d97..58509db 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -771,6 +771,17 @@ config VFIO_CCW
  To compile this driver as a module, choose M here: the
  module will be called vfio_ccw.
 
+config VFIO_AP
+   def_tristate m
+   prompt "VFIO support for AP devices"
+   depends on ZCRYPT && VFIO_MDEV_DEVICE
+   help
+   This driver grants access to Adjunct Processor (AP) devices
+   via the VFIO mediated device interface.
+
+   To compile this driver as a module, choose M here: the module
+   will be called vfio_ap.
+
 endmenu
 
 menu "Dump support"
diff --git a/drivers/s390/crypto/Makefile b/drivers/s390/crypto/Makefile
index b59af54..48e466e 100644
--- a/drivers/s390/crypto/Makefile
+++ b/drivers/s390/crypto/Makefile
@@ -15,3 +15,7 @@ obj-$(CONFIG_ZCRYPT) += zcrypt_pcixcc.o zcrypt_cex2a.o 
zcrypt_cex4.o
 # pkey kernel module
 pkey-objs := pkey_api.o
 obj-$(CONFIG_PKEY) += pkey.o
+
+# adjunct processor matrix
+vfio_ap-objs := vfio_ap_drv.o
+obj-$(CONFIG_VFIO_AP) += vfio_ap.o
diff --git a/drivers/s390/crypto/vfio_ap_drv.c 
b/drivers/s390/crypto/vfio_ap_drv.c
new file mode 100644
index 000..459e595
--- /dev/null
+++ b/drivers/s390/crypto/vfio_ap_drv.c
@@ -0,0 +1,135 @@
+/*
+ * VFIO based AP device driver
+ *
+ * Copyright IBM Corp. 2017
+ *
+ * Author(s): Tony Krowiak 
+ */
+
+#include 
+#include 
+#include 
+
+#include "vfio_ap_private.h"
+
+#define VFIO_AP_ROOT_NAME "vfio_ap"
+#define VFIO_AP_DEV_TYPE_NAME "ap_matrix"
+#define VFIO_AP_DEV_NAME "matrix"
+
+MODULE_AUTHOR("IBM Corporation");
+MODULE_DESCRIPTION("VFIO AP device driver, Copyright IBM Corp. 2017");
+MODULE_LICENSE("GPL v2");
+
+static struct device *vfio_ap_root_device;
+
+static struct ap_driver vfio_ap_drv;
+
+static struct ap_matrix *ap_matrix;
+
+static struct device_type vfio_ap_dev_type = {
+   .name = VFIO_AP_DEV_TYPE_NAME,
+};
+
+/* Only type 10 adapters (CEX4 and later) are supported
+ * by the AP matrix device driver
+ */
+static struct ap_device_id ap_queue_ids[] = {
+   { .dev_type = AP_DEVICE_TYPE_CEX4,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX5,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { .dev_type = AP_DEVICE_TYPE_CEX6,
+ .match_flags = AP_DEVICE_ID_MATCH_QUEUE_TYPE },
+   { /* end of sibling */ },
+};
+
+MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids);
+
+static int vfio_ap_queue_dev_probe(struct ap_device *apdev)
+{
+   return 0;
+}
+
+static void vfio_ap_matrix_dev_release(struct device *dev)
+{
+   struct ap_matrix *ap_matrix = dev_get_drvdata(dev);
+
+