Received-SPF: pass (google.com: domain of linux-kernel+bounces-94792-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Message-ID: <d325e811-c80d-49ba-85cf-06893cec659a@intel.com>
Date: Thu, 7 Mar 2024 13:48:55 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 13/16] KVM: x86/mmu: Handle no-slot faults at the
 beginning of kvm_faultin_pfn()
Content-Language: en-US
To: Sean Christopherson <seanjc@google.com>, Paolo Bonzini
	<pbonzini@redhat.com>
CC: <kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, Yan Zhao
	<yan.y.zhao@intel.com>, Isaku Yamahata <isaku.yamahata@intel.com>, "Michael
 Roth" <michael.roth@amd.com>, Yu Zhang <yu.c.zhang@linux.intel.com>, "Chao
 Peng" <chao.p.peng@linux.intel.com>, Fuad Tabba <tabba@google.com>, "David
 Matlack" <dmatlack@google.com>
References: <20240228024147.41573-1-seanjc@google.com>
 <20240228024147.41573-14-seanjc@google.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240228024147.41573-14-seanjc@google.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?UytpMDlQMzRsZmpGY3EveTFKclJMbkRYS1FRUkFXZ3lPUXFTcXM3R2p1MFlU?=
 =?utf-8?B?MFZuRHVLS0dNMFY1bW4yd0xtMUttN2FmS2IyWlpHS3orT3F3SmdwQWl3NWlE?=
 =?utf-8?B?SmV3M2pvL2lpOFkraEtUQWRYWmwwUVVJY3ZHK2oxOGQrRUdacWlhTzZnVDhi?=
 =?utf-8?B?UE43WVI4bFo0RUV5RjU1SmNnR2hieSt1NkN4T3BveDQ3YlJwRDZGWmg3SXlO?=
 =?utf-8?B?RmFUaHJucjdsc25XU2cwdVRWU29iRFNoZm9jam1tQjZVdWVaaTZvU2NZSnNh?=
 =?utf-8?B?WHpDT0p5ZmhRaERkMVJualJFUjZYVkpUek1pY0NhRHZHZGZ6c1RxWXVNOHlH?=
 =?utf-8?B?K2RIclAwTjVpZjJSNVFCTGRTZGxPd1dua0tBN0ppM1hQL2duSnR0UGovYm16?=
 =?utf-8?B?ZXF6OWFRbXFUY0V2QjFUU0M2dlZNUUllaVVidmVxUkJvUXJxQldIQUNIMGJJ?=
 =?utf-8?B?N1k5L0tjKy9oMkpnNGdKRnA2d0dxZXk0Y09GcEVHSUQyTDJYbGp1RGpnTFlz?=
 =?utf-8?B?L0NibGRTRFhzM2tXWXNUMDVHYTMyT0dCaFJzbDAzeUxmdU1POW9TcUN2QXgx?=
 =?utf-8?B?MUIxTXhwaWZoWXVuMzY3eE1TLzFEZExCYk9JRncySHFTQ0tFSGUyT0w5bGFY?=
 =?utf-8?B?OVNxY0ZmclhCSVhBT2pFR2V3VjZMUUY1NVlGZXkwRSsrOHhBL05zaHNRRXVI?=
 =?utf-8?B?S1QrT1c5UmE2NlNIR1UzTjlTWjNUaFVwdTlCS2xNZTIwTFlwRGJoQjZ4elN3?=
 =?utf-8?B?aXRNNktoWEROM1htQk96Uk51R1VLUERFQ0svY1VLdXJjRlh3MHcwZTJGalds?=
 =?utf-8?B?T2NtV01FZkNZUzQwQXdZd3BpZW9tdUZpWXorVVU3S3FSbTVzSTFSN3VGV3ky?=
 =?utf-8?B?VktYdC94Qy9Oa2pxMmxTZzQwL3JOUWNMTWQxOGZmRmNGT0IxajJvU2MzNFJi?=
 =?utf-8?B?YjhJK1JWREFCOHBFN0h4S1JwTjJ5V2FtVkE5ZXlJbUdLZWRjdFd6Y0gyNjBa?=
 =?utf-8?B?UEJ3VkpyV3hEWjd2bE43SkpROEpldndrMUxPTVJ5Qm1LUEVpTjBUc0dQOWpw?=
 =?utf-8?B?em1RTnkxeERORUVzNDNkaTJ6Y3NlNUh5cGM2SUhwM2ZPa2xYTWNheWIydDhN?=
 =?utf-8?B?SVlIRS9obTJlWTJRNkE4WldLOGhDNTV3VEZZei9Xc0xJaXl0QW5Za292TjFJ?=
 =?utf-8?B?OWlVbTViWk1oV0dxV0pqWG1HSjB1Yit3WDFRK1BaSFA2WmdiamVnWmNBTklI?=
 =?utf-8?B?ZWluK3ZLSkwvUnN5Q0xFZEZnYjhMUjhLeDh4cWRTUWV5U0pvTjNBVk9ON1hr?=
 =?utf-8?B?VlBES2VRVXUrSHFKVXlqaTZKSWUyOGp2eE15RkYwaUpDUmJFc3llRlduNlBP?=
 =?utf-8?B?UmRTRERlMFQyM1hvV0EvNlF5akxjYk04bG1XdlNUR1VwSXdjbGNrQ1B2ejV4?=
 =?utf-8?B?S1daSGpQZkIzWUUwRmd5OXczRVpkQ2FJTE9qSHN2TE1kTDhZUFIrcFY1b3RU?=
 =?utf-8?B?TVdrblhsalkvVUlYZEk3L3hhVEJKbytBODkwallzbm9pY1RBVVRQN0I1VkVt?=
 =?utf-8?B?TzU0MXVGSm8vdlVIQ29zcWNoYzlQakE0MWsyRlNidmM5K3ZtNzk3RTNxV2ZP?=
 =?utf-8?B?ajIxc2MrRUlVd2VNRzJ4QnhGRnI5R24wV29KK3dVTVNPQk83cHAxTHVJaHRn?=
 =?utf-8?B?V3ppZFg1cTJqbzNySlRkZXNqS2JXZnBEV284ZTNDUlZxczU1SzMrY05EcDBv?=
 =?utf-8?B?Z0xvaW1HbkhLZ0JPNmtaTHF0Z1BtRm0yTCtmRHRaKzE2WUI5Qk1UR3BsOUNF?=
 =?utf-8?B?SURYVGJTNlZtV3dQd1RKS2N2bk93NXQybnhuWEtIQkZVTUJ1SkYvbXo4N092?=
 =?utf-8?B?dWFSejFUWHN5M2hGNlROcDVOS1Z1NlMvekJzY0ZsWlZjTkdnNVJzTVhVWHhM?=
 =?utf-8?B?Mm9MbU90aWpnZjlHNTgzOUxub29aaE01T1V3OFpUdEtYdVFDbXdOMDVhMzk5?=
 =?utf-8?B?V1BlQnFvOEFlcStWMWJYaDlRbDgxN0JuVkZqNXRibWpvOGFuQVB6aWE2a2Fp?=
 =?utf-8?B?ZjBSTWtaeGpucFRLaU9lUkdTcWNHTUpCSjJBeGlXSWsvZ2tJTFlkT1luVkVJ?=
 =?utf-8?Q?5dK0O9CdOBD3mwtkKTzdgDNe6?=
X-MS-Exchange-CrossTenant-Network-Message-Id: c6a3cb23-eced-44da-1e34-08dc3e4062bc
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Mar 2024 00:49:03.6513
 (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: iJqeUTUvVASkjZtsoli14vmG0GuOiCXVp/vx/xq0PhgUcc3Gzl3MXEJKaVdgCknbJDy0UzMGyuqWrnjXWiQgEg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6728
X-OriginatorOrg: intel.com


On 28/02/2024 3:41 pm, Sean Christopherson wrote:
> Handle the "no memslot" case at the beginning of kvm_faultin_pfn(), just
> after the private versus shared check, so that there's no need to
> repeatedly query whether or not a slot exists.  This also makes it more
> obvious that, except for private vs. shared attributes, the process of
> faulting in a pfn simply doesn't apply to gfns without a slot.
> 
> Opportunistically stuff @fault's metadata in kvm_handle_noslot_fault() so
> that it doesn't need to be duplicated in all paths that invoke
> kvm_handle_noslot_fault(), and to minimize the probability of not stuffing
> the right fields.
> 
> Leave the existing handle behind, but convert it to a WARN, to guard
> against __kvm_faultin_pfn() unexpectedly nullifying fault->slot.
> 
> Cc: David Matlack <dmatlack@google.com>
> Signed-off-by: Sean Christopherson <seanjc@google.com>

Reviewed-by: Kai Huang <kai.huang@intel.com>

One nit ...


> --- a/arch/x86/kvm/mmu/mmu_internal.h
> +++ b/arch/x86/kvm/mmu/mmu_internal.h
> @@ -235,7 +235,7 @@ struct kvm_page_fault {
>   	/* The memslot containing gfn. May be NULL. */
>   	struct kvm_memory_slot *slot;
>   
> -	/* Outputs of kvm_faultin_pfn.  */
> +	/* Outputs of kvm_faultin_pfn. */
>   	unsigned long mmu_seq;
>   	kvm_pfn_t pfn;
>   	hva_t hva;

.. how about get rid of this non-related fix?

Yeah it's annoying but do in a separate patch?