Re: [PATCH v5 18/21] powerpc64: Add prefixed instructions to instruction data type
From: Christophe Leroy <hidden>
Date: 2020-04-06 10:27:27
Le 06/04/2020 à 11:52, Alistair Popple a écrit : [...]
quoted
@@ -32,14 +76,31 @@ static inline struct ppc_inst ppc_inst_swab(structppc_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