Sean Christopherson <[email protected]> writes: > From: Sean Christopherson <[email protected]> > > Retrieve the active PCID only when writing a guest CR3 value, i.e. don't > get the PCID when using EPT or NPT. The PCID is especially problematic > for EPT as the bits have different meaning, and so the PCID and must be > manually stripped, which is annoying and unnecessary. And on VMX, > getting the active PCID also involves reading the guest's CR3 and > CR4.PCIDE, i.e. may add pointless VMREADs. > > Opportunistically rename the pgd/pgd_level params to root_hpa and > root_level to better reflect their new roles. Keep the function names, > as "load the guest PGD" is still accurate/correct. > > Last, and probably least, pass root_hpa as a hpa_t/u64 instead of an > unsigned long. The EPTP holds a 64-bit value, even in 32-bit mode, so > in theory EPT could support HIGHMEM for 32-bit KVM. Never mind that > doing so would require changing the MMU page allocators and reworking > the MMU to use kmap(). > > Signed-off-by: Sean Christopherson <[email protected]> > Signed-off-by: Sean Christopherson <[email protected]> > --- > arch/x86/include/asm/kvm_host.h | 4 ++-- > arch/x86/kvm/mmu.h | 4 ++-- > arch/x86/kvm/svm/svm.c | 10 ++++++---- > arch/x86/kvm/vmx/vmx.c | 13 ++++++------- > arch/x86/kvm/vmx/vmx.h | 3 +-- > 5 files changed, 17 insertions(+), 17 deletions(-) > > diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h > index 2da6c9f5935a..51725e994451 100644 > --- a/arch/x86/include/asm/kvm_host.h > +++ b/arch/x86/include/asm/kvm_host.h > @@ -1245,8 +1245,8 @@ struct kvm_x86_ops { > int (*set_identity_map_addr)(struct kvm *kvm, u64 ident_addr); > u64 (*get_mt_mask)(struct kvm_vcpu *vcpu, gfn_t gfn, bool is_mmio); > > - void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, unsigned long pgd, > - int pgd_level); > + void (*load_mmu_pgd)(struct kvm_vcpu *vcpu, hpa_t root_hpa, > + int root_level); > > bool (*has_wbinvd_exit)(void); > > diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h > index 67e8c7c7a6ce..88d0ed5225a4 100644 > --- a/arch/x86/kvm/mmu.h > +++ b/arch/x86/kvm/mmu.h > @@ -107,8 +107,8 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu) > if (!VALID_PAGE(root_hpa)) > return; > > - static_call(kvm_x86_load_mmu_pgd)(vcpu, root_hpa | > kvm_get_active_pcid(vcpu), > - vcpu->arch.mmu->shadow_root_level); > + static_call(kvm_x86_load_mmu_pgd)(vcpu, root_hpa, > + vcpu->arch.mmu->shadow_root_level); > } > > int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u32 error_code, > diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c > index dfc8fe231e8b..c7a685bf6862 100644 > --- a/arch/x86/kvm/svm/svm.c > +++ b/arch/x86/kvm/svm/svm.c > @@ -3902,14 +3902,14 @@ static __no_kcsan fastpath_t svm_vcpu_run(struct > kvm_vcpu *vcpu) > return svm_exit_handlers_fastpath(vcpu); > } > > -static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long root, > +static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, > int root_level) > { > struct vcpu_svm *svm = to_svm(vcpu); > unsigned long cr3; > > if (npt_enabled) { > - svm->vmcb->control.nested_cr3 = __sme_set(root); > + svm->vmcb->control.nested_cr3 = __sme_set(root_hpa); > vmcb_mark_dirty(svm->vmcb, VMCB_NPT); > > /* Loading L2's CR3 is handled by enter_svm_guest_mode. */ > @@ -3917,9 +3917,11 @@ static void svm_load_mmu_pgd(struct kvm_vcpu *vcpu, > unsigned long root, > return; > cr3 = vcpu->arch.cr3; > } else if (vcpu->arch.mmu->shadow_root_level >= PT64_ROOT_4LEVEL) { > - cr3 = __sme_set(root); > + cr3 = __sme_set(root_hpa) | kvm_get_active_pcid(vcpu); > } else { > - cr3 = root; > + /* PCID in the guest should be impossible with a 32-bit MMU. */ > + WARN_ON_ONCE(kvm_get_active_pcid(vcpu)); > + cr3 = root_hpa; > } > > svm->vmcb->save.cr3 = cr3; > diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c > index 6d7e760fdfa0..dde2ebc7cf3a 100644 > --- a/arch/x86/kvm/vmx/vmx.c > +++ b/arch/x86/kvm/vmx/vmx.c > @@ -3088,8 +3088,7 @@ static int vmx_get_max_tdp_level(void) > return 4; > } > > -u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa, > - int root_level) > +u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level) > { > u64 eptp = VMX_EPTP_MT_WB; > > @@ -3098,13 +3097,13 @@ u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned > long root_hpa, > if (enable_ept_ad_bits && > (!is_guest_mode(vcpu) || nested_ept_ad_enabled(vcpu))) > eptp |= VMX_EPTP_AD_ENABLE_BIT; > - eptp |= (root_hpa & PAGE_MASK); > + eptp |= root_hpa; > > return eptp; > } > > -static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, unsigned long pgd, > - int pgd_level) > +static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa, > + int root_level) > { > struct kvm *kvm = vcpu->kvm; > bool update_guest_cr3 = true; > @@ -3112,7 +3111,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, > unsigned long pgd, > u64 eptp; > > if (enable_ept) { > - eptp = construct_eptp(vcpu, pgd, pgd_level); > + eptp = construct_eptp(vcpu, root_hpa, root_level); > vmcs_write64(EPT_POINTER, eptp); > > if (kvm_x86_ops.tlb_remote_flush) { > @@ -3131,7 +3130,7 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, > unsigned long pgd, > update_guest_cr3 = false; > vmx_ept_load_pdptrs(vcpu); > } else { > - guest_cr3 = pgd; > + guest_cr3 = root_hpa | kvm_get_active_pcid(vcpu); > } > > if (update_guest_cr3) > diff --git a/arch/x86/kvm/vmx/vmx.h b/arch/x86/kvm/vmx/vmx.h > index 89da5e1251f1..4795955490cb 100644 > --- a/arch/x86/kvm/vmx/vmx.h > +++ b/arch/x86/kvm/vmx/vmx.h > @@ -376,8 +376,7 @@ void set_cr4_guest_host_mask(struct vcpu_vmx *vmx); > void ept_save_pdptrs(struct kvm_vcpu *vcpu); > void vmx_get_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int > seg); > void vmx_set_segment(struct kvm_vcpu *vcpu, struct kvm_segment *var, int > seg); > -u64 construct_eptp(struct kvm_vcpu *vcpu, unsigned long root_hpa, > - int root_level); > +u64 construct_eptp(struct kvm_vcpu *vcpu, hpa_t root_hpa, int root_level); > > void vmx_update_exception_bitmap(struct kvm_vcpu *vcpu); > void vmx_update_msr_bitmap(struct kvm_vcpu *vcpu);
The rest of the series carries my R-b so let's not make this patch feel sad. Personally, I'd suggest we split this into 're-name/re-type' and 'PCID' patches but it's not a big deal. Reviewed-by: Vitaly Kuznetsov <[email protected]> -- Vitaly
