All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Roth <michael.roth@amd.com>
To: Paolo Bonzini <pbonzini@redhat.com>
Cc: <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>
Subject: Re: [PATCH v12 18/29] KVM: SEV: Use a VMSA physical address variable for populating VMCB
Date: Wed, 17 Apr 2024 15:57:25 -0500	[thread overview]
Message-ID: <20240417205725.yougm6og3cuea2hu@amd.com> (raw)
In-Reply-To: <CABgObfZNVR-VKst8dDFZ4gs_zSWE8NE2gj5-Y4TNh0AnBfti7w@mail.gmail.com>

On Tue, Apr 16, 2024 at 01:53:24PM +0200, Paolo Bonzini wrote:
> On Sat, Mar 30, 2024 at 10:01 PM Paolo Bonzini <pbonzini@redhat.com> wrote:
> >
> > On 3/29/24 23:58, Michael Roth wrote:
> > > From: Tom Lendacky<thomas.lendacky@amd.com>
> > >
> > > In preparation to support SEV-SNP AP Creation, use a variable that holds
> > > the VMSA physical address rather than converting the virtual address.
> > > This will allow SEV-SNP AP Creation to set the new physical address that
> > > will be used should the vCPU reset path be taken.
> > >
> > > Signed-off-by: Tom Lendacky<thomas.lendacky@amd.com>
> > > Signed-off-by: Ashish Kalra<ashish.kalra@amd.com>
> > > Signed-off-by: Michael Roth<michael.roth@amd.com>
> > > ---
> >
> > I'll get back to this one after Easter, but it looks like Sean had some
> > objections at https://lore.kernel.org/lkml/ZeCqnq7dLcJI41O9@google.com/.
> 
> So IIUC the gist of the solution here would be to replace
> 
>    /* Use the new VMSA */
>    svm->sev_es.vmsa_pa = pfn_to_hpa(pfn);
>    svm->vmcb->control.vmsa_pa = svm->sev_es.vmsa_pa;
> 
> with something like
> 
>    /* Use the new VMSA */
>    __free_page(virt_to_page(svm->sev_es.vmsa));

One downside to free'ing VMSA at this point is there are a number of
additional cleanup routines like wbinvd_on_all_cpus() and in sev_free_vcpu()
which will need to be called before we are able to safely free the page back
to the system.

It would be simple to wrap all that up in an sev_free_vmsa() helper and
also call it here rather than defer it, but from a performance
perspective it would be nice to defer it to shutdown path.


>    svm->sev_es.vmsa = pfn_to_kaddr(pfn);
>    svm->vmcb->control.vmsa_pa = __pa(svm->sev_es.vmsa);

It turns out sev_es_init_vmcb() always ends up setting control.vmsa_pa
again using the new vmsa stored in sev_es.vmsa before the AP re-enters the
guest:

  svm->vmcb->control.vmsa_pa = __pa(svm->sev_es.vmsa);

If we modify that code to instead do:

  if (!svm->sev_es.snp_has_guest_vmsa)
    svm->vmcb->control.vmsa_pa = __pa(svm->sev_es.vmsa);
      
Then it will instead continue to use the control.vmsa_pa set here in
__sev_snp_update_protected_guest_state(), in which case svm->sev_es.vmsa
will only ever be used to store the initial VMSA that was allocated by KVM.
Given that...

> 
> and wrap the __free_page() in sev_free_vcpu() with "if
> (!svm->sev_es.snp_ap_create)".

If we take the deferred approach above, then no checks are needed here
and the KVM-allocated VMSA is cleaned up the same way it is handled for
SEV-ES. SNP never needs to piggy-back off of sev_es.vmsa to pass around
VMSAs that reside in guest memory.

I can still rework things to free KVM-allocated VMSA immediately here if
you prefer but for now I have things implemented as above to keep
SEV-ES/SNP handling similar and avoid performance penalty during guest
boot. I've pushed the revised AP creation patch here for reference:

  https://github.com/mdroth/linux/commit/5a7e76231a7629ba62f8b0bba8039d93d3595ecb

Thanks for the suggestions, this all looks a good bit cleaner either way.

-Mike

> 
> This should remove the need for svm->sev_es.vmsa_pa. It is always
> equal to svm->vmcb->control.vmsa_pa anyway.
> 
> Also, it's possible to remove
> 
>    /*
>     * gmem pages aren't currently migratable, but if this ever
>     * changes then care should be taken to ensure
>     * svm->sev_es.vmsa_pa is pinned through some other means.
>     */
>    kvm_release_pfn_clean(pfn);
> 
> if sev_free_vcpu() does
> 
>    if (svm->sev_es.snp_ap_create) {
>      __free_page(virt_to_page(svm->sev_es.vmsa));
>    } else {
>      put_page(virt_to_page(svm->sev_es.vmsa));
>    }
> 
> and while at it, please reverse the polarity of snp_ap_create and
> rename it to snp_ap_created.
> 
> Paolo
> 

  parent reply	other threads:[~2024-04-17 20:57 UTC|newest]

