lkml.org 
[lkml]   [2020]   [Apr]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH v7 02/15] s390/vfio-ap: manage link between queue struct and matrix mdev
    From
    Date


    On 4/9/20 11:06 AM, Cornelia Huck wrote:
    > On Tue, 7 Apr 2020 15:20:02 -0400
    > Tony Krowiak <akrowiak@linux.ibm.com> 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@linux.ibm.com>
    >> ---
    >> 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.
    >

    \
     
     \ /
      Last update: 2020-04-10 17:42    [W:4.181 / U:0.016 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site