* [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall()
@ 2023-01-30 13:53 Kirill A. Shutemov
2023-01-31 8:32 ` Peter Zijlstra
2023-01-31 8:34 ` Peter Zijlstra
0 siblings, 2 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2023-01-30 13:53 UTC (permalink / raw)
To: Dave Hansen, Borislav Petkov, Andy Lutomirski
Cc: Kuppuswamy Sathyanarayanan, Thomas Gleixner, Elena Reshetova, x86,
linux-coco, linux-kernel, Kirill A. Shutemov, kernel test robot,
Peter Zijlstra
If compiled with CONFIG_FRAME_POINTER=y, objtool in not happy that
__tdx_hypercall() messes up RBP.
objtool: __tdx_hypercall+0x7f: return with modified stack frame
Rework the function to store TDX_HCALL_ flags on stack instead of RBP.
Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
Fixes: c30c4b2555ba ("x86/tdx: Refactor __tdx_hypercall() to allow pass down more arguments")
Link: https://lore.kernel.org/all/202301290255.buUBs99R-lkp@intel.com
Reported-by: kernel test robot <lkp@intel.com>
Cc: Peter Zijlstra <peterz@infradead.org>
---
The patch is against tip/x86/tdx. tip/sched/core removes
TDX_HCALL_ISSUE_STI. The trird hunk of the patch is not relevant
after that.
---
arch/x86/coco/tdx/tdcall.S | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/arch/x86/coco/tdx/tdcall.S b/arch/x86/coco/tdx/tdcall.S
index 5da06d1a9ba3..2bd436a4790d 100644
--- a/arch/x86/coco/tdx/tdcall.S
+++ b/arch/x86/coco/tdx/tdcall.S
@@ -131,11 +131,10 @@ SYM_FUNC_START(__tdx_hypercall)
push %r13
push %r12
push %rbx
- push %rbp
/* Free RDI and RSI to be used as TDVMCALL arguments */
movq %rdi, %rax
- movq %rsi, %rbp
+ push %rsi
/* Copy hypercall registers from arg struct: */
movq TDX_HYPERCALL_r8(%rax), %r8
@@ -168,7 +167,7 @@ SYM_FUNC_START(__tdx_hypercall)
* HLT operation indefinitely. Since this is the not the desired
* result, conditionally call STI before TDCALL.
*/
- testq $TDX_HCALL_ISSUE_STI, %rbp
+ testq $TDX_HCALL_ISSUE_STI, 8(%rsp)
jz .Lskip_sti
sti
.Lskip_sti:
@@ -188,7 +187,7 @@ SYM_FUNC_START(__tdx_hypercall)
pop %rax
/* Copy hypercall result registers to arg struct if needed */
- testq $TDX_HCALL_HAS_OUTPUT, %rbp
+ testq $TDX_HCALL_HAS_OUTPUT, (%rsp)
jz .Lout
movq %r8, TDX_HYPERCALL_r8(%rax)
@@ -218,11 +217,12 @@ SYM_FUNC_START(__tdx_hypercall)
xor %r10d, %r10d
xor %r11d, %r11d
xor %rdi, %rdi
- xor %rsi, %rsi
xor %rdx, %rdx
+ /* Remove TDX_HCALL_* flags from the stack */
+ pop %rsi
+
/* Restore callee-saved GPRs as mandated by the x86_64 ABI */
- pop %rbp
pop %rbx
pop %r12
pop %r13
--
2.39.1
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall()
2023-01-30 13:53 [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall() Kirill A. Shutemov
@ 2023-01-31 8:32 ` Peter Zijlstra
2023-01-31 8:50 ` Kirill A. Shutemov
2023-01-31 8:34 ` Peter Zijlstra
1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2023-01-31 8:32 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Dave Hansen, Borislav Petkov, Andy Lutomirski,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Elena Reshetova, x86,
linux-coco, linux-kernel, kernel test robot
On Mon, Jan 30, 2023 at 04:53:54PM +0300, Kirill A. Shutemov wrote:
> If compiled with CONFIG_FRAME_POINTER=y, objtool in not happy that
> __tdx_hypercall() messes up RBP.
>
> objtool: __tdx_hypercall+0x7f: return with modified stack frame
>
> Rework the function to store TDX_HCALL_ flags on stack instead of RBP.
>
> Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> Fixes: c30c4b2555ba ("x86/tdx: Refactor __tdx_hypercall() to allow pass down more arguments")
> Link: https://lore.kernel.org/all/202301290255.buUBs99R-lkp@intel.com
> Reported-by: kernel test robot <lkp@intel.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> ---
>
> The patch is against tip/x86/tdx. tip/sched/core removes
> TDX_HCALL_ISSUE_STI. The trird hunk of the patch is not relevant
> after that.
Right, this should work. But it does leave me wondering, should we
perhaps strive to completely remove the flags thing and move to
__tdx_hypercall() and __tdx_hypercall_ret() or something? That is,
simply have two different functions, one with and one without return
data.
It should be trivial to generate that without actual code duplication.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall()
2023-01-31 8:32 ` Peter Zijlstra
@ 2023-01-31 8:50 ` Kirill A. Shutemov
0 siblings, 0 replies; 6+ messages in thread
From: Kirill A. Shutemov @ 2023-01-31 8:50 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dave Hansen, Borislav Petkov, Andy Lutomirski,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Elena Reshetova, x86,
linux-coco, linux-kernel, kernel test robot
On Tue, Jan 31, 2023 at 09:32:37AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 30, 2023 at 04:53:54PM +0300, Kirill A. Shutemov wrote:
> > If compiled with CONFIG_FRAME_POINTER=y, objtool in not happy that
Oops, just noticed a typo. s/in/is/
> > __tdx_hypercall() messes up RBP.
> >
> > objtool: __tdx_hypercall+0x7f: return with modified stack frame
> >
> > Rework the function to store TDX_HCALL_ flags on stack instead of RBP.
> >
> > Signed-off-by: Kirill A. Shutemov <kirill.shutemov@linux.intel.com>
> > Fixes: c30c4b2555ba ("x86/tdx: Refactor __tdx_hypercall() to allow pass down more arguments")
> > Link: https://lore.kernel.org/all/202301290255.buUBs99R-lkp@intel.com
> > Reported-by: kernel test robot <lkp@intel.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>
> > ---
> >
> > The patch is against tip/x86/tdx. tip/sched/core removes
> > TDX_HCALL_ISSUE_STI. The trird hunk of the patch is not relevant
> > after that.
>
> Right, this should work. But it does leave me wondering, should we
> perhaps strive to completely remove the flags thing and move to
> __tdx_hypercall() and __tdx_hypercall_ret() or something? That is,
> simply have two different functions, one with and one without return
> data.
>
> It should be trivial to generate that without actual code duplication.
Yeah, that's doable. I will give it a try. I guess on top this one (plus
sched/core changes) should be.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall()
2023-01-30 13:53 [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall() Kirill A. Shutemov
2023-01-31 8:32 ` Peter Zijlstra
@ 2023-01-31 8:34 ` Peter Zijlstra
2023-01-31 8:39 ` Kirill A. Shutemov
1 sibling, 1 reply; 6+ messages in thread
From: Peter Zijlstra @ 2023-01-31 8:34 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Dave Hansen, Borislav Petkov, Andy Lutomirski,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Elena Reshetova, x86,
linux-coco, linux-kernel, kernel test robot
On Mon, Jan 30, 2023 at 04:53:54PM +0300, Kirill A. Shutemov wrote:
> If compiled with CONFIG_FRAME_POINTER=y, objtool in not happy that
> __tdx_hypercall() messes up RBP.
>
> objtool: __tdx_hypercall+0x7f: return with modified stack frame
>
> Rework the function to store TDX_HCALL_ flags on stack instead of RBP.
Also, on IRC you mentioned that per TDX spec, BP is a valid argument
register too and you were going to raise this and get it fixed, TDX
hypercalls must not use BP to pass data.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall()
2023-01-31 8:34 ` Peter Zijlstra
@ 2023-01-31 8:39 ` Kirill A. Shutemov
2023-01-31 9:39 ` Peter Zijlstra
0 siblings, 1 reply; 6+ messages in thread
From: Kirill A. Shutemov @ 2023-01-31 8:39 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dave Hansen, Borislav Petkov, Andy Lutomirski,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Elena Reshetova, x86,
linux-coco, linux-kernel, kernel test robot
On Tue, Jan 31, 2023 at 09:34:12AM +0100, Peter Zijlstra wrote:
> On Mon, Jan 30, 2023 at 04:53:54PM +0300, Kirill A. Shutemov wrote:
> > If compiled with CONFIG_FRAME_POINTER=y, objtool in not happy that
> > __tdx_hypercall() messes up RBP.
> >
> > objtool: __tdx_hypercall+0x7f: return with modified stack frame
> >
> > Rework the function to store TDX_HCALL_ flags on stack instead of RBP.
>
> Also, on IRC you mentioned that per TDX spec, BP is a valid argument
> register too and you were going to raise this and get it fixed, TDX
> hypercalls must not use BP to pass data.
I've raised the question yesterday. No progress so far. It will take time
to get it into the public spec.
--
Kiryl Shutsemau / Kirill A. Shutemov
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall()
2023-01-31 8:39 ` Kirill A. Shutemov
@ 2023-01-31 9:39 ` Peter Zijlstra
0 siblings, 0 replies; 6+ messages in thread
From: Peter Zijlstra @ 2023-01-31 9:39 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Dave Hansen, Borislav Petkov, Andy Lutomirski,
Kuppuswamy Sathyanarayanan, Thomas Gleixner, Elena Reshetova, x86,
linux-coco, linux-kernel, kernel test robot
On Tue, Jan 31, 2023 at 11:39:01AM +0300, Kirill A. Shutemov wrote:
> On Tue, Jan 31, 2023 at 09:34:12AM +0100, Peter Zijlstra wrote:
> > On Mon, Jan 30, 2023 at 04:53:54PM +0300, Kirill A. Shutemov wrote:
> > > If compiled with CONFIG_FRAME_POINTER=y, objtool in not happy that
> > > __tdx_hypercall() messes up RBP.
> > >
> > > objtool: __tdx_hypercall+0x7f: return with modified stack frame
> > >
> > > Rework the function to store TDX_HCALL_ flags on stack instead of RBP.
> >
> > Also, on IRC you mentioned that per TDX spec, BP is a valid argument
> > register too and you were going to raise this and get it fixed, TDX
> > hypercalls must not use BP to pass data.
>
> I've raised the question yesterday. No progress so far. It will take time
> to get it into the public spec.
Sure, just making sure it's not forgotten about. Thanks!
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-01-31 9:40 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-01-30 13:53 [PATCH] x86/tdx: Do not corrupt frame-pointer in __tdx_hypercall() Kirill A. Shutemov
2023-01-31 8:32 ` Peter Zijlstra
2023-01-31 8:50 ` Kirill A. Shutemov
2023-01-31 8:34 ` Peter Zijlstra
2023-01-31 8:39 ` Kirill A. Shutemov
2023-01-31 9:39 ` Peter Zijlstra
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox