Received-SPF: pass (google.com: domain of linux-kernel+bounces-187309-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:4601:e00::3 as permitted sender) client-ip=2604:1380:4601:e00::3;
Message-ID: <a0e1a41f-e3f2-48f0-87f6-f065b5e5c0e5@intel.com>
Date: Thu, 23 May 2024 18:07:40 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v10 24/27] KVM: x86: Enable CET virtualization for VMX and
 advertise to userspace
To: "Edgecombe, Rick P" <rick.p.edgecombe@intel.com>, "Hansen, Dave"
	<dave.hansen@intel.com>
CC: "Gao, Chao" <chao.gao@intel.com>, "seanjc@google.com" <seanjc@google.com>,
	"x86@kernel.org" <x86@kernel.org>, "peterz@infradead.org"
	<peterz@infradead.org>, "john.allen@amd.com" <john.allen@amd.com>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"tglx@linutronix.de" <tglx@linutronix.de>, "kvm@vger.kernel.org"
	<kvm@vger.kernel.org>, "pbonzini@redhat.com" <pbonzini@redhat.com>,
	"mlevitsk@redhat.com" <mlevitsk@redhat.com>
References: <20240219074733.122080-1-weijiang.yang@intel.com>
 <20240219074733.122080-25-weijiang.yang@intel.com>
 <ZjLNEPwXwPFJ5HJ3@google.com>
 <39b95ac6-f163-4461-93f3-eaa653ab1355@intel.com>
 <ZkYauRJBhaw9P1A_@google.com> <87r0e0ke8w.ffs@tglx>
 <ZkdpKiSyOwB3NwRD@google.com>
 <a170e420-efc3-47f9-b825-43229c999c0d@intel.com>
 <ZkuD1uglu5WFSoxR@google.com>
 <df5fa770-1f9b-4fa7-a20f-57f51b0d345b@intel.com>
 <12ccd0d3-e9a8-47b6-9564-7146d0a79f3a@intel.com>
 <40fd07cbd39c8c8911b2e46f2314f7dbf20d4a9a.camel@intel.com>
