Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <3558aa15-b084-5641-4f78-00816523a8bc@intel.com>
Date:   Tue, 8 Aug 2023 23:16:36 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.14.0
Subject: Re: [PATCH v5 08/19] KVM:x86: Report KVM supported CET MSRs as
 to-be-saved
Content-Language: en-US
To:     Sean Christopherson <seanjc@google.com>,
        Chao Gao <chao.gao@intel.com>
CC:     <pbonzini@redhat.com>, <peterz@infradead.org>,
        <john.allen@amd.com>, <kvm@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <rick.p.edgecombe@intel.com>,
        <binbin.wu@linux.intel.com>
References: <20230803042732.88515-1-weijiang.yang@intel.com>
 <20230803042732.88515-9-weijiang.yang@intel.com>
 <ZMuDyzxqtIpeoy34@chao-email>
 <83d767df-c9ef-1bee-40c0-2360598aafa8@intel.com>
 <ZMyR5Ztfjd9EMgIR@chao-email> <ZM1IlPrWz/R6D0O5@google.com>
From:   "Yang, Weijiang" <weijiang.yang@intel.com>
In-Reply-To: <ZM1IlPrWz/R6D0O5@google.com>
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?VUEraTFKb2JVUzI1a2lyMnNEZVcxWGg5SVB1bWNRMWF2UzRmSTBHY0JjMFJC?=
 =?utf-8?B?WFR3RlBSckN1Q3psS2VteXVjNitnQ3N2dko3am1nN0VFSXhjV29seVR4a3lu?=
 =?utf-8?B?YXFwRkZubDJ3WlhscjIrRlg3K0dIOWxiUGcrenFYcVIzVUFLclg2UGR4SG5u?=
 =?utf-8?B?Zk82cUxjNys3RWNPOFlQaHpvQWo5ZzZTZjNlTGwvM21wSUtMazhoUDQ0NXlp?=
 =?utf-8?B?d001RmdNOEErZVdzV2xaWDRGZUF4R05jRVRUMTNHMmxFSllqNXc3Y2ZIVEV0?=
 =?utf-8?B?b3J6V2M2WFMxZVdDMVk5bmtOcDBxbTY0RWtuNE9aWnNjdnFqSnJhTThEdzhB?=
 =?utf-8?B?QU9aQXBEekx5amY5bTBVblQxTnAveGVtSXUvNzE2MWZPODdITUhyWElGa3RU?=
 =?utf-8?B?OWtaNjBnRzExU1JIMmZLVytETlpFSGtyejZmTDdsV2NGcE1wdWhFbVNYdVlh?=
 =?utf-8?B?WmRudElQTWdFbU9PZlpzSDdzYXI3dlN3aXBBT3EyL0tVYThIbWtscmRWTDh1?=
 =?utf-8?B?TDcwbWE1ZS8rUVNEQ3UrS1JCSVZPTzhKUXdDaFRYRWFZUFRjT284WG5UcDBk?=
 =?utf-8?B?Tk0vVTVpRTV2cWZzdCsxT2RiamdNWTRrQnpVVFo4ZXdwRjlKSWZIODlMSzJp?=
 =?utf-8?B?WDBsWUpwcDgya2hoTHAyNUtIK2dyVkRUeDZHWWNHUWtnL3FKOUszcGdPVEdJ?=
 =?utf-8?B?aCtWZzdPNDdMOFRhQzI1WUtkTy9SMGh3M3Jrak1YOFIxYUZHSmFiUlRCeGgw?=
 =?utf-8?B?TEVLdE1tQnNNd3dFMkc0MTJ5b0htNHRYeWJOTTNzZGpLcDN2SHlNZ3E4bzdY?=
 =?utf-8?B?RjdwaUVjMVp4TWViSmR1SjhGemt1aTdWbzR2R08yemV6d2pFQmdBdVNmOC9I?=
 =?utf-8?B?dlNweDJlZ25uRVhoTERESmtOc0dhWkMyUnlqblIxTVZncXRjT0dPcElleWlp?=
 =?utf-8?B?bkRud3k0Rk4rTEsvQjg2QktCN1E4TTRBUDZMTnF1ZmFyckpIOW5qNW94amJw?=
 =?utf-8?B?UGZveHRGdmVSRFIzU1l3SUxQRFJIZmNyVG5sQnFzeFVtRFRaU05iNlpDd09C?=
 =?utf-8?B?dFRmcmI3Y2YySlptNTVHQWpiNmNKRDBaOG0vVDVHRjliczE3VU0wVG5PUTBU?=
 =?utf-8?B?bHJ1SW5QY29pSHV3SU5VZFBuZlp3MFZCMzBjRk9VSmFYNnh3Y3NyVkxnK2d5?=
 =?utf-8?B?NVJaUDdCaWRnUGtLbCtQZk1OdG5UQ2JLbGJ1ekg3TnZtWERscmgyZDFmM0JD?=
 =?utf-8?B?ampGc2VjU0ZuZEw2b3JuQXgrV3RBcmlxN3VMUEpZZHo1RUFITERKZTJXYjh5?=
 =?utf-8?B?cVZ3QnRwNlhmS21qekp2VTI1c01wYm5STkZsakpGZHV3RVBDa0N6b3greWVa?=
 =?utf-8?B?VGtPVTljYzlOcXhHSm11SVVaVUF1b1ZWa2ZyU2ZPQXVyN0xrZWJNZmlBU3JQ?=
 =?utf-8?B?dE5Wdjc1MDA1cGdsV2FlT1pGOWFwU2d6Vi9uNUFXZlhxa3M0Z2Rra0dNOFhJ?=
 =?utf-8?B?QUtWdUZrbWFMYm1VR2U2eFNtaktaV1hRWG5WazIzYUpaVG9LNFljUUFDTFpN?=
 =?utf-8?B?b3FKR2duRWpPd1FmNTBxT0hJNTdlSzVHNWJ4RzBrcVkwSmNpQ0g0OSt1VFpX?=
 =?utf-8?B?cmV2bWVXUnUxdDNuTktmUjVvSjNoZTZiSk5RK0ZzVGhDbkNMbnA3ZzZhd1pa?=
 =?utf-8?B?Q0JmeHdROVVrZjdQRTdJUzkrTVE4bkE5NUQ3cE5OaUg2T2g1cGZqNDlhODRJ?=
 =?utf-8?B?dkNKOEVnMnlkYmxWbTBGTHpISFo1VFcrY2FzSlBhRlZsL3h3VTlLb3RxdlJQ?=
 =?utf-8?B?dnNQaFV5WjlMUE1hOWFaeFNCSGE5d2U4cmcrTy9aTWZRRFQ5anhJRFJ6OUZ4?=
 =?utf-8?B?czN3TUpoL3VnV0MzdEhxU0Z5dlhyY1l0QU5mdktCSFlueEo0UzAwVVZDTXl4?=
 =?utf-8?B?dldjN20rcjJOS29mSFZLY0xhek4rRlRpOUJ3dlNDd21qVHovTlJIK2pBQk1B?=
 =?utf-8?B?TTRTYzRXQUFBV2dSQUcxQkhLdTAyRDUvNm5nNVhzRlF3dEZkN0JOa3JTOVRw?=
 =?utf-8?B?bEptd1Q4ZzVYaHRYVmEyR2VKTEFpTW94Z2s5a2VXeU9MeEdRUjUrZjRaZUMw?=
 =?utf-8?B?M0VvRWxKSEdtbFBtemUrcGc3MlNHbko4L2FDbkRlVkdlbzlyaDlVS2RxWFhk?=
 =?utf-8?B?ZGc9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: ae377628-be6a-4dab-a2d6-08db98227ba1
