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: <3e55fd58-1d1c-437a-9e0a-72ac92facbb5@intel.com>
Date: Thu, 26 Jun 2025 07:46:55 -0700
From: Dave Hansen <dave.hansen@...el.com>
To: "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
 Sean Christopherson <seanjc@...gle.com>
Cc: Rick P Edgecombe <rick.p.edgecombe@...el.com>,
 "pbonzini@...hat.com" <pbonzini@...hat.com>,
 "dave.hansen@...ux.intel.com" <dave.hansen@...ux.intel.com>,
 Chao Gao <chao.gao@...el.com>, "bp@...en8.de" <bp@...en8.de>,
 Kai Huang <kai.huang@...el.com>, "x86@...nel.org" <x86@...nel.org>,
 "mingo@...hat.com" <mingo@...hat.com>, Yan Y Zhao <yan.y.zhao@...el.com>,
 "tglx@...utronix.de" <tglx@...utronix.de>,
 "kvm@...r.kernel.org" <kvm@...r.kernel.org>,
 "linux-coco@...ts.linux.dev" <linux-coco@...ts.linux.dev>,
 Isaku Yamahata <isaku.yamahata@...el.com>,
 "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCHv2 01/12] x86/tdx: Consolidate TDX error handling

On 6/26/25 02:25, kirill.shutemov@...ux.intel.com wrote:
>> Can we turn them into macros that make it super obvious they are checking if the
>> error code *is* xyz?  E.g.
>>
>> #define IS_TDX_ERR_OPERAND_BUSY
>> #define IS_TDX_ERR_OPERAND_INVALID
>> #define IS_TDX_ERR_NO_ENTROPY
>> #define IS_TDX_ERR_SW_ERROR
>>>> As is, it's not at all clear that things like tdx_success() are
>> simply checks, as opposed to commands.
> I remember Dave explicitly asked for inline functions over macros
> where possible.
> 
> Can we keep them as functions, but give the naming scheme you
> proposing (but lowercase)?

Macros versus function isn't super important. I think Sean was asking if
we could do:

	if (err == IS_TDX_ERR_OPERAND_BUSY)
		...

instead of:

	if (tdx_operand_busy(err))
		...

We can do that, bu we first need to know that the whole bottom of the
return code register is empty. Unfortunately, a quick grep of the TDX
module source shows a bunch of these:

>     return_val = api_error_with_operand_id(TDX_OPERAND_BUSY, OPERAND_ID_MIGSC);

I'll refrain from casting judgement on why the TDX module needs such
fancy, fine-grained error codes and our little hobby kernel over here
mostly gets by on a couple errno's ... but I digress.

Those fancy pants error codes are why we need:

static inline u64 tdx_status(u64 err)
{
	return err & TDX_STATUS_MASK;
}

and can't just check the err directly. We need to mask out the fancy
pants bits first.

To get to what Sean is asking for, we'd have to do the tdx_status()
masking in the low-level SEAMCALL helpers and have them all return a
masked error code. Or maybe just bite the bullet and mostly move over to
errno's.

That wouldn't be horrible. For errno's or a masked TDX-format err,
callers could always go digging in tdx_module_args if they need the bits
that got masked out. But it would take some work.