From: Michael Roth <michael.roth@amd.com>
To: Binbin Wu <binbin.wu@linux.intel.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>, <kvm@vger.kernel.org>,
<linux-coco@lists.linux.dev>, <linux-mm@kvack.org>,
<linux-crypto@vger.kernel.org>, <x86@kernel.org>,
<linux-kernel@vger.kernel.org>, <tglx@linutronix.de>,
<mingo@redhat.com>, <jroedel@suse.de>, <thomas.lendacky@amd.com>,
<hpa@zytor.com>, <ardb@kernel.org>, <seanjc@google.com>,
<vkuznets@redhat.com>, <jmattson@google.com>, <luto@kernel.org>,
<dave.hansen@linux.intel.com>, <slp@redhat.com>,
<pgonda@google.com>, <peterz@infradead.org>,
<srinivas.pandruvada@linux.intel.com>, <rientjes@google.com>,
<dovmurik@linux.ibm.com>, <tobin@ibm.com>, <bp@alien8.de>,
<vbabka@suse.cz>, <kirill@shutemov.name>, <ak@linux.intel.com>,
<tony.luck@intel.com>,
<sathyanarayanan.kuppuswamy@linux.intel.com>,
<alpergun@google.com>, <jarkko@kernel.org>,
<ashish.kalra@amd.com>, <nikunj.dadhania@amd.com>,
<pankaj.gupta@amd.com>, <liam.merwick@oracle.com>,
Brijesh Singh <brijesh.singh@amd.com>,
"Yamahata, Isaku" <isaku.yamahata@intel.com>
Subject: Re: [PATCH v15 09/20] KVM: SEV: Add support to handle MSR based Page State Change VMGEXIT
Date: Tue, 21 May 2024 16:49:52 -0500 [thread overview]
Message-ID: <rczrxq3lhqguarwh4cwxwa35j5riiagbilcw32oaxd7aqpyaq7@6bqrqn6ontba> (raw)
In-Reply-To: <7d6a4320-89f5-48ce-95ff-54b00e7e9597@linux.intel.com>
On Tue, May 21, 2024 at 08:49:59AM +0800, Binbin Wu wrote:
>
>
> On 5/17/2024 1:23 AM, Paolo Bonzini wrote:
> > On Thu, May 16, 2024 at 10:29 AM Binbin Wu <binbin.wu@linux.intel.com> wrote:
> > >
> > >
> > > On 5/1/2024 4:51 PM, Michael Roth wrote:
> > > > SEV-SNP VMs can ask the hypervisor to change the page state in the RMP
> > > > table to be private or shared using the Page State Change MSR protocol
> > > > as defined in the GHCB specification.
> > > >
> > > > When using gmem, private/shared memory is allocated through separate
> > > > pools, and KVM relies on userspace issuing a KVM_SET_MEMORY_ATTRIBUTES
> > > > KVM ioctl to tell the KVM MMU whether or not a particular GFN should be
> > > > backed by private memory or not.
> > > >
> > > > Forward these page state change requests to userspace so that it can
> > > > issue the expected KVM ioctls. The KVM MMU will handle updating the RMP
> > > > entries when it is ready to map a private page into a guest.
> > > >
> > > > Use the existing KVM_HC_MAP_GPA_RANGE hypercall format to deliver these
> > > > requests to userspace via KVM_EXIT_HYPERCALL.
> > > >
> > > > Signed-off-by: Michael Roth <michael.roth@amd.com>
> > > > Co-developed-by: Brijesh Singh <brijesh.singh@amd.com>
> > > > Signed-off-by: Brijesh Singh <brijesh.singh@amd.com>
> > > > Signed-off-by: Ashish Kalra <ashish.kalra@amd.com>
> > > > ---
> > > > arch/x86/include/asm/sev-common.h | 6 ++++
> > > > arch/x86/kvm/svm/sev.c | 48 +++++++++++++++++++++++++++++++
> > > > 2 files changed, 54 insertions(+)
> > > >
> > > > diff --git a/arch/x86/include/asm/sev-common.h b/arch/x86/include/asm/sev-common.h
> > > > index 1006bfffe07a..6d68db812de1 100644
> > > > --- a/arch/x86/include/asm/sev-common.h
> > > > +++ b/arch/x86/include/asm/sev-common.h
> > > > @@ -101,11 +101,17 @@ enum psc_op {
> > > > /* GHCBData[11:0] */ \
> > > > GHCB_MSR_PSC_REQ)
> > > >
> > > > +#define GHCB_MSR_PSC_REQ_TO_GFN(msr) (((msr) & GENMASK_ULL(51, 12)) >> 12)
> > > > +#define GHCB_MSR_PSC_REQ_TO_OP(msr) (((msr) & GENMASK_ULL(55, 52)) >> 52)
> > > > +
> > > > #define GHCB_MSR_PSC_RESP 0x015
> > > > #define GHCB_MSR_PSC_RESP_VAL(val) \
> > > > /* GHCBData[63:32] */ \
> > > > (((u64)(val) & GENMASK_ULL(63, 32)) >> 32)
> > > >
> > > > +/* Set highest bit as a generic error response */
> > > > +#define GHCB_MSR_PSC_RESP_ERROR (BIT_ULL(63) | GHCB_MSR_PSC_RESP)
> > > > +
> > > > /* GHCB Hypervisor Feature Request/Response */
> > > > #define GHCB_MSR_HV_FT_REQ 0x080
> > > > #define GHCB_MSR_HV_FT_RESP 0x081
> > > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > > index e1ac5af4cb74..720775c9d0b8 100644
> > > > --- a/arch/x86/kvm/svm/sev.c
> > > > +++ b/arch/x86/kvm/svm/sev.c
> > > > @@ -3461,6 +3461,48 @@ static void set_ghcb_msr(struct vcpu_svm *svm, u64 value)
> > > > svm->vmcb->control.ghcb_gpa = value;
> > > > }
> > > >
> > > > +static int snp_complete_psc_msr(struct kvm_vcpu *vcpu)
> > > > +{
> > > > + struct vcpu_svm *svm = to_svm(vcpu);
> > > > +
> > > > + if (vcpu->run->hypercall.ret)
> > > Do we have definition of ret? I didn't find clear documentation about it.
> > > According to the code, 0 means succssful. Is there any other error codes
> > > need to or can be interpreted?
> > They are defined in include/uapi/linux/kvm_para.h
> >
> > #define KVM_ENOSYS 1000
> > #define KVM_EFAULT EFAULT /* 14 */
> > #define KVM_EINVAL EINVAL /* 22 */
> > #define KVM_E2BIG E2BIG /* 7 */
> > #define KVM_EPERM EPERM /* 1*/
> > #define KVM_EOPNOTSUPP 95
> >
> > Linux however does not expect the hypercall to fail for SEV/SEV-ES; and
> > it will terminate the guest if the PSC operation fails for SEV-SNP. So
> > it's best for userspace if the hypercall always succeeds. :)
> Thanks for the info.
>
> For TDX, it wants to restrict the size of memory range for conversion in one
> hypercall to avoid a too long latency.
> Previously, in TDX QEMU patchset v5, the limitation is in userspace and if
> the size is too big, the status_code will set to TDG_VP_VMCALL_RETRY and the
> failed GPA for guest to retry is updated.
> https://lore.kernel.org/all/20240229063726.610065-51-xiaoyao.li@intel.com/
>
> When TDX converts TDVMCALL_MAP_GPA to KVM_HC_MAP_GPA_RANGE, do you think
> which is more reasonable to set the restriction? In KVM (TDX specific code)
> or userspace?
> If userspace is preferred, then the interface needs to be extended to
> support it.
With SNP we might get a batch of requests in a single GHCB request, and
potentially each of those requests need to get set out to userspace as
a single KVM_HC_MAP_GPA_RANGE. The subsequent patch here handles that in
a loop by issuing a new KVM_HC_MAP_GPA_RANGE via the completion handler.
So we also sort of need to split large requests into multiple userspace
requests in some cases.
It seems like TDX should be able to do something similar by limiting the
size of each KVM_HC_MAP_GPA_RANGE to TDX_MAP_GPA_MAX_LEN, and then
returning TDG_VP_VMCALL_RETRY to guest if the original size was greater
than TDX_MAP_GPA_MAX_LEN. But at that point you're effectively done with
the entire request and can return to guest, so it actually seems a little
more straightforward than the SNP case above. E.g. TDX has a 1:1 mapping
between TDG_VP_VMCALL_MAP_GPA and KVM_HC_MAP_GPA_RANGE events. (And even
similar names :))
So doesn't seem like there's a good reason to expose any of these
throttling details to userspace, in which case existing
KVM_HC_MAP_GPA_RANGE interface seems like it should be sufficient.
-Mike
>
>
> >
> > > For TDX, it may also want to use KVM_HC_MAP_GPA_RANGE hypercall to
> > > userspace via KVM_EXIT_HYPERCALL.
> > Yes, definitely.
> >
> > Paolo
> >
>
next prev parent reply other threads:[~2024-05-21 21:50 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-01 8:51 [PATCH v15 00/20] Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support Michael Roth
2024-05-01 8:51 ` [PATCH v15 01/20] Revert "KVM: x86: Add gmem hook for determining max NPT mapping level" Michael Roth
2024-05-01 8:51 ` [PATCH v15 02/20] KVM: x86: Add hook for determining max NPT mapping level Michael Roth
2024-05-02 23:11 ` Isaku Yamahata
2024-05-07 17:48 ` Paolo Bonzini
2024-05-01 8:51 ` [PATCH v15 03/20] KVM: SEV: Select KVM_GENERIC_PRIVATE_MEM when CONFIG_KVM_AMD_SEV=y Michael Roth
2024-05-01 8:51 ` [PATCH v15 04/20] KVM: SEV: Add initial SEV-SNP support Michael Roth
2024-05-01 8:51 ` [PATCH v15 05/20] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command Michael Roth
2024-05-01 8:51 ` [PATCH v15 06/20] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command Michael Roth
2024-05-01 8:51 ` [PATCH v15 07/20] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_FINISH command Michael Roth
2024-05-01 8:51 ` [PATCH v15 08/20] KVM: SEV: Add support to handle GHCB GPA register VMGEXIT Michael Roth
2024-05-01 8:51 ` [PATCH v15 09/20] KVM: SEV: Add support to handle MSR based Page State Change VMGEXIT Michael Roth
2024-05-16 8:28 ` Binbin Wu
2024-05-16 17:23 ` Paolo Bonzini
2024-05-21 0:49 ` Binbin Wu
2024-05-21 21:49 ` Michael Roth [this message]
2024-05-27 12:25 ` Binbin Wu
2024-05-28 10:39 ` Paolo Bonzini
2024-05-29 20:02 ` Sean Christopherson
2024-05-31 1:22 ` Binbin Wu
2024-05-31 13:10 ` Paolo Bonzini
2024-05-30 16:47 ` Zhi Wang
2024-05-01 8:52 ` [PATCH v15 10/20] KVM: SEV: Add support to handle " Michael Roth
2024-05-01 8:52 ` [PATCH v15 11/20] KVM: SEV: Add support to handle RMP nested page faults Michael Roth
2024-05-01 8:52 ` [PATCH v15 12/20] KVM: SEV: Support SEV-SNP AP Creation NAE event Michael Roth
2024-05-01 8:52 ` [PATCH v15 13/20] KVM: SEV: Implement gmem hook for initializing private pages Michael Roth
2024-05-20 10:16 ` Huang, Kai
2024-05-20 17:35 ` Sean Christopherson
2024-05-20 21:57 ` Huang, Kai
2024-05-20 23:15 ` Sean Christopherson
2024-05-20 23:41 ` Huang, Kai
2024-05-21 0:30 ` Sean Christopherson
2024-05-20 19:14 ` Isaku Yamahata
2024-05-01 8:52 ` [PATCH v15 14/20] KVM: SEV: Implement gmem hook for invalidating " Michael Roth
2024-05-01 8:52 ` [PATCH v15 15/20] KVM: x86: Implement hook for determining max NPT mapping level Michael Roth
2024-05-01 8:52 ` [PATCH v15 16/20] KVM: SEV: Avoid WBINVD for HVA-based MMU notifications for SNP Michael Roth
2024-05-01 8:52 ` [PATCH v15 17/20] KVM: SVM: Add module parameter to enable SEV-SNP Michael Roth
2024-05-01 8:52 ` [PATCH v15 18/20] KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event Michael Roth
2024-05-01 8:52 ` [PATCH v15 19/20] KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST " Michael Roth
2024-05-13 23:48 ` Sean Christopherson
2024-05-14 2:51 ` Michael Roth
2024-05-14 14:36 ` Sean Christopherson
2024-05-15 1:25 ` [PATCH] KVM: SEV: Replace KVM_EXIT_VMGEXIT with KVM_EXIT_SNP_REQ_CERTS Michael Roth
2024-05-01 8:52 ` [PATCH v15 20/20] crypto: ccp: Add the SNP_VLEK_LOAD command Michael Roth
2024-05-07 18:04 ` [PATCH v15 00/20] Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support Paolo Bonzini
2024-05-07 18:14 ` Michael Roth
2024-05-10 2:34 ` Michael Roth
2024-05-10 1:58 ` [PATCH v15 21/23] KVM: MMU: Disable fast path for private memslots Michael Roth
2024-05-10 1:58 ` [PATCH v15 22/23] KVM: SEV: Fix return code interpretation for RMP nested page faults Michael Roth
2024-05-10 13:58 ` Sean Christopherson
2024-05-10 15:36 ` Michael Roth
2024-05-10 16:01 ` Paolo Bonzini
2024-05-10 16:37 ` Michael Roth
2024-05-10 16:59 ` Paolo Bonzini
2024-05-10 17:25 ` Paolo Bonzini
2024-05-14 8:10 ` Borislav Petkov
2024-05-10 1:58 ` [PATCH v15 23/23] KVM: SEV: Fix PSC handling for SMASH/UNSMASH and partial update ops Michael Roth
2024-05-10 17:09 ` Paolo Bonzini
2024-05-10 19:08 ` Michael Roth
2024-05-10 13:47 ` [PATCH v15 21/23] KVM: MMU: Disable fast path for private memslots Sean Christopherson
2024-05-10 13:50 ` Paolo Bonzini
2024-05-10 15:27 ` Michael Roth
2024-05-10 15:59 ` Sean Christopherson
2024-05-10 17:47 ` Isaku Yamahata
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=rczrxq3lhqguarwh4cwxwa35j5riiagbilcw32oaxd7aqpyaq7@6bqrqn6ontba \
--to=michael.roth@amd.com \
--cc=ak@linux.intel.com \
--cc=alpergun@google.com \
--cc=ardb@kernel.org \
--cc=ashish.kalra@amd.com \
--cc=binbin.wu@linux.intel.com \
--cc=bp@alien8.de \
--cc=brijesh.singh@amd.com \
--cc=dave.hansen@linux.intel.com \
--cc=dovmurik@linux.ibm.com \
--cc=hpa@zytor.com \
--cc=isaku.yamahata@intel.com \
--cc=jarkko@kernel.org \
--cc=jmattson@google.com \
--cc=jroedel@suse.de \
--cc=kirill@shutemov.name \
--cc=kvm@vger.kernel.org \
--cc=liam.merwick@oracle.com \
--cc=linux-coco@lists.linux.dev \
--cc=linux-crypto@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=luto@kernel.org \
--cc=mingo@redhat.com \
--cc=nikunj.dadhania@amd.com \
--cc=pankaj.gupta@amd.com \
--cc=pbonzini@redhat.com \
--cc=peterz@infradead.org \
--cc=pgonda@google.com \
--cc=rientjes@google.com \
--cc=sathyanarayanan.kuppuswamy@linux.intel.com \
--cc=seanjc@google.com \
--cc=slp@redhat.com \
--cc=srinivas.pandruvada@linux.intel.com \
--cc=tglx@linutronix.de \
--cc=thomas.lendacky@amd.com \
--cc=tobin@ibm.com \
--cc=tony.luck@intel.com \
--cc=vbabka@suse.cz \
--cc=vkuznets@redhat.com \
--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).