Messages in this thread Patch in this message | | | From | Dan Williams <> | Date | Mon, 24 May 2021 18:45:30 -0700 | Subject | Re: [RFC v2-fix-v2 2/2] x86/tdx: Ignore WBINVD instruction for TDX guest |
| |
On Mon, May 24, 2021 at 6:02 PM Andi Kleen <ak@linux.intel.com> wrote: > > > > That makes KVM also broken for the cases where wbinvd is needed, > > > Or maybe your analysis is wrong?
I'm well aware of the fact that wbinvd is problematic for hypervisors and is an attack vector for a guest to DOS the host.
> > > > but > > it does not make the description of this patch correct. > > If KVM was broken I'm sure we would hear about it.
KVM does not try to support the cases where wbinvd being unavailable would break the system. That is not the claim being made in this patch.
> The ACPI cases are for S3, which is not supported in guests, or for the > old style manual IO port C6, which isn't supported either.
> The persistent memory cases would require working DMA mappings,
No, that analysis is wrong.The wbinvd audit would have found that persistent memory secure-erase and unlock, which has nothing to do with DMA, needs wbinvd to ensure that the CPU has not retained a copy of the PMEM contents from before the unlock happened and it needs to make sure that any data that was meant to be destroyed by an erasure is not retained in cache.
> which we > currently don't support. If DMA mappings were added we would need to > para virtualized WBINVD, like the comments say. > > AFAIK all the rest is for some caching attribute change, which is not > possible in KVM (because it uses EPT.IgnorePAT=1) nor in TDX (which does > the same). Some are for MTRR which is completely disabled if you're > running under EPT.
It's fine to not support the above cases, I am asking for the explanation to demonstrate the known risks and the known mitigations. IgnorePAT is not the mitigation, the mitigation is an audit to describe why the known users are unlikely to be triggered. Even better would be an addition patch that does something like:
iff --git a/drivers/nvdimm/security.c b/drivers/nvdimm/security.c index 4b80150e4afa..a6b13a1ae319 100644 --- a/drivers/nvdimm/security.c +++ b/drivers/nvdimm/security.c @@ -170,6 +170,9 @@ static int __nvdimm_security_unlock(struct nvdimm *nvdimm) const void *data; int rc;
+ if (is_protected_guest()) + return -ENXIO; + /* The bus lock should be held at the top level of the call stack */ lockdep_assert_held(&nvdimm_bus->reconfig_mutex);
...to explicitly error out a wbinvd use case before data is altered and wbinvd is needed.
| |