Messages in this thread | | | Subject | Re: [RFC v2-fix-v3 1/1] x86/tdx: Skip WBINVD instruction for TDX guest | From | Dave Hansen <> | Date | Tue, 8 Jun 2021 15:53:17 -0700 |
| |
On 6/8/21 3:36 PM, Kuppuswamy, Sathyanarayanan wrote: > On 6/8/21 3:17 PM, Dave Hansen wrote: >> On 6/8/21 2:35 PM, Kuppuswamy Sathyanarayanan wrote: >>> Persistent memory is also currently not supported. Another code >>> path that uses WBINVD is the MTRR driver, but EPT/virtualization >>> always disables MTRRs so those are not needed. This all implies >>> WBINVD is not needed with current TDX. >> >> It's one thing to declare something unsupported. It's quite another to >> declare it unsupported and then back it up with code to ensure that any >> attempted use is thwarted. > > Only audited and supported drivers will be allowed to enumerate after > device filter support patch is merged. Till we merge that patch, If > any of these unsupported features (with WBINVD usage) are enabled in TDX, > it will lead to sigfault (due to unhandled #VE).
A kernel driver using WBINVD will "sigfault"? I'm not sure what that means. How does the kernel "sigfault"?
> In this patch we only create exception for ACPI sleep driver code. If > commit log is confusing, I can remove information about other unsupported > feature (with WBINVD usage).
Yes, the changelog is horribly confusing. But simply removing this information is insufficient to rectify the deficiency.
I've lost trust that due diligence will be performed on this series on its own. I've seen too many broken promises and too many holes.
Here's what I want to see: a list of all of the unique call sites for WBINVD in the kernel. I want a written down methodology for how the list of call sites was generated. I want to see an item-by-item list of why those call sites are unreachable with the TDX guest code. It might be because they've been patched in this patch, or the driver has been disabled, or because the TDX architecture spec would somehow prohibit the situation where it might be needed. But, there needs to be a list, and you have to show your work. If you refer to code from this series as helping to prevent WBINVD, then it has to be earlier in this series, not in some other series and not later in this series.
Just eyeballing it, there are ~50 places in the kernel that need auditing.
Right now, we mostly have indiscriminate hand-waving about this not being a problem. It's a hard NAK from me on this patch until this audit is in place.
| |