From: Lai Jiangshan <[email protected]>
Allocate the tables when allocating the local shadow page.
Signed-off-by: Lai Jiangshan <[email protected]>
---
arch/x86/kvm/mmu/mmu.c | 17 +++++++++--------
1 file changed, 9 insertions(+), 8 deletions(-)
diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
index 63c2b2c6122c..73e6a8e1e1a9 100644
--- a/arch/x86/kvm/mmu/mmu.c
+++ b/arch/x86/kvm/mmu/mmu.c
@@ -1809,10 +1809,12 @@ static bool using_local_root_page(struct kvm_mmu *mmu)
* 2 or 3 levels of local shadow pages on top of non-local shadow pages.
*
* Local shadow pages are locally allocated. If the local shadow page's level
- * is PT32E_ROOT_LEVEL, it will use the preallocated mmu->pae_root for its
- * sp->spt. Because sp->spt may need to be put in the 32 bits CR3 (even in
- * x86_64) or decrypted. Using the preallocated one to handle these
- * requirements makes the allocation simpler.
+ * is PT32E_ROOT_LEVEL, and it is not shadowing nested NPT for 32-bit L1 in
+ * 64-bit L0 (or said when the shadow pagetable's level is PT32E_ROOT_LEVEL),
+ * it will use the preallocated mmu->pae_root for its sp->spt. Because sp->spt
+ * need to be put in the 32-bit CR3 (even in 64-bit host) or decrypted. Using
+ * the preallocated one to handle these requirements makes the allocation
+ * simpler.
*
* Local shadow pages are only visible to local VCPU except through
* sp->parent_ptes rmap from their children, so they are not in the
@@ -1852,13 +1854,12 @@ kvm_mmu_alloc_local_shadow_page(struct kvm_vcpu *vcpu, union kvm_mmu_page_role r
sp->gfn = 0;
sp->role = role;
/*
- * Use the preallocated mmu->pae_root when the shadow page's
- * level is PT32E_ROOT_LEVEL which may need to be put in the 32 bits
+ * Use the preallocated mmu->pae_root when the shadow pagetable's
+ * level is PT32E_ROOT_LEVEL which need to be put in the 32 bits
* CR3 (even in x86_64) or decrypted. The preallocated one is prepared
* for the requirements.
*/
- if (role.level == PT32E_ROOT_LEVEL &&
- !WARN_ON_ONCE(!vcpu->arch.mmu->pae_root))
+ if (vcpu->arch.mmu->root_role.level == PT32E_ROOT_LEVEL)
sp->spt = vcpu->arch.mmu->pae_root;
else
sp->spt = kvm_mmu_memory_cache_alloc(&vcpu->arch.mmu_shadow_page_cache);
--
2.19.1.6.gb485710b
On Tue, Jul 19, 2022, Sean Christopherson wrote:
> On Sat, May 21, 2022, Lai Jiangshan wrote:
> Actually, I think the series is buggy. That patch, which precedes this one, does
>
> if (vcpu->arch.mmu->root_role.level != PT32E_ROOT_LEVEL)
> return 0;
>
> i.e. does NOT allocate pae_root for a 64-bit host, which means that running KVM
> against the on-demand patch would result in the WARN firing and bad things happening.
Gah, I take that back, pae_root is allocated by mmu_alloc_special_roots().
On Sat, May 21, 2022, Lai Jiangshan wrote:
> From: Lai Jiangshan <[email protected]>
>
> Allocate the tables when allocating the local shadow page.
This absolutely needs a much more verbose changelog.
> Signed-off-by: Lai Jiangshan <[email protected]>
> ---
> arch/x86/kvm/mmu/mmu.c | 17 +++++++++--------
> 1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 63c2b2c6122c..73e6a8e1e1a9 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -1809,10 +1809,12 @@ static bool using_local_root_page(struct kvm_mmu *mmu)
> * 2 or 3 levels of local shadow pages on top of non-local shadow pages.
> *
> * Local shadow pages are locally allocated. If the local shadow page's level
> - * is PT32E_ROOT_LEVEL, it will use the preallocated mmu->pae_root for its
> - * sp->spt. Because sp->spt may need to be put in the 32 bits CR3 (even in
> - * x86_64) or decrypted. Using the preallocated one to handle these
> - * requirements makes the allocation simpler.
> + * is PT32E_ROOT_LEVEL, and it is not shadowing nested NPT for 32-bit L1 in
> + * 64-bit L0 (or said when the shadow pagetable's level is PT32E_ROOT_LEVEL),
> + * it will use the preallocated mmu->pae_root for its sp->spt. Because sp->spt
> + * need to be put in the 32-bit CR3 (even in 64-bit host) or decrypted. Using
> + * the preallocated one to handle these requirements makes the allocation
> + * simpler.
> *
> * Local shadow pages are only visible to local VCPU except through
> * sp->parent_ptes rmap from their children, so they are not in the
> @@ -1852,13 +1854,12 @@ kvm_mmu_alloc_local_shadow_page(struct kvm_vcpu *vcpu, union kvm_mmu_page_role r
> sp->gfn = 0;
> sp->role = role;
> /*
> - * Use the preallocated mmu->pae_root when the shadow page's
> - * level is PT32E_ROOT_LEVEL which may need to be put in the 32 bits
> + * Use the preallocated mmu->pae_root when the shadow pagetable's
> + * level is PT32E_ROOT_LEVEL which need to be put in the 32 bits
> * CR3 (even in x86_64) or decrypted. The preallocated one is prepared
> * for the requirements.
> */
> - if (role.level == PT32E_ROOT_LEVEL &&
> - !WARN_ON_ONCE(!vcpu->arch.mmu->pae_root))
Why remove this WARN_ON_ONCE()? And shouldn't this also interact with
KVM: X86/MMU: Allocate mmu->pae_root for PAE paging on-demand
Actually, I think the series is buggy. That patch, which precedes this one, does
if (vcpu->arch.mmu->root_role.level != PT32E_ROOT_LEVEL)
return 0;
i.e. does NOT allocate pae_root for a 64-bit host, which means that running KVM
against the on-demand patch would result in the WARN firing and bad things happening.
> + if (vcpu->arch.mmu->root_role.level == PT32E_ROOT_LEVEL)
> sp->spt = vcpu->arch.mmu->pae_root;
> else
> sp->spt = kvm_mmu_memory_cache_alloc(&vcpu->arch.mmu_shadow_page_cache);
> --
> 2.19.1.6.gb485710b
>