lkml.org 
[lkml]   [2021]   [Jun]   [8]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC v2-fix-v3 1/1] x86/tdx: Skip WBINVD instruction for TDX guest
    From
    Date
    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.

    \
     
     \ /
      Last update: 2021-06-09 00:54    [W:3.469 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site