Messages in this thread | ![/](/images/icornerl.gif) | | Subject | Re: [PATCHv7.1 02/30] x86/tdx: Provide common base for SEAMCALL and TDCALL C wrappers | From | Kai Huang <> | Date | Tue, 05 Apr 2022 11:35:39 +1200 |
| |
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
| ![\](/images/icornerr.gif) |