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 16:16:25 +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 13/19] KVM:VMX: Set up interception for CET MSRs
Message-ID: <ZMyz2S8A4HqhPIfy@chao-email>
References: <20230803042732.88515-1-weijiang.yang@intel.com>
 <20230803042732.88515-14-weijiang.yang@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230803042732.88515-14-weijiang.yang@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?ApFPTehu1J5WYnpbQUWpZTlBKfCuWx7QDR1U+6Ez9fGD7oPYegxV3mkH+2SB?=
 =?us-ascii?Q?txXfLDekeMrC06dsU/ZaHL6yjcrAl/dy3zVrxHQGZ+iTqXhgbW0gTdKkRso1?=
 =?us-ascii?Q?dRsznKfCdNcDjSJKn7Tdl+88K9qzKfDEXUu5Bl2rb5KabSSHV8mSnMhPn7OJ?=
 =?us-ascii?Q?S3AZBGAc5J3DKoRg4akujdTxubIjgwgqrtuKVyVJYZ8bZCmJzAQdtotCGL1x?=
 =?us-ascii?Q?M5KTTAVRNDujG/bVFR4MiDeC4h8P+RyOmcP+W6+X8pL4+FW+WEP+q4en5u+5?=
 =?us-ascii?Q?p0ddGQ9aFbaGokgpct7oBmr08KBFnUl1qRXmtNwjrLaWarDAHYlZx3JN3l2e?=
 =?us-ascii?Q?d6WvTyAeljL+v/X1EofCsLTrGP03IOA2YNZacy97fBeXJYJQbNtdhatA7RC8?=
 =?us-ascii?Q?Iz7CAc6GnMF9fP+GQ+eag+JDRWm0mqabk5JfVrfrTwNuVEt82p/1++I0amzM?=
 =?us-ascii?Q?D4IC4rMsYZS55CEW7+4sptPeXwHyCnJ4f5TscKpzgVXaQHzbb2rx6nZrNnrp?=
 =?us-ascii?Q?ZVuq4/hiN5ibd60VA5wGwr3IcnrRBt/q5Qw/h+4N5JBGaGbR7niQUCvg6qH9?=
 =?us-ascii?Q?WIXVwoD3uVMUleCHpLaW3muaMCqhUBOU1uvk/EcgnMtycbvtoMydGh6v+klU?=
 =?us-ascii?Q?zWnNKXVI8lwEkG0cM7pUG5S1oEeHnb/6p21GU02h28qx9uOhBKgS0nq1LZ8j?=
 =?us-ascii?Q?kBxOuF5tgo4HhftCtdzUnWRyuqxEqGVDlUJ0jpoh1iliF3sO7mC4cMe7/T2O?=
 =?us-ascii?Q?K7G93WgPcLhuAWsU6WLCPAgBKWeudXvvbWcu6fkB5ZU6z3N2ce7ORujIbnPm?=
 =?us-ascii?Q?+uCHOll6CGinMMpJTHwA9bVvIA7iPqduQoLMhKPol+Q4if9zqJbKSWOcTBqV?=
 =?us-ascii?Q?BmUOuK8sjsG1CABwxTrjV4fNoJ7X7l9v+rp+w+Fu9nUyadEfJ8lLcZtiDA+C?=
 =?us-ascii?Q?Rrdqdrj2hejKzrqMjhAK6+EPX4zXKFfNOZM51NuVhrjaS7IIurDOtXCS9Y0J?=
 =?us-ascii?Q?HSyGEGfsI7Wkyg/lqaI9OlgdL7AuCRXR7ZylUgep/7yM5VWM6PkfXT061JtV?=
 =?us-ascii?Q?a1I6NL2r8wpz+xqnJRf+lZo4N4iyS97vc+16HN9yI+iqOkltQxZkenoE9EsV?=
 =?us-ascii?Q?SZocHR5+OaFEXkJRf8FxbpCkwbl4DqsnhMT7M9nbpmbCJ/Er7UZCdgK4vID7?=
 =?us-ascii?Q?oLI2mjdizKMPetLtEXsbF3IiUJAc2Lcv379vildwJ6dcL5qiN2VpGuwr/w9z?=
 =?us-ascii?Q?xCMh1xBbBQnNZGQF1f5+UDaL9vJP117wcGQzgN/dBvSuSbXw0HMxiDx801D4?=
 =?us-ascii?Q?Azq0AYgB6/Bj0T/SeHa/UAo0Aq42Xy5YgJGegrUz6dOiuKk9IcaTZcQz4qMd?=
 =?us-ascii?Q?M8DHhzL0oW9rSj8ZiMow7T6LJSWetPJNuPLdG1EDMc4lZ9Hq7C927toOfich?=
 =?us-ascii?Q?7NhobzmP5sBtyr6N+x7pHtLJ9VFkl0hFDDpwdDIOedg3XEnkC7FiUK7pe9vz?=
 =?us-ascii?Q?l+uunYc9t8xgzUmrpgcuDYUG/q6xzR2Oy7x6QNJ8MXBxH7mQfmbgvek+hFZo?=
 =?us-ascii?Q?ItJ4uqRDuyuR1gR7Gy+rgvkQC3nIZyNjgBdvJjvr?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 0e57ac5f-d17d-4709-6e2e-08db94c31e63
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB6780.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 04 Aug 2023 08:16:35.3458
 (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: LUyaYcNYYO5JrinIQ+GxjSv1xLftutB3iJjjgGDf3j6XBJkDInFGKheDeitZ4XBhdz27uhUk4xo7UO/jpmMuCA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7174
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        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 Thu, Aug 03, 2023 at 12:27:26AM -0400, Yang Weijiang wrote:
>Pass through CET MSRs when the associated feature is enabled.
>Shadow Stack feature requires all the CET MSRs to make it
>architectural support in guest. IBT feature only depends on
>MSR_IA32_U_CET and MSR_IA32_S_CET to enable both user and
>supervisor IBT. Note, This MSR design introduced an architectual
>limitation of SHSTK and IBT control for guest, i.e., when SHSTK
>is exposed, IBT is also available to guest from architectual level
>since IBT relies on subset of SHSTK relevant MSRs.
>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>

Reviewed-by: Chao Gao <chao.gao@intel.com>

one nit below

>---
> arch/x86/kvm/vmx/vmx.c | 41 +++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 41 insertions(+)
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index ccf750e79608..6779b8a63789 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -709,6 +709,10 @@ static bool is_valid_passthrough_msr(u32 msr)
> 	case MSR_LBR_CORE_TO ... MSR_LBR_CORE_TO + 8:
> 		/* LBR MSRs. These are handled in vmx_update_intercept_for_lbr_msrs() */
> 		return true;
>+	case MSR_IA32_U_CET:
>+	case MSR_IA32_S_CET:
>+	case MSR_IA32_PL0_SSP ... MSR_IA32_INT_SSP_TAB:
>+		return true;
> 	}
> 
> 	r = possible_passthrough_msr_slot(msr) != -ENOENT;
>@@ -7747,6 +7751,41 @@ static void update_intel_pt_cfg(struct kvm_vcpu *vcpu)
> 		vmx->pt_desc.ctl_bitmask &= ~(0xfULL << (32 + i * 4));
> }
> 
>+static void vmx_update_intercept_for_cet_msr(struct kvm_vcpu *vcpu)
>+{
>+	bool incpt;
>+
>+	if (kvm_cpu_cap_has(X86_FEATURE_SHSTK)) {
>+		incpt = !guest_cpuid_has(vcpu, X86_FEATURE_SHSTK);

...

>+
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_U_CET,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_S_CET,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL0_SSP,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL1_SSP,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL2_SSP,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL3_SSP,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_INT_SSP_TAB,
>+					  MSR_TYPE_RW, incpt);
>+		if (!incpt)
>+			return;
>+	}
>+
>+	if (kvm_cpu_cap_has(X86_FEATURE_IBT)) {
>+		incpt = !guest_can_use(vcpu, X86_FEATURE_IBT);

can you use guest_can_use() or guest_cpuid_has() consistently?

>+
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_U_CET,
>+					  MSR_TYPE_RW, incpt);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_S_CET,
>+					  MSR_TYPE_RW, incpt);
>+	}
>+}
>+
> static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> {
> 	struct vcpu_vmx *vmx = to_vmx(vcpu);
>@@ -7814,6 +7853,8 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> 
> 	/* Refresh #PF interception to account for MAXPHYADDR changes. */
> 	vmx_update_exception_bitmap(vcpu);
>+
>+	vmx_update_intercept_for_cet_msr(vcpu);
> }
> 
> static u64 vmx_get_perf_capabilities(void)
>-- 
>2.27.0
>