Messages in this thread | ![/](/images/icornerl.gif) | | 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 11:06:47 +0000 |
| |
On Fri, 2023-07-21 at 10:01 +0200, Peter Zijlstra wrote: > On Fri, Jul 21, 2023 at 05:19:30AM +0000, Huang, Kai wrote: > > > Sorry I got a build regression from lkp: > > > > vmlinux.o: warning: objtool: __halt+0x3a: call to __tdx_hypercall() leaves > > .noinstr.text section > > Clearly you should enable CONFIG_DEBUG_ENTRY in your own development > .config ;-)
Ah thanks a lot!
I've been investigating how to reproduce at my own environment this afternoon!
LKP people are not aware of what triggered this check either. I'll pass to them.
> > > 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? > > Yes this is fine, we violate noinstr for WARN etc.. There's no point > being pendantic about these things if we're going to take the machine > down anyway.
Thanks. I'll fold above to this patch and sent out a v3 (given that it seems you also want a patch to adjust the layout of 'struct tdx_module_args' for TDH.VP.ENTER).
| ![\](/images/icornerr.gif) |