Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
IronPort-SDR: QPL46HyHdMlyjw/4LIaV6zp4tb2aXf0arlSVC9pPETV9lHSsse1WV1O+ygCEaf/cpNteLQC+98
 eGrn0oihwCmg==
IronPort-SDR: 32uvF3M49APq7R4goH9f3+e1lzOgGcAcYk/ITMCi4a16MgcllgVzcJU1seXh1rLuwvfnX4uemN
 eFbhaKJy2yXA==
From:   "Liu, Jing2" <jing2.liu@intel.com>
To:     "Bae, Chang Seok" <chang.seok.bae@intel.com>,
        "bp@suse.de" <bp@suse.de>, "luto@kernel.org" <luto@kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@kernel.org" <mingo@kernel.org>,
        "x86@kernel.org" <x86@kernel.org>
CC:     "Brown, Len" <len.brown@intel.com>,
        "Hansen, Dave" <dave.hansen@intel.com>,
        "Shankar, Ravi V" <ravi.v.shankar@intel.com>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>
Subject: RE: [PATCH v3 10/21] x86/fpu/xstate: Update xstate save function to
 support dynamic xstate
Thread-Topic: [PATCH v3 10/21] x86/fpu/xstate: Update xstate save function to
 support dynamic xstate
Thread-Index: AQHW2UTrFqBCV+fQw0+L013CA01Sqaob5epQ
Date:   Thu, 7 Jan 2021 08:41:17 +0000
Message-ID: <BYAPR11MB3256BBBB24F9131D99CF7EF5A9AF0@BYAPR11MB3256.namprd11.prod.outlook.com>
References: <20201223155717.19556-1-chang.seok.bae@intel.com>
 <20201223155717.19556-11-chang.seok.bae@intel.com>
In-Reply-To: <20201223155717.19556-11-chang.seok.bae@intel.com>
Accept-Language: en-US
Content-Language: en-US
dlp-reaction: no-action
dlp-version: 11.5.1.3
dlp-product: dlpe-windows
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: e2cfc46e-d96f-420b-b18e-08d8b2e8002b
X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jan 2021 08:41:17.6696
 (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: 5xIKBMZrocuUATNayvZnPyRcaMdtwOU6MBF8l9MB8roEW/AImBNMhpqJrz3riw6Dx4fglsWX00PM8Wzp5+CrnA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB3815
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org


-----Original Message-----
From: Bae, Chang Seok <chang.seok.bae@intel.com>=20
Sent: Wednesday, December 23, 2020 11:57 PM
To: bp@suse.de; luto@kernel.org; tglx@linutronix.de; mingo@kernel.org; x86@=
kernel.org
Cc: Brown, Len <len.brown@intel.com>; Hansen, Dave <dave.hansen@intel.com>;=
 Liu, Jing2 <jing2.liu@intel.com>; Shankar, Ravi V <ravi.v.shankar@intel.co=
m>; linux-kernel@vger.kernel.org; Bae, Chang Seok <chang.seok.bae@intel.com=
>; kvm@vger.kernel.org
Subject: [PATCH v3 10/21] x86/fpu/xstate: Update xstate save function to su=
pport dynamic xstate

copy_xregs_to_kernel() used to save all user states in a kernel buffer.
When the dynamic user state is enabled, it becomes conditional which state =
to be saved.

fpu->state_mask can indicate which state components are reserved to be
saved in XSAVE buffer. Use it as XSAVE's instruction mask to select states.

KVM used to save all xstate via copy_xregs_to_kernel(). Update KVM to set a=
 valid fpu->state_mask, which will be necessary to correctly handle dynamic=
 state buffers.

See comments together below.

No functional change until the kernel supports dynamic user states.

Signed-off-by: Chang S. Bae <chang.seok.bae@intel.com>
Reviewed-by: Len Brown <len.brown@intel.com>
Cc: x86@kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: kvm@vger.kernel.org
[...]
 		/*
 		 * AVX512 state is tracked here because its use is diff --git a/arch/x86=
/kvm/x86.c b/arch/x86/kvm/x86.c index 4aecfba04bd3..93b5bacad67a 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -9214,15 +9214,20 @@ static int complete_emulated_mmio(struct kvm_vcpu *=
vcpu)
=20
 static void kvm_save_current_fpu(struct fpu *fpu)  {
+	struct fpu *src_fpu =3D &current->thread.fpu;
+
 	/*
 	 * If the target FPU state is not resident in the CPU registers, just
 	 * memcpy() from current, else save CPU state directly to the target.
 	 */
-	if (test_thread_flag(TIF_NEED_FPU_LOAD))
-		memcpy(&fpu->state, &current->thread.fpu.state,
+	if (test_thread_flag(TIF_NEED_FPU_LOAD)) {
+		memcpy(&fpu->state, &src_fpu->state,
 		       fpu_kernel_xstate_min_size);
For kvm, if we assume that it does not support dynamic features until this =
series,
memcpy for only fpu->state is correct.=20
I think this kind of assumption is reasonable and we only make original xst=
ate work.

-	else
+	} else {
+		if (fpu->state_mask !=3D src_fpu->state_mask)
+			fpu->state_mask =3D src_fpu->state_mask;

Though dynamic feature is not supported in kvm now, this function still nee=
d
consider more things for fpu->state_mask.
I suggest that we can set it before if...else (for both cases) and not chan=
ge other.=20

Thanks,
Jing

 		copy_fpregs_to_fpstate(fpu);
+	}

 }

=20
 /* Swap (qemu) user FPU context for the guest FPU context. */
--
2.17.1