On 17.07.23 г. 9:35 ч., Huang, Kai wrote:
+/* Called from __tdx_hypercall() for unrecoverable failure */
+static noinstr void __tdx_hypercall_failed(void)
+{
+ instrumentation_begin();
+ panic("TDVMCALL failed. TDX module bug?");
+}
So what's the deal with this instrumentation here. The instruction is
noinstr, so you want to make just the panic call itself instrumentable?,
if so where's the instrumentation_end() cal;?No instrumentation_end()
call. Actually is this complexity really worth it for the failure case?
AFAICS there is a single call site for __tdx_hypercall_failed so why
noot call panic() directly ?
W/o this patch, the __tdx_hypercall_failed() is called from the TDX_HYPERCALL
assembly, which is in .noinstr.text, and 'instrumentation_begin()' was needed to
avoid the build warning I suppose.
However now with this patch __tdx_hypercall_failed() is called from
__tdx_hypercall() which is a C function w/o 'noinstr' annotation, thus I believe
instrumentation_begin() and 'noinstr' annotation are not needed anymore.
I didn't notice this while moving this function around and my kernel build test
didn't warn me about this. I'll change in next version.
In fact, perhaps this patch perhaps is too big for review. I will also try to
split it to smaller ones.
Can't you simply call panic() directly? Less going around the code while
someone is reading it?
<snip>
