[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 05 Apr 2022 11:35:39 +1200
From: Kai Huang <kai.huang@...el.com>
To: Dave Hansen <dave.hansen@...el.com>,
"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
bp@...en8.de
Cc: aarcange@...hat.com, ak@...ux.intel.com, brijesh.singh@....com,
dan.j.williams@...el.com, dave.hansen@...ux.intel.com,
david@...hat.com, hpa@...or.com, jgross@...e.com,
jmattson@...gle.com, joro@...tes.org, jpoimboe@...hat.com,
knsathya@...nel.org, linux-kernel@...r.kernel.org, luto@...nel.org,
mingo@...hat.com, pbonzini@...hat.com, peterz@...radead.org,
sathyanarayanan.kuppuswamy@...ux.intel.com, sdeep@...are.com,
seanjc@...gle.com, tglx@...utronix.de, thomas.lendacky@....com,
tony.luck@...el.com, vkuznets@...hat.com, wanpengli@...cent.com,
x86@...nel.org
Subject: Re: [PATCHv7.1 02/30] x86/tdx: Provide common base for SEAMCALL
and TDCALL C wrappers
On Mon, 2022-04-04 at 06:51 -0700, Dave Hansen wrote:
> On 4/3/22 20:19, Kai Huang wrote:
> > Btw, I previous suggested perhaps we can just use -1ULL instead of above value
> > for TDX_SEAMCALL_VMFAILINVALID, but didn't get response. The reason is this
> > value will only be used when detecting P-SEAMLDR using P-SEAMLDR's SEAMLDR.INFO
> > SEAMCALL. Note your above SW-defined error codes is based on error code
> > definition for TDX module, but actually P-SEAMLDR has different error code
> > definition:
>
> I suggested moving away from the -1 because it didn't really carry any
> additional information. For folks that have the spec open day in and
> day out, it's easy for you to go look up what the components of that -1
> _mean_.
>
> It sounds like there's a bug here (mixing up the P-SEAMLDR and TDX
> module error ABIs), but that doesn't mean that moving to -1 is the right
> answer.
I think it doesn't need to carry any additional information. The error code is
used to represent VMfailInvalid, which happens before any P-SEAMLDR and TDX
module internal functionality is reached. We just need a value which will
*never* conflict with actual error code returned by P-SEAMLDR and TDX module to
represent this case.
Both error code formats defined by P-SEAMLDR and TDX module has some reserved
bits which will never be set to 1. I think we can just add a simple comment
explaining that and choose a value which has 1 set for those reserved bits (even
doesn't have to be -1). For example:
/*
* Use -1ULL which will never conflict with any actual error code
* returned by both the P-SEAMLDR and the TDX module to represent
* VMfailInvalid. Both error code definitions defined by the
* P-SEAMLDR and the TDX module have some reserved bits which will
* never be set to 1.
*/
#define TDX_SEAMCALL_VMFAILINVALID GENMASK_ULL(63, 0)
>
> Please just build up an error value the same way it was done for the
> software-defined TDX module error codes.
In this way the assembly code will need to set different value based on whether
%rax is a P-SEAMLDR leaf function and TDX module leaf function. I think it's
unnecessary. As I said above, I think this error doesn't need to have any
additional information. We just need a value which will never conflict with any
actual error code from P-SEAMLDR and TDX module.
--
Thanks,
-Kai
Powered by blists - more mailing lists