Received-SPF: pass (google.com: domain of linux-kernel+bounces-85378-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Wed, 28 Feb 2024 08:39:09 -0800
In-Reply-To: <CABgObfYpRJnDdQrxp=OgjhbT9A+LHK36MHjMvzcQJsHAmfX++w@mail.gmail.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240227232100.478238-1-pbonzini@redhat.com> <Zd6LK7RpZZ8t-5CY@google.com>
 <CABgObfYpRJnDdQrxp=OgjhbT9A+LHK36MHjMvzcQJsHAmfX++w@mail.gmail.com>
Message-ID: <Zd9hrfJ5xRI6HeZp@google.com>
Subject: Re: [PATCH 00/21] TDX/SNP part 1 of n, for 6.9
From: Sean Christopherson <seanjc@google.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, michael.roth@amd.com, 
	isaku.yamahata@intel.com, thomas.lendacky@amd.com
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Wed, Feb 28, 2024, Paolo Bonzini wrote:
> On Wed, Feb 28, 2024 at 2:25=E2=80=AFAM Sean Christopherson <seanjc@googl=
e.com> wrote:
> > > Michael Roth (2):
> > >   KVM: x86: Add gmem hook for invalidating memory
> > >   KVM: x86: Add gmem hook for determining max NPT mapping level
> > >
> > > Paolo Bonzini (6):
> > >   KVM: x86/mmu: pass error code back to MMU when async pf is ready
> > >   KVM: x86/mmu: Use PFERR_GUEST_ENC_MASK to indicate fault is private
> >
> > This doesn't work.  The ENC flag gets set on any SNP *capable* CPU, whi=
ch results
> > in false positives for SEV and SEV-ES guests[*].
>=20
> You didn't look at the patch did you? :)

Guilty, sort of.  I looked (and tested) the patch from the TDX series, but =
I didn't
look at what you postd.  But it's a moot point, because now I did look at w=
hat you
posted, and it's still broken :-)

> It does check for has_private_mem (alternatively I could have dropped the=
 bit
> in SVM code for SEV and SEV-ES guests).

The problem isn't with *KVM* setting the bit, it's with *hardware* setting =
the
bit for SEV and SEV-ES guests.  That results in this:

  .is_private =3D vcpu->kvm->arch.has_private_mem && (err & PFERR_GUEST_ENC=
_MASK),

marking the fault as private.  Which, in a vacuum, isn't technically wrong,=
 since
from hardware's perspective the vCPU access was "private".  But from KVM's
perspective, SEV and SEV-ES guests don't have private memory, they have mem=
ory
that can be *encrypted*, and marking the access as "private" results in vio=
lations
of KVM's rules for private memory.  Specifically, it results in KVM trigger=
ing
emulated MMIO for faults that are marked private, which we want to disallow=
 for
SNP and TDX.

And because the flag only gets set on SNP capable hardware (in my limited t=
esting
of a whole two systems), running the same VM on different hardware would re=
sult
in faults being marked private on one system, but not the other.  Which mea=
ns that
KVM can't rely on the flag being set for SEV or SEV-ES guests, i.e. we can'=
t
retroactively enforce anything (not to mention that that might break existi=
ng VMs).