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:06:22 +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 13/20] KVM:VMX: Emulate read and write to CET MSRs
Message-ID: <ZMDT/r4sEfMj5Bmu@chao-email>
References: <20230721030352.72414-1-weijiang.yang@intel.com>
 <20230721030352.72414-14-weijiang.yang@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20230721030352.72414-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?WFa61eoMpZLkDobtfUG6inXPZxi4aET9em65yoF762y23T+sj+avg3gcKD1E?=
 =?us-ascii?Q?cebCMb6bd5Yh52WRNWNVWSZCDXQa+9o7nVGT+rWI0t1k9jptckTB5BRJ0LpT?=
 =?us-ascii?Q?H87377rVBkTTDqch+HcBavBOcoeQnxFWbTVnqg2bhwIbfMJs54CdWjp4klia?=
 =?us-ascii?Q?zGSDSMQVZSvMC5dlo4+GVumTn5kKPxtH9qLlZkvnpzn+Vfm3N37SxE3iqJEx?=
 =?us-ascii?Q?Jvda5yuPTrC+7oKXbpnLvIlnqHQe3tzBBryP9pG2uhZ70ngBGNmAOS06jqPo?=
 =?us-ascii?Q?fnnWpsmibQ2oANgS/p9kU3XulFBDBGSbAJZOH0RzfbK6uLONjb/ablheIt+6?=
 =?us-ascii?Q?ZJQDQqiVcYRgs4ZysmZP6tybGbEbFc1YbrlRA7BakLiujqcQJ5xppqP5eYNH?=
 =?us-ascii?Q?3kRm+zUdlqlPry4in6KSMJqMH12AjGX8dcKRgE0FDfaGOEV12bCFsblEUqdp?=
 =?us-ascii?Q?hAhzyhUMdIbvItHaDi9Gf+CMKhKRl3FShZfMeXKSzIcJYJWS4KCw/5xTz74f?=
 =?us-ascii?Q?vOMOF9SAB1kqQ1a6QyAU1PGkc/j00ljw8ijE545KRfh/1nq3Yn6vk30zfUKt?=
 =?us-ascii?Q?/hXFS9UVFvtOOfcjRhNqlKojC57tNSjXdRyjlsSI7GiGtT4Krc3n5FqQawVu?=
 =?us-ascii?Q?wNWFrZ+Vyo1ikzaVl06JZ4TpIaa8OpGVvKR3SSunLmCdBnZYHMSYIZa8Vxta?=
 =?us-ascii?Q?1CmaOK5Rt5sVXy1mbx4I2kUpZRzHCyDflJA0zf8A0yIN1auaIYTqUHuhm3Tz?=
 =?us-ascii?Q?VzYtRZSbL9VPCTasYWeVmfiQPOuM3ZoRqru4zJY256/DnbkHj7Hp5sF7C7Yo?=
 =?us-ascii?Q?lPe1V9dq1QaNXdF75SarhFXjkiFzKKofVtA+91iuq+9rTDjXjDytaXFgb36L?=
 =?us-ascii?Q?y7rx2Lc6oYi90i8ChwZoFxVwjbm8G1lAN8Fw4HHOhy1IlBkcTYjuvXu8HcwQ?=
 =?us-ascii?Q?42MpIaXDy8ZUWYF2KB5+JbRAELfGMktEisJOBopRyWbqaEK0OOfYBaNyjcBs?=
 =?us-ascii?Q?vpai1Gg9HJq1BZ4lIS2w61OS3tfWuSi/7Y8Ow3a0RiHHQoIQ50NJaYNRVBe5?=
 =?us-ascii?Q?OIwcge9YjLf9jBY27VBGKyonAn3MP0n6+kd3X62Duo2F7rOJdDx1T9wBO+rN?=
 =?us-ascii?Q?NJc9x5/Xw96fuyac2mcRFf7DjdHGNAEWnNkhM8wxg8pjZZqdC7YXNtrLbwov?=
 =?us-ascii?Q?+TXV2+C6RL5L6txZmNIFOtfKWGdpNT31pKaJE395iqCbqZahA2cvoCzJRJ68?=
 =?us-ascii?Q?kWmIrpMTGp/TWU9Dd0jdPz4y6kD9pnE8gPxXU2Tm80iQqRQdQnJVI2HPERxC?=
 =?us-ascii?Q?rf3n92NQsoCkpzB5xD+DxbczwfSNT9IWURWtCVXom+yTUo4kKQUx5YUNmfI8?=
 =?us-ascii?Q?bS2dAbSNwaJukJmj3syPX28CE4Mm7GnGjYhmfHQxwGUxc/gUkxLkBfZDKVzk?=
 =?us-ascii?Q?bvhieDlgkn34Xid1VjrxKBGydEUq4uTgNKy8MuG4PdUURzUfRP8GrNS4KUMC?=
 =?us-ascii?Q?UhDJM3w8dC1muQy4ysIPAUhVvyk7oGKrWW/0d+U4sNIqOFZpDJkekNAhUq+D?=
 =?us-ascii?Q?DahS5XgI3pi9SVwozxlS78Q4rzBwfLmw6tWd7lAx?=
