| Message ID | 1553528210-30562-2-git-send-email-frank.heimes@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | Fixes for 1818854 | expand |
On 25.03.19 16:36, frank.heimes@canonical.com wrote: > BugLink: http://bugs.launchpad.net/bugs/1818854 > > s390: vfio_ap: link the vfio_ap devices to the vfio_ap bus subsystem > > Libudev relies on having a subsystem link for non-root devices. To > avoid libudev (and potentially other userspace tools) choking on the > matrix device let us introduce a matrix bus and with it the matrix > bus subsytem. Also make the matrix device reside within the matrix > bus. > > Doing this we remove the forced link from the matrix device to the > vfio_ap driver and the device_type we do not need anymore. > > Since the associated matrix driver is not the vfio_ap driver any more, > we have to change the search for the devices on the vfio_ap driver in > the function vfio_ap_verify_queue_reserved. > Fixes: 1fde573413b5 ("s390: vfio-ap: base implementation of VFIO AP device driver") > Cc: stable@vger.kernel.org > > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Tony Krowiak <akrowiak@linux.ibm.com> > Acked-by: Halil Pasic <pasic@linux.ibm.com> > Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com> > (cherry picked from commit 36360658eb5a6cf04bb9f2704d1e4ce54037ec99) > Signed-off-by: Frank Heimes <frank.heimes@canonical.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/s390/crypto/vfio_ap_drv.c | 44 ++++++++++++++++++++++++++++------- > drivers/s390/crypto/vfio_ap_ops.c | 4 ++-- > drivers/s390/crypto/vfio_ap_private.h | 1 + > 3 files changed, 38 insertions(+), 11 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c > index 8b51821..05d16ed 100644 > --- a/drivers/s390/crypto/vfio_ap_drv.c > +++ b/drivers/s390/crypto/vfio_ap_drv.c > @@ -14,7 +14,6 @@ > #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"); > @@ -23,10 +22,6 @@ MODULE_LICENSE("GPL v2"); > > static struct ap_driver vfio_ap_drv; > > -static struct device_type vfio_ap_dev_type = { > - .name = VFIO_AP_DEV_TYPE_NAME, > -}; > - > struct ap_matrix_dev *matrix_dev; > > /* Only type 10 adapters (CEX4 and later) are supported > @@ -61,6 +56,22 @@ static void vfio_ap_matrix_dev_release(struct device *dev) > kfree(matrix_dev); > } > > +static int matrix_bus_match(struct device *dev, struct device_driver *drv) > +{ > + return 1; > +} > + > +static struct bus_type matrix_bus = { > + .name = "matrix", > + .match = &matrix_bus_match, > +}; > + > +static struct device_driver matrix_driver = { > + .name = "vfio_ap", > + .bus = &matrix_bus, > + .suppress_bind_attrs = true, > +}; > + > static int vfio_ap_matrix_dev_create(void) > { > int ret; > @@ -70,6 +81,10 @@ static int vfio_ap_matrix_dev_create(void) > if (IS_ERR(root_device)) > return PTR_ERR(root_device); > > + ret = bus_register(&matrix_bus); > + if (ret) > + goto bus_register_err; > + > matrix_dev = kzalloc(sizeof(*matrix_dev), GFP_KERNEL); > if (!matrix_dev) { > ret = -ENOMEM; > @@ -86,30 +101,41 @@ static int vfio_ap_matrix_dev_create(void) > mutex_init(&matrix_dev->lock); > INIT_LIST_HEAD(&matrix_dev->mdev_list); > > - matrix_dev->device.type = &vfio_ap_dev_type; > dev_set_name(&matrix_dev->device, "%s", VFIO_AP_DEV_NAME); > matrix_dev->device.parent = root_device; > + matrix_dev->device.bus = &matrix_bus; > matrix_dev->device.release = vfio_ap_matrix_dev_release; > - matrix_dev->device.driver = &vfio_ap_drv.driver; > + matrix_dev->vfio_ap_drv = &vfio_ap_drv; > > ret = device_register(&matrix_dev->device); > if (ret) > goto matrix_reg_err; > > + ret = driver_register(&matrix_driver); > + if (ret) > + goto matrix_drv_err; > + > return 0; > > +matrix_drv_err: > + device_unregister(&matrix_dev->device); > matrix_reg_err: > put_device(&matrix_dev->device); > matrix_alloc_err: > + bus_unregister(&matrix_bus); > +bus_register_err: > root_device_unregister(root_device); > - > return ret; > } > > static void vfio_ap_matrix_dev_destroy(void) > { > + struct device *root_device = matrix_dev->device.parent; > + > + driver_unregister(&matrix_driver); > device_unregister(&matrix_dev->device); > - root_device_unregister(matrix_dev->device.parent); > + bus_unregister(&matrix_bus); > + root_device_unregister(root_device); > } > > int __init vfio_ap_init(void) > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index ea99165..42d850f 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -198,8 +198,8 @@ static int vfio_ap_verify_queue_reserved(unsigned long *apid, > qres.apqi = apqi; > qres.reserved = false; > > - ret = driver_for_each_device(matrix_dev->device.driver, NULL, &qres, > - vfio_ap_has_queue); > + ret = driver_for_each_device(&matrix_dev->vfio_ap_drv->driver, NULL, > + &qres, vfio_ap_has_queue); > if (ret) > return ret; > > diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h > index 5675492..76b7f98 100644 > --- a/drivers/s390/crypto/vfio_ap_private.h > +++ b/drivers/s390/crypto/vfio_ap_private.h > @@ -40,6 +40,7 @@ struct ap_matrix_dev { > struct ap_config_info info; > struct list_head mdev_list; > struct mutex lock; > + struct ap_driver *vfio_ap_drv; > }; > > extern struct ap_matrix_dev *matrix_dev; >
Hi Frank, There are some issues with this patch submission, that unfortunately weren't spotted before applying it. The first one is that if the person submitting the patch is not the original author of the commit, a line in the following format needs to be added as the first line of the email body: From: [author name] <[author email]> In the same format as 'git send-email' does when you provide it a patch generated by 'git format-patch'. Otherwise the email submitter will be added as the author of the commit on the repo. On 3/25/19 4:36 PM, frank.heimes@canonical.com wrote: > BugLink: http://bugs.launchpad.net/bugs/1818854 > > s390: vfio_ap: link the vfio_ap devices to the vfio_ap bus subsystem The second issue is repeating the subject of the commit on the email body. This is not needed, 'git am' will automatically get this from the email subject. Otherwise the subject line will appear on the commit message as well. Of course you can craft an email for a patch submission manually, but my recommendation is using 'git format-patch' (you can use --subject-prefix to add the tags you want), edit the patch to add the provenance information and the BugLink and sending it with 'git send-email --suppress-cc=all'. Those commands will take care of everything needed so we can easily use 'git am' to apply your patch. I will amend the commits in our repo to fix it. Thanks, Kleber > > Libudev relies on having a subsystem link for non-root devices. To > avoid libudev (and potentially other userspace tools) choking on the > matrix device let us introduce a matrix bus and with it the matrix > bus subsytem. Also make the matrix device reside within the matrix > bus. > > Doing this we remove the forced link from the matrix device to the > vfio_ap driver and the device_type we do not need anymore. > > Since the associated matrix driver is not the vfio_ap driver any more, > we have to change the search for the devices on the vfio_ap driver in > the function vfio_ap_verify_queue_reserved. > Fixes: 1fde573413b5 ("s390: vfio-ap: base implementation of VFIO AP device driver") > Cc: stable@vger.kernel.org > > Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> > Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> > Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> > Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> > Reviewed-by: Cornelia Huck <cohuck@redhat.com> > Reviewed-by: Tony Krowiak <akrowiak@linux.ibm.com> > Acked-by: Halil Pasic <pasic@linux.ibm.com> > Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com> > (cherry picked from commit 36360658eb5a6cf04bb9f2704d1e4ce54037ec99) > Signed-off-by: Frank Heimes <frank.heimes@canonical.com> > --- > drivers/s390/crypto/vfio_ap_drv.c | 44 ++++++++++++++++++++++++++++------- > drivers/s390/crypto/vfio_ap_ops.c | 4 ++-- > drivers/s390/crypto/vfio_ap_private.h | 1 + > 3 files changed, 38 insertions(+), 11 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c > index 8b51821..05d16ed 100644 > --- a/drivers/s390/crypto/vfio_ap_drv.c > +++ b/drivers/s390/crypto/vfio_ap_drv.c > @@ -14,7 +14,6 @@ > #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"); > @@ -23,10 +22,6 @@ MODULE_LICENSE("GPL v2"); > > static struct ap_driver vfio_ap_drv; > > -static struct device_type vfio_ap_dev_type = { > - .name = VFIO_AP_DEV_TYPE_NAME, > -}; > - > struct ap_matrix_dev *matrix_dev; > > /* Only type 10 adapters (CEX4 and later) are supported > @@ -61,6 +56,22 @@ static void vfio_ap_matrix_dev_release(struct device *dev) > kfree(matrix_dev); > } > > +static int matrix_bus_match(struct device *dev, struct device_driver *drv) > +{ > + return 1; > +} > + > +static struct bus_type matrix_bus = { > + .name = "matrix", > + .match = &matrix_bus_match, > +}; > + > +static struct device_driver matrix_driver = { > + .name = "vfio_ap", > + .bus = &matrix_bus, > + .suppress_bind_attrs = true, > +}; > + > static int vfio_ap_matrix_dev_create(void) > { > int ret; > @@ -70,6 +81,10 @@ static int vfio_ap_matrix_dev_create(void) > if (IS_ERR(root_device)) > return PTR_ERR(root_device); > > + ret = bus_register(&matrix_bus); > + if (ret) > + goto bus_register_err; > + > matrix_dev = kzalloc(sizeof(*matrix_dev), GFP_KERNEL); > if (!matrix_dev) { > ret = -ENOMEM; > @@ -86,30 +101,41 @@ static int vfio_ap_matrix_dev_create(void) > mutex_init(&matrix_dev->lock); > INIT_LIST_HEAD(&matrix_dev->mdev_list); > > - matrix_dev->device.type = &vfio_ap_dev_type; > dev_set_name(&matrix_dev->device, "%s", VFIO_AP_DEV_NAME); > matrix_dev->device.parent = root_device; > + matrix_dev->device.bus = &matrix_bus; > matrix_dev->device.release = vfio_ap_matrix_dev_release; > - matrix_dev->device.driver = &vfio_ap_drv.driver; > + matrix_dev->vfio_ap_drv = &vfio_ap_drv; > > ret = device_register(&matrix_dev->device); > if (ret) > goto matrix_reg_err; > > + ret = driver_register(&matrix_driver); > + if (ret) > + goto matrix_drv_err; > + > return 0; > > +matrix_drv_err: > + device_unregister(&matrix_dev->device); > matrix_reg_err: > put_device(&matrix_dev->device); > matrix_alloc_err: > + bus_unregister(&matrix_bus); > +bus_register_err: > root_device_unregister(root_device); > - > return ret; > } > > static void vfio_ap_matrix_dev_destroy(void) > { > + struct device *root_device = matrix_dev->device.parent; > + > + driver_unregister(&matrix_driver); > device_unregister(&matrix_dev->device); > - root_device_unregister(matrix_dev->device.parent); > + bus_unregister(&matrix_bus); > + root_device_unregister(root_device); > } > > int __init vfio_ap_init(void) > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index ea99165..42d850f 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -198,8 +198,8 @@ static int vfio_ap_verify_queue_reserved(unsigned long *apid, > qres.apqi = apqi; > qres.reserved = false; > > - ret = driver_for_each_device(matrix_dev->device.driver, NULL, &qres, > - vfio_ap_has_queue); > + ret = driver_for_each_device(&matrix_dev->vfio_ap_drv->driver, NULL, > + &qres, vfio_ap_has_queue); > if (ret) > return ret; > > diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h > index 5675492..76b7f98 100644 > --- a/drivers/s390/crypto/vfio_ap_private.h > +++ b/drivers/s390/crypto/vfio_ap_private.h > @@ -40,6 +40,7 @@ struct ap_matrix_dev { > struct ap_config_info info; > struct list_head mdev_list; > struct mutex lock; > + struct ap_driver *vfio_ap_drv; > }; > > extern struct ap_matrix_dev *matrix_dev; >
diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c index 8b51821..05d16ed 100644 --- a/drivers/s390/crypto/vfio_ap_drv.c +++ b/drivers/s390/crypto/vfio_ap_drv.c @@ -14,7 +14,6 @@ #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"); @@ -23,10 +22,6 @@ MODULE_LICENSE("GPL v2"); static struct ap_driver vfio_ap_drv; -static struct device_type vfio_ap_dev_type = { - .name = VFIO_AP_DEV_TYPE_NAME, -}; - struct ap_matrix_dev *matrix_dev; /* Only type 10 adapters (CEX4 and later) are supported @@ -61,6 +56,22 @@ static void vfio_ap_matrix_dev_release(struct device *dev) kfree(matrix_dev); } +static int matrix_bus_match(struct device *dev, struct device_driver *drv) +{ + return 1; +} + +static struct bus_type matrix_bus = { + .name = "matrix", + .match = &matrix_bus_match, +}; + +static struct device_driver matrix_driver = { + .name = "vfio_ap", + .bus = &matrix_bus, + .suppress_bind_attrs = true, +}; + static int vfio_ap_matrix_dev_create(void) { int ret; @@ -70,6 +81,10 @@ static int vfio_ap_matrix_dev_create(void) if (IS_ERR(root_device)) return PTR_ERR(root_device); + ret = bus_register(&matrix_bus); + if (ret) + goto bus_register_err; + matrix_dev = kzalloc(sizeof(*matrix_dev), GFP_KERNEL); if (!matrix_dev) { ret = -ENOMEM; @@ -86,30 +101,41 @@ static int vfio_ap_matrix_dev_create(void) mutex_init(&matrix_dev->lock); INIT_LIST_HEAD(&matrix_dev->mdev_list); - matrix_dev->device.type = &vfio_ap_dev_type; dev_set_name(&matrix_dev->device, "%s", VFIO_AP_DEV_NAME); matrix_dev->device.parent = root_device; + matrix_dev->device.bus = &matrix_bus; matrix_dev->device.release = vfio_ap_matrix_dev_release; - matrix_dev->device.driver = &vfio_ap_drv.driver; + matrix_dev->vfio_ap_drv = &vfio_ap_drv; ret = device_register(&matrix_dev->device); if (ret) goto matrix_reg_err; + ret = driver_register(&matrix_driver); + if (ret) + goto matrix_drv_err; + return 0; +matrix_drv_err: + device_unregister(&matrix_dev->device); matrix_reg_err: put_device(&matrix_dev->device); matrix_alloc_err: + bus_unregister(&matrix_bus); +bus_register_err: root_device_unregister(root_device); - return ret; } static void vfio_ap_matrix_dev_destroy(void) { + struct device *root_device = matrix_dev->device.parent; + + driver_unregister(&matrix_driver); device_unregister(&matrix_dev->device); - root_device_unregister(matrix_dev->device.parent); + bus_unregister(&matrix_bus); + root_device_unregister(root_device); } int __init vfio_ap_init(void) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index ea99165..42d850f 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -198,8 +198,8 @@ static int vfio_ap_verify_queue_reserved(unsigned long *apid, qres.apqi = apqi; qres.reserved = false; - ret = driver_for_each_device(matrix_dev->device.driver, NULL, &qres, - vfio_ap_has_queue); + ret = driver_for_each_device(&matrix_dev->vfio_ap_drv->driver, NULL, + &qres, vfio_ap_has_queue); if (ret) return ret; diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h index 5675492..76b7f98 100644 --- a/drivers/s390/crypto/vfio_ap_private.h +++ b/drivers/s390/crypto/vfio_ap_private.h @@ -40,6 +40,7 @@ struct ap_matrix_dev { struct ap_config_info info; struct list_head mdev_list; struct mutex lock; + struct ap_driver *vfio_ap_drv; }; extern struct ap_matrix_dev *matrix_dev;
BugLink: http://bugs.launchpad.net/bugs/1818854 s390: vfio_ap: link the vfio_ap devices to the vfio_ap bus subsystem Libudev relies on having a subsystem link for non-root devices. To avoid libudev (and potentially other userspace tools) choking on the matrix device let us introduce a matrix bus and with it the matrix bus subsytem. Also make the matrix device reside within the matrix bus. Doing this we remove the forced link from the matrix device to the vfio_ap driver and the device_type we do not need anymore. Since the associated matrix driver is not the vfio_ap driver any more, we have to change the search for the devices on the vfio_ap driver in the function vfio_ap_verify_queue_reserved. Fixes: 1fde573413b5 ("s390: vfio-ap: base implementation of VFIO AP device driver") Cc: stable@vger.kernel.org Reported-by: Marc Hartmayer <mhartmay@linux.ibm.com> Reported-by: Christian Borntraeger <borntraeger@de.ibm.com> Signed-off-by: Pierre Morel <pmorel@linux.ibm.com> Tested-by: Christian Borntraeger <borntraeger@de.ibm.com> Reviewed-by: Cornelia Huck <cohuck@redhat.com> Reviewed-by: Tony Krowiak <akrowiak@linux.ibm.com> Acked-by: Halil Pasic <pasic@linux.ibm.com> Signed-off-by: Martin Schwidefsky <schwidefsky@de.ibm.com> (cherry picked from commit 36360658eb5a6cf04bb9f2704d1e4ce54037ec99) Signed-off-by: Frank Heimes <frank.heimes@canonical.com> --- drivers/s390/crypto/vfio_ap_drv.c | 44 ++++++++++++++++++++++++++++------- drivers/s390/crypto/vfio_ap_ops.c | 4 ++-- drivers/s390/crypto/vfio_ap_private.h | 1 + 3 files changed, 38 insertions(+), 11 deletions(-)