Received-SPF: pass (google.com: domain of linux-kernel+bounces-147749-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 16 Apr 2024 16:21:37 -0700
In-Reply-To: <af4c78c1-ccd5-4c4d-92de-564a44fa815b@intel.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240411144322.14585-1-xry111@xry111.site> <20240411144322.14585-2-xry111@xry111.site>
 <ZhgOPRIA5lyhTfGs@google.com> <af4c78c1-ccd5-4c4d-92de-564a44fa815b@intel.com>
Message-ID: <Zh8IAcT0fQoyes_1@google.com>
Subject: Re: [PATCH v7 2/2] x86/mm: Don't disable INVLPG if the kernel is
 running on a hypervisor
From: Sean Christopherson <seanjc@google.com>
To: Dave Hansen <dave.hansen@intel.com>
Cc: Xi Ruoyao <xry111@xry111.site>, Dave Hansen <dave.hansen@linux.intel.com>, 
	Michael Kelley <mhklinux@outlook.com>, Pawan Gupta <pawan.kumar.gupta@linux.intel.com>, 
	Andy Lutomirski <luto@kernel.org>, Peter Zijlstra <peterz@infradead.org>, 
	Thomas Gleixner <tglx@linutronix.de>, Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>, 
	"H. Peter Anvin" <hpa@zytor.com>, x86@kernel.org, linux-kernel@vger.kernel.org, 
	Andrew Cooper <andrew.cooper3@citrix.com>
Content-Type: text/plain; charset="us-ascii"

On Thu, Apr 11, 2024, Dave Hansen wrote:
> On 4/11/24 09:22, Sean Christopherson wrote:
> > In other words, simply checking HYPERVISOR *might* be safe, but it might not.
> > If we wanted to be paranoid, this could also check X86_FEATURE_VMX, which also
> > doesn't guarantee VMX non-root mode and would unnecessarily restrict PCID usage
> > to setups that allow nested VMX, but AFAIK there aren't any hypervisors which
> > fully emulate VMX.
> 
> X86_FEATURE_HYPERVISOR is most commonly used for vulnerabilities to see
> if the data coming out of CPUID is likely to be garbage or not.  I think
> that's the most important thing to focus on.
> 
> It's arguable that x86_match_cpu() itself should just have a:
> 
> 	/*
> 	 * Don't even waste our time when running under a hypervisor.
> 	 * They lie.
> 	 */
> 	if (boot_cpu_bas(X86_FEATURE_HYPERVISOR))
> 		return NULL;
> 
> (well, it should probably actually be in the for() loop because folks
> might be looking for an X86_FEATURE_* that is set by software or derived
> from actually agreed-upon host<->guest ABI, but you get my point...)
> 
> If the hypervisor is duplicitous enough to keep X86_FEATURE_HYPERVISOR
> from getting set, then the hypervisor gets to clean up the mess.  The
> kernel can just wash its hands of the whole thing.
> 
> So, there are two broad cases and a few sub-cases:
> 
> 1. "Nice" hypervisor.  Kernel sees X86_FEATURE_HYPERVISOR and knows that
>    x86_match_cpu() and invlpg_miss_ids[] are irrelevant because:
>  1a.  It is running in VMX non-root mode and is not vulnerable, or
>  1b.  CPUID is a lie and x86_match_cpu() is meaningless, or
>  1c.  The kernel is in ring3 and can't execute INVLPG anyway.  Whatever
>       is running in ring0 will have to deal with it.
> 2. X86_FEATURE_HYPERVISOR is unset.
>  2a. "Mean" hypervisor. All bets are off anyway.
>  2b. Actual bare metal. Actually look for the bug.
> 
> I _think_ I'm OK with skipping the mitigation in all of the #1 cases and
> applying it in both of the #2 cases.  I don't think that checking for
> VMX makes it much better.
> 
> Am I missing anything?

I'm a-ok with just checking HYPERVISOR, I agree that the hypervisor is fully
responsible for correctly emulating PCID and INVLPG stuff for (1c).

My reaction was really just to the changelog equating HYPERVSIOR to VMX non-root.