[v3,12/17] livepatch/x86/arm[32, 64]: Unify arch_livepatch_revert

Commit Message

Konrad Rzeszutek Wilk Sept. 12, 2017, 12:37 a.m. UTC

The arch_livepatch_revert is very similar between the platforms.
Lets unify it as much as possible.

Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>
---
 xen/arch/arm/livepatch.c    | 10 +---------
 xen/arch/x86/livepatch.c    | 10 ++--------
 xen/common/livepatch.c      | 14 +++++++++++++-
 xen/include/xen/livepatch.h |  3 +--
 4 files changed, 17 insertions(+), 20 deletions(-)

Comments

Julien Grall Sept. 14, 2017, 1:23 p.m. UTC | #1

Hi Konrad,

On 12/09/17 01:37, Konrad Rzeszutek Wilk wrote:
> diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
> index 1659ffcdf0..065c1a323a 100644
> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -117,11 +117,10 @@ extern struct livepatch_vmap_stash livepatch_vmap;
>    * These functions are called around the critical region patching live code,
>    * for an architecture to take make appropratie global state adjustments.
>    */
> -int arch_livepatch_quiesce(struct livepatch_func *func, unsigned int nfuncs);

I guess this should belonged to the previous patch?

>   void arch_livepatch_revive(void);
>   
>   void arch_livepatch_apply(struct livepatch_func *func);
> -void arch_livepatch_revert(const struct livepatch_func *func);
> +void arch_livepatch_revert(uint32_t *new_ptr, unsigned int len);
>   void arch_livepatch_post_action(void);
>   
>   void arch_livepatch_mask(void);
> 

Cheers,

Patch

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index 2debb5368c..e1d5d58f97 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -39,16 +39,8 @@  int arch_livepatch_verify_func(const struct livepatch_func *func)
     return 0;
 }
 
-void arch_livepatch_revert(const struct livepatch_func *func)
+void arch_livepatch_revert(uint32_t *new_ptr, unsigned int len)
 {
-    uint32_t *new_ptr;
-    unsigned int len;
-
-    new_ptr = func->old_addr - (void *)_start + livepatch_vmap.text;
-
-    len = livepatch_insn_len(func);
-    memcpy(new_ptr, func->opaque, len);
-
     clean_and_invalidate_dcache_va_range(new_ptr, len);
 }
 
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 5273f5a176..12287d445f 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -81,15 +81,9 @@  void noinline arch_livepatch_apply(struct livepatch_func *func)
  * "noinline" to cause control flow change and thus invalidate I$ and
  * cause refetch after modification.
  */
-void noinline arch_livepatch_revert(const struct livepatch_func *func)
+void noinline arch_livepatch_revert(uint32_t *new_ptr, unsigned int len)
 {
-    uint32_t *new_ptr;
-    unsigned int len;
-
-    new_ptr = func->old_addr - (void *)_start + livepatch_vmap.text;
-
-    len = livepatch_insn_len(func);
-    memcpy(new_ptr, func->opaque, len);
+    /* Nothing to do. */
 }
 
 /*
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index eb7d4098fd..93083cda1a 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1128,6 +1128,18 @@  static void livepatch_revive(void)
     livepatch_vmap.offset = 0;
 }
 
+static void livepatch_revert(const struct livepatch_func *func)
+{
+    uint32_t *new_ptr;
+    unsigned int len;
+
+    new_ptr = func->old_addr - (void *)_start + livepatch_vmap.text;
+
+    len = livepatch_insn_len(func);
+    memcpy(new_ptr, func->opaque, len);
+
+    arch_livepatch_revert(new_ptr, len);
+}
 /*
  * The following functions get the CPUs into an appropriate state and
  * apply (or revert) each of the payload's functions. This is needed
@@ -1191,7 +1203,7 @@  static int revert_payload(struct payload *data)
     }
 
     for ( i = 0; i < data->nfuncs; i++ )
-        arch_livepatch_revert(&data->funcs[i]);
+        livepatch_revert(&data->funcs[i]);
 
     /*
      * Since we are running with IRQs disabled and the hooks may call common
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 1659ffcdf0..065c1a323a 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -117,11 +117,10 @@  extern struct livepatch_vmap_stash livepatch_vmap;
  * These functions are called around the critical region patching live code,
  * for an architecture to take make appropratie global state adjustments.
  */
-int arch_livepatch_quiesce(struct livepatch_func *func, unsigned int nfuncs);
 void arch_livepatch_revive(void);
 
 void arch_livepatch_apply(struct livepatch_func *func);
-void arch_livepatch_revert(const struct livepatch_func *func);
+void arch_livepatch_revert(uint32_t *new_ptr, unsigned int len);
 void arch_livepatch_post_action(void);
 
 void arch_livepatch_mask(void);