[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