Re: [PATCH 10/28] KVM: x86/mmu: pass PFERR_GUEST_PAGE/FINAL_MASK to kvm_translate_gpa

[mlevitsk]

On Tue, 2026-05-05 at 21:52 +0200, Paolo Bonzini wrote:
> The XS/XU bit for EPT are only applied to final accesses, and use the
> U bit from the page walk itself.  While strictly speaking not necessary
> (any value of PFERR_USER_MASK would be the same for page table accesses,
> because they're reads and writes only), it is clearer and less hackish
> to only apply MBEC to PFERR_GUEST_FINAL_MASK.  Allow kvm-intel.ko to
> distinguish the two cases.
> 
> Tested-by: David Riley <d.riley@xxxxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
>  arch/x86/kvm/hyperv.c          | 3 ++-
>  arch/x86/kvm/mmu/mmu.c         | 3 ++-
>  arch/x86/kvm/mmu/paging_tmpl.h | 7 +++++--
>  arch/x86/kvm/x86.c             | 3 ++-
>  4 files changed, 11 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
> index 9b140bbdc1d8..cf9dd565b894 100644
> --- a/arch/x86/kvm/hyperv.c
> +++ b/arch/x86/kvm/hyperv.c
> @@ -2041,7 +2041,8 @@ static u64 kvm_hv_flush_tlb(struct kvm_vcpu *vcpu, struct kvm_hv_hcall *hc)
>   * read with kvm_read_guest().
>   */
>   if (!hc->fast && is_guest_mode(vcpu)) {
> - hc->ingpa = translate_nested_gpa(vcpu, hc->ingpa, 0, NULL);
> + hc->ingpa = translate_nested_gpa(vcpu, hc->ingpa,
> + PFERR_GUEST_FINAL_MASK, NULL);
>   if (unlikely(hc->ingpa == INVALID_GPA))
>   return HV_STATUS_INVALID_HYPERCALL_INPUT;
>   }
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index fa6a5e4ee09a..46412e4d207f 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4348,7 +4348,8 @@ static gpa_t nonpaging_gva_to_gpa(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  {
>   if (exception)
>   exception->error_code = 0;
> - return kvm_translate_gpa(vcpu, mmu, vaddr, access, exception);
> + return kvm_translate_gpa(vcpu, mmu, vaddr, access | PFERR_GUEST_FINAL_MASK,
> + exception);


This is an existing problem, but now that I look at this and at the next patch, it is not
100% clear what 'access' is, especially for someone who is not familiar with he code.

Also soon we will have 'pte_access', which can make this problem worse.

I think that there is a need for a comment here, in this or in the next patch to avoid
a confusion.
What do you think?

>  }
>  
>  static bool mmio_info_in_cache(struct kvm_vcpu *vcpu, u64 addr, bool direct)
> diff --git a/arch/x86/kvm/mmu/paging_tmpl.h b/arch/x86/kvm/mmu/paging_tmpl.h
> index fb1b5d8b23e5..567f8b77ffe0 100644
> --- a/arch/x86/kvm/mmu/paging_tmpl.h
> +++ b/arch/x86/kvm/mmu/paging_tmpl.h
> @@ -376,7 +376,8 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>   walker->pte_gpa[walker->level - 1] = pte_gpa;
>  
>   real_gpa = kvm_translate_gpa(vcpu, mmu, gfn_to_gpa(table_gfn),
> -      nested_access, &walker->fault);
> +      nested_access | PFERR_GUEST_PAGE_MASK,
> +      &walker->fault);
>  
>   /*
>   * FIXME: This can happen if emulation (for of an INS/OUTS
> @@ -444,7 +445,9 @@ static int FNAME(walk_addr_generic)(struct guest_walker *walker,
>   gfn += pse36_gfn_delta(pte);
>  #endif
>  
> - real_gpa = kvm_translate_gpa(vcpu, mmu, gfn_to_gpa(gfn), access, &walker->fault);
> + real_gpa = kvm_translate_gpa(vcpu, mmu, gfn_to_gpa(gfn),
> +      access | PFERR_GUEST_FINAL_MASK,
> +      &walker->fault);
>   if (real_gpa == INVALID_GPA)
>   return 0;
>  
> diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
> index 0a1b63c63d1a..ef1e3ae13887 100644
> --- a/arch/x86/kvm/x86.c
> +++ b/arch/x86/kvm/x86.c
> @@ -1072,7 +1072,8 @@ int load_pdptrs(struct kvm_vcpu *vcpu, unsigned long cr3)
>   * to an L1 GPA.
>   */
>   real_gpa = kvm_translate_gpa(vcpu, mmu, gfn_to_gpa(pdpt_gfn),
> -      PFERR_USER_MASK | PFERR_WRITE_MASK, NULL);
> +      PFERR_USER_MASK | PFERR_WRITE_MASK |
> +      PFERR_GUEST_PAGE_MASK, NULL);
>   if (real_gpa == INVALID_GPA)
>   return 0;
>  

Looks correct to me otherwise.

Reviewed-by: Maxim Levitsky <mlevitsk@xxxxxxxxxx>
Best regards,
	Maxim Levitsky