Content-Language: en-US
From: "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <40fd07cbd39c8c8911b2e46f2314f7dbf20d4a9a.camel@intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 8bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cUIyQWNuRlJhOENPQTc4UU4zM2d2cElHdi9zNW5YQ2hSRHN4bG1FbFlKV2lG?=
 =?utf-8?B?S3hsWkhhZldrc281ZjRoNC82U3dMQitwRGxla3Bud1Q1L21hV0lXYlJZM0k0?=
 =?utf-8?B?MzNYRytESzYzVnJBMUMxajlhZ3NrRFNubFd3MXVuZnRvbFZ6TTlUNWpjRXdF?=
 =?utf-8?B?cUExQ0xiZGhsUW5mbzJ4Z2lVUzZtZUkyV1gwaWIyS09kU2RTUk5UNkRyYlBE?=
 =?utf-8?B?SU0xWUxlSm1sd1BST0ptQ3ZWRlBkUG1LbHJjc2JZdXlkQTNNdmtjWmd4VVFo?=
 =?utf-8?B?MjhWMmxZUldjL3ZlZEZCeWJoM1lCOEpzSWFBU2NVeHIrOCs5YlFqVW5QOXB6?=
 =?utf-8?B?WlVybFFrZFVTTnFnanJJWjdLUWhCNHpXWDhGUGU3cXFzRy9XaTVHZ1lGN3dD?=
 =?utf-8?B?Y1AyZGxUL2FUdU40VzFYTlRTTk85SWg5Wkk2MEp6eVJOeWcvcE1OUVVJL0hL?=
 =?utf-8?B?TmlFc1NoN2hpTmFNekIrSVhTbnpGSDg4V25YaXdGa09MY0R5UzQydkJYSkRT?=
 =?utf-8?B?d2Jha21Yd3NuQktBVTdySVhHWkRrWlUvK0Zmc0dqQmJNME9BZjduQXBFdHlR?=
 =?utf-8?B?OW5mOXNvWHJpNjN4SVY3Z2dtdjZralB0UTM4Nit5WmY0bW8zWjFFZWhNRGZi?=
 =?utf-8?B?aSsyQ3VuTVhhREFXY1pSeHkwWDROeGxRaU9OVnVqVXF0TzJFRzZBRzMzWHkw?=
 =?utf-8?B?RFJiaHFGK29aek1Ud1g2VnBYQ256M2xBMjNhaWczQzhBVGVjSGZPeC9hWFFl?=
 =?utf-8?B?eHhENFp1M2FHUHJNbTBob2ZiNUZ2dFlCL2IzRUJ2bDJta3lMMUFHY1FERk80?=
 =?utf-8?B?TlY0Uld3d0hscXgvazVGMUdTVU5ub1ZmZEVaRzFFWXVaME5tZGxlUThtTkdz?=
 =?utf-8?B?My9hR2RDTTV5K3N3U0luNVBldm5Jb2xUZ1ptWXNnZ1R2Y1MyZ0NTb1FzaGE2?=
 =?utf-8?B?eG1WTStTMFFHd2lOU0F0ZVF6T3k3bjlMN1gwMFhMazQwZUp1UnlkM3lRVW9B?=
 =?utf-8?B?Ym1EU25JWXIyVGVLaUoxTUtJNG5TOVBGYUFjUTFWK00waXFKYUZ1b3lQYzRz?=
 =?utf-8?B?VzVaMitBcUNGSnV4ZEpSS28wMXVuanVmdGdlM2doNHdjU0NaTHI2WDhJYW9x?=
 =?utf-8?B?aDJvT2ZuWHVQNVJQZThTVFJrTUYvaHpJSUtOZmZKekRvckN2aHRlTlVZVm5p?=
 =?utf-8?B?ai96MjMwbVZRVGVCbTJmdkZHdHBvQkJzNmhBcGp4Zy8ycjcyT05Sb2JRRnZp?=
 =?utf-8?B?OVRPTlZOajI2aytqRjVGT05JNm8xVFhPd3NDSW5jVFFGT2NZSkRVOWRFMzU3?=
 =?utf-8?B?YVl0a1o0cm1EdUFENFUxa1pGUDhJa2k0bmRBZTlseXVRVGdTTi9WSHRzWEE2?=
 =?utf-8?B?a2pSdjNzZVVFUytvSjFLcW9XdkRrNHZvZ0hDVlg0eTNvdWpRdXYwaHdBVVZh?=
 =?utf-8?B?QXJVN1hSWUF3aGlsZnlPQkpUWkdrV2hQRjZaWDN3R3VLNFZoTEI0OTBUc1Ri?=
 =?utf-8?B?Q0h3Rkx6WEt2bmllWVAwVXRLWms4ZUxyR2NKc0NDM1Y1S0ZXd21vL3pVRFF2?=
 =?utf-8?B?YlB0S2l5VjQ4dzk3ZGtxdmQ4V3FNQkt6K3F3RGlzWnMxSWZLTkNCeGNHbVJ4?=
 =?utf-8?B?RzVIN3Y4RVYyUUtlekpWV3hoc1hCR1d4eGlMQzIrcytTeTBKZ0svUEludDRT?=
 =?utf-8?B?cDA1SzhBM08yOEpwb0RTMmpvQWdMTHpBRmhuMURoK0hBVFJESC9udGRRcTBq?=
 =?utf-8?B?S0xsM3lCMHZlZFNWaERVc1VFMW1adjhoRzRWNU40cEpnUjhvUEl2VGZKa0JR?=
 =?utf-8?B?bWE2alAxeHc4dWpicHZGdlJ2dzRuQy83eERoL1JkYTEvNk9LVDVXMXZ5eElt?=
 =?utf-8?B?M0NlMGd0dVVWSmlaVG9SYVpSekRVcHptL0tGdFRHK0d1bCtMNkE4eUROOTR5?=
 =?utf-8?B?NE10TXZCVjZvREhIWHBEVEVNaDdxY0hWWEVoSEVhU1FKMnorL2E3TzdTOG5z?=
 =?utf-8?B?U05mVUY4N0hxTGxSVjJrNXptMXNBTnBjMkk2NjZPd21QWFBxNS8xUFVRWUV0?=
 =?utf-8?B?Tm9odWZSWC9JMzJIdlQ5SStEWjBBMTJHSXlYSDAvdndGSHpWTi9YaTdjcEgw?=
 =?utf-8?B?bkRCdmszN3pUSXFnY0Vpb0g1MCtUZjZFSFIwckFPbmoyMXh2Vm4yV1A1VnRv?=
 =?utf-8?B?SFE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 510a4911-12cd-47e2-214a-08dc7b10334a
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 May 2024 10:07:49.3712
 (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: f0JvHYTtwT2XuJHmFhv1pOgXTVjtxLy/RPyZxP2UImtTb/X9VZ6VFDuHLT3E/2zC1/qgNN5VzNjkJJnTb/e4Mw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SN7PR11MB6776
X-OriginatorOrg: intel.com

On 5/22/2024 11:06 PM, Edgecombe, Rick P wrote:
> On Wed, 2024-05-22 at 17:03 +0800, Yang, Weijiang wrote:
>> Side topic:  would it be reasonable to enforce IBT dependency on
>> XFEATURE_CET_USER when *user* IBT
>> enabling patches are landing in kernel? Then guest kernel can play with user
>> IBT alone if VMM
>> userspace just wants to enable IBT for guest. Or when SHSTK is disabled for
>> whatever reason.
> I think earlier there was a comment that CET would be less likely to need to be
> disabled for security reasons, so there would not be utility for a system wide
> disable (that affects KVM). I recently remembered we actually already had a
> reason come up.
>
> The EDK2 SMI handler uses shadow stack and had a bug around saving and restoring
> CET state. Using IBT in the kernel was causing systems to hang. The temporary
> fix was to disable IBT.
>
> So the point is, let's not try to find a narrow way to get away with enabling as
> much as technically possible in KVM.
>
> The simple obviously correct solution would be:
> XFEATURE_CET_USER + XFEATURE_CET_KERNEL + X86_FEATURE_IBT = KVM IBT support
> XFEATURE_CET_USER + XFEATURE_CET_KERNEL + X86_FEATURE_SHSTK = KVM SHSTK support

Yes, I can easily achieve it by removing the raw cpuid check for KVM IBT. Host side CET xstate
support check is already there in this patch.

>
> It should be correct both with and without that patch to enable
> XFEATURE_CET_USER for X86_FEATURE_IBT.

IMHO, given the fact user IBT hasn't been enabled in kernel, it's not too bad just discarding the patch.
I can highlight the issue somewhere in this series.

>
> Then the two missing changes to expand support would be:
> 1. Fixing that ibt=off disables X86_FEATURE_IBT. The fix is to move to bool as
> peterz suggested.
> 2. Making XFEATURE_CET_USER also depend on X86_FEATURE_IBT (the patch in this
> series)
>
> We should do those, but in a later small series. Does it seem reasonable? Can we
> just do the simple obvious solution above for now?

It makes sense for me, but I want to hear x86 and KVM maintainers' voice for it.

Thanks!