Received-SPF: pass (google.com: domain of linux-kernel+bounces-146491-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Date: Tue, 16 Apr 2024 16:22:35 +0800
From: Chao Gao <chao.gao@intel.com>
To: <isaku.yamahata@intel.com>
CC: <kvm@vger.kernel.org>, <isaku.yamahata@gmail.com>,
	<linux-kernel@vger.kernel.org>, Sean Christopherson <seanjc@google.com>,
	Paolo Bonzini <pbonzini@redhat.com>, Michael Roth <michael.roth@amd.com>,
	David Matlack <dmatlack@google.com>, Federico Parola
	<federico.parola@polito.it>, Kai Huang <kai.huang@intel.com>
Subject: Re: [PATCH v2 03/10] KVM: x86/mmu: Extract __kvm_mmu_do_page_fault()
Message-ID: <Zh41S8fh0IvXlKwX@chao-email>
References: <cover.1712785629.git.isaku.yamahata@intel.com>
 <ddf1d98420f562707b11e12c416cce8fdb986bb1.1712785629.git.isaku.yamahata@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <ddf1d98420f562707b11e12c416cce8fdb986bb1.1712785629.git.isaku.yamahata@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?JFRnusJDxMYEH6Y0vyPXdlkxBIn3Kc43lXbXQPPNkrf4YuEP+N1kAbzRQEMk?=
 =?us-ascii?Q?fI0/QuW+Mahumd/2YUnBeTSthDcq9jDzZu8PWQFIn+rdHvXoAYajtq8bSLeK?=
 =?us-ascii?Q?dGcGo+1U6o9MP/6AHiprOD2/DwMbXuWeWGXpP4mugnRB/3vVE5KSXh0Xq8C3?=
 =?us-ascii?Q?VWV0lFWfS/m/yCDRqkLCekbUxto4+Gn5J0/lSLr+h5Hqu61sRJr2fSzrz0W+?=
 =?us-ascii?Q?U+3uV/rN3TojNhb3K8c2s5cZ69xjt+sWHV0WFM9KJHscvzhE3tAztA5hB2kv?=
 =?us-ascii?Q?4SFKRSoqEQLBhFVUuH9SxnECQVy1kB73sQWHn9UBm8Fk4TpmtaVrjqY8S7MU?=
 =?us-ascii?Q?rHhKqF8IQcubsP26Cif0l1KJuVbfQz8q4EmFwI/yU0k96eu67VmGWWGrIUU3?=
 =?us-ascii?Q?u2jVlgVw0yZJTWp8Lppocx1nr+MVRKgNDHJ9zLERTFcVFtrVX8Ia7f5/cmJN?=
 =?us-ascii?Q?YUUrgZlh1z5vtgjxOtvSitfIOXwrOWRtf2lsUzlwbNECIhKsL6oYzkxhxOMq?=
 =?us-ascii?Q?uvKU1MmD4ScxH69Lrif8+DRMRT5pmf3xN1/9MTTJj4r1Ramz7/vsXDS3oTAi?=
 =?us-ascii?Q?OBlULLAUYop8pPbqLw5YCbQ/l+8IRQPcKuwG0z+mN71lNW22RJpPTCFxwGLw?=
 =?us-ascii?Q?rDOb3gvQqYaY44HZmFvIe5SFhdZ/3AXaYo25U8PCliaA6mQtyYAxb0hFBIgO?=
 =?us-ascii?Q?C2fm4osk4Cl0CLr00FFVT8V8MS5OuYLRtJy6qQWJ3p29HVglS4/qnwwoRCDy?=
 =?us-ascii?Q?WGkziYB1swgp7CW8qPk2kIH7fqxBDhk9fLhLFYUJa3CRoSCT2eGB+JE8ho5C?=
 =?us-ascii?Q?0yXfdLh8RiMSaU2EtoXGDLhAUiql0DhYmEylsLfUKYSBaIOMBU+t3w47iuwr?=
 =?us-ascii?Q?cNDQrGRFl7B3JEXTpxciKQ0xoQEOb9yCT7Edehdkd48cEtujukHLbtaFDDhg?=
 =?us-ascii?Q?TzRQJ8OJ2Uus2mheZtPyVFFe4ouj2z2frqCZOwKZVCCPTK+2AnCEmL09mNT6?=
 =?us-ascii?Q?nBoMS+XIoDxh4NBSOly381dbCiRdFQhvNKs2Xd/rW0yGVxVg6TRJrn6EnBdK?=
 =?us-ascii?Q?dDMYvvTGaqf9fovVuSInQu3KKzwWfHLxn/E0huHm03/p2MdCVR0VDIc2jUMe?=
 =?us-ascii?Q?b6HbdUlHqHhaxvuFVoOdPrZDpGa774XqJ5MuB5vjONj1LbD5u5r9wseLJMV5?=
 =?us-ascii?Q?fy/PKfPwZJZEZrStNn32V4nF/s3Cf/J/PzjaMNn4LWzOmrpc+KwmDqQJU2vW?=
 =?us-ascii?Q?lUs7dg0WBe+7M3PYDRHqs13jPOtfmU/Nt2TCp1O/neTfgOtKxaU5zDjOaIeX?=
 =?us-ascii?Q?J648tejdeo4Mtsiey6x5pVI2wWBzyixD5rRnqy6g9/4kcdfEUqVQNlvdChsK?=
 =?us-ascii?Q?2y7WuPoQXptxMDnoBMJpCSmmMTHmx9PUFTje2EOzeAN2/uUiq12Rg+HqmO5Y?=
 =?us-ascii?Q?15PQLnQ10JnEX+376Z+tZc2qPiKFONFcr8c0cB+Z4EtyrNJZB/TTqWt0QjQz?=
 =?us-ascii?Q?hPSQImjlNIry9+ouQlwzUplxkAAK/xV2UYDIXji2oPSITQt1EDFHaMzzmURQ?=
 =?us-ascii?Q?0pwiv7oFTPrzOnIw1u1T8GLVd2ypr0neuanJG+0k?=
X-MS-Exchange-CrossTenant-Network-Message-Id: bf0d3a50-6446-44f3-682c-08dc5dee642e
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 16 Apr 2024 08:22:44.5096
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 40ZqAMdWolXnqxfZZNZl3l5vX+o79CsecwSd2gcjvojPJJvg2PgtAXh/5i0p1syjjiBbQZyw+YfmGe5kz+y6SQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SJ0PR11MB5055
X-OriginatorOrg: intel.com


>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
>
>