Received-SPF: pass (google.com: domain of linux-kernel+bounces-69653-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Fri, 16 Feb 2024 17:40:51 -0800
In-Reply-To: <CABgObfY=aGJNMk4CYb7nvauBWLJVbwVaA69bOK4bLteH7YyBNA@mail.gmail.com>
Precedence: bulk
Mime-Version: 1.0
References: <20240209183743.22030-1-pbonzini@redhat.com> <ZcZ_m5By49jsKNXn@google.com>
 <CABgObfaum2=MpXE2kJsETe31RqWnXJQWBQ2iCMvFUoJXJkhF+w@mail.gmail.com>
 <ZcrX_4vbXNxiQYtM@google.com> <CABgObfY=aGJNMk4CYb7nvauBWLJVbwVaA69bOK4bLteH7YyBNA@mail.gmail.com>
Message-ID: <ZdAOo2AAm_NrTdOe@google.com>
Subject: Re: [PATCH 00/10] KVM: SEV: allow customizing VMSA features
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, 
	aik@amd.com, isaku.yamahata@intel.com
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

On Tue, Feb 13, 2024, Paolo Bonzini wrote:
> On Tue, Feb 13, 2024 at 3:46=E2=80=AFAM Sean Christopherson <seanjc@googl=
e.com> wrote:
> >   __u32 flags;
> >   __u32 vm_type;
> >   union {
> >         struct tdx;
> >         struct sev;
> >         struct sev_es;
> >         struct sev_snp;
> >         __u8 pad[<big size>]
> >   };
> >
> > Rinse and repeat for APIs that have a common purpose, but different pay=
loads.
> >
> > Similar to KVM_{SET,GET}_NESTED_STATE, where the data is wildly differe=
nt, and
> > there's very little overlap between {svm,vmx}_set_nested_state(), I fin=
d it quite
> > valuable to have a single set of APIs.  E.g. I don't have to translate =
between
> > VMX and SVM terminology when thinking about the APIs, when discussing t=
hem, etc.
> >
> > That's especially true for all this CoCo goo, where the names are ridic=
ulously
> > divergent, and often not exactly intuitive.  E.g. LAUNCH_MEASURE reads =
like
> > "measure the launch", but surprise, it's "get the measurement".
>=20
> I agree, but then you'd have to do things like "CPUID data is passed
> via UPDATE_DATA for SEV and INIT_VM for TDX (and probably not at all
> for pKVM)". And in one case the firmware may prefer to encrypt in
> place, in the other you cannot do that at all.
>=20
> There was a reason why SVM support was not added from the beginning.
> Before adding nested get/set support for SVM, the whole nested
> virtualization was made as similar as possible in design and
> functionality to VMX. Of course it cannot be entirely the same, but
> for example they share the overall idea that pending events and L2
> state are taken from vCPU state; kvm_nested_state only stores global
> processor state (VMXON/VMCS pointers on VMX, and GIF on SVM) and,
> while in guest mode, L1 state and control bits. This ensures that the
> same userspace flow can work for both VMX and SVM. However, in this
> case we can't really control what is done in firmware.
>=20
> > The effort doesn't seem huge, so long as we don't try to make the param=
eters
> > common across vendor code.  The list of APIs doesn't seem insurmountabl=
e (note,
> > I'm not entirely sure these are correct mappings):
>=20
> While the effort isn't huge, the benefit is also pretty small, which
> comes to a second big difference with GET/SET_NESTED_STATE: because
> there is a GET ioctl, we have the possibility of retrieving the "black
> box" and passing it back. With CoCo it's anyway userspace's task to
> fill in the parameter structs. I just don't see the possibility of
> sharing any code except the final ioctl, which to be honest is not
> much to show. And the higher price might be in re-reviewing code that
> has already been reviewed, both in KVM and in userspace.

Yeah, I realize I'm probably grasping at straws.  *sigh*