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;
Date:   Fri, 4 Aug 2023 13:51:33 +0800
From:   Chao Gao <chao.gao@intel.com>
To:     "Yang, Weijiang" <weijiang.yang@intel.com>
CC:     <seanjc@google.com>, <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>
Subject: Re: [PATCH v5 08/19] KVM:x86: Report KVM supported CET MSRs as
 to-be-saved
Message-ID: <ZMyR5Ztfjd9EMgIR@chao-email>
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>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <83d767df-c9ef-1bee-40c0-2360598aafa8@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?PGhaNo8QZF2eqBOTyHqED+8kRXNfEDUoiKmKRl+7u8cGnFJB9JYW0gpWHZT3?=
 =?us-ascii?Q?G14k12twdKKdPQuIXq7Aew1BJSU7igcM35ZkJHXqm/tCEvCY4Zv92zpN7ziP?=
 =?us-ascii?Q?NrtgHm9/IHZF62ODqmjFywN4X0dcYs6iAgrF0GCiu4HVK73TqamcB+9uVQ2A?=
 =?us-ascii?Q?Tj7djj4GL/zFl3tPpXl/Tm8+pM3Wj0TLm/R1/yPRlvJLQK1wjwnioyzxGTYS?=
 =?us-ascii?Q?33rwN3zP5s3yaa26R8CLU5jSCGKI4yHqwCdbIDtpy5mKoWzgECI1UiHaiSZk?=
 =?us-ascii?Q?x7hK07PbDYErTNe1hKY+1zntXc7gckpDIQsYRt7Hj3h4K8ontYlQeJ1CLUBy?=
 =?us-ascii?Q?N0ahehalo5xtxULRTbC5s1PEOiI1bwSIGuUuu4ZxExTPKvtR/rCo2j0KHRYy?=
 =?us-ascii?Q?UnzbpaIKe/ufHOCfqaUml0RubKTh/B+axVxZ+TtOH8OrAUHClDaOwRmUQlcX?=
 =?us-ascii?Q?+3ZAu2kxNGjabFRRbc/9gWWBkf10i7RwdUhTMpDSv7gSz8jk8Fm5wS5ojgoI?=
 =?us-ascii?Q?SxUoTUicw1OPt2qlU6at0ijZoAaPYfGn9acLJwDIKzdyCSXub0dUXeOP9L9A?=
 =?us-ascii?Q?VlKvlwVJr+vGez+EU95XdyzMWH9JnDkeagawdNv9Te7dmPMUJ6VjAedsrjDU?=
 =?us-ascii?Q?xITA70W+KKNAaIjNsh/e2k5FLYqBDUcNW37ggQf0/9meI4NVElv12Up0Pz4c?=
 =?us-ascii?Q?JZ1t57Cn6aNddyuRMFfyGtewcf+Ai4Vw4No2yr5pBXxrkwRQzRgHS6A9sW2H?=
 =?us-ascii?Q?DaCJqWHt7NPbAfNIzsMNxOP4UoKO7WABObdFMc1rqPfePQB1QHocUU/WvjMN?=
 =?us-ascii?Q?kflpLghsF1Sw5YLjdNph6HX07UBQCwpfH33+8dsboaPd4pV4q4ykOQZPvDI0?=
 =?us-ascii?Q?SHCvYISqZewJf8ENUvws67qHecXoZYmPlJmQK37UXZ5fYOdOFCE52Kx7anDD?=
 =?us-ascii?Q?h8dUFRzeufPQiCEzJuPu9db1IcW1nBtA+AWhp3AtIRocNgKOKHedvzOgLb5g?=
 =?us-ascii?Q?iDHAut51P3kdK8QJ/tlyvlz7dNJzQkgg8tz8YRZCrMWN29vjzDsFYe9CQyjX?=
 =?us-ascii?Q?J7/p4qchpNm50h+2EcQT2D5s0PBmPwvHgyOMJYd0GZc90B7+Z5YgXhovPTCa?=
 =?us-ascii?Q?UWoh9GFC/L4Z1OQfiZ+dTUoDFwABBG+gnIeojIKNm3GnxSJV7PyB5fDVgWJ2?=
 =?us-ascii?Q?1XSCivBY/BcsDpqGlPxv3LialRZkZjTBDsRerQmorSafz6NkhrFW7NBQBJxw?=
 =?us-ascii?Q?/FhOgVhC1eRqqg9GFg5O6fsGyrdNo/WTqRX9yvbkeTwWGg9dkkzmqz6NYArq?=
 =?us-ascii?Q?cUawJ5wX6qFV/svB23rF7ldR19Di+zIeHiFwJ6OEiEwP22979TCaOKDiZNM4?=
 =?us-ascii?Q?aJgvigTv+g/nFjLKYr1F3LtV1ucogV5dD7r6OdPVEo9k8o9Q03NcCPIc5CAQ?=
 =?us-ascii?Q?XpHKwhzuWx3aGTGQHRZwetizUfK6BlCWWP/TW+OAFAAXlbTAhyWdYTNmyKbD?=
 =?us-ascii?Q?diW5tCKUCgc3e8AgY+DAhouvBfjuNUqlfduip3wnt9WUCLtMvaVywUYMZfjE?=
 =?us-ascii?Q?6VsKn2PUedUvAYx6qYYojIZtHT5/Pi78rL1HDTfv?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 41c3e66c-b0e8-4fb3-3391-08db94aee195
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB6780.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Aug 2023 05:51:43.3975
 (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: P8GqZiGiPCQdaKf5naY8zk8xFrAdIBloI1Ub7yvM1rbNtuEeu/iWJtyDWI0Sumgc6lxFzJ6rWZtwW+trdRa8/A==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DS0PR11MB8720
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,
        RCVD_IN_DNSWL_BLOCKED,SPF_HELO_NONE,SPF_NONE,T_SCC_BODY_TEXT_LINE,
        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 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?

>> > +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().

>
>kvm_cpu_cap_has(X86_FEATURE_SHSTK) || kvm_cpu_cap_has(X86_FEATURE_IBT)
>
>only tells at least one of the CET features is supported by KVM.
>
>> then patch 16 has no need to do
>> 
>> +	/*
>> +	 * If SHSTK and IBT are not available in KVM, clear CET user bit in
>> +	 * kvm_caps.supported_xss so that kvm_is_cet__supported() returns
>> +	 * false when called.
>> +	 */
>> +	if (!kvm_cpu_cap_has(X86_FEATURE_SHSTK) &&
>> +	    !kvm_cpu_cap_has(X86_FEATURE_IBT))
>> +		kvm_caps.supported_xss &= ~CET_XSTATE_MASK;
>