Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <973add45-9fd2-7abc-3a97-96a26c263ea0@linux.intel.com>
 <20210609194926.1949859-1-sathyanarayanan.kuppuswamy@linux.intel.com> <7c06b567-9a65-8c7c-6046-3dcb32d4bb15@intel.com>
In-Reply-To: <7c06b567-9a65-8c7c-6046-3dcb32d4bb15@intel.com>
From:   Dan Williams <dan.j.williams@intel.com>
Date:   Wed, 9 Jun 2021 14:38:26 -0700
Message-ID: <CAPcyv4ismoYra_8=Qj=XLLs9+18tRiv8Y48GJWOAHvpzi3BZiw@mail.gmail.com>
Subject: Re: [RFC v2-fix-v5 1/1] x86: Skip WBINVD instruction for VM guest
To:     Dave Hansen <dave.hansen@intel.com>
Cc:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Tony Luck <tony.luck@intel.com>,
        Andi Kleen <ak@linux.intel.com>,
        Kirill Shutemov <kirill.shutemov@linux.intel.com>,
        Kuppuswamy Sathyanarayanan <knsathya@kernel.org>,
        Raj Ashok <ashok.raj@intel.com>,
        Sean Christopherson <seanjc@google.com>,
        Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Jun 9, 2021 at 2:03 PM Dave Hansen <dave.hansen@intel.com> wrote:
>
> This changelog lacks both clear problem statements and a clear solution
> implemented within the patch.
>
> Here's a proposed changelog.  It clearly spells out the two problems
> caused by WBINVD within a guest, and the proposed solution which fixes
> those two problems.

Looks good to me modulo the comment below...

>
> Is this missing anything?
>
> --
>
> VM guests that support ACPI use standard ACPI mechanisms to signal sleep
> state entry to the host.  To ACPI, reboot is simply another sleep state.
>
> ACPI specifies that the platform preserve memory contents over (some)
> sleep states.  It does not specify any requirements for data
> preservation in CPU caches.  The ACPI specification mandates the use of
> WBINVD to flush the contents of the CPU caches to memory before entering
> specific sleep states, thus ensuring data in caches can survive sleep
> state transitions.e
>
> Unlike when entering sleep states bare metal, no actions within a guest
> can cause data in processor caches to be lost.  That makes these WBINVD
> invocations harmless but superfluous within a guest. (<--- problem #1)
>
> In TDX guests, these WBINVD operations cause #VE exceptions.  For debug,
> it would be ideal for the #VE handler to be able to WARN() when an
> unexpected WBINVD occurs. (<--- problem #2)

...but it doesn't WARN() it triggers unhandled #VE, unless I missed
another patch that precedes this that turns it into a WARN()? If a
code path expects WBINVD for correct operation and the guest can't
execute that sounds fatal, not a WARN to me.

> Avoid WBINVD for all ACPI cache-flushing operations which occur while
> running under a hypervisor, which includes TDX guests.  This both avoids
> TDX warnings and optimizes away superfluous WBINVD invocations. (<----
> solution)
>