Re: [PATCH v4 6/9] KVM: x86: When KVM judges CR3 valid or not, consider LAM bits

[Chao Gao <chao.gao@intel.com>]

On Thu, Feb 09, 2023 at 10:40:19AM +0800, Robert Hoo wrote:
>Before apply to kvm_vcpu_is_illegal_gpa(), clear LAM bits if it's valid.

I prefer to squash this patch into patch 2 because it is also related to
CR3 LAM bits handling.

>
>Signed-off-by: Robert Hoo <robert.hu@linux.intel.com>
>Reviewed-by: Jingqi Liu <jingqi.liu@intel.com>
>---
> arch/x86/kvm/x86.c | 10 +++++++++-
> 1 file changed, 9 insertions(+), 1 deletion(-)
>
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 1bdc8c0c80c0..3218f465ae71 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -1231,6 +1231,14 @@ static void kvm_invalidate_pcid(struct kvm_vcpu *vcpu, unsigned long pcid)
> 	kvm_mmu_free_roots(vcpu->kvm, mmu, roots_to_free);
> }
> 
>+static bool kvm_is_valid_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)

Since this function takes a "vcpu" argument, probably
kvm_vcpu_is_valid_cr3() is slightly better.

>+{
>+	if (guest_cpuid_has(vcpu, X86_FEATURE_LAM))

check if the vcpu is in the 64 bit long mode?

>+		cr3 &= ~(X86_CR3_LAM_U48 | X86_CR3_LAM_U57);
>+
>+	return kvm_vcpu_is_legal_gpa(vcpu, cr3);
>+}
>+
> int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
> {
> 	bool skip_tlb_flush = false;
>@@ -1254,7 +1262,7 @@ int kvm_set_cr3(struct kvm_vcpu *vcpu, unsigned long cr3)
> 	 * stuff CR3, e.g. for RSM emulation, and there is no guarantee that
> 	 * the current vCPU mode is accurate.
> 	 */
>-	if (kvm_vcpu_is_illegal_gpa(vcpu, cr3))
>+	if (!kvm_is_valid_cr3(vcpu, cr3))

There are other call sites of kvm_vcpu_is_illegal_gpa() to validate cr3.
Do you need to modify them?

> 		return 1;
> 
> 	if (is_pae_paging(vcpu) && !load_pdptrs(vcpu, cr3))
>-- 
>2.31.1
>