| 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: <20240423235013.GO3596705@ls.amr.corp.intel.com>
Date: Tue, 23 Apr 2024 16:50:13 -0700
From: Isaku Yamahata <isaku.yamahata@...el.com>
To: Paolo Bonzini <pbonzini@...hat.com>
Cc: linux-kernel@...r.kernel.org, kvm@...r.kernel.org, seanjc@...gle.com,
michael.roth@....com, isaku.yamahata@...el.com,
isaku.yamahata@...ux.intel.com
Subject: Re: [PATCH 09/11] KVM: guest_memfd: Add interface for populating
gmem pages with user data
On Thu, Apr 04, 2024 at 02:50:31PM -0400,
Paolo Bonzini <pbonzini@...hat.com> wrote:
> During guest run-time, kvm_arch_gmem_prepare() is issued as needed to
> prepare newly-allocated gmem pages prior to mapping them into the guest.
> In the case of SEV-SNP, this mainly involves setting the pages to
> private in the RMP table.
>
> However, for the GPA ranges comprising the initial guest payload, which
> are encrypted/measured prior to starting the guest, the gmem pages need
> to be accessed prior to setting them to private in the RMP table so they
> can be initialized with the userspace-provided data. Additionally, an
> SNP firmware call is needed afterward to encrypt them in-place and
> measure the contents into the guest's launch digest.
>
> While it is possible to bypass the kvm_arch_gmem_prepare() hooks so that
> this handling can be done in an open-coded/vendor-specific manner, this
> may expose more gmem-internal state/dependencies to external callers
> than necessary. Try to avoid this by implementing an interface that
> tries to handle as much of the common functionality inside gmem as
> possible, while also making it generic enough to potentially be
> usable/extensible for TDX as well.
I explored how TDX will use this hook. However, it resulted in not using this
hook, and instead used kvm_tdp_mmu_get_walk() with a twist. The patch is below.
Because SEV-SNP manages the RMP that is not tied to NPT directly, SEV-SNP can
ignore TDP MMU page tables when updating RMP.
On the other hand, TDX essentially updates Secure-EPT when it adds a page to
the guest by TDH.MEM.PAGE.ADD(). It needs to protect KVM TDP MMU page tables
with mmu_lock, not guest memfd file mapping with invalidate_lock. The hook
doesn't apply to TDX well. The resulted KVM_TDX_INIT_MEM_REGION logic is as
follows.
get_user_pages_fast(source addr)
read_lock(mmu_lock)
kvm_tdp_mmu_get_walk_private_pfn(vcpu, gpa, &pfn);
if the page table doesn't map gpa, error.
TDH.MEM.PAGE.ADD()
TDH.MR.EXTEND()
read_unlock(mmu_lock)
put_page()
>From 7d4024049b51969a2431805c2117992fc7ec0981 Mon Sep 17 00:00:00 2001
Message-ID: <7d4024049b51969a2431805c2117992fc7ec0981.1713913379.git.isaku.yamahata@...el.com>
In-Reply-To: <cover.1713913379.git.isaku.yamahata@...el.com>
References: <cover.1713913379.git.isaku.yamahata@...el.com>
From: Isaku Yamahata <isaku.yamahata@...el.com>
Date: Tue, 23 Apr 2024 11:33:44 -0700
Subject: [PATCH] KVM: x86/tdp_mmu: Add a helper function to walk down the TDP MMU
KVM_TDX_INIT_MEM_REGION needs to check if the given GFN is already
populated. Add wrapping logic to kvm_tdp_mmu_get_walk() to export it.
Alternatives are as follows. Choose the approach of this patch as the
least intrusive change.
- Refactor kvm page fault handler. Populating part and unlock function.
The page fault handler to populate with keeping lock, TDH.MEM.PAGE.ADD(),
unlock.
- Add a callback function to struct kvm_page_fault and call it
after the page fault handler before unlocking mmu_lock and releasing PFN.
Based on the feedback of
https://lore.kernel.org/kvm/ZfBkle1eZFfjPI8l@google.com/
https://lore.kernel.org/kvm/Zh8DHbb8FzoVErgX@google.com/
Signed-off-by: Isaku Yamahata <isaku.yamahata@...el.com>
---
arch/x86/kvm/mmu.h | 3 +++
arch/x86/kvm/mmu/tdp_mmu.c | 44 ++++++++++++++++++++++++++++++++------
2 files changed, 40 insertions(+), 7 deletions(-)
diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
index 712e9408f634..4f61f4b9fd64 100644
--- a/arch/x86/kvm/mmu.h
+++ b/arch/x86/kvm/mmu.h
@@ -287,6 +287,9 @@ extern bool tdp_mmu_enabled;
#define tdp_mmu_enabled false
#endif
+int kvm_tdp_mmu_get_walk_private_pfn(struct kvm_vcpu *vcpu, u64 gpa,
+ kvm_pfn_t *pfn);
+
static inline bool kvm_memslots_have_rmaps(struct kvm *kvm)
{
return !tdp_mmu_enabled || kvm_shadow_root_allocated(kvm);
diff --git a/arch/x86/kvm/mmu/tdp_mmu.c b/arch/x86/kvm/mmu/tdp_mmu.c
index 3592ae4e485f..bafcd8aeb3b3 100644
--- a/arch/x86/kvm/mmu/tdp_mmu.c
+++ b/arch/x86/kvm/mmu/tdp_mmu.c
@@ -2035,14 +2035,25 @@ bool kvm_tdp_mmu_write_protect_gfn(struct kvm *kvm,
*
* Must be called between kvm_tdp_mmu_walk_lockless_{begin,end}.
*/
-int kvm_tdp_mmu_get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes,
- int *root_level)
+static int __kvm_tdp_mmu_get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes,
+ bool is_private)
{
struct tdp_iter iter;
struct kvm_mmu *mmu = vcpu->arch.mmu;
gfn_t gfn = addr >> PAGE_SHIFT;
int leaf = -1;
+ tdp_mmu_for_each_pte(iter, mmu, is_private, gfn, gfn + 1) {
+ leaf = iter.level;
+ sptes[leaf] = iter.old_spte;
+ }
+
+ return leaf;
+}
+
+int kvm_tdp_mmu_get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes,
+ int *root_level)
+{
*root_level = vcpu->arch.mmu->root_role.level;
/*
@@ -2050,15 +2061,34 @@ int kvm_tdp_mmu_get_walk(struct kvm_vcpu *vcpu, u64 addr, u64 *sptes,
* instructions in protected guest memory can't be parsed by VMM.
*/
if (WARN_ON_ONCE(kvm_gfn_shared_mask(vcpu->kvm)))
- return leaf;
+ return -1;
- tdp_mmu_for_each_pte(iter, mmu, false, gfn, gfn + 1) {
- leaf = iter.level;
- sptes[leaf] = iter.old_spte;
+ return __kvm_tdp_mmu_get_walk(vcpu, addr, sptes, false);
+}
+
+int kvm_tdp_mmu_get_walk_private_pfn(struct kvm_vcpu *vcpu, u64 gpa,
+ kvm_pfn_t *pfn)
+{
+ u64 sptes[PT64_ROOT_MAX_LEVEL + 1], spte;
+ int leaf;
+
+ lockdep_assert_held(&vcpu->kvm->mmu_lock);
+
+ kvm_tdp_mmu_walk_lockless_begin();
+ leaf = __kvm_tdp_mmu_get_walk(vcpu, gpa, sptes, true);
+ kvm_tdp_mmu_walk_lockless_end();
+ if (leaf < 0)
+ return -ENOENT;
+
+ spte = sptes[leaf];
+ if (is_shadow_present_pte(spte) && is_last_spte(spte, leaf)) {
+ *pfn = spte_to_pfn(spte);
+ return leaf;
}
- return leaf;
+ return -ENOENT;
}
+EXPORT_SYMBOL_GPL(kvm_tdp_mmu_get_walk_private_pfn);
/*
* Returns the last level spte pointer of the shadow page walk for the given
--
2.43.2
--
Isaku Yamahata <isaku.yamahata@...el.com>
Powered by blists - more mailing lists