Re: [PATCH v4 4/9] KVM: x86: MMU: Integrate LAM bits when build guest CR3

Chao Gao
2023-02-10T14:04:51Z

On Thu, Feb 09, 2023 at 10:40:17AM +0800, Robert Hoo wrote:
>When calc the new CR3 value, take LAM bits in.

I prefer to merge this one into patch 2 because both are related to
CR3_LAM_U48/U57 handling. Merging them can give us the whole picture of
how the new LAM bits are handled:
* strip them from CR3 when allocating/finding a shadow root
* stitch them with other fields to form a shadow CR3

I have a couple questions:
1. in kvm_set_cr3(),

/* PDPTRs are always reloaded for PAE paging. */
if (cr3 == kvm_read_cr3(vcpu) && !is_pae_paging(vcpu))
goto handle_tlb_flush;

Shouldn't we strip off CR3_LAM_U48/U57 and do the comparison?
It depends on whether toggling CR3_LAM_U48/U57 causes a TLB flush.

2. also in kvm_set_cr3(),

if (cr3 != kvm_read_cr3(vcpu))
kvm_mmu_new_pgd(vcpu, cr3);

is it necessary to use a new pgd if only CR3_LAM_U48/U57 were changed?

>
>Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
>Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
>---
> arch/x86/kvm/mmu.h | 5 +++++
> arch/x86/kvm/vmx/vmx.c | 3 ++-
> 2 files changed, 7 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
>index 6bdaacb6faa0..866f2b7cb509 100644
>--- a/arch/x86/kvm/mmu.h
>+++ b/arch/x86/kvm/mmu.h
>@@ -142,6 +142,11 @@ static inline unsigned long kvm_get_active_pcid(struct kvm_vcpu *vcpu)
> return kvm_get_pcid(vcpu, kvm_read_cr3(vcpu));
> }
>
>+static inline u64 kvm_get_active_lam(struct kvm_vcpu *vcpu)
>+{
>+ return kvm_read_cr3(vcpu) & (X86_CR3_LAM_U48 | X86_CR3_LAM_U57);
>+}
>+
> static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
> {
> u64 root_hpa = vcpu->arch.mmu->root.hpa;
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index fe5615fd8295..66edd091f145 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -3289,7 +3289,8 @@ static void vmx_load_mmu_pgd(struct kvm_vcpu *vcpu, hpa_t root_hpa,
> update_guest_cr3 = false;
> vmx_ept_load_pdptrs(vcpu);
> } else {
>- guest_cr3 = root_hpa | kvm_get_active_pcid(vcpu);
>+ guest_cr3 = root_hpa | kvm_get_active_pcid(vcpu) |
>+ kvm_get_active_lam(vcpu);
> }
>
> if (update_guest_cr3)
>--
>2.31.1
>