linux-kernel - Re: [PATCH v2 07/11] x86/tdx: Make TDX_HYPERCALL asm similar to TDX_MODULE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <152cfe1f60a9f90a3950c37e676990be6cad59b2.camel@intel.com>
Date:   Fri, 21 Jul 2023 05:19:30 +0000
From:   "Huang, Kai" <kai.huang@...el.com>
To:     "kirill.shutemov@...ux.intel.com" <kirill.shutemov@...ux.intel.com>,
        "peterz@...radead.org" <peterz@...radead.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     "Hansen, Dave" <dave.hansen@...el.com>,
        "Christopherson,, Sean" <seanjc@...gle.com>,
        "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "pbonzini@...hat.com" <pbonzini@...hat.com>,
        "n.borisov.lkml@...il.com" <n.borisov.lkml@...il.com>,
        "Yamahata, Isaku" <isaku.yamahata@...el.com>,
        "sathyanarayanan.kuppuswamy@...ux.intel.com" 
        <sathyanarayanan.kuppuswamy@...ux.intel.com>
Subject: Re: [PATCH v2 07/11] x86/tdx: Make TDX_HYPERCALL asm similar to
 TDX_MODULE_CALL


> 
> 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?