Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type

[Christophe Leroy]

Le 06/04/2020 à 11:52, Alistair Popple a écrit :

[...]

@@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(struct
ppc_inst x) return ppc_inst(swab32(ppc_inst_val(x)));
  }

+static inline u32 ppc_inst_val(struct ppc_inst x)
+{
+       return x.val;
+}
+
  static inline struct ppc_inst ppc_inst_read(const struct ppc_inst *ptr)
  {
        return *ptr;
  }

+static inline void ppc_inst_write(struct ppc_inst *ptr, struct ppc_inst x)
+{
+       *ptr = x;
+}
+
+#endif /* __powerpc64__ */
+
  static inline bool ppc_inst_equal(struct ppc_inst x, struct ppc_inst y)
  {
        return !memcmp(&x, &y, sizeof(struct ppc_inst));
  }

Apologies for not picking this up earlier, I was hoping to get to the bottom
of the issue I was seeing before you sent out v5. However the above definition
of instruction equality does not seem correct because it does not consider the
case when an instruction is not prefixed - a non-prefixed instruction should be
considered equal if the first 32-bit opcode/value is the same. Something like:

        if (ppc_inst_prefixed(x) != ppc_inst_prefixed(y))
                return false;
        else if (ppc_inst_prefixed(x))
                return !memcmp(&x, &y, sizeof(struct ppc_inst));

Are we sure memcmp() is a good candidate for the comparison ? Can we do 
simpler ? Especially, I understood a prefixed instruction is a 64 bits 
properly aligned instruction, can we do a simple u64 compare ? Or is GCC 
intelligent enough to do that without calling memcmp() function which is 
heavy ?

        else
                return x.val == y.val;

This was causing failures in ftrace_modify_code() as it would falsely detect
two non-prefixed instructions as being not equal due to differences in the 
suffix.
  

Christophe