On 12/4/20 2:05 PM, Halil Pasic wrote:
On Fri, 4 Dec 2020 09:43:59 -0500
Tony Krowiak <akrowiak@xxxxxxxxxxxxx> wrote:
+{
+ if (matrix_mdev->kvm) {
+ (matrix_mdev->kvm);
+ matrix_mdev->kvm->arch.crypto.pqap_hook = NULL;
Is a plain assignment to arch.crypto.pqap_hook apropriate, or do we need
to take more care?
For instance kvm_arch_crypto_set_masks() takes kvm->lock before poking
kvm->arch.crypto.crycb.
I do not think so. The CRYCB is used by KVM to provide crypto resources
to the guest so it makes sense to protect it from changes to it while
passing
the AP devices through to the guest. The hook is used only when an AQIC
executed on the guest is intercepted by KVM. If the notifier
is being invoked to notify vfio_ap that KVM has been set to NULL, this means
the guest is gone in which case there will be no AP instructions to
intercept.
If the update to pqap_hook isn't observed as atomic we still have a
problem. With torn writes or reads we would try to use a corrupt function
pointer. While the compiler probably ain't likely to generate silly code
for the above assignment (multiple write instructions less then
quadword wide), I know of nothing that would prohibit the compiler to do
so.
I'm sorry, but I still don't understand why you think this is a problem
given what I stated above.
I'm not certain about the scope of the kvm->lock (if it's supposed to
protect the whole sub-tree of objects). Maybe Janosch can help us out.
@Janosch: what do you think?
Regards,
Halil
