From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     "Wang, Wei W" <wei.w.wang@intel.com>,
        "Christopherson,, Sean" <seanjc@google.com>,
        "Liu, Jing2" <jing2.liu@intel.com>
CC:     "x86@kernel.org" <x86@kernel.org>,
        "kvm@vger.kernel.org" <kvm@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-doc@vger.kernel.org" <linux-doc@vger.kernel.org>,
        "linux-kselftest@vger.kernel.org" <linux-kselftest@vger.kernel.org>,
        "tglx@linutronix.de" <tglx@linutronix.de>,
        "mingo@redhat.com" <mingo@redhat.com>,
        "bp@alien8.de" <bp@alien8.de>,
        "dave.hansen@linux.intel.com" <dave.hansen@linux.intel.com>,
        "pbonzini@redhat.com" <pbonzini@redhat.com>,
        "corbet@lwn.net" <corbet@lwn.net>,
        "shuah@kernel.org" <shuah@kernel.org>,
        "Nakajima, Jun" <jun.nakajima@intel.com>,
        "jing2.liu@linux.intel.com" <jing2.liu@linux.intel.com>,
        "Zeng, Guang" <guang.zeng@intel.com>,
        "Zhong, Yang" <yang.zhong@intel.com>
Subject: RE: [PATCH v3 19/22] kvm: x86: Get/set expanded xstate buffer
Thread-Topic: [PATCH v3 19/22] kvm: x86: Get/set expanded xstate buffer
Thread-Index: AQHX9zE4nQc2VVNaCkmSJ6dfEGDKnaxIqm0AgAAm9QCAADtnYA==
Date:   Wed, 29 Dec 2021 06:36:43 +0000
Message-ID: <BN9PR11MB5276411D19957E7EA51C53A98C449@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20211222124052.644626-1-jing2.liu@intel.com>
 <20211222124052.644626-20-jing2.liu@intel.com> <YcuuCMCQryzUFoAZ@google.com>
 <96d1266eeeb8434f9e23f9a051e252e6@intel.com>
