| 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
| ||
|
Message-ID: <4eae43fb-28f8-4e84-afe1-812b71f890d4@amd.com>
Date: Tue, 22 Oct 2024 20:18:17 -0500
From: Mike Day <michael.day@....com>
To: Elliot Berman <quic_eberman@...cinc.com>,
Andrew Morton <akpm@...ux-foundation.org>,
Sean Christopherson <seanjc@...gle.com>, Paolo Bonzini
<pbonzini@...hat.com>, Thomas Gleixner <tglx@...utronix.de>,
Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
Dave Hansen <dave.hansen@...ux.intel.com>, Fuad Tabba <tabba@...gle.com>,
David Hildenbrand <david@...hat.com>, Patrick Roy <roypat@...zon.co.uk>,
qperret@...gle.com, Ackerley Tng <ackerleytng@...gle.com>,
Mike Rapoport <rppt@...nel.org>, x86@...nel.org,
"H. Peter Anvin" <hpa@...or.com>
Cc: linux-kernel@...r.kernel.org, linux-mm@...ck.org, kvm@...r.kernel.org,
linux-coco@...ts.linux.dev, linux-arm-msm@...r.kernel.org
Subject: Re: [PATCH RFC v2 3/5] kvm: Convert to use guest_memfd library
On 8/29/24 17:24, Elliot Berman wrote:
> Use the recently created mm/guest_memfd implementation. No functional
> change intended.
>
> Note: I've only compile-tested this. Appreciate some help from SEV folks
> to be able to test this.
Is there an updated patchset?
>
> Signed-off-by: Elliot Berman <quic_eberman@...cinc.com>
> ---
> arch/x86/kvm/svm/sev.c | 3 +-
> virt/kvm/Kconfig | 1 +
> virt/kvm/guest_memfd.c | 371 ++++++++++---------------------------------------
> virt/kvm/kvm_main.c | 2 -
> virt/kvm/kvm_mm.h | 6 -
> 5 files changed, 77 insertions(+), 306 deletions(-)
>
> diff --git a/arch/x86/kvm/svm/sev.c b/arch/x86/kvm/svm/sev.c
> index 714c517dd4b72..f3a6857270943 100644
> --- a/arch/x86/kvm/svm/sev.c
> +++ b/arch/x86/kvm/svm/sev.c
> @@ -2297,8 +2297,7 @@ static int sev_gmem_post_populate(struct kvm *kvm, gfn_t gfn_start, kvm_pfn_t pf
> kunmap_local(vaddr);
> }
>
> - ret = rmp_make_private(pfn + i, gfn << PAGE_SHIFT, PG_LEVEL_4K,
> - sev_get_asid(kvm), true);
Need to keep rmp_make_private(), it is updating firmware reverse mapping (RMP) to assign the folio to the
guest. Would be used in combination with guest_memfd_make_inaccessible(), but that call cannot be
made from here, needs to move elsewhere.
> +static inline struct kvm_gmem *inode_to_kvm_gmem(struct inode *inode)
> +{
> + struct list_head *gmem_list = &inode->i_mapping->i_private_list;
> +
> + return list_first_entry_or_null(gmem_list, struct kvm_gmem, entry);
gmem SEV-SNP guests end up creating multiple struct kvm_gmem objects per guest, each one having
different memory slots. So this will not always return the correct gmem object for an SEV-SNP guest.
> +}
> +
> -static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
> - pgoff_t index, struct folio *folio)
> +static int kvm_gmem_prepare_inaccessible(struct inode *inode, struct folio *folio)
> {
> #ifdef CONFIG_HAVE_KVM_ARCH_GMEM_PREPARE
> - kvm_pfn_t pfn = folio_file_pfn(folio, index);
> - gfn_t gfn = slot->base_gfn + index - slot->gmem.pgoff;
> + kvm_pfn_t pfn = folio_file_pfn(folio, 0);
> + gfn_t gfn = slot->base_gfn + folio_index(folio) - slot->gmem.pgoff;
There is no longer a struct kvm_memory_slot * in the prototype, so this won't compile. It creates
an impedence mismatch with the way kvm gmem calls prepare_folio() on SEV-SNP.
> int rc = kvm_arch_gmem_prepare(kvm, gfn, pfn, folio_order(folio));
> if (rc) {
> pr_warn_ratelimited("gmem: Failed to prepare folio for index %lx GFN %llx PFN %llx error %d.\n",
> @@ -42,67 +46,7 @@ static int __kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slo
> return 0;
> }
>
> -static inline void kvm_gmem_mark_prepared(struct folio *folio)
> -{
> - folio_mark_uptodate(folio);
> -}
mark_prepared takes on additional meaning with SEV-SNP beyond uptodate, although this
could be separated into a different state. "preparation" includes setting the Reverse MaPping (RMP)
assigned bit - it eventually ends up in the sev code making and RMP assignment and
clearing the folio (from :/arch/x86/kvm/svm/sev.c)
if (!folio_test_uptodate(folio)) {
unsigned long nr_pages = level == PG_LEVEL_4K ? 1 : 512;
int i;
pr_debug("%s: folio not up-to-date, clearing folio pages.\n", __func__);
for (i = 0; i < nr_pages; i++)
clear_highpage(pfn_to_page(pfn_aligned + i));
{mark|test}_uptodate is still intertwined with the architectural code, probably should be
disentangled in favor of "prepare."
> -
> -/*
> - * Process @folio, which contains @gfn, so that the guest can use it.
> - * The folio must be locked and the gfn must be contained in @slot.
> - * On successful return the guest sees a zero page so as to avoid
> - * leaking host data and the up-to-date flag is set.
> - */
> -static int kvm_gmem_prepare_folio(struct kvm *kvm, struct kvm_memory_slot *slot,
> - gfn_t gfn, struct folio *folio)
>
Is it correct that gmem->prepare_inaccessible() is the direct analogue to
kvm_gmem_prepare_folio?
> -#ifdef CONFIG_HAVE_KVM_ARCH_GMEM_INVALIDATE
> -static void kvm_gmem_free_folio(struct folio *folio)
> -{
> - struct page *page = folio_page(folio, 0);
> - kvm_pfn_t pfn = page_to_pfn(page);
> - int order = folio_order(folio);
> -
> - kvm_arch_gmem_invalidate(pfn, pfn + (1ul << order));
kvm_arch_gmem_invalidate() is necessary for gmem SEV-SNP - it calls sev_gmem_invalidate()
which performs RMP modifications and flushes caches. When a guest page is split or released
these operations must occur.
> @@ -656,19 +444,12 @@ long kvm_gmem_populate(struct kvm *kvm, gfn_t start_gfn, void __user *src, long
> break;
> }
>
> - folio = __kvm_gmem_get_pfn(file, slot, gfn, &pfn, &is_prepared, &max_order);
> + folio = __kvm_gmem_get_pfn(file, slot, gfn, &pfn, true, &max_order);
probably need to retain a check _is_prepared() here instead of always declaring the folio prepared.
thanks,
Mike
Powered by blists - more mailing lists