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:   Wed, 26 Jul 2023 16:30:53 +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 v4 14/20] KVM:VMX: Set up interception for CET MSRs
Message-ID: <ZMDZvfJu1yhBigXz@chao-email>
References: <20230721030352.72414-1-weijiang.yang@intel.com>
 <20230721030352.72414-15-weijiang.yang@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230721030352.72414-15-weijiang.yang@intel.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?CBPz5KLoORCyXyIjekqHZg494sOy5I2yU3GgfPA0ktlIiBGNE99xtjyZvbuY?=
 =?us-ascii?Q?GeJXKWZWbGD2bYwKKiqQLqSNRaPmE94gdUrpCKSc40Hr4os94RQ1hYV8zJC3?=
 =?us-ascii?Q?AcofOQZPHQkvSUxUvBkN4L9LPyYHfF/yeoTMT2p7b2YeFHjYzeh8LsyfdLPw?=
 =?us-ascii?Q?E7roNI3uw1xdjkRKfPKa/NWjCLjLnzxDFX2jG32i4D+Zm/A2PD39WBdtMVz0?=
 =?us-ascii?Q?Fw7a5bwyOHGp83/gW8rS11sf2l9bOnVAmSvMeth720weSdqTDe/5nrsCzd6D?=
 =?us-ascii?Q?n94TAgBqT4Da80Q25mtqfS4I45rXtiBQPRsPV2xFaT4vPHNRBmQhPdKWRCV5?=
 =?us-ascii?Q?8kw0PH0htAfX4T7ZWl1Q5mGbmQAZfDDnItCLOMEMq7thFRpwiowQJW45pj2P?=
 =?us-ascii?Q?22EjBAlyXQcOcUh8fnTU3qF1aOfImep2t2SdanONwZtqDXodvJ13/IOO9IvU?=
 =?us-ascii?Q?wbgQ1O4tR7pGN54DMQTPxiL+ceg0fLxWNQNDdZlqVkVly1iCf5BIVuOwmy4a?=
 =?us-ascii?Q?AfskdjC6o+8/zThYHnKgbkwZ995XaLs/FAO0/5M+9mjaozttsxUXkiQOWQL3?=
 =?us-ascii?Q?QfTE8Tct0O74jsspHrGFzZ+x0HTTpucm9MzO2wQQiuXM6t6Vc/L16hXco13Y?=
 =?us-ascii?Q?aNc1f6QPAAaTOtBoe07zkdAZEOR/aXUvR7+Z0Z4YeZFaZb0o6XxvytlYjNIq?=
 =?us-ascii?Q?2VezKUA1WS9cS7HIQrynewJNxSpvGSNtS2xurEHMDsZEbXOd27UX5PsCIBiR?=
 =?us-ascii?Q?A2j0FggP2pfOQCMH7QzWgkXwbT3g25IQaTDu9Phs3IBaiteP3Tl181vZMX+4?=
 =?us-ascii?Q?4ryDcul6DiWmkQgGpfJWsAedjcP9IBqApOQsKAOuH1zVRgQvx2HdEdifCmQX?=
 =?us-ascii?Q?+l+R7HzsnSSMUja7V7Jx+RLSvJ3IRXzray2wxQwy7VpbSCtSB4ouo7BJT/yU?=
 =?us-ascii?Q?w+n2CyPpFRaDq4LZJ5+WP36mOy7/BmKHl64n+W66xALHnZGUE6clVUSxhPVD?=
 =?us-ascii?Q?KGJz9dsmPmmBKowJNt8UzX7YSWydUA1+VhvwxY4z1Q1JqMHSXS0dfkqZrJI2?=
 =?us-ascii?Q?y1zk0qGFlXW/lwY2AYUd4N12waFVyEzFshtCZqQakKowpDGjT/Hnugon5Hgo?=
 =?us-ascii?Q?DQwXHQRQmhEubb0XDipDOKgOCHy+SDC8ASaQRTGhlj/CqTQ7ETA3i1yS3sXM?=
 =?us-ascii?Q?8D97/tVC2sVMhymZfKZqCmdKFXeCR9QblXS6Rpk7NzKocHZgLVzMX0P9r+TZ?=
 =?us-ascii?Q?mGiLb0wx5ikuzPRFsW3Hag4jnoVnZbYXR4JcvdVK5yWZLjIEqf/9NDH2biBC?=
 =?us-ascii?Q?2kfz8afo+lB4mr5XXZqJWjAi6ZGRuS8SX2f+lcXhu7bjBU2eXal2gB+8TfqK?=
 =?us-ascii?Q?WU7TNtJaL8V77ihZBNq1jbbAAMLW4X7C2VLW1r6DfTfMQZcILjim58Nm4tJz?=
 =?us-ascii?Q?MA+NMH4Bwj8OW4tKbrNRHCD9Qe3e1n6msulNZ+ihRirw0Y73SUuZ6iUcYgft?=
 =?us-ascii?Q?lYaG3qLVDEVMoGftPPxef5xlp/HhBGcfM/ePi4mKiQDO0o7eq80ogO5TN1Xd?=
 =?us-ascii?Q?sKzK577SoynkWAjNm+J3QTn+uxdNjZ0UJbDfBmX+?=
