On Mon,  4 Jul 2022 21:59:03 -0300
Jason Gunthorpe <j...@nvidia.com> wrote:
> diff --git a/drivers/s390/cio/vfio_ccw_ops.c b/drivers/s390/cio/vfio_ccw_ops.c
> index b49e2e9db2dc6f..09e0ce7b72324c 100644
> --- a/drivers/s390/cio/vfio_ccw_ops.c
> +++ b/drivers/s390/cio/vfio_ccw_ops.c
> @@ -44,31 +44,19 @@ static int vfio_ccw_mdev_reset(struct vfio_ccw_private 
> *private)
>       return ret;
>  }
>  
> -static int vfio_ccw_mdev_notifier(struct notifier_block *nb,
> -                               unsigned long action,
> -                               void *data)
> +static void vfio_ccw_dma_unmap(struct vfio_device *vdev, u64 iova, u64 
> length)
>  {
>       struct vfio_ccw_private *private =
> -             container_of(nb, struct vfio_ccw_private, nb);
> -
> -     /*
> -      * Vendor drivers MUST unpin pages in response to an
> -      * invalidation.
> -      */
> -     if (action == VFIO_IOMMU_NOTIFY_DMA_UNMAP) {
> -             struct vfio_iommu_type1_dma_unmap *unmap = data;
> -
> -             if (!cp_iova_pinned(&private->cp, unmap->iova))
> -                     return NOTIFY_OK;
> +             container_of(vdev, struct vfio_ccw_private, vdev);
>  
> -             if (vfio_ccw_mdev_reset(private))
> -                     return NOTIFY_BAD;
> +     /* Drivers MUST unpin pages in response to an invalidation. */
> +     if (!cp_iova_pinned(&private->cp, iova))
> +             return;
>  
> -             cp_free(&private->cp);
> -             return NOTIFY_OK;
> -     }
> +     if (vfio_ccw_mdev_reset(private))
> +             return;
>  
> -     return NOTIFY_DONE;
> +     cp_free(&private->cp);
>  }


The cp_free() call is gone here with [1], so I think this function now
just ends with:

        ...
        vfio_ccw_mdev_reset(private);
}

There are also minor contextual differences elsewhere from that series,
so a quick respin to record the changes on list would be appreciated.

However the above kind of highlights that NOTIFY_BAD that silently gets
dropped here.  I realize we weren't testing the return value of the
notifier call chain and really we didn't intend that notifiers could
return a failure here, but does this warrant some logging or suggest
future work to allow a device to go offline here?  Thanks.

Alex

[1]https://lore.kernel.org/all/20220707135737.720765-1-far...@linux.ibm.com/

Reply via email to