Re: [PATCH v5 4/9] riscv/kprobe: Add common RVI and RVC instruction decoder code

[liaochang (A)]

在 2023/1/3 2:03, Björn Töpel 写道:
> Chen Guokai <chenguokai17@xxxxxxxxxxxxxxxx> writes:
> 
>> From: Liao Chang <liaochang1@xxxxxxxxxx>
> 
>> diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
>> index cb6ff7dccb92..74d8c1ba9064 100644
>> --- a/arch/riscv/kernel/probes/simulate-insn.h
>> +++ b/arch/riscv/kernel/probes/simulate-insn.h
>> @@ -37,6 +37,40 @@ __RISCV_INSN_FUNCS(c_jalr,	0xf007, 0x9002);
>>  __RISCV_INSN_FUNCS(c_beqz,	0xe003, 0xc001);
>>  __RISCV_INSN_FUNCS(c_bnez,	0xe003, 0xe001);
>>  __RISCV_INSN_FUNCS(c_ebreak,	0xffff, 0x9002);
>> +/* RVC(S) instructions contain rs1 and rs2 */
>> +__RISCV_INSN_FUNCS(c_sq,	0xe003, 0xa000);
>> +__RISCV_INSN_FUNCS(c_sw,	0xe003, 0xc000);
>> +__RISCV_INSN_FUNCS(c_sd,	0xe003, 0xe000);
>> +/* RVC(A) instructions contain rs1 and rs2 */
>> +__RISCV_INSN_FUNCS(c_sub,	0xfc03, 0x8c01);
> 
> Incorrect mask.

Thanks for checking, i study the opcode of C_SUB [1], the correct mask should be 0xFC63.

      15 14 13 12 | 11 10  9  8 | 7 6 5 4 | 3 2  1 0
c.sub:       1  0  0  0 |  1  1  rs1'/rd' 0 0   rs2'   0 1
mask:           F |           C |       6 |        3
value:          8 |           C |       0 |        1

> 
>> +__RISCV_INSN_FUNCS(c_subw,	0xfc43, 0x9c01);
>> +/* RVC(L) instructions contain rs1 */
>> +__RISCV_INSN_FUNCS(c_lq,	0xe003, 0x2000);
>> +__RISCV_INSN_FUNCS(c_lw,	0xe003, 0x4000);
>> +__RISCV_INSN_FUNCS(c_ld,	0xe003, 0x6000);
>> +/* RVC(I) instructions contain rs1 */
>> +__RISCV_INSN_FUNCS(c_addi,	0xe003, 0x0001);
>> +__RISCV_INSN_FUNCS(c_addiw,	0xe003, 0x2001);
>> +__RISCV_INSN_FUNCS(c_addi16sp,	0xe183, 0x6101);
>> +__RISCV_INSN_FUNCS(c_slli,	0xe003, 0x0002);
>> +/* RVC(B) instructions contain rs1 */
>> +__RISCV_INSN_FUNCS(c_sri,	0xe803, 0x8001);
>> +__RISCV_INSN_FUNCS(c_andi,	0xec03, 0x8801);
>> +/* RVC(SS) instructions contain rs2 */
>> +__RISCV_INSN_FUNCS(c_sqsp,	0xe003, 0xa002);
>> +__RISCV_INSN_FUNCS(c_swsp,	0xe003, 0xc002);
>> +__RISCV_INSN_FUNCS(c_sdsp,	0xe003, 0xe002);
>> +/* RVC(R) instructions contain rs2 and rd */
>> +__RISCV_INSN_FUNCS(c_mv,	0xe003, 0x8002);
> 
> Shouldn't the mask be 0xf003?

Actually, the mask should be 0xf003 indeedly, but it also bring another problem that
it can't tell C.MV and C.JR via the mask and value parts. Look opcodes below:

      15 14 13 12 | 11 10  9  8 | 7 6 5 4 | 3 2  1 0
C.JR:  1  0  0  0 |             rs1           0  1 0
C.MV:  1  0  0  0 |              rd         rs2  1 0

The only differece between C.MV and C.JR is the bits[2~6], these bitfield of C.JR is zero,
the ones of C.MV is rs2 which never be zero. In order to tell C.MV and C.JR correclty, it
is better to adjust the mask of C.JR to be 0xf07f as your patch(riscv, kprobe: Stricter c.jr/c.jalr decoding)

Looking forward to your feedback.

> 
> 
> Björn


[1] https://github.com/riscv/riscv-isa-manual/releases

-- 
BR,
Liao, Chang