X-MS-Exchange-CrossTenant-Network-Message-Id: eff9207d-682c-4ca7-9196-08db8db2a70f
X-MS-Exchange-CrossTenant-AuthSource: PH8PR11MB6780.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 26 Jul 2023 08:31:05.2595
 (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: P2lvqMIKEyY894GtzMNzJLPzGar9v6mGzJbmdZhj3/UPxbCk10KF2usX2HroZ6STffcXPksaYuKZK2MSejKxTA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MW5PR11MB5908
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,
        RCVD_IN_MSPIKE_H2,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 Thu, Jul 20, 2023 at 11:03:46PM -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.

If a guest supports SHSTK only, KVM has no way to prevent guest from
enabling IBT because the U/S_CET are pass-thru'd. it is a problem.

I am wondering if it is necessary to pass-thru U/S_CET MSRs. Probably
the answer is yes at least for U_CET MSR because the MSR is per-task.

>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>---
> arch/x86/kvm/vmx/vmx.c | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index b29817ec6f2e..85cb7e748a89 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;
>@@ -7758,6 +7762,34 @@ 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)
>+{
>+	if (guest_can_use(vcpu, X86_FEATURE_SHSTK)) {
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_U_CET,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_S_CET,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL0_SSP,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL1_SSP,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL2_SSP,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_PL3_SSP,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_INT_SSP_TAB,
>+					  MSR_TYPE_RW, false);
>+		return;
>+	}
>+
>+	if (guest_can_use(vcpu, X86_FEATURE_IBT)) {
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_U_CET,
>+					  MSR_TYPE_RW, false);
>+		vmx_set_intercept_for_msr(vcpu, MSR_IA32_S_CET,
>+					  MSR_TYPE_RW, false);
>+	}

This is incorrect. see

https://lore.kernel.org/all/ZJYzPn7ipYfO0fLZ@google.com/

>+}
>+
> static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> {
> 	struct vcpu_vmx *vmx = to_vmx(vcpu);
>@@ -7825,6 +7857,9 @@ static void vmx_vcpu_after_set_cpuid(struct kvm_vcpu *vcpu)
> 
> 	/* Refresh #PF interception to account for MAXPHYADDR changes. */
> 	vmx_update_exception_bitmap(vcpu);
>+
>+	if (kvm_is_cet_supported())

Nit: this check is not necessary. here isn't a hot path. and if
kvm_is_cet_supported() is false, guest_can_use(., X86_FEATURE_SHSTK/IBT)
should be false.

>+		vmx_update_intercept_for_cet_msr(vcpu);
> }
> 
> static u64 vmx_get_perf_capabilities(void)
>-- 
>2.27.0
>