Re: [PATCH v2 04/13] RISC-V: Move riscv_insn_is_* macros into a common header

[Conor Dooley <conor@kernel.org>]

On Tue, Nov 29, 2022 at 11:09:36PM +0000, Conor Dooley wrote:
> On Mon, Nov 28, 2022 at 11:26:23AM +0100, Heiko Stuebner wrote:
> > From: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > 
> > Right now the riscv kernel has (at least) two independent sets
> > of functions to check if an encoded instruction is of a specific
> > type. One in kgdb and one kprobes simulate-insn code.
> > 
> > More parts of the kernel will probably need this in the future,
> > so instead of allowing this duplication to go on further,
> > move macros that do the function declaration in a common header,
> > similar to at least aarch64.
> > 
> > Signed-off-by: Heiko Stuebner <heiko.stuebner@vrull.eu>
> > ---
> >  arch/riscv/include/asm/parse_asm.h       | 41 ++++++++++++++++----
> >  arch/riscv/kernel/kgdb.c                 | 49 ++++++++----------------
> >  arch/riscv/kernel/probes/simulate-insn.h | 26 +++----------
> >  3 files changed, 54 insertions(+), 62 deletions(-)
> 
> > diff --git a/arch/riscv/kernel/probes/simulate-insn.h b/arch/riscv/kernel/probes/simulate-insn.h
> > index cb6ff7dccb92..29fb16cd335c 100644
> > --- a/arch/riscv/kernel/probes/simulate-insn.h
> > +++ b/arch/riscv/kernel/probes/simulate-insn.h
> > @@ -3,14 +3,7 @@
> >  #ifndef _RISCV_KERNEL_PROBES_SIMULATE_INSN_H
> >  #define _RISCV_KERNEL_PROBES_SIMULATE_INSN_H
> >  
> > -#define __RISCV_INSN_FUNCS(name, mask, val)				\
> > -static __always_inline bool riscv_insn_is_##name(probe_opcode_t code)	\
> > -{									\
> > -	BUILD_BUG_ON(~(mask) & (val));					\
> > -	return (code & (mask)) == (val);				\
> > -}									\
> > -bool simulate_##name(u32 opcode, unsigned long addr,			\
> > -		     struct pt_regs *regs)
> > +#include <asm/parse_asm.h>
> >  
> >  #define RISCV_INSN_REJECTED(name, code)					\
> >  	do {								\
> > @@ -30,18 +23,9 @@ __RISCV_INSN_FUNCS(fence,	0x7f, 0x0f);
> >  		}							\
> >  	} while (0)
> >  
> > -__RISCV_INSN_FUNCS(c_j,		0xe003, 0xa001);
> > -__RISCV_INSN_FUNCS(c_jr,	0xf007, 0x8002);
> > -__RISCV_INSN_FUNCS(c_jal,	0xe003, 0x2001);
> > -__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);
> > -
> > -__RISCV_INSN_FUNCS(auipc,	0x7f, 0x17);
> > -__RISCV_INSN_FUNCS(branch,	0x7f, 0x63);
> > -
> > -__RISCV_INSN_FUNCS(jal,		0x7f, 0x6f);
> > -__RISCV_INSN_FUNCS(jalr,	0x707f, 0x67);
> > +bool simulate_auipc(u32 opcode, unsigned long addr, struct pt_regs *regs);
> > +bool simulate_branch(u32 opcode, unsigned long addr, struct pt_regs *regs);
> > +bool simulate_jal(u32 opcode, unsigned long addr, struct pt_regs *regs);
> > +bool simulate_jalr(u32 opcode, unsigned long addr, struct pt_regs *regs);
> 
> I assume the other ones didn't actually have a user and so didn't need a
> function created? Code movement here looks fine. Dropping the double
> definitions is always nice :)
> 
> Reviewed-by: Conor Dooley <conor.dooley@microchip.com>

I should've waited for allmodconfig on this particular commit to finish:
/stuff/linux/arch/riscv/kernel/probes/decode-insn.c:43:2: error: call to undeclared function 'riscv_insn_is_auipc'; ISO C99 and later do not support implicit function declarations [-Werror,-Wimplicit-function-declaration]
        RISCV_INSN_SET_SIMULATE(auipc,          insn);
        ^
/stuff/linux/arch/riscv/kernel/probes/simulate-insn.h:20:7: note: expanded from macro 'RISCV_INSN_SET_SIMULATE'
                if (riscv_insn_is_##name(code)) {                       \
                    ^
<scratch space>:62:1: note: expanded from here
riscv_insn_is_auipc
^
1 error generated.


_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv