Messages in this thread | | | From | Ard Biesheuvel <> | Date | Thu, 6 Oct 2022 11:08:32 +0200 | Subject | Re: [PATCH 1/2] arm64: jump_label: mark arguments as const to satisfy asm constraints |
| |
On Thu, 6 Oct 2022 at 10:55, Jisheng Zhang <jszhang@kernel.org> wrote: > > On Thu, Oct 06, 2022 at 10:17:44AM +0200, Ard Biesheuvel wrote: > > On Thu, 6 Oct 2022 at 10:05, Jisheng Zhang <jszhang@kernel.org> wrote: > > > > > > Inspired by x86 commit 864b435514b2("x86/jump_label: Mark arguments as > > > const to satisfy asm constraints"), mark arch_static_branch()'s and > > > arch_static_branch_jump()'s arguments as const to satisfy asm > > > constraints. And Steven in [1] also pointed out that "The "i" > > > constraint needs to be a constant." > > > > > > Tested with building a simple external kernel module with "O0". > > > > > > [1]https://lore.kernel.org/all/20210212094059.5f8d05e8@gandalf.local.home/ > > > > > > Signed-off-by: Jisheng Zhang <jszhang@kernel.org> > > > --- > > > arch/arm64/include/asm/jump_label.h | 8 ++++---- > > > 1 file changed, 4 insertions(+), 4 deletions(-) > > > > > > diff --git a/arch/arm64/include/asm/jump_label.h b/arch/arm64/include/asm/jump_label.h > > > index cea441b6aa5d..48ddc0f45d22 100644 > > > --- a/arch/arm64/include/asm/jump_label.h > > > +++ b/arch/arm64/include/asm/jump_label.h > > > @@ -15,8 +15,8 @@ > > > > > > #define JUMP_LABEL_NOP_SIZE AARCH64_INSN_SIZE > > > > > > -static __always_inline bool arch_static_branch(struct static_key *key, > > > - bool branch) > > > +static __always_inline bool arch_static_branch(struct static_key * const key, > > > + const bool branch) > > > { > > > asm_volatile_goto( > > > "1: nop \n\t" > > > > Is this still necessary if we specify the constraints in a more > > reasonable manner: > > > > " .quad %c0 - . + %1 \n\t" > > : : "i"(key), "i"(branch) : : l_yes); > > Just tried this locally with the simple external kernel module, the > asm operand 0 probably does not match constraints can still be > reproduced with "O0". >
OK so the key pointer needs to be pointer-to-const, which makes sense. How about when using "Si" as the constraint for operand 0?
> > > > > instead of the horrid hack with the char* cast and using a bool as an > > array index? > > > > > > > > > > > @@ -32,8 +32,8 @@ static __always_inline bool arch_static_branch(struct static_key *key, > > > return true; > > > } > > > > > > -static __always_inline bool arch_static_branch_jump(struct static_key *key, > > > - bool branch) > > > +static __always_inline bool arch_static_branch_jump(struct static_key * const key, > > > + const bool branch) > > > { > > > asm_volatile_goto( > > > "1: b %l[l_yes] \n\t" > > > -- > > > 2.37.2 > > >
| |