[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ZiBP/j6Ic7hGrbxN@yzhao56-desk.sh.intel.com>
Date: Thu, 18 Apr 2024 06:41:02 +0800
From: Yan Zhao <yan.y.zhao@...el.com>
To: Sean Christopherson <seanjc@...gle.com>
CC: Ackerley Tng <ackerleytng@...gle.com>, <sagis@...gle.com>,
<linux-kselftest@...r.kernel.org>, <afranji@...gle.com>,
<erdemaktas@...gle.com>, <isaku.yamahata@...el.com>, <pbonzini@...hat.com>,
<shuah@...nel.org>, <pgonda@...gle.com>, <haibo1.xu@...el.com>,
<chao.p.peng@...ux.intel.com>, <vannapurve@...gle.com>,
<runanwang@...gle.com>, <vipinsh@...gle.com>, <jmattson@...gle.com>,
<dmatlack@...gle.com>, <linux-kernel@...r.kernel.org>, <kvm@...r.kernel.org>,
<linux-mm@...ck.org>
Subject: Re: [RFC PATCH v5 09/29] KVM: selftests: TDX: Add report_fatal_error
test
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?
Powered by blists - more mailing lists