| Message ID | 20220521131700.3661-12-jiangshanlai@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | KVM: X86/MMU: Use one-off local shadow page for special roots | expand |
On Sat, May 21, 2022, Lai Jiangshan wrote: > From: Lai Jiangshan <jiangshan.ljs@antgroup.com> > > Allocate the tables when allocating the local shadow page. This absolutely needs a much more verbose changelog. > Signed-off-by: Lai Jiangshan <jiangshan.ljs@antgroup.com> > --- > 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 >
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().
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);