Message-ID: <CABayD+cXH0oeV4-Ah3y6ThhNt3dhd0qDh6JmimjSz=EFjC+SYw@mail.gmail.com>
Date:   Thu, 11 Mar 2021 12:48:07 -0800
From:   Steve Rutherford <srutherford@...gle.com>
To:     Ashish Kalra <ashish.kalra@....com>
Cc:     Will Deacon <will@...nel.org>,
        Sean Christopherson <seanjc@...gle.com>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "joro@...tes.org" <joro@...tes.org>,
        "Lendacky, Thomas" <Thomas.Lendacky@....com>,
        "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "venu.busireddy@...cle.com" <venu.busireddy@...cle.com>,
        "Singh, Brijesh" <brijesh.singh@....com>,
        Quentin Perret <qperret@...gle.com>, maz@...nel.org
Subject: Re: [PATCH v10 10/16] KVM: x86: Introduce KVM_GET_SHARED_PAGES_LIST ioctl

On Thu, Mar 11, 2021 at 10:15 AM Ashish Kalra <ashish.kalra@....com> wrote:
>
> On Wed, Mar 03, 2021 at 06:54:41PM +0000, Will Deacon wrote:
> > [+Marc]
> >
> > On Tue, Mar 02, 2021 at 02:55:43PM +0000, Ashish Kalra wrote:
> > > On Fri, Feb 26, 2021 at 09:44:41AM -0800, Sean Christopherson wrote:
> > > > On Fri, Feb 26, 2021, Ashish Kalra wrote:
> > > > > On Thu, Feb 25, 2021 at 02:59:27PM -0800, Steve Rutherford wrote:
> > > > > > On Thu, Feb 25, 2021 at 12:20 PM Ashish Kalra <ashish.kalra@....com> wrote:
> > > > > > Thanks for grabbing the data!
> > > > > >
> > > > > > I am fine with both paths. Sean has stated an explicit desire for
> > > > > > hypercall exiting, so I think that would be the current consensus.
> > > >
> > > > Yep, though it'd be good to get Paolo's input, too.
> > > >
> > > > > > If we want to do hypercall exiting, this should be in a follow-up
> > > > > > series where we implement something more generic, e.g. a hypercall
> > > > > > exiting bitmap or hypercall exit list. If we are taking the hypercall
> > > > > > exit route, we can drop the kvm side of the hypercall.
> > > >
> > > > I don't think this is a good candidate for arbitrary hypercall interception.  Or
> > > > rather, I think hypercall interception should be an orthogonal implementation.
> > > >
> > > > The guest, including guest firmware, needs to be aware that the hypercall is
> > > > supported, and the ABI needs to be well-defined.  Relying on userspace VMMs to
> > > > implement a common ABI is an unnecessary risk.
> > > >
> > > > We could make KVM's default behavior be a nop, i.e. have KVM enforce the ABI but
> > > > require further VMM intervention.  But, I just don't see the point, it would
> > > > save only a few lines of code.  It would also limit what KVM could do in the
> > > > future, e.g. if KVM wanted to do its own bookkeeping _and_ exit to userspace,
> > > > then mandatory interception would essentially make it impossible for KVM to do
> > > > bookkeeping while still honoring the interception request.
> > > >
> > > > However, I do think it would make sense to have the userspace exit be a generic
> > > > exit type.  But hey, we already have the necessary ABI defined for that!  It's
> > > > just not used anywhere.
> > > >
> > > >   /* KVM_EXIT_HYPERCALL */
> > > >   struct {
> > > >           __u64 nr;
> > > >           __u64 args[6];
> > > >           __u64 ret;
> > > >           __u32 longmode;
> > > >           __u32 pad;
> > > >   } hypercall;
> > > >
> > > >
> > > > > > Userspace could also handle the MSR using MSR filters (would need to
> > > > > > confirm that).  Then userspace could also be in control of the cpuid bit.
> > > >
> > > > An MSR is not a great fit; it's x86 specific and limited to 64 bits of data.
> > > > The data limitation could be fudged by shoving data into non-standard GPRs, but
> > > > that will result in truly heinous guest code, and extensibility issues.
> > > >
> > > > The data limitation is a moot point, because the x86-only thing is a deal
> > > > breaker.  arm64's pKVM work has a near-identical use case for a guest to share
> > > > memory with a host.  I can't think of a clever way to avoid having to support
> > > > TDX's and SNP's hypervisor-agnostic variants, but we can at least not have
> > > > multiple KVM variants.
> > >
> > > Looking at arm64's pKVM work, i see that it is a recently introduced RFC
> > > patch-set and probably relevant to arm64 nVHE hypervisor
> > > mode/implementation, and potentially makes sense as it adds guest
> > > memory protection as both host and guest kernels are running on the same
> > > privilege level ?
> > >
> > > Though i do see that the pKVM stuff adds two hypercalls, specifically :
> > >
> > > pkvm_create_mappings() ( I assume this is for setting shared memory
> > > regions between host and guest) &
> > > pkvm_create_private_mappings().
> > >
> > > And the use-cases are quite similar to memory protection architectues
> > > use cases, for example, use with virtio devices, guest DMA I/O, etc.
> >
> > These hypercalls are both private to the host kernel communicating with
> > its hypervisor counterpart, so I don't think they're particularly
> > relevant here. As far as I can see, the more useful thing is to allow
> > the guest to communicate back to the host (and the VMM) that it has opened
> > up a memory window, perhaps for virtio rings or some other shared memory.
> >
> > We hacked this up as a prototype in the past:
> >
> > https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fandroid-kvm.googlesource.com%2Flinux%2F%2B%2Fd12a9e2c12a52cf7140d40cd9fa092dc8a85fac9%255E%2521%2F&amp;data=04%7C01%7Cashish.kalra%40amd.com%7C7ae6bbd9fa6442f9edcc08d8de75d14b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637503944913839841%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Juon5nJ7BB6moTWYssRXOWrDOrYfZLmA%2BLrz3s12Ook%3D&amp;reserved=0
> >
> > but that's all arm64-specific and if we're solving the same problem as
> > you, then let's avoid arch-specific stuff if possible. The way in which
> > the guest discovers the interface will be arch-specific (we already have
> > a mechanism for that and some hypercalls are already allocated by specs
> > from Arm), but the interface back to the VMM and some (most?) of the host
> > handling could be shared.
> >
>
> I have started implementing a similar "hypercall to userspace"
> functionality for these DMA_SHARE/DMA_UNSHARE type of interfaces
> corresponding to SEV guest's add/remove shared regions on the x86 platform.
>
> This does not implement a generic hypercall exiting infrastructure,
> mainly extends the KVM hypercall support to return back to userspace
> specifically for add/remove shared region hypercalls and then re-uses
> the complete userspace I/O callback functionality to resume the guest
> after returning back from userspace handling of the hypercall.
>
> Looking fwd. to any comments/feedback/thoughts on the above.
Others have mentioned a lack of appetite for generic hypercall
intercepts, so this is the right approach.

Thanks,
Steve

Powered by blists - more mailing lists