Re: [PATCH v9 09/22] s390: vfio-ap: register matrix device with VFIO mdev framework

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 08/14/2018 07:19 AM, Cornelia Huck wrote:
On Mon, 13 Aug 2018 17:48:06 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote:

From: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>

Registers the matrix device created by the VFIO AP device
driver with the VFIO mediated device framework.
Registering the matrix device will create the sysfs
structures needed to create mediated matrix devices
each of which will be used to configure the AP matrix
for a guest and connect it to the VFIO AP device driver.

Registering the matrix device with the VFIO mediated device
framework will create the following sysfs structures:

/sys/devices/vfio_ap/matrix/
...... [mdev_supported_types]
......... [vfio_ap-passthrough]
............ create

To create a mediated device for the AP matrix device, write a UUID
to the create file:

	uuidgen > create

A symbolic link to the mediated device's directory will be created in the
devices subdirectory named after the generated $uuid:

/sys/devices/vfio_ap/matrix/
...... [mdev_supported_types]
......... [vfio_ap-passthrough]
............ [devices]
............... [$uuid]

A symbolic link to the mediated device will also be created
in the vfio_ap matrix's directory:

/sys/devices/vfio_ap/matrix/[$uuid]

Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
Reviewed-by: Halil Pasic <pasic@xxxxxxxxxxxxx>
Tested-by: Michael Mueller <mimu@xxxxxxxxxxxxx>
Tested-by: Farhan Ali <alifm@xxxxxxxxxxxxx>
Signed-off-by: Christian Borntraeger <borntraeger@xxxxxxxxxx>
---
  MAINTAINERS                           |    1 +
  drivers/s390/crypto/Makefile          |    2 +-
  drivers/s390/crypto/vfio_ap_drv.c     |   23 ++++++
  drivers/s390/crypto/vfio_ap_ops.c     |  124 +++++++++++++++++++++++++++++++++
  drivers/s390/crypto/vfio_ap_private.h |   45 ++++++++++++
  include/uapi/linux/vfio.h             |    1 +
  6 files changed, 195 insertions(+), 1 deletions(-)
  create mode 100644 drivers/s390/crypto/vfio_ap_ops.c


diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
new file mode 100644
index 0000000..8018c2d
--- /dev/null
+++ b/drivers/s390/crypto/vfio_ap_ops.c
@@ -0,0 +1,124 @@
+// SPDX-License-Identifier: GPL-2.0+
+/*
+ * Adjunct processor matrix VFIO device driver callbacks.
+ *
+ * Copyright IBM Corp. 2018
+ *
+ * Author(s): Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
+ *	      Halil Pasic <pasic@xxxxxxxxxxxxx>
+ *	      Pierre Morel <pmorel@xxxxxxxxxxxxx>
+ */
+#include <linux/string.h>
+#include <linux/vfio.h>
+#include <linux/device.h>
+#include <linux/list.h>
+#include <linux/ctype.h>
+
+#include "vfio_ap_private.h"
+
+#define VFIO_AP_MDEV_TYPE_HWVIRT "passthrough"
+#define VFIO_AP_MDEV_NAME_HWVIRT "VFIO AP Passthrough Device"
+
+static void vfio_ap_matrix_init(struct ap_config_info *info,
+				struct ap_matrix *matrix)
+{
+	matrix->apm_max = info->apxa ? info->Na : 63;
+	matrix->aqm_max = info->apxa ? info->Nd : 15;
+	matrix->adm_max = info->apxa ? info->Nd : 15;
+}
+
+static int vfio_ap_mdev_create(struct kobject *kobj, struct mdev_device *mdev)
+{
+	struct ap_matrix_mdev *matrix_mdev;
+
+	matrix_mdev = kzalloc(sizeof(*matrix_mdev), GFP_KERNEL);
+	if (!matrix_mdev)
+		return -ENOMEM;
+
+	matrix_mdev->name = dev_name(mdev_dev(mdev));
+	vfio_ap_matrix_init(&matrix_dev.info, &matrix_mdev->matrix);
+	mdev_set_drvdata(mdev, matrix_mdev);
+
+	if (atomic_dec_if_positive(&matrix_dev.available_instances) < 0) {
+		kfree(matrix_mdev);
+		return -EPERM;
+	}
Maybe move this check to the top of the function?

Please ignore my previous response to your comment. I can't move the call to
atomic_dec_if_positive() to the top of the function because it decrements the available_instances and if the kzalloc() of matrix_mdev fails, then the value
would have to then be incremented to remain valid. What I can do is this:

1. Check the value of available_instances using atomic_read() at the top of
   the function and if it is zero, return an error.

2. Replace the call to atomic_dec_if_positive() with a call to atomic_dec()
   to decrement the available_instances.

I agree that it makes sense to return before attempting to allocate the
matrix_mdev if available_instances is zero.


+
+	mutex_lock(&matrix_dev.lock);
+	list_add(&matrix_mdev->list, &matrix_dev.mdev_list);
+	mutex_unlock(&matrix_dev.lock);
+
+	return 0;
+}





[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]

  Powered by Linux