Kernel KVM virtualization development
 help / color / mirror / Atom feed
From: Sean Christopherson <seanjc@google.com>
To: Hyunwoo Kim <imv4bel@gmail.com>
Cc: Michael Roth <michael.roth@amd.com>,
	pbonzini@redhat.com, tglx@kernel.org,  mingo@redhat.com,
	bp@alien8.de, dave.hansen@linux.intel.com, x86@kernel.org,
	 hpa@zytor.com, kvm@vger.kernel.org
Subject: Re: [PATCH] KVM: SEV: Don't return a still-assigned gmem page to the host
Date: Thu, 11 Jun 2026 05:47:52 -0700	[thread overview]
Message-ID: <aiqueNUndQsbMNud@google.com> (raw)
In-Reply-To: <aiqNPBQzoU9f8RwI@v4bel>

On Thu, Jun 11, 2026, Hyunwoo Kim wrote:
> On Wed, Jun 10, 2026 at 05:16:57PM -0500, Michael Roth wrote:
> > On Thu, Jun 11, 2026 at 01:10:03AM +0900, Hyunwoo Kim wrote:
> > > ---
> > >  arch/x86/kvm/svm/sev.c | 6 +++++-
> > >  1 file changed, 5 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> > > index 6c6a6d663e29..8fee6ec529f9 100644
> > > --- a/arch/x86/kvm/svm/sev.c
> > > +++ b/arch/x86/kvm/svm/sev.c
> > > @@ -5178,8 +5178,12 @@ void sev_gmem_invalidate(kvm_pfn_t start, kvm_pfn_t end)
> > > 
> > >                 rc = rmp_make_shared(pfn, use_2m_update ? PG_LEVEL_2M : PG_LEVEL_4K);
> > >                 if (WARN_ONCE(rc, "SEV: Failed to update RMP entry for PFN 0x%llx error %d\n",
> > > -                             pfn, rc))
> > > +                             pfn, rc)) {
> > > +                       /* Still assigned to the guest; pin and leak rather than freeing. */
> > > +                       folio_get(page_folio(pfn_to_page(pfn)));
> > > +                       snp_leak_pages(pfn, use_2m_update ? PTRS_PER_PMD : 1);
> > >                         goto next_pfn;
> > > +               }
> > 
> > This roughly aligns with what would happen if snp_page_reclaim() fails
> > in sev_gmem_post_populate(), while the guest is being initialized via
> > KVM_SEV_SNP_LAUNCH_UPDATE ioctl, which calls into kvm_gmem_populate().
> > 
> > However, in kvm_gmem_populate(), we still free the page. Maybe, to
> > address both cases, we should just add a parameter to snp_leak_pages()
> > to tell it to take an extra ref and use that in both of these paths.
> > 
> > Or we can just do the direct folio_get() in both cases, the above
> > formalizes the handling convention a little better though IMO.
> 
> If I understand correctly, an extra ref alone still seems to leave the
> LRU corruption that sashiko flagged:
> 
> https://lore.kernel.org/all/20260610162623.061BA1F00898@smtp.kernel.org/
> 
> A gmem folio is on the unevictable LRU, and taking a ref keeps the folio
> on the LRU. page->buddy_list, which snp_leak_pages() uses, shares the
> same union as folio->lru, so leaking the page overwrites the folio's LRU
> pointers. Both paths deal with a gmem folio, so the same applies.
> 
> To handle this properly, the folio would need to be taken off the LRU
> before leaking, with something like folio_isolate_lru(), but that is
> mm-internal and does not look usable from KVM. How should we proceed?
> Please let me know if I am missing something.

I'm inclined to do nothing.  rmp_make_shared() should only fail in this case if
there's a fatal bug somewhere, no?  Either that or do BUG_ON(), because at some
point these types of errors are simply unrecoverable.

  reply	other threads:[~2026-06-11 12:47 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-06-10 16:10 [PATCH] KVM: SEV: Don't return a still-assigned gmem page to the host Hyunwoo Kim
2026-06-10 16:26 ` sashiko-bot
2026-06-10 18:25   ` Sean Christopherson
2026-06-10 22:16 ` Michael Roth
2026-06-11 10:26   ` Hyunwoo Kim
2026-06-11 12:47     ` Sean Christopherson [this message]
2026-06-11 14:05       ` Hyunwoo Kim
2026-06-11 15:23         ` Sean Christopherson
2026-06-11 17:07           ` Hyunwoo Kim
     [not found]             ` <airxMoy44ZxkbioH@google.com>
2026-06-11 17:34               ` Hyunwoo Kim
2026-06-11 23:15           ` Michael Roth
2026-06-12  0:10             ` Sean Christopherson
2026-06-11 16:47     ` Michael Roth

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=aiqueNUndQsbMNud@google.com \
    --to=seanjc@google.com \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=hpa@zytor.com \
    --cc=imv4bel@gmail.com \
    --cc=kvm@vger.kernel.org \
    --cc=michael.roth@amd.com \
    --cc=mingo@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=tglx@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