X-MS-Exchange-CrossTenant-AuthSource: PH0PR11MB4965.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 08 Aug 2023 15:16:48.0485
 (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: ZiwifMUXCRxvD1omXLvh4i5t0EwL/mT0zjDIYd4V5wy2oUSc13USqwweklJVbp+dRbfSaQPPm6to7+MBUvzJyw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CY8PR11MB7340
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-3.9 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,URIBL_BLOCKED
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 8/5/2023 2:51 AM, Sean Christopherson wrote:
> On Fri, Aug 04, 2023, Chao Gao wrote:
>> On Fri, Aug 04, 2023 at 11:13:36AM +0800, Yang, Weijiang wrote:
>>>>> @@ -7214,6 +7217,13 @@ static void kvm_probe_msr_to_save(u32 msr_index)
>>>>> 		if (!kvm_caps.supported_xss)
>>>>> 			return;
>>>>> 		break;
>>>>> +	case MSR_IA32_U_CET:
>>>>> +	case MSR_IA32_S_CET:
>>>>> +	case MSR_KVM_GUEST_SSP:
>>>>> +	case MSR_IA32_PL0_SSP ... MSR_IA32_INT_SSP_TAB:
>>>>> +		if (!kvm_is_cet_supported())
>>>> shall we consider the case where IBT is supported while SS isn't
>>>> (e.g., in L1 guest)?
>>> Yes, but userspace should be able to access SHSTK MSRs even only IBT is exposed to guest so
>>> far as KVM can support SHSTK MSRs.
>> Why should userspace be allowed to access SHSTK MSRs in this case? L1 may not
>> even enumerate SHSTK (qemu removes -shstk explicitly but keeps IBT), how KVM in
>> L1 can allow its userspace to do that?
> +1.  And specifically, this isn't about SHSTK being exposed to the guest, it's about
> SHSTK being _supported by KVM_.  This is all about KVM telling userspace what MSRs
> are valid and/or need to be saved+restored.  If KVM doesn't support a feature,
> then the MSRs are invalid and there is no reason for userspace to save+restore
> the MSRs on live migration.
OK, will use kvm_cpu_cap_has() to check KVM support before add CET MSRs to the lists.
>>>>> +static inline bool kvm_is_cet_supported(void)
>>>>> +{
>>>>> +	return (kvm_caps.supported_xss & CET_XSTATE_MASK) == CET_XSTATE_MASK;
>>>> why not just check if SHSTK or IBT is supported explicitly, i.e.,
>>>>
>>>> 	return kvm_cpu_cap_has(X86_FEATURE_SHSTK) ||
>>>> 	       kvm_cpu_cap_has(X86_FEATURE_IBT);
>>>>
>>>> this is straightforward. And strictly speaking, the support of a feature and
>>>> the support of managing a feature's state via XSAVE(S) are two different things.x
>>> I think using exiting check implies two things:
>>> 1. Platform/KVM can support CET features.
>>> 2. CET user mode MSRs are backed by host thus are guaranteed to be valid.
>>> i.e., the purpose is to check guest CET dependencies instead of features' availability.
>> When KVM claims a feature is supported, it should ensure all its dependencies are
>> met. that's, KVM's support of a feature also imples all dependencies are met.
>> Function-wise, the two approaches have no difference. I just think checking
>> KVM's support of SHSTK/IBT is more clear because the function name is
>> kvm_is_cet_supported() rather than e.g., kvm_is_cet_state_managed_by_xsave().
> +1, one of the big reasons kvm_cpu_cap_has() came about was being KVM had a giant
> mess of one-off helpers.
I see, thanks!