From patchwork Fri Mar 11 00:25:26 2022 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Matlack X-Patchwork-Id: 1604205 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: bilbo.ozlabs.org; dkim=pass (2048-bit key; secure) header.d=lists.infradead.org header.i=@lists.infradead.org header.a=rsa-sha256 header.s=bombadil.20210309 header.b=HNxh99bL; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=google.com header.i=@google.com header.a=rsa-sha256 header.s=20210112 header.b=fkEfUCit; dkim-atps=neutral Authentication-Results: ozlabs.org; spf=none (no SPF record) smtp.mailfrom=lists.infradead.org (client-ip=2607:7c80:54:e::133; helo=bombadil.infradead.org; envelope-from=kvm-riscv-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org; receiver=) Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (No client certificate requested) by bilbo.ozlabs.org (Postfix) with ESMTPS id 4KF6DW5v7Tz9s5P for ; Fri, 11 Mar 2022 11:26:19 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:Cc:To:From:Subject:References: Mime-Version:Message-Id:In-Reply-To:Date:Reply-To:Content-ID: Content-Description:Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc :Resent-Message-ID:List-Owner; bh=RcqU4ve0QeNIxmx8q0gHbc1E9Ug8wyLCoP3gwy6ao6M=; b=HNxh99bL38lmhxiRoxETbZvNQc oN5Y/+JspGENiOeK56gVl95Oj+FELNTLY0M8f8Kp5FlUc1OOmKTTcn0CGMGYh1eFOgfiOOLKFDSdP +J53k9SpvZTBpg43/1ssFvNgMjMpE7wnMK3iyenJw0ekNGPZ/FtwHytOmUgr9FUCiQ92SfmKciQe4 e3NOa1N6OPy/TPfOPRBqA0FWyIYXjFjIgz8q+6viTQlILwHhuOVR+QAXL4Zraq3BVK2XcTHTSOmRO zp5RNY+mPoGoyzW6BrgIvAbOJtH8MPSncSPCgMLGuStR2tV0AOuh5EIgqVKftLlKVokLvAutF90sR fFgkYacg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1nST6n-00EPgL-KN; Fri, 11 Mar 2022 00:26:17 +0000 Received: from mail-pf1-x449.google.com ([2607:f8b0:4864:20::449]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1nST6j-00EPca-Ue for kvm-riscv@lists.infradead.org; Fri, 11 Mar 2022 00:26:15 +0000 Received: by mail-pf1-x449.google.com with SMTP id y27-20020aa7943b000000b004f6decccdb5so4200503pfo.1 for ; Thu, 10 Mar 2022 16:26:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20210112; h=date:in-reply-to:message-id:mime-version:references:subject:from:to :cc; bh=+N8CyLDUI8dOs6IazZEgayEpuN+Pk+7Zaf+44sPVu9M=; b=fkEfUCit5tqWvdDlI8v5ZSGJVY9seUe6cbf2whXXIZ5bwxCgzP0IqvRSWDUoHnUm0C W3XxUPolVkh0JOGVkZrBPLTgYMhmCmL3KGSLazm6lEXEdrUp9eA1e+Zxh3obazelnzBh dZq3DeFT1PL7lfJmBODoMSeUte5Fn4XfcSn9I3jPcoaKyZaVho/0yEOenJX9x6bY1C/o FG6pjk4smgKYWPjXUN1uX4ZD02cguLFEYY/MSYP3fEl3xF5comBq5sUT/YInXbYrQJMv JiMc8dEQa7jZfyLvcEOZDz+Kp9eA+3xn4embgt7o+mxQ1YtN7sOkwpiLsRTj0LtlLVPv dRCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:date:in-reply-to:message-id:mime-version :references:subject:from:to:cc; bh=+N8CyLDUI8dOs6IazZEgayEpuN+Pk+7Zaf+44sPVu9M=; b=tBBEX6UbK/0itk94g97eEgzYbkaOcx1rkKze0bGk4sQbfK+rGxXcbcysReJ5D0wgyG WR8ifQkJTkLk8CZvhKGOwNMwoJjEsLvml2GFw0thKvAx3HtznY0L23bzGbtmg4N0q6NL qzxCfATFDIinXdYpkx54PN9keHwu9+pVZtK0AceuMnDVi3BbhFdAiN2trXbFb2lkUquD d3q4n/CA9KYavt0AAXKGZqncGvII31WexC7v9ckJ3acaBLXqLjud4LMWPiOQ5loYVqBU 27kXJUlx/gQu8u90/NJVULhHwMe9i3x4GQ/VqhPDjSuaFl1Ri3rD1uUFbbFsd05hcD5H R+wQ== X-Gm-Message-State: AOAM530VZFOQvspNl6hCSminboNFGV28p62Ic7K++k8e9EoUefucQUmM kmiJDh8Lp9lCJNzMTXliCdpIPHqwwNKWWw== X-Google-Smtp-Source: ABdhPJzEurWCjkJGrlFQG7dN5CM2sI5brKP2s1Ygv17frKolnNrY7kNpKXDlaqlxOBpcKkeByFPsKnJeUISXWA== X-Received: from dmatlack-heavy.c.googlers.com ([fda3:e722:ac3:cc00:7f:e700:c0a8:19cd]) (user=dmatlack job=sendgmr) by 2002:a17:90b:4d81:b0:1bf:8ce4:4f51 with SMTP id oj1-20020a17090b4d8100b001bf8ce44f51mr322717pjb.0.1646958371853; Thu, 10 Mar 2022 16:26:11 -0800 (PST) Date: Fri, 11 Mar 2022 00:25:26 +0000 In-Reply-To: <20220311002528.2230172-1-dmatlack@google.com> Message-Id: <20220311002528.2230172-25-dmatlack@google.com> Mime-Version: 1.0 References: <20220311002528.2230172-1-dmatlack@google.com> X-Mailer: git-send-email 2.35.1.723.g4982287a31-goog Subject: [PATCH v2 24/26] KVM: x86/mmu: Split huge pages aliased by multiple SPTEs From: David Matlack To: Paolo Bonzini Cc: Marc Zyngier , Huacai Chen , Aleksandar Markovic , Anup Patel , Paul Walmsley , Palmer Dabbelt , Albert Ou , Sean Christopherson , Andrew Jones , Ben Gardon , Peter Xu , maciej.szmigiero@oracle.com, "moderated list:KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64)" , "open list:KERNEL VIRTUAL MACHINE FOR MIPS (KVM/mips)" , "open list:KERNEL VIRTUAL MACHINE FOR MIPS (KVM/mips)" , "open list:KERNEL VIRTUAL MACHINE FOR RISC-V (KVM/riscv)" , Peter Feiner , David Matlack X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20220310_162614_023273_7E307E13 X-CRM114-Status: GOOD ( 26.15 ) X-Spam-Score: -7.7 (-------) X-Spam-Report: Spam detection software, running on the system "bombadil.infradead.org", has NOT identified this incoming email as spam. The original message has been attached to this so you can view it or label similar future email. If you have any questions, see the administrator of that system for details. Content preview: The existing huge page splitting code bails if it encounters a huge page that is aliased by another SPTE that has already been split (either due to NX huge pages or eager page splitting). Extend the h [...] Content analysis details: (-7.7 points, 5.0 required) pts rule name description ---- ---------------------- -------------------------------------------------- -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at https://www.dnswl.org/, no trust [2607:f8b0:4864:20:0:0:0:449 listed in] [list.dnswl.org] -0.0 SPF_PASS SPF: sender matches SPF record 0.0 SPF_HELO_NONE SPF: HELO does not publish an SPF Record -7.5 USER_IN_DEF_DKIM_WL From: address is in the default DKIM white-list -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_EF Message has a valid DKIM or DK signature from envelope-from domain -0.0 DKIMWL_WL_MED DKIMwl.org - Medium trust sender X-BeenThere: kvm-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "kvm-riscv" Errors-To: kvm-riscv-bounces+incoming=patchwork.ozlabs.org@lists.infradead.org The existing huge page splitting code bails if it encounters a huge page that is aliased by another SPTE that has already been split (either due to NX huge pages or eager page splitting). Extend the huge page splitting code to also handle such aliases. The thing we have to be careful about is dealing with what's already in the lower level page table. If eager page splitting was the only operation that split huge pages, this would be fine. However huge pages can also be split by NX huge pages. This means the lower level page table may only be partially filled in and may point to even lower level page tables that are partially filled in. We can fill in the rest of the page table but dealing with the lower level page tables would be too complex. To handle this we flush TLBs after dropping the huge SPTE whenever we are about to install a lower level page table that was partially filled in (*). We can skip the TLB flush if the lower level page table was empty (no aliasing) or identical to what we were already going to populate it with (aliased huge page that was just eagerly split). (*) This TLB flush could probably be delayed until we're about to drop the MMU lock, which would also let us batch flushes for multiple splits. However such scenarios should be rare in practice (a huge page must be aliased in multiple SPTEs and have been split for NX Huge Pages in only some of them). Flushing immediately is simpler to plumb and also reduces the chances of tripping over a CPU bug (e.g. see iTLB multi-hit). Signed-off-by: David Matlack --- arch/x86/include/asm/kvm_host.h | 5 ++- arch/x86/kvm/mmu/mmu.c | 73 +++++++++++++++------------------ 2 files changed, 36 insertions(+), 42 deletions(-) diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h index 00a5c0bcc2eb..275d00528805 100644 --- a/arch/x86/include/asm/kvm_host.h +++ b/arch/x86/include/asm/kvm_host.h @@ -1245,9 +1245,10 @@ struct kvm_arch { * Memory cache used to allocate pte_list_desc structs while splitting * huge pages. In the worst case, to split one huge page we need 512 * pte_list_desc structs to add each new lower level leaf sptep to the - * memslot rmap. + * memslot rmap plus 1 to extend the parent_ptes rmap of the new lower + * level page table. */ -#define HUGE_PAGE_SPLIT_DESC_CACHE_CAPACITY 512 +#define HUGE_PAGE_SPLIT_DESC_CACHE_CAPACITY 513 __DEFINE_KVM_MMU_MEMORY_CACHE(huge_page_split_desc_cache, HUGE_PAGE_SPLIT_DESC_CACHE_CAPACITY); }; diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c index 95b8e2ef562f..68785b422a08 100644 --- a/arch/x86/kvm/mmu/mmu.c +++ b/arch/x86/kvm/mmu/mmu.c @@ -6208,6 +6208,7 @@ static struct kvm_mmu_page *kvm_mmu_get_sp_for_split(struct kvm *kvm, { struct kvm_mmu_page *split_sp; union kvm_mmu_page_role role; + bool created = false; unsigned int access; gfn_t gfn; @@ -6220,25 +6221,21 @@ static struct kvm_mmu_page *kvm_mmu_get_sp_for_split(struct kvm *kvm, */ role = kvm_mmu_child_role(huge_sptep, true, access); split_sp = kvm_mmu_find_direct_sp(kvm, gfn, role); - - /* - * Opt not to split if the lower-level SP already exists. This requires - * more complex handling as the SP may be already partially filled in - * and may need extra pte_list_desc structs to update parent_ptes. - */ if (split_sp) - return NULL; + goto out; + created = true; swap(split_sp, *spp); init_shadow_page(kvm, split_sp, slot, gfn, role); - trace_kvm_mmu_get_page(split_sp, true); +out: + trace_kvm_mmu_get_page(split_sp, created); return split_sp; } -static int kvm_mmu_split_huge_page(struct kvm *kvm, - const struct kvm_memory_slot *slot, - u64 *huge_sptep, struct kvm_mmu_page **spp) +static void kvm_mmu_split_huge_page(struct kvm *kvm, + const struct kvm_memory_slot *slot, + u64 *huge_sptep, struct kvm_mmu_page **spp) { struct kvm_mmu_memory_cache *cache; @@ -6246,22 +6243,11 @@ static int kvm_mmu_split_huge_page(struct kvm *kvm, u64 huge_spte, split_spte; int split_level, index; unsigned int access; + bool flush = false; u64 *split_sptep; gfn_t split_gfn; split_sp = kvm_mmu_get_sp_for_split(kvm, slot, huge_sptep, spp); - if (!split_sp) - return -EOPNOTSUPP; - - /* - * We did not allocate an extra pte_list_desc struct to add huge_sptep - * to split_sp->parent_ptes. An extra pte_list_desc struct should never - * be necessary in practice though since split_sp is brand new. - * - * Note, this makes it safe to pass NULL to __link_shadow_page() below. - */ - if (WARN_ON_ONCE(split_sp->parent_ptes.val)) - return -EINVAL; huge_spte = READ_ONCE(*huge_sptep); @@ -6273,7 +6259,20 @@ static int kvm_mmu_split_huge_page(struct kvm *kvm, split_sptep = &split_sp->spt[index]; split_gfn = kvm_mmu_page_get_gfn(split_sp, index); - BUG_ON(is_shadow_present_pte(*split_sptep)); + /* + * split_sp may have populated page table entries if this huge + * page is aliased in multiple shadow page table entries. We + * know the existing SP will be mapping the same GFN->PFN + * translation since this is a direct SP. However, the SPTE may + * point to an even lower level page table that may only be + * partially filled in (e.g. for NX huge pages). In other words, + * we may be unmapping a portion of the huge page, which + * requires a TLB flush. + */ + if (is_shadow_present_pte(*split_sptep)) { + flush |= !is_last_spte(*split_sptep, split_level); + continue; + } split_spte = make_huge_page_split_spte( huge_spte, split_level + 1, index, access); @@ -6284,15 +6283,12 @@ static int kvm_mmu_split_huge_page(struct kvm *kvm, /* * Replace the huge spte with a pointer to the populated lower level - * page table. Since we are making this change without a TLB flush vCPUs - * will see a mix of the split mappings and the original huge mapping, - * depending on what's currently in their TLB. This is fine from a - * correctness standpoint since the translation will be identical. + * page table. If the lower-level page table indentically maps the huge + * page, there's no need for a TLB flush. Otherwise, flush TLBs after + * dropping the huge page and before installing the shadow page table. */ - __drop_large_spte(kvm, huge_sptep, false); - __link_shadow_page(NULL, huge_sptep, split_sp); - - return 0; + __drop_large_spte(kvm, huge_sptep, flush); + __link_shadow_page(cache, huge_sptep, split_sp); } static bool should_split_huge_page(u64 *huge_sptep) @@ -6347,16 +6343,13 @@ static bool rmap_try_split_huge_pages(struct kvm *kvm, if (dropped_lock) goto restart; - r = kvm_mmu_split_huge_page(kvm, slot, huge_sptep, &sp); - - trace_kvm_mmu_split_huge_page(gfn, spte, level, r); - /* - * If splitting is successful we must restart the iterator - * because huge_sptep has just been removed from it. + * After splitting we must restart the iterator because + * huge_sptep has just been removed from it. */ - if (!r) - goto restart; + kvm_mmu_split_huge_page(kvm, slot, huge_sptep, &sp); + trace_kvm_mmu_split_huge_page(gfn, spte, level, 0); + goto restart; } if (sp)