linux-kernel - Re: [PATCH v2 03/10] KVM: x86/mmu: Extract __kvm_mmu_do_page

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Tue, 16 Apr 2024 16:22:35 +0800
From: Chao Gao <chao.gao@...el.com>
To: <isaku.yamahata@...el.com>
CC: <kvm@...r.kernel.org>, <isaku.yamahata@...il.com>,
	<linux-kernel@...r.kernel.org>, Sean Christopherson <seanjc@...gle.com>,
	Paolo Bonzini <pbonzini@...hat.com>, Michael Roth <michael.roth@....com>,
	David Matlack <dmatlack@...gle.com>, Federico Parola
	<federico.parola@...ito.it>, Kai Huang <kai.huang@...el.com>
Subject: Re: [PATCH v2 03/10] KVM: x86/mmu: Extract __kvm_mmu_do_page_fault()


>This patch makes the emulation_type always set irrelevant to the return
>code.  kvm_mmu_page_fault() is the only caller of kvm_mmu_do_page_fault(),
>and references the value only when PF_RET_EMULATE is returned.  Therefore,
>this adjustment doesn't affect functionality.

This is benign. But what's the benefit of doing this?

>+static inline int __kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>+					  u64 err, bool prefetch, int *emulation_type)
> {
> 	struct kvm_page_fault fault = {
> 		.addr = cr2_or_gpa,
>@@ -318,14 +318,6 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> 		fault.slot = kvm_vcpu_gfn_to_memslot(vcpu, fault.gfn);
> 	}
> 
>-	/*
>-	 * Async #PF "faults", a.k.a. prefetch faults, are not faults from the
>-	 * guest perspective and have already been counted at the time of the
>-	 * original fault.
>-	 */
>-	if (!prefetch)
>-		vcpu->stat.pf_taken++;
>-
> 	if (IS_ENABLED(CONFIG_MITIGATION_RETPOLINE) && fault.is_tdp)
> 		r = kvm_tdp_page_fault(vcpu, &fault);
> 	else
>@@ -333,12 +325,30 @@ static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
> 
> 	if (r == RET_PF_EMULATE && fault.is_private) {
> 		kvm_mmu_prepare_memory_fault_exit(vcpu, &fault);
>-		return -EFAULT;
>+		r = -EFAULT;
> 	}
> 
> 	if (fault.write_fault_to_shadow_pgtable && emulation_type)
> 		*emulation_type |= EMULTYPE_WRITE_PF_TO_SP;
> 
>+	return r;
>+}
>+
>+static inline int kvm_mmu_do_page_fault(struct kvm_vcpu *vcpu, gpa_t cr2_or_gpa,
>+					u64 err, bool prefetch, int *emulation_type)
>+{
>+	int r;
>+
>+	/*
>+	 * Async #PF "faults", a.k.a. prefetch faults, are not faults from the
>+	 * guest perspective and have already been counted at the time of the
>+	 * original fault.
>+	 */
>+	if (!prefetch)
>+		vcpu->stat.pf_taken++;
>+
>+	r = __kvm_mmu_do_page_fault(vcpu, cr2_or_gpa, err, prefetch, emulation_type);

bail out if r < 0?
	
>+
> 	/*
> 	 * Similar to above, prefetch faults aren't truly spurious, and the
> 	 * async #PF path doesn't do emulation.  Do count faults that are fixed
>-- 
>2.43.2
>
>