Re: [Xen-devel] [PATCH 1/5] arm/alternatives: Fix apply_alternatives() API

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 29 Jan 2018, Andrew Cooper wrote:
> The !HAS_ALTERNATIVE case has bit-rotten and won't even compile.
> 
> The x86 side of apply_alternatives() is void, while the ARM side returns int.
> However, the functions can't fail and no return values are checked.  Switch
> the ARM side to be void as well.
> 
> One observation is that __apply_alternatives_multi_stop() is only used at boot
> time, so can become __init and its static data can become __initdata
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

Acked-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> CC: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> 
> TODO: Drop the !HAS_ALTERNATIVE case on ARM?

Yeah, we could do. Julien, what do you think?


> ---
>  xen/arch/arm/alternative.c        | 18 +++++++-----------
>  xen/include/asm-arm/alternative.h |  7 ++++---
>  2 files changed, 11 insertions(+), 14 deletions(-)
> 
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> index 9ffdc47..99112e1 100644
> --- a/xen/arch/arm/alternative.c
> +++ b/xen/arch/arm/alternative.c
> @@ -98,7 +98,7 @@ static u32 get_alt_insn(const struct alt_instr *alt,
>   * The region patched should be read-write to allow __apply_alternatives
>   * to replacing the instructions when necessary.
>   */
> -static int __apply_alternatives(const struct alt_region *region)
> +static void __apply_alternatives(const struct alt_region *region)
>  {
>      const struct alt_instr *alt;
>      const u32 *replptr;
> @@ -135,17 +135,15 @@ static int __apply_alternatives(const struct alt_region 
> *region)
>  
>      /* Nuke the instruction cache */
>      invalidate_icache();
> -
> -    return 0;
>  }
>  
>  /*
>   * We might be patching the stop_machine state machine, so implement a
>   * really simple polling protocol here.
>   */
> -static int __apply_alternatives_multi_stop(void *unused)
> +static int __init __apply_alternatives_multi_stop(void *unused)
>  {
> -    static int patched = 0;
> +    static int __initdata patched = 0;
>  
>      /* We always have a CPU 0 at this point (__init) */
>      if ( smp_processor_id() )
> @@ -156,7 +154,6 @@ static int __apply_alternatives_multi_stop(void *unused)
>      }
>      else
>      {
> -        int ret;
>          struct alt_region region;
>          mfn_t xen_mfn = virt_to_mfn(_start);
>          paddr_t xen_size = _end - _start;
> @@ -196,9 +193,7 @@ static int __apply_alternatives_multi_stop(void *unused)
>          region.begin = (void *)__alt_instructions - (void *)_start + xenmap;
>          region.end = (void *)__alt_instructions_end - (void *)_start + 
> xenmap;
>  
> -        ret = __apply_alternatives(&region);
> -        /* The patching is not expected to fail during boot. */
> -        BUG_ON(ret != 0);
> +        __apply_alternatives(&region);
>  
>          unregister_virtual_region(&patch_region);
>  
> @@ -228,14 +223,15 @@ void __init apply_alternatives_all(void)
>      BUG_ON(ret);
>  }
>  
> -int apply_alternatives(const struct alt_instr *start, const struct alt_instr 
> *end)
> +void apply_alternatives(const struct alt_instr *start,
> +                        const struct alt_instr *end)
>  {
>      const struct alt_region region = {
>          .begin = start,
>          .end = end,
>      };
>  
> -    return __apply_alternatives(&region);
> +    __apply_alternatives(&region);
>  }
>  
>  /*
> diff --git a/xen/include/asm-arm/alternative.h 
> b/xen/include/asm-arm/alternative.h
> index 6cc9d0d..49a055e 100644
> --- a/xen/include/asm-arm/alternative.h
> +++ b/xen/include/asm-arm/alternative.h
> @@ -25,7 +25,8 @@ struct alt_instr {
>  #define ALT_REPL_PTR(a)              __ALT_PTR(a, alt_offset)
>  
>  void __init apply_alternatives_all(void);
> -int apply_alternatives(const struct alt_instr *start, const struct alt_instr 
> *end);
> +void apply_alternatives(const struct alt_instr *start,
> +                        const struct alt_instr *end);
>  
>  #define ALTINSTR_ENTRY(feature)                                              
>       \
>       " .word 661b - .\n"                             /* label           */ \
> @@ -158,9 +159,9 @@ static inline void apply_alternatives_all(void)
>  {
>  }
>  
> -static inline int apply_alternatives(void *start, size_t length)
> +static inline void apply_alternatives(const struct alt_instr *start,
> +                                      const struct alt_instr *end)
>  {
> -    return 0;
>  }
>  
>  #endif
> -- 
> 2.1.4
> 

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel