Received-SPF: pass (google.com: domain of linux-kernel+bounces-175540-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: "Li, Xin3" <xin3.li@intel.com>
To: "Gao, Chao" <chao.gao@intel.com>
CC: "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"kvm@vger.kernel.org" <kvm@vger.kernel.org>, "linux-doc@vger.kernel.org"
	<linux-doc@vger.kernel.org>, "linux-kselftest@vger.kernel.org"
	<linux-kselftest@vger.kernel.org>, "seanjc@google.com" <seanjc@google.com>,
	"pbonzini@redhat.com" <pbonzini@redhat.com>, "corbet@lwn.net"
	<corbet@lwn.net>, "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>, "x86@kernel.org"
	<x86@kernel.org>, "hpa@zytor.com" <hpa@zytor.com>, "shuah@kernel.org"
	<shuah@kernel.org>, "vkuznets@redhat.com" <vkuznets@redhat.com>,
	"peterz@infradead.org" <peterz@infradead.org>, "Shankar, Ravi V"
	<ravi.v.shankar@intel.com>, "xin@zytor.com" <xin@zytor.com>
Subject: RE: [PATCH v2 12/25] KVM: VMX: Handle FRED event data
Thread-Topic: [PATCH v2 12/25] KVM: VMX: Handle FRED event data
Thread-Index: AQHaWfA7ZVshGt6l4U6e8iRthcdnlLGApUEAgA/9uwA=
Date: Fri, 10 May 2024 09:36:03 +0000
Message-ID: <SA1PR11MB6734740F9B6085E0997A4179A8E72@SA1PR11MB6734.namprd11.prod.outlook.com>
References: <20240207172646.3981-1-xin3.li@intel.com>
 <20240207172646.3981-13-xin3.li@intel.com> <ZjBiLDJ4SdQ0p5xm@chao-email>
In-Reply-To: <ZjBiLDJ4SdQ0p5xm@chao-email>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?bu6y4b37yLEj0XLo3Yv+m2XgqltwgRuCwVFjK3OhcRW4tsY5HqyJb1yMsYAz?=
 =?us-ascii?Q?rmk+qH5FrIzUSdACu+OR9M1in9T2REO6IA24TdwlUrCeIpzSGyxjvJtQs6nM?=
 =?us-ascii?Q?SerjWAsfmoEIbFKIC2RCj2qrnswyYyYQ7j6fmq3d3JLOTUaBkwvosYN8G9RK?=
 =?us-ascii?Q?0vzPC8BBs6MekB4aM84IkpA0hwloGgsmkYw9ahOnYrXq5s1PW8I93FHccZLP?=
 =?us-ascii?Q?Z8zPsYwFADGHvLH2XQFLgTh3VRQrkG6yBF4dEkx4mYgay1p9eSNL0cKCoLzc?=
 =?us-ascii?Q?Gvus3EMIOihsQCskDAPRMNLb2TteJIYum3s0dqwDERFWnbXwJeJY2WH+kD/k?=
 =?us-ascii?Q?Jj11YRAc2qAlruTymojN8R9DqeT2jYSxFYr5gFhRjDxQ68yWSlxCumZSCciC?=
 =?us-ascii?Q?/oGo3ODHndEY9Tf0PiaweVfDD73+gPP39Jbsp59kYc+TyclFFDiS5tG++zh/?=
 =?us-ascii?Q?Zthqn5ggqeLpDvo0bv76ZiVjxmo9gWnaztG1MvwkW3JlLnFBcUtwf1tIDDQh?=
 =?us-ascii?Q?sRTpEoEo6+1KlkbK7v6CEjVFdZ5d7QdiU2kLJScLkUevYW+39121bWShlRiq?=
 =?us-ascii?Q?X4CLABLgft8F+NJjSUXBH1LxLZjrxB9IZ1nKXQ00Ljmft5JyJIJhBK1kz6KU?=
 =?us-ascii?Q?p88vRR+2DHzR61gdvnkN6tUNudXe9sw0KXOPTg7FLxn8CwypYE4q0qnj5GPD?=
 =?us-ascii?Q?438t8/jX4uqLjzN5xuHp+cLi+D6dkpKF5BbEof8o6eYbfuKOOzCqZPSf3ASI?=
 =?us-ascii?Q?YgfmjJKjzZVC5Iyhl6gPcveZ4p3FcWDC+ZAm0rQswYissGs92SNGH3WfPsER?=
 =?us-ascii?Q?BQ+zos7KOmXrsGe1WMAgu+SD8vDL1tRa/oI/dO+PlWYbU6kBkGQSNQ4BCh6d?=
 =?us-ascii?Q?Y0P0wrphS2U5CiAm29vEwDpg6r+6yIvS9670/mouemtkzR61UT9O1s58Zgwl?=
 =?us-ascii?Q?yPDZ9SW9w469o0opM1N7kCVVHrfmRMmck0T29z/PAp8wsFYbghKyu5A0B0qJ?=
 =?us-ascii?Q?Qdy95nhJmrzYuwk+cxOfnyXBM2pEnXwbWvD/HpzOYhY8feVysodCtsibx1Rq?=
 =?us-ascii?Q?zvId3PXZyYKAgX+tdwuBDe6hn+RjYC1TwNj0qE/ClyGlkY8yiqZRa4sfaqtG?=
 =?us-ascii?Q?JkwzuPqhtxAw+wgFcU0Kvo9biUKZ/ITa4R/Y8fo+N6RdsUjehWlsP3igqx26?=
 =?us-ascii?Q?Sq1D39hbqXXZtu/2bMYWEnoNgcZMDOVvSThuPuJci9D8Uy+lk7BrH2aQokDv?=
 =?us-ascii?Q?m9JMwuyv5VWWJrZ3QNVRd8v9PruYYe/nY4IUZpgyokeqDRNNXbobyrh7oZM6?=
 =?us-ascii?Q?r20AwOaBf9yL+dL6iJZNTnhsqMcnCpNxS0qnbOAQ2Psrv5qXx7pcNuONFsZQ?=
 =?us-ascii?Q?ohjMueAI1GJkZYmKwjy8lYP6qI7ZAxhdD5+DI1XiPC7q86SOJ3QJeJaTaYp4?=
 =?us-ascii?Q?CNmpNL0RzixRbHk0KZSNiWhiAUHKbkSQ+w3j/RMUt/hacf/QycqZvwi4sXnr?=
 =?us-ascii?Q?Kf+GaElN2X8RCjdHDDzLgUIj5Y52auv637WiU+hp+gdV+W4fd6833b4kb9tn?=
 =?us-ascii?Q?AlAjHzqbLitGpKRlC8s=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk
X-Mailing-List: linux-kernel@vger.kernel.org
List-Id: <linux-kernel.vger.kernel.org>
List-Subscribe: <mailto:linux-kernel+subscribe@vger.kernel.org>
List-Unsubscribe: <mailto:linux-kernel+unsubscribe@vger.kernel.org>
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: SA1PR11MB6734.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: a38be1dd-557e-472b-bcb3-08dc70d49c74
X-MS-Exchange-CrossTenant-originalarrivaltime: 10 May 2024 09:36:03.9953
 (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: uv+6LrdFJS4H3QUNwppUNUMxqkTyJZElP+sdcxPx/19f3D8svhtAguFouxi5X9OKqWg+8YzmTjfBHFGA3YBZMA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA3PR11MB8118
X-OriginatorOrg: intel.com

> >+               if (kvm_is_fred_enabled(vcpu)) {
> >+                       u64 event_data =3D 0;
> >+
> >+                       if (is_debug(intr_info))
> >+                               /*
> >+                                * Compared to DR6, FRED #DB event data =
saved on
> >+                                * the stack frame have bits 4 ~ 11 and =
16 ~ 31
> >+                                * inverted, i.e.,
> >+                                *   fred_db_event_data =3D dr6 ^ 0xFFFF=
0FF0UL
> >+                                */
> >+                               event_data =3D vcpu->arch.dr6 ^ DR6_RESE=
RVED;
> >+                       else if (is_page_fault(intr_info))
> >+                               event_data =3D vcpu->arch.cr2;
> >+                       else if (is_nm_fault(intr_info))
> >+                               event_data =3D
> >+ to_vmx(vcpu)->fred_xfd_event_data;
> >+
>=20
> IMO, deriving an event_data from CR2/DR6 is a little short-sighted becaus=
e the
> event_data and CR2/DR6 __can__ be different, e.g., L1 VMM __can__ set CR2=
 to A
> and event_data field to B (!=3DA) when injecting #PF.

VMM should guarantee a FRED guest _sees_ consistent values in CR6/DR6
and event data. If not it's just a VMM bug that we need to fix.

>=20
> And this approach cannot be extended to handle a (future) exception whose
> event_data isn't tied to a dedicated register like CR2/DR6.

See below.

> Adding a new field fred_xfd_event_data in struct vcpu has problems too:
> fred_xfd_event_data gets lost during migration;

I'm not bothered, because this is not hard to fix, right?

> strickly speaking, event_data is tied
> to an exception rather than a CPU. e.g., the CPU may detect a nested exce=
ption when
> delivering one and both have their own event_data.

No, don't get me wrong. An event data has to be _regenerated_ after
a nested exception is handled and the original instruction flow is
restarted.=20
sometimes the original event could be gone.

We don't say an event data is tied to an exception or a CPU, which
is just confusing, or misleading.

> I think we can make event_data a property of exceptions. i.e., add a payl=
oad2 to
> struct kvm_queued_exception. and add new APIs to kvm_queue_exception* fam=
ily to
> accept a payload2 and in VMX code, just program payload2 to the VMCS even=
t_data
> field if FRED is enabled. KVM ABI should be extended as well to pass
> payload2 to userspace like how the payload is handled in
> kvm_vcpu_ioctl_x86_get/put_vcpu_events.

Yes, it's very likely that we will need to add a payload2 in future,
but NOT now. 2 reasons:

1) The first-generation FRED is designed to NOT go too far from what
   IDT can do. And FRED event data is conceptually an alias of CR2/DR6
   in the latest FRED spec (not considering xfd event data for now).
   And the existing payload is a nice match for now;

2) FRED is an extendable CPU architecture, which allows the structure
   of event data to become way bigger and complicated. Let's not assume
   anything and add a payload2 too early.