| Message ID | 20220621155134.1932383-11-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/vfio-ap: dynamic configuration support | expand |
On 6/21/22 11:51, Tony Krowiak wrote: > The functions backing the matrix mdev's sysfs attribute interfaces to > assign/unassign adapters, domains and control domains must take and > release the locks required to perform a dynamic update of a guest's APCB > in the proper order. > > The proper order for taking the locks is: > > matrix_dev->guests_lock => kvm->lock => matrix_dev->mdevs_lock > > The proper order for releasing the locks is: > > matrix_dev->mdevs_lock => kvm->lock => matrix_dev->guests_lock > > Two new macros are introduced for this purpose: One to take the locks and > the other to release the locks. These macros will be used by the > assignment/unassignment functions to prepare for dynamic update of > the KVM guest's APCB. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 69 +++++++++++++++++++++++++------ > 1 file changed, 57 insertions(+), 12 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index f31db1248740..b8f901e6b580 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -75,6 +75,51 @@ static const struct vfio_device_ops vfio_ap_matrix_dev_ops; > mutex_unlock(&matrix_dev->guests_lock); \ > }) > > +/** > + * get_update_locks_for_mdev: Acquire the locks required to dynamically update a > + * KVM guest's APCB in the proper order. > + * > + * @matrix_mdev: a pointer to a struct ap_matrix_mdev object containing the AP > + * configuration data to use to update a KVM guest's APCB. > + * > + * The proper locking order is: > + * 1. matrix_dev->guests_lock: required to use the KVM pointer to update a KVM > + * guest's APCB. > + * 2. matrix_mdev->kvm->lock: required to update a guest's APCB > + * 3. matrix_dev->mdevs_lock: required to access data stored in a matrix_mdev > + * > + * Note: If @matrix_mdev is NULL or is not attached to a KVM guest, the KVM > + * lock will not be taken. > + */ > +#define get_update_locks_for_mdev(matrix_mdev) ({ \ > + mutex_lock(&matrix_dev->guests_lock); \ > + if (matrix_mdev && matrix_mdev->kvm) \ > + mutex_lock(&matrix_mdev->kvm->lock); \ > + mutex_lock(&matrix_dev->mdevs_lock); \ > +}) > + > +/** > + * release_update_locks_for_mdev: Release the locks used to dynamically update a > + * KVM guest's APCB in the proper order. > + * > + * @matrix_mdev: a pointer to a struct ap_matrix_mdev object containing the AP > + * configuration data to use to update a KVM guest's APCB. > + * > + * The proper unlocking order is: > + * 1. matrix_dev->mdevs_lock > + * 2. matrix_mdev->kvm->lock > + * 3. matrix_dev->guests_lock > + * > + * Note: If @matrix_mdev is NULL or is not attached to a KVM guest, the KVM > + * lock will not be released. > + */ > +#define release_update_locks_for_mdev(matrix_mdev) ({ \ > + mutex_unlock(&matrix_dev->mdevs_lock); \ > + if (matrix_mdev && matrix_mdev->kvm) \ > + mutex_unlock(&matrix_mdev->kvm->lock); \ > + mutex_unlock(&matrix_dev->guests_lock); \ > +}) > + > /** > * vfio_ap_mdev_get_queue - retrieve a queue with a specific APQN from a > * hash table of queues assigned to a matrix mdev > @@ -830,7 +875,7 @@ static ssize_t assign_adapter_store(struct device *dev, > > struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); > > - mutex_lock(&matrix_dev->mdevs_lock); > + get_update_locks_for_mdev(matrix_mdev); > > /* If the KVM guest is running, disallow assignment of adapter */ > if (matrix_mdev->kvm) { > @@ -862,7 +907,7 @@ static ssize_t assign_adapter_store(struct device *dev, > matrix_mdev->matrix.aqm, matrix_mdev); > ret = count; > done: > - mutex_unlock(&matrix_dev->mdevs_lock); > + release_update_locks_for_mdev(matrix_mdev); > > return ret; > } > @@ -905,7 +950,7 @@ static ssize_t unassign_adapter_store(struct device *dev, > unsigned long apid; > struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); > > - mutex_lock(&matrix_dev->mdevs_lock); > + get_update_locks_for_mdev(matrix_mdev); > > /* If the KVM guest is running, disallow unassignment of adapter */ > if (matrix_mdev->kvm) { > @@ -930,7 +975,7 @@ static ssize_t unassign_adapter_store(struct device *dev, > > ret = count; > done: > - mutex_unlock(&matrix_dev->mdevs_lock); > + release_update_locks_for_mdev(matrix_mdev); > return ret; > } > static DEVICE_ATTR_WO(unassign_adapter); > @@ -985,7 +1030,7 @@ static ssize_t assign_domain_store(struct device *dev, > struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); > unsigned long max_apqi = matrix_mdev->matrix.aqm_max; > > - mutex_lock(&matrix_dev->mdevs_lock); > + get_update_locks_for_mdev(matrix_mdev); > > /* If the KVM guest is running, disallow assignment of domain */ > if (matrix_mdev->kvm) { > @@ -1016,7 +1061,7 @@ static ssize_t assign_domain_store(struct device *dev, > matrix_mdev); > ret = count; > done: > - mutex_unlock(&matrix_dev->mdevs_lock); > + release_update_locks_for_mdev(matrix_mdev); > > return ret; > } > @@ -1059,7 +1104,7 @@ static ssize_t unassign_domain_store(struct device *dev, > unsigned long apqi; > struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); > > - mutex_lock(&matrix_dev->mdevs_lock); > + get_update_locks_for_mdev(matrix_mdev); > > /* If the KVM guest is running, disallow unassignment of domain */ > if (matrix_mdev->kvm) { > @@ -1085,7 +1130,7 @@ static ssize_t unassign_domain_store(struct device *dev, > ret = count; > > done: > - mutex_unlock(&matrix_dev->mdevs_lock); > + release_update_locks_for_mdev(matrix_mdev); > return ret; > } > static DEVICE_ATTR_WO(unassign_domain); > @@ -1112,7 +1157,7 @@ static ssize_t assign_control_domain_store(struct device *dev, > unsigned long id; > struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); > > - mutex_lock(&matrix_dev->mdevs_lock); > + get_update_locks_for_mdev(matrix_mdev); > > /* If the KVM guest is running, disallow assignment of control domain */ > if (matrix_mdev->kvm) { > @@ -1138,7 +1183,7 @@ static ssize_t assign_control_domain_store(struct device *dev, > vfio_ap_mdev_filter_cdoms(matrix_mdev); > ret = count; > done: > - mutex_unlock(&matrix_dev->mdevs_lock); > + release_update_locks_for_mdev(matrix_mdev); > return ret; > } > static DEVICE_ATTR_WO(assign_control_domain); > @@ -1166,7 +1211,7 @@ static ssize_t unassign_control_domain_store(struct device *dev, > struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); > unsigned long max_domid = matrix_mdev->matrix.adm_max; > > - mutex_lock(&matrix_dev->mdevs_lock); > + get_update_locks_for_mdev(matrix_mdev); > > /* If a KVM guest is running, disallow unassignment of control domain */ > if (matrix_mdev->kvm) { > @@ -1189,7 +1234,7 @@ static ssize_t unassign_control_domain_store(struct device *dev, > > ret = count; > done: > - mutex_unlock(&matrix_dev->mdevs_lock); > + release_update_locks_for_mdev(matrix_mdev); > return ret; > } > static DEVICE_ATTR_WO(unassign_control_domain); Reviewed-by: Jason J. Herne <jjherne@linux.ibm.com>
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index f31db1248740..b8f901e6b580 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -75,6 +75,51 @@ static const struct vfio_device_ops vfio_ap_matrix_dev_ops; mutex_unlock(&matrix_dev->guests_lock); \ }) +/** + * get_update_locks_for_mdev: Acquire the locks required to dynamically update a + * KVM guest's APCB in the proper order. + * + * @matrix_mdev: a pointer to a struct ap_matrix_mdev object containing the AP + * configuration data to use to update a KVM guest's APCB. + * + * The proper locking order is: + * 1. matrix_dev->guests_lock: required to use the KVM pointer to update a KVM + * guest's APCB. + * 2. matrix_mdev->kvm->lock: required to update a guest's APCB + * 3. matrix_dev->mdevs_lock: required to access data stored in a matrix_mdev + * + * Note: If @matrix_mdev is NULL or is not attached to a KVM guest, the KVM + * lock will not be taken. + */ +#define get_update_locks_for_mdev(matrix_mdev) ({ \ + mutex_lock(&matrix_dev->guests_lock); \ + if (matrix_mdev && matrix_mdev->kvm) \ + mutex_lock(&matrix_mdev->kvm->lock); \ + mutex_lock(&matrix_dev->mdevs_lock); \ +}) + +/** + * release_update_locks_for_mdev: Release the locks used to dynamically update a + * KVM guest's APCB in the proper order. + * + * @matrix_mdev: a pointer to a struct ap_matrix_mdev object containing the AP + * configuration data to use to update a KVM guest's APCB. + * + * The proper unlocking order is: + * 1. matrix_dev->mdevs_lock + * 2. matrix_mdev->kvm->lock + * 3. matrix_dev->guests_lock + * + * Note: If @matrix_mdev is NULL or is not attached to a KVM guest, the KVM + * lock will not be released. + */ +#define release_update_locks_for_mdev(matrix_mdev) ({ \ + mutex_unlock(&matrix_dev->mdevs_lock); \ + if (matrix_mdev && matrix_mdev->kvm) \ + mutex_unlock(&matrix_mdev->kvm->lock); \ + mutex_unlock(&matrix_dev->guests_lock); \ +}) + /** * vfio_ap_mdev_get_queue - retrieve a queue with a specific APQN from a * hash table of queues assigned to a matrix mdev @@ -830,7 +875,7 @@ static ssize_t assign_adapter_store(struct device *dev, struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); - mutex_lock(&matrix_dev->mdevs_lock); + get_update_locks_for_mdev(matrix_mdev); /* If the KVM guest is running, disallow assignment of adapter */ if (matrix_mdev->kvm) { @@ -862,7 +907,7 @@ static ssize_t assign_adapter_store(struct device *dev, matrix_mdev->matrix.aqm, matrix_mdev); ret = count; done: - mutex_unlock(&matrix_dev->mdevs_lock); + release_update_locks_for_mdev(matrix_mdev); return ret; } @@ -905,7 +950,7 @@ static ssize_t unassign_adapter_store(struct device *dev, unsigned long apid; struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); - mutex_lock(&matrix_dev->mdevs_lock); + get_update_locks_for_mdev(matrix_mdev); /* If the KVM guest is running, disallow unassignment of adapter */ if (matrix_mdev->kvm) { @@ -930,7 +975,7 @@ static ssize_t unassign_adapter_store(struct device *dev, ret = count; done: - mutex_unlock(&matrix_dev->mdevs_lock); + release_update_locks_for_mdev(matrix_mdev); return ret; } static DEVICE_ATTR_WO(unassign_adapter); @@ -985,7 +1030,7 @@ static ssize_t assign_domain_store(struct device *dev, struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); unsigned long max_apqi = matrix_mdev->matrix.aqm_max; - mutex_lock(&matrix_dev->mdevs_lock); + get_update_locks_for_mdev(matrix_mdev); /* If the KVM guest is running, disallow assignment of domain */ if (matrix_mdev->kvm) { @@ -1016,7 +1061,7 @@ static ssize_t assign_domain_store(struct device *dev, matrix_mdev); ret = count; done: - mutex_unlock(&matrix_dev->mdevs_lock); + release_update_locks_for_mdev(matrix_mdev); return ret; } @@ -1059,7 +1104,7 @@ static ssize_t unassign_domain_store(struct device *dev, unsigned long apqi; struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); - mutex_lock(&matrix_dev->mdevs_lock); + get_update_locks_for_mdev(matrix_mdev); /* If the KVM guest is running, disallow unassignment of domain */ if (matrix_mdev->kvm) { @@ -1085,7 +1130,7 @@ static ssize_t unassign_domain_store(struct device *dev, ret = count; done: - mutex_unlock(&matrix_dev->mdevs_lock); + release_update_locks_for_mdev(matrix_mdev); return ret; } static DEVICE_ATTR_WO(unassign_domain); @@ -1112,7 +1157,7 @@ static ssize_t assign_control_domain_store(struct device *dev, unsigned long id; struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); - mutex_lock(&matrix_dev->mdevs_lock); + get_update_locks_for_mdev(matrix_mdev); /* If the KVM guest is running, disallow assignment of control domain */ if (matrix_mdev->kvm) { @@ -1138,7 +1183,7 @@ static ssize_t assign_control_domain_store(struct device *dev, vfio_ap_mdev_filter_cdoms(matrix_mdev); ret = count; done: - mutex_unlock(&matrix_dev->mdevs_lock); + release_update_locks_for_mdev(matrix_mdev); return ret; } static DEVICE_ATTR_WO(assign_control_domain); @@ -1166,7 +1211,7 @@ static ssize_t unassign_control_domain_store(struct device *dev, struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); unsigned long max_domid = matrix_mdev->matrix.adm_max; - mutex_lock(&matrix_dev->mdevs_lock); + get_update_locks_for_mdev(matrix_mdev); /* If a KVM guest is running, disallow unassignment of control domain */ if (matrix_mdev->kvm) { @@ -1189,7 +1234,7 @@ static ssize_t unassign_control_domain_store(struct device *dev, ret = count; done: - mutex_unlock(&matrix_dev->mdevs_lock); + release_update_locks_for_mdev(matrix_mdev); return ret; } static DEVICE_ATTR_WO(unassign_control_domain);
The functions backing the matrix mdev's sysfs attribute interfaces to assign/unassign adapters, domains and control domains must take and release the locks required to perform a dynamic update of a guest's APCB in the proper order. The proper order for taking the locks is: matrix_dev->guests_lock => kvm->lock => matrix_dev->mdevs_lock The proper order for releasing the locks is: matrix_dev->mdevs_lock => kvm->lock => matrix_dev->guests_lock Two new macros are introduced for this purpose: One to take the locks and the other to release the locks. These macros will be used by the assignment/unassignment functions to prepare for dynamic update of the KVM guest's APCB. Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> --- drivers/s390/crypto/vfio_ap_ops.c | 69 +++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 12 deletions(-)