lkml.org 
[lkml]   [2021]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC v2 16/32] x86/tdx: Handle MWAIT, MONITOR and WBINVD
    From
    Date
    >> To prevent TD guest from using MWAIT/MONITOR instructions,
    >> support for these instructions are already disabled by TDX
    >> module (SEAM). So CPUID flags for these instructions should
    >> be in disabled state.
    > Why does this not result in a #UD if the instruction is disabled by
    > SEAM?

    It's just the TDX module (SEAM is the execution mode used by the TDX module)


    > How is it possible to execute a disabled instruction (one
    > precluded by CPUID) to the point where it triggers #VE instead of #UD?

    That's how the TDX module works. It never injects anything else other
    than #VE. You can still get other exceptions of course, but they won't
    come from the TDX module.

    >> After the above mentioned preventive measures, if TD guests still
    >> execute these instructions, add appropriate warning messages in #VE
    >> handler. For WBIND instruction, since it's related to memory writeback
    >> and cache flushes, it's mainly used in context of IO devices. Since
    >> TDX 1.0 does not support non-virtual I/O devices, skipping it should
    >> not cause any fatal issues.
    > WBINVD is in a different class than MWAIT/MONITOR since it is not
    > identified by CPUID, it can't possibly have the same #UD behaviour.
    > It's not clear why WBINVD is included in the same patch as
    > MWAIT/MONITOR?

    Because these are all instructions we never expect to execute, so
    nothing special is needed for them. That's a unique class that logically
    fits together.


    >
    > I disagree with the assertion that WBINVD is mainly used in the
    > context of I/O devices, it's also used for ACPI power management
    > paths.

    You mean S3? That's of course also not supported inside TDX.


    > WBINVD dependent functionality should be dynamically disabled
    > rather than warned about.
    >
    > Does a TDX guest support out-of-tree modules? The kernel is already
    > tainted when out-of-tree modules are loaded. In other words in-tree
    > modules preclude forbidden instructions because they can just be
    > audited, and out-of-tree modules are ok to trigger abrupt failure if
    > they attempt to use forbidden instructions.

    We already did a lot of bi^wdiscussion on this on the last review.

    Originally we had a different handling, this was the result of previous
    feedback.

    It doesn't really matter because it should never happen.


    >
    >> But to let users know about its usage, use
    >> WARN() to report about it.. For MWAIT/MONITOR instruction, since its
    >> unsupported use WARN() to report unsupported usage.
    > I'm not sure how useful warning is outside of a kernel developer's
    > debug environment. The kernel should know what instructions are
    > disabled and which are available. WBINVD in particular has potential
    > data integrity implications. Code that might lead to a WBINVD usage
    > should be disabled, not run all the way up to where WBINVD is
    > attempted and then trigger an after-the-fact WARN_ONCE().

    We don't expect the warning to ever happen. Yes all of this will be
    disabled. Nearly all are in code paths that cannot happen inside TDX
    anyways due to missing PCI-IDs or different cpuids, and S3 is explicitly
    disabled and would be impossible anyways due to lack of BIOS support.




    >
    > The WBINVD change deserves to be split off from MWAIT/MONITOR, and
    > more thought needs to be put into where these spurious instruction
    > usages are arising.

    I disagree. We already spent a lot of cycles on this. WBINVD makes never
    sense in current TDX and all the code will be disabled.


    -Andi

    \
     
     \ /
      Last update: 2021-05-11 04:18    [W:4.443 / U:1.908 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site