Re: [PATCH v2 07/11] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL

[Huang, Kai]

> 
> With above changes, change the asm function from __tdx_hypercall() to
> __tdcall_hypercall(), and reimplement __tdx_hypercall() as the C wrapper
> of it.  This avoids having to add another wrapper of __tdx_hypercall()
> (_tdx_hypercall() is already taken).
> 
[...]
> 
> By changing __tdx_hypercall() to a C function, __tdx_hypercall_failed()
> is no longer called from '.noinstr.text', thus remove the 'noinstr'
> annotation and the 'instrumentation_begin()'.
> 

Hi Peter,

Sorry I got a build regression from lkp:

vmlinux.o: warning: objtool: __halt+0x3a: call to __tdx_hypercall() leaves
.noinstr.text section

The reason is __halt() is annotated with "__cpuidle", but it calls
__tdx_hypercall(), which is now a normal C function, w/o
instrumentation_begin()/end().

I think we can annotate __tdx_hypercall() as 'noinstr' too and keep
__tdx_hypercall_failed() unchanged.  Anyway in the upstream code,
__tdx_hypercall() is in '.noinstr.text'.

diff --git a/arch/x86/coco/tdx/tdx-shared.c b/arch/x86/coco/tdx/tdx-shared.c
index 13139ee171c8..b47c8cce91b0 100644
--- a/arch/x86/coco/tdx/tdx-shared.c
+++ b/arch/x86/coco/tdx/tdx-shared.c
@@ -70,7 +70,7 @@ bool tdx_accept_memory(phys_addr_t start, phys_addr_t end)
        return true;
 }
 
-u64 __tdx_hypercall(struct tdx_hypercall_args *args)
+noinstr u64 __tdx_hypercall(struct tdx_hypercall_args *args)
 {
        struct tdx_module_args margs = {
                .rcx = TDVMCALL_EXPOSE_REGS_MASK,
diff --git a/arch/x86/coco/tdx/tdx.c b/arch/x86/coco/tdx/tdx.c
index aba58484ba10..f10b0e512a36 100644
--- a/arch/x86/coco/tdx/tdx.c
+++ b/arch/x86/coco/tdx/tdx.c
@@ -37,8 +37,9 @@
 
 #define TDREPORT_SUBTYPE_0     0
 
-void __tdx_hypercall_failed(void)
+noinstr void __tdx_hypercall_failed(void)
 {
+       instrumentation_begin();
        panic("TDVMCALL failed. TDX module bug?");
 }


We can fold this to this patch.

Do you have any comments?