From: Ben Gardon <bgardon@google.com>
To: Sean Christopherson <seanjc@google.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>,
Vitaly Kuznetsov <vkuznets@redhat.com>,
Wanpeng Li <wanpengli@tencent.com>,
Jim Mattson <jmattson@google.com>, Joerg Roedel <joro@8bytes.org>,
kvm <kvm@vger.kernel.org>, LKML <linux-kernel@vger.kernel.org>,
Brijesh Singh <brijesh.singh@amd.com>,
Tom Lendacky <thomas.lendacky@amd.com>
Subject: Re: [PATCH 02/15] KVM: x86/mmu: Alloc page for PDPTEs when shadowing 32-bit NPT with 64-bit
Date: Wed, 3 Mar 2021 09:28:03 -0800 [thread overview]
Message-ID: <CANgfPd9n3HjFOR5230i9_W9-CZjKKQSp+wzDB+Eymqrr3F8xeQ@mail.gmail.com> (raw)
In-Reply-To: <20210302184540.2829328-3-seanjc@google.com>
On Tue, Mar 2, 2021 at 10:45 AM Sean Christopherson <seanjc@google.com> wrote:
>
> Allocate the so called pae_root page on-demand, along with the lm_root
> page, when shadowing 32-bit NPT with 64-bit NPT, i.e. when running a
> 32-bit L1. KVM currently only allocates the page when NPT is disabled,
> or when L0 is 32-bit (using PAE paging).
>
> Note, there is an existing memory leak involving the MMU roots, as KVM
> fails to free the PAE roots on failure. This will be addressed in a
> future commit.
>
> Fixes: ee6268ba3a68 ("KVM: x86: Skip pae_root shadow allocation if tdp enabled")
> Fixes: b6b80c78af83 ("KVM: x86/mmu: Allocate PAE root array when using SVM's 32-bit NPT")
> Cc: stable@vger.kernel.org
> Signed-off-by: Sean Christopherson <seanjc@google.com>
Reviewed-by: Ben Gardon <bgardon@google.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 44 ++++++++++++++++++++++++++++--------------
> 1 file changed, 29 insertions(+), 15 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 0987cc1d53eb..2ed3fac1244e 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -3187,14 +3187,14 @@ void kvm_mmu_free_roots(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
> if (mmu->shadow_root_level >= PT64_ROOT_4LEVEL &&
> (mmu->root_level >= PT64_ROOT_4LEVEL || mmu->direct_map)) {
> mmu_free_root_page(kvm, &mmu->root_hpa, &invalid_list);
> - } else {
> + } else if (mmu->pae_root) {
> for (i = 0; i < 4; ++i)
> if (mmu->pae_root[i] != 0)
I was about to comment on how weird this check is since pae_root can
also be INVALID_PAGE but that case is handled in mmu_free_root_page...
but then I realized that you're already addressing that problem in
patch 7.
> mmu_free_root_page(kvm,
> &mmu->pae_root[i],
> &invalid_list);
> - mmu->root_hpa = INVALID_PAGE;
> }
> + mmu->root_hpa = INVALID_PAGE;
> mmu->root_pgd = 0;
> }
>
> @@ -3306,9 +3306,23 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
> * the shadow page table may be a PAE or a long mode page table.
> */
> pm_mask = PT_PRESENT_MASK;
> - if (vcpu->arch.mmu->shadow_root_level == PT64_ROOT_4LEVEL)
> + if (vcpu->arch.mmu->shadow_root_level == PT64_ROOT_4LEVEL) {
> pm_mask |= PT_ACCESSED_MASK | PT_WRITABLE_MASK | PT_USER_MASK;
>
> + /*
> + * Allocate the page for the PDPTEs when shadowing 32-bit NPT
> + * with 64-bit only when needed. Unlike 32-bit NPT, it doesn't
> + * need to be in low mem. See also lm_root below.
> + */
> + if (!vcpu->arch.mmu->pae_root) {
> + WARN_ON_ONCE(!tdp_enabled);
> +
> + vcpu->arch.mmu->pae_root = (void *)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> + if (!vcpu->arch.mmu->pae_root)
> + return -ENOMEM;
> + }
> + }
> +
> for (i = 0; i < 4; ++i) {
> MMU_WARN_ON(VALID_PAGE(vcpu->arch.mmu->pae_root[i]));
> if (vcpu->arch.mmu->root_level == PT32E_ROOT_LEVEL) {
> @@ -3331,21 +3345,19 @@ static int mmu_alloc_shadow_roots(struct kvm_vcpu *vcpu)
> vcpu->arch.mmu->root_hpa = __pa(vcpu->arch.mmu->pae_root);
>
> /*
> - * If we shadow a 32 bit page table with a long mode page
> - * table we enter this path.
> + * When shadowing 32-bit or PAE NPT with 64-bit NPT, the PML4 and PDP
> + * tables are allocated and initialized at MMU creation as there is no
> + * equivalent level in the guest's NPT to shadow. Allocate the tables
> + * on demand, as running a 32-bit L1 VMM is very rare. The PDP is
> + * handled above (to share logic with PAE), deal with the PML4 here.
> */
> if (vcpu->arch.mmu->shadow_root_level == PT64_ROOT_4LEVEL) {
> if (vcpu->arch.mmu->lm_root == NULL) {
> - /*
> - * The additional page necessary for this is only
> - * allocated on demand.
> - */
> -
> u64 *lm_root;
>
> lm_root = (void*)get_zeroed_page(GFP_KERNEL_ACCOUNT);
> - if (lm_root == NULL)
> - return 1;
> + if (!lm_root)
> + return -ENOMEM;
>
> lm_root[0] = __pa(vcpu->arch.mmu->pae_root) | pm_mask;
>
> @@ -5248,9 +5260,11 @@ static int __kvm_mmu_create(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
> * while the PDP table is a per-vCPU construct that's allocated at MMU
> * creation. When emulating 32-bit mode, cr3 is only 32 bits even on
> * x86_64. Therefore we need to allocate the PDP table in the first
> - * 4GB of memory, which happens to fit the DMA32 zone. Except for
> - * SVM's 32-bit NPT support, TDP paging doesn't use PAE paging and can
> - * skip allocating the PDP table.
> + * 4GB of memory, which happens to fit the DMA32 zone. TDP paging
> + * generally doesn't use PAE paging and can skip allocating the PDP
> + * table. The main exception, handled here, is SVM's 32-bit NPT. The
> + * other exception is for shadowing L1's 32-bit or PAE NPT on 64-bit
> + * KVM; that horror is handled on-demand by mmu_alloc_shadow_roots().
> */
> if (tdp_enabled && kvm_mmu_get_tdp_level(vcpu) > PT32E_ROOT_LEVEL)
> return 0;
> --
> 2.30.1.766.gb4fecdf3b7-goog
>
next prev parent reply other threads:[~2021-03-03 20:56 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2021-03-02 18:45 [PATCH 00/15] KVM: x86/mmu: Lots of bug fixes Sean Christopherson
2021-03-02 18:45 ` [PATCH 01/15] KVM: nSVM: Set the shadow root level to the TDP level for nested NPT Sean Christopherson
2021-03-02 18:45 ` [PATCH 02/15] KVM: x86/mmu: Alloc page for PDPTEs when shadowing 32-bit NPT with 64-bit Sean Christopherson
2021-03-03 17:28 ` Ben Gardon [this message]
2021-03-02 18:45 ` [PATCH 03/15] KVM: x86/mmu: Ensure MMU pages are available when allocating roots Sean Christopherson
2021-03-03 0:21 ` Ben Gardon
2021-03-03 16:46 ` Sean Christopherson
2021-03-02 18:45 ` [PATCH 04/15] KVM: x86/mmu: Allocate the lm_root before allocating PAE roots Sean Christopherson
2021-03-02 18:45 ` [PATCH 05/15] KVM: x86/mmu: Check PDPTRs " Sean Christopherson
2021-03-02 18:45 ` [PATCH 06/15] KVM: x86/mmu: Fix and unconditionally enable WARNs to detect PAE leaks Sean Christopherson
2021-03-02 18:45 ` [PATCH 07/15] KVM: x86/mmu: Use '0' as the one and only value for an invalid PAE root Sean Christopherson
2021-03-02 18:45 ` [PATCH 08/15] KVM: x86/mmu: Set the C-bit in the PDPTRs and LM pseudo-PDPTRs Sean Christopherson
2021-03-02 18:45 ` [PATCH 09/15] KVM: x86/mmu: Mark the PAE roots as decrypted for shadow paging Sean Christopherson
2021-03-02 18:45 ` [PATCH 10/15] KVM: SVM: Don't strip the C-bit from CR2 on #PF interception Sean Christopherson
2021-03-02 18:45 ` [PATCH 11/15] KVM: nVMX: Defer the MMU reload to the normal path on an EPTP switch Sean Christopherson
2021-03-02 18:45 ` [PATCH 12/15] KVM: x86: Defer the MMU unload to the normal path on an global INVPCID Sean Christopherson
2021-03-02 18:45 ` [PATCH 13/15] KVM: x86/mmu: Unexport MMU load/unload functions Sean Christopherson
2021-03-02 18:45 ` [PATCH 14/15] KVM: x86/mmu: Sync roots after MMU load iff load as successful Sean Christopherson
2021-03-02 18:45 ` [PATCH 15/15] KVM: x86/mmu: WARN on NULL pae_root and bad shadow root level Sean Christopherson
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=CANgfPd9n3HjFOR5230i9_W9-CZjKKQSp+wzDB+Eymqrr3F8xeQ@mail.gmail.com \
--to=bgardon@google.com \
--cc=brijesh.singh@amd.com \
--cc=jmattson@google.com \
--cc=joro@8bytes.org \
--cc=kvm@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=pbonzini@redhat.com \
--cc=seanjc@google.com \
--cc=thomas.lendacky@amd.com \
--cc=vkuznets@redhat.com \
--cc=wanpengli@tencent.com \
/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;
as well as URLs for NNTP newsgroup(s).