On 05/23/2019 12:36 PM, Will Deacon wrote: > [+Daniel and Jean-Philippe] > > On Thu, May 23, 2019 at 05:12:00PM +0900, Yoshihiro Shimoda wrote: >> The following build warning happens on gcc 8.1.0. >> >> linux/arch/arm64/include/asm/insn.h: In function 'aarch64_insn_is_ldadd': >> linux/arch/arm64/include/asm/insn.h:280:257: warning: bitwise comparison >> always evaluates to false [-Wtautological-compare] >> __AARCH64_INSN_FUNCS(ldadd, 0x3F20FC00, 0xB8200000) >> >> Since the second argument is mask value and compare with the third >> argument value, the bit 31 is always masked and then this macro is >> always false. So, this patch fixes the issue. >> >> Reported-by: Kuninori Morimoto <[email protected]> >> Fixes: 34b8ab091f9ef57a ("bpf, arm64: use more scalable stadd over ldxr / >> stxr loop in xadd") >> Tested-by: Kuninori Morimoto <[email protected]> >> Signed-off-by: Yoshihiro Shimoda <[email protected]> >> --- >> I'm not sure the second argument "0xBF20FC00" is OK or not (we can set >> to 0xFF20FC00 instead). So, I marked RFC on this patch. >> >> arch/arm64/include/asm/insn.h | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/arch/arm64/include/asm/insn.h b/arch/arm64/include/asm/insn.h >> index ec894de..c9e3cdc 100644 >> --- a/arch/arm64/include/asm/insn.h >> +++ b/arch/arm64/include/asm/insn.h >> @@ -277,7 +277,7 @@ __AARCH64_INSN_FUNCS(adrp, 0x9F000000, 0x90000000) >> __AARCH64_INSN_FUNCS(prfm, 0x3FC00000, 0x39800000) >> __AARCH64_INSN_FUNCS(prfm_lit, 0xFF000000, 0xD8000000) >> __AARCH64_INSN_FUNCS(str_reg, 0x3FE0EC00, 0x38206800) >> -__AARCH64_INSN_FUNCS(ldadd, 0x3F20FC00, 0xB8200000) >> +__AARCH64_INSN_FUNCS(ldadd, 0xBF20FC00, 0xB8200000) > > Looking at the ISA encoding, I think that top digit should indeed be 'B', > but I haven't checked the rest of the instruction. > > However, I'm fairly sure we tested this so now I'm a bit worried that I'm > missing something :/
Hmm, good catch, the mask aka aarch64_insn_is_ldadd() is not used anywhere in the tree, just the aarch64_insn_get_ldadd_value(). Latter was runtime tested via BPF JIT as well as through disassembler that it emits ldadd. I initially had a different mask value than Jean-Philippe, but that was probably due to confusion on my side. In any case, value should be correct though. Thanks, Daniel
