linux-kernel - Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YGzEJa8izE//+Niy@google.com>
Date:   Tue, 6 Apr 2021 20:27:17 +0000
From:   Sean Christopherson <seanjc@...gle.com>
To:     Steve Rutherford <srutherford@...gle.com>
Cc:     Ashish Kalra <ashish.kalra@....com>,
        Paolo Bonzini <pbonzini@...hat.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>,
        "H. Peter Anvin" <hpa@...or.com>, Joerg Roedel <joro@...tes.org>,
        Borislav Petkov <bp@...e.de>,
        Tom Lendacky <thomas.lendacky@....com>,
        X86 ML <x86@...nel.org>, KVM list <kvm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Venu Busireddy <venu.busireddy@...cle.com>,
        Brijesh Singh <brijesh.singh@....com>,
        Will Deacon <will@...nel.org>, maz@...nel.org,
        Quentin Perret <qperret@...gle.com>
Subject: Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS
 hypercall

On Tue, Apr 06, 2021, Steve Rutherford wrote:
> On Tue, Apr 6, 2021 at 9:08 AM Ashish Kalra <ashish.kalra@....com> wrote:
> > I see the following in Documentation/virt/kvm/api.rst :
> > ..
> > ..
> > /* KVM_EXIT_HYPERCALL */
> >                 struct {
> >                         __u64 nr;
> >                         __u64 args[6];
> >                         __u64 ret;
> >                         __u32 longmode;
> >                         __u32 pad;
> >                 } hypercall;
> >
> > Unused.  This was once used for 'hypercall to userspace'.  To implement
> > such functionality, use KVM_EXIT_IO (x86) or KVM_EXIT_MMIO (all except s390).
> >
> > This mentions this exitcode to be unused and implementing this
> > functionality using KVM_EXIT_IO for x86?
> 
> I suspect this description is historical. It was originally from 2009.
> KVM_EXIT_IO is used for IO port reads/writes.

The purpose of the comment is to discourage use of hypercalls for things that
can instead be done via port I/O and/or MMIO.  The biggest advantage is that KVM
doesn't need to act as an intermediary; userspace can define whatever paravirt
shenanigans it so desires and KVM naturally handles the I/O accesses.

MMIO in particular also allows for finer granularity of permissions in the guest,
e.g. the guest kernel can expose the address to a userspace application to allow
said application to make "hypercalls".  Port I/O technically has similar
properties, but the I/O bitmaps are heinous.

For this particular case, because we want to make a _KVM_ hypercall that just
happens to get punted to userspace, and because there is no known or projected
use case for letting guest userspace control memory encryption it makes sense to
use a "real" hypercall.

> Personally, I would prefer to stay the course and use a name similar
> to KVM_EXIT_DMA_SHARE, say KVM_EXIT_MEM_SHARE and
> KVM_EXIT_MEM_UNSHARE. These just seem very clear, which I appreciate.
> Reusing hypercall would work, but shoehorning this into
> KVM_EXIT_HYPERCALL when we don't have generic hypercall exits feels a
> bit off in my mind. Note: that preference isn't particularly strong.

I'm not against adding a new exit type, I'm against adding _two_ new exit types.

I also don't like using "(UN)SHARE" because there may be future use cases where
the hypercall isn't used to "share' memory, but to inform the host of a change
in state.  I don't have a concrete example, but it's not completely absurd to
think that there might be a scenario where a guest has the ability to use multiple
keys and needs to communicate key usage to the host.  Linux already has horrific
(IMO) names for describing encrypted vs. "other" memory, I'd strongly prefer to
avoid adding yet another potentially wrong name to the mix.