2023-06-02 15:21:06

by Josh Poimboeuf

Subject: [PATCH] drm/vmwgfx: Silence RBP clobber warnings

VMware hypercalls take the RBP register as input. This breaks basic
frame pointer convention, as RBP should never be clobbered.

So frame pointer unwinding is broken for the instructions surrounding
the hypercall with the clobbered RBP. There's nothing that can be done
about that. Just tell objtool to ignore it.

Silences the following warnings:

vmlinux.o: warning: objtool: vmw_port_hb_in+0x1df: return with modified stack frame
vmlinux.o: warning: objtool: vmw_port_hb_out+0x1dd: return with modified stack frame

Reported-by: kernel test robot <[email protected]>
Link: https://lore.kernel.org/oe-kbuild-all/[email protected]/
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/include/asm/unwind_hints.h | 6 ++++++
drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h | 14 ++++++++++----
2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/arch/x86/include/asm/unwind_hints.h b/arch/x86/include/asm/unwind_hints.h
index 01cb9692b160..91ab6e5426c6 100644
--- a/arch/x86/include/asm/unwind_hints.h
+++ b/arch/x86/include/asm/unwind_hints.h
@@ -79,6 +79,12 @@
#define UNWIND_HINT_FUNC \
UNWIND_HINT(UNWIND_HINT_TYPE_FUNC, ORC_REG_SP, 8, 0)

+#define UNWIND_HINT_SAVE \
+ UNWIND_HINT(UNWIND_HINT_TYPE_SAVE, 0, 0, 0)
+
+#define UNWIND_HINT_RESTORE \
+ UNWIND_HINT(UNWIND_HINT_TYPE_RESTORE, 0, 0, 0)
+
#endif /* __ASSEMBLY__ */

#endif /* _ASM_X86_UNWIND_HINTS_H */
diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
index 0b74ca2dfb7b..79050a78fa4c 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg_x86.h
@@ -105,10 +105,13 @@
flags, magic, bp, \
eax, ebx, ecx, edx, si, di) \
({ \
- asm volatile ("push %%rbp;" \
+ asm volatile ( \
+ UNWIND_HINT_SAVE \
+ "push %%rbp;" \
"mov %12, %%rbp;" \
VMWARE_HYPERCALL_HB_OUT \
- "pop %%rbp;" : \
+ "pop %%rbp;" \
+ UNWIND_HINT_RESTORE : \
"=a"(eax), \
"=b"(ebx), \
"=c"(ecx), \
@@ -130,10 +133,13 @@
flags, magic, bp, \
eax, ebx, ecx, edx, si, di) \
({ \
- asm volatile ("push %%rbp;" \
+ asm volatile ( \
+ UNWIND_HINT_SAVE \
+ "push %%rbp;" \
"mov %12, %%rbp;" \
VMWARE_HYPERCALL_HB_IN \
- "pop %%rbp" : \
+ "pop %%rbp;" \
+ UNWIND_HINT_RESTORE : \
"=a"(eax), \
"=b"(ebx), \
"=c"(ecx), \
--
2.40.1

2023-06-02 15:36:46

by Peter Zijlstra

Subject: Re: [PATCH] drm/vmwgfx: Silence RBP clobber warnings

On Fri, Jun 02, 2023 at 07:56:34AM -0700, Josh Poimboeuf wrote:
> VMware hypercalls take the RBP register as input. This breaks basic
> frame pointer convention, as RBP should never be clobbered.
>
> So frame pointer unwinding is broken for the instructions surrounding
> the hypercall with the clobbered RBP. There's nothing that can be done
> about that. Just tell objtool to ignore it.
>

That's a pretty horrific ABI, one that violates the oldest x86 calling
convention in existence.

VMware folks, shame!!

2023-06-02 15:47:21

by Josh Poimboeuf

Subject: Re: [PATCH] drm/vmwgfx: Silence RBP clobber warnings

On Fri, Jun 02, 2023 at 05:16:39PM +0200, Peter Zijlstra wrote:
> On Fri, Jun 02, 2023 at 07:56:34AM -0700, Josh Poimboeuf wrote:
> > VMware hypercalls take the RBP register as input. This breaks basic
> > frame pointer convention, as RBP should never be clobbered.
> >
> > So frame pointer unwinding is broken for the instructions surrounding
> > the hypercall with the clobbered RBP. There's nothing that can be done
> > about that. Just tell objtool to ignore it.
> >
>
> That's a pretty horrific ABI, one that violates the oldest x86 calling
> convention in existence.
>
> VMware folks, shame!!

Agreed :-(

BTW, please ignore the patch, I'll be sending a v2.

I realized (with Peter's prodding on IRC) that reliable ORC unwinding
would be broken if the function has a frame pointer. Which can
happen if the function needs an aligned stack.

--
Josh