| 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
| ||
|
Message-ID: <Z9r92tCdsjoJTQ88@ghost>
Date: Wed, 19 Mar 2025 10:24:42 -0700
From: Charlie Jenkins <charlie@...osinc.com>
To: Andrew Jones <ajones@...tanamicro.com>
Cc: Paul Walmsley <paul.walmsley@...ive.com>,
Palmer Dabbelt <palmer@...belt.com>,
Ard Biesheuvel <ardb@...nel.org>,
Ben Dooks <ben.dooks@...ethink.co.uk>,
Pasha Bouzarjomehri <pasha@...osinc.com>,
Emil Renner Berthing <emil.renner.berthing@...onical.com>,
Alexandre Ghiti <alexghiti@...osinc.com>,
Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Mark Rutland <mark.rutland@....com>,
Albert Ou <aou@...s.berkeley.edu>,
Peter Zijlstra <peterz@...radead.org>,
Josh Poimboeuf <jpoimboe@...nel.org>,
Jason Baron <jbaron@...mai.com>, linux-riscv@...ts.infradead.org,
linux-kernel@...r.kernel.org, linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCH v9 1/2] riscv: Move nop definition to insn-def.h
On Wed, Mar 19, 2025 at 04:27:39PM +0100, Andrew Jones wrote:
> On Tue, Mar 18, 2025 at 05:38:45PM -0700, Charlie Jenkins wrote:
> > We have duplicated the definition of the nop instruction in ftrace.h and
> > in jump_label.c. Move this definition into the generic file insn-def.h
> > so that they can share the definition with each other and with future
> > files.
> >
> > Signed-off-by: Charlie Jenkins <charlie@...osinc.com>
> > ---
> > arch/riscv/include/asm/ftrace.h | 1 -
> > arch/riscv/include/asm/insn-def.h | 2 ++
> > arch/riscv/kernel/ftrace.c | 6 +++---
> > arch/riscv/kernel/jump_label.c | 4 ++--
> > 4 files changed, 7 insertions(+), 6 deletions(-)
> >
> > diff --git a/arch/riscv/include/asm/ftrace.h b/arch/riscv/include/asm/ftrace.h
> > index c4721ce44ca474654b37b3d51bc0a63d46bc1eff..b7f361a50f6445d02a0d88eef5547ee27c1fb52e 100644
> > --- a/arch/riscv/include/asm/ftrace.h
> > +++ b/arch/riscv/include/asm/ftrace.h
> > @@ -79,7 +79,6 @@ struct dyn_arch_ftrace {
> > #define AUIPC_RA (0x00000097)
> > #define JALR_T0 (0x000282e7)
> > #define AUIPC_T0 (0x00000297)
> > -#define NOP4 (0x00000013)
> >
> > #define to_jalr_t0(offset) \
> > (((offset & JALR_OFFSET_MASK) << JALR_SHIFT) | JALR_T0)
> > diff --git a/arch/riscv/include/asm/insn-def.h b/arch/riscv/include/asm/insn-def.h
> > index 9a913010cdd93cdfdd93f467e7880e20cce0dd2b..0a1fc5134f29da190554c59f8cee3b5374c9aa2d 100644
> > --- a/arch/riscv/include/asm/insn-def.h
> > +++ b/arch/riscv/include/asm/insn-def.h
> > @@ -200,4 +200,6 @@
> > #define ZAWRS_WRS_NTO ".4byte 0x00d00073"
> > #define ZAWRS_WRS_STO ".4byte 0x01d00073"
> >
> > +#define RISCV_INSN_NOP4 0x00000013U
>
> This should be _AC(0x00000013, U), but since this is the first of its kind
> (all other defines are of the form ".4byte ..." -- either explicitly, like
> the ones above, or through the INSN_* macros), then it feels like it
> either doesn't belong here at all or that we should provide it and a
> ".4byte ..." version.
Sure, I don't have a strong opinion about this. I was debating adding
the .4byte version also but decided against it because it isn't being
used yet, I will add it.
- Charlie
>
> Thanks,
> drew
>
> > +
> > #endif /* __ASM_INSN_DEF_H */
> > diff --git a/arch/riscv/kernel/ftrace.c b/arch/riscv/kernel/ftrace.c
> > index 3524db5e4fa014a4594465f849d898a030bfb7b8..674dcdfae7a149c339f1e791adb450535f22991b 100644
> > --- a/arch/riscv/kernel/ftrace.c
> > +++ b/arch/riscv/kernel/ftrace.c
> > @@ -36,7 +36,7 @@ static int ftrace_check_current_call(unsigned long hook_pos,
> > unsigned int *expected)
> > {
> > unsigned int replaced[2];
> > - unsigned int nops[2] = {NOP4, NOP4};
> > + unsigned int nops[2] = {RISCV_INSN_NOP4, RISCV_INSN_NOP4};
> >
> > /* we expect nops at the hook position */
> > if (!expected)
> > @@ -68,7 +68,7 @@ static int __ftrace_modify_call(unsigned long hook_pos, unsigned long target,
> > bool enable, bool ra)
> > {
> > unsigned int call[2];
> > - unsigned int nops[2] = {NOP4, NOP4};
> > + unsigned int nops[2] = {RISCV_INSN_NOP4, RISCV_INSN_NOP4};
> >
> > if (ra)
> > make_call_ra(hook_pos, target, call);
> > @@ -97,7 +97,7 @@ int ftrace_make_call(struct dyn_ftrace *rec, unsigned long addr)
> > int ftrace_make_nop(struct module *mod, struct dyn_ftrace *rec,
> > unsigned long addr)
> > {
> > - unsigned int nops[2] = {NOP4, NOP4};
> > + unsigned int nops[2] = {RISCV_INSN_NOP4, RISCV_INSN_NOP4};
> >
> > if (patch_insn_write((void *)rec->ip, nops, MCOUNT_INSN_SIZE))
> > return -EPERM;
> > diff --git a/arch/riscv/kernel/jump_label.c b/arch/riscv/kernel/jump_label.c
> > index 654ed159c830b3d5e34ac58bf367107066eb73a1..b4c1a6a3fbd28533552036194f27ed206bea305d 100644
> > --- a/arch/riscv/kernel/jump_label.c
> > +++ b/arch/riscv/kernel/jump_label.c
> > @@ -11,8 +11,8 @@
> > #include <asm/bug.h>
> > #include <asm/cacheflush.h>
> > #include <asm/text-patching.h>
> > +#include <asm/insn-def.h>
> >
> > -#define RISCV_INSN_NOP 0x00000013U
> > #define RISCV_INSN_JAL 0x0000006fU
> >
> > bool arch_jump_label_transform_queue(struct jump_entry *entry,
> > @@ -33,7 +33,7 @@ bool arch_jump_label_transform_queue(struct jump_entry *entry,
> > (((u32)offset & GENMASK(10, 1)) << (21 - 1)) |
> > (((u32)offset & GENMASK(20, 20)) << (31 - 20));
> > } else {
> > - insn = RISCV_INSN_NOP;
> > + insn = RISCV_INSN_NOP4;
> > }
> >
> > if (early_boot_irqs_disabled) {
> >
> > --
> > 2.43.0
> >
> >
> > _______________________________________________
> > linux-riscv mailing list
> > linux-riscv@...ts.infradead.org
> > http://lists.infradead.org/mailman/listinfo/linux-riscv
Powered by blists - more mailing lists