Re: [PATCH] KVM: SVM: Issue WBINVD after deactivating an SEV guest

From: Tom Lendacky
Date: Sat Mar 21 2020 - 08:16:47 EST


On 3/21/20 4:00 AM, Greg KH wrote:
On Fri, Mar 20, 2020 at 03:37:23PM -0500, Tom Lendacky wrote:
On 3/20/20 3:34 PM, David Rientjes wrote:
On Fri, 20 Mar 2020, Tom Lendacky wrote:

Currently, CLFLUSH is used to flush SEV guest memory before the guest is
terminated (or a memory hotplug region is removed). However, CLFLUSH is
not enough to ensure that SEV guest tagged data is flushed from the cache.

With 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations"), the
original WBINVD was removed. This then exposed crashes at random times
because of a cache flush race with a page that had both a hypervisor and
a guest tag in the cache.

Restore the WBINVD when destroying an SEV guest and add a WBINVD to the
svm_unregister_enc_region() function to ensure hotplug memory is flushed
when removed. The DF_FLUSH can still be avoided at this point.

Fixes: 33af3a7ef9e6 ("KVM: SVM: Reduce WBINVD/DF_FLUSH invocations")
Signed-off-by: Tom Lendacky <thomas.lendacky@xxxxxxx>

Acked-by: David Rientjes <rientjes@xxxxxxxxxx>

Should this be marked for stable?

The Fixes tag should take care of that.

No it does not.
Please read:
https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fwww.kernel.org%2Fdoc%2Fhtml%2Flatest%2Fprocess%2Fstable-kernel-rules.html&amp;data=02%7C01%7Cthomas.lendacky%40amd.com%7C197f666080144732040108d7cd765107%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637203780365719535&amp;sdata=NKgNt6Hd7y6BGBdpI52ckCxZvIsCRuEf9FJ7GW2PqPw%3D&amp;reserved=0
for how to do this properly.

Yes, I have had to go around and clean up after maintainers who don't
seem to realize this, but for KVM patches I have been explicitly told to
NOT take any patch unless it has a cc: stable on it, due to issues that
have happened in the past.

So for this subsystem, what you suggested guaranteed it would NOT get
picked up, please do not do that.

Thanks for clarifying that, Greg.

Then, yes, it should have the Cc: to stable that David mentioned. If it
gets applied without that, I'll follow the process to send an email to
stable to get it included in 5.5-stable.

Thanks,
Tom


greg k-h