LKML Archive mirror
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Steve Rutherford <srutherford@google.com>
Cc: Ashish Kalra <ashish.kalra@amd.com>,
	Paolo Bonzini <pbonzini@redhat.com>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	Joerg Roedel <joro@8bytes.org>, Borislav Petkov <bp@suse.de>,
	Tom Lendacky <thomas.lendacky@amd.com>, X86 ML <x86@kernel.org>,
	KVM list <kvm@vger.kernel.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Venu Busireddy <venu.busireddy@oracle.com>,
	Brijesh Singh <brijesh.singh@amd.com>,
	Will Deacon <will@kernel.org>,
	maz@kernel.org, Quentin Perret <qperret@google.com>
Subject: Re: [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall
Date: Tue, 6 Apr 2021 20:27:17 +0000	[thread overview]
Message-ID: <YGzEJa8izE//+Niy@google.com> (raw)
In-Reply-To: <CABayD+f9o1CZTdak-ktKXpJnxcOAP4KPnYCDBzry91QcK6WVcw@mail.gmail.com>

On Tue, Apr 06, 2021, Steve Rutherford wrote:
> On Tue, Apr 6, 2021 at 9:08 AM Ashish Kalra <ashish.kalra@amd.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.

  reply	other threads:[~2021-04-06 20:27 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-05 14:20 [PATCH v11 00/13] Add AMD SEV guest live migration support Ashish Kalra
2021-04-05 14:21 ` [PATCH v11 01/13] KVM: SVM: Add KVM_SEV SEND_START command Ashish Kalra
2021-04-05 14:23 ` [PATCH v11 02/13] KVM: SVM: Add KVM_SEND_UPDATE_DATA command Ashish Kalra
2021-04-05 14:23 ` [PATCH v11 03/13] KVM: SVM: Add KVM_SEV_SEND_FINISH command Ashish Kalra
2021-04-05 14:24 ` [PATCH v11 04/13] KVM: SVM: Add support for KVM_SEV_RECEIVE_START command Ashish Kalra
2021-04-05 14:24 ` [PATCH v11 05/13] KVM: SVM: Add KVM_SEV_RECEIVE_UPDATE_DATA command Ashish Kalra
2021-04-05 14:25 ` [PATCH v11 06/13] KVM: SVM: Add KVM_SEV_RECEIVE_FINISH command Ashish Kalra
2021-04-05 14:26 ` [PATCH v11 07/13] KVM: x86: Add AMD SEV specific Hypercall3 Ashish Kalra
2021-04-05 14:28 ` [PATCH v11 08/13] KVM: X86: Introduce KVM_HC_PAGE_ENC_STATUS hypercall Ashish Kalra
2021-04-05 20:42   ` Steve Rutherford
2021-04-06  6:22     ` Ashish Kalra
2021-04-06 15:11       ` Sean Christopherson
2021-04-06 15:20         ` Sean Christopherson
2021-04-06 18:14       ` Ashish Kalra
2021-04-06 15:48   ` Sean Christopherson
2021-04-06 16:07     ` Ashish Kalra
2021-04-06 20:14       ` Steve Rutherford
2021-04-06 20:27         ` Sean Christopherson [this message]
2021-04-07 14:01     ` Ashish Kalra
2021-04-05 14:29 ` [PATCH v11 09/13] mm: x86: Invoke hypercall when page encryption status is changed Ashish Kalra
2021-04-05 14:30 ` [PATCH v11 10/13] KVM: x86: Introduce new KVM_FEATURE_SEV_LIVE_MIGRATION feature & Custom MSR Ashish Kalra
2021-04-06  1:39   ` Steve Rutherford
2021-04-06 13:26     ` Ashish Kalra
2021-04-06 13:47       ` Paolo Bonzini
2021-04-06 13:59         ` Ashish Kalra
2021-04-06 19:41           ` Steve Rutherford
2021-04-05 14:30 ` [PATCH v11 11/13] EFI: Introduce the new AMD Memory Encryption GUID Ashish Kalra
2021-04-05 14:31 ` [PATCH v11 12/13] x86/kvm: Add guest support for detecting and enabling SEV Live Migration feature Ashish Kalra
2021-04-05 14:35 ` [PATCH v11 13/13] x86/kvm: Add kexec support for SEV Live Migration Ashish Kalra
2021-04-05 15:17 ` [PATCH v11 00/13] Add AMD SEV guest live migration support Peter Gonda
2021-04-05 18:27   ` Steve Rutherford
2021-04-06 13:48     ` Paolo Bonzini
2021-04-06  1:43 ` Steve Rutherford

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YGzEJa8izE//+Niy@google.com \
    --to=seanjc@google.com \
    --cc=ashish.kalra@amd.com \
    --cc=bp@suse.de \
    --cc=brijesh.singh@amd.com \
    --cc=hpa@zytor.com \
    --cc=joro@8bytes.org \
    --cc=kvm@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maz@kernel.org \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=qperret@google.com \
    --cc=srutherford@google.com \
    --cc=tglx@linutronix.de \
    --cc=thomas.lendacky@amd.com \
    --cc=venu.busireddy@oracle.com \
    --cc=will@kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).