On 5/23/21 6:57 PM, Jason Gunthorpe wrote:
On Fri, May 21, 2021 at 03:36:48PM -0400, Tony Krowiak wrote:+static struct kvm_s390_crypto_hook +*kvm_arch_crypto_find_hook(enum kvm_s390_crypto_hook_type type) +{ + struct kvm_s390_crypto_hook *crypto_hook; + + list_for_each_entry(crypto_hook, &crypto_hooks, node) { + if (crypto_hook->type == type) + return crypto_hook; + } + + return NULL; +} + +int kvm_arch_crypto_register_hook(struct kvm_s390_crypto_hook *hook) +{ + struct kvm_s390_crypto_hook *crypto_hook; + + mutex_lock(&crypto_hooks_lock); + crypto_hook = kvm_arch_crypto_find_hook(hook->type); + if (crypto_hook) { + if (crypto_hook->owner != hook->owner) + return -EACCES; + list_replace(&crypto_hook->node, &hook->node);This is all dead code right? This is only called from a module init function so it can't be called twice.
That is true only if you are considering the current case. Is it guaranteed that only the vfio_ap module will call this function and is there a guarantee that it will always and only be called from the vfio_ap module init function? For example, suppose a hook is added to intercept the AP NQAP or DQAP instruction? We don't know how or when those handlers will be registered or unregistered. We don't even know if the handlers will be part of the vfio_ap module or not.
Just always fail if the hook is already used and delete the owner stuff.
I suppose that is reasonable and simpler.
But this is alot of complicated and unused code to solve a lock ordering problem..
If you have a better solution, I'm all ears. I've been down this road a couple of times now and solving lock ordering for multiple asynchronous processes is not trivial. This seems like a reasonable solution and provides for flexibility for including additional hooks to handle interception of other AP instructions.
Jason
