linux-kernel - Re: [PATCH] LoongArch: Correct the definition of is_branch

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <5e051682-c4fa-64d7-8567-149b5bd7d403@loongson.cn>
Date:   Sat, 17 Dec 2022 09:49:09 +0800
From:   Jinyang He <hejinyang@...ngson.cn>
To:     Tiezhu Yang <yangtiezhu@...ngson.cn>,
        Huacai Chen <chenhuacai@...nel.org>,
        WANG Xuerui <kernel@...0n.name>
Cc:     loongarch@...ts.linux.dev, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] LoongArch: Correct the definition of is_branch_ins()


On 2022-12-16 14:11, Tiezhu Yang wrote:
>
>
> On 12/16/2022 11:18 AM, Jinyang He wrote:
>> Hi, Tiezhu,
>>
>>
>> On 2022-12-14 16:30, Tiezhu Yang wrote:
>>> The current definition of is_branch_ins() is not correct,
>>
>> But the branch instruction opcode only use the high 6 bits,
>
> Yes, I noticed that, the logic result of current code is right,
> but it seems a little strange (only consider reg1i21_format)
> at the first glance, the initial aim of this patch is to make
> it theoretically correct, maybe it is not the best change.
>
> I think we can neglect the instruction formats and check the
> high 6 bits instead, what do you think of the following change?

We defined many instruction format because of variable-width opcode 
field and parameter field. IMHO if there is no way to solve that problem 
really, keeping original codes is better. That depends on the 
maintainers, of course.


Thanks,

Jinyang


>
> diff --git a/arch/loongarch/include/asm/inst.h 
> b/arch/loongarch/include/asm/inst.h
> index c00e151..fd31752 100644
> --- a/arch/loongarch/include/asm/inst.h
> +++ b/arch/loongarch/include/asm/inst.h
> @@ -329,8 +329,8 @@ static inline bool is_pc_ins(union 
> loongarch_instruction *ip)
>
>  static inline bool is_branch_ins(union loongarch_instruction *ip)
>  {
> -       return ip->reg1i21_format.opcode >= beqz_op &&
> -               ip->reg1i21_format.opcode <= bgeu_op;
> +       return ((ip->word >> 26) & 0x3f) >= beqz_op &&
> +               ((ip->word >> 26) & 0x3f) <= bgeu_op;
>  }
>
> Thanks,
> Tiezhu
>
>
>