Re: [PATCH] livepatch: apply_alternatives() is only used for livepatch

[Roger Pau Monné]

On Tue, Jun 06, 2023 at 07:10:05PM +0100, Julien Grall wrote:
> Hi Roger,
> 
> On 06/06/2023 18:23, Roger Pau Monne wrote:
> > Guard it with CONFIG_LIVEPATCH.  Note alternatives are applied at boot
> > using _apply_alternatives().
> > 
> > Signed-off-by: Roger Pau Monné <roger....@citrix.com>
> 
> Reviewed-by: Julien Grall <jgr...@amazon.com>

Thanks!

> > ---
> >   xen/arch/arm/alternative.c             | 2 ++
> >   xen/arch/arm/include/asm/alternative.h | 2 ++
> >   xen/arch/x86/alternative.c             | 5 +++--
> >   xen/arch/x86/include/asm/alternative.h | 2 ++
> >   4 files changed, 9 insertions(+), 2 deletions(-)
> > 
> > diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> > index 7366af4ea646..016e66978b6d 100644
> > --- a/xen/arch/arm/alternative.c
> > +++ b/xen/arch/arm/alternative.c
> > @@ -223,6 +223,7 @@ void __init apply_alternatives_all(void)
> >       vunmap(xenmap);
> >   }
> > +#ifdef CONFIG_LIVEPATCH
> >   int apply_alternatives(const struct alt_instr *start, const struct 
> > alt_instr *end)
> >   {
> >       const struct alt_region region = {
> > @@ -232,6 +233,7 @@ int apply_alternatives(const struct alt_instr *start, 
> > const struct alt_instr *en
> >       return __apply_alternatives(&region, 0);
> >   }
> > +#endif
> >   /*
> >    * Local variables:
> > diff --git a/xen/arch/arm/include/asm/alternative.h 
> > b/xen/arch/arm/include/asm/alternative.h
> > index d3210e82f9e5..ce82dc1ca0d2 100644
> > --- a/xen/arch/arm/include/asm/alternative.h
> > +++ b/xen/arch/arm/include/asm/alternative.h
> > @@ -29,7 +29,9 @@ typedef void (*alternative_cb_t)(const struct alt_instr 
> > *alt,
> >                              int nr_inst);
> >   void apply_alternatives_all(void);
> > +#ifdef CONFIG_LIVEPATCH
> >   int apply_alternatives(const struct alt_instr *start, const struct 
> > alt_instr *end);
> > +#endif
> >   #define ALTINSTR_ENTRY(feature, cb)                                       
> >       \
> >     " .word 661b - .\n"                             /* label           */ \
> > diff --git a/xen/arch/x86/alternative.c b/xen/arch/x86/alternative.c
> > index 99482766b51f..21af0e825822 100644
> > --- a/xen/arch/x86/alternative.c
> > +++ b/xen/arch/x86/alternative.c
> > @@ -358,11 +358,12 @@ static void init_or_livepatch 
> > _apply_alternatives(struct alt_instr *start,
> >       }
> >   }
> > -void init_or_livepatch apply_alternatives(struct alt_instr *start,
> > -                                          struct alt_instr *end)
> 
> NIT: It sounds like the init_* was a left-over after commit 67d01cdb5518
> ("x86: infrastructure to allow converting certain indirect calls to direct
> ones").

I think it doesn't warrant a fixes tag in this case.  The commit you
point out should also have added the livepatch guards in x86 at least,
since that's the only caller of apply_alternatives() after the
patch.

Thanks, Roger.