Received-SPF: pass (google.com: domain of linux-kernel+bounces-149288-linux.lists.archive=gmail.com@vger.kernel.org designates 139.178.88.99 as permitted sender) client-ip=139.178.88.99;
Date: Thu, 18 Apr 2024 06:41:02 +0800
From: Yan Zhao <yan.y.zhao@intel.com>
To: Sean Christopherson <seanjc@google.com>
CC: Ackerley Tng <ackerleytng@google.com>, <sagis@google.com>,
	<linux-kselftest@vger.kernel.org>, <afranji@google.com>,
	<erdemaktas@google.com>, <isaku.yamahata@intel.com>, <pbonzini@redhat.com>,
	<shuah@kernel.org>, <pgonda@google.com>, <haibo1.xu@intel.com>,
	<chao.p.peng@linux.intel.com>, <vannapurve@google.com>,
	<runanwang@google.com>, <vipinsh@google.com>, <jmattson@google.com>,
	<dmatlack@google.com>, <linux-kernel@vger.kernel.org>, <kvm@vger.kernel.org>,
	<linux-mm@kvack.org>
Subject: Re: [RFC PATCH v5 09/29] KVM: selftests: TDX: Add report_fatal_error
 test
Message-ID: <ZiBP/j6Ic7hGrbxN@yzhao56-desk.sh.intel.com>
Reply-To: Yan Zhao <yan.y.zhao@intel.com>
References: <ZhkhvtijbhxKKAEk@yzhao56-desk.sh.intel.com>
 <diqzr0f7jbj6.fsf@ctop-sg.c.googlers.com>
 <Zhz8xNpQoi0wCQgL@yzhao56-desk.sh.intel.com>
 <Zh7Iay40VQgNvsFW@google.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <Zh7Iay40VQgNvsFW@google.com>
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?Js8wGY4gUZsBmWTwEhjvYsRSUkfQ6J6QB0RNcThqD3vBvrqXjxfi3nQGoJQu?=
 =?us-ascii?Q?OZZ5bqJEBbTLq85wQUWNOhR9cpOcDZR48c7nenSzfjr+t0G/840euMsfA1Iv?=
 =?us-ascii?Q?QC1kM70fKZXs/b5AGt1G9DqWJjaHdZc/dorjCZFIg37EwcffJFc0LqptfEHx?=
 =?us-ascii?Q?9OrKyPLm21LruwGLTRguypsbWram4OY4YP4IVj+qk2LCx57nCvtb0QWfxYjl?=
 =?us-ascii?Q?QznWeXQfqxxX4Qo7Q3vD3d1KxuN4CllDMoHX3iAvN+8wVbXEKOEOjFjHUi+g?=
 =?us-ascii?Q?pLKmEK+IVNl4dJBw0XB0P1RZzbA+bfOztrRaSdlx7ZSEQYm4SBTKZnusWEdB?=
 =?us-ascii?Q?j8TUHG9z3srPQtruVH2YN5dK2R2amKdMNqTlmRWHU4n0BbVaCskCGiL24j+d?=
 =?us-ascii?Q?QlT1R3zjuF1MZo9+enOOEBCh8HfzX3NdO2/KRLTyojIFTQibGNmPPhDjkKxr?=
 =?us-ascii?Q?U0Duj1E0lyqMma2UGI7h5FIQ+oCbTW1cc30t+fpkQAKtrOcE/duGk8UhuxPg?=
 =?us-ascii?Q?g/whMCy439+p76OLitnXnphMLC1hwrJiP+7MFqve9+mDUnEzk3004I3zkmSX?=
 =?us-ascii?Q?Ypbw6xvhkJRuwFWUlxio4cYfmb9XTeZrVLYfZSaVBZOy9MTJoiPLBxCpSY/F?=
 =?us-ascii?Q?svY7FbQyOTHxSELCnXuy66+1EqPeW6uSl4MRmIXQrFrPe6njosVIQ6G1UqsP?=
 =?us-ascii?Q?3w71qT3Ky4vPkAfI+PhFCL0HVVe46TnjisKY+3ntlSsN2PZ/cpvhjPOvRW1O?=
 =?us-ascii?Q?mqs3LWNyyACZ4as/ovRJaum6yqIvAYijnd6/YVmEsDnrdWcGT4tiTsWhdfzb?=
 =?us-ascii?Q?UO74QvQZufE8+jj1zI6AVTlmNr+Nhm4KSXS6WX3e9rNNjq4GlXl7fI0e77sG?=
 =?us-ascii?Q?fduI54gjN0TAbBvBNfHSYocM2wKhxpT2b2ugMtorEd2XHyv7I+MGqaRBhLZw?=
 =?us-ascii?Q?Cht2k8eOhWzi4bvNM+Y2NtTENcQcBFhbQYBo4cm65Zv3v3m+a3mmC0CZhdnB?=
 =?us-ascii?Q?FMRoI5qwd8yl37NdtB1Btpmm/1rI+hu9C/hcJV6dXDj50YDHXw4/fb5Ec7ya?=
 =?us-ascii?Q?nRC1EP9NeIXDfAfJc9Wml1IAaeI9FWSVjJWAMjbiIUhGNbiA0qd4L0wl+FVp?=
 =?us-ascii?Q?3V4HHf8r4j6CNLaoGrfqvCiF508/tVLz7Jx6vkQat/rqEZa9g7zO7+1yiRxI?=
 =?us-ascii?Q?c4NutHzClqdUSqs54xdcWZWE4h3hZvikflaaGMytouDF3zmljBMn0DoOC4Im?=
 =?us-ascii?Q?axxels0K5IW6eeyvVfZKtOIv3lCAN8kY1EXdRXemod/oCxpUN3yfIbi9N1hg?=
 =?us-ascii?Q?VHZjvtYPNcRNKhED1TJBKr1Nb8MC5OOvziHBLynQ6/OB4aLnYFFL+askZrbQ?=
 =?us-ascii?Q?FQGDNjK0rJcWEinONbqoPrPdhFlGWw9KXbOlekCsz9G7RSeKBCtXWnAw+lco?=
 =?us-ascii?Q?+WQSHpxDzrYF2A6uoKxw3rxiZ8wL+6Sm/qyJM+5U05ncmG9mr7Sn0DkbvDqj?=
 =?us-ascii?Q?5l1fdlEMHYvR7hQdMTGX55y7C4U9UxdXCJQ4WS3D+cQgJwGxTABs1dKO0Kbe?=
 =?us-ascii?Q?pwC/DvT+XxlBDlg8cSzrJsix3us/gtorf9mW3pxO?=
