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

2016-08-17 Thread Julien Grall 

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 

---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Jan Beulich 
Cc: Andrew Cooper 

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 
 #include 

-#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 = >instr_offset + a->instr_offset;
-const void *replacement = >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@lists.xen.org
https://lists.xen.org/xen-devel

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

2016-08-17 Thread Jan Beulich 
>>> On 15.08.16 at 01:07,  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 

x86 part
Acked-by: Jan Beulich 
with one adjustment:

> --- 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)

This needs another set of parentheses around the entire expression
(also in the ARM variant).

And I'd like to also note that from an x86 perspective the "Move" in the
title isn't really correct.

Jan


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

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

2016-08-14 Thread Konrad Rzeszutek Wilk 
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 

---
Cc: Stefano Stabellini 
Cc: Julien Grall 
Cc: Jan Beulich 
Cc: Andrew Cooper 

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 
 #include 
 
-#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 = >instr_offset + a->instr_offset;
-const void *replacement = >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)
+
 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,
-- 
2.4.11


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