[v2,12/20] x86, arm: Change arch_livepatch_quiesce() decleration.

Commit Message

Konrad Rzeszutek Wilk Aug. 25, 2016, 1:37 p.m. UTC

On ARM we need an alternative VA region to poke in the
hypervisor .text data. And since this is setup during runtime
we may fail (it uses vmap so most likely error is ENOMEM).

As such this error needs to be bubbled up and also abort
the livepatching if it occurs.

Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

---
Cc: Stefano Stabellini <[e-mail]>
Cc: Julien Grall <[e-mail]>
Cc: Jan Beulich <[e-mail]>
Cc: Andrew Cooper <[e-mail]>

v2: Initial submission. Spun of from "livepatch: Initial ARM64 support"
---
 xen/arch/arm/livepatch.c    |  3 ++-
 xen/arch/x86/livepatch.c    |  4 +++-
 xen/common/livepatch.c      | 16 ++++++++++++++--
 xen/include/xen/livepatch.h |  2 +-
 4 files changed, 20 insertions(+), 5 deletions(-)

Comments

Andrew Cooper Aug. 25, 2016, 1:59 p.m. UTC | #1

On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote:

> On ARM we need an alternative VA region to poke in the
> hypervisor .text data. And since this is setup during runtime
> we may fail (it uses vmap so most likely error is ENOMEM).
>
> As such this error needs to be bubbled up and also abort
> the livepatching if it occurs.
>
> Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

Reviewed-by: Andrew Cooper <[e-mail]>

Julien Grall Sept. 1, 2016, 1:13 p.m. UTC | #2

Hi Konrad,

NIT: title: s/decleration/declaration/

On 25/08/16 14:37, Konrad Rzeszutek Wilk wrote:
> On ARM we need an alternative VA region to poke in the
> hypervisor .text data. And since this is setup during runtime
> we may fail (it uses vmap so most likely error is ENOMEM).
>
> As such this error needs to be bubbled up and also abort
> the livepatching if it occurs.
>
> Signed-off-by: Konrad Rzeszutek Wilk <[e-mail]>

Reviewed-by: Julien Grall <[e-mail]>

Regards,

Patch

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index aba1320..755f596 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -7,8 +7,9 @@ 
 #include <xen/livepatch_elf.h>
 #include <xen/livepatch.h>
 
-void arch_livepatch_quiesce(void)
+int arch_livepatch_quiesce(void)
 {
+    return -ENOSYS;
 }
 
 void arch_livepatch_revive(void)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 67dda07..5491c09 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -16,10 +16,12 @@ 
 #define PATCH_INSN_SIZE 5
 #define MAX_INSN_SIZE 15
 
-void arch_livepatch_quiesce(void)
+int arch_livepatch_quiesce(void)
 {
     /* Disable WP to allow changes to read-only pages. */
     write_cr0(read_cr0() & ~X86_CR0_WP);
+
+    return 0;
 }
 
 void arch_livepatch_revive(void)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 7e36d97..7d21326 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -1088,6 +1088,7 @@  static int livepatch_list(xen_sysctl_livepatch_list_t *list)
 static int apply_payload(struct payload *data)
 {
     unsigned int i;
+    int rc;
 
     printk(XENLOG_INFO LIVEPATCH "%s: Applying %u functions\n",
             data->name, data->nfuncs);
@@ -1096,7 +1097,12 @@  static int apply_payload(struct payload *data)
     for ( i = 0; i < data->n_bss; i++ )
         memset(data->bss[i], 0, data->bss_size[i]);
 
-    arch_livepatch_quiesce();
+    rc = arch_livepatch_quiesce();
+    if ( rc )
+    {
+        printk(XENLOG_ERR LIVEPATCH "%s: unable to quiesce!\n", data->name);
+        return rc;
+    }
 
     /*
      * Since we are running with IRQs disabled and the hooks may call common
@@ -1128,10 +1134,16 @@  static int apply_payload(struct payload *data)
 static int revert_payload(struct payload *data)
 {
     unsigned int i;
+    int rc;
 
     printk(XENLOG_INFO LIVEPATCH "%s: Reverting\n", data->name);
 
-    arch_livepatch_quiesce();
+    rc = arch_livepatch_quiesce();
+    if ( rc )
+    {
+        printk(XENLOG_ERR LIVEPATCH "%s: unable to quiesce!\n", data->name);
+        return rc;
+    }
 
     for ( i = 0; i < data->nfuncs; i++ )
         arch_livepatch_revert_jmp(&data->funcs[i]);
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 2e64686..6f30c0d 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -72,7 +72,7 @@  int arch_livepatch_verify_func(const struct livepatch_func *func);
  * These functions are called around the critical region patching live code,
  * for an architecture to take make appropratie global state adjustments.
  */
-void arch_livepatch_quiesce(void);
+int arch_livepatch_quiesce(void);
 void arch_livepatch_revive(void);
 
 void arch_livepatch_apply_jmp(struct livepatch_func *func);