Received-SPF: pass (google.com: domain of linux-kernel+bounces-86772-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <c0d80c37-ff1c-9c94-e1ac-78d26ee4da15@oracle.com>
Date: Thu, 29 Feb 2024 05:43:55 -0800
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.15.1
Subject: Re: [PATCH 02/16] KVM: x86: Remove separate "bit" defines for page
 fault error code masks
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-3-seanjc@google.com>
From: Dongli Zhang <dongli.zhang@oracle.com>
In-Reply-To: <20240228024147.41573-3-seanjc@google.com>
Content-Type: text/plain; charset=UTF-8
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?SmFxcWdzTmx1WWZNT1V5V2VtTWlCQWJYWElYZ1BDZTh0dXJlSUlsTWtTTitT?=
 =?utf-8?B?bWVUZ2h5TTkvSDlyUVVrS3cweGxZL0xZZHV0OXIxV2N4SzhiS2pHQVMyQUxo?=
 =?utf-8?B?OUZiZ0NkUHdDVlUwM1BUREJMcjV2SHFDcDJod0sxOVYvOHVpVzJXaTRYQlBZ?=
 =?utf-8?B?ZitYbklBa0FGREt5dmpjaXRzZS9ieHpaSEFHallxdWJqcGlndmpIUVhZc3JV?=
 =?utf-8?B?cXlKNmxLMkFOVjZpdVArd2hvVFV4UEZVRW9pcjdJNlRPZU1XQjBvbGM1VUJ3?=
 =?utf-8?B?YmYySGJmaE8vbzhKV2NkWVdDZGFyTDhWR3BSM05NcFVkTHdhZkV6cVNwcDkv?=
 =?utf-8?B?VzFacHU2aTBpY2NwQjZZVVZxR1N6ZlF5VDhHdWdtazRmaE5rRGVEZzhHanlm?=
 =?utf-8?B?ZnZWUFJzWmxiLzVaQ3U4MEZhVitZMkQvZjYvUkg3TW10a3IyTDBVTkNUN2R4?=
 =?utf-8?B?M3dGWE5PNWN3U3d2RDduS3FSWHp0S0xuOW45MktGeUJBYUlHN1VLdlFVWUtQ?=
 =?utf-8?B?MDdJM1o4V2Y4dlp1dXZQU1JGYmU2bjJwM255R08wOTBNeTBEdDF4ckJuSkZ2?=
 =?utf-8?B?Wkt2N1ZxcGxscHhqZHVHb0JnWVRBZXRGT2k1RHZPZFE1anYwL2NuM1kxd1Js?=
 =?utf-8?B?U0JTeTNvc1hIZ1JodU8xVXByWlBTUlhROWVZRlN5TjNhSkZOTVh1ZnBWOVc4?=
 =?utf-8?B?U2o3Q1lsTzNNT1UxSGFNOVJKd1lLV3NtdDEvVmtGUHhqbGRZblpYTldiZ0Jp?=
 =?utf-8?B?OGs4elNmU1h0a24remtaTkxSdDk3VjVWOEpqS0U0THFSRGVZaGxVSlI2Qkwy?=
 =?utf-8?B?VDlzbHNYT0RPMTRBcjJMaGVjSmw0TWd0YlVIaFh2b21JM21QL3k5R2xIU2hN?=
 =?utf-8?B?RWJpdk1FUThPZHltUGpqSjZvT2V2M3lvYStvUWxIYmNXNjlnSy9ieVV3anpB?=
 =?utf-8?B?Z1ZqcnNuMm9RL21FcVNObUlQQi85L0xHNXpQSTdnZDU1M0JacXI3K1F1REgy?=
 =?utf-8?B?NUVoV0dmNmwycmFaRDQreWQxeFpTelBpaXhtd0xKS295T2gwYXVrNGg0S29E?=
 =?utf-8?B?QTZLeWdNNEhxNVNuRkVHdDB3YlJjUGxrRTZyRVE1b0JNVVhKOXZRQWhwT1lX?=
 =?utf-8?B?dEhRbzVORHNjcUZrYzcxOHhqTlhLZ1dYd3RGM3ZlQ05hQkJ3QzJOUlkrdVd6?=
 =?utf-8?B?bXo4NlpKZGFvaDF4SjJ4M1R0Z0NWRGY2RkZDd2FlMks1a2ZzYSttS2JESTRY?=
 =?utf-8?B?ZmJ1YnJMZk9nZjRjbUQvQXUxeVF3TW5WWnFDYk80Q3lISTg1Z01ZUmVTS1RL?=
 =?utf-8?B?VU53cjhRY2VMQlNzSWFvVnVvTW9rTWtWV3hta2FudW10NFlkKzJybUo3czFU?=
 =?utf-8?B?ZFNaWE5XaS9xZEl3Y0V4OGppRXNKMGFBKzl6SGJteHVsVGVCY093dDNDRk5z?=
 =?utf-8?B?TzRMbHhic3FRb2E1QWM3dTZ6SHJwb2lvWWZhRTVmeTdlakRvbEtodjRkM2Ev?=
 =?utf-8?B?Wm85M3N1SkNKVm9aV2lHcTIrWUcydWw0dkRjQjgybDRPYW5wZWZXV2tiWlFh?=
 =?utf-8?B?SXBOajlCRXBIQzYzMUJoOEFDRWplQ2ljeWZMSDhBNW5TS0l5RUNPRU1BZWxj?=
 =?utf-8?B?REN1bFRKNXVBUENwTEttRjZxTFNsekJEVFZDNWRKNmI3aDJTMVlPbGNCN0Ja?=
 =?utf-8?B?TFNVbVl1QUlIOEpwY0V3dE9CUFFaeVF6VzdrcFlRb2xQOEtOU1IvZVZMM1NC?=
 =?utf-8?B?UDRpc3J0ckh2eXpxTmJMOERwcDEyY0ZUdjFrenNzV2dtMXp2TWFicjFvNHY4?=
 =?utf-8?B?ZE9IcURLZVNXMEhaK3Zkb1p3VmRtTytBUkUzdUlWRUltTHk0bHVIMnFHOFA5?=
 =?utf-8?B?aEhwWndZNkVsZDFNMFNTSytBVXIzZzJvSTNxbXFmZ1ZhSnVnYU1KSDREb3hL?=
 =?utf-8?B?ZTZvMU9US2pBK25kNGlhalZIZmVhRFpKSC9Qb3doekgvWE5xeXNXVmlGNDBh?=
 =?utf-8?B?dHREZFo1MmZuQ1ZCbFkwZjNGTnFyT1dPd2g2ZGZJaFRhZHY4c0JFZU92SlBm?=
 =?utf-8?B?UnlVRlN3eTNkYk5nMDRkQk5maDVFeFF5TzRPczhrMitHQVNRRG5YQ3dZWXFQ?=
 =?utf-8?B?MnFCMGovbGZXenowYlhDUnArTGNCSUxmRS9MSlFEUlVtS25UWER4SlJHTGVS?=
 =?utf-8?B?TkE9PQ==?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	MfMyGHsx0zXE51fFqTxibgDDE+YLJPwEOkbnbiVy8SJKynJZYoyOiRWlRW36CjVypt+IZdULRm8ZFOihd35HlVk9buTxb4S4m/EU7mNEUYQePkw1aroxNZm2+QGVNR4DRU2MvpgUw9vMIi4UYIMKOsux4xAZyXt+wddJEj9eILqMcGPFXLBvFjj0cNv9u8nL5qh7Je4IpJks2nVv4ZXISQJ/lcC9UbfMbGkKZsJvXRC98UimiQtsLP4ddR0TI4+Mv1y+OS1M4vbTDtrfvzYd1j4Rbt3Rd9PZS09SlenB3JKrdx2rmzz4vAu/3zULzVQ/Z48UesQVydF3vPGcJfvYCyBLy4pa4R9FP3syXQxAatudhzUt4jL8gjj6RbnD7H2IuIGUgJ/DvCSF/wcC53ayPKE5yv7oHPqXIjooLZhKvkQosCpv6WXfT5S1WdA4SNkzBjKPhKN6s0xSZDG/uJA4m+jW16IRDjNDSioNV4tL0z2AuWShNSdCVPC9WrbDYRAW5y19R5qwHpMykNpkQ1LDHoWX0YqvaVTEA0orESMm4PlEy5VXRIhMSMb1Rx/vv+gitSbMHr1eD81NcwNNECwjFH2joe0hWiohVIEmougR3iA=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: ae1f83ca-f746-4b00-cff5-08dc392c7a13
X-MS-Exchange-CrossTenant-AuthSource: BYAPR10MB2663.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 29 Feb 2024 13:43:57.0232
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 8kX1beN7byWwwzsGnVCxE4AV9YCuSk/UmYD0p9+RWihHsYrnYDNJ3BLWn+3Udf2hqc59AiyMUVYfdPvLF4FlDQ==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR10MB4921
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-02-29_02,2024-02-29_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 adultscore=0 bulkscore=0 suspectscore=0
 spamscore=0 phishscore=0 malwarescore=0 mlxscore=0 mlxlogscore=999
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2311290000
 definitions=main-2402290106
X-Proofpoint-ORIG-GUID: g1Kxm_gMzc-8jV5TNxJpMXF7mUwXoSSW
X-Proofpoint-GUID: g1Kxm_gMzc-8jV5TNxJpMXF7mUwXoSSW

I remember I read somewhere suggesting not to change the headers in selftest.

Just double-check if there is requirement to edit
tools/testing/selftests/kvm/include/x86_64/processor.h.

Dongli Zhang

On 2/27/24 18:41, Sean Christopherson wrote:
> Open code the bit number directly in the PFERR_* masks and drop the
> intermediate PFERR_*_BIT defines, as having to bounce through two macros
> just to see which flag corresponds to which bit is quite annoying, as is
> having to define two macros just to add recognition of a new flag.
> 
> Use ilog2() to derive the bit in permission_fault(), the one function that
> actually needs the bit number (it does clever shifting to manipulate flags
> in order to avoid conditional branches).
> 
> No functional change intended.
> 
> Signed-off-by: Sean Christopherson <seanjc@google.com>
> ---
>  arch/x86/include/asm/kvm_host.h | 32 ++++++++++----------------------
>  arch/x86/kvm/mmu.h              |  4 ++--
>  2 files changed, 12 insertions(+), 24 deletions(-)
> 
> diff --git a/arch/x86/include/asm/kvm_host.h b/arch/x86/include/asm/kvm_host.h
> index aaf5a25ea7ed..88cc523bafa8 100644
> --- a/arch/x86/include/asm/kvm_host.h
> +++ b/arch/x86/include/asm/kvm_host.h
> @@ -254,28 +254,16 @@ enum x86_intercept_stage;
>  	KVM_GUESTDBG_INJECT_DB | \
>  	KVM_GUESTDBG_BLOCKIRQ)
>  
> -
> -#define PFERR_PRESENT_BIT 0
> -#define PFERR_WRITE_BIT 1
> -#define PFERR_USER_BIT 2
> -#define PFERR_RSVD_BIT 3
> -#define PFERR_FETCH_BIT 4
> -#define PFERR_PK_BIT 5
> -#define PFERR_SGX_BIT 15
> -#define PFERR_GUEST_FINAL_BIT 32
> -#define PFERR_GUEST_PAGE_BIT 33
> -#define PFERR_IMPLICIT_ACCESS_BIT 48
> -
> -#define PFERR_PRESENT_MASK	BIT(PFERR_PRESENT_BIT)
> -#define PFERR_WRITE_MASK	BIT(PFERR_WRITE_BIT)
> -#define PFERR_USER_MASK		BIT(PFERR_USER_BIT)
> -#define PFERR_RSVD_MASK		BIT(PFERR_RSVD_BIT)
> -#define PFERR_FETCH_MASK	BIT(PFERR_FETCH_BIT)
> -#define PFERR_PK_MASK		BIT(PFERR_PK_BIT)
> -#define PFERR_SGX_MASK		BIT(PFERR_SGX_BIT)
> -#define PFERR_GUEST_FINAL_MASK	BIT_ULL(PFERR_GUEST_FINAL_BIT)
> -#define PFERR_GUEST_PAGE_MASK	BIT_ULL(PFERR_GUEST_PAGE_BIT)
> -#define PFERR_IMPLICIT_ACCESS	BIT_ULL(PFERR_IMPLICIT_ACCESS_BIT)
> +#define PFERR_PRESENT_MASK	BIT(0)
> +#define PFERR_WRITE_MASK	BIT(1)
> +#define PFERR_USER_MASK		BIT(2)
> +#define PFERR_RSVD_MASK		BIT(3)
> +#define PFERR_FETCH_MASK	BIT(4)
> +#define PFERR_PK_MASK		BIT(5)
> +#define PFERR_SGX_MASK		BIT(15)
> +#define PFERR_GUEST_FINAL_MASK	BIT_ULL(32)
> +#define PFERR_GUEST_PAGE_MASK	BIT_ULL(33)
> +#define PFERR_IMPLICIT_ACCESS	BIT_ULL(48)
>  
>  #define PFERR_NESTED_GUEST_PAGE (PFERR_GUEST_PAGE_MASK |	\
>  				 PFERR_WRITE_MASK |		\
> diff --git a/arch/x86/kvm/mmu.h b/arch/x86/kvm/mmu.h
> index 60f21bb4c27b..e8b620a85627 100644
> --- a/arch/x86/kvm/mmu.h
> +++ b/arch/x86/kvm/mmu.h
> @@ -213,7 +213,7 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  	 */
>  	u64 implicit_access = access & PFERR_IMPLICIT_ACCESS;
>  	bool not_smap = ((rflags & X86_EFLAGS_AC) | implicit_access) == X86_EFLAGS_AC;
> -	int index = (pfec + (not_smap << PFERR_RSVD_BIT)) >> 1;
> +	int index = (pfec + (not_smap << ilog2(PFERR_RSVD_MASK))) >> 1;
>  	u32 errcode = PFERR_PRESENT_MASK;
>  	bool fault;
>  
> @@ -235,7 +235,7 @@ static inline u8 permission_fault(struct kvm_vcpu *vcpu, struct kvm_mmu *mmu,
>  
>  		/* clear present bit, replace PFEC.RSVD with ACC_USER_MASK. */
>  		offset = (pfec & ~1) +
> -			((pte_access & PT_USER_MASK) << (PFERR_RSVD_BIT - PT_USER_SHIFT));
> +			((pte_access & PT_USER_MASK) << (ilog2(PFERR_RSVD_MASK) - PT_USER_SHIFT));
>  
>  		pkru_bits &= mmu->pkru_mask >> offset;
>  		errcode |= -pkru_bits & PFERR_PK_MASK;