linux-kernel - Re: [PATCHv2 01/12] x86/tdx: Consolidate TDX error handling

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <70484aa1b553ca250d893f80b2687b5d915e5309.camel@intel.com>
Date: Wed, 30 Jul 2025 18:32:17 +0000
From: "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
To: "Hansen, Dave" <dave.hansen@...el.com>, "seanjc@...gle.com"
	<seanjc@...gle.com>
CC: "Gao, Chao" <chao.gao@...el.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "x86@...nel.org" <x86@...nel.org>, "Huang,
 Kai" <kai.huang@...el.com>, "bp@...en8.de" <bp@...en8.de>, "mingo@...hat.com"
	<mingo@...hat.com>, "kirill.shutemov@...ux.intel.com"
	<kirill.shutemov@...ux.intel.com>, "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>, "pbonzini@...hat.com" <pbonzini@...hat.com>,
	"Zhao, Yan Y" <yan.y.zhao@...el.com>, "linux-coco@...ts.linux.dev"
	<linux-coco@...ts.linux.dev>, "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
	"tglx@...utronix.de" <tglx@...utronix.de>, "Yamahata, Isaku"
	<isaku.yamahata@...el.com>
Subject: Re: [PATCHv2 01/12] x86/tdx: Consolidate TDX error handling

On Thu, 2025-06-26 at 09:59 -0700, Dave Hansen wrote:
> On 6/26/25 08:51, Sean Christopherson wrote:
> > No, I was thinking:
> > 
> >  	if (IS_TDX_ERR_OPERAND_BUSY(err))
> > 
> > e.g. to so that it looks like IS_ERR(), which is a familiar pattern.
> 
> That would be a more more compelling if IS_ERR() worked on integers. It
> works on pointers, so I'm not sure it's a pattern we want to apply to
> integers here.
> 
> I kind of hate all of this. I'd kinda prefer that we just shove the TDX
> error codes as far up into the helpers as possible rather than making
> them easier to deal with in random code.

Hi,

Picking this up from Kirill. At a high level Dave seems to want to encapsulate
the TDX error code stuff more, and Sean wants something more lightweight. This
seems to be partly a style difference between arch/x86 and KVM, but also a
tension between how much TDX interface to wrap (i.e. the SEAMCALL wrapper
layer).

But at a code level, the helpers have basically identical logic. The difference
between IS_TDX_ERR_OPERAND_BUSY() and tdx_operand_busy() seems more about
whether they look more raw. Since KVM side has many more users of error code
parsing, I'll lean towards Sean's preference of the all caps macro-like
signature. Since Dave points out IS_ERR() operates on pointers, I'll go with
something else. TDX docs call these "Completion Status Codes", so maybe:
STATUS_OPERAND_BUSY()?

As far as leaking TDX bits out of the SEAMCALL wrappers. I did consider trying
to convert the error codes into errno codes at the wrapper level, which arch/x86
side already does internally. I think we could mostly do that for the wrappers
that KVM uses, but there would be few cases (VCPU_NOT_ASSOCIATED) where you
would have to look at the code to see which errno matches to which TDX concept.

The other problem with translating it to errno would be that we print out the
TDX error codes in a lot of warning cases (KVM_BUG_ON(), etc). We already went
through this somewhat with the TDX extended error codes. The bits of the normal
error code could be very useful for debugging too, and only the KVM callers
knows whether to print them out or not. So we would need to return the TDX
format error code anyway, and at that point the TDX->errno conversion would seem
like superfluous complexity.

So STATUS_OPERAND_BUSY() seems like an ok thing to try next for v3 of this
series at least. Unless anyone has any strong objections ahead of time.

Thanks,

Rick