Re: [PATCH v3 02/31] KVM: MMU: Introduce struct kvm_page_fault

From: David Matlack
Date: Tue Sep 28 2021 - 19:25:47 EST


On Fri, Sep 24, 2021 at 12:31:23PM -0400, Paolo Bonzini wrote:
> Create a single structure for arguments that are passed from
> kvm_mmu_do_page_fault to the page fault handlers. Later
> the structure will grow to include various output parameters
> that are passed back to the next steps in the page fault
> handling.
>
> Suggested-by: Isaku Yamahata <isaku.yamahata@xxxxxxxxx>
> Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
> ---
> arch/x86/kvm/mmu.h | 34 +++++++++++++++++++++++++++++++---
> 1 file changed, 31 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index e9688a9f7b57..0553ef92946e 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -114,17 +114,45 @@ static inline void kvm_mmu_load_pgd(struct kvm_vcpu *vcpu)
> vcpu->arch.mmu->shadow_root_level);
> }
>
> +struct kvm_page_fault {
> + /* arguments to kvm_mmu_do_page_fault. */
> + const gpa_t addr;
> + const u32 error_code;
> + const bool prefault;

This is somewhat tangential to your change but... I notice KVM uses
"prefetch" and "prefault" interchangably. If we changed prefault to
prefetch here and in kvm_mmu_do_page_fault then that would make the
naming consistent throughout KVM ("prefetch" for everything).

> +
> + /* Derived from error_code. */
> + const bool exec;
> + const bool write;
> + const bool present;
> + const bool rsvd;
> + const bool user;
> +
> + /* Derived from mmu. */
> + const bool is_tdp;
> +};
> +
> int kvm_tdp_page_fault(struct kvm_vcpu *vcpu, gpa_t gpa, u32 error_code,
> bool prefault);
>
> static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> u32 err, bool prefault)
> {
> + struct kvm_page_fault fault = {
> + .addr = cr2_or_gpa,
> + .error_code = err,
> + .exec = err & PFERR_FETCH_MASK,
> + .write = err & PFERR_WRITE_MASK,
> + .present = err & PFERR_PRESENT_MASK,
> + .rsvd = err & PFERR_RSVD_MASK,
> + .user = err & PFERR_USER_MASK,
> + .prefault = prefault,
> + .is_tdp = likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault),
> + };
> #ifdef CONFIG_RETPOLINE
> - if (likely(vcpu->arch.mmu->page_fault == kvm_tdp_page_fault))
> - return kvm_tdp_page_fault(vcpu, cr2_or_gpa, err, prefault);
> + if (fault.is_tdp)
> + return kvm_tdp_page_fault(vcpu, fault.addr, fault.error_code, fault.prefault);
> #endif
> - return vcpu->arch.mmu->page_fault(vcpu, cr2_or_gpa, err, prefault);
> + return vcpu->arch.mmu->page_fault(vcpu, fault.addr, fault.error_code, fault.prefault);
> }
>
> /*
> --
> 2.27.0
>
>