[PATCH] x86/static_call: Remove text_mutex from __static_call_fixup()

[PATCH] x86/static_call: Remove text_mutex from __static_call_fixup()

[David Kaplan]

__static_call_fixup() is only called from apply_returns() which modifies
text either on boot when only one CPU is active, or on module load.  In
both cases, it does not modify live code.  This is why this function passes
'true' for the modinit argument to __static_call_transform() which causes
__static_call_transform() to use text_poke_early().

text_poke_early() does not require that the text_mutex is held, as this
function is only used when free of any races with other CPUs.  Therefore
__static_call_fixup() does not need to take the mutex so remove it.

Signed-off-by: David Kaplan <david.kaplan@amd.com>
---
 arch/x86/kernel/static_call.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/arch/x86/kernel/static_call.c b/arch/x86/kernel/static_call.c
index 378c388d1b31..748fc98d416f 100644
--- a/arch/x86/kernel/static_call.c
+++ b/arch/x86/kernel/static_call.c
@@ -212,10 +212,8 @@ bool __static_call_fixup(void *tramp, u8 op, void *dest)
 		return false;
 	}
 
-	mutex_lock(&text_mutex);
 	if (op == RET_INSN_OPCODE || dest == &__x86_return_thunk)
 		__static_call_transform(tramp, RET, NULL, true);
-	mutex_unlock(&text_mutex);
 
 	return true;
 }

base-commit: 8411fdb92360d0b4d3337492a25ee2de7fb6c425
-- 
2.34.1

Re: [PATCH] x86/static_call: Remove text_mutex from __static_call_fixup()

[Borislav Petkov]

Hey,

On Thu, Nov 06, 2025 at 11:53:20AM -0600, David Kaplan wrote:
> __static_call_fixup() is only called from apply_returns() which modifies
> text either on boot when only one CPU is active, or on module load.  In
> both cases, it does not modify live code.  This is why this function passes
> 'true' for the modinit argument to __static_call_transform() which causes
> __static_call_transform() to use text_poke_early().
> 
> text_poke_early() does not require that the text_mutex is held, as this
> function is only used when free of any races with other CPUs.  Therefore
> __static_call_fixup() does not need to take the mutex so remove it.
> 
> Signed-off-by: David Kaplan <david.kaplan@amd.com>
> ---
>  arch/x86/kernel/static_call.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/arch/x86/kernel/static_call.c b/arch/x86/kernel/static_call.c
> index 378c388d1b31..748fc98d416f 100644
> --- a/arch/x86/kernel/static_call.c
> +++ b/arch/x86/kernel/static_call.c
> @@ -212,10 +212,8 @@ bool __static_call_fixup(void *tramp, u8 op, void *dest)
>  		return false;
>  	}
>  
> -	mutex_lock(&text_mutex);
>  	if (op == RET_INSN_OPCODE || dest == &__x86_return_thunk)
>  		__static_call_transform(tramp, RET, NULL, true);
> -	mutex_unlock(&text_mutex);
>  
>  	return true;
>  }
> 

can you please elaborate here what the actual problem was?

I know it had to do with dynamic mitigations, I guess something about
lockdep complaining... can you paste the splat here?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

RE: [PATCH] x86/static_call: Remove text_mutex from __static_call_fixup()

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Borislav Petkov <bp@alien8.de>
> Sent: Wednesday, December 10, 2025 1:55 PM
> To: Kaplan, David <David.Kaplan@amd.com>
> Cc: Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@kernel.org>; Jason Baron <jbaron@akamai.com>; Steven Rostedt
> <rostedt@goodmis.org>; Ard Biesheuvel <ardb@kernel.org>; Thomas Gleixner
> <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; Dave Hansen
> <dave.hansen@linux.intel.com>; x86@kernel.org; H. Peter Anvin
> <hpa@zytor.com>; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] x86/static_call: Remove text_mutex from
> __static_call_fixup()
>
> Caution: This message originated from an External Source. Use proper caution
> when opening attachments, clicking links, or responding.
>
>
> Hey,
>
> On Thu, Nov 06, 2025 at 11:53:20AM -0600, David Kaplan wrote:
> > __static_call_fixup() is only called from apply_returns() which modifies
> > text either on boot when only one CPU is active, or on module load.  In
> > both cases, it does not modify live code.  This is why this function passes
> > 'true' for the modinit argument to __static_call_transform() which causes
> > __static_call_transform() to use text_poke_early().
> >
> > text_poke_early() does not require that the text_mutex is held, as this
> > function is only used when free of any races with other CPUs.  Therefore
> > __static_call_fixup() does not need to take the mutex so remove it.
> >
> > Signed-off-by: David Kaplan <david.kaplan@amd.com>
> > ---
> >  arch/x86/kernel/static_call.c | 2 --
> >  1 file changed, 2 deletions(-)
> >
> > diff --git a/arch/x86/kernel/static_call.c b/arch/x86/kernel/static_call.c
> > index 378c388d1b31..748fc98d416f 100644
> > --- a/arch/x86/kernel/static_call.c
> > +++ b/arch/x86/kernel/static_call.c
> > @@ -212,10 +212,8 @@ bool __static_call_fixup(void *tramp, u8 op, void
> *dest)
> >               return false;
> >       }
> >
> > -     mutex_lock(&text_mutex);
> >       if (op == RET_INSN_OPCODE || dest == &__x86_return_thunk)
> >               __static_call_transform(tramp, RET, NULL, true);
> > -     mutex_unlock(&text_mutex);
> >
> >       return true;
> >  }
> >
>
> can you please elaborate here what the actual problem was?
>
> I know it had to do with dynamic mitigations, I guess something about
> lockdep complaining... can you paste the splat here?
>

