From:   "Tian, Kevin" <kevin.tian@intel.com>
To:     "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>,
        "Wang, Wei W" <wei.w.wang@intel.com>,
        "Zhong, Yang" <yang.zhong@intel.com>
Subject: RE: [PATCH v3 05/22] kvm: x86: Check permitted dynamic xfeatures at
 KVM_SET_CPUID2
Thread-Topic: [PATCH v3 05/22] kvm: x86: Check permitted dynamic xfeatures at
 KVM_SET_CPUID2
Thread-Index: AQHX9zEx73/scnIpjECK4V7v5r/FGqxImcEAgAArYBA=
Date:   Wed, 29 Dec 2021 02:18:28 +0000
Message-ID: <BN9PR11MB52762FBBB5E887385F8DCC038C449@BN9PR11MB5276.namprd11.prod.outlook.com>
References: <20211222124052.644626-1-jing2.liu@intel.com>
 <20211222124052.644626-6-jing2.liu@intel.com> <YcugDN+9d8t4r9bV@google.com>
In-Reply-To: <YcugDN+9d8t4r9bV@google.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?uYanAd8ohKRLWaOBDciFikvXLIUijgK93nO1jwA7RTQQhi9GVYvbCWWtlo95?=
 =?us-ascii?Q?z5PcW7NfNcnmR8SGh3fnQ6YVYFpMBXf8byq4xZ/qoy0ztoTplr+NSHsNYK8K?=
 =?us-ascii?Q?sfrvo3laiP76smZazP2l7DiQzIC+/6Rtx7e2GSCZ8WK7XzFnqMZ5/1SfT5vK?=
 =?us-ascii?Q?XOrFWF5rC5uczUTRUR4F2bFpSzcBOfDLTJXuAa2Cu9s/7GFUPCKN0OMHNEJ9?=
 =?us-ascii?Q?1CMjQ2DS1fF1l2hqoyvFn/VAbCWs+FBWmNToSJQGJK0eN0Vd1tV0S0NvtgGZ?=
 =?us-ascii?Q?SOnjgAyMxRo8f3Dqj/F4/2NCLO9oI8iJAXgcVISGMzce/8MLKOjUzh+clHuh?=
 =?us-ascii?Q?Vx1t0ZUyOeiqTv2V3BPAcalsvAbX7DC/BzSUobKutD8inhXC7SiOz3uJc/Dg?=
 =?us-ascii?Q?QKZl9ZAuWDK+nWvaezNahuYkYHEnWcDrsGFdIXeeXVlxkhAoM8aM/vomDtoI?=
 =?us-ascii?Q?vwKgWTBubmnlWwpWMxRTon/AkZJWpAq2N6SQYQCRsPnkpf+EfGxZ8LIrfAVV?=
 =?us-ascii?Q?1MQ3XLtvI58Jk1+zjDrmH3dQmm2sHiV3Md4PeWZ1+zYzigsXzXN3oHewLDK0?=
 =?us-ascii?Q?4u6Ct/A0SDUXVqCNruNAEBZtpAFBW5Sawqqj+c6ChhlABf1jdratJhqKtiYl?=
 =?us-ascii?Q?vppfKCDIirsJA/1hRqBl/tleKqUIhkX0M3eokVNFq1Hq1/nHaFMRQpVRpAUj?=
 =?us-ascii?Q?mJM7rPsfHi2C9a74xLGZDG8Q8raJbti+p5gIA/5qS/x4DaWYtfbtQTz1LgTb?=
 =?us-ascii?Q?S6DXSW9/dmww4gGdjKGEWmklta/ECimkANWY8P2L3/70+d8Rp+KghJxVBLkn?=
 =?us-ascii?Q?ZHfuNH/6mqMQzeYOFFmjabyTzx5kqkY8Wr6/Or3sHV1TmeOMBC2M1Kc/Jpjj?=
 =?us-ascii?Q?/6q7erPQh2kYrnsiiSaiyPnPbRsPAECjCYuQbMPRdnuMPH9Cs8MpIZoZM67A?=
 =?us-ascii?Q?kue+m9GGcMemoUKrXthlJnoQk32m9aVEdi7abPUpbU2vf9ZfgwiO8yJdNR+J?=
 =?us-ascii?Q?u3QVCbIP2hh5+UpJuuvNg2sxxnct9pk6y0kk2Wq4hJ859kDsC7HdRX1qahf+?=
 =?us-ascii?Q?EHVQDeMmNadzti3ZK5k8hL86R81DXPlCZMnv2HR7wPaaWePTNiTOTqpA13Ni?=
 =?us-ascii?Q?hDSy8QXBbaXiR69Z/qA9ZCiN9BEJXcQK7LQsR0Jg1MgORmGmfnN+uWg4MepI?=
 =?us-ascii?Q?HsctZ7wAfLoHV4s5uRQfzYbRYVDuuN8XyuFdd/vZZ8Igjlc4qQcyIjg0Bzmy?=
 =?us-ascii?Q?YrdbBSbwVD3+6kDoa5knPWZxahE09JxNjubLb7YuismwTR/wbdjFTaLLRe/J?=
 =?us-ascii?Q?Mbieue69DzLf7JxJGVJQiJRpRbuUcaUbPxUnHOxSeyjvx6HqRxfx7HmTz6Gd?=
 =?us-ascii?Q?+E+k3JxzHWbQVfvoKWvTUaqxQKlRBoGLdiMGnRTRqeuPbVIb4YovLBb85ZS4?=
 =?us-ascii?Q?GQ683zZ8qGAVaSRZchOKRLyCtsT1ThoFCT8WqOshUGAVoO915keqgy+QJxO3?=
 =?us-ascii?Q?X1vLDSyDSx9uMqPoCfUWNigficIL0q6mMiFPUz9Aft4ZkID5228BNrmab772?=
 =?us-ascii?Q?UIyFg50pTR7vR4JypsTqVZ01OKpFgfT80vQfh0ubN+GsRWwmz6dQc2DU35yh?=
 =?us-ascii?Q?UDufOEG4dSOgnaZJ3aAzoXI=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: e0cffb3b-f5cf-4fa3-b5cc-08d9ca71806d
X-MS-Exchange-CrossTenant-originalarrivaltime: 29 Dec 2021 02:18:28.3160
 (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: y/2Qqsy/eYI7Y5UYG2r3jjW9GjKWCsCsexB1AMUkXk4FZEsLfjEVNGe1rHZdoMSQhm9Mfh+BgbcKxPKcBVasoA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BN7PR11MB2706
X-OriginatorOrg: intel.com
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

> From: Sean Christopherson <seanjc@google.com>
> Sent: Wednesday, December 29, 2021 7:39 AM
>=20
> On Wed, Dec 22, 2021, Jing Liu wrote:
> > Guest xstate permissions should be set by userspace VMM before vcpu
> > creation. Extend KVM_SET_CPUID2 to verify that every feature reported
> > in CPUID[0xD] has proper permission set.
>=20
> Why?  Nothing in the changelog, code, or comments explains why KVM
> _needs_ to prevent
> userspace from advertising bogus features to the guest.  E.g. the virtual
> address
> width check exists because _KVM_ will do the wrong thing given a width
> other than 48
> or 57, and explicity says as much in a comment.

Advertising a known bogus feature due to lacking of permission does
no good compared to failing it early even before the guest is running. This
also avoids tons of complexity at run-time to deal with permission
violation (e.g. introducing new kvm exit reason if you tracked the v2
discussion).

But yes, we should add a clear comment here.

Thanks
Kevin