| Message ID | 20200821195616.13554-3-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/vfio-ap: dynamic configuration support | expand |
On Fri, 21 Aug 2020 15:56:02 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > This patch refactor's the vfio_ap device driver to use the AP bus's s/refactor's/refactors/ > ap_get_qdev() function to retrieve the vfio_ap_queue struct containing > information about a queue that is bound to the vfio_ap device driver. > The bus's ap_get_qdev() function retrieves the queue device from a > hashtable keyed by APQN. This is much more efficient than looping over > the list of devices attached to the AP bus by several orders of > magnitude. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > Reported-by: kernel test robot <lkp@intel.com> > --- > drivers/s390/crypto/vfio_ap_drv.c | 27 ++------- > drivers/s390/crypto/vfio_ap_ops.c | 86 +++++++++++++++------------ > drivers/s390/crypto/vfio_ap_private.h | 8 ++- > 3 files changed, 59 insertions(+), 62 deletions(-) > (...) > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index e0bde8518745..ad3925f04f61 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -26,43 +26,26 @@ > > static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); > > -static int match_apqn(struct device *dev, const void *data) > -{ > - struct vfio_ap_queue *q = dev_get_drvdata(dev); > - > - return (q->apqn == *(int *)(data)) ? 1 : 0; > -} > - > /** > - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list > - * @matrix_mdev: the associated mediated matrix > + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. > * @apqn: The queue APQN > * > - * Retrieve a queue with a specific APQN from the list of the > - * devices of the vfio_ap_drv. > - * Verify that the APID and the APQI are set in the matrix. > + * Retrieve a queue with a specific APQN from the AP queue devices attached to > + * the AP bus. > * > - * Returns the pointer to the associated vfio_ap_queue > + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. > */ > -static struct vfio_ap_queue *vfio_ap_get_queue( > - struct ap_matrix_mdev *matrix_mdev, > - int apqn) > +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) > { > + struct ap_queue *queue; > struct vfio_ap_queue *q; > - struct device *dev; > > - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) > - return NULL; > - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) I think you should add some explanation to the patch description why testing the matrix bitmasks is not needed anymore. > + queue = ap_get_qdev(apqn); > + if (!queue) > return NULL; > > - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, > - &apqn, match_apqn); > - if (!dev) > - return NULL; > - q = dev_get_drvdata(dev); > - q->matrix_mdev = matrix_mdev; > - put_device(dev); > + q = dev_get_drvdata(&queue->ap_dev.device); > + put_device(&queue->ap_dev.device); > > return q; > } (...)
On 8/25/20 6:13 AM, Cornelia Huck wrote: > On Fri, 21 Aug 2020 15:56:02 -0400 > Tony Krowiak<akrowiak@linux.ibm.com> wrote: > >> This patch refactor's the vfio_ap device driver to use the AP bus's > s/refactor's/refactors/ Of course, what was I thinking?:) >> ap_get_qdev() function to retrieve the vfio_ap_queue struct containing >> information about a queue that is bound to the vfio_ap device driver. >> The bus's ap_get_qdev() function retrieves the queue device from a >> hashtable keyed by APQN. This is much more efficient than looping over >> the list of devices attached to the AP bus by several orders of >> magnitude. >> >> Signed-off-by: Tony Krowiak<akrowiak@linux.ibm.com> >> Reported-by: kernel test robot<lkp@intel.com> >> --- >> drivers/s390/crypto/vfio_ap_drv.c | 27 ++------- >> drivers/s390/crypto/vfio_ap_ops.c | 86 +++++++++++++++------------ >> drivers/s390/crypto/vfio_ap_private.h | 8 ++- >> 3 files changed, 59 insertions(+), 62 deletions(-) >> > (...) > >> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c >> index e0bde8518745..ad3925f04f61 100644 >> --- a/drivers/s390/crypto/vfio_ap_ops.c >> +++ b/drivers/s390/crypto/vfio_ap_ops.c >> @@ -26,43 +26,26 @@ >> >> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); >> >> -static int match_apqn(struct device *dev, const void *data) >> -{ >> - struct vfio_ap_queue *q = dev_get_drvdata(dev); >> - >> - return (q->apqn == *(int *)(data)) ? 1 : 0; >> -} >> - >> /** >> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list >> - * @matrix_mdev: the associated mediated matrix >> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. >> * @apqn: The queue APQN >> * >> - * Retrieve a queue with a specific APQN from the list of the >> - * devices of the vfio_ap_drv. >> - * Verify that the APID and the APQI are set in the matrix. >> + * Retrieve a queue with a specific APQN from the AP queue devices attached to >> + * the AP bus. >> * >> - * Returns the pointer to the associated vfio_ap_queue >> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. >> */ >> -static struct vfio_ap_queue *vfio_ap_get_queue( >> - struct ap_matrix_mdev *matrix_mdev, >> - int apqn) >> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) >> { >> + struct ap_queue *queue; >> struct vfio_ap_queue *q; >> - struct device *dev; >> >> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) >> - return NULL; >> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) > I think you should add some explanation to the patch description why > testing the matrix bitmasks is not needed anymore. As a result of this comment, I took a closer look at the code to determine the reason for eliminating the matrix_mdev parameter. The reason is because the code below (i.e., find the device and get the driver data) was also repeated in the vfio_ap_irq_disable_apqn() function, so I replaced it with a call to the function above; however, the vfio_ap_irq_disable_apqn() function does not have a reference to the matrix_mdev, so I eliminated the matrix_mdev parameter. Note that the vfio_ap_irq_disable_apqn() is called for each APQN assigned to a matrix mdev, so there is no need to test the bitmasks there. The other place from which the function above is called is the handle_pqap() function which does have a reference to the matrix_mdev. In order to ensure the integrity of the instruction being intercepted - i.e., PQAP(AQIC) enable/disable IRQ for aN AP queue - the testing of the matrix bitmasks probably ought to be performed, so it will be done there instead of in the vfio_ap_get_queue() function above. > + queue = ap_get_qdev(apqn); > + if (!queue) > return NULL; > > - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, > - &apqn, match_apqn); > - if (!dev) > - return NULL; > - q = dev_get_drvdata(dev); > - q->matrix_mdev = matrix_mdev; > - put_device(dev); > + q = dev_get_drvdata(&queue->ap_dev.device); > + put_device(&queue->ap_dev.device); > > return q; > } > (...) >
On Thu, 27 Aug 2020 10:24:07 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > On 8/25/20 6:13 AM, Cornelia Huck wrote: > > On Fri, 21 Aug 2020 15:56:02 -0400 > > Tony Krowiak<akrowiak@linux.ibm.com> wrote: > >> /** > >> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list > >> - * @matrix_mdev: the associated mediated matrix > >> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. > >> * @apqn: The queue APQN > >> * > >> - * Retrieve a queue with a specific APQN from the list of the > >> - * devices of the vfio_ap_drv. > >> - * Verify that the APID and the APQI are set in the matrix. > >> + * Retrieve a queue with a specific APQN from the AP queue devices attached to > >> + * the AP bus. > >> * > >> - * Returns the pointer to the associated vfio_ap_queue > >> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. > >> */ > >> -static struct vfio_ap_queue *vfio_ap_get_queue( > >> - struct ap_matrix_mdev *matrix_mdev, > >> - int apqn) > >> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) > >> { > >> + struct ap_queue *queue; > >> struct vfio_ap_queue *q; > >> - struct device *dev; > >> > >> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) > >> - return NULL; > >> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) > > I think you should add some explanation to the patch description why > > testing the matrix bitmasks is not needed anymore. > > As a result of this comment, I took a closer look at the code to > determine the reason for eliminating the matrix_mdev > parameter. The reason is because the code below (i.e., find the device > and get the driver data) was also repeated in the vfio_ap_irq_disable_apqn() > function, so I replaced it with a call to the function above; however, the > vfio_ap_irq_disable_apqn() function does not have a reference to the > matrix_mdev, so I eliminated the matrix_mdev parameter. Note that the > vfio_ap_irq_disable_apqn() is called for each APQN assigned to a matrix > mdev, so there is no need to test the bitmasks there. > > The other place from which the function above is called is > the handle_pqap() function which does have a reference to the > matrix_mdev. In order to ensure the integrity of the instruction > being intercepted - i.e., PQAP(AQIC) enable/disable IRQ for aN > AP queue - the testing of the matrix bitmasks probably ought to > be performed, so it will be done there instead of in the > vfio_ap_get_queue() function above. Should you add a comment that vfio_ap_get_queue() assumes that the caller makes sure that this is only called for APQNs that are assigned to a matrix? > > > > + queue = ap_get_qdev(apqn); > > + if (!queue) > > return NULL; > > > > - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, > > - &apqn, match_apqn); > > - if (!dev) > > - return NULL; > > - q = dev_get_drvdata(dev); > > - q->matrix_mdev = matrix_mdev; > > - put_device(dev); > > + q = dev_get_drvdata(&queue->ap_dev.device); > > + put_device(&queue->ap_dev.device); > > > > return q; > > } > > (...) > > >
On 8/28/20 4:13 AM, Cornelia Huck wrote: > On Thu, 27 Aug 2020 10:24:07 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> On 8/25/20 6:13 AM, Cornelia Huck wrote: >>> On Fri, 21 Aug 2020 15:56:02 -0400 >>> Tony Krowiak<akrowiak@linux.ibm.com> wrote: >>>> /** >>>> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list >>>> - * @matrix_mdev: the associated mediated matrix >>>> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. >>>> * @apqn: The queue APQN >>>> * >>>> - * Retrieve a queue with a specific APQN from the list of the >>>> - * devices of the vfio_ap_drv. >>>> - * Verify that the APID and the APQI are set in the matrix. >>>> + * Retrieve a queue with a specific APQN from the AP queue devices attached to >>>> + * the AP bus. >>>> * >>>> - * Returns the pointer to the associated vfio_ap_queue >>>> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. >>>> */ >>>> -static struct vfio_ap_queue *vfio_ap_get_queue( >>>> - struct ap_matrix_mdev *matrix_mdev, >>>> - int apqn) >>>> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) >>>> { >>>> + struct ap_queue *queue; >>>> struct vfio_ap_queue *q; >>>> - struct device *dev; >>>> >>>> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) >>>> - return NULL; >>>> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) >>> I think you should add some explanation to the patch description why >>> testing the matrix bitmasks is not needed anymore. >> As a result of this comment, I took a closer look at the code to >> determine the reason for eliminating the matrix_mdev >> parameter. The reason is because the code below (i.e., find the device >> and get the driver data) was also repeated in the vfio_ap_irq_disable_apqn() >> function, so I replaced it with a call to the function above; however, the >> vfio_ap_irq_disable_apqn() function does not have a reference to the >> matrix_mdev, so I eliminated the matrix_mdev parameter. Note that the >> vfio_ap_irq_disable_apqn() is called for each APQN assigned to a matrix >> mdev, so there is no need to test the bitmasks there. >> >> The other place from which the function above is called is >> the handle_pqap() function which does have a reference to the >> matrix_mdev. In order to ensure the integrity of the instruction >> being intercepted - i.e., PQAP(AQIC) enable/disable IRQ for aN >> AP queue - the testing of the matrix bitmasks probably ought to >> be performed, so it will be done there instead of in the >> vfio_ap_get_queue() function above. > Should you add a comment that vfio_ap_get_queue() assumes that the > caller makes sure that this is only called for APQNs that are assigned > to a matrix? I suppose it wouldn't hurt. > >> >>> + queue = ap_get_qdev(apqn); >>> + if (!queue) >>> return NULL; >>> >>> - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, >>> - &apqn, match_apqn); >>> - if (!dev) >>> - return NULL; >>> - q = dev_get_drvdata(dev); >>> - q->matrix_mdev = matrix_mdev; >>> - put_device(dev); >>> + q = dev_get_drvdata(&queue->ap_dev.device); >>> + put_device(&queue->ap_dev.device); >>> >>> return q; >>> } >>> (...) >>>
On 21.08.20 21:56, Tony Krowiak wrote: > This patch refactor's the vfio_ap device driver to use the AP bus's > ap_get_qdev() function to retrieve the vfio_ap_queue struct containing > information about a queue that is bound to the vfio_ap device driver. > The bus's ap_get_qdev() function retrieves the queue device from a > hashtable keyed by APQN. This is much more efficient than looping over > the list of devices attached to the AP bus by several orders of > magnitude. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > Reported-by: kernel test robot <lkp@intel.com> I think this can go. No need to mark that an earlier version of this patch had an issue. [...] > diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h > index f46dde56b464..a2aa05bec718 100644 > --- a/drivers/s390/crypto/vfio_ap_private.h > +++ b/drivers/s390/crypto/vfio_ap_private.h > @@ -18,6 +18,7 @@ > #include <linux/delay.h> > #include <linux/mutex.h> > #include <linux/kvm_host.h> > +#include <linux/hashtable.h> I dont think that this header file needs it. Any user of it will now include this. Can you move this include into the respective C file when the hash stuff is used? Other than that this looks good.
On 9/4/20 4:11 AM, Christian Borntraeger wrote: > > On 21.08.20 21:56, Tony Krowiak wrote: >> This patch refactor's the vfio_ap device driver to use the AP bus's >> ap_get_qdev() function to retrieve the vfio_ap_queue struct containing >> information about a queue that is bound to the vfio_ap device driver. >> The bus's ap_get_qdev() function retrieves the queue device from a >> hashtable keyed by APQN. This is much more efficient than looping over >> the list of devices attached to the AP bus by several orders of >> magnitude. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> >> Reported-by: kernel test robot <lkp@intel.com> > I think this can go. No need to mark that an earlier version of this patch had an issue. I was just following the instructions in the robot comments. I'll get rid of it. > > > [...] > >> diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h >> index f46dde56b464..a2aa05bec718 100644 >> --- a/drivers/s390/crypto/vfio_ap_private.h >> +++ b/drivers/s390/crypto/vfio_ap_private.h >> @@ -18,6 +18,7 @@ >> #include <linux/delay.h> >> #include <linux/mutex.h> >> #include <linux/kvm_host.h> >> +#include <linux/hashtable.h> > I dont think that this header file needs it. Any user of it will now include this. > Can you move this include into the respective C file when the hash stuff is > used? I can. > > > Other than that this looks good.
On Thu, 27 Aug 2020 10:24:07 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > > > On 8/25/20 6:13 AM, Cornelia Huck wrote: > > On Fri, 21 Aug 2020 15:56:02 -0400 > > Tony Krowiak<akrowiak@linux.ibm.com> wrote: > > > >> This patch refactor's the vfio_ap device driver to use the AP bus's > > s/refactor's/refactors/ > > Of course, what was I thinking?:) > > >> ap_get_qdev() function to retrieve the vfio_ap_queue struct containing > >> information about a queue that is bound to the vfio_ap device driver. > >> The bus's ap_get_qdev() function retrieves the queue device from a > >> hashtable keyed by APQN. This is much more efficient than looping over > >> the list of devices attached to the AP bus by several orders of > >> magnitude. > >> > >> Signed-off-by: Tony Krowiak<akrowiak@linux.ibm.com> > >> Reported-by: kernel test robot<lkp@intel.com> > >> --- > >> drivers/s390/crypto/vfio_ap_drv.c | 27 ++------- > >> drivers/s390/crypto/vfio_ap_ops.c | 86 +++++++++++++++------------ > >> drivers/s390/crypto/vfio_ap_private.h | 8 ++- > >> 3 files changed, 59 insertions(+), 62 deletions(-) > >> > > (...) > > > >> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > >> index e0bde8518745..ad3925f04f61 100644 > >> --- a/drivers/s390/crypto/vfio_ap_ops.c > >> +++ b/drivers/s390/crypto/vfio_ap_ops.c > >> @@ -26,43 +26,26 @@ > >> > >> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); > >> > >> -static int match_apqn(struct device *dev, const void *data) > >> -{ > >> - struct vfio_ap_queue *q = dev_get_drvdata(dev); > >> - > >> - return (q->apqn == *(int *)(data)) ? 1 : 0; > >> -} > >> - > >> /** > >> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list > >> - * @matrix_mdev: the associated mediated matrix > >> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. > >> * @apqn: The queue APQN > >> * > >> - * Retrieve a queue with a specific APQN from the list of the > >> - * devices of the vfio_ap_drv. > >> - * Verify that the APID and the APQI are set in the matrix. > >> + * Retrieve a queue with a specific APQN from the AP queue devices attached to > >> + * the AP bus. > >> * > >> - * Returns the pointer to the associated vfio_ap_queue > >> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. > >> */ > >> -static struct vfio_ap_queue *vfio_ap_get_queue( > >> - struct ap_matrix_mdev *matrix_mdev, > >> - int apqn) > >> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) > >> { > >> + struct ap_queue *queue; > >> struct vfio_ap_queue *q; > >> - struct device *dev; > >> > >> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) > >> - return NULL; > >> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) > > I think you should add some explanation to the patch description why > > testing the matrix bitmasks is not needed anymore. > > As a result of this comment, I took a closer look at the code to > determine the reason for eliminating the matrix_mdev > parameter. The reason is because the code below (i.e., find the device > and get the driver data) was also repeated in the vfio_ap_irq_disable_apqn() > function, so I replaced it with a call to the function above; however, the > vfio_ap_irq_disable_apqn() function does not have a reference to the > matrix_mdev, so I eliminated the matrix_mdev parameter. Note that the > vfio_ap_irq_disable_apqn() is called for each APQN assigned to a matrix > mdev, so there is no need to test the bitmasks there. > > The other place from which the function above is called is > the handle_pqap() function which does have a reference to the > matrix_mdev. In order to ensure the integrity of the instruction > being intercepted - i.e., PQAP(AQIC) enable/disable IRQ for aN > AP queue - the testing of the matrix bitmasks probably ought to > be performed, so it will be done there instead of in the > vfio_ap_get_queue() function above. I'm a little confused. I do agree that in handle_pqap() we do want to make sure that we only operate on queues that belong to the given guest that issued the PQAP instruction. AFAICT with this patch set applied, this is not the case any more. Does that 'will be done there instead' refer to v11? Another question is, can we use vfio_ap_get_mdev_queue() in handle_pqap() (instead of vfio_ap_get_queue())? > > > > + queue = ap_get_qdev(apqn); > > + if (!queue) > > return NULL; > > > > - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, > > - &apqn, match_apqn); > > - if (!dev) > > - return NULL; > > - q = dev_get_drvdata(dev); > > - q->matrix_mdev = matrix_mdev; > > - put_device(dev); > > + q = dev_get_drvdata(&queue->ap_dev.device); > > + put_device(&queue->ap_dev.device); > > > > return q; > > } > > (...) > > >
On Fri, 21 Aug 2020 15:56:02 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -26,43 +26,26 @@ > > static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); > > -static int match_apqn(struct device *dev, const void *data) > -{ > - struct vfio_ap_queue *q = dev_get_drvdata(dev); > - > - return (q->apqn == *(int *)(data)) ? 1 : 0; > -} > - > /** > - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list > - * @matrix_mdev: the associated mediated matrix > + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. > * @apqn: The queue APQN > * > - * Retrieve a queue with a specific APQN from the list of the > - * devices of the vfio_ap_drv. > - * Verify that the APID and the APQI are set in the matrix. > + * Retrieve a queue with a specific APQN from the AP queue devices attached to > + * the AP bus. > * > - * Returns the pointer to the associated vfio_ap_queue > + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. > */ > -static struct vfio_ap_queue *vfio_ap_get_queue( > - struct ap_matrix_mdev *matrix_mdev, > - int apqn) > +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) > { > + struct ap_queue *queue; > struct vfio_ap_queue *q; > - struct device *dev; > > - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) > - return NULL; > - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) > + queue = ap_get_qdev(apqn); > + if (!queue) > return NULL; > > - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, > - &apqn, match_apqn); > - if (!dev) > - return NULL; > - q = dev_get_drvdata(dev); > - q->matrix_mdev = matrix_mdev; > - put_device(dev); > + q = dev_get_drvdata(&queue->ap_dev.device); Is this cast here safe? (I don't think it is.) > + put_device(&queue->ap_dev.device); > > return q; > }
On 9/24/20 10:27 PM, Halil Pasic wrote: > On Fri, 21 Aug 2020 15:56:02 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> --- a/drivers/s390/crypto/vfio_ap_ops.c >> +++ b/drivers/s390/crypto/vfio_ap_ops.c >> @@ -26,43 +26,26 @@ >> >> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); >> >> -static int match_apqn(struct device *dev, const void *data) >> -{ >> - struct vfio_ap_queue *q = dev_get_drvdata(dev); >> - >> - return (q->apqn == *(int *)(data)) ? 1 : 0; >> -} >> - >> /** >> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list >> - * @matrix_mdev: the associated mediated matrix >> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. >> * @apqn: The queue APQN >> * >> - * Retrieve a queue with a specific APQN from the list of the >> - * devices of the vfio_ap_drv. >> - * Verify that the APID and the APQI are set in the matrix. >> + * Retrieve a queue with a specific APQN from the AP queue devices attached to >> + * the AP bus. >> * >> - * Returns the pointer to the associated vfio_ap_queue >> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. >> */ >> -static struct vfio_ap_queue *vfio_ap_get_queue( >> - struct ap_matrix_mdev *matrix_mdev, >> - int apqn) >> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) >> { >> + struct ap_queue *queue; >> struct vfio_ap_queue *q; >> - struct device *dev; >> >> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) >> - return NULL; >> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) >> + queue = ap_get_qdev(apqn); >> + if (!queue) >> return NULL; >> >> - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, >> - &apqn, match_apqn); >> - if (!dev) >> - return NULL; >> - q = dev_get_drvdata(dev); >> - q->matrix_mdev = matrix_mdev; >> - put_device(dev); >> + q = dev_get_drvdata(&queue->ap_dev.device); > Is this cast here safe? (I don't think it is.) In the probe, we execute: dev_set_drvdata(&queue->ap_dev.device, q); I don't get any compile nor execution errors. Why wouldn't it be safe? > >> + put_device(&queue->ap_dev.device); >> >> return q; >> }
On Tue, 29 Sep 2020 09:07:40 -0400 Tony Krowiak <akrowiak@linux.ibm.com> wrote: > > > On 9/24/20 10:27 PM, Halil Pasic wrote: > > On Fri, 21 Aug 2020 15:56:02 -0400 > > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > > > >> --- a/drivers/s390/crypto/vfio_ap_ops.c > >> +++ b/drivers/s390/crypto/vfio_ap_ops.c > >> @@ -26,43 +26,26 @@ > >> > >> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); > >> > >> -static int match_apqn(struct device *dev, const void *data) > >> -{ > >> - struct vfio_ap_queue *q = dev_get_drvdata(dev); > >> - > >> - return (q->apqn == *(int *)(data)) ? 1 : 0; > >> -} > >> - > >> /** > >> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list > >> - * @matrix_mdev: the associated mediated matrix > >> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. > >> * @apqn: The queue APQN > >> * > >> - * Retrieve a queue with a specific APQN from the list of the > >> - * devices of the vfio_ap_drv. > >> - * Verify that the APID and the APQI are set in the matrix. > >> + * Retrieve a queue with a specific APQN from the AP queue devices attached to > >> + * the AP bus. > >> * > >> - * Returns the pointer to the associated vfio_ap_queue > >> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. > >> */ > >> -static struct vfio_ap_queue *vfio_ap_get_queue( > >> - struct ap_matrix_mdev *matrix_mdev, > >> - int apqn) > >> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) > >> { > >> + struct ap_queue *queue; > >> struct vfio_ap_queue *q; > >> - struct device *dev; > >> > >> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) > >> - return NULL; > >> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) > >> + queue = ap_get_qdev(apqn); > >> + if (!queue) > >> return NULL; > >> > >> - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, > >> - &apqn, match_apqn); > >> - if (!dev) > >> - return NULL; > >> - q = dev_get_drvdata(dev); > >> - q->matrix_mdev = matrix_mdev; > >> - put_device(dev); > >> + q = dev_get_drvdata(&queue->ap_dev.device); > > Is this cast here safe? (I don't think it is.) > > In the probe, we execute: > dev_set_drvdata(&queue->ap_dev.device, q); > > I don't get any compile nor execution errors. Why wouldn't it be safe? > Because the queue may or may not be bound to the vfio_ap driver. AFAICT this function can be called with an arbitrary APQN. If it is bound to another driver then drvdata is not likely to hold a struct vfio_ap_queue. > > > >> + put_device(&queue->ap_dev.device); > >> > >> return q; > >> } >
On 9/29/20 9:37 AM, Halil Pasic wrote: > On Tue, 29 Sep 2020 09:07:40 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> >> On 9/24/20 10:27 PM, Halil Pasic wrote: >>> On Fri, 21 Aug 2020 15:56:02 -0400 >>> Tony Krowiak <akrowiak@linux.ibm.com> wrote: >>> >>>> --- a/drivers/s390/crypto/vfio_ap_ops.c >>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c >>>> @@ -26,43 +26,26 @@ >>>> >>>> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); >>>> >>>> -static int match_apqn(struct device *dev, const void *data) >>>> -{ >>>> - struct vfio_ap_queue *q = dev_get_drvdata(dev); >>>> - >>>> - return (q->apqn == *(int *)(data)) ? 1 : 0; >>>> -} >>>> - >>>> /** >>>> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list >>>> - * @matrix_mdev: the associated mediated matrix >>>> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. >>>> * @apqn: The queue APQN >>>> * >>>> - * Retrieve a queue with a specific APQN from the list of the >>>> - * devices of the vfio_ap_drv. >>>> - * Verify that the APID and the APQI are set in the matrix. >>>> + * Retrieve a queue with a specific APQN from the AP queue devices attached to >>>> + * the AP bus. >>>> * >>>> - * Returns the pointer to the associated vfio_ap_queue >>>> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. >>>> */ >>>> -static struct vfio_ap_queue *vfio_ap_get_queue( >>>> - struct ap_matrix_mdev *matrix_mdev, >>>> - int apqn) >>>> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) >>>> { >>>> + struct ap_queue *queue; >>>> struct vfio_ap_queue *q; >>>> - struct device *dev; >>>> >>>> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) >>>> - return NULL; >>>> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) >>>> + queue = ap_get_qdev(apqn); >>>> + if (!queue) >>>> return NULL; >>>> >>>> - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, >>>> - &apqn, match_apqn); >>>> - if (!dev) >>>> - return NULL; >>>> - q = dev_get_drvdata(dev); >>>> - q->matrix_mdev = matrix_mdev; >>>> - put_device(dev); >>>> + q = dev_get_drvdata(&queue->ap_dev.device); >>> Is this cast here safe? (I don't think it is.) >> In the probe, we execute: >> dev_set_drvdata(&queue->ap_dev.device, q); >> >> I don't get any compile nor execution errors. Why wouldn't it be safe? >> > Because the queue may or may not be bound to the vfio_ap driver. AFAICT > this function can be called with an arbitrary APQN. > > If it is bound to another driver then drvdata is not likely to hold a > struct vfio_ap_queue. Then the function will return NULL. All callers must check for NULL before using it which is the case in all places where this function is called. > > >>>> + put_device(&queue->ap_dev.device); >>>> >>>> return q; >>>> }
On 9/24/20 10:11 PM, Halil Pasic wrote: > On Thu, 27 Aug 2020 10:24:07 -0400 > Tony Krowiak <akrowiak@linux.ibm.com> wrote: > >> >> On 8/25/20 6:13 AM, Cornelia Huck wrote: >>> On Fri, 21 Aug 2020 15:56:02 -0400 >>> Tony Krowiak<akrowiak@linux.ibm.com> wrote: >>> >>>> This patch refactor's the vfio_ap device driver to use the AP bus's >>> s/refactor's/refactors/ >> Of course, what was I thinking?:) >> >>>> ap_get_qdev() function to retrieve the vfio_ap_queue struct containing >>>> information about a queue that is bound to the vfio_ap device driver. >>>> The bus's ap_get_qdev() function retrieves the queue device from a >>>> hashtable keyed by APQN. This is much more efficient than looping over >>>> the list of devices attached to the AP bus by several orders of >>>> magnitude. >>>> >>>> Signed-off-by: Tony Krowiak<akrowiak@linux.ibm.com> >>>> Reported-by: kernel test robot<lkp@intel.com> >>>> --- >>>> drivers/s390/crypto/vfio_ap_drv.c | 27 ++------- >>>> drivers/s390/crypto/vfio_ap_ops.c | 86 +++++++++++++++------------ >>>> drivers/s390/crypto/vfio_ap_private.h | 8 ++- >>>> 3 files changed, 59 insertions(+), 62 deletions(-) >>>> >>> (...) >>> >>>> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c >>>> index e0bde8518745..ad3925f04f61 100644 >>>> --- a/drivers/s390/crypto/vfio_ap_ops.c >>>> +++ b/drivers/s390/crypto/vfio_ap_ops.c >>>> @@ -26,43 +26,26 @@vfio_ap_get_queue() >>>> >>>> static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); >>>> >>>> -static int match_apqn(struct device *dev, const void *data) >>>> -{ >>>> - struct vfio_ap_queue *q = dev_get_drvdata(dev); >>>> - >>>> - return (q->apqn == *(int *)(data)) ? 1 : 0; >>>> -} >>>> - >>>> /** >>>> - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list >>>> - * @matrix_mdev: the associated mediated matrix >>>> + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. >>>> * @apqn: The queue APQN >>>> * >>>> - * Retrieve a queue with a specific APQN from the list of the >>>> - * devices of the vfio_ap_drv. >>>> - * Verify that the APID and the APQI are set in the matrix. >>>> + * Retrieve a queue with a specific APQN from the AP queue devices attached to >>>> + * the AP bus. >>>> * >>>> - * Returns the pointer to the associated vfio_ap_queue >>>> + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. >>>> */ >>>> -static struct vfio_ap_queue *vfio_ap_get_queue( >>>> - struct ap_matrix_mdev *matrix_mdev, >>>> - int apqn) >>>> +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) >>>> { >>>> + struct ap_queue *queue; >>>> struct vfio_ap_queue *q; >>>> - struct device *dev; >>>> >>>> - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) >>>> - return NULL; >>>> - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) >>> I think you should add some explanation to the patch description why >>> testing the matrix bitmasks is not needed anymore. >> As a result of this comment, I took a closer look at the code to >> determine the reason for eliminating the matrix_mdev >> parameter. The reason is because the code below (i.e., find the device >> and get the driver data) was also repeated in the vfio_ap_irq_disable_apqn() >> function, so I replaced it with a call to the function above; however, the >> vfio_ap_irq_disable_apqn() function does not have a reference to the >> matrix_mdev, so I eliminated the matrix_mdev parameter. Note that the >> vfio_ap_irq_disable_apqn() is called for each APQN assigned to a matrix >> mdev, so there is no need to test the bitmasks there. >> >> The other place from which the function above is called is >> the handle_pqap() function which does have a reference to the >> matrix_mdev. In order to ensure the integrity of the instruction >> being intercepted - i.e., PQAP(AQIC) enable/disable IRQ for aN >> AP queue - the testing of the matrix bitmasks probably ought to >> be performed, so it will be done there instead of in the >> vfio_ap_get_queue() function above. > I'm a little confused. I do agree that in handle_pqap() we do want to > make sure that we only operate on queues that belong to the given guest > that issued the PQAP instruction. > > AFAICT with this patch set applied, this is not the case any more. Does > that 'will be done there instead' refer to v11? I understand your confusion, so here is what I'm going to do to clear things up. I will leave the signature of the vfio_ap_get_queue() function the same and leave in the bitmap checking. As per your comment below, in patch 3 I will replace the call to vfio_ap_get_queue() with a call to vfio_ap_get_mdev_queue(). Since the vfio_ap_get_mdev_queue() function is mdev-specific, I can then remove the mdev parameter from the vfio_ap_get_queue() function since it will no longer be needed. > Another question is, can we use vfio_ap_get_mdev_queue() in > handle_pqap() (instead of vfio_ap_get_queue())? Yes, we can and should do that as it will eliminate both the need to test the matrix bitmasks and several lines of code; however, that function is not available until patch 3/16, so that change will be made there. > >> >>> + queue = ap_get_qdev(apqn); >>> + if (!queue) >>> return NULL; >>> >>> - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, >>> - &apqn, match_apqn); >>> - if (!dev) >>> - return NULL; >>> - q = dev_get_drvdata(dev); >>> - q->matrix_mdev = matrix_mdev; >>> - put_device(dev); >>> + q = dev_get_drvdata(&queue->ap_dev.device); >>> + put_device(&queue->ap_dev.device); >>> >>> return q; >>> } >>> (...) >>>
diff --git a/drivers/s390/crypto/vfio_ap_drv.c b/drivers/s390/crypto/vfio_ap_drv.c index f4ceb380dd61..24cdef60039a 100644 --- a/drivers/s390/crypto/vfio_ap_drv.c +++ b/drivers/s390/crypto/vfio_ap_drv.c @@ -53,15 +53,9 @@ MODULE_DEVICE_TABLE(vfio_ap, ap_queue_ids); */ static int vfio_ap_queue_dev_probe(struct ap_device *apdev) { - struct vfio_ap_queue *q; - - q = kzalloc(sizeof(*q), GFP_KERNEL); - if (!q) - return -ENOMEM; - dev_set_drvdata(&apdev->device, q); - q->apqn = to_ap_queue(&apdev->device)->qid; - q->saved_isc = VFIO_AP_ISC_INVALID; - return 0; + struct ap_queue *queue = to_ap_queue(&apdev->device); + + return vfio_ap_mdev_probe_queue(queue); } /** @@ -72,18 +66,9 @@ static int vfio_ap_queue_dev_probe(struct ap_device *apdev) */ static void vfio_ap_queue_dev_remove(struct ap_device *apdev) { - struct vfio_ap_queue *q; - int apid, apqi; - - mutex_lock(&matrix_dev->lock); - q = dev_get_drvdata(&apdev->device); - dev_set_drvdata(&apdev->device, NULL); - apid = AP_QID_CARD(q->apqn); - apqi = AP_QID_QUEUE(q->apqn); - vfio_ap_mdev_reset_queue(apid, apqi, 1); - vfio_ap_irq_disable(q); - kfree(q); - mutex_unlock(&matrix_dev->lock); + struct ap_queue *queue = to_ap_queue(&apdev->device); + + vfio_ap_mdev_remove_queue(queue); } static void vfio_ap_matrix_dev_release(struct device *dev) diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index e0bde8518745..ad3925f04f61 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -26,43 +26,26 @@ static int vfio_ap_mdev_reset_queues(struct mdev_device *mdev); -static int match_apqn(struct device *dev, const void *data) -{ - struct vfio_ap_queue *q = dev_get_drvdata(dev); - - return (q->apqn == *(int *)(data)) ? 1 : 0; -} - /** - * vfio_ap_get_queue: Retrieve a queue with a specific APQN from a list - * @matrix_mdev: the associated mediated matrix + * vfio_ap_get_queue: Retrieve a queue with a specific APQN. * @apqn: The queue APQN * - * Retrieve a queue with a specific APQN from the list of the - * devices of the vfio_ap_drv. - * Verify that the APID and the APQI are set in the matrix. + * Retrieve a queue with a specific APQN from the AP queue devices attached to + * the AP bus. * - * Returns the pointer to the associated vfio_ap_queue + * Returns the pointer to the vfio_ap_queue with the specified APQN, or NULL. */ -static struct vfio_ap_queue *vfio_ap_get_queue( - struct ap_matrix_mdev *matrix_mdev, - int apqn) +static struct vfio_ap_queue *vfio_ap_get_queue(unsigned long apqn) { + struct ap_queue *queue; struct vfio_ap_queue *q; - struct device *dev; - if (!test_bit_inv(AP_QID_CARD(apqn), matrix_mdev->matrix.apm)) - return NULL; - if (!test_bit_inv(AP_QID_QUEUE(apqn), matrix_mdev->matrix.aqm)) + queue = ap_get_qdev(apqn); + if (!queue) return NULL; - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, - &apqn, match_apqn); - if (!dev) - return NULL; - q = dev_get_drvdata(dev); - q->matrix_mdev = matrix_mdev; - put_device(dev); + q = dev_get_drvdata(&queue->ap_dev.device); + put_device(&queue->ap_dev.device); return q; } @@ -144,7 +127,7 @@ static void vfio_ap_free_aqic_resources(struct vfio_ap_queue *q) * Returns if ap_aqic function failed with invalid, deconfigured or * checkstopped AP. */ -struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q) +static struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q) { struct ap_qirq_ctrl aqic_gisa = {}; struct ap_queue_status status; @@ -293,10 +276,11 @@ static int handle_pqap(struct kvm_vcpu *vcpu) matrix_mdev = container_of(vcpu->kvm->arch.crypto.pqap_hook, struct ap_matrix_mdev, pqap_hook); - q = vfio_ap_get_queue(matrix_mdev, apqn); + q = vfio_ap_get_queue(apqn); if (!q) goto out_unlock; + q->matrix_mdev = matrix_mdev; status = vcpu->run->s.regs.gprs[1]; /* If IR bit(16) is set we enable the interrupt */ @@ -1116,20 +1100,15 @@ static int vfio_ap_mdev_group_notifier(struct notifier_block *nb, static void vfio_ap_irq_disable_apqn(int apqn) { - struct device *dev; struct vfio_ap_queue *q; - dev = driver_find_device(&matrix_dev->vfio_ap_drv->driver, NULL, - &apqn, match_apqn); - if (dev) { - q = dev_get_drvdata(dev); + q = vfio_ap_get_queue(apqn); + if (q) vfio_ap_irq_disable(q); - put_device(dev); - } } -int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi, - unsigned int retry) +static int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi, + unsigned int retry) { struct ap_queue_status status; int retry2 = 2; @@ -1302,3 +1281,34 @@ void vfio_ap_mdev_unregister(void) { mdev_unregister_device(&matrix_dev->device); } + +int vfio_ap_mdev_probe_queue(struct ap_queue *queue) +{ + struct vfio_ap_queue *q; + + q = kzalloc(sizeof(*q), GFP_KERNEL); + if (!q) + return -ENOMEM; + + dev_set_drvdata(&queue->ap_dev.device, q); + q->apqn = queue->qid; + q->saved_isc = VFIO_AP_ISC_INVALID; + + return 0; +} + +void vfio_ap_mdev_remove_queue(struct ap_queue *queue) +{ + struct vfio_ap_queue *q; + int apid, apqi; + + mutex_lock(&matrix_dev->lock); + q = dev_get_drvdata(&queue->ap_dev.device); + dev_set_drvdata(&queue->ap_dev.device, NULL); + apid = AP_QID_CARD(q->apqn); + apqi = AP_QID_QUEUE(q->apqn); + vfio_ap_mdev_reset_queue(apid, apqi, 1); + vfio_ap_irq_disable(q); + kfree(q); + mutex_unlock(&matrix_dev->lock); +} diff --git a/drivers/s390/crypto/vfio_ap_private.h b/drivers/s390/crypto/vfio_ap_private.h index f46dde56b464..a2aa05bec718 100644 --- a/drivers/s390/crypto/vfio_ap_private.h +++ b/drivers/s390/crypto/vfio_ap_private.h @@ -18,6 +18,7 @@ #include <linux/delay.h> #include <linux/mutex.h> #include <linux/kvm_host.h> +#include <linux/hashtable.h> #include "ap_bus.h" @@ -90,8 +91,6 @@ struct ap_matrix_mdev { extern int vfio_ap_mdev_register(void); extern void vfio_ap_mdev_unregister(void); -int vfio_ap_mdev_reset_queue(unsigned int apid, unsigned int apqi, - unsigned int retry); struct vfio_ap_queue { struct ap_matrix_mdev *matrix_mdev; @@ -100,5 +99,8 @@ struct vfio_ap_queue { #define VFIO_AP_ISC_INVALID 0xff unsigned char saved_isc; }; -struct ap_queue_status vfio_ap_irq_disable(struct vfio_ap_queue *q); + +int vfio_ap_mdev_probe_queue(struct ap_queue *queue); +void vfio_ap_mdev_remove_queue(struct ap_queue *queue); + #endif /* _VFIO_AP_PRIVATE_H_ */
This patch refactor's the vfio_ap device driver to use the AP bus's ap_get_qdev() function to retrieve the vfio_ap_queue struct containing information about a queue that is bound to the vfio_ap device driver. The bus's ap_get_qdev() function retrieves the queue device from a hashtable keyed by APQN. This is much more efficient than looping over the list of devices attached to the AP bus by several orders of magnitude. Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> Reported-by: kernel test robot <lkp@intel.com> --- drivers/s390/crypto/vfio_ap_drv.c | 27 ++------- drivers/s390/crypto/vfio_ap_ops.c | 86 +++++++++++++++------------ drivers/s390/crypto/vfio_ap_private.h | 8 ++- 3 files changed, 59 insertions(+), 62 deletions(-)