In-Reply-To: <96d1266eeeb8434f9e23f9a051e252e6@intel.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?/nFYEiyYCdQwgz5weEP8dZZkdr5sC8NErwrzawobc9SpcQ+kBRgC2ctPZO/q?=
 =?us-ascii?Q?grJopTo7gjEosLkfjcLdUsV2R8QqhX63d70j7I3SHpSFPxn6lhZJZ/21tAPT?=
 =?us-ascii?Q?dGRqS+F94B3JVFSNuB9H8pbKFqjbvFqk4tyx4eD8mD1zCBeqrLeGXFpKY5fo?=
 =?us-ascii?Q?JHD12e0V8XX8OuM/YgZaLTlugjMOeMXm1OJEg2HTozK0uCA3FG15e6NNFuo2?=
 =?us-ascii?Q?KlvFfuF6azCBIr4jOyBoibh4pyGUWLJJH/GasK1PJygt3vrIKCb4se8Weaug?=
 =?us-ascii?Q?x1fTRitByOBvIV0qZ+0/X9CxujB+LfVrcMr+YEGknQiobMizw5gDAZKgn1iS?=
 =?us-ascii?Q?eL5Khk/pJVYkx8HVZ50L++wnKO1ev3B1DB1esinZ9LNcCMCtGJR+gNdVSNnl?=
 =?us-ascii?Q?EYKHURJUFY9BXqiT1iKVPo2i64M/swFmRSEUOOBhVfd0O1C+iWQmB0J5OGjM?=
 =?us-ascii?Q?m9PlYA2H0uFYlJHwoTO7w6ugnk85lZuFjNjBRMDT4zej0jSBDwkMaN/ejQlv?=
 =?us-ascii?Q?8edonM4z5dYIs6Wo88KPbdoo+0p638gqU4W2ngq0Zkdx8V0ibc/PDPL1qXZZ?=
 =?us-ascii?Q?GLmS91Uz+G+2x9T4HQvGpAfUptK0nb1iXCsL99Pf3XRNRwQ0yTjLv4iV8em+?=
 =?us-ascii?Q?81PGMZFkNvmlE1SSK15iJBppqhz6PhFYRut9wMobWOTIHlKFWbkkPxfb78sp?=
 =?us-ascii?Q?S8NiMI/SwI8zlqGvi3RCaAp296edp1iqLXzVD9Vx6RmWJfpBjYXhGGZzbjCD?=
 =?us-ascii?Q?aj/2eM5MNdQ9D8xJ95oo7iWpKwb/MgmsCzjz6QJoWa42NFFH65jRFFBZ6qLG?=
 =?us-ascii?Q?WVf11aYn7w8y5dpe2lCnvU/HlNOPwZQphY+7d+6c8DHbaVgMwX7qgmKfqEY8?=
 =?us-ascii?Q?dKie+LmsUkgqWi9Lylgpa4xuldZYyo9Nw+RUS3Hs4YQGKYr6htE8iTyLl0V9?=
 =?us-ascii?Q?Vk6Vc/gwjKAIlsvrz98xrN6eBXOWqyciOvE9iJNEW83Ao/wBxNn0n9mAUP5a?=
 =?us-ascii?Q?yuVdcbuAwx8YRjNlrVuJvFfRd6JsmJpXTBhxk6fpH4CSIiv36hcswC1uqHPk?=
 =?us-ascii?Q?N1QBrNU6EuM2N0C+FyVPdbodijW9wKnZqGyEA4zCKE7+XRf9HiY3zDw8ipP6?=
 =?us-ascii?Q?ZcWvZ2sngOzTtqXgVbHOgOo++wtFxCLF+51E8dnLCAwBkNNqfFfJeyh0PxtW?=
 =?us-ascii?Q?YqL63E2xR/n58IFHdN5AKQ0jcFUVj83pZQKJWq0wtiCS2TyHdvWw1M6EON82?=
 =?us-ascii?Q?gHD24o89+JthpU6plHeyjE0aRvZA8Tgt+j7RFXT0T6Aw6CJMQ14Rd5NnuNbl?=
 =?us-ascii?Q?HFrWL35HG/tUWfDk5Sww6BxcmsiroB/fKGX4K9eun+S1FyOFzM9L+7HCCxno?=
 =?us-ascii?Q?xsDs5whkzpjVwtPYdavurjjFCnrP3SKQf4sOrgWh4iahen18e9ZpYgw7lzNB?=
 =?us-ascii?Q?Vg2+p5ECl9WaWjpZjv2eKPB8Yg+jWBO68CUl9r5UlkwJohB27VIZp02HiPtM?=
 =?us-ascii?Q?TksWJA8HQU6FBiU0LN9P5SfFsYxwOSpEaGklJOe0LPnb+heU1/SH4MqA8292?=
 =?us-ascii?Q?1XGkqkXZfTriauiaUJ87Jk0dOLjgECAE8he14RAyfAdTRWBcTsmfbWf4YtKB?=
 =?us-ascii?Q?7cx167gtDn18SPf42LGUjt8=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: BN9PR11MB5276.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a8b2234f-48ca-4c91-c74a-08d9ca9594a3
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Dec 2021 06:36:44.0075
 (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: OpAZlBdWyLdwhp/xMBBxymYRXpXdzjXzVbLG6KqOen0/lHwdAaOhWO65fF7n1f1K6STBxRx+NicOptO/ZIbGLw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN6PR1101MB2196
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Wang, Wei W <wei.w.wang@intel.com>
> Sent: Wednesday, December 29, 2021 10:58 AM
>=20
> > > Reuse KVM_SET_XSAVE for both old/new formats by reimplementing it to
> > > do properly-sized memdup_user() based on the guest fpu container.
> >
> > I'm confused, the first sentence says KVM_SET_XSAVE isn't suitable, the
> > second says it can be reused with minimal effort.
>=20
> Probably "doesn't support" sounds better than "isn't suitable" above. But
> plan to reword a bit:
>=20
> With KVM_CAP_XSAVE, userspace uses a hardcoded 4KB buffer to get/set
> xstate data from/to
> KVM. This doesn't work when dynamic features (e.g. AMX) are used by the
> guest, as KVM uses
> a full expanded xstate buffer for the guest fpu emulation, which is large=
r
> than 4KB.
>=20
> Add KVM_CAP_XSAVE2, and userspace gets the required xstate buffer size
> from KVM via
> KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2). KVM_SET_XSAVE is extended
> with the support to
> work with larger xstate data size passed from userspace. KVM_GET_XSAVE2
> is preferred to
> extending KVM_GET_XSAVE to work with large buffer size for backward-
> compatible considerations.
> (Link: https://lkml.org/lkml/2021/12/15/510)
>=20
> Also, update the api doc with the new KVM_GET_XSAVE2 ioctl.
>=20

Revised to:

--

With KVM_CAP_XSAVE, userspace uses a hardcoded 4KB buffer to get/set
xstate data from/to KVM. This doesn't work when dynamic xfeatures
(e.g. AMX) are exposed to the guest as they require a larger buffer
size.

Introduce a new capability (KVM_CAP_XSAVE2). Userspace VMM gets the
required xstate buffer size via KVM_CHECK_EXTENSION(KVM_CAP_XSAVE2).
KVM_SET_XSAVE is extended to work with both legacy and new capabilities
by doing properly-sized memdup_user() based on the guest fpu container.
KVM_GET_XSAVE is kept for backward-compatible reason. Instead,
KVM_GET_XSAVE2 is introduced under KVM_CAP_XSAVE2 as the preferred
interface for getting xstate buffer (4KB or larger size) from KVM.
(Link: https://lkml.org/lkml/2021/12/15/510)

Also, update the api doc with the new KVM_GET_XSAVE2 ioctl

--

Thanks
Kevin