Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:1 as permitted sender) client-ip=2620:137:e000::3:1;
Message-ID: <18f95549-8971-4b03-8569-61d9b1763364@intel.com>
Date:   Mon, 27 Nov 2023 10:55:48 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v7 02/26] x86/fpu/xstate: Refine CET user xstate bit
 enabling
To:     Peter Zijlstra <peterz@infradead.org>
CC:     <seanjc@google.com>, <pbonzini@redhat.com>,
        <dave.hansen@intel.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <chao.gao@intel.com>,
        <rick.p.edgecombe@intel.com>, <mlevitsk@redhat.com>,
        <john.allen@amd.com>
References: <20231124055330.138870-1-weijiang.yang@intel.com>
 <20231124055330.138870-3-weijiang.yang@intel.com>
 <20231124094029.GK3818@noisy.programming.kicks-ass.net>
Content-Language: en-US
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <20231124094029.GK3818@noisy.programming.kicks-ass.net>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?YnIwUGlablV6OXh2bExvYVZyVk1ib1JFaFNydUNQcjdVeDNYZjMvZXROaUZl?=
 =?utf-8?B?Smt0bmdHVDkzM1RHbi9hZG5SNk9Vd2h0WkloVmJYcnBTOXJubGZ1NjlDSWxj?=
 =?utf-8?B?ckM1d2VSZG94cEZ6TTVDcnBOQklJRUxaclVHRVlvVkNRaTl6SitIY2J1SDRM?=
 =?utf-8?B?K3owRlpnbGt6T3orTE1aZkxYSllXb2cvQjhDU05OSW5qL29OSHNwek80SGdU?=
 =?utf-8?B?QWtsYVpRSytWZnE4VFRYYXNqSE82dWo3VStFRGxTQitWZEZkZjRSL3BUWVdx?=
 =?utf-8?B?YVEwV0NYQ3htWnpHb1JUcGYvdEUvSkdUbzVKa3NBcFRYOGlubkdQWVJXSk9S?=
 =?utf-8?B?REFKZFMxVmZYWnBZN1FQK2xQUVEwWDNiZWwwdU0rR3NNVGNGbHZDN2htclVL?=
 =?utf-8?B?M3Y1RllKcVZzWk9qOFZKcFFRbEJoMi9kbnBQODZFbXUxVENzdkN4QnJHNERL?=
 =?utf-8?B?Tll6Q0xtem9iZFFoTFBaQUN0VkxNbVdyMDllbklWRk5wTTRmZ3RhYUtiR0pU?=
 =?utf-8?B?SXRSVS9EaHdwM2pMT1FNK1lrcnpWOUcvNU9keGRhTEVwTW50SmIvN0FQcjYx?=
 =?utf-8?B?eStrUVVtRDU5MUh6ZnhnWWNkeExFaUZveWFrbW1rZ1JmMHVlQ1U2amdQODgx?=
 =?utf-8?B?MzRQZlc3V0NiSXVmWDNPd2JGTDNRKzJFVGx4V3dpT2dsTkFFTUl6bXRHTGJy?=
 =?utf-8?B?RWM1R0pGRkdiUVpsdjl0UDBRRVEzc1NJMURDVEI1V25ydGwyRVdwVk93eExS?=
 =?utf-8?B?TGFpa3RsVXl5TzRCRDJsZzhqMVVFaXhzdHR1V21YZlhlcks4ZjVyY0JCNUxt?=
 =?utf-8?B?STl0ZDgrdEZBcktqUzAwTU5yU09obnFQSVlsUGN0aFFucXZBM084TkpzRUk2?=
 =?utf-8?B?U05PdkpGcWZiV1NZZmhJcnNkaGd0eEJWaWxDY3UyNGZCUFdoalEzUDdaS2JK?=
 =?utf-8?B?Kys1Q29yaGEzV1JZR1FhU1pJd0VmM09TYmRFV3dDaGJwd1VicGUyNVEwc3Iw?=
 =?utf-8?B?VnpUeDhqS2pMWTBzWVlUZVdnV0J3aU5ndXp0dFNMQk1QYkIzYWdCSHJlbnp5?=
 =?utf-8?B?NXVqMUI1YjlhWStUSVpGV0wwaTEyVW91ZlhBclFWM3d4WDVlY3NSeFUwV2ZV?=
 =?utf-8?B?UzlGQlBvWHBwcnNnQmprTjdDb2lIVWx3TDcrQm5Sc3N3cHoxeGtxaGRzOXdn?=
 =?utf-8?B?Vy8zMW40WTJESFZTTlg0cXJLRDVtM0xUWWt3ckRoK2FNdDZDYWhqaThQWjhi?=
 =?utf-8?B?cGVZTUV0cG54UWNyeGhIWWV3RTF3cFZzajRBSGxnYjJOU0IzNzY5VGZLNk14?=
 =?utf-8?B?YTdTUVY5ajZJZ0lJZGk5V2docW03aUtHOWpMbklRK3lMaE5XNTh3OGpDd2FQ?=
 =?utf-8?B?ZGZIeDRrSUxLRE9wajVtWFhYbEYyOFU0clFnQVpXaVpSN2U5U0plV0ZpVGI0?=
 =?utf-8?B?QTVsa1hCc0JZbGk5WEY1OFFDM0V3L3lqWHY4ck4rMGtPV1JRakZzMjBWUjVz?=
 =?utf-8?B?dHdrNCtDT0pWMG9SSXF6blpjV21kS1RkWmZiWnJEVDdrSm1OM0laMzhXK1Vk?=
 =?utf-8?B?S0pLckFqeHB2UnBzUTRsTU9SOU9DMThMeGI0WWZ3ZkZGanRia0NXSTk5VmI5?=
 =?utf-8?B?Sk1GMTdZMHUxNEZoZU1ZOTkrYlBDaTlUV2t6NmYxM0F5ZDZFUEtFbVZ3ekFY?=
 =?utf-8?B?d0QxMTVjLy9qeFJyam1UZTI3Nk54RUNSUjY0b3ZNOFd1Mm14NlBNQlM4WUF5?=
 =?utf-8?B?VWd0TDdpNHcrajRsTERqYkhROGNCa2VvN01SaFJ3T01BZFR3eFRLTGZBazBG?=
 =?utf-8?B?N05XS1UzdUd0QjI1Yndod1RrRVdKbmdwdS9GNkYxaWlyaDVZS0R0eGNtT1F2?=
 =?utf-8?B?NFk0RjJUVnBiYzlsOGFYako1Q2R0aGRzcndHbFdMYUpIOUxlME5HTFRrOGFT?=
 =?utf-8?B?ZDY2ZTFGcitkNWxMbHhjQm0xMlZEcjR2dXJ1SjVqS0Fpb01CaE5QTWh3UVFI?=
 =?utf-8?B?L0dLUzhrYTdwQUtCMHFaUm5CdTZXdERJS0IxNFFNcHVWREJGOTdKOG9JVnVQ?=
 =?utf-8?B?RDNsSzJLbm5CVytkVUFpd0pzQnJZVTY0K0pvdzNNd0VjYTJneVFlRGdKUkJG?=
 =?utf-8?B?Q3VyUFc1MldPQ3FiWjZEeWRLNU1Vd0k5QWp3bjVRbWt6a1NCSXRQclFIbmIv?=
 =?utf-8?B?TUE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 46ee3fec-8d6d-4a9f-4ded-08dbeef461ce
X-MS-Exchange-CrossTenant-AuthSource: SA2PR11MB4972.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 27 Nov 2023 02:55:58.6217
 (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: Rw+eJ+wWltp2LdnU3jsJaDxYJIGXIQ9V8yHJlHB0KRtQuNYY8fkxsfk7UEg/Zns+15mSxK9nADndEQOYtx95vw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB6142
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE
	autolearn=unavailable autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on morse.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (morse.vger.email [0.0.0.0]); Sun, 26 Nov 2023 18:56:13 -0800 (PST)

On 11/24/2023 5:40 PM, Peter Zijlstra wrote:
> On Fri, Nov 24, 2023 at 12:53:06AM -0500, Yang Weijiang wrote:
>> Remove XFEATURE_CET_USER entry from dependency array as the entry doesn't
>> reflect true dependency between CET features and the user xstate bit.
>> Enable the bit in fpu_kernel_cfg.max_features when either SHSTK or IBT is
>> available.
>>
>> Both user mode shadow stack and indirect branch tracking features depend
>> on XFEATURE_CET_USER bit in XSS to automatically save/restore user mode
>> xstate registers, i.e., IA32_U_CET and IA32_PL3_SSP whenever necessary.
>>
>> Note, the issue, i.e., CPUID only enumerates IBT but no SHSTK is resulted
>> from CET KVM series which synthesizes guest CPUIDs based on userspace
>> settings,in real world the case is rare. In other words, the exitings
>> dependency check is correct when only user mode SHSTK is available.
>>
>> Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>> Reviewed-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>> Tested-by: Rick Edgecombe <rick.p.edgecombe@intel.com>
>> ---
>>   arch/x86/kernel/fpu/xstate.c | 9 ++++++++-
>>   1 file changed, 8 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/x86/kernel/fpu/xstate.c b/arch/x86/kernel/fpu/xstate.c
>> index 73f6bc00d178..6e50a4251e2b 100644
>> --- a/arch/x86/kernel/fpu/xstate.c
>> +++ b/arch/x86/kernel/fpu/xstate.c
>> @@ -73,7 +73,6 @@ static unsigned short xsave_cpuid_features[] __initdata = {
>>   	[XFEATURE_PT_UNIMPLEMENTED_SO_FAR]	= X86_FEATURE_INTEL_PT,
>>   	[XFEATURE_PKRU]				= X86_FEATURE_OSPKE,
>>   	[XFEATURE_PASID]			= X86_FEATURE_ENQCMD,
>> -	[XFEATURE_CET_USER]			= X86_FEATURE_SHSTK,
>>   	[XFEATURE_XTILE_CFG]			= X86_FEATURE_AMX_TILE,
>>   	[XFEATURE_XTILE_DATA]			= X86_FEATURE_AMX_TILE,
>>   };
>> @@ -798,6 +797,14 @@ void __init fpu__init_system_xstate(unsigned int legacy_size)
>>   			fpu_kernel_cfg.max_features &= ~BIT_ULL(i);
>>   	}
>>   
>> +	/*
>> +	 * CET user mode xstate bit has been cleared by above sanity check.
>> +	 * Now pick it up if either SHSTK or IBT is available. Either feature
>> +	 * depends on the xstate bit to save/restore user mode states.
>> +	 */
>> +	if (boot_cpu_has(X86_FEATURE_SHSTK) || boot_cpu_has(X86_FEATURE_IBT))
>> +		fpu_kernel_cfg.max_features |= BIT_ULL(XFEATURE_CET_USER);
> So booting a host with "ibt=off" will clear the FEATURE_IBT, this was
> fine before this patch-set, but possibly not with.
>
> That kernel argument really only wants to tell the kernel not to use IBT
> itself, but not inhibit IBT from being used by guests.

Thanks for pointing this out!
If ibt=off actually causes XFEATURE_CET_USER unset in fpu_kernel_cfg.max_features, in KVM part (patch 24), we already have below check to disable SHSTK/IBT in this cases, so looks like it won't bring issues.

+if ((kvm_caps.supported_xss & (XFEATURE_MASK_CET_USER |

+XFEATURE_MASK_CET_KERNEL)) !=

+(XFEATURE_MASK_CET_USER | XFEATURE_MASK_CET_KERNEL)) {

+kvm_cpu_cap_clear(X86_FEATURE_SHSTK);

+kvm_cpu_cap_clear(X86_FEATURE_IBT);

+kvm_caps.supported_xss &= ~XFEATURE_CET_USER;

+kvm_caps.supported_xss &= ~XFEATURE_CET_KERNEL;

+}

+

>