X-MS-Exchange-CrossTenant-Network-Message-Id: d3e4f219-1011-484a-33db-08db8daf3900
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:06:32.1084
 (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: Mo9XVuwTrTfg5cRp5zNv5Mpd8fDdIk9XC8q7hPsKKMlJrwsiQEFIblU7TWMEPIBe1WaAl89fWWvZc32Il7y24w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB6514
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,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:45PM -0400, Yang Weijiang wrote:
>Add VMX specific emulation for CET MSR read and write.
>IBT feature is only available on Intel platforms now and the
>virtualization interface to the control fields is vensor
>specific, so split this part from the common code.
>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>---
> arch/x86/kvm/vmx/vmx.c | 40 ++++++++++++++++++++++++++++++++++++++++
> arch/x86/kvm/x86.c     |  7 -------
> 2 files changed, 40 insertions(+), 7 deletions(-)
>
>diff --git a/arch/x86/kvm/vmx/vmx.c b/arch/x86/kvm/vmx/vmx.c
>index c8d9870cfecb..b29817ec6f2e 100644
>--- a/arch/x86/kvm/vmx/vmx.c
>+++ b/arch/x86/kvm/vmx/vmx.c
>@@ -2093,6 +2093,21 @@ static int vmx_get_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> 		else
> 			msr_info->data = vmx->pt_desc.guest.addr_a[index / 2];
> 		break;
>+	case MSR_IA32_U_CET:
>+	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
>+		return kvm_get_msr_common(vcpu, msr_info);

kvm_get_msr_common() is called for the "default" case. so this can be dropped.

>+	case MSR_IA32_S_CET:
>+	case MSR_KVM_GUEST_SSP:
>+	case MSR_IA32_INT_SSP_TAB:
>+		if (kvm_get_msr_common(vcpu, msr_info))
>+			return 1;
>+		if (msr_info->index == MSR_KVM_GUEST_SSP)
>+			msr_info->data = vmcs_readl(GUEST_SSP);
>+		else if (msr_info->index == MSR_IA32_S_CET)
>+			msr_info->data = vmcs_readl(GUEST_S_CET);
>+		else if (msr_info->index == MSR_IA32_INT_SSP_TAB)
>+			msr_info->data = vmcs_readl(GUEST_INTR_SSP_TABLE);
>+		break;
> 	case MSR_IA32_DEBUGCTLMSR:
> 		msr_info->data = vmcs_read64(GUEST_IA32_DEBUGCTL);
> 		break;
>@@ -2402,6 +2417,31 @@ static int vmx_set_msr(struct kvm_vcpu *vcpu, struct msr_data *msr_info)
> 		else
> 			vmx->pt_desc.guest.addr_a[index / 2] = data;
> 		break;
>+#define VMX_CET_CONTROL_MASK		(~GENMASK_ULL(9, 6))

bits9-6 are reserved for both intel and amd. Shouldn't this check be
done in the common code?

>+#define CET_LEG_BITMAP_BASE(data)	((data) >> 12)
>+#define CET_EXCLUSIVE_BITS		(CET_SUPPRESS | CET_WAIT_ENDBR)

>+	case MSR_IA32_PL0_SSP ... MSR_IA32_PL3_SSP:
>+		return kvm_set_msr_common(vcpu, msr_info);

this hunk can be dropped as well.

>+		break;
>+	case MSR_IA32_U_CET:
>+	case MSR_IA32_S_CET:
>+	case MSR_KVM_GUEST_SSP:
>+	case MSR_IA32_INT_SSP_TAB:
>+		if ((msr_index == MSR_IA32_U_CET ||
>+		     msr_index == MSR_IA32_S_CET) &&
>+		    ((data & ~VMX_CET_CONTROL_MASK) ||
>+		     !IS_ALIGNED(CET_LEG_BITMAP_BASE(data), 4) ||
>+		     (data & CET_EXCLUSIVE_BITS) == CET_EXCLUSIVE_BITS))
>+			return 1;

how about

	case MSR_IA32_U_CET:
	case MSR_IA32_S_CET:
		if ((data & ~VMX_CET_CONTROL_MASK) || ...
			...

	case MSR_KVM_GUEST_SSP:
	case MSR_IA32_INT_SSP_TAB: