Re: [RFC PATCH 34/56] x86/alternative: Save old bytes for alternatives

[David Laight]

On Wed, 29 Oct 2025 16:26:58 +0000
"Kaplan, David" <David.Kaplan@xxxxxxx> wrote:

> [AMD Official Use Only - AMD Internal Distribution Only]
> 
> > -----Original Message-----
> > From: Nikolay Borisov <nik.borisov@xxxxxxxx>
> > Sent: Wednesday, October 29, 2025 4:37 AM
> > To: Kaplan, David <David.Kaplan@xxxxxxx>; Juergen Gross
> > <jgross@xxxxxxxx>; Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Borislav Petkov
> > <bp@xxxxxxxxx>; Peter Zijlstra <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf
> > <jpoimboe@xxxxxxxxxx>; Pawan Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>;
> > Ingo Molnar <mingo@xxxxxxxxxx>; Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>;
> > x86@xxxxxxxxxx; H . Peter Anvin <hpa@xxxxxxxxx>
> > Cc: Alexander Graf <graf@xxxxxxxxxx>; Boris Ostrovsky
> > <boris.ostrovsky@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> > Subject: Re: [RFC PATCH 34/56] x86/alternative: Save old bytes for alternatives
> >
> > Caution: This message originated from an External Source. Use proper caution
> > when opening attachments, clicking links, or responding.
> >
> >
> > On 10/27/25 16:19, Kaplan, David wrote:  
> > > [AMD Official Use Only - AMD Internal Distribution Only]
> > >  
> > >> -----Original Message-----
> > >> From: Nikolay Borisov <nik.borisov@xxxxxxxx>
> > >> Sent: Monday, October 27, 2025 6:35 AM
> > >> To: Kaplan, David <David.Kaplan@xxxxxxx>; Juergen Gross  
> > <jgross@xxxxxxxx>;  
> > >> Thomas Gleixner <tglx@xxxxxxxxxxxxx>; Borislav Petkov <bp@xxxxxxxxx>; Peter
> > >> Zijlstra <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf <jpoimboe@xxxxxxxxxx>;  
> > Pawan  
> > >> Gupta <pawan.kumar.gupta@xxxxxxxxxxxxxxx>; Ingo Molnar  
> > <mingo@xxxxxxxxxx>;  
> > >> Dave Hansen <dave.hansen@xxxxxxxxxxxxxxx>; x86@xxxxxxxxxx; H . Peter Anvin
> > >> <hpa@xxxxxxxxx>
> > >> Cc: Alexander Graf <graf@xxxxxxxxxx>; Boris Ostrovsky
> > >> <boris.ostrovsky@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> > >> Subject: Re: [RFC PATCH 34/56] x86/alternative: Save old bytes for alternatives
> > >>
> > >> Caution: This message originated from an External Source. Use proper caution
> > >> when opening attachments, clicking links, or responding.
> > >>
> > >>
> > >> On 10/15/25 16:45, Kaplan, David wrote:  
> > >>> [AMD Official Use Only - AMD Internal Distribution Only]
> > >>>  
> > >>>> -----Original Message-----
> > >>>> From: Juergen Gross <jgross@xxxxxxxx>
> > >>>> Sent: Wednesday, October 15, 2025 5:38 AM
> > >>>> To: Kaplan, David <David.Kaplan@xxxxxxx>; Thomas Gleixner
> > >>>> <tglx@xxxxxxxxxxxxx>; Borislav Petkov <bp@xxxxxxxxx>; Peter Zijlstra
> > >>>> <peterz@xxxxxxxxxxxxx>; Josh Poimboeuf <jpoimboe@xxxxxxxxxx>; Pawan  
> > >> Gupta  
> > >>>> <pawan.kumar.gupta@xxxxxxxxxxxxxxx>; Ingo Molnar <mingo@xxxxxxxxxx>;  
> > >> Dave  
> > >>>> Hansen <dave.hansen@xxxxxxxxxxxxxxx>; x86@xxxxxxxxxx; H . Peter Anvin
> > >>>> <hpa@xxxxxxxxx>
> > >>>> Cc: Alexander Graf <graf@xxxxxxxxxx>; Boris Ostrovsky
> > >>>> <boris.ostrovsky@xxxxxxxxxx>; linux-kernel@xxxxxxxxxxxxxxx
> > >>>> Subject: Re: [RFC PATCH 34/56] x86/alternative: Save old bytes for  
> > alternatives  
> > >>>>
> > >>>> On 13.10.25 16:34, David Kaplan wrote:  
> > >>>>> Save the existing instruction bytes at each alternative site when patching.
> > >>>>> This is only done the first time, and these will be used later to help
> > >>>>> restore the code back to its original form.
> > >>>>>
> > >>>>> Signed-off-by: David Kaplan <david.kaplan@xxxxxxx>  
> > >>>>
> > >>>> Instead of saving the original instructions at runtime, why don't you
> > >>>> expand struct alt_instr to have an additional offset to a saved copy
> > >>>> of the original instruction, located in .altinstr_replacement?
> > >>>>
> > >>>> The new field should be guarded with #ifdef  
> > CONFIG_DYNAMIC_MITIGATIONS,  
> > >>>> of course, like the added handling in the ALTERNATIVE() macros.
> > >>>>  
> > >>>
> > >>> That's an interesting idea, I think that could work.  That would make the kernel  
> > >> image on disk (slightly) larger though, as the original bytes will essentially be
> > >> duplicated (at the original location and in .altinstr_replacement).  I'm not sure  
> > which  
> > >> is the better trade-off (kernel image bytes on disk vs runtime memory usage).
> > >> Although I think we're talking about a relatively small amount of memory  
> > regardless.  
> > >> Most of the runtime overhead of dynamic mitigations comes from remembering  
> > the  
> > >> call sites/returns.
> > >>
> > >> It's not just about memory usage per-se but also memory pressure from
> > >> allocation and the resulting fragmentation, though I'd think that
> > >> majority of the allocation will fit into kmalloc-32 bucket, still having
> > >> them as part of the kernel image eliminates the additional allocs.  
> > >
> > > I see.  Just to understand, the issue is more with the numerous small allocations  
> > right? (that is the kmalloc at each alt_site)  And less about the single large allocation
> > of the array?
> >
> > Yep, do you have some statistics how many allocs have to be done?
> >  
> 
> On a typical kernel, I'm seeing 6427 kmallocs() from this with a total size of ~36kb.
> 
> If that is too many, another option could be to go through and figure out the total size needed and then do one big allocation.

Is there also an 8 byte pointer to each allocation? They add up as well.
Is may be worth doing multiple (say) 4k allocations in a list (or array of pointers).
Then the pointer can be replaced by an offset into the overall 'big buffer'.
Align the entries (a bit) and maybe a the 8 byte pointer can be replaced with
a 16bit index?

	David


> 
> --David Kaplan