Re: [RFC PATCH v3 14/59] KVM: x86: Add vm_type to differentiate legacy VMs from protected VMs

From: Sean Christopherson <seanjc@google.com>
To: Isaku Yamahata <isaku.yamahata@gmail.com>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	isaku.yamahata@intel.com, Ingo Molnar <mingo@redhat.com>,
	Borislav Petkov <bp@alien8.de>, "H . Peter Anvin" <hpa@zytor.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Vitaly Kuznetsov <vkuznets@redhat.com>,
	Wanpeng Li <wanpengli@tencent.com>,
	Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
	erdemaktas@google.com, Connor Kuehl <ckuehl@redhat.com>,
	linux-kernel@vger.kernel.org, kvm@vger.kernel.org,
	Sean Christopherson <sean.j.christopherson@intel.com>,
	Xiaoyao Li <xiaoyao.li@intel.com>
Subject: Re: [RFC PATCH v3 14/59] KVM: x86: Add vm_type to differentiate legacy VMs from protected VMs
Date: Fri, 3 Dec 2021 16:14:42 +0000	[thread overview]
Message-ID: <YapCclOiQXWGXVEr@google.com> (raw)
In-Reply-To: <20211201193737.GB1166703@private.email.ne.jp>

On Wed, Dec 01, 2021, Isaku Yamahata wrote:
> On Mon, Nov 29, 2021 at 05:35:34PM +0000,
> Sean Christopherson <seanjc@google.com> wrote:
> 
> > On Thu, Nov 25, 2021, Thomas Gleixner wrote:
> > > On Wed, Nov 24 2021 at 16:19, isaku yamahata wrote:
> > > > From: Sean Christopherson <sean.j.christopherson@intel.com>
> > > >
> > > > Add a capability to effectively allow userspace to query what VM types
> > > > are supported by KVM.
> > > 
> > > I really don't see why this has to be named legacy. There are enough
> > > reasonable use cases which are perfectly fine using the non-encrypted
> > > muck. Just because there is a new hyped feature does not make anything
> > > else legacy.
> > 
> > Yeah, this was brought up in the past.  The current proposal is to use
> > KVM_X86_DEFAULT_VM[1], though at one point the plan was to use a generic
> > KVM_VM_TYPE_DEFAULT for all architectures[2], not sure what happened to that idea.
> > 
> > [1] https://lore.kernel.org/all/YY6aqVkHNEfEp990@google.com/
> > [2] https://lore.kernel.org/all/YQsjQ5aJokV1HZ8N@google.com/
> 
> Currently <feature>_{unsupported, disallowed} are added and the check is
>  sprinkled and warn in the corresponding low level tdx code.  It helped to
>  detect dubious behavior of guest or qemu.

KVM shouldn't log a message or WARN unless the issue is detected at a late sanity
check, i.e. where failure indicates a KVM bug.  Other than that, I agree that KVM
should reject ioctls() that directly violate the rules of a confidential VM with
an appropriate error code.  I don't think KVM should reject everything though,
e.g. if the guest attempts to send an SMI, dropping the request on the floor is
the least awful option because we can't communicate an error to the guest without
making up our own architecture, and exiting to userspace with -EINVAL from deep
in KVM would be both painful to implement and an overreaction since doing so would
likely kill the guest.

> The other approach is to silently ignore them (SMI, INIT, IRQ etc) without
> such check.  The pros is, the code would be simpler and it's what SEV does today.
> the cons is, it would bes hard to track down such cases and the user would
> be confused.  For example, when user requests reset/SMI, it's silently ignored.
> The some check would still be needed.
> Any thoughts?
> 
> -- 
> Isaku Yamahata <isaku.yamahata@gmail.com>