Re: [PATCH v3 2/2] s390/vfio-ap: control access to PQAP(AQIC) interception handler

From: Jason Gunthorpe
Date: Fri May 21 2021 - 14:30:37 EST


On Fri, May 21, 2021 at 02:24:33PM -0400, Tony Krowiak wrote:
>
> > The simple solution in sketch is just this:
>
> The code below results in a lockdep WARN_ON for the
> reasons documented in my comments interspersed in
> the code.

Sure, to be expected for a 2 min effort, but you seem to have solved
it by trivially putting things in the right locking order?

> After trying several different permutations using RCU and
> an rw_semaphore, I came to the conclusion that setting and
> clearing the hook pointer while the mdev fd is being open
> and closed or when the mdev is being removed unnecessarily
> complicates things. There is no good reason to set/clear the
> function pointer at this time, nor is there any compelling
> reason to store the function pointer in a satellite structure
> of the kvm struct. Since the hook function's lifespan coincides
> with the lifespan of the vfio_ap module, why not store it
> when the module is loaded and clear it when the module is
> unloaded?

Well, the hook function isn't really the problem..

> Access to the function pointer can be controlled by a lock
> that is independent of the matrix_dev->lock, thus avoiding
> potential lock dependencies. Access to the mdev is controlled by
> the matrix_dev lock, so if the mdev is retrieved from the
> matrix_dev->mdev_list in the hook function, then we are assured
> that the mdev will never be accessed after it is freed; problem solved.

This just transforms the problem into needing to hold a lock around
mdev_list while accessing a member of the mdev_list

Is it simpler?

Jason