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: <CAPcyv4iAgXnMmg+Z1cqrgeQUcuQgXZ1WCtAaNmeHuLT_5QArUw@mail.gmail.com>
 <20210609011030.751451-1-sathyanarayanan.kuppuswamy@linux.intel.com>
In-Reply-To: <20210609011030.751451-1-sathyanarayanan.kuppuswamy@linux.intel.com>
From:   Dan Williams <dan.j.williams@intel.com>
Date:   Tue, 8 Jun 2021 20:40:42 -0700
Message-ID: <CAPcyv4gLeKPfYOx1kmg-mO1_mNd+XGqVO-CbqX+2d52GZ+DSFw@mail.gmail.com>
Subject: Re: [RFC v2-fix-v4 1/1] x86/tdx: Skip WBINVD instruction for TDX guest
To:     Kuppuswamy Sathyanarayanan 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc:     Peter Zijlstra <peterz@infradead.org>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@intel.com>,
        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 Tue, Jun 8, 2021 at 6:10 PM Kuppuswamy Sathyanarayanan
<sathyanarayanan.kuppuswamy@linux.intel.com> wrote:
>
> Current TDX spec does not have support to emulate the WBINVD
> instruction. If any feature that uses WBINVD is enabled/used
> in TDX guest, it will lead to un-handled #VE exception, which
> will be handled as #GP fault.
>
> ACPI drivers also uses WBINVD instruction for cache flushes in
> reboot or shutdown code path. Since TDX guest has requirement
> to support shutdown feature, skip WBINVD instruction usage
> in ACPI drivers for TDX guest.

This sounds awkward...

> Since cache is always coherent in TDX guests, making wbinvd as

This is incorrect, ACPI cache flushing is not about I/O or CPU coherency...

> noop should not cause any issues in above mentioned code path.

..."should" is a famous last word...

> The end-behavior is the same as KVM guest (treat as noops).

..."KVM gets away with it" is not a justification that TDX can stand
on otherwise we would not be here fixing up ACPICA properly.

How about:

"TDX guests use standard ACPI mechanisms to signal sleep state entry
(including reboot) to the host. The ACPI specification mandates WBINVD
on any sleep state entry with the expectation that the platform is
only responsible for maintaining the state of memory over sleep
states, not preserving dirty data in any CPU caches. ACPI cache
flushing requirements pre-date the advent of virtualization. Given TDX
guest sleep state entry does not affect any host power rails it is not
required to flush caches. The host is responsible for maintaining
cache state over its own bare metal sleep state transitions that
power-off the cache. If the host fails to manage caches over its sleep
state transitions the guest..."

I don't know how to finish the last sentence. What does TDX do if it
is resumed after host suspend and the host somehow arranged for dirty
TDX lines to be lost. Will that be noticed by TDX integrity
mechanisms? I did not immediately find an answer to this with a brief
look at the specs.

>
> In future, once TDX guest specification adds support for WBINVD
> hypercall, we can pass the handle to KVM to handle it.

I expect if the specification wanted operating systems to plan for
this eventuality it would have made a note of it. I expect this
sentence can just be deleted.