Re: [[RESEND] iommufd PATCH v2 2/2] vfio/ap: validate iova during dma_unmap and trigger irq disable

Anthony Krowiak
2022-11-29T22:02:41Z

LGTM:

Reviewed-by: Tony Krowiak <akrowiak@linux.ibm.com>

On 11/29/22 5:58 AM, Yi Liu wrote:
> From: Matthew Rosato <mjrosato@linux.ibm.com>
>
> Currently, each mapped iova is stashed in its associated vfio_ap_queue;
> when we get an unmap request, validate that it matches with one or more
> of these stashed values before attempting unpins.
>
> Each stashed iova represents IRQ that was enabled for a queue. Therefore,
> if a match is found, trigger IRQ disable for this queue to ensure that
> underlying firmware will no longer try to use the associated pfn after
> the page is unpinned. IRQ disable will also handle the associated unpin.
>
> Cc: Tony Krowiak <akrowiak@linux.ibm.com>
> Cc: Halil Pasic <pasic@linux.ibm.com>
> Cc: Jason Herne <jjherne@linux.ibm.com>
> Cc: linux-s390@vger.kernel.org
> Signed-off-by: Matthew Rosato <mjrosato@linux.ibm.com>
> Signed-off-by: Yi Liu <yi.l.liu@intel.com>
> ---
> drivers/s390/crypto/vfio_ap_ops.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/s390/crypto/vfio_ap_ops.c b/drivers/s390/crypto/vfio_ap_ops.c
> index 0b4cc8c597ae..8bf353d46820 100644
> --- a/drivers/s390/crypto/vfio_ap_ops.c
> +++ b/drivers/s390/crypto/vfio_ap_ops.c
> @@ -1535,13 +1535,29 @@ static int vfio_ap_mdev_set_kvm(struct ap_matrix_mdev *matrix_mdev,
> return 0;
> }
>
> +static void unmap_iova(struct ap_matrix_mdev *matrix_mdev, u64 iova, u64 length)
> +{
> + struct ap_queue_table *qtable = &matrix_mdev->qtable;
> + struct vfio_ap_queue *q;
> + int loop_cursor;
> +
> + hash_for_each(qtable->queues, loop_cursor, q, mdev_qnode) {
> + if (q->saved_iova >= iova && q->saved_iova < iova + length)
> + vfio_ap_irq_disable(q);
> + }
> +}
> +
> static void vfio_ap_mdev_dma_unmap(struct vfio_device *vdev, u64 iova,
> u64 length)
> {
> struct ap_matrix_mdev *matrix_mdev =
> container_of(vdev, struct ap_matrix_mdev, vdev);
>
> - vfio_unpin_pages(&matrix_mdev->vdev, iova, 1);
> + mutex_lock(&matrix_dev->mdevs_lock);
> +
> + unmap_iova(matrix_mdev, iova, length);
> +
> + mutex_unlock(&matrix_dev->mdevs_lock);
> }
>
> /**