On Tue, May 25, 2021 at 09:16:30AM -0400, Tony Krowiak wrote:
Why can't you put the locks in the right order? It looked trivial, I'm confused.
On 5/24/21 10:37 AM, Jason J. Herne wrote:
On 5/21/21 3:36 PM, Tony Krowiak wrote:That would be great, however; when I implemented something similar, it
The function pointer to the handler that processes interception of theTypo: You meant kvm_arch_crypto_UNregister_hook() in the second bullet.
PQAP instruction is contained in the mdev. If the mdev is removed and
its storage de-allocated during the processing of the PQAP instruction,
the function pointer could get wiped out before the function is called
because there is currently nothing that controls access to it.
This patch introduces two new functions:
* The kvm_arch_crypto_register_hook() function registers a function
for processing intercepted crypto instructions.
* The kvm_arch_crypto_register_hook() function un-registers a function
pointer that was previously registered.
Just one overall observation on this one. The whole hook system seems
kind of over-engineered if this is our only use for it. It looks like a
kvm_s390_crypto_hook is meant to link a specific module with a function
pointer. Do we really need this concept?
I think a simpler design could be to just place a mutex and a function
pointer in the kvm_s390_crypto struct. Then you can grab the mutex in
vfio_ap_ops.c when registering/unregistering. You would also grab the
mutex in priv.c when calling the function pointer. What I am suggesting
is essentially the exact same scheme you have implemented here, but
simpler and with less infrastructure.
resulted in a
lockdep splat between the lock used to protect the hook and the
matrix_dev->lock used to
protect updates to matrix_mdev (including the freeing thereof). After
pulling what little hair
I have left out, this seemed like a reasonable solution, over-engineered
though it may be.
If somebody has a simpler solution, I'm all ears.