X-MS-Exchange-CrossTenant-Network-Message-Id: b5c40a7b-cc5f-480e-deca-08dc5f2f8a6a
X-MS-Exchange-CrossTenant-AuthSource: DS7PR11MB5966.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Apr 2024 22:41:51.2362
 (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: KlOJh6PLYo5c0Azez0Sw6LVq/Ef7JALH4nGkGZaAxew/nx6GyAQ7TdoxgMHG7mbZbQINlc9spQ/dSS9cQnCVfA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA1PR11MB7199
X-OriginatorOrg: intel.com

On Tue, Apr 16, 2024 at 11:50:19AM -0700, Sean Christopherson wrote:
> On Mon, Apr 15, 2024, Yan Zhao wrote:
> > On Mon, Apr 15, 2024 at 08:05:49AM +0000, Ackerley Tng wrote:
> > > >> The Intel GHCI Spec says in R12, bit 63 is set if the GPA is valid. As a
> > > > But above "__LINE__" is obviously not a valid GPA.
> > > >
> > > > Do you think it's better to check "data_gpa" is with shared bit on and
> > > > aligned in 4K before setting bit 63?
> > > >
> > > 
> > > I read "valid" in the spec to mean that the value in R13 "should be
> > > considered as useful" or "should be passed on to the host VMM via the
> > > TDX module", and not so much as in "validated".
> > > 
> > > We could validate the data_gpa as you suggested to check alignment and
> > > shared bit, but perhaps that could be a higher-level function that calls
> > > tdg_vp_vmcall_report_fatal_error()?
> > > 
> > > If it helps, shall we rename "data_gpa" to "data" for this lower-level,
> > > generic helper function and remove these two lines
> > > 
> > > if (data_gpa)
> > > 	error_code |= 0x8000000000000000;
> > > 
> > > A higher-level function could perhaps do the validation as you suggested
> > > and then set bit 63.
> > This could be all right. But I'm not sure if it would be a burden for
> > higher-level function to set bit 63 which is of GHCI details.
> > 
> > What about adding another "data_gpa_valid" parameter and then test
> > "data_gpa_valid" rather than test "data_gpa" to set bit 63?
> 
> Who cares what the GHCI says about validity?  The GHCI is a spec for getting
> random guests to play nice with random hosts.  Selftests own both, and the goal
> of selftests is to test that KVM (and KVM's dependencies) adhere to their relevant
> specs.  And more importantly, KVM is NOT inheriting the GHCI ABI verbatim[*].
> 
> So except for the bits and bobs that *KVM* (or the TDX module) gets involved in,
> just ignore the GHCI (or even deliberately abuse it).  To put it differently, use
> selftests verify *KVM's* ABI and functionality.
> 
> As it pertains to this thread, while I haven't looked at any of this in detail,
> I'm guessing that whether or not bit 63 is set is a complete "don't care", i.e.
> KVM and the TDX Module should pass it through as-is.
> 
> [*] https://lore.kernel.org/all/Zg18ul8Q4PGQMWam@google.com
Ok. It makes sense to KVM_EXIT_TDX.
But what if the TDVMCALL is handled in TDX specific code in kernel in future?
(not possible?)
Should guest set bits correctly according to GHCI?