Thread overview: 96+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-29 22:58 [PATCH v12 00/29] Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support Michael Roth
2024-03-29 22:58 ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 01/29] [TEMP] x86/kvm/Kconfig: Have KVM_AMD_SEV select ARCH_HAS_CC_PLATFORM Michael Roth
2024-03-29 22:58 ` [PATCH v12 02/29] [TEMP] x86/cc: Add cc_platform_set/_clear() helpers Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 03/29] [TEMP] x86/CPU/AMD: Track SNP host status with cc_platform_*() Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 04/29] [TEMP] fixup! KVM: SEV: sync FPU and AVX state at LAUNCH_UPDATE_VMSA time Michael Roth
2024-03-29 22:58 ` [PATCH v12 05/29] KVM: x86: Define RMP page fault error bits for #NPF Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 19:28   ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 06/29] KVM: SEV: Select KVM_GENERIC_PRIVATE_MEM when CONFIG_KVM_AMD_SEV=y Michael Roth
2024-03-29 22:58 ` [PATCH v12 07/29] KVM: SEV: Add support to handle AP reset MSR protocol Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 08/29] KVM: SEV: Add GHCB handling for Hypervisor Feature Support requests Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 09/29] KVM: SEV: Add initial SEV-SNP support Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 19:58   ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 10/29] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_START command Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 20:20   ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 11/29] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_UPDATE command Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 20:31   ` Paolo Bonzini
2024-04-01 22:22     ` Michael Roth
2024-04-02 22:58       ` Isaku Yamahata
2024-04-03 12:51         ` Paolo Bonzini
2024-04-03 15:37           ` Isaku Yamahata
2024-04-04 16:03   ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 12/29] KVM: SEV: Add KVM_SEV_SNP_LAUNCH_FINISH command Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 20:41   ` Paolo Bonzini
2024-04-01 23:17     ` Michael Roth
2024-04-03 12:56       ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 13/29] KVM: SEV: Add support to handle GHCB GPA register VMGEXIT Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 14/29] KVM: SEV: Add support to handle MSR based Page State Change VMGEXIT Michael Roth
2024-03-29 22:58 ` [PATCH v12 15/29] KVM: SEV: Add support to handle " Michael Roth
2024-03-29 22:58 ` [PATCH v12 16/29] KVM: x86: Export the kvm_zap_gfn_range() for the SNP use Michael Roth
2024-03-30 20:51   ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 17/29] KVM: SEV: Add support to handle RMP nested page faults Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 20:55   ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 18/29] KVM: SEV: Use a VMSA physical address variable for populating VMCB Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 21:01   ` Paolo Bonzini
2024-04-16 11:53     ` Paolo Bonzini
2024-04-16 14:25       ` Tom Lendacky
2024-04-16 17:00         ` Paolo Bonzini
2024-04-17 20:57       ` Michael Roth [this message]
2024-03-29 22:58 ` [PATCH v12 19/29] KVM: SEV: Support SEV-SNP AP Creation NAE event Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 20/29] KVM: SEV: Add support for GHCB-based termination requests Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 21/29] KVM: SEV: Implement gmem hook for initializing private pages Michael Roth
2024-03-30 21:05   ` Paolo Bonzini
2024-03-30 21:05     ` Paolo Bonzini
2024-03-30 21:05     ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 22/29] KVM: SEV: Implement gmem hook for invalidating " Michael Roth
2024-03-30 21:31   ` Paolo Bonzini
2024-03-30 21:31     ` Paolo Bonzini
2024-03-30 21:31     ` Paolo Bonzini
2024-04-18 19:57     ` Michael Roth
2024-03-29 22:58 ` [PATCH v12 23/29] KVM: x86: Implement gmem hook for determining max NPT mapping level Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-30 21:35   ` Paolo Bonzini
2024-03-30 21:35     ` Paolo Bonzini
2024-03-30 21:35     ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 24/29] KVM: SEV: Avoid WBINVD for HVA-based MMU notifications for SNP Michael Roth
2024-03-30 21:35   ` Paolo Bonzini
2024-03-30 21:35     ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 25/29] KVM: SVM: Add module parameter to enable the SEV-SNP Michael Roth
2024-03-30 21:35   ` Paolo Bonzini
2024-03-30 21:35     ` Paolo Bonzini
2024-03-29 22:58 ` [PATCH v12 26/29] KVM: SEV: Provide support for SNP_GUEST_REQUEST NAE event Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-04-10 22:14   ` Tom Lendacky
2024-03-29 22:58 ` [PATCH v12 27/29] crypto: ccp: Add the SNP_VLEK_LOAD command Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-04-10 22:20   ` Tom Lendacky
2024-03-29 22:58 ` [PATCH v12 28/29] crypto: ccp: Add the SNP_{PAUSE,RESUME}_ATTESTATION commands Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-04-10 22:27   ` Tom Lendacky
2024-03-29 22:58 ` [PATCH v12 29/29] KVM: SEV: Provide support for SNP_EXTENDED_GUEST_REQUEST NAE event Michael Roth
2024-03-29 22:58   ` Michael Roth
2024-04-11 13:33   ` Tom Lendacky
2024-03-30 21:44 ` [PATCH v12 00/29] Add AMD Secure Nested Paging (SEV-SNP) Hypervisor Support Paolo Bonzini
2024-03-30 21:44   ` Paolo Bonzini
2024-03-30 21:44   ` Paolo Bonzini

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=20240417205725.yougm6og3cuea2hu@amd.com \
    --to=michael.roth@amd.com \
    --cc=ak@linux.intel.com \
    --cc=alpergun@google.com \
    --cc=ardb@kernel.org \
    --cc=ashish.kalra@amd.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=dovmurik@linux.ibm.com \
    --cc=hpa@zytor.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.