Re: [Xen-devel] [PATCH v1 3/9] x86/arm64: Move the ALT_[ORIG|REPL]_PTR macros to header files.

[Julien Grall <julien.grall@xxxxxxx>]

Hi Konrad,

On 15/08/16 00:07, Konrad Rzeszutek Wilk wrote:
> That way common code can use the same macro to access
> the most common attributes without much #ifdef.
>
> Take advantage of it right away in the livepatch code.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>
> ---
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx>
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> v1: First submission
> ---
>  xen/arch/arm/alternative.c        | 4 ----
>  xen/common/livepatch.c            | 4 ++--
>  xen/include/asm-arm/alternative.h | 4 ++++
>  xen/include/asm-x86/alternative.h | 4 ++++
>  4 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> index 8ee5a11..bf4101c 100644
> --- a/xen/arch/arm/alternative.c
> +++ b/xen/arch/arm/alternative.c
> @@ -32,10 +32,6 @@
>  #include <asm/insn.h>
>  #include <asm/page.h>
>
> -#define __ALT_PTR(a,f)      (u32 *)((void *)&(a)->f + (a)->f)
> -#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
> -#define ALT_REPL_PTR(a)     __ALT_PTR(a, alt_offset)
> -
>  extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
>
>  struct alt_region {
> diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
> index 3a22de2..9c45270 100644
> --- a/xen/common/livepatch.c
> +++ b/xen/common/livepatch.c
> @@ -700,8 +700,8 @@ static int prepare_payload(struct payload *payload,
>
>          for ( a = start; a < end; a++ )
>          {
> -            const void *instr = &a->instr_offset + a->instr_offset;
> -            const void *replacement = &a->repl_offset + a->repl_offset;
> +            const void *instr = ALT_ORIG_PTR(a);
> +            const void *replacement = ALT_REPL_PTR(a);
>
>              if ( (instr < region->start && instr >= region->end) ||
>                   (replacement < region->start && replacement >= region->end) )
> diff --git a/xen/include/asm-arm/alternative.h 
> b/xen/include/asm-arm/alternative.h
> index 4287bac..58d5fa7 100644
> --- a/xen/include/asm-arm/alternative.h
> +++ b/xen/include/asm-arm/alternative.h
> @@ -21,6 +21,10 @@ struct alt_instr {
>         u8  alt_len;            /* size of new instruction(s), <= orig_len */
>  };
>
> +#define __ALT_PTR(a,f)      (u32 *)((void *)&(a)->f + (a)->f)
> +#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
> +#define ALT_REPL_PTR(a)     __ALT_PTR(a, alt_offset)
> +
alternative.h is a verbatim copy of the Linux one. So can you add a 
comment such as /* Xen: helpers used by the common code */?
Also this should be indented with hard tab as the file is following the 
Linux coding style.
>  void __init apply_alternatives_all(void);
>  int apply_alternatives(void *start, size_t length);
>
> diff --git a/xen/include/asm-x86/alternative.h 
> b/xen/include/asm-x86/alternative.h
> index acaeded..b72c401 100644
> --- a/xen/include/asm-x86/alternative.h
> +++ b/xen/include/asm-x86/alternative.h
> @@ -23,6 +23,10 @@ struct alt_instr {
>      u8  replacementlen;     /* length of new instruction, <= instrlen */
>  };
>
> +#define __ALT_PTR(a,f)      (u8 *)((void *)&(a)->f + (a)->f)
> +#define ALT_ORIG_PTR(a)     __ALT_PTR(a, instr_offset)
> +#define ALT_REPL_PTR(a)     __ALT_PTR(a, repl_offset)
> +
>  extern void add_nops(void *insns, unsigned int len);
>  /* Similar to apply_alternatives except it can be run with IRQs enabled. */
>  extern void apply_alternatives_nocheck(struct alt_instr *start,
Regards,

--
Julien Grall

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel