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

From: "Heiko Stübner" <heiko@sntech.de>
To: Conor Dooley <conor@kernel.org>
Cc: linux-riscv@lists.infradead.org, palmer@dabbelt.com,
	christoph.muellner@vrull.eu, prabhakar.csengg@gmail.com,
	philipp.tomsich@vrull.eu, ajones@ventanamicro.com,
	emil.renner.berthing@canonical.com
Subject: Re: [PATCH v2 04/13] RISC-V: Move riscv_insn_is_* macros into a common header
Date: Wed, 30 Nov 2022 15:53:13 +0100	[thread overview]
Message-ID: <9886285.T7Z3S40VBb@diego> (raw)
In-Reply-To: <Y4aRK9CvoH56A+Ai@spud>

Am Mittwoch, 30. November 2022, 00:09:31 CET schrieb Conor Dooley:
> 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?

Yep, simulate-insn.c only actually defined these functions, so the original
macro just defined empty prototypes for the rest.

Heiko

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