Received-SPF: pass (google.com: domain of linux-kernel+bounces-38374-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 25 Jan 2024 18:17:26 +0800
From: Chao Gao <chao.gao@intel.com>
To: Yang Weijiang <weijiang.yang@intel.com>
CC: <seanjc@google.com>, <pbonzini@redhat.com>, <dave.hansen@intel.com>,
	<kvm@vger.kernel.org>, <linux-kernel@vger.kernel.org>, <x86@kernel.org>,
	<yuan.yao@linux.intel.com>, <peterz@infradead.org>,
	<rick.p.edgecombe@intel.com>, <mlevitsk@redhat.com>, <john.allen@amd.com>
Subject: Re: [PATCH v9 10/27] KVM: x86: Refine xsave-managed guest
 register/MSR reset handling
Message-ID: <ZbI1NqPLpnSZc6g9@chao-email>
References: <20240124024200.102792-1-weijiang.yang@intel.com>
 <20240124024200.102792-11-weijiang.yang@intel.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20240124024200.102792-11-weijiang.yang@intel.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?fGiVdarBTanZGBqghcUtt+z/17j4C3HfJ4d8dZJ83ehMA97Uv2Pr+1Iel+U1?=
 =?us-ascii?Q?wSVeiCT2ctNYVhi9O4uY4FKjmJsshYA2I6F5JgpV71mSWrIavDRCOoqHWhGn?=
 =?us-ascii?Q?dhkToSham2eF2LWfG5hgrgmfNUR4LOij8c+6qwUdHingO8BHlPspYT4R9tbH?=
 =?us-ascii?Q?UabHBaOHo5Ym90vymJqsnk+yAIbPiRGYAENAQAiM0exYKOtx+5PsjRPPah/Y?=
 =?us-ascii?Q?37r4RQVPPYFuW6QALzNsjXj+NeMlorcapXN7J0sVfwLOzHkFgEqOkQHeyAxv?=
 =?us-ascii?Q?wtPawtxwaLp1CHLiCWzHKvem0B6nxipxmNxiD+PgNgLHYX7s4GvkxBYfu7dF?=
 =?us-ascii?Q?7m7/ni1RaG14nWXxh3i8XbPgSWzilOArWDjS1j9gs1DuA5IIqHOZv3a6ptH1?=
 =?us-ascii?Q?YnkYuU4dAVSz9aN7zejV72EWDIzrc3bojoQ7/fp/WrhH3grWeG4uw5sGn6ol?=
 =?us-ascii?Q?KKLgh34yEYzQ2j3TEd+1Q/jMSk4fCyvESLtsynWaFMtQ0zAaWLEA2A1dgS0e?=
 =?us-ascii?Q?DMAWnz0IqgqIO4l0+4oRrB6FXgTbkKGxUgrOxMUVs8NavchtnIBLHZuuUE3H?=
 =?us-ascii?Q?wnO0NCG45FvvwN1ySXVkwZ7brWVzR+F4VxyW7onnm2VVYHRMKaraLfHGFrw/?=
 =?us-ascii?Q?BhzRdZP4/ZVT0rVcW6aJi6puh7WIa3b/yOKuKHrz0BlGfrrcFS5eIsss9EW1?=
 =?us-ascii?Q?wDPva5C0x0z5hQlaxIurVpRrRjmWWO7xBBfm9J+5qqsH35eN7UJOeU+NhC43?=
 =?us-ascii?Q?vc6o4IUbGyoIpE6dcs4OCRDAZAHn0swQfvn5hjYr05nSoRwxcagBUYbZB/sp?=
 =?us-ascii?Q?OjA5BNR4Ywu2RuRc1vP9OSeRb5wioNlc1PueRyGRR46P9JlWnbMREBLFwdbI?=
 =?us-ascii?Q?Wh/MC+Ns0OOGX0Q4v+QTA9dTFr4UXIKYmnPy5WQ71KVj/6H15zVkERpnNRUL?=
 =?us-ascii?Q?dmUtTzhiBpMwdtGUvWc+6mRxJACc/gyzO7spE5YbsR6ZBvExGOL31bXV2cXd?=
 =?us-ascii?Q?brSNk5yO1kTgQ8si2Zi72xyfIrkoUzRMXF5DFbzX7+ayRljeLKh1REGT3/DW?=
 =?us-ascii?Q?po/rZeS5k5lYhKqPTqASLeoqphULZhqowcFbdW6QtshvrBRr8SsgOsnZ4rMB?=
 =?us-ascii?Q?FzrK1N1sGtkS9/xIGwYky4rnQ1HG6uOqFhwcteR62YaOcpJt9d8F2ePsCsGi?=
 =?us-ascii?Q?R/z3XKFXWhSoI2wxDpRWfeS+UnhDeYwC3D+cThcW9fWmZWoQJXHyaFfk7Uw5?=
 =?us-ascii?Q?W4ab+BnVWEZmQb0pcXy6cvdDuSIuu8lrHTXRXS2PjMLyExugFJ4lsPl4MrBa?=
 =?us-ascii?Q?X/y8IqiWNKJp/dsz/ufFJS13tGqEdraZpV7PyFUFA/T1bV2lI7lN+4y/j1bS?=
 =?us-ascii?Q?k5bBHupF5eQivU688BPWMaa4zoWBaGcuEA3h6qMsSWnqMFO5g52LJ+0qXRUA?=
 =?us-ascii?Q?C7JgoHg8vjG9sJkQCLYpFd+66nnX6o1HvfOdaLoX6VqUCbbmqsYoc4V+53Ha?=
 =?us-ascii?Q?7s/DJSzvGmSHAyB8FZRuEyB7/PAS26KCwvytSgWfkTpj2ADybNNSo6LzPC0V?=
 =?us-ascii?Q?MDizXRcMoX59QK/8k3QTO6UStgzXMfKuKz5l4YKN?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 19c8df37-de2c-40e4-4090-08dc1d8eda79
X-MS-Exchange-CrossTenant-AuthSource: CH3PR11MB8660.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 25 Jan 2024 10:17:36.9557
 (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: p8GsQgCSPJIyTWH6TFe22exOuU9cKiqUryM5GGHbbW9k7ZcwKTnCyiKwzZ3gKSXeSRXjuIrdwlsll+KIHRVMAg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5268
X-OriginatorOrg: intel.com

On Tue, Jan 23, 2024 at 06:41:43PM -0800, Yang Weijiang wrote:
>Tweak the code a bit to facilitate resetting more xstate components in
>the future, e.g., adding CET's xstate-managed MSRs.
>
>No functional change intended.
>
>Signed-off-by: Yang Weijiang <weijiang.yang@intel.com>
>Reviewed-by: Maxim Levitsky <mlevitsk@redhat.com>
>---
> arch/x86/kvm/x86.c | 15 ++++++++++++---
> 1 file changed, 12 insertions(+), 3 deletions(-)
>
>diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
>index 0e7dc3398293..3671f4868d1b 100644
>--- a/arch/x86/kvm/x86.c
>+++ b/arch/x86/kvm/x86.c
>@@ -12205,6 +12205,11 @@ void kvm_arch_vcpu_destroy(struct kvm_vcpu *vcpu)
> 		static_branch_dec(&kvm_has_noapic_vcpu);
> }
> 
>+static inline bool is_xstate_reset_needed(void)
>+{
>+	return kvm_cpu_cap_has(X86_FEATURE_MPX);
>+}
>+
> void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> {
> 	struct kvm_cpuid_entry2 *cpuid_0x1;
>@@ -12262,7 +12267,7 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> 	kvm_async_pf_hash_reset(vcpu);
> 	vcpu->arch.apf.halted = false;
> 
>-	if (vcpu->arch.guest_fpu.fpstate && kvm_mpx_supported()) {
>+	if (vcpu->arch.guest_fpu.fpstate && is_xstate_reset_needed()) {
> 		struct fpstate *fpstate = vcpu->arch.guest_fpu.fpstate;
> 
> 		/*
>@@ -12272,8 +12277,12 @@ void kvm_vcpu_reset(struct kvm_vcpu *vcpu, bool init_event)
> 		if (init_event)
> 			kvm_put_guest_fpu(vcpu);
> 
>-		fpstate_clear_xstate_component(fpstate, XFEATURE_BNDREGS);
>-		fpstate_clear_xstate_component(fpstate, XFEATURE_BNDCSR);
>+		if (kvm_cpu_cap_has(X86_FEATURE_MPX)) {
>+			fpstate_clear_xstate_component(fpstate,
>+						       XFEATURE_BNDREGS);
>+			fpstate_clear_xstate_component(fpstate,
>+						       XFEATURE_BNDCSR);
>+		}

Checking whether KVM supports MPX is indirect and adds complexity.

how about something like:

#define XSTATE_NEED_RESET_MASK		(XFEATURE_MASK_BNDREGS | \
					 XFEATURE_MASK_BNDCSR)

	u64 reset_mask;
	...

	reset_mask = (kvm_caps.supported_xcr0 | kvm_caps.supported_xss) &
			XSTATE_NEED_RESET_MASK;
	if (vcpu->arch.guest_fpu.fpstate && reset_mask) {
	...
		for_each_set_bit(i, &reset_mask, XFEATURE_MAX)
			fpstate_clear_xstate_component(fpstate, i);
	...
	}

then in patch 24, you can simply add CET_U/S into XSTATE_NEED_RESET_MASK.