Re: [PATCH V2 6/7] KVM: X86/MMU: Allocate mmu->pae_root for PAE paging on-demand

David Matlack
2022-05-17T16:57:25Z

On Tue, May 03, 2022 at 11:07:34PM +0800, Lai Jiangshan wrote:
> From: Lai Jiangshan <jiangshan.ljs@antgroup.com>
>
> mmu->pae_root for non-PAE paging is allocated on-demand, but
> mmu->pae_root for PAE paging is allocated early when struct kvm_mmu is
> being created.
>
> Simplify the code to allocate mmu->pae_root for PAE paging and make
> it on-demand.
>
> Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com>
> ---
> arch/x86/kvm/mmu/mmu.c | 99 ++++++++++++++-------------------
> arch/x86/kvm/mmu/mmu_internal.h | 10 ----
> 2 files changed, 42 insertions(+), 67 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index bcb3e2730277..c97f830c5f8c 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -691,6 +691,41 @@ static void walk_shadow_page_lockless_end(struct kvm_vcpu *vcpu)
> }
> }
>
> +static int mmu_alloc_pae_root(struct kvm_vcpu *vcpu)
> +{
> + struct page *page;
> +
> + if (vcpu->arch.mmu->root_role.level != PT32E_ROOT_LEVEL)
> + return 0;
> + if (vcpu->arch.mmu->pae_root)
> + return 0;
> +
> + /*
> + * Allocate a page to hold the four PDPTEs for PAE paging when emulating
> + * 32-bit mode. CR3 is only 32 bits even on x86_64 in this case.
> + * Therefore we need to allocate the PDP table in the first 4GB of
> + * memory, which happens to fit the DMA32 zone.
> + */
> + page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_ZERO | __GFP_DMA32);
> + if (!page)
> + return -ENOMEM;
> + vcpu->arch.mmu->pae_root = page_address(page);
> +
> + /*
> + * CR3 is only 32 bits when PAE paging is used, thus it's impossible to
> + * get the CPU to treat the PDPTEs as encrypted. Decrypt the page so
> + * that KVM's writes and the CPU's reads get along. Note, this is
> + * only necessary when using shadow paging, as 64-bit NPT can get at
> + * the C-bit even when shadowing 32-bit NPT, and SME isn't supported
> + * by 32-bit kernels (when KVM itself uses 32-bit NPT).
> + */
> + if (!tdp_enabled)
> + set_memory_decrypted((unsigned long)vcpu->arch.mmu->pae_root, 1);
> + else
> + WARN_ON_ONCE(shadow_me_value);
> + return 0;
> +}
> +
> static int mmu_topup_memory_caches(struct kvm_vcpu *vcpu, bool maybe_indirect)
> {
> int r;
> @@ -5031,6 +5066,9 @@ int kvm_mmu_load(struct kvm_vcpu *vcpu)
> r = mmu_topup_memory_caches(vcpu, !vcpu->arch.mmu->root_role.direct);
> if (r)
> goto out;
> + r = mmu_alloc_pae_root(vcpu);
> + if (r)
> + return r;
> r = mmu_alloc_special_roots(vcpu);
> if (r)
> goto out;
> @@ -5495,63 +5533,18 @@ static void free_mmu_pages(struct kvm_mmu *mmu)
> free_page((unsigned long)mmu->pml5_root);
> }
>
> -static int __kvm_mmu_create(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)
> +static void __kvm_mmu_create(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu)

vcpu is now unused.

> {
> - struct page *page;
> int i;
>
> mmu->root.hpa = INVALID_PAGE;
> mmu->root.pgd = 0;
> for (i = 0; i < KVM_MMU_NUM_PREV_ROOTS; i++)
> mmu->prev_roots[i] = KVM_MMU_ROOT_INFO_INVALID;

optional: Consider open-coding this directly in kvm_mmu_create() and
drop __kvm_mmu_create().

> -
> - /* vcpu->arch.guest_mmu isn't used when !tdp_enabled. */
> - if (!tdp_enabled && mmu == &vcpu->arch.guest_mmu)
> - return 0;
> -
> - /*
> - * When using PAE paging, the four PDPTEs are treated as 'root' pages,
> - * 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. 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_special_roots().
> - */
> - if (tdp_enabled && kvm_mmu_get_tdp_level(vcpu) > PT32E_ROOT_LEVEL)
> - return 0;
> -
> - page = alloc_page(GFP_KERNEL_ACCOUNT | __GFP_DMA32);
> - if (!page)
> - return -ENOMEM;
> -
> - mmu->pae_root = page_address(page);
> -
> - /*
> - * CR3 is only 32 bits when PAE paging is used, thus it's impossible to
> - * get the CPU to treat the PDPTEs as encrypted. Decrypt the page so
> - * that KVM's writes and the CPU's reads get along. Note, this is
> - * only necessary when using shadow paging, as 64-bit NPT can get at
> - * the C-bit even when shadowing 32-bit NPT, and SME isn't supported
> - * by 32-bit kernels (when KVM itself uses 32-bit NPT).
> - */
> - if (!tdp_enabled)
> - set_memory_decrypted((unsigned long)mmu->pae_root, 1);
> - else
> - WARN_ON_ONCE(shadow_me_value);
> -
> - for (i = 0; i < 4; ++i)
> - mmu->pae_root[i] = INVALID_PAE_ROOT;
> -
> - return 0;
> }
>
> int kvm_mmu_create(struct kvm_vcpu *vcpu)

kvm_mmu_create() could return void now too.

> {
> - int ret;
> -
> vcpu->arch.mmu_pte_list_desc_cache.kmem_cache = pte_list_desc_cache;
> vcpu->arch.mmu_pte_list_desc_cache.gfp_zero = __GFP_ZERO;
>
> @@ -5563,18 +5556,10 @@ int kvm_mmu_create(struct kvm_vcpu *vcpu)
> vcpu->arch.mmu = &vcpu->arch.root_mmu;
> vcpu->arch.walk_mmu = &vcpu->arch.root_mmu;
>
> - ret = __kvm_mmu_create(vcpu, &vcpu->arch.guest_mmu);
> - if (ret)
> - return ret;
> -
> - ret = __kvm_mmu_create(vcpu, &vcpu->arch.root_mmu);
> - if (ret)
> - goto fail_allocate_root;
> + __kvm_mmu_create(vcpu, &vcpu->arch.guest_mmu);
> + __kvm_mmu_create(vcpu, &vcpu->arch.root_mmu);
>
> - return ret;
> - fail_allocate_root:
> - free_mmu_pages(&vcpu->arch.guest_mmu);
> - return ret;
> + return 0;
> }
>
> #define BATCH_ZAP_PAGES 10
> diff --git a/arch/x86/kvm/mmu/mmu_internal.h b/arch/x86/kvm/mmu/mmu_internal.h
> index 1bff453f7cbe..d5673a42680f 100644
> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -20,16 +20,6 @@ extern bool dbg;
> #define MMU_WARN_ON(x) do { } while (0)
> #endif
>
> -/*
> - * Unlike regular MMU roots, PAE "roots", a.k.a. PDPTEs/PDPTRs, have a PRESENT
> - * bit, and thus are guaranteed to be non-zero when valid. And, when a guest
> - * PDPTR is !PRESENT, its corresponding PAE root cannot be set to INVALID_PAGE,
> - * as the CPU would treat that as PRESENT PDPTR with reserved bits set. Use
> - * '0' instead of INVALID_PAGE to indicate an invalid PAE root.
> - */
> -#define INVALID_PAE_ROOT 0
> -#define IS_VALID_PAE_ROOT(x) (!!(x))
> -
> typedef u64 __rcu *tdp_ptep_t;
>
> struct kvm_mmu_page {
> --
> 2.19.1.6.gb485710b
>