Re: [RFC PATCH v3 2/4] objtool: x86 instruction decoder and big endian cross compiles

2020-10-04 Thread Masami Hiramatsu
On Fri, 2 Oct 2020 10:18:41 -0500
Josh Poimboeuf  wrote:

> On Thu, Oct 01, 2020 at 12:17:25AM +0200, Vasily Gorbik wrote:
> > From: Martin Schwidefsky 
> > 
> > Currently objtool seems to be the only tool from build tools needed
> > which breaks x86 cross compilation on big endian systems. Make the x86
> > instruction decoder of the objtool usable on big endian machines.
> > 
> > Signed-off-by: Martin Schwidefsky 
> > Co-developed-by: Vasily Gorbik 
> > Signed-off-by: Vasily Gorbik 
> 
> Since this changes the decoder (which is shared with the kernel), please
> prefix the subject with "x86/insn:" instead of "objtool".

Thanks for pointing it.

> 
> This patch is a bit ugly, but I don't necessarily have a better idea.
> 
> Masami?

Yeah, agreed. Maybe we can split that part in different header, but
that makes it more ugly. And code itself looks good to me.
(I like insn_field_set() idea :) )

Acked-by: Masami Hiramatsu 

Thank you!


> -- 
> Josh
> 


-- 
Masami Hiramatsu 


Re: [RFC PATCH v3 2/4] objtool: x86 instruction decoder and big endian cross compiles

2020-10-02 Thread Josh Poimboeuf
On Thu, Oct 01, 2020 at 12:17:25AM +0200, Vasily Gorbik wrote:
> From: Martin Schwidefsky 
> 
> Currently objtool seems to be the only tool from build tools needed
> which breaks x86 cross compilation on big endian systems. Make the x86
> instruction decoder of the objtool usable on big endian machines.
> 
> Signed-off-by: Martin Schwidefsky 
> Co-developed-by: Vasily Gorbik 
> Signed-off-by: Vasily Gorbik 

Since this changes the decoder (which is shared with the kernel), please
prefix the subject with "x86/insn:" instead of "objtool".

This patch is a bit ugly, but I don't necessarily have a better idea.

Masami?

-- 
Josh



[RFC PATCH v3 2/4] objtool: x86 instruction decoder and big endian cross compiles

2020-09-30 Thread Vasily Gorbik
From: Martin Schwidefsky 

Currently objtool seems to be the only tool from build tools needed
which breaks x86 cross compilation on big endian systems. Make the x86
instruction decoder of the objtool usable on big endian machines.

Signed-off-by: Martin Schwidefsky 
Co-developed-by: Vasily Gorbik 
Signed-off-by: Vasily Gorbik 
---
 arch/x86/include/asm/insn.h   |  35 ++
 arch/x86/lib/insn.c   | 108 +++---
 tools/arch/x86/include/asm/insn.h |  35 ++
 tools/arch/x86/lib/insn.c | 108 +++---
 4 files changed, 178 insertions(+), 108 deletions(-)

diff --git a/arch/x86/include/asm/insn.h b/arch/x86/include/asm/insn.h
index 5c1ae3eff9d4..bff79b22da54 100644
--- a/arch/x86/include/asm/insn.h
+++ b/arch/x86/include/asm/insn.h
@@ -7,9 +7,12 @@
  * Copyright (C) IBM Corporation, 2009
  */
 
+#include 
 /* insn_attr_t is defined in inat.h */
 #include 
 
+#if defined(__BYTE_ORDER) ? __BYTE_ORDER == __LITTLE_ENDIAN : 
defined(__LITTLE_ENDIAN)
+
 struct insn_field {
union {
insn_value_t value;
@@ -20,6 +23,38 @@ struct insn_field {
unsigned char nbytes;
 };
 
+static inline void insn_field_set(struct insn_field *p, insn_value_t v,
+ unsigned char n)
+{
+   p->value = v;
+   p->nbytes = n;
+}
+
+#else
+
+struct insn_field {
+   insn_value_t value;
+   union {
+   insn_value_t little;
+   insn_byte_t bytes[4];
+   };
+   /* !0 if we've run insn_get_xxx() for this field */
+   unsigned char got;
+   unsigned char nbytes;
+};
+
+static inline void insn_field_set(struct insn_field *p, insn_value_t v,
+ unsigned char n)
+{
+   p->value = v;
+#ifndef __KERNEL__
+   p->little = __bswap_32(v);
+#endif
+   p->nbytes = n;
+}
+
+#endif
+
 struct insn {
struct insn_field prefixes; /*
 * Prefixes
diff --git a/arch/x86/lib/insn.c b/arch/x86/lib/insn.c
index 404279563891..bbd4a5f15d83 100644
--- a/arch/x86/lib/insn.c
+++ b/arch/x86/lib/insn.c
@@ -5,6 +5,7 @@
  * Copyright (C) IBM Corporation, 2002, 2004, 2009
  */
 
+#include 
 #ifdef __KERNEL__
 #include 
 #else
@@ -15,15 +16,35 @@
 
 #include 
 
+#define leXX_to_cpu(t, r)  \
+({ \
+   __typeof__(t) v;\
+   switch (sizeof(t)) {\
+   case 4: \
+   v = le32_to_cpu(r); \
+   break;  \
+   case 2: \
+   v = le16_to_cpu(r); \
+   break;  \
+   case 1: \
+   v = r;  \
+   break;  \
+   default:\
+   BUILD_BUG();\
+   break;  \
+   }   \
+   v;  \
+})
+
 /* Verify next sizeof(t) bytes can be on the same instruction */
 #define validate_next(t, insn, n)  \
((insn)->next_byte + sizeof(t) + n <= (insn)->end_kaddr)
 
 #define __get_next(t, insn)\
-   ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); r; })
+   ({ t r = *(t*)insn->next_byte; insn->next_byte += sizeof(t); 
leXX_to_cpu(t, r); })
 
 #define __peek_nbyte_next(t, insn, n)  \
-   ({ t r = *(t*)((insn)->next_byte + n); r; })
+   ({ t r = *(t*)((insn)->next_byte + n); leXX_to_cpu(t, r); })
 
 #define get_next(t, insn)  \
({ if (unlikely(!validate_next(t, insn, 0))) goto err_out; 
__get_next(t, insn); })
@@ -157,8 +178,7 @@ void insn_get_prefixes(struct insn *insn)
b = peek_next(insn_byte_t, insn);
attr = inat_get_opcode_attribute(b);
if (inat_is_rex_prefix(attr)) {
-   insn->rex_prefix.value = b;
-   insn->rex_prefix.nbytes = 1;
+   insn_field_set(>rex_prefix, b, 1);
insn->next_byte++;
if (X86_REX_W(b))
/* REX.W overrides opnd_size */
@@ -295,8 +315,7 @@ void insn_get_modrm(struct insn *insn)
 
if (inat_has_modrm(insn->attr)) {
mod = get_next(insn_byte_t, insn);
-