Received-SPF: pass (google.com: domain of linux-kernel+bounces-87624-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <3779953f-4d07-41d7-b450-bbc2afffaa43@intel.com>
Date: Fri, 1 Mar 2024 11:11:57 +1300
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 06/16] KVM: x86/mmu: WARN if upper 32 bits of legacy #PF
 error code are non-zero
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-7-seanjc@google.com>
From: "Huang, Kai" <kai.huang@intel.com>
In-Reply-To: <20240228024147.41573-7-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?QVl6Z0RSZFNmakxKa0QyK3NtTldUS1o0cDMxRGZKTzM3UGYrTlUvMnUyZ1o2?=
 =?utf-8?B?U3A3MU5ieFoyTHk1YkRvSWhlOWFsQmMydjZNM0sxbzJhbUhtT2hDdE9GTUlk?=
 =?utf-8?B?UENkMEpRVnl3aVVaTGxXeW8zRU5PeitOMnZobmRMTHNsaWUvZDVsR2ZYSjZO?=
 =?utf-8?B?UGpIcHVNb2gzeENsOEg5VW9zM0g1d3Z5d29IcTVKUHlpSDR5WkgxeXZGcVNM?=
 =?utf-8?B?MHJ4V0YxUlk3d0dZWjJZU3VHZWxLVTJFN2ZsWVZpL2J1ekRXMjR1MWl6ZHpO?=
 =?utf-8?B?OTd0T2laOW42QXV2bWdkaWdTbXV3bUJKZEs3M0psSlppbW9makU3QWhncUZP?=
 =?utf-8?B?bDNNQ3lJMFNnQ2xLd1VJanRGZisyUDFqSG1Qam9rcnpsdWR5QkE1M1YvVnFN?=
 =?utf-8?B?aHBsMDFVckZZMkd2WGw4cksvWFg5T28vNUdkY0pTQm9UT29Fd3p2TnZYejNy?=
 =?utf-8?B?WGpDM1Jjb1UyN1Y3MEZmWFM4OXE3OHBxS3J5VmhHQnVvSmdoTU8zK1hoTDAx?=
 =?utf-8?B?T09EeUl5QmJXYkVxbVluMmxXU3BoOHVEU04vSzg0Z1c2cEMvZ3VMVWFOd3Q5?=
 =?utf-8?B?YlBQS3ZyTTF4ZHVyTFppMTZ3YXJkY2l1bXBZSUFpWmZ4K0grL05uQWRGNE9T?=
 =?utf-8?B?RndGTlE5eHVWMlhvU3BtYkZWdjZSQ3ZQME5vMnAzUnM3d3RvL3kwWjFBOTRR?=
 =?utf-8?B?cUFRcjlKL2w5QkM0L3QwTnVPQXpPeW10U09SbUx1dE9pWXJTb1MxT3VKVUJU?=
 =?utf-8?B?dDJqSERMTjJ6NytrWEQwNW8vcDhob0hDNHo1MFZCaWM4b3lzRlZpNGpLMjM5?=
 =?utf-8?B?c29FaXc3WEpUWE9EMzBIejFpTU5xZmR2ekgrS2M4Z0NHcWhDbng1d1hnbmk4?=
 =?utf-8?B?K2kxcXBlcnJYOUphd0hJS3pmSzdhVGZGeUxMOFBhOGV3SzNpOVN4bkR4dHBV?=
 =?utf-8?B?ZUFQQ2JNdkZvZGRSeE91T3hqS1NmMkZBWUxSMSswb2ErcVhObS95eGVJbHVO?=
 =?utf-8?B?dmNmOXNNcFQ1M1o1NkNYMU5sV3pzQjNEeTNVVkFxR1J6NG4wLzMrZVZ2QWFl?=
 =?utf-8?B?bmk3eG1lR2NQU0RLMThIMVFtY0ZiQXF2ak1EVmJ5djRCVjFoc0RMT3I2aXAr?=
 =?utf-8?B?ZGJPRERYaU1QYXovL21zWG1VRjRUL2JMYlZ6ck9IUi96UmFWQVd0NUdOYjND?=
 =?utf-8?B?S3hkbVRSQi9OcS9CQzhEeXpQZlJiZ1RNazczVno0UDV5Q0JYeU02UGpxeUl4?=
 =?utf-8?B?RlBFREVyeXBSczBSSkkzNW5jK09XNDQ5ZXB5Ny9sbkVmdTFuekVuaU1xR2Nr?=
 =?utf-8?B?RjUvN0NaQ2plZTBvUEZ1Z2ZoSi9wRWRsMVJDb2tRVlhWb2dlankxZUFNMTRH?=
 =?utf-8?B?VXdpWDJ0dVhJaWNVU3lqZlhISjNER2syNE1weHhZTnJTd1N6dTdwdm80eHNS?=
 =?utf-8?B?NTY1eTdsNFllZzd6dkRYNXpZTlplaXRlYUtzTjZMdjlsdjJPMWZEVTJIV0RX?=
 =?utf-8?B?QWFGZEZiT3h2UWRQRUZZQXJnKzFFSEdXUkpwZ05pSVNkc2E3eW16c01uNlk5?=
 =?utf-8?B?RTI1OG1udThFYzlyMGY2Y2F6a0U4dzU1MVNnRk5MM29JTGFhVFRBZVJqRWs2?=
 =?utf-8?B?KzA1ZnFtTkdLNHNpS0lpUStNbnkwZHFBc0k5T0djRXJGcXFxQkczcXc4cGlj?=
 =?utf-8?B?elpwZVRtMHRsS213dC9rQ2tUeGpPOW5ScDIzTlhPR01zK2o0R201S2lTRUU5?=
 =?utf-8?B?a0V5QjZHdnE3NG8xN0RKNC9UeVY0aGdYYnUybWw3T284MWt6WGQ2MjdTM3ZF?=
 =?utf-8?B?RXFmdTFFVFpJbnNxSXJhVGdVcitqT2Q2ZDA2Rkx3citvdjRVUjNUbzVRdGUw?=
 =?utf-8?B?YnZOK256UWUvQUtpR3NaSFphT0VwSjU2bDRCZDlLaTNtSHZReDZ2NGhqM1FN?=
 =?utf-8?B?UDdZVkN3ZXlwVnBCNmlvY0FsUlFQanNCREVmb3MydjFJekcwcHYrVzNzY2g5?=
 =?utf-8?B?RnVjYkMwVjRtMVlzNzZKeUEzNmdYdVdpOWF0S3E2U0VIenZTZEhYRG5SblNY?=
 =?utf-8?B?UFg4TEpQbjZUSEhHWjJvRTdwUG5ZSzFpUHExVGVJLzFLYXV4NGQzZmk3aHEx?=
 =?utf-8?Q?F9jB6XFXe/7CFvvl/y2NXVSpK?=
X-MS-Exchange-CrossTenant-Network-Message-Id: a2125dfc-3628-41ca-517e-08dc39737762
X-MS-Exchange-CrossTenant-AuthSource: BL1PR11MB5978.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Feb 2024 22:12:06.8394
 (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: WQ1UCqC2zQpHk2DyfrBepgpf9cBbjgmAd9sALQBGTOAtuxuvtQLNur3NQfNkDkvuyNVqwTdInIjop/A91CuyOw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN9PR11MB5243
X-OriginatorOrg: intel.com


On 28/02/2024 3:41 pm, Sean Christopherson wrote:
> WARN if bits 63:32 are non-zero when handling an intercepted legacy #PF,

I found "legacy #PF" is a little bit confusing but I couldn't figure out 
a better name either :-)

> as the error code for #PF is limited to 32 bits (and in practice, 16 bits
> on Intel CPUS).  This behavior is architectural, is part of KVM's ABI
> (see kvm_vcpu_events.error_code), and is explicitly documented as being
> preserved for intecerpted #PF in both the APM:
> 
>    The error code saved in EXITINFO1 is the same as would be pushed onto
>    the stack by a non-intercepted #PF exception in protected mode.
> 
> and even more explicitly in the SDM as VMCS.VM_EXIT_INTR_ERROR_CODE is a
> 32-bit field.
> 
> Simply drop the upper bits of hardware provides garbage, as spurious

"of" -> "if" ?

> information should do no harm (though in all likelihood hardware is buggy
> and the kernel is doomed).
> 
> Handling all upper 32 bits in the #PF path will allow moving the sanity
> check on synthetic checks from kvm_mmu_page_fault() to npf_interception(),
> which in turn will allow deriving PFERR_PRIVATE_ACCESS from AMD's
> PFERR_GUEST_ENC_MASK without running afoul of the sanity check.
> 
> Note, this also why Intel uses bit 15 for SGX (highest bit on Intel CPUs)

"this" -> "this is" ?

> and AMD uses bit 31 for RMP (highest bit on AMD CPUs); using the highest
> bit minimizes the probability of a collision with the "other" vendor,
> without needing to plumb more bits through microcode.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>   arch/x86/kvm/mmu/mmu.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/arch/x86/kvm/mmu/mmu.c b/arch/x86/kvm/mmu/mmu.c
> index 7807bdcd87e8..5d892bd59c97 100644
> --- a/arch/x86/kvm/mmu/mmu.c
> +++ b/arch/x86/kvm/mmu/mmu.c
> @@ -4553,6 +4553,13 @@ int kvm_handle_page_fault(struct kvm_vcpu *vcpu, u64 error_code,
>   	if (WARN_ON_ONCE(fault_address >> 32))
>   		return -EFAULT;
>   #endif
> +	/*
> +	 * Legacy #PF exception only have a 32-bit error code.  Simply drop the

"have" -> "has" ?

> +	 * upper bits as KVM doesn't use them for #PF (because they are never
> +	 * set), and to ensure there are no collisions with KVM-defined bits.
> +	 */
> +	if (WARN_ON_ONCE(error_code >> 32))
> +		error_code = lower_32_bits(error_code);
>   
>   	vcpu->arch.l1tf_flush_l1d = true;
>   	if (!flags) {
Reviewed-by: Kai Huang <kai.huang@intel.com>