Re: [PATCH v9 05/18] x86/virt/tdx: Add SEAMCALL infrastructure

[Dave Hansen]

On 2/13/23 03:59, Kai Huang wrote:
> diff --git a/arch/x86/include/asm/tdx.h b/arch/x86/include/asm/tdx.h
> index 4a3ee64c1ca7..5c5ecfddb15b 100644
> --- a/arch/x86/include/asm/tdx.h
> +++ b/arch/x86/include/asm/tdx.h
> @@ -8,6 +8,10 @@
>  #include <asm/ptrace.h>
>  #include <asm/shared/tdx.h>
>  
> +#ifdef CONFIG_INTEL_TDX_HOST
...
> +#define TDX_SEAMCALL_GP			(TDX_SW_ERROR | X86_TRAP_GP)
> +#define TDX_SEAMCALL_UD			(TDX_SW_ERROR | X86_TRAP_UD)
> +
> +#endif

All these kinds of header #ifdefs do it make it harder to write code in
.c files without matching #ifdefs.  Think of code like this completely
made up example:

	if (!tdx_enable()) {
		// Success!  Make a seamcall:
		int something = tdx_seamcall();
		if (something == TDX_SEAMCALL_UD)
			// oh no!
	}

tdx_enable() can never return 0 if CONFIG_INTEL_TDX_HOST=n, so the
entire if() block is optimized away by the compiler.  *BUT*, if you've
#ifdef'd away TDX_SEAMCALL_UD, you'll get a compile error.  People
usually fix the compile error like this:

	if (!tdx_enable()) {
#ifdef CONFIG_INTEL_TDX_HOST
		// Success!  Make a seamcall:
		int something = tdx_seamcall();
		if (something == TDX_SEAMCALL_UD)
			// oh no!
#endif
	}

Which isn't great.

Defining things unconditionally in header files is *FINE*, as long as
the #ifdefs are there somewhere to make the code go away at compile time.

Please post an updated (and tested) patch as a reply to this.