Yeah, the full splat can be found in https://lore.kernel.org/lkml/d255d8a9-8e45-485e-9853-80c343bbb73b@suse.com/

The problem was that patching under dynamic mitigations is done under NMI context, so it complained about taking a mutex in NMI context.

Thing is, dynamic mitigation patching happens with all CPUs stopped and synchronized at a safe point.  This allows for using text_poke_early(), as we do during boot.  This particular code path also always ends up at text_poke_early(), which does not require the text_mutex being held.  So taking it seems unnecessary (unless I've missed something in my analysis).

--David Kaplan

RE: [PATCH] x86/static_call: Remove text_mutex from __static_call_fixup()

[Kaplan, David]

[AMD Official Use Only - AMD Internal Distribution Only]

> -----Original Message-----
> From: Kaplan, David
> Sent: Wednesday, December 10, 2025 1:40 PM
> To: 'Borislav Petkov' <bp@alien8.de>
> Cc: Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> <jpoimboe@kernel.org>; Jason Baron <jbaron@akamai.com>; Steven Rostedt
> <rostedt@goodmis.org>; Ard Biesheuvel <ardb@kernel.org>; Thomas Gleixner
> <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; Dave Hansen
> <dave.hansen@linux.intel.com>; x86@kernel.org; H. Peter Anvin
> <hpa@zytor.com>; linux-kernel@vger.kernel.org
> Subject: RE: [PATCH] x86/static_call: Remove text_mutex from
> __static_call_fixup()
>
>
>
> > -----Original Message-----
> > From: Borislav Petkov <bp@alien8.de>
> > Sent: Wednesday, December 10, 2025 1:55 PM
> > To: Kaplan, David <David.Kaplan@amd.com>
> > Cc: Peter Zijlstra <peterz@infradead.org>; Josh Poimboeuf
> > <jpoimboe@kernel.org>; Jason Baron <jbaron@akamai.com>; Steven
> Rostedt
> > <rostedt@goodmis.org>; Ard Biesheuvel <ardb@kernel.org>; Thomas
> Gleixner
> > <tglx@linutronix.de>; Ingo Molnar <mingo@redhat.com>; Dave Hansen
> > <dave.hansen@linux.intel.com>; x86@kernel.org; H. Peter Anvin
> > <hpa@zytor.com>; linux-kernel@vger.kernel.org
> > Subject: Re: [PATCH] x86/static_call: Remove text_mutex from
> > __static_call_fixup()
> >
> > Caution: This message originated from an External Source. Use proper
> caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > Hey,
> >
> > On Thu, Nov 06, 2025 at 11:53:20AM -0600, David Kaplan wrote:
> > > __static_call_fixup() is only called from apply_returns() which modifies
> > > text either on boot when only one CPU is active, or on module load.  In
> > > both cases, it does not modify live code.  This is why this function passes
> > > 'true' for the modinit argument to __static_call_transform() which causes
> > > __static_call_transform() to use text_poke_early().
> > >
> > > text_poke_early() does not require that the text_mutex is held, as this
> > > function is only used when free of any races with other CPUs.  Therefore
> > > __static_call_fixup() does not need to take the mutex so remove it.
> > >
> > > Signed-off-by: David Kaplan <david.kaplan@amd.com>
> > > ---
> > >  arch/x86/kernel/static_call.c | 2 --
> > >  1 file changed, 2 deletions(-)
> > >
> > > diff --git a/arch/x86/kernel/static_call.c b/arch/x86/kernel/static_call.c
> > > index 378c388d1b31..748fc98d416f 100644
> > > --- a/arch/x86/kernel/static_call.c
> > > +++ b/arch/x86/kernel/static_call.c
> > > @@ -212,10 +212,8 @@ bool __static_call_fixup(void *tramp, u8 op, void
> > *dest)
> > >               return false;
> > >       }
> > >
> > > -     mutex_lock(&text_mutex);
> > >       if (op == RET_INSN_OPCODE || dest == &__x86_return_thunk)
> > >               __static_call_transform(tramp, RET, NULL, true);
> > > -     mutex_unlock(&text_mutex);
> > >
> > >       return true;
> > >  }
> > >
> >
> > can you please elaborate here what the actual problem was?
> >
> > I know it had to do with dynamic mitigations, I guess something about
> > lockdep complaining... can you paste the splat here?
> >
>
> Yeah, the full splat can be found in https://lore.kernel.org/lkml/d255d8a9-
> 8e45-485e-9853-80c343bbb73b@suse.com/
>
> The problem was that patching under dynamic mitigations is done under NMI
> context, so it complained about taking a mutex in NMI context.
>
> Thing is, dynamic mitigation patching happens with all CPUs stopped and
> synchronized at a safe point.  This allows for using text_poke_early(), as we do
> during boot.  This particular code path also always ends up at
> text_poke_early(), which does not require the text_mutex being held.  So
> taking it seems unnecessary (unless I've missed something in my analysis).
>

After some discussions at LPC, I believe this patch is still needed.  In current upstream code, this function is only called by the BSP during early boot and text_mutex isn't required.  Importantly, none of the other logic that uses text_poke_early() acquires the text_mutex either.

With dynamic mitigations, I think I do need to acquire text_mutex since I'm going and modifying a bunch of text, but that needs to be done before stop_machine_nmi().  And so we can't have this function try to take it again.

--David Kaplan