Messages in this thread Patch in this message | | | From | "Huang, Kai" <> | Subject | Re: [PATCH v2 07/11] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE_CALL | Date | Fri, 21 Jul 2023 05:19:30 +0000 |
| |
> > 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?
| |