| Message ID | 20220215005040.52697-8-akrowiak@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | s390/vfio-ap: dynamic configuration support | expand |
On 2/14/22 19:50, Tony Krowiak wrote: > Refresh the guest's APCB by filtering the APQNs assigned to the matrix mdev > that do not reference an AP queue device bound to the vfio_ap device > driver. The mdev's APQNs will be filtered according to the following rules: > > * The APID of each adapter and the APQI of each domain that is not in the > host's AP configuration is filtered out. > > * The APID of each adapter comprising an APQN that does not reference a > queue device bound to the vfio_ap device driver is filtered. The APQNs > are derived from the Cartesian product of the APID of each adapter and > APQI of each domain assigned to the mdev. > > The control domains that are not assigned to the host's AP configuration > will also be filtered before assigning them to the guest's APCB. > > Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> > --- > drivers/s390/crypto/vfio_ap_ops.c | 96 ++++++++++++++++++++++++++++++- > 1 file changed, 93 insertions(+), 3 deletions(-) > > diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c > index 4b676a55f203..b67b2f0faeea 100644 > --- a/drivers/s390/crypto/vfio_ap_ops.c > +++ b/drivers/s390/crypto/vfio_ap_ops.c > @@ -317,6 +317,63 @@ static void vfio_ap_matrix_init(struct ap_config_info *info, > matrix->adm_max = info->apxa ? info->Nd : 15; > } > > +static void vfio_ap_mdev_filter_cdoms(struct ap_matrix_mdev *matrix_mdev) > +{ > + bitmap_and(matrix_mdev->shadow_apcb.adm, matrix_mdev->matrix.adm, > + (unsigned long *)matrix_dev->info.adm, AP_DOMAINS); > +} > + > +/* > + * vfio_ap_mdev_filter_matrix - copy the mdev's AP configuration to the KVM > + * guest's APCB then filter the APIDs that do not > + * comprise at least one APQN that references a > + * queue device bound to the vfio_ap device driver. > + * > + * @matrix_mdev: the mdev whose AP configuration is to be filtered. > + */ > +static void vfio_ap_mdev_filter_matrix(unsigned long *apm, unsigned long *aqm, > + struct ap_matrix_mdev *matrix_mdev) > +{ > + int ret; > + unsigned long apid, apqi, apqn; > + > + ret = ap_qci(&matrix_dev->info); > + if (ret) > + return; > + > + vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->shadow_apcb); Do you need to call vfio_ap_matrix_init here? It seems to me like this would only be necesarry if apxa could be dynamically added or removed. Here is a copy of vfio_ap_matrix_init, for reference: 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; } It seems like this should be figured out once and stored when the ap_matrix_mdev struct is first created. Unless I'm wrong, and the status of apxa can change dynamically, in which case the maximums would need to be updated somewhere.
On 3/2/22 14:35, Jason J. Herne wrote: > On 2/14/22 19:50, Tony Krowiak wrote: >> Refresh the guest's APCB by filtering the APQNs assigned to the >> matrix mdev >> that do not reference an AP queue device bound to the vfio_ap device >> driver. The mdev's APQNs will be filtered according to the following >> rules: >> >> * The APID of each adapter and the APQI of each domain that is not in >> the >> host's AP configuration is filtered out. >> >> * The APID of each adapter comprising an APQN that does not reference a >> queue device bound to the vfio_ap device driver is filtered. The APQNs >> are derived from the Cartesian product of the APID of each adapter and >> APQI of each domain assigned to the mdev. >> >> The control domains that are not assigned to the host's AP configuration >> will also be filtered before assigning them to the guest's APCB. >> >> Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> >> --- >> drivers/s390/crypto/vfio_ap_ops.c | 96 ++++++++++++++++++++++++++++++- >> 1 file changed, 93 insertions(+), 3 deletions(-) >> >> diff --git a/drivers/s390/crypto/vfio_ap_ops.c >> b/drivers/s390/crypto/vfio_ap_ops.c >> index 4b676a55f203..b67b2f0faeea 100644 >> --- a/drivers/s390/crypto/vfio_ap_ops.c >> +++ b/drivers/s390/crypto/vfio_ap_ops.c >> @@ -317,6 +317,63 @@ static void vfio_ap_matrix_init(struct >> ap_config_info *info, >> matrix->adm_max = info->apxa ? info->Nd : 15; >> } >> +static void vfio_ap_mdev_filter_cdoms(struct ap_matrix_mdev >> *matrix_mdev) >> +{ >> + bitmap_and(matrix_mdev->shadow_apcb.adm, matrix_mdev->matrix.adm, >> + (unsigned long *)matrix_dev->info.adm, AP_DOMAINS); >> +} >> + >> +/* >> + * vfio_ap_mdev_filter_matrix - copy the mdev's AP configuration to >> the KVM >> + * guest's APCB then filter the APIDs that do not >> + * comprise at least one APQN that references a >> + * queue device bound to the vfio_ap device driver. >> + * >> + * @matrix_mdev: the mdev whose AP configuration is to be filtered. >> + */ >> +static void vfio_ap_mdev_filter_matrix(unsigned long *apm, unsigned >> long *aqm, >> + struct ap_matrix_mdev *matrix_mdev) >> +{ >> + int ret; >> + unsigned long apid, apqi, apqn; >> + >> + ret = ap_qci(&matrix_dev->info); >> + if (ret) >> + return; >> + >> + vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->shadow_apcb); > > Do you need to call vfio_ap_matrix_init here? It seems to me like this > would > only be necesarry if apxa could be dynamically added or removed. Here > is a > copy of vfio_ap_matrix_init, for reference: > > 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; > } > > It seems like this should be figured out once and stored when the > ap_matrix_mdev struct is first created. Unless I'm wrong, and the > status of > apxa can change dynamically, in which case the maximums would need to be > updated somewhere. It's an interesting question to which I don't have a definitive answer. I'll run it by our architects. On the other hand, making this call here is not entirely unreasonable and merely superfluous at worst, but I'll look into it. Tony K > >
diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c index 4b676a55f203..b67b2f0faeea 100644 --- a/drivers/s390/crypto/vfio_ap_ops.c +++ b/drivers/s390/crypto/vfio_ap_ops.c @@ -317,6 +317,63 @@ static void vfio_ap_matrix_init(struct ap_config_info *info, matrix->adm_max = info->apxa ? info->Nd : 15; } +static void vfio_ap_mdev_filter_cdoms(struct ap_matrix_mdev *matrix_mdev) +{ + bitmap_and(matrix_mdev->shadow_apcb.adm, matrix_mdev->matrix.adm, + (unsigned long *)matrix_dev->info.adm, AP_DOMAINS); +} + +/* + * vfio_ap_mdev_filter_matrix - copy the mdev's AP configuration to the KVM + * guest's APCB then filter the APIDs that do not + * comprise at least one APQN that references a + * queue device bound to the vfio_ap device driver. + * + * @matrix_mdev: the mdev whose AP configuration is to be filtered. + */ +static void vfio_ap_mdev_filter_matrix(unsigned long *apm, unsigned long *aqm, + struct ap_matrix_mdev *matrix_mdev) +{ + int ret; + unsigned long apid, apqi, apqn; + + ret = ap_qci(&matrix_dev->info); + if (ret) + return; + + vfio_ap_matrix_init(&matrix_dev->info, &matrix_mdev->shadow_apcb); + + /* + * Copy the adapters, domains and control domains to the shadow_apcb + * from the matrix mdev, but only those that are assigned to the host's + * AP configuration. + */ + bitmap_and(matrix_mdev->shadow_apcb.apm, matrix_mdev->matrix.apm, + (unsigned long *)matrix_dev->info.apm, AP_DEVICES); + bitmap_and(matrix_mdev->shadow_apcb.aqm, matrix_mdev->matrix.aqm, + (unsigned long *)matrix_dev->info.aqm, AP_DOMAINS); + + for_each_set_bit_inv(apid, apm, AP_DEVICES) { + for_each_set_bit_inv(apqi, aqm, AP_DOMAINS) { + /* + * If the APQN is not bound to the vfio_ap device + * driver, then we can't assign it to the guest's + * AP configuration. The AP architecture won't + * allow filtering of a single APQN, so let's filter + * the APID since an adapter represents a physical + * hardware device. + */ + apqn = AP_MKQID(apid, apqi); + + if (!vfio_ap_mdev_get_queue(matrix_mdev, apqn)) { + clear_bit_inv(apid, + matrix_mdev->shadow_apcb.apm); + break; + } + } + } +} + static int vfio_ap_mdev_probe(struct mdev_device *mdev) { struct ap_matrix_mdev *matrix_mdev; @@ -672,6 +729,8 @@ static ssize_t assign_adapter_store(struct device *dev, { int ret; unsigned long apid; + DECLARE_BITMAP(apm, AP_DEVICES); + struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); mutex_lock(&matrix_dev->lock); @@ -701,12 +760,15 @@ static ssize_t assign_adapter_store(struct device *dev, goto done; set_bit_inv(apid, matrix_mdev->matrix.apm); + memset(apm, 0, sizeof(apm)); + set_bit_inv(apid, apm); ret = vfio_ap_mdev_verify_no_sharing(matrix_mdev); if (ret) goto share_err; vfio_ap_mdev_link_adapter(matrix_mdev, apid); + vfio_ap_mdev_filter_matrix(apm, matrix_mdev->matrix.aqm, matrix_mdev); ret = count; goto done; @@ -775,6 +837,10 @@ static ssize_t unassign_adapter_store(struct device *dev, clear_bit_inv((unsigned long)apid, matrix_mdev->matrix.apm); vfio_ap_mdev_unlink_adapter(matrix_mdev, apid); + + if (test_bit_inv(apid, matrix_mdev->shadow_apcb.apm)) + clear_bit_inv(apid, matrix_mdev->shadow_apcb.apm); + ret = count; done: mutex_unlock(&matrix_dev->lock); @@ -848,6 +914,7 @@ static ssize_t assign_domain_store(struct device *dev, { int ret; unsigned long apqi; + DECLARE_BITMAP(aqm, AP_DOMAINS); struct ap_matrix_mdev *matrix_mdev = dev_get_drvdata(dev); unsigned long max_apqi = matrix_mdev->matrix.aqm_max; @@ -872,12 +939,15 @@ static ssize_t assign_domain_store(struct device *dev, goto done; set_bit_inv(apqi, matrix_mdev->matrix.aqm); + memset(aqm, 0, sizeof(aqm)); + set_bit_inv(apqi, aqm); ret = vfio_ap_mdev_verify_no_sharing(matrix_mdev); if (ret) goto share_err; vfio_ap_mdev_link_domain(matrix_mdev, apqi); + vfio_ap_mdev_filter_matrix(matrix_mdev->matrix.apm, aqm, matrix_mdev); ret = count; goto done; @@ -946,6 +1016,10 @@ static ssize_t unassign_domain_store(struct device *dev, clear_bit_inv((unsigned long)apqi, matrix_mdev->matrix.aqm); vfio_ap_mdev_unlink_domain(matrix_mdev, apqi); + + if (test_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm)) + clear_bit_inv(apqi, matrix_mdev->shadow_apcb.aqm); + ret = count; done: @@ -999,6 +1073,7 @@ static ssize_t assign_control_domain_store(struct device *dev, * number of control domains that can be assigned. */ set_bit_inv(id, matrix_mdev->matrix.adm); + vfio_ap_mdev_filter_cdoms(matrix_mdev); ret = count; done: mutex_unlock(&matrix_dev->lock); @@ -1046,6 +1121,10 @@ static ssize_t unassign_control_domain_store(struct device *dev, } clear_bit_inv(domid, matrix_mdev->matrix.adm); + + if (test_bit_inv(domid, matrix_mdev->shadow_apcb.adm)) + clear_bit_inv(domid, matrix_mdev->shadow_apcb.adm); + ret = count; done: mutex_unlock(&matrix_dev->lock); @@ -1186,8 +1265,6 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev, kvm_get_kvm(kvm); matrix_mdev->kvm = kvm; - memcpy(&matrix_mdev->shadow_apcb, &matrix_mdev->matrix, - sizeof(struct ap_matrix)); kvm_arch_crypto_set_masks(kvm, matrix_mdev->shadow_apcb.apm, matrix_mdev->shadow_apcb.aqm, matrix_mdev->shadow_apcb.adm); @@ -1528,6 +1605,7 @@ static void vfio_ap_queue_link_mdev(struct vfio_ap_queue *q) int vfio_ap_mdev_probe_queue(struct ap_device *apdev) { struct vfio_ap_queue *q; + DECLARE_BITMAP(apm, AP_DEVICES); q = kzalloc(sizeof(*q), GFP_KERNEL); if (!q) @@ -1537,6 +1615,12 @@ int vfio_ap_mdev_probe_queue(struct ap_device *apdev) q->apqn = to_ap_queue(&apdev->device)->qid; q->saved_isc = VFIO_AP_ISC_INVALID; vfio_ap_queue_link_mdev(q); + if (q->matrix_mdev) { + memset(apm, 0, sizeof(apm)); + set_bit_inv(AP_QID_CARD(q->apqn), apm); + vfio_ap_mdev_filter_matrix(apm, q->matrix_mdev->matrix.aqm, + q->matrix_mdev); + } dev_set_drvdata(&apdev->device, q); mutex_unlock(&matrix_dev->lock); @@ -1545,14 +1629,20 @@ int vfio_ap_mdev_probe_queue(struct ap_device *apdev) void vfio_ap_mdev_remove_queue(struct ap_device *apdev) { + unsigned long apid; struct vfio_ap_queue *q; mutex_lock(&matrix_dev->lock); q = dev_get_drvdata(&apdev->device); - if (q->matrix_mdev) + if (q->matrix_mdev) { vfio_ap_unlink_queue_fr_mdev(q); + apid = AP_QID_CARD(q->apqn); + if (test_bit_inv(apid, q->matrix_mdev->shadow_apcb.apm)) + clear_bit_inv(apid, q->matrix_mdev->shadow_apcb.apm); + } + vfio_ap_mdev_reset_queue(q, 1); dev_set_drvdata(&apdev->device, NULL); kfree(q);
Refresh the guest's APCB by filtering the APQNs assigned to the matrix mdev that do not reference an AP queue device bound to the vfio_ap device driver. The mdev's APQNs will be filtered according to the following rules: * The APID of each adapter and the APQI of each domain that is not in the host's AP configuration is filtered out. * The APID of each adapter comprising an APQN that does not reference a queue device bound to the vfio_ap device driver is filtered. The APQNs are derived from the Cartesian product of the APID of each adapter and APQI of each domain assigned to the mdev. The control domains that are not assigned to the host's AP configuration will also be filtered before assigning them to the guest's APCB. Signed-off-by: Tony Krowiak <akrowiak@linux.ibm.com> --- drivers/s390/crypto/vfio_ap_ops.c | 96 ++++++++++++++++++++++++++++++- 1 file changed, 93 insertions(+), 3 deletions(-)