On 4/9/20 11:06 AM, Cornelia Huck wrote:
On Tue, 7 Apr 2020 15:20:02 -0400
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
A vfio_ap_queue structure is created for each queue device probed. To
ensure that the matrix mdev to which a queue's APQN is assigned is linked
to the queue structure as long as the queue device is bound to the vfio_ap
device driver, let's go ahead and manage these links when the queue device
is probed and removed as well as whenever an adapter or domain is assigned
to or unassigned from the matrix mdev.
Signed-off-by: Tony Krowiak <akrowiak@xxxxxxxxxxxxx>
---
drivers/s390/crypto/vfio_ap_ops.c | 75 ++++++++++++++++++++++++++++---
1 file changed, 70 insertions(+), 5 deletions(-)
(...)
@@ -536,6 +531,31 @@ static int vfio_ap_mdev_verify_no_sharing(struct ap_matrix_mdev *matrix_mdev)
return 0;
}
+/**
+ * vfio_ap_mdev_qlinks_for_apid
Hm... maybe the function name should express that there's some actual
(un)linking going on?
vfio_ap_mdev_link_by_apid?
Or make this vfio_ap_mdev_link_queues() and pass in an indicator whether
the passed value is an apid or an aqid? Both function names look so
very similar to be easily confused (at least to me).
Forget my last response, I like your function names better.
+ *
+ * @matrix_mdev: a matrix mediated device
+ * @apqi: the APID of one or more APQNs assigned to @matrix_mdev
+ *
+ * Set the link to @matrix_mdev for each queue device bound to the vfio_ap
+ * device driver with an APQN assigned to @matrix_mdev with the specified @apid.
+ *
+ * Note: If @matrix_mdev is NULL, the link to @matrix_mdev will be severed.
+ */
+static void vfio_ap_mdev_qlinks_for_apid(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long apid)
+{
+ unsigned long apqi;
+ struct vfio_ap_queue *q;
+
+ for_each_set_bit_inv(apqi, matrix_mdev->matrix.aqm,
+ matrix_mdev->matrix.aqm_max + 1) {
+ q = vfio_ap_get_queue(AP_MKQID(apid, apqi));
+ if (q)
+ q->matrix_mdev = matrix_mdev;
+ }
+}
+
/**
* assign_adapter_store
*
(...)
@@ -682,6 +704,31 @@ vfio_ap_mdev_verify_queues_reserved_for_apqi(struct ap_matrix_mdev *matrix_mdev,
return 0;
}
+/**
+ * vfio_ap_mdev_qlinks_for_apqi
See my comment above.
+ *
+ * @matrix_mdev: a matrix mediated device
+ * @apqi: the APQI of one or more APQNs assigned to @matrix_mdev
+ *
+ * Set the link to @matrix_mdev for each queue device bound to the vfio_ap
+ * device driver with an APQN assigned to @matrix_mdev with the specified @apqi.
+ *
+ * Note: If @matrix_mdev is NULL, the link to @matrix_mdev will be severed.
+ */
+static void vfio_ap_mdev_qlinks_for_apqi(struct ap_matrix_mdev *matrix_mdev,
+ unsigned long apqi)
+{
+ unsigned long apid;
+ struct vfio_ap_queue *q;
+
+ for_each_set_bit_inv(apid, matrix_mdev->matrix.apm,
+ matrix_mdev->matrix.apm_max + 1) {
+ q = vfio_ap_get_queue(AP_MKQID(apid, apqi));
+ if (q)
+ q->matrix_mdev = matrix_mdev;
+ }
+}
+
/**
* assign_domain_store
*
(...)
@@ -1270,6 +1319,21 @@ void vfio_ap_mdev_unregister(void)
mdev_unregister_device(&matrix_dev->device);
}
+static void vfio_ap_mdev_for_queue(struct vfio_ap_queue *q)
vfio_ap_queue_link_mdev()? It is the other direction from the linking
above.
See my comment above.
+{
+ unsigned long apid = AP_QID_CARD(q->apqn);
+ unsigned long apqi = AP_QID_QUEUE(q->apqn);
+ struct ap_matrix_mdev *matrix_mdev;
+
+ list_for_each_entry(matrix_mdev, &matrix_dev->mdev_list, node) {
+ if (test_bit_inv(apid, matrix_mdev->matrix.apm) &&
+ test_bit_inv(apqi, matrix_mdev->matrix.aqm)) {
+ q->matrix_mdev = matrix_mdev;
+ break;
+ }
+ }
+}
+
int vfio_ap_mdev_probe_queue(struct ap_queue *queue)
{
struct vfio_ap_queue *q;
@@ -1282,6 +1346,7 @@ int vfio_ap_mdev_probe_queue(struct ap_queue *queue)
dev_set_drvdata(&queue->ap_dev.device, q);
q->apqn = queue->qid;
q->saved_isc = VFIO_AP_ISC_INVALID;
+ vfio_ap_mdev_for_queue(q);
hash_add(matrix_dev->qtable, &q->qnode, q->apqn);
mutex_unlock(&matrix_dev->lock);
In general, looks sane.
