On Fri, 13 Oct 2017 13:38:51 -0400 Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> wrote: > Registers the matrix device created by the AP matrix bus with > the VFIO mediated device framework. Registering the matrix > device will create the sysfs structures needed to create > mediated matrix devices that can be configured with a matrix > of adapters, usage domains and control domains for a guest > machine. > > Registering the matrix device with the VFIO mediated device > framework will create the following sysfs structures: > > /sys/devices/ap_matrix > ... [matrix] > ...... [mdev_supported_types] > ......... [ap_matrix-passthrough] > ............ available_instances > ............ create > ............ device_api > ............ [devices] > ............ name > > 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/ap_matrix > ... [matrix] > ...... [mdev_supported_types] > ......... [ap_matrix-passthrough] > ............ [devices] > ............... [$uuid] > > Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> > --- > MAINTAINERS | 1 + > drivers/s390/crypto/Makefile | 2 +- > drivers/s390/crypto/ap_matrix_bus.c | 1 + > drivers/s390/crypto/ap_matrix_bus.h | 4 + > drivers/s390/crypto/vfio_ap_matrix_drv.c | 5 + > drivers/s390/crypto/vfio_ap_matrix_ops.c | 172 ++++++++++++++++++++++++++ > drivers/s390/crypto/vfio_ap_matrix_private.h | 3 + > include/uapi/linux/vfio.h | 1 + > 8 files changed, 188 insertions(+), 1 deletions(-) > create mode 100644 drivers/s390/crypto/vfio_ap_matrix_ops.c > > diff --git a/drivers/s390/crypto/ap_matrix_bus.c b/drivers/s390/crypto/ap_matrix_bus.c > index 66bfa54..418c23b 100644 > --- a/drivers/s390/crypto/ap_matrix_bus.c > +++ b/drivers/s390/crypto/ap_matrix_bus.c > @@ -61,6 +61,7 @@ static int ap_matrix_dev_create(void) > matrix->device.bus = &ap_matrix_bus_type; > matrix->device.parent = ap_matrix_root_device; > matrix->device.release = ap_matrix_dev_release; > + INIT_LIST_HEAD(&matrix->queues); Don't you also need to init the spin lock? > > ret = device_register(&matrix->device); > if (ret) { > diff --git a/drivers/s390/crypto/ap_matrix_bus.h b/drivers/s390/crypto/ap_matrix_bus.h > index c2aff23..3eccc36 100644 > --- a/drivers/s390/crypto/ap_matrix_bus.h > +++ b/drivers/s390/crypto/ap_matrix_bus.h > @@ -12,8 +12,12 @@ > > #include <linux/device.h> > > +#include "ap_bus.h" Why do you need this include now? (IOW, does that need to go into another patch?) > + > struct ap_matrix { > struct device device; > + spinlock_t qlock; > + struct list_head queues; > }; > > struct ap_matrix *ap_matrix_get_device(void); > diff --git a/drivers/s390/crypto/vfio_ap_matrix_ops.c b/drivers/s390/crypto/vfio_ap_matrix_ops.c > new file mode 100644 > index 0000000..7d01f18 > --- /dev/null > +++ b/drivers/s390/crypto/vfio_ap_matrix_ops.c > @@ -0,0 +1,172 @@ > +/* > + * Adjunct processor matrix VFIO device driver callbacks. > + * > + * Copyright IBM Corp. 2017 > + * Author(s): Tony Krowiak <akrowiak@xxxxxxxxxxxxxxxxxx> > + * > + */ > +#include <asm/ap-config.h> > +#include <linux/string.h> > +#include <linux/vfio.h> > +#include <linux/mdev.h> > +#include <linux/device.h> > +#include <linux/list.h> > +#include <linux/ctype.h> > + > +#include "vfio_ap_matrix_private.h" > + > +#define AP_MATRIX_MDEV_TYPE_HWVIRT "passthrough" > +#define AP_MATRIX_MDEV_NAME_HWVIRT "AP Matrix Passthrough Device" > + > +#define AP_MATRIX_MAX_AVAILABLE_INSTANCES 255 > + > +struct ap_matrix_mdev { > + struct mdev_device *mdev; > + struct list_head node; > +}; > + > +struct ap_matrix *matrix; > +struct mutex mdev_devices_lock; > +struct list_head mdev_devices; > +int available_instances; Having this as a global variable feels wrong. I see that vfio-ccw keeps things like this in driver data attached to its parent. Can you do something like that as well? > + > +static struct ap_matrix_mdev *ap_matrix_mdev_find_by_uuid(uuid_le uuid) > +{ > + struct ap_matrix_mdev *matrix_mdev; > + > + list_for_each_entry(matrix_mdev, &mdev_devices, node) { > + if (uuid_le_cmp(mdev_uuid(matrix_mdev->mdev), uuid) == 0) > + return matrix_mdev; > + } > + > + return NULL; > +} (...) > +int ap_matrix_mdev_register(struct ap_matrix *ap_matrix) > +{ > + int ret; > + struct device *dev = &ap_matrix->device; > + > + ret = mdev_register_device(dev, &vfio_ap_matrix_ops); > + if (ret) > + return ret; > + > + matrix = ap_matrix; I'd just grab the (one) matrix device here... > + mutex_init(&mdev_devices_lock); > + INIT_LIST_HEAD(&mdev_devices); > + available_instances = AP_MATRIX_MAX_AVAILABLE_INSTANCES; > + > + return 0; > +} > + > +void ap_matrix_mdev_unregister(struct ap_matrix *ap_matrix) > +{ > + struct device *dev; > + > + if (ap_matrix == matrix) { ...and lose this extra if. > + dev = &matrix->device; > + available_instances--; > + mdev_unregister_device(dev); > + } > +} -- To unsubscribe from this list: send the line "unsubscribe linux-s390" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html