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;
User-Agent: Cyrus-JMAP/3.5.0-alpha0-519-g27a961944e-fm-20210531.001-g27a96194
Mime-Version: 1.0
Message-Id: <e9b2aab2-4a6f-4739-a939-c448414e6af2@www.fastmail.com>
In-Reply-To: <4e5ac34f-28cb-def6-0b87-e560fa42e5e5@linux.intel.com>
References: <CAPcyv4iAgXnMmg+Z1cqrgeQUcuQgXZ1WCtAaNmeHuLT_5QArUw@mail.gmail.com>
 <20210609011030.751451-1-sathyanarayanan.kuppuswamy@linux.intel.com>
 <CAPcyv4gLeKPfYOx1kmg-mO1_mNd+XGqVO-CbqX+2d52GZ+DSFw@mail.gmail.com>
 <23418f34-7c03-7477-6fbf-1b36b4718cb9@kernel.org>
 <4e5ac34f-28cb-def6-0b87-e560fa42e5e5@linux.intel.com>
Date:   Tue, 08 Jun 2021 21:32:26 -0700
From:   "Andy Lutomirski" <luto@kernel.org>
To:     "Andi Kleen" <ak@linux.intel.com>,
        "Williams, Dan J" <dan.j.williams@intel.com>,
        "Sathyanarayanan Kuppuswamy" 
        <sathyanarayanan.kuppuswamy@linux.intel.com>
Cc:     "Peter Zijlstra (Intel)" <peterz@infradead.org>,
        "Dave Hansen" <dave.hansen@intel.com>,
        "Tony Luck" <tony.luck@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>
Subject: =?UTF-8?Q?Re:_[RFC_v2-fix-v4_1/1]_x86/tdx:_Skip_WBINVD_instruction_for_T?=
 =?UTF-8?Q?DX_guest?=
Content-Type: text/plain
Precedence: bulk

On Tue, Jun 8, 2021, at 9:25 PM, Andi Kleen wrote:
> 
> > I like this description, but shouldn't the logic be:
> >
> > if (!CPUID has hypervisor bit set)
> >    wbinvd();
> >
> > As far as I know, most hypervisors will turn WBINVD into a noop and,
> > even if they don't, it seems to be that something must be really quite
> > wrong for a guest to need to WBINVD for ACPI purposes.
> 
> KVM only turns it into a noop if there is no VT-d, because with VT-d you 
> might need it to turn mappings into uncached and vice versa.

Wow, I found the kvm_arch_register_noncoherent_dma() stuff.  That's horrifying.  What's it for?  Are there actually guests that use devices exposed by VFIO that expect WBINVD to work?  That's a giant DoS hole.

> 
> But yes the change would make sense for reboot. BTW I suspect for the 
> reboot path it isn't really needed anywhere modern, so it might actually 
> be ok to completely disable it. But that's some risk, so doing it only 
> for hypervisor is reasonable.

I agree.