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

2018-01-30 Thread Julien Grall 

Hi,

On 29/01/18 20:23, Stefano Stabellini wrote:

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 


Acked-by: Stefano Stabellini 



---
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Konrad Rzeszutek Wilk 
CC: Ross Lagerwall 

TODO: Drop the !HAS_ALTERNATIVE case on ARM?


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


Sounds good. In that case, we should remove all the reference of 
HAS_ALTERNATIVE in the code.


Cheers,


--
Julien Grall

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

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

2018-01-29 Thread Stefano Stabellini 
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 

Acked-by: Stefano Stabellini 


> ---
> CC: Jan Beulich 
> CC: Stefano Stabellini 
> CC: Julien Grall 
> CC: Konrad Rzeszutek Wilk 
> CC: Ross Lagerwall 
> 
> 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(®ion);
> -/* The patching is not expected to fail during boot. */
> -BUG_ON(ret != 0);
> +__apply_alternatives(®ion);
>  
>  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(®ion);
> +__apply_alternatives(®ion);
>  }
>  
>  /*
> 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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

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

2018-01-29 Thread Andrew Cooper 
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 
---
CC: Jan Beulich 
CC: Stefano Stabellini 
CC: Julien Grall 
CC: Konrad Rzeszutek Wilk 
CC: Ross Lagerwall 

TODO: Drop the !HAS_ALTERNATIVE case on ARM?
---
 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(®ion);
-/* The patching is not expected to fail during boot. */
-BUG_ON(ret != 0);
+__apply_alternatives(®ion);
 
 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(®ion);
+__apply_alternatives(®ion);
 }
 
 /*
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@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel