All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3] Livepatch for ARM 64 and 32.
@ 2016-09-11 20:35 Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
                   ` (18 more replies)
  0 siblings, 19 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3

v2: [http://www.gossamer-threads.com/lists/xen/devel/444461?page=last]
 - Addressed the two outstanding concerns: CPU bit feature to test alternative
   test-case, and errata #843419 on some Cortex-A53 
 - Dealt with comments from Jan, Julien and Andrew.
 - Fixed (offlist) with Julien ARM32 not branching properly.
 - Committed some of the patches that had been reviewed/Acked.
v1 (and RFC): [https://lists.xen.org/archives/html/xen-devel/2016-08/msg01835.html]
 - Acted on most all comments.
 - Added ARM 32 support.

The patches are based on: [PATCH v5] Livepatch fixes and features for v4.8.
(https://lists.xen.org/archives/html/xen-devel/2016-09/msg01114.html)

And the git tree is:
 git://xenbits.xen.org/people/konradwilk/xen.git livepatch.v4.8.v5

These patches enable livepatching to work on ARM64 and ARM32. They have
been tested on multi-CPU environments (Foundation Model, 4CPU for ARM64;
and ARM CubieTruck for ARM32).

There are also support patches and one general bug-fix ("arm: poison
initmem when it is freed.") - which could be ignored.

Enjoy!

Konrad Rzeszutek Wilk (18):
      arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE]
      livepatch: Reject payloads with .alternative or .ex_table if support is not built-in.
      arm/x86: change [modify,destroy]_xen_mappings to return error
      arm/mm: Introduce modify_xen_mappings
      arm: poison initmem when it is freed.
      livepatch: Initial ARM64 support.
      livepatch: ARM/x86: Check displacement of old_addr and new_addr
      livepatch: ARM 32|64: Ignore mapping symbols: $[d,a,x]
      livepatch/arm/x86: Check payload for for unwelcomed symbols.
      livepatch: Move test-cases to their own sub-directory in test.
      livepatch: tests: Move the .name value to .rodata
      livepatch: tests: Make them compile under ARM64
      livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
      xen/arm32: Add an helper to invalidate all instruction caches
      bug/x86/arm: Align bug_frames sections.
      livepatch: ARM32 support.
      livepatch, arm[32|64]: Share arch_livepatch_revert_jmp
      livepatch: arm[32,64],x86: NOP test-case

 .gitignore                                  |   8 +-
 MAINTAINERS                                 |   2 +
 docs/misc/livepatch.markdown                |  14 +-
 xen/Makefile                                |   5 +-
 xen/arch/arm/Kconfig                        |   4 +
 xen/arch/arm/Makefile                       |  16 +-
 xen/arch/arm/arm32/Makefile                 |   1 +
 xen/arch/arm/arm32/livepatch.c              | 290 +++++++++++++++++
 xen/arch/arm/arm64/Makefile                 |   1 +
 xen/arch/arm/arm64/livepatch.c              | 468 ++++++++++++++++++++++++++++
 xen/arch/arm/domain.c                       |   6 +
 xen/arch/arm/livepatch.c                    | 159 ++++++++--
 xen/arch/arm/mm.c                           |  33 +-
 xen/arch/arm/traps.c                        |   6 +
 xen/arch/x86/Kconfig                        |   3 +
 xen/arch/x86/Makefile                       |   5 -
 xen/arch/x86/livepatch.c                    |  20 +-
 xen/arch/x86/mm.c                           |   7 +-
 xen/arch/x86/test/Makefile                  |  85 -----
 xen/arch/x86/test/xen_bye_world.c           |  34 --
 xen/arch/x86/test/xen_bye_world_func.c      |  22 --
 xen/arch/x86/test/xen_hello_world.c         |  63 ----
 xen/arch/x86/test/xen_hello_world_func.c    |  39 ---
 xen/arch/x86/test/xen_replace_world.c       |  33 --
 xen/arch/x86/test/xen_replace_world_func.c  |  22 --
 xen/common/Kconfig                          |   8 +-
 xen/common/livepatch.c                      |  20 +-
 xen/common/livepatch_elf.c                  |   9 +
 xen/include/asm-arm/alternative.h           |   2 +
 xen/include/asm-arm/arm32/page.h            |  13 +
 xen/include/asm-arm/bug.h                   |   1 +
 xen/include/asm-arm/config.h                |   5 +
 xen/include/asm-arm/cpufeature.h            |   5 +
 xen/include/asm-arm/livepatch.h             |  39 +++
 xen/include/asm-arm/page.h                  |  11 +
 xen/include/asm-x86/bug.h                   |   1 +
 xen/include/asm-x86/cpufeature.h            |   1 +
 xen/include/asm-x86/livepatch.h             |   3 +
 xen/include/xen/elfstructs.h                |  79 ++++-
 xen/include/xen/livepatch.h                 |  26 +-
 xen/include/xen/mm.h                        |   4 +-
 xen/include/xen/types.h                     |   9 +
 xen/test/Makefile                           |   7 +
 xen/test/livepatch/Makefile                 | 108 +++++++
 xen/test/livepatch/xen_bye_world.c          |  34 ++
 xen/test/livepatch/xen_bye_world_func.c     |  22 ++
 xen/test/livepatch/xen_hello_world.c        |  63 ++++
 xen/test/livepatch/xen_hello_world_func.c   |  47 +++
 xen/test/livepatch/xen_nop.c                |  49 +++
 xen/test/livepatch/xen_replace_world.c      |  33 ++
 xen/test/livepatch/xen_replace_world_func.c |  22 ++
 51 files changed, 1611 insertions(+), 356 deletions(-)



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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE]
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-12 15:28   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 02/18] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in Konrad Rzeszutek Wilk
                   ` (17 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Doug Goldstein, Jan Beulich, Konrad Rzeszutek Wilk

x86 implements all of them by default - and we just
add two extra HAS_ variables to be declared in autoconf.h.

ARM 64 only has alternative while ARM 32 has none of them.
The ARM64 is going to be a bit funny as there is an
ALTERNATIVE already and we end up selecting the HAS_ALTERNATIVE
whenever the ALTERNATIVE is selected.

And while at it change the livepatch common code that
would benefit from this.

Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Doug Goldstein <cardoe@cardoe.com>

v2: First submission
v3: Move the config options to common code
    Don't include <xen/config.h> in the file.
    Don't even include <xen/kconfig.h> in the file as xen/Rules.mk automatically
    includes the config.h for every GCC invocation.
---
 xen/arch/arm/Kconfig   | 4 ++++
 xen/arch/x86/Kconfig   | 3 +++
 xen/common/Kconfig     | 6 ++++++
 xen/common/livepatch.c | 4 +++-
 4 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 797c91f..bf640c5 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -45,8 +45,12 @@ config ACPI
 config HAS_GICV3
 	bool
 
+config HAS_ALTERNATIVE
+	bool
+
 config ALTERNATIVE
 	bool
+	select HAS_ALTERNATIVE
 
 endmenu
 
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 265fd79..056a77f 100644
--- a/xen/arch/x86/Kconfig
+++ b/xen/arch/x86/Kconfig
@@ -7,8 +7,10 @@ config X86
 	select ACPI_LEGACY_TABLES_LOOKUP
 	select COMPAT
 	select CORE_PARKING
+	select HAS_ALTERNATIVE
 	select HAS_CPUFREQ
 	select HAS_EHCI
+	select HAS_EX_TABLE
 	select HAS_GDBSX
 	select HAS_IOPORTS
 	select HAS_KEXEC
@@ -22,6 +24,7 @@ config X86
 	select NUMA
 	select VGA
 
+
 config ARCH_DEFCONFIG
 	string
 	default "arch/x86/configs/x86_64_defconfig"
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 4331874..81e0017 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -11,9 +11,15 @@ config COMPAT
 config CORE_PARKING
 	bool
 
+config HAS_ALTERNATIVE
+	bool
+
 config HAS_DEVICE_TREE
 	bool
 
+config HAS_EX_TABLE
+	bool
+
 config HAS_MEM_ACCESS
 	bool
 
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index f75171b..a2ddb61 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -628,7 +628,7 @@ static int prepare_payload(struct payload *payload,
                                   sizeof(*region->frame[i].bugs);
     }
 
-#ifndef CONFIG_ARM
+#ifdef CONFIG_HAS_ALTERNATIVE
     sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
     if ( sec )
     {
@@ -659,7 +659,9 @@ static int prepare_payload(struct payload *payload,
         }
         apply_alternatives(start, end);
     }
+#endif
 
+#ifdef CONFIG_HAS_EX_TABLE
     sec = livepatch_elf_sec_by_name(elf, ".ex_table");
     if ( sec )
     {
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 02/18] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-16 12:50   ` Julien Grall
  2016-09-11 20:35 ` [PATCH v3 03/18] arm/x86: change [modify, destroy]_xen_mappings to return error Konrad Rzeszutek Wilk
                   ` (16 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

If the payload had the sections mentioned but the hypervisor
did not support some of them (say on ARM the .ex_table) - instead
of ignoring them - it should forbid loading of such payload.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3: New submission.
---
 xen/common/livepatch.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index a2ddb61..5baa418 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -628,10 +628,10 @@ static int prepare_payload(struct payload *payload,
                                   sizeof(*region->frame[i].bugs);
     }
 
-#ifdef CONFIG_HAS_ALTERNATIVE
     sec = livepatch_elf_sec_by_name(elf, ".altinstructions");
     if ( sec )
     {
+#ifdef CONFIG_HAS_ALTERNATIVE
         struct alt_instr *a, *start, *end;
 
         if ( sec->sec->sh_size % sizeof(*a) )
@@ -658,13 +658,17 @@ static int prepare_payload(struct payload *payload,
             }
         }
         apply_alternatives(start, end);
-    }
+#else
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: We don't support alternative patching!\n",
+                elf->name);
+        return -EOPNOTSUPP;
 #endif
+    }
 
-#ifdef CONFIG_HAS_EX_TABLE
     sec = livepatch_elf_sec_by_name(elf, ".ex_table");
     if ( sec )
     {
+#ifdef CONFIG_HAS_EX_TABLE
         struct exception_table_entry *s, *e;
 
         if ( !sec->sec->sh_size ||
@@ -683,8 +687,12 @@ static int prepare_payload(struct payload *payload,
 
         region->ex = s;
         region->ex_end = e;
-    }
+#else
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: We don't support .ex_table!\n",
+                elf->name);
+        return -EOPNOTSUPP;
 #endif
+    }
 
     return 0;
 }
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 03/18] arm/x86: change [modify, destroy]_xen_mappings to return error
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 02/18] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-16 13:06   ` Julien Grall
  2016-09-11 20:35 ` [PATCH v3 04/18] arm/mm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
                   ` (15 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

The implementation on x86 always returns zero, but
other platforms may return error values.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v2: First submission
v3: Add destroy_xen_mappings per Andrew's request.
    Add Andrew's Reviewed-by.
---
 xen/arch/arm/mm.c        | 4 ++--
 xen/arch/x86/livepatch.c | 4 +---
 xen/arch/x86/mm.c        | 7 ++++---
 xen/include/xen/mm.h     | 4 ++--
 4 files changed, 9 insertions(+), 10 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 4e256c2..7ae9f63 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -917,9 +917,9 @@ int populate_pt_range(unsigned long virt, unsigned long mfn,
     return create_xen_entries(RESERVE, virt, mfn, nr_mfns, 0);
 }
 
-void destroy_xen_mappings(unsigned long v, unsigned long e)
+int destroy_xen_mappings(unsigned long v, unsigned long e)
 {
-    create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
+    return create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
 }
 
 enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index 2f54d7b..c4d4b4d 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -224,9 +224,7 @@ int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
     else
         flag = PAGE_HYPERVISOR_RO;
 
-    modify_xen_mappings(start, start + pages * PAGE_SIZE, flag);
-
-    return 0;
+    return modify_xen_mappings(start, start + pages * PAGE_SIZE, flag);
 }
 
 void __init arch_livepatch_init(void)
diff --git a/xen/arch/x86/mm.c b/xen/arch/x86/mm.c
index b9debcc..eddf098 100644
--- a/xen/arch/x86/mm.c
+++ b/xen/arch/x86/mm.c
@@ -5976,7 +5976,7 @@ int populate_pt_range(unsigned long virt, unsigned long mfn,
  *
  * It is an error to call with present flags over an unpopulated range.
  */
-void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
+int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
 {
     bool_t locking = system_state > SYS_STATE_boot;
     l2_pgentry_t *pl2e;
@@ -6151,13 +6151,14 @@ void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int nf)
     flush_area(NULL, FLUSH_TLB_GLOBAL);
 
 #undef FLAGS_MASK
+    return 0;
 }
 
 #undef flush_area
 
-void destroy_xen_mappings(unsigned long s, unsigned long e)
+int destroy_xen_mappings(unsigned long s, unsigned long e)
 {
-    modify_xen_mappings(s, e, _PAGE_NONE);
+    return modify_xen_mappings(s, e, _PAGE_NONE);
 }
 
 void __set_fixmap(
diff --git a/xen/include/xen/mm.h b/xen/include/xen/mm.h
index 58bc0b8..f470e49 100644
--- a/xen/include/xen/mm.h
+++ b/xen/include/xen/mm.h
@@ -146,8 +146,8 @@ int map_pages_to_xen(
     unsigned long nr_mfns,
     unsigned int flags);
 /* Alter the permissions of a range of Xen virtual address space. */
-void modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags);
-void destroy_xen_mappings(unsigned long v, unsigned long e);
+int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags);
+int destroy_xen_mappings(unsigned long v, unsigned long e);
 /*
  * Create only non-leaf page table entries for the
  * page range in Xen virtual address space.
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 04/18] arm/mm: Introduce modify_xen_mappings
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (2 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 03/18] arm/x86: change [modify, destroy]_xen_mappings to return error Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 05/18] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
                   ` (14 subsequent siblings)
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Konrad Rzeszutek Wilk

Which is only used by Livepatch code. The purpose behind
this call is to modify the page table entries flags.

Specifically the .ro and .nx flags. The current mechanism
puts cache attributes in the flags and the .ro and .nx are
locked down and assumed to be .ro=0, nx=1.

Livepatch needs .nx=0 and also .ro to be set to 1.

We introduce a new 'flags' where various bits determine
whether .ro and .nx bits are set or cleared. We can't use
an enum as the function prototype would diverge from x86.

Reviewed-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

--
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
[Added as he wrote the x86 modify_xen_mappings version]

RFC: First submission.
v1: Add #defines to make it simpler to understand the bit-shifting.
v2: Add sanity checks on ro=0 and nx=0 never occuring (executable
    read/write code).
    Piggyback on "x86: change modify_xen_mappings to return error"
    to be able to return an error value.
v3: Added Julien's Reviewed-by
    s/PTE/hypervisor PTE/
---
 xen/arch/arm/mm.c          | 27 ++++++++++++++++++++++++---
 xen/include/asm-arm/page.h | 11 +++++++++++
 2 files changed, 35 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 7ae9f63..07e2037 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -836,6 +836,7 @@ static int create_xen_table(lpae_t *entry)
 enum xenmap_operation {
     INSERT,
     REMOVE,
+    MODIFY,
     RESERVE
 };
 
@@ -881,14 +882,28 @@ static int create_xen_entries(enum xenmap_operation op,
                 pte.pt.table = 1;
                 write_pte(&third[third_table_offset(addr)], pte);
                 break;
+            case MODIFY:
             case REMOVE:
                 if ( !third[third_table_offset(addr)].pt.valid )
                 {
-                    printk("create_xen_entries: trying to remove a non-existing mapping addr=%lx\n",
-                           addr);
+                    printk("create_xen_entries: trying to %s a non-existing mapping addr=%lx\n",
+                           op == REMOVE ? "remove" : "modify", addr);
                     return -EINVAL;
                 }
-                pte.bits = 0;
+                if ( op == REMOVE )
+                    pte.bits = 0;
+                else
+                {
+                    pte = third[third_table_offset(addr)];
+                    pte.pt.ro = PTE_RO_MASK(ai);
+                    pte.pt.xn = PTE_NX_MASK(ai);
+                    if ( !pte.pt.ro && !pte.pt.xn )
+                    {
+                        printk("create_xen_entries: Incorrect combination for addr=%lx\n",
+                               addr);
+                        return -EINVAL;
+                    }
+                }
                 write_pte(&third[third_table_offset(addr)], pte);
                 break;
             default:
@@ -922,6 +937,12 @@ int destroy_xen_mappings(unsigned long v, unsigned long e)
     return create_xen_entries(REMOVE, v, 0, (e - v) >> PAGE_SHIFT, 0);
 }
 
+int modify_xen_mappings(unsigned long s, unsigned long e, unsigned int flags)
+{
+    ASSERT((flags & (PTE_NX | PTE_RO)) == flags);
+    return create_xen_entries(MODIFY, s, 0, (e - s) >> PAGE_SHIFT, flags);
+}
+
 enum mg { mg_clear, mg_ro, mg_rw, mg_rx };
 static void set_pte_flags_on_range(const char *p, unsigned long l, enum mg mg)
 {
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 05d9f82..015ed63 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -66,6 +66,17 @@
 #define PAGE_HYPERVISOR_WC      (DEV_WC)
 
 /*
+ * Defines for changing the hypervisor PTE .ro and .nx bits. This is only to be
+ * used with modify_xen_mappings.
+ */
+#define _PTE_NX_BIT     0U
+#define _PTE_RO_BIT     1U
+#define PTE_NX          (1U << _PTE_NX_BIT)
+#define PTE_RO          (1U << _PTE_RO_BIT)
+#define PTE_NX_MASK(x)  (((x) >> _PTE_NX_BIT) & 0x1U)
+#define PTE_RO_MASK(x)  (((x) >> _PTE_RO_BIT) & 0x1U)
+
+/*
  * Stage 2 Memory Type.
  *
  * These are valid in the MemAttr[3:0] field of an LPAE stage 2 page
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 05/18] arm: poison initmem when it is freed.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (3 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 04/18] arm/mm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-16 13:31   ` Julien Grall
  2016-09-11 20:35 ` [PATCH v3 06/18] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
                   ` (13 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Konrad Rzeszutek Wilk

The current byte sequence is '0xcc' which makes sense on x86,
but on ARM it is:

cccccccc        stclgt  12, cr12, [ip], {204}   ; 0xcc

Picking something more ARM applicable such as:

efefefef        svc     0x00efefef

Creates a nice crash if one executes that code:
(XEN) CPU1: Unexpected Trap: Supervisor Call

We don't have to worry about Thumb code so this instruction
is a safe to execute.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
---
 xen/arch/arm/mm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 07e2037..0fa5623 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -995,7 +995,7 @@ void free_init_memory(void)
     paddr_t pa = virt_to_maddr(__init_begin);
     unsigned long len = __init_end - __init_begin;
     set_pte_flags_on_range(__init_begin, len, mg_rw);
-    memset(__init_begin, 0xcc, len);
+    memset(__init_begin, 0xef, len);
     set_pte_flags_on_range(__init_begin, len, mg_clear);
     init_domheap_pages(pa, pa + len);
     printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 06/18] livepatch: Initial ARM64 support.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (4 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 05/18] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-12  7:45   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 07/18] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
                   ` (12 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Konrad Rzeszutek Wilk

As compared to x86 the va of the hypervisor .text
is locked down - we cannot modify the running pagetables
to have the .ro flag unset. We borrow the same idea that
alternative patching has - which is to vmap the entire
.text region and use the alternative virtual address
for patching.

Since we are doing vmap we may fail, hence the
arch_livepatch_quiesce was changed (see "x86,arm:
Change arch_livepatch_quiesce() declaration") to return
an error value which will be bubbled in payload->rc and
provided to the user (along with messages in the ring buffer).

The livepatch virtual address space (where the new functions
are) needs to be close to the hypervisor virtual address
so that the trampoline can reach it. As such we re-use
the BOOT_RELOC_VIRT_START which is not used after bootup
(alternatively we can also use the space after the _end to
FIXMAP_ADDR(0), but that may be too small).

The ELF relocation engine at the start was coded from
the "ELF for the ARM 64-bit Architecture (AArch64)"
(http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.pdf)
but after a while of trying to write the correct bit shifting
and masking from scratch I ended up borrowing from Linux, the
'reloc_insn_imm' (Linux v4.7 arch/arm64/kernel/module.c function.
See 257cb251925f854da435cbf79b140984413871ac "arm64: Loadable modules")

And while at it - we also utilize code from Linux to construct
the right branch instruction (see "arm64/insn: introduce
aarch64_insn_gen_{nop|branch_imm}() helper functions").

In the livepatch payload loading code we tweak the #ifdef to
only exclude ARM_32. The exceptions are not part of ARM 32/64 hence
they are still behind the #ifdef.

We also expand the MAINTAINERS file to include the arm64 and arm32
platform specific livepatch file.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc  Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

RFC: Wholy cow! It works!
v1: Finished the various TODOs and reviews outlined by Julien in RFC.
v2: Call check_for_livepatch_work in leave_hypervisor_tail not in
    reset_stack_and_jump
   - Move ARM 32 components in other patch
   - Blacklist platform options in Kconfig
   - Added R_AARCH64_LDST128_ABS_LO12_NC, R_AARCH64_TSTBR14, and
     R_AARCH64_ADR_PREL_PG_HI21_NC
   - Added do_reloc and reloc_data along with overflow checks from Linux.
   - Use apply_alternatives without any #ifdef
   - Use leave_hypervisor_tail to call check_for_livepatch_work
   - Add ASSERT that isns is not AARCH64_BREAK_FAULT
   - Spun out arch_livepatch_quiesce changes in seperate patch.
   - Spun out changes to config.h (document ones) to seperate patch.
   - Move the livepatch.h to include/xen/asm-arm.
   - Add LIVEPATCH_VMAP_END and LIVEPATCH_VMAP_START in config.h
   - In arch_livepatch_secure use switch for va_type.
   - Drop the #ifndef CONFIG_ARM for .ex_table (see
     "arm/x86: Add HAS_ALTERNATIVE and HAS_EX_TABLE")
   - Piggyback on "x86: change modify_xen_mappings to return error"
      so that arch_livepatch_secure can return errors.
   - Moves comment about branch predictor out of this patch.
v3: Fix StyleGuide violations (switch statements)
   - Fix incorrect cast of addr when reverting.
   - Drop old_ptr variable.
   - Use bool_t values instead of numbers.
   - Sprinkle \n as requested by Julien.
   - In arch_livepatch_quiesce use 1U instead of 1.
   - Use C99 #defines for [U,]INT[16,32]_[MIN,MAX] instead of Linux
     kernel ones ([S,U][16,32]_[MIN,MAX]).
   - Include the ELF relocations for R_AARCH64_[ABS,PRE]16, and
     all the various groupings for R_AARCH64_MOVW_[UABS,SABS,PREL]_*
     family.
   - Redo the NOP patching to use more of the opaque size (so up to 7
     instructions in one go).
   - Drop the cpu_to_le32 macros as they are not needed (and can allow
     use to share more code with ARM32).
   - Flush out func->old_addr instead of vmap pointer when reverting.
---
 MAINTAINERS                     |   1 +
 xen/arch/arm/Makefile           |  13 +-
 xen/arch/arm/arm32/Makefile     |   1 +
 xen/arch/arm/arm32/livepatch.c  |  38 ++++
 xen/arch/arm/arm64/Makefile     |   1 +
 xen/arch/arm/arm64/livepatch.c  | 477 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/domain.c           |   6 +
 xen/arch/arm/livepatch.c        | 103 +++++++--
 xen/arch/arm/traps.c            |   6 +
 xen/common/Kconfig              |   2 +-
 xen/include/asm-arm/config.h    |   5 +
 xen/include/asm-arm/livepatch.h |  28 +++
 xen/include/xen/elfstructs.h    |  57 ++++-
 xen/include/xen/types.h         |   9 +
 14 files changed, 722 insertions(+), 25 deletions(-)
 create mode 100644 xen/arch/arm/arm32/livepatch.c
 create mode 100644 xen/arch/arm/arm64/livepatch.c
 create mode 100644 xen/include/asm-arm/livepatch.h

diff --git a/MAINTAINERS b/MAINTAINERS
index 97720a8..ae0b6bc 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -269,6 +269,7 @@ S:  Supported
 F:  docs/misc/livepatch.markdown
 F:  tools/misc/xen-livepatch.c
 F:  xen/arch/*/livepatch*
+F:  xen/arch/*/*/livepatch*
 F:  xen/common/livepatch*
 F:  xen/include/xen/livepatch*
 
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 64fdf41..c3ce5a3 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -59,6 +59,15 @@ ALL_OBJS := $(TARGET_SUBARCH)/head.o $(ALL_OBJS)
 
 DEPS += $(TARGET_SUBARCH)/.head.o.d
 
+ifdef CONFIG_LIVEPATCH
+all_symbols = --all-symbols
+ifdef CONFIG_FAST_SYMBOL_LOOKUP
+all_symbols = --all-symbols --sort-by-name
+endif
+else
+all_symbols =
+endif
+
 $(TARGET): $(TARGET)-syms $(TARGET).axf
 	$(OBJCOPY) -O binary -S $< $@
 ifeq ($(CONFIG_ARM_64),y)
@@ -94,12 +103,12 @@ $(TARGET)-syms: prelink.o xen.lds $(BASEDIR)/common/symbols-dummy.o
 	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
 	    $(BASEDIR)/common/symbols-dummy.o -o $(@D)/.$(@F).0
 	$(NM) -pa --format=sysv $(@D)/.$(@F).0 \
-		| $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).0.S
+		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).0.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).0.o
 	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o \
 	    $(@D)/.$(@F).0.o -o $(@D)/.$(@F).1
 	$(NM) -pa --format=sysv $(@D)/.$(@F).1 \
-		| $(BASEDIR)/tools/symbols --sysv --sort >$(@D)/.$(@F).1.S
+		| $(BASEDIR)/tools/symbols $(all_symbols) --sysv --sort >$(@D)/.$(@F).1.S
 	$(MAKE) -f $(BASEDIR)/Rules.mk $(@D)/.$(@F).1.o
 	$(LD) $(LDFLAGS) -T xen.lds -N prelink.o $(build_id_linker) \
 	    $(@D)/.$(@F).1.o -o $@
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index b20db64..4395693 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -4,6 +4,7 @@ obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
 obj-y += entry.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-y += proc-v7.o proc-caxx.o
 obj-y += smpboot.o
 obj-y += traps.o
diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
new file mode 100644
index 0000000..c33b68d
--- /dev/null
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -0,0 +1,38 @@
+/*
+ *  Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include <xen/errno.h>
+#include <xen/lib.h>
+#include <xen/livepatch_elf.h>
+#include <xen/livepatch.h>
+
+void arch_livepatch_apply_jmp(struct livepatch_func *func)
+{
+}
+
+void arch_livepatch_revert_jmp(const struct livepatch_func *func)
+{
+}
+
+int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
+{
+    return -EOPNOTSUPP;
+}
+
+int arch_livepatch_perform_rela(struct livepatch_elf *elf,
+                                const struct livepatch_elf_sec *base,
+                                const struct livepatch_elf_sec *rela)
+{
+    return -ENOSYS;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c1fa43f..149b6b3 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -6,6 +6,7 @@ obj-y += domctl.o
 obj-y += domain.o
 obj-y += entry.o
 obj-y += insn.o
+obj-$(CONFIG_LIVEPATCH) += livepatch.o
 obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
new file mode 100644
index 0000000..ecaf3f6
--- /dev/null
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -0,0 +1,477 @@
+/*
+ *  Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ */
+
+#include <xen/bitops.h>
+#include <xen/errno.h>
+#include <xen/lib.h>
+#include <xen/livepatch_elf.h>
+#include <xen/livepatch.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+
+#include <asm/bitops.h>
+#include <asm/byteorder.h>
+#include <asm/insn.h>
+#include <asm/livepatch.h>
+
+void arch_livepatch_apply_jmp(struct livepatch_func *func)
+{
+    uint32_t insn;
+    uint32_t *new_ptr;
+    unsigned int i, len;
+
+    BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
+    BUILD_BUG_ON(PATCH_INSN_SIZE != sizeof(insn));
+
+    ASSERT(vmap_of_xen_text);
+
+    len = arch_livepatch_insn_len(func);
+    if ( !len )
+        return;
+
+    /* Save old ones. */
+    memcpy(func->opaque, func->old_addr, len);
+
+    if ( func->new_addr )
+        insn = aarch64_insn_gen_branch_imm((unsigned long)func->old_addr,
+                                           (unsigned long)func->new_addr,
+                                           AARCH64_INSN_BRANCH_NOLINK);
+    else
+        insn = aarch64_insn_gen_nop();
+
+    ASSERT(insn != AARCH64_BREAK_FAULT);
+
+    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    len = len / sizeof(uint32_t);
+
+    /* PATCH! */
+    for ( i = 0; i < len; i++ )
+        *(new_ptr + i) = insn;
+
+    clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr) * len);
+}
+
+void arch_livepatch_revert_jmp(const struct livepatch_func *func)
+{
+    uint32_t *new_ptr;
+    unsigned int i, len;
+
+    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    len = arch_livepatch_insn_len(func) / sizeof(uint32_t);
+    for ( i = 0; i < len; i++ )
+    {
+        uint32_t insn;
+
+        memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), PATCH_INSN_SIZE);
+        /* PATCH! */
+        *(new_ptr + i) = insn;
+    }
+
+    clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
+}
+
+int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
+{
+    const Elf_Ehdr *hdr = elf->hdr;
+
+    if ( hdr->e_machine != EM_AARCH64 ||
+         hdr->e_ident[EI_CLASS] != ELFCLASS64 )
+    {
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF Machine type!\n",
+                elf->name);
+        return -EOPNOTSUPP;
+    }
+
+    return 0;
+}
+
+enum aarch64_reloc_op {
+    RELOC_OP_NONE,
+    RELOC_OP_ABS,
+    RELOC_OP_PREL,
+    RELOC_OP_PAGE,
+};
+
+static u64 do_reloc(enum aarch64_reloc_op reloc_op, void *place, u64 val)
+{
+    switch ( reloc_op )
+    {
+    case RELOC_OP_ABS:
+        return val;
+
+    case RELOC_OP_PREL:
+        return val - (u64)place;
+
+    case RELOC_OP_PAGE:
+        return (val & ~0xfff) - ((u64)place & ~0xfff);
+
+    case RELOC_OP_NONE:
+        return 0;
+
+    }
+
+    dprintk(XENLOG_DEBUG, LIVEPATCH "do_reloc: unknown relocation operation %d\n", reloc_op);
+
+    return 0;
+}
+
+static int reloc_data(enum aarch64_reloc_op op, void *place, u64 val, int len)
+{
+    s64 sval = do_reloc(op, place, val);
+
+    switch ( len )
+    {
+    case 16:
+        *(s16 *)place = sval;
+        if ( sval < INT16_MIN || sval > UINT16_MAX )
+	        return -EOVERFLOW;
+        break;
+
+    case 32:
+        *(s32 *)place = sval;
+        if ( sval < INT32_MIN || sval > UINT32_MAX )
+	        return -EOVERFLOW;
+        break;
+
+    case 64:
+        *(s64 *)place = sval;
+        break;
+
+    default:
+        dprintk(XENLOG_DEBUG, LIVEPATCH "Invalid length (%d) for data relocation\n", len);
+        return 0;
+    }
+
+    return 0;
+}
+
+enum aarch64_insn_movw_imm_type {
+    AARCH64_INSN_IMM_MOVNZ,
+    AARCH64_INSN_IMM_MOVKZ,
+};
+
+static int reloc_insn_movw(enum aarch64_reloc_op op, void *dest, u64 val,
+                           int lsb, enum aarch64_insn_movw_imm_type imm_type)
+{
+    u64 imm;
+    s64 sval;
+    u32 insn = *(u32 *)dest;
+
+    sval = do_reloc(op, dest, val);
+    imm = sval >> lsb;
+
+    if ( imm_type == AARCH64_INSN_IMM_MOVNZ )
+    {
+        /*
+         * For signed MOVW relocations, we have to manipulate the
+         * instruction encoding depending on whether or not the
+         * immediate is less than zero.
+         */
+        insn &= ~(3 << 29);
+        if ( sval >= 0 )
+        {
+            /* >=0: Set the instruction to MOVZ (opcode 10b). */
+            insn |= 2 << 29;
+        }
+        else
+        {
+            /*
+             * <0: Set the instruction to MOVN (opcode 00b).
+             *     Since we've masked the opcode already, we
+             *     don't need to do anything other than
+             *     inverting the new immediate field.
+             */
+            imm = ~imm;
+        }
+    }
+
+    /* Update the instruction with the new encoding. */
+    insn = aarch64_insn_encode_immediate(AARCH64_INSN_IMM_16, insn, imm);
+    *(u32 *)dest = insn;
+
+    if ( imm > UINT16_MAX )
+        return -EOVERFLOW;
+
+    return 0;
+}
+
+static int reloc_insn_imm(enum aarch64_reloc_op op, void *dest, u64 val,
+                          int lsb, int len, enum aarch64_insn_imm_type imm_type)
+{
+    u64 imm, imm_mask;
+    s64 sval;
+    u32 insn = *(u32 *)dest;
+
+    /* Calculate the relocation value. */
+    sval = do_reloc(op, dest, val);
+    sval >>= lsb;
+
+    /* Extract the value bits and shift them to bit 0. */
+    imm_mask = (BIT(lsb + len) - 1) >> lsb;
+    imm = sval & imm_mask;
+
+    /* Update the instruction's immediate field. */
+    insn = aarch64_insn_encode_immediate(imm_type, insn, imm);
+    *(u32 *)dest = insn;
+
+    /*
+     * Extract the upper value bits (including the sign bit) and
+     * shift them to bit 0.
+     */
+    sval = (s64)(sval & ~(imm_mask >> 1)) >> (len - 1);
+
+    /*
+     * Overflow has occurred if the upper bits are not all equal to
+     * the sign bit of the value.
+     */
+    if ( (u64)(sval + 1) >= 2 )
+        return -EOVERFLOW;
+    return 0;
+}
+
+int arch_livepatch_perform_rela(struct livepatch_elf *elf,
+                                const struct livepatch_elf_sec *base,
+                                const struct livepatch_elf_sec *rela)
+{
+    const Elf_RelA *r;
+    unsigned int symndx, i;
+    uint64_t val;
+    void *dest;
+    bool_t overflow_check;
+
+    for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
+    {
+        int ovf = 0;
+
+        r = rela->data + i * rela->sec->sh_entsize;
+
+        symndx = ELF64_R_SYM(r->r_info);
+
+        if ( symndx > elf->nsym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation wants symbol@%u which is past end!\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
+
+        dest = base->load_addr + r->r_offset; /* P */
+        val = elf->sym[symndx].sym->st_value +  r->r_addend; /* S+A */
+
+        overflow_check = true;
+
+        /* ARM64 operations at minimum are always 32-bit. */
+        if ( r->r_offset >= base->sec->sh_size ||
+            (r->r_offset + sizeof(uint32_t)) > base->sec->sh_size )
+            goto bad_offset;
+
+        switch ( ELF64_R_TYPE(r->r_info) )
+        {
+        /* Data */
+        case R_AARCH64_ABS64:
+            if ( r->r_offset + sizeof(uint64_t) > base->sec->sh_size )
+                goto bad_offset;
+            overflow_check = false;
+            ovf = reloc_data(RELOC_OP_ABS, dest, val, 64);
+            break;
+
+        case R_AARCH64_ABS32:
+            ovf = reloc_data(RELOC_OP_ABS, dest, val, 32);
+            break;
+
+        case R_AARCH64_ABS16:
+            ovf = reloc_data(RELOC_OP_ABS, dest, val, 16);
+            break;
+
+        case R_AARCH64_PREL64:
+            if ( r->r_offset + sizeof(uint64_t) > base->sec->sh_size )
+                goto bad_offset;
+            overflow_check = false;
+            ovf = reloc_data(RELOC_OP_PREL, dest, val, 64);
+            break;
+
+        case R_AARCH64_PREL32:
+            ovf = reloc_data(RELOC_OP_PREL, dest, val, 32);
+            break;
+
+        case R_AARCH64_PREL16:
+            ovf = reloc_data(RELOC_OP_PREL, dest, val, 16);
+            break;
+
+        /* MOVW instruction relocations. */
+        case R_AARCH64_MOVW_UABS_G0_NC:
+            overflow_check = false;
+
+        case R_AARCH64_MOVW_UABS_G0:
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 0,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_UABS_G1_NC:
+            overflow_check = false;
+
+        case R_AARCH64_MOVW_UABS_G1:
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 16,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_UABS_G2_NC:
+            overflow_check = false;
+
+        case R_AARCH64_MOVW_UABS_G2:
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 32,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_UABS_G3:
+            /* We're using the top bits so we can't overflow. */
+            overflow_check = false;
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 48,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_SABS_G0:
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 0,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        case R_AARCH64_MOVW_SABS_G1:
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 16,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        case R_AARCH64_MOVW_SABS_G2:
+            ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 32,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G0_NC:
+            overflow_check = false;
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 0,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G0:
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 0,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G1_NC:
+            overflow_check = false;
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 16,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G1:
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 16,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G2_NC:
+            overflow_check = false;
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 32,
+                                  AARCH64_INSN_IMM_MOVKZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G2:
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 32,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        case R_AARCH64_MOVW_PREL_G3:
+            /* We're using the top bits so we can't overflow. */
+            overflow_check = false;
+            ovf = reloc_insn_movw(RELOC_OP_PREL, dest, val, 48,
+                                  AARCH64_INSN_IMM_MOVNZ);
+            break;
+
+        /* Instructions. */
+        case R_AARCH64_ADR_PREL_LO21:
+            ovf = reloc_insn_imm(RELOC_OP_PREL, dest, val, 0, 21,
+                                 AARCH64_INSN_IMM_ADR);
+            break;
+
+        case R_AARCH64_ADR_PREL_PG_HI21_NC:
+            overflow_check = false;
+        case R_AARCH64_ADR_PREL_PG_HI21:
+            ovf = reloc_insn_imm(RELOC_OP_PAGE, dest, val, 12, 21,
+                                 AARCH64_INSN_IMM_ADR);
+            break;
+
+        case R_AARCH64_LDST8_ABS_LO12_NC:
+        case R_AARCH64_ADD_ABS_LO12_NC:
+            overflow_check = false;
+            ovf = reloc_insn_imm(RELOC_OP_ABS, dest, val, 0, 12,
+                                 AARCH64_INSN_IMM_12);
+            break;
+
+        case R_AARCH64_LDST16_ABS_LO12_NC:
+            overflow_check = false;
+            ovf = reloc_insn_imm(RELOC_OP_ABS, dest, val, 1, 11,
+                                 AARCH64_INSN_IMM_12);
+            break;
+
+        case R_AARCH64_LDST32_ABS_LO12_NC:
+            overflow_check = false;
+            ovf = reloc_insn_imm(RELOC_OP_ABS, dest, val, 2, 10,
+                                 AARCH64_INSN_IMM_12);
+            break;
+
+        case R_AARCH64_LDST64_ABS_LO12_NC:
+            overflow_check = false;
+            ovf = reloc_insn_imm(RELOC_OP_ABS, dest, val, 3, 9,
+                                 AARCH64_INSN_IMM_12);
+            break;
+
+        case R_AARCH64_LDST128_ABS_LO12_NC:
+            overflow_check = false;
+            ovf = reloc_insn_imm(RELOC_OP_ABS, dest, val, 4, 8,
+                                 AARCH64_INSN_IMM_12);
+            break;
+
+        case R_AARCH64_TSTBR14:
+            ovf = reloc_insn_imm(RELOC_OP_PREL, dest, val, 2, 19,
+                                 AARCH64_INSN_IMM_14);
+            break;
+
+        case R_AARCH64_CONDBR19:
+            ovf = reloc_insn_imm(RELOC_OP_PREL, dest, val, 2, 19,
+                                 AARCH64_INSN_IMM_19);
+            break;
+
+        case R_AARCH64_JUMP26:
+        case R_AARCH64_CALL26:
+            ovf = reloc_insn_imm(RELOC_OP_PREL, dest, val, 2, 26,
+                                 AARCH64_INSN_IMM_26);
+            break;
+
+        default:
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Unhandled relocation %lu\n",
+                    elf->name, ELF64_R_TYPE(r->r_info));
+            return -EOPNOTSUPP;
+        }
+
+        if ( overflow_check && ovf == -EOVERFLOW )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Overflow in relocation %u in %s for %s!\n",
+                    elf->name, i, rela->name, base->name);
+            return ovf;
+        }
+    }
+    return 0;
+
+ bad_offset:
+    dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative relocation offset is past %s section!\n",
+            elf->name, base->name);
+    return -EINVAL;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
index 20bb2ba..607ee59 100644
--- a/xen/arch/arm/domain.c
+++ b/xen/arch/arm/domain.c
@@ -13,6 +13,7 @@
 #include <xen/hypercall.h>
 #include <xen/init.h>
 #include <xen/lib.h>
+#include <xen/livepatch.h>
 #include <xen/sched.h>
 #include <xen/softirq.h>
 #include <xen/wait.h>
@@ -55,6 +56,11 @@ void idle_loop(void)
 
         do_tasklet();
         do_softirq();
+        /*
+         * We MUST be last (or before dsb, wfi). Otherwise after we get the
+         * softirq we would execute dsb,wfi (and sleep) and not patch.
+         */
+        check_for_livepatch_work();
     }
 }
 
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index 755f596..7a3e2f7 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -6,44 +6,82 @@
 #include <xen/lib.h>
 #include <xen/livepatch_elf.h>
 #include <xen/livepatch.h>
+#include <xen/vmap.h>
+
+#include <asm/livepatch.h>
+#include <asm/mm.h>
+
+void *vmap_of_xen_text;
 
 int arch_livepatch_quiesce(void)
 {
-    return -ENOSYS;
+    mfn_t text_mfn;
+    unsigned int text_order;
+
+    if ( vmap_of_xen_text )
+        return -EINVAL;
+
+    text_mfn = _mfn(virt_to_mfn(_start));
+    text_order = get_order_from_bytes(_end - _start);
+
+    /*
+     * The text section is read-only. So re-map Xen to be able to patch
+     * the code.
+     */
+    vmap_of_xen_text = __vmap(&text_mfn, 1U << text_order, 1, 1, PAGE_HYPERVISOR,
+                              VMAP_DEFAULT);
+
+    if ( !vmap_of_xen_text )
+    {
+        printk(XENLOG_ERR LIVEPATCH "Failed to setup vmap of hypervisor! (order=%u)\n",
+               text_order);
+        return -ENOMEM;
+    }
+
+    return 0;
 }
 
 void arch_livepatch_revive(void)
 {
+    /*
+     * Nuke the instruction cache. Data cache has been cleaned before in
+     * arch_livepatch_apply_jmp.
+     */
+    invalidate_icache();
+
+    if ( vmap_of_xen_text )
+        vunmap(vmap_of_xen_text);
+
+    vmap_of_xen_text = NULL;
 }
 
 int arch_livepatch_verify_func(const struct livepatch_func *func)
 {
-    return -ENOSYS;
-}
+    /* If NOPing only do up to maximum amount we can put in the ->opaque. */
+    if ( !func->new_addr && func->new_size > sizeof(func->opaque) &&
+         func->new_size % PATCH_INSN_SIZE )
+        return -EOPNOTSUPP;
 
-void arch_livepatch_apply_jmp(struct livepatch_func *func)
-{
-}
+    if ( func->old_size < PATCH_INSN_SIZE )
+        return -EINVAL;
 
-void arch_livepatch_revert_jmp(const struct livepatch_func *func)
-{
+    return 0;
 }
 
 void arch_livepatch_post_action(void)
 {
+    /* arch_livepatch_revive has nuked the instruction cache. */
 }
 
 void arch_livepatch_mask(void)
 {
+    /* Mask System Error (SError) */
+    local_abort_disable();
 }
 
 void arch_livepatch_unmask(void)
 {
-}
-
-int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
-{
-    return -ENOSYS;
+    local_abort_enable();
 }
 
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
@@ -53,20 +91,43 @@ int arch_livepatch_perform_rel(struct livepatch_elf *elf,
     return -ENOSYS;
 }
 
-int arch_livepatch_perform_rela(struct livepatch_elf *elf,
-                                const struct livepatch_elf_sec *base,
-                                const struct livepatch_elf_sec *rela)
-{
-    return -ENOSYS;
-}
-
 int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
 {
-    return -ENOSYS;
+    unsigned long start = (unsigned long)va;
+    unsigned int flags = 0;
+
+    ASSERT(va);
+    ASSERT(pages);
+
+    switch ( type )
+    {
+    case LIVEPATCH_VA_RX:
+        flags = PTE_RO; /* R set, NX clear */
+        break;
+
+    case LIVEPATCH_VA_RW:
+        flags = PTE_NX; /* R clear, NX set */
+        break;
+
+    case LIVEPATCH_VA_RO:
+        flags = PTE_NX | PTE_RO; /* R set, NX set */
+        break;
+
+    default:
+        return -EINVAL;
+    }
+
+    return modify_xen_mappings(start, start + pages * PAGE_SIZE, flags);
 }
 
 void __init arch_livepatch_init(void)
 {
+    void *start, *end;
+
+    start = (void *)LIVEPATCH_VMAP_START;
+    end = (void *)LIVEPATCH_VMAP_END;
+
+    vm_init_type(VMAP_XEN, start, end);
 }
 
 /*
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 39a05fd..cd6c222 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -24,6 +24,7 @@
 #include <xen/symbols.h>
 #include <xen/irq.h>
 #include <xen/lib.h>
+#include <xen/livepatch.h>
 #include <xen/mm.h>
 #include <xen/errno.h>
 #include <xen/hypercall.h>
@@ -2689,6 +2690,11 @@ asmlinkage void leave_hypervisor_tail(void)
         }
         local_irq_enable();
         do_softirq();
+        /*
+         * Must be the last one - as the IPI will trigger us to come here
+         * and we want to patch the hypervisor with almost no stack.
+         */
+        check_for_livepatch_work();
     }
 }
 
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 81e0017..0f26027 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -217,7 +217,7 @@ config CRYPTO
 config LIVEPATCH
 	bool "Live patching support (TECH PREVIEW)"
 	default n
-	depends on X86 && HAS_BUILD_ID = "y"
+	depends on !ARM_32 && HAS_BUILD_ID = "y"
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily
diff --git a/xen/include/asm-arm/config.h b/xen/include/asm-arm/config.h
index 6772555..ba61f65 100644
--- a/xen/include/asm-arm/config.h
+++ b/xen/include/asm-arm/config.h
@@ -80,6 +80,7 @@
  *   4M -   6M   Fixmap: special-purpose 4K mapping slots
  *   6M -   8M   Early boot mapping of FDT
  *   8M -  10M   Early relocation address (used when relocating Xen)
+ *               and later for livepatch vmap (if compiled in)
  *
  * ARM32 layout:
  *   0  -  10M   <COMMON>
@@ -113,6 +114,10 @@
 #define FIXMAP_ADDR(n)        (_AT(vaddr_t,0x00400000) + (n) * PAGE_SIZE)
 #define BOOT_FDT_VIRT_START    _AT(vaddr_t,0x00600000)
 #define BOOT_RELOC_VIRT_START  _AT(vaddr_t,0x00800000)
+#ifdef CONFIG_LIVEPATCH
+#define LIVEPATCH_VMAP_START   _AT(vaddr_t,0x00800000)
+#define LIVEPATCH_VMAP_END     (LIVEPATCH_VMAP_START + MB(2))
+#endif
 
 #define HYPERVISOR_VIRT_START  XEN_VIRT_START
 
diff --git a/xen/include/asm-arm/livepatch.h b/xen/include/asm-arm/livepatch.h
new file mode 100644
index 0000000..8c8d625
--- /dev/null
+++ b/xen/include/asm-arm/livepatch.h
@@ -0,0 +1,28 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#ifndef __XEN_ARM_LIVEPATCH_H__
+#define __XEN_ARM_LIVEPATCH_H__
+
+/* On ARM32,64 instructions are always 4 bytes long. */
+#define PATCH_INSN_SIZE 4
+
+/*
+ * The va of the hypervisor .text region. We need this as the
+ * normal va are write protected.
+ */
+extern void *vmap_of_xen_text;
+
+#endif /* __XEN_ARM_LIVEPATCH_H__ */
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index 5f2082e..7329987 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -177,6 +177,7 @@ typedef struct {
 #define EM_IA_64	50		/* Intel Merced */
 #define EM_X86_64	62		/* AMD x86-64 architecture */
 #define EM_VAX		75		/* DEC VAX */
+#define EM_AARCH64	183		/* ARM 64-bit */
 
 /* Version */
 #define EV_NONE		0		/* Invalid */
@@ -353,12 +354,66 @@ typedef struct {
 #define	ELF64_R_TYPE(info)	((info) & 0xFFFFFFFF)
 #define ELF64_R_INFO(s,t) 	(((s) << 32) + (u_int32_t)(t))
 
-/* x86-64 relocation types. We list only the ones Live Patch implements. */
+/*
+ * Relocation types for x86_64 and ARM 64. We list only the ones Live Patch
+ * implements.
+ */
 #define R_X86_64_NONE		0	/* No reloc */
 #define R_X86_64_64	    	1	/* Direct 64 bit  */
 #define R_X86_64_PC32		2	/* PC relative 32 bit signed */
 #define R_X86_64_PLT32		4	/* 32 bit PLT address */
 
+/*
+ * S - address of symbol.
+ * A - addend for relocation (r_addend)
+ * P - address of the dest being relocated (derieved from r_offset)
+ * NC -  No check for overflow.
+ *
+ * The defines also use _PREL for PC-relative address, and _NC is No Check.
+ */
+#define R_AARCH64_ABS64			257 /* Direct 64 bit. S+A, NC*/
+#define R_AARCH64_ABS32			258 /* Direct 32 bit. S+A */
+#define R_AARCH64_ABS16			259 /* Direct 16 bit, S+A */
+#define R_AARCH64_PREL64		260 /* S+A-P, NC */
+#define R_AARCH64_PREL32		261 /* S+A-P */
+#define R_AARCH64_PREL16		262 /* S+A-P */
+
+/* Instructions. */
+#define R_AARCH64_MOVW_UABS_G0		263
+#define R_AARCH64_MOVW_UABS_G0_NC	264
+#define R_AARCH64_MOVW_UABS_G1		265
+#define R_AARCH64_MOVW_UABS_G1_NC	266
+#define R_AARCH64_MOVW_UABS_G2		267
+#define R_AARCH64_MOVW_UABS_G2_NC	268
+#define R_AARCH64_MOVW_UABS_G3		269
+
+#define R_AARCH64_MOVW_SABS_G0		270
+#define R_AARCH64_MOVW_SABS_G1		271
+#define R_AARCH64_MOVW_SABS_G2		272
+
+#define R_AARCH64_ADR_PREL_LO21		274 /* ADR imm, [20:0]. S+A-P */
+#define R_AARCH64_ADR_PREL_PG_HI21	275 /* ADRP imm, [32:12]. Page(S+A) - Page(P).*/
+#define R_AARCH64_ADR_PREL_PG_HI21_NC	276
+#define R_AARCH64_ADD_ABS_LO12_NC	277 /* ADD imm. [11:0]. S+A, NC */
+
+#define R_AARCH64_TSTBR14		279
+#define R_AARCH64_CONDBR19		280 /* Bits 20:2, S+A-P */
+#define R_AARCH64_JUMP26		282 /* Bits 27:2, S+A-P */
+#define R_AARCH64_CALL26		283 /* Bits 27:2, S+A-P */
+#define R_AARCH64_LDST16_ABS_LO12_NC	284 /* LD/ST to bits 11:1, S+A, NC */
+#define R_AARCH64_LDST32_ABS_LO12_NC	285 /* LD/ST to bits 11:2, S+A, NC */
+#define R_AARCH64_LDST64_ABS_LO12_NC	286 /* LD/ST to bits 11:3, S+A, NC */
+#define R_AARCH64_LDST8_ABS_LO12_NC	278 /* LD/ST to bits 11:0, S+A, NC */
+#define R_AARCH64_LDST128_ABS_LO12_NC	299
+
+#define R_AARCH64_MOVW_PREL_G0		287
+#define R_AARCH64_MOVW_PREL_G0_NC	288
+#define R_AARCH64_MOVW_PREL_G1		289
+#define R_AARCH64_MOVW_PREL_G1_NC	290
+#define R_AARCH64_MOVW_PREL_G2		291
+#define R_AARCH64_MOVW_PREL_G2_NC	292
+#define R_AARCH64_MOVW_PREL_G3		293
+
 /* Program Header */
 typedef struct {
 	Elf32_Word	p_type;		/* segment type */
diff --git a/xen/include/xen/types.h b/xen/include/xen/types.h
index 7bdc83b..c79c353 100644
--- a/xen/include/xen/types.h
+++ b/xen/include/xen/types.h
@@ -14,6 +14,15 @@
 #define NULL ((void*)0)
 #endif
 
+#define INT16_MIN       (-32767-1)
+#define INT32_MIN       (-2147483647-1)
+
+#define INT16_MAX       (32767)
+#define INT32_MAX       (2147483647)
+
+#define UINT16_MAX      (65535)
+#define UINT32_MAX      (4294967295U)
+
 #define INT_MAX         ((int)(~0U>>1))
 #define INT_MIN         (-INT_MAX - 1)
 #define UINT_MAX        (~0U)
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 07/18] livepatch: ARM/x86: Check displacement of old_addr and new_addr
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (5 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 06/18] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-12 15:36   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x] Konrad Rzeszutek Wilk
                   ` (11 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

If the distance is too great we are in trouble - as our relocation
distance can surely be clipped, or still have a valid width - but
cause an overflow of distance.

On various architectures the maximum displacement for a unconditional
branch/jump varies. ARM32 is +/- 32MB, ARM64 is +/- 128MB while x86
for 32-bit relocations is +/- 2G.

Note: On x86 we could use the 64-bit jmpq instruction which
would provide much bigger displacement to do a jump, but we would
still have issues with the new function not being able to reach
any of the old functions (as all the relocations would assume 32-bit
displacement).

On ARM the conditional branch supports even a smaller displacement
but fortunatly we are not using that.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>

v3: New submission.
---
 docs/misc/livepatch.markdown    | 14 +++++++++++++-
 xen/arch/arm/arm64/livepatch.c  |  1 +
 xen/arch/x86/livepatch.c        |  2 +-
 xen/common/livepatch.c          |  4 ++++
 xen/include/asm-arm/livepatch.h | 11 +++++++++++
 xen/include/asm-x86/livepatch.h |  3 +++
 xen/include/xen/livepatch.h     | 22 +++++++++++++++++++---
 7 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 53b5371..52c281d 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -1099,7 +1099,7 @@ can be changed during payload application.
 The hypervisor should verify that the in-place patching would fit within
 the code or data.
 
-### Trampoline (e9 opcode)
+### Trampoline (e9 opcode), x86
 
 The e9 opcode used for jmpq uses a 32-bit signed displacement. That means
 we are limited to up to 2GB of virtual address to place the new code
@@ -1133,3 +1133,15 @@ that in the hypervisor is advised.
 The tool for generating payloads currently does perform a compile-time
 check to ensure that the function to be replaced is large enough.
 
+The hypervisor also checks the displacement during loading of the payload.
+
+#### Trampoline (ea opcode), ARM
+
+The 0xea000000 instruction (with proper offset) is used for an unconditional
+branch to the new code. This means we are limited on ARM32 to +/- 32MB
+displacement and on ARM64 to +/- 128MB displacement.
+
+The new code is placed in the 8M - 10M virtual address space while the
+Xen code is in 2M - 4M. That gives us enough space.
+
+The hypervisor also checks the displacement during loading of the payload.
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index ecaf3f6..f072671 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -40,6 +40,7 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func)
     else
         insn = aarch64_insn_gen_nop();
 
+    /* Verified in arch_livepatch_verify_distance. */
     ASSERT(insn != AARCH64_BREAK_FAULT);
 
     new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index c4d4b4d..cf3eb22 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -44,7 +44,7 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func)
 {
     uint8_t *old_ptr;
     uint8_t insn[sizeof(func->opaque)];
-    size_t len;
+    unsigned int len;
 
     old_ptr = func->old_addr;
     len = arch_livepatch_insn_len(func);
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 5baa418..4f156c2 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -530,6 +530,10 @@ static int prepare_payload(struct payload *payload,
         rc = resolve_old_address(f, elf);
         if ( rc )
             return rc;
+
+        rc = arch_livepatch_verify_distance(f);
+        if ( rc )
+            return rc;
     }
 
     sec = livepatch_elf_sec_by_name(elf, ".livepatch.hooks.load");
diff --git a/xen/include/asm-arm/livepatch.h b/xen/include/asm-arm/livepatch.h
index 8c8d625..a632cfa 100644
--- a/xen/include/asm-arm/livepatch.h
+++ b/xen/include/asm-arm/livepatch.h
@@ -6,6 +6,8 @@
 #ifndef __XEN_ARM_LIVEPATCH_H__
 #define __XEN_ARM_LIVEPATCH_H__
 
+#include <xen/sizes.h> /* For SZ_* macros. */
+
 /* On ARM32,64 instructions are always 4 bytes long. */
 #define PATCH_INSN_SIZE 4
 
@@ -15,6 +17,15 @@
  */
 extern void *vmap_of_xen_text;
 
+/* These ranges are only for unconditional branches. */
+#ifdef CONFIG_ARM_32
+/* ARM32: A4.3 IN ARM DDI 0406C.j -  we are using only ARM instructions in Xen.*/
+#define LIVEPATCH_ARCH_RANGE SZ_32M
+#else
+/* ARM64: C1.3.2 in ARM DDI 0487A.j */
+#define LIVEPATCH_ARCH_RANGE SZ_128M
+#endif
+
 #endif /* __XEN_ARM_LIVEPATCH_H__ */
 
 /*
diff --git a/xen/include/asm-x86/livepatch.h b/xen/include/asm-x86/livepatch.h
index 63ea079..0cae242 100644
--- a/xen/include/asm-x86/livepatch.h
+++ b/xen/include/asm-x86/livepatch.h
@@ -6,7 +6,10 @@
 #ifndef __XEN_X86_LIVEPATCH_H__
 #define __XEN_X86_LIVEPATCH_H__
 
+#include <xen/sizes.h> /* For SZ_* macros. */
+
 #define PATCH_INSN_SIZE 5
+#define LIVEPATCH_ARCH_RANGE SZ_2G
 
 #endif /* __XEN_X86_LIVEPATCH_H__ */
 
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 8258e02..904785a 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -12,6 +12,7 @@ struct livepatch_elf_sym;
 struct xen_sysctl_livepatch_op;
 
 #include <xen/elfstructs.h>
+#include <xen/errno.h> /* For -ENOSYS or -EOVERFLOW */
 #ifdef CONFIG_LIVEPATCH
 
 /*
@@ -66,16 +67,32 @@ int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type types
 void arch_livepatch_init(void);
 
 #include <public/sysctl.h> /* For struct livepatch_func. */
-#include <asm/livepatch.h> /* For PATCH_INSN_SIZE. */
+#include <asm/livepatch.h> /* For LIVEPATCH_ARCH_RANGE and PATCH_INSN_SIZE */
 int arch_livepatch_verify_func(const struct livepatch_func *func);
 
-static inline size_t arch_livepatch_insn_len(const struct livepatch_func *func)
+static inline
+unsigned int arch_livepatch_insn_len(const struct livepatch_func *func)
 {
     if ( !func->new_addr )
         return func->new_size;
 
     return PATCH_INSN_SIZE;
 }
+
+static inline int arch_livepatch_verify_distance(const struct livepatch_func *func)
+{
+    long offset;
+    long range = (long)LIVEPATCH_ARCH_RANGE;
+
+    if ( !func->new_addr ) /* Ignore NOPs. */
+        return 0;
+
+    offset = ((long)func->old_addr - (long)func->new_addr);
+    if ( offset < -range || offset >= range )
+        return -EOVERFLOW;
+
+    return 0;
+}
 /*
  * These functions are called around the critical region patching live code,
  * for an architecture to take make appropratie global state adjustments.
@@ -100,7 +117,6 @@ void arch_livepatch_unmask(void);
 #define init_or_livepatch_data        __initdata
 #define init_or_livepatch             __init
 
-#include <xen/errno.h> /* For -ENOSYS */
 static inline int livepatch_op(struct xen_sysctl_livepatch_op *op)
 {
     return -ENOSYS;
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x]
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (6 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 07/18] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-12 15:52   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 09/18] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
                   ` (10 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

Those symbols are used to help final linkers to replace insn.
The ARM ELF specification mandates that they are present
to denote the start of certain CPU features. There are two
variants of it - short and long format.

Either way - we can ignore these symbols.

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> [x86 bits]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v1: First submission
v2: Update the order of symbols, fix title
    Add {} in after the first if - per Jan's recommendation.
v3: Add Andrew's Review tag
    Make the function return an bool_t.
    Skip check for '$t'
    Fix spelling of comments.
    s/arch_is_payload_symbol/arch_livepatch_symbol_ok/
---
 xen/arch/arm/livepatch.c    | 33 +++++++++++++++++++++++++++++++++
 xen/arch/x86/livepatch.c    |  7 +++++++
 xen/common/livepatch.c      |  2 +-
 xen/include/xen/livepatch.h |  2 ++
 4 files changed, 43 insertions(+), 1 deletion(-)

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index 7a3e2f7..d2ae128 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -84,6 +84,39 @@ void arch_livepatch_unmask(void)
     local_abort_enable();
 }
 
+bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym)
+{
+    /*
+     * - Mapping symbols - denote the "start of a sequence of bytes of the
+     *   appropriate type" to mark certain features - such as start of region
+     *   containing data ($d); ARM ($a), or A64 ($x) instructions.
+     *   We ignore Thumb instructions ($t) as we shouldn't have them.
+     *
+     * The format is either short: '$x' or long: '$x.<any>'. We do not
+     * need this and more importantly - each payload will contain this
+     * resulting in symbol collisions.
+     */
+    if ( *sym->name == '$' && sym->name[1] != '\0' )
+    {
+        char p = sym->name[1];
+        size_t len = strlen(sym->name);
+
+        if ( (len >= 3 && ( sym->name[2] == '.' )) || (len == 2) )
+        {
+            if ( p == 'd' ||
+#ifdef CONFIG_ARM_32
+                 p == 'a'
+#else
+                 p == 'x'
+#endif
+               )
+                return false;
+        }
+    }
+    return true;
+}
+
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index cf3eb22..bbcb5fe 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -118,6 +118,13 @@ int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
     return 0;
 }
 
+bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym)
+{
+    /* No special checks on x86. */
+    return true;
+}
+
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela)
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index 4f156c2..3c726a4 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -734,7 +734,7 @@ static bool_t is_payload_symbol(const struct livepatch_elf *elf,
          !strncmp(sym->name, ".L", 2) )
         return 0;
 
-    return 1;
+    return arch_livepatch_symbol_ok(elf, sym);
 }
 
 static int build_symbol_table(struct payload *payload,
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 904785a..c67b02c 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -46,6 +46,8 @@ bool_t is_patch(const void *addr);
 
 /* Arch hooks. */
 int arch_livepatch_verify_elf(const struct livepatch_elf *elf);
+bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym);
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela);
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 09/18] livepatch/arm/x86: Check payload for for unwelcomed symbols.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (7 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x] Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-12 15:55   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 10/18] livepatch: Move test-cases to their own sub-directory in test Konrad Rzeszutek Wilk
                   ` (9 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

Certain platforms, such as ARM [32|64] add extra mapping symbols
such as $x (for ARM64 instructions), or more interesting to
this patch: $t (for Thumb instructions). These symbols are suppose
to help the final linker to make any adjustments (such as
add an veneer). But more importantly - we do not compile Xen
with any Thumb instructions (which are variable length) - and
if we find these mapping symbols we should disallow such payload.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3: New submission.
    Use &sym[i] instead of sym (as that will always be NULL).
---
 xen/arch/arm/livepatch.c    | 14 ++++++++++++++
 xen/arch/x86/livepatch.c    |  7 +++++++
 xen/common/livepatch_elf.c  |  9 +++++++++
 xen/include/xen/livepatch.h |  2 ++
 4 files changed, 32 insertions(+)

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index d2ae128..dd94c94 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -117,6 +117,20 @@ bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
     return true;
 }
 
+int arch_livepatch_symbol_check(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym)
+{
+#ifdef CONFIG_ARM_32
+    /*
+     * Xen does not use Thumb instructions - and we should not see any of
+     * them. If we do, abort.
+     */
+    if ( sym->name && *sym->name == '$' && sym->name[1] == 't' )
+        return -EINVAL;
+#endif
+    return 0;
+}
+
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela)
diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c
index bbcb5fe..27d4aac 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -125,6 +125,13 @@ bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
     return true;
 }
 
+int arch_livepatch_symbol_check(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym)
+{
+    /* No special checks on x86. */
+    return 0;
+}
+
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela)
diff --git a/xen/common/livepatch_elf.c b/xen/common/livepatch_elf.c
index e2264ad..9e2aa31 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -243,6 +243,7 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data)
     for ( i = 1; i < nsym; i++ )
     {
         const Elf_Sym *s = symtab_sec->data + symtab_sec->sec->sh_entsize * i;
+        int rc;
 
         delta = s->st_name;
         /* Boundary check within the .strtab. */
@@ -255,6 +256,14 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data)
 
         sym[i].sym = s;
         sym[i].name = strtab_sec->data + delta;
+        /* On ARM we should NEVER see $t* symbols. */
+        rc = arch_livepatch_symbol_check(elf, &sym[i]);
+        if ( rc )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Symbol '%s' should not be in payload!\n",
+                    elf->name, sym[i].name);
+            return rc;
+        }
     }
     elf->nsym = nsym;
 
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index c67b02c..edd905f 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -48,6 +48,8 @@ bool_t is_patch(const void *addr);
 int arch_livepatch_verify_elf(const struct livepatch_elf *elf);
 bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
                                 const struct livepatch_elf_sym *sym);
+int arch_livepatch_symbol_check(const struct livepatch_elf *elf,
+                                const struct livepatch_elf_sym *sym);
 int arch_livepatch_perform_rel(struct livepatch_elf *elf,
                                const struct livepatch_elf_sec *base,
                                const struct livepatch_elf_sec *rela);
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 10/18] livepatch: Move test-cases to their own sub-directory in test.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (8 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 09/18] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 11/18] livepatch: tests: Move the .name value to .rodata Konrad Rzeszutek Wilk
                   ` (8 subsequent siblings)
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

So they can be shared with ARM64 (but not yet, so they
are only built on x86).

No functional change.

We also need to tweak the MAINTAINERS and .gitignore file.

Also we need to update SUBDIRS to include the new 'test'
directory so 'cscope' can show the example livepatches.

Acked-by: Jan Beulich <jbeulich@suse.com> [for directory]
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Julien Grall <julien.grall@arm.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v1: First submission
v2: Move to test/livepatch per Jan's recommendation
v3: Sort MAINTAINERS for livepatch.
    Add Jan's Acked-by.
    Added on the SUBDIRS the 'test' directory
    Change title of patch (common-> own sub-directory)
---
 .gitignore                                  |  8 +--
 MAINTAINERS                                 |  1 +
 xen/Makefile                                |  5 +-
 xen/arch/arm/Makefile                       |  3 -
 xen/arch/x86/Makefile                       |  5 --
 xen/arch/x86/test/Makefile                  | 85 -----------------------------
 xen/arch/x86/test/xen_bye_world.c           | 34 ------------
 xen/arch/x86/test/xen_bye_world_func.c      | 22 --------
 xen/arch/x86/test/xen_hello_world.c         | 63 ---------------------
 xen/arch/x86/test/xen_hello_world_func.c    | 39 -------------
 xen/arch/x86/test/xen_replace_world.c       | 33 -----------
 xen/arch/x86/test/xen_replace_world_func.c  | 22 --------
 xen/test/Makefile                           |  9 +++
 xen/test/livepatch/Makefile                 | 85 +++++++++++++++++++++++++++++
 xen/test/livepatch/xen_bye_world.c          | 34 ++++++++++++
 xen/test/livepatch/xen_bye_world_func.c     | 22 ++++++++
 xen/test/livepatch/xen_hello_world.c        | 63 +++++++++++++++++++++
 xen/test/livepatch/xen_hello_world_func.c   | 39 +++++++++++++
 xen/test/livepatch/xen_replace_world.c      | 33 +++++++++++
 xen/test/livepatch/xen_replace_world_func.c | 22 ++++++++
 20 files changed, 315 insertions(+), 312 deletions(-)
 delete mode 100644 xen/arch/x86/test/Makefile
 delete mode 100644 xen/arch/x86/test/xen_bye_world.c
 delete mode 100644 xen/arch/x86/test/xen_bye_world_func.c
 delete mode 100644 xen/arch/x86/test/xen_hello_world.c
 delete mode 100644 xen/arch/x86/test/xen_hello_world_func.c
 delete mode 100644 xen/arch/x86/test/xen_replace_world.c
 delete mode 100644 xen/arch/x86/test/xen_replace_world_func.c
 create mode 100644 xen/test/Makefile
 create mode 100644 xen/test/livepatch/Makefile
 create mode 100644 xen/test/livepatch/xen_bye_world.c
 create mode 100644 xen/test/livepatch/xen_bye_world_func.c
 create mode 100644 xen/test/livepatch/xen_hello_world.c
 create mode 100644 xen/test/livepatch/xen_hello_world_func.c
 create mode 100644 xen/test/livepatch/xen_replace_world.c
 create mode 100644 xen/test/livepatch/xen_replace_world_func.c

diff --git a/.gitignore b/.gitignore
index cc64fc9..eeabe0b 100644
--- a/.gitignore
+++ b/.gitignore
@@ -254,10 +254,6 @@ xen/arch/x86/efi.lds
 xen/arch/x86/efi/check.efi
 xen/arch/x86/efi/disabled
 xen/arch/x86/efi/mkreloc
-xen/arch/x86/test/config.h
-xen/arch/x86/test/xen_hello_world.livepatch
-xen/arch/x86/test/xen_bye_world.livepatch
-xen/arch/x86/test/xen_replace_world.livepatch
 xen/arch/*/efi/boot.c
 xen/arch/*/efi/compat.c
 xen/arch/*/efi/efi.h
@@ -274,6 +270,10 @@ xen/include/public/public
 xen/include/xen/*.new
 xen/include/xen/acm_policy.h
 xen/include/xen/compile.h
+xen/test/livepatch/config.h
+xen/test/livepatch/xen_bye_world.livepatch
+xen/test/livepatch/xen_hello_world.livepatch
+xen/test/livepatch/xen_replace_world.livepatch
 xen/tools/kconfig/.tmp_gtkcheck
 xen/tools/kconfig/.tmp_qtcheck
 xen/tools/symbols
diff --git a/MAINTAINERS b/MAINTAINERS
index ae0b6bc..edc8603 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -272,6 +272,7 @@ F:  xen/arch/*/livepatch*
 F:  xen/arch/*/*/livepatch*
 F:  xen/common/livepatch*
 F:  xen/include/xen/livepatch*
+F:  xen/test/livepatch/*
 
 MACHINE CHECK (MCA) & RAS
 M:	Christoph Egger <chegger@amazon.de>
diff --git a/xen/Makefile b/xen/Makefile
index 012509b..e989a20 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -80,7 +80,7 @@ _install: $(TARGET)$(CONFIG_XEN_INSTALL_SUFFIX)
 
 .PHONY: _tests
 _tests:
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) tests
+	$(MAKE) -f $(BASEDIR)/Rules.mk -C test tests
 
 .PHONY: _uninstall
 _uninstall: D=$(DESTDIR)
@@ -114,6 +114,7 @@ _clean: delete-unfresh-files
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C xsm clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C crypto clean
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C arch/$(TARGET_ARCH) clean
+	$(MAKE) -f $(BASEDIR)/Rules.mk -C test clean
 	$(MAKE) -f $(BASEDIR)/tools/kconfig/Makefile.kconfig ARCH=$(ARCH) SRCARCH=$(SRCARCH) clean
 	find . \( -name "*.o" -o -name ".*.d" \) -exec rm -f {} \;
 	rm -f include/asm $(TARGET) $(TARGET).gz $(TARGET).efi $(TARGET).efi.map $(TARGET)-syms $(TARGET)-syms.map *~ core
@@ -189,7 +190,7 @@ include/asm-$(TARGET_ARCH)/asm-offsets.h: arch/$(TARGET_ARCH)/asm-offsets.s
 	  echo ""; \
 	  echo "#endif") <$< >$@
 
-SUBDIRS = xsm arch/$(TARGET_ARCH) common drivers
+SUBDIRS = xsm arch/$(TARGET_ARCH) common drivers test
 define all_sources
     ( find include/asm-$(TARGET_ARCH) -name '*.h' -print; \
       find include -name 'asm-*' -prune -o -name '*.h' -print; \
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index c3ce5a3..516e0af 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -74,9 +74,6 @@ ifeq ($(CONFIG_ARM_64),y)
 	ln -sf $(notdir $@)  ../../$(notdir $@).efi
 endif
 
-.PHONY: tests
-tests:
-
 $(TARGET).axf: $(TARGET)-syms
 	# XXX: VE model loads by VMA so instead of
 	# making a proper ELF we link with LMA == VMA and adjust crudely
diff --git a/xen/arch/x86/Makefile b/xen/arch/x86/Makefile
index d3875c5..931917d 100644
--- a/xen/arch/x86/Makefile
+++ b/xen/arch/x86/Makefile
@@ -93,10 +93,6 @@ $(TARGET): $(TARGET)-syms $(efi-y) boot/mkelf32
 	./boot/mkelf32 $(notes_phdrs) $(TARGET)-syms $(TARGET) 0x100000 \
 	`$(NM) -nr $(TARGET)-syms | head -n 1 | sed -e 's/^\([^ ]*\).*/0x\1/'`
 
-.PHONY: tests
-tests:
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C test livepatch
-
 ALL_OBJS := $(BASEDIR)/arch/x86/boot/built_in.o $(BASEDIR)/arch/x86/efi/built_in.o $(ALL_OBJS)
 
 ifeq ($(lto),y)
@@ -226,4 +222,3 @@ clean::
 	rm -f $(BASEDIR)/.xen.efi.[0-9]* efi/*.o efi/.*.d efi/*.efi efi/disabled efi/mkreloc
 	rm -f boot/reloc.S boot/reloc.lnk boot/reloc.bin
 	rm -f note.o
-	$(MAKE) -f $(BASEDIR)/Rules.mk -C test clean
diff --git a/xen/arch/x86/test/Makefile b/xen/arch/x86/test/Makefile
deleted file mode 100644
index 23dff1d..0000000
--- a/xen/arch/x86/test/Makefile
+++ /dev/null
@@ -1,85 +0,0 @@
-include $(XEN_ROOT)/Config.mk
-
-CODE_ADDR=$(shell nm --defined $(1) | grep $(2) | awk '{print "0x"$$1}')
-CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
-
-.PHONY: default
-
-LIVEPATCH := xen_hello_world.livepatch
-LIVEPATCH_BYE := xen_bye_world.livepatch
-LIVEPATCH_REPLACE := xen_replace_world.livepatch
-
-default: livepatch
-
-install: livepatch
-	$(INSTALL_DATA) $(LIVEPATCH) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
-	$(INSTALL_DATA) $(LIVEPATCH_BYE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
-	$(INSTALL_DATA) $(LIVEPATCH_REPLACE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
-uninstall:
-	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
-	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
-	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
-
-.PHONY: clean
-clean::
-	rm -f *.o .*.o.d *.livepatch config.h
-
-#
-# To compute these values we need the binary files: xen-syms
-# and xen_hello_world_func.o to be already compiled.
-#
-.PHONY: config.h
-config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version)
-config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world)
-config.h: xen_hello_world_func.o
-	(set -e; \
-	 echo "#define NEW_CODE_SZ $(NEW_CODE_SZ)"; \
-	 echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@
-
-xen_hello_world.o: config.h
-
-.PHONY: $(LIVEPATCH)
-$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
-	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
-
-#
-# This target is only accessible if CONFIG_LIVEPATCH is defined, which
-# depends on $(build_id_linker) being available. Hence we do not
-# need any checks.
-#
-# N.B. The reason we don't use arch/x86/note.o is that it may
-# not be built (it is for EFI builds), and that we do not have
-# the note.o.bin to muck with (as it gets deleted)
-#
-.PHONY: note.o
-note.o:
-	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
-	$(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 \
-		   --rename-section=.data=.livepatch.depends -S $@.bin $@
-	rm -f $@.bin
-
-#
-# Extract the build-id of the xen_hello_world.livepatch
-# (which xen_bye_world will depend on).
-#
-.PHONY: hello_world_note.o
-hello_world_note.o: $(LIVEPATCH)
-	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@.bin
-	$(OBJCOPY)  -I binary -O elf64-x86-64 -B i386:x86-64 \
-		   --rename-section=.data=.livepatch.depends -S $@.bin $@
-	rm -f $@.bin
-
-xen_bye_world.o: config.h
-
-.PHONY: $(LIVEPATCH_BYE)
-$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
-	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
-
-xen_replace_world.o: config.h
-
-.PHONY: $(LIVEPATCH_REPLACE)
-$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
-	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
-
-.PHONY: livepatch
-livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE)
diff --git a/xen/arch/x86/test/xen_bye_world.c b/xen/arch/x86/test/xen_bye_world.c
deleted file mode 100644
index b75e0b1..0000000
--- a/xen/arch/x86/test/xen_bye_world.c
+++ /dev/null
@@ -1,34 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- */
-
-#include "config.h"
-#include <xen/lib.h>
-#include <xen/types.h>
-#include <xen/version.h>
-#include <xen/livepatch.h>
-
-#include <public/sysctl.h>
-
-static char bye_world_patch_this_fnc[] = "xen_extra_version";
-extern const char *xen_bye_world(void);
-
-struct livepatch_func __section(".livepatch.funcs") livepatch_xen_bye_world = {
-    .version = LIVEPATCH_PAYLOAD_VERSION,
-    .name = bye_world_patch_this_fnc,
-    .new_addr = xen_bye_world,
-    .old_addr = xen_extra_version,
-    .new_size = NEW_CODE_SZ,
-    .old_size = OLD_CODE_SZ,
-};
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/test/xen_bye_world_func.c b/xen/arch/x86/test/xen_bye_world_func.c
deleted file mode 100644
index 32ef341..0000000
--- a/xen/arch/x86/test/xen_bye_world_func.c
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- */
-
-#include <xen/types.h>
-
-/* Our replacement function for xen_hello_world. */
-const char *xen_bye_world(void)
-{
-    return "Bye World!";
-}
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/test/xen_hello_world.c b/xen/arch/x86/test/xen_hello_world.c
deleted file mode 100644
index 472fe2e..0000000
--- a/xen/arch/x86/test/xen_hello_world.c
+++ /dev/null
@@ -1,63 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- */
-
-#include "config.h"
-#include <xen/lib.h>
-#include <xen/types.h>
-#include <xen/version.h>
-#include <xen/livepatch.h>
-#include <xen/livepatch_payload.h>
-
-#include <public/sysctl.h>
-
-static char hello_world_patch_this_fnc[] = "xen_extra_version";
-extern const char *xen_hello_world(void);
-/*
- * .bss is only cleared once - during upload. Apply/revert/apply will result
- * in cnt to have a non-zero value (see the hooks below).
- */
-static unsigned int cnt;
-
-static void apply_hook(void)
-{
-    printk(KERN_DEBUG "Hook executing.\n");
-}
-
-static void revert_hook(void)
-{
-    printk(KERN_DEBUG "Hook unloaded.\n");
-}
-
-static void  hi_func(void)
-{
-    printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt);
-};
-
-LIVEPATCH_LOAD_HOOK(apply_hook);
-LIVEPATCH_UNLOAD_HOOK(revert_hook);
-
-/* Imbalance here. Two load and three unload. */
-
-LIVEPATCH_LOAD_HOOK(hi_func);
-LIVEPATCH_UNLOAD_HOOK(hi_func);
-
-struct livepatch_func __section(".livepatch.funcs") livepatch_xen_hello_world = {
-    .version = LIVEPATCH_PAYLOAD_VERSION,
-    .name = hello_world_patch_this_fnc,
-    .new_addr = xen_hello_world,
-    .old_addr = xen_extra_version,
-    .new_size = NEW_CODE_SZ,
-    .old_size = OLD_CODE_SZ,
-};
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/test/xen_hello_world_func.c b/xen/arch/x86/test/xen_hello_world_func.c
deleted file mode 100644
index 03d6b84..0000000
--- a/xen/arch/x86/test/xen_hello_world_func.c
+++ /dev/null
@@ -1,39 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- */
-
-#include <xen/types.h>
-
-#include <asm/alternative.h>
-#include <asm/nops.h>
-#include <asm/uaccess.h>
-
-static unsigned long *non_canonical_addr = (unsigned long *)0xdead000000000000ULL;
-
-/* Our replacement function for xen_extra_version. */
-const char *xen_hello_world(void)
-{
-    unsigned long tmp;
-    int rc;
-
-    alternative(ASM_NOP8, ASM_NOP1, X86_FEATURE_LM);
-    /*
-     * Any BUG, or WARN_ON will contain symbol and payload name. Furthermore
-     * exceptions will be caught and processed properly.
-     */
-    rc = __get_user(tmp, non_canonical_addr);
-    BUG_ON(rc != -EFAULT);
-
-    return "Hello World";
-}
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/test/xen_replace_world.c b/xen/arch/x86/test/xen_replace_world.c
deleted file mode 100644
index a2a221a..0000000
--- a/xen/arch/x86/test/xen_replace_world.c
+++ /dev/null
@@ -1,33 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- */
-
-#include "config.h"
-#include <xen/lib.h>
-#include <xen/types.h>
-#include <xen/livepatch.h>
-
-#include <public/sysctl.h>
-
-static char xen_replace_world_name[] = "xen_extra_version";
-extern const char *xen_replace_world(void);
-
-struct livepatch_func __section(".livepatch.funcs") livepatch_xen_replace_world = {
-    .version = LIVEPATCH_PAYLOAD_VERSION,
-    .name = xen_replace_world_name,
-    .old_addr = 0, /* Forces the hypervisor to lookup .name */
-    .new_addr = xen_replace_world,
-    .new_size = NEW_CODE_SZ,
-    .old_size = OLD_CODE_SZ,
-};
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/arch/x86/test/xen_replace_world_func.c b/xen/arch/x86/test/xen_replace_world_func.c
deleted file mode 100644
index afb5cda..0000000
--- a/xen/arch/x86/test/xen_replace_world_func.c
+++ /dev/null
@@ -1,22 +0,0 @@
-/*
- * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
- *
- */
-
-#include <xen/types.h>
-
-/* Our replacement function for xen_hello_world. */
-const char *xen_replace_world(void)
-{
-    return "Hello Again World!";
-}
-
-/*
- * Local variables:
- * mode: C
- * c-file-style: "BSD"
- * c-basic-offset: 4
- * tab-width: 4
- * indent-tabs-mode: nil
- * End:
- */
diff --git a/xen/test/Makefile b/xen/test/Makefile
new file mode 100644
index 0000000..8c53040
--- /dev/null
+++ b/xen/test/Makefile
@@ -0,0 +1,9 @@
+.PHONY: tests
+tests:
+ifeq ($(XEN_TARGET_ARCH),x86_64)
+	$(MAKE) -f $(BASEDIR)/Rules.mk -C livepatch livepatch
+endif
+
+.PHONY: clean
+clean::
+	$(MAKE) -f $(BASEDIR)/Rules.mk -C livepatch clean
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
new file mode 100644
index 0000000..23dff1d
--- /dev/null
+++ b/xen/test/livepatch/Makefile
@@ -0,0 +1,85 @@
+include $(XEN_ROOT)/Config.mk
+
+CODE_ADDR=$(shell nm --defined $(1) | grep $(2) | awk '{print "0x"$$1}')
+CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
+
+.PHONY: default
+
+LIVEPATCH := xen_hello_world.livepatch
+LIVEPATCH_BYE := xen_bye_world.livepatch
+LIVEPATCH_REPLACE := xen_replace_world.livepatch
+
+default: livepatch
+
+install: livepatch
+	$(INSTALL_DATA) $(LIVEPATCH) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
+	$(INSTALL_DATA) $(LIVEPATCH_BYE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
+	$(INSTALL_DATA) $(LIVEPATCH_REPLACE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
+uninstall:
+	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
+	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
+	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
+
+.PHONY: clean
+clean::
+	rm -f *.o .*.o.d *.livepatch config.h
+
+#
+# To compute these values we need the binary files: xen-syms
+# and xen_hello_world_func.o to be already compiled.
+#
+.PHONY: config.h
+config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version)
+config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world)
+config.h: xen_hello_world_func.o
+	(set -e; \
+	 echo "#define NEW_CODE_SZ $(NEW_CODE_SZ)"; \
+	 echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@
+
+xen_hello_world.o: config.h
+
+.PHONY: $(LIVEPATCH)
+$(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH) $^
+
+#
+# This target is only accessible if CONFIG_LIVEPATCH is defined, which
+# depends on $(build_id_linker) being available. Hence we do not
+# need any checks.
+#
+# N.B. The reason we don't use arch/x86/note.o is that it may
+# not be built (it is for EFI builds), and that we do not have
+# the note.o.bin to muck with (as it gets deleted)
+#
+.PHONY: note.o
+note.o:
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
+	$(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 \
+		   --rename-section=.data=.livepatch.depends -S $@.bin $@
+	rm -f $@.bin
+
+#
+# Extract the build-id of the xen_hello_world.livepatch
+# (which xen_bye_world will depend on).
+#
+.PHONY: hello_world_note.o
+hello_world_note.o: $(LIVEPATCH)
+	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@.bin
+	$(OBJCOPY)  -I binary -O elf64-x86-64 -B i386:x86-64 \
+		   --rename-section=.data=.livepatch.depends -S $@.bin $@
+	rm -f $@.bin
+
+xen_bye_world.o: config.h
+
+.PHONY: $(LIVEPATCH_BYE)
+$(LIVEPATCH_BYE): xen_bye_world_func.o xen_bye_world.o hello_world_note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_BYE) $^
+
+xen_replace_world.o: config.h
+
+.PHONY: $(LIVEPATCH_REPLACE)
+$(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
+
+.PHONY: livepatch
+livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE)
diff --git a/xen/test/livepatch/xen_bye_world.c b/xen/test/livepatch/xen_bye_world.c
new file mode 100644
index 0000000..b75e0b1
--- /dev/null
+++ b/xen/test/livepatch/xen_bye_world.c
@@ -0,0 +1,34 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/version.h>
+#include <xen/livepatch.h>
+
+#include <public/sysctl.h>
+
+static char bye_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_bye_world(void);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_bye_world = {
+    .version = LIVEPATCH_PAYLOAD_VERSION,
+    .name = bye_world_patch_this_fnc,
+    .new_addr = xen_bye_world,
+    .old_addr = xen_extra_version,
+    .new_size = NEW_CODE_SZ,
+    .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/test/livepatch/xen_bye_world_func.c b/xen/test/livepatch/xen_bye_world_func.c
new file mode 100644
index 0000000..32ef341
--- /dev/null
+++ b/xen/test/livepatch/xen_bye_world_func.c
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/types.h>
+
+/* Our replacement function for xen_hello_world. */
+const char *xen_bye_world(void)
+{
+    return "Bye World!";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/test/livepatch/xen_hello_world.c b/xen/test/livepatch/xen_hello_world.c
new file mode 100644
index 0000000..472fe2e
--- /dev/null
+++ b/xen/test/livepatch/xen_hello_world.c
@@ -0,0 +1,63 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/version.h>
+#include <xen/livepatch.h>
+#include <xen/livepatch_payload.h>
+
+#include <public/sysctl.h>
+
+static char hello_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_hello_world(void);
+/*
+ * .bss is only cleared once - during upload. Apply/revert/apply will result
+ * in cnt to have a non-zero value (see the hooks below).
+ */
+static unsigned int cnt;
+
+static void apply_hook(void)
+{
+    printk(KERN_DEBUG "Hook executing.\n");
+}
+
+static void revert_hook(void)
+{
+    printk(KERN_DEBUG "Hook unloaded.\n");
+}
+
+static void  hi_func(void)
+{
+    printk(KERN_DEBUG "%s: Hi! (called %u times)\n", __func__, ++cnt);
+};
+
+LIVEPATCH_LOAD_HOOK(apply_hook);
+LIVEPATCH_UNLOAD_HOOK(revert_hook);
+
+/* Imbalance here. Two load and three unload. */
+
+LIVEPATCH_LOAD_HOOK(hi_func);
+LIVEPATCH_UNLOAD_HOOK(hi_func);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_hello_world = {
+    .version = LIVEPATCH_PAYLOAD_VERSION,
+    .name = hello_world_patch_this_fnc,
+    .new_addr = xen_hello_world,
+    .old_addr = xen_extra_version,
+    .new_size = NEW_CODE_SZ,
+    .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/test/livepatch/xen_hello_world_func.c b/xen/test/livepatch/xen_hello_world_func.c
new file mode 100644
index 0000000..03d6b84
--- /dev/null
+++ b/xen/test/livepatch/xen_hello_world_func.c
@@ -0,0 +1,39 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/types.h>
+
+#include <asm/alternative.h>
+#include <asm/nops.h>
+#include <asm/uaccess.h>
+
+static unsigned long *non_canonical_addr = (unsigned long *)0xdead000000000000ULL;
+
+/* Our replacement function for xen_extra_version. */
+const char *xen_hello_world(void)
+{
+    unsigned long tmp;
+    int rc;
+
+    alternative(ASM_NOP8, ASM_NOP1, X86_FEATURE_LM);
+    /*
+     * Any BUG, or WARN_ON will contain symbol and payload name. Furthermore
+     * exceptions will be caught and processed properly.
+     */
+    rc = __get_user(tmp, non_canonical_addr);
+    BUG_ON(rc != -EFAULT);
+
+    return "Hello World";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/test/livepatch/xen_replace_world.c b/xen/test/livepatch/xen_replace_world.c
new file mode 100644
index 0000000..a2a221a
--- /dev/null
+++ b/xen/test/livepatch/xen_replace_world.c
@@ -0,0 +1,33 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/lib.h>
+#include <xen/types.h>
+#include <xen/livepatch.h>
+
+#include <public/sysctl.h>
+
+static char xen_replace_world_name[] = "xen_extra_version";
+extern const char *xen_replace_world(void);
+
+struct livepatch_func __section(".livepatch.funcs") livepatch_xen_replace_world = {
+    .version = LIVEPATCH_PAYLOAD_VERSION,
+    .name = xen_replace_world_name,
+    .old_addr = 0, /* Forces the hypervisor to lookup .name */
+    .new_addr = xen_replace_world,
+    .new_size = NEW_CODE_SZ,
+    .old_size = OLD_CODE_SZ,
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/test/livepatch/xen_replace_world_func.c b/xen/test/livepatch/xen_replace_world_func.c
new file mode 100644
index 0000000..afb5cda
--- /dev/null
+++ b/xen/test/livepatch/xen_replace_world_func.c
@@ -0,0 +1,22 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include <xen/types.h>
+
+/* Our replacement function for xen_hello_world. */
+const char *xen_replace_world(void)
+{
+    return "Hello Again World!";
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 11/18] livepatch: tests: Move the .name value to .rodata
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (9 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 10/18] livepatch: Move test-cases to their own sub-directory in test Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 12/18] livepatch: tests: Make them compile under ARM64 Konrad Rzeszutek Wilk
                   ` (7 subsequent siblings)
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

Right now the contents of 'name' are all located in
the .data section. We want them in the .rodata section
so change the type to have const on them.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>

v3: First submission.
---
 xen/test/livepatch/xen_bye_world.c     | 2 +-
 xen/test/livepatch/xen_hello_world.c   | 2 +-
 xen/test/livepatch/xen_replace_world.c | 2 +-
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/xen/test/livepatch/xen_bye_world.c b/xen/test/livepatch/xen_bye_world.c
index b75e0b1..2700f0e 100644
--- a/xen/test/livepatch/xen_bye_world.c
+++ b/xen/test/livepatch/xen_bye_world.c
@@ -11,7 +11,7 @@
 
 #include <public/sysctl.h>
 
-static char bye_world_patch_this_fnc[] = "xen_extra_version";
+static const char bye_world_patch_this_fnc[] = "xen_extra_version";
 extern const char *xen_bye_world(void);
 
 struct livepatch_func __section(".livepatch.funcs") livepatch_xen_bye_world = {
diff --git a/xen/test/livepatch/xen_hello_world.c b/xen/test/livepatch/xen_hello_world.c
index 472fe2e..3cd059f 100644
--- a/xen/test/livepatch/xen_hello_world.c
+++ b/xen/test/livepatch/xen_hello_world.c
@@ -12,7 +12,7 @@
 
 #include <public/sysctl.h>
 
-static char hello_world_patch_this_fnc[] = "xen_extra_version";
+static const char hello_world_patch_this_fnc[] = "xen_extra_version";
 extern const char *xen_hello_world(void);
 /*
  * .bss is only cleared once - during upload. Apply/revert/apply will result
diff --git a/xen/test/livepatch/xen_replace_world.c b/xen/test/livepatch/xen_replace_world.c
index a2a221a..78a8f52 100644
--- a/xen/test/livepatch/xen_replace_world.c
+++ b/xen/test/livepatch/xen_replace_world.c
@@ -10,7 +10,7 @@
 
 #include <public/sysctl.h>
 
-static char xen_replace_world_name[] = "xen_extra_version";
+static const char xen_replace_world_name[] = "xen_extra_version";
 extern const char *xen_replace_world(void);
 
 struct livepatch_func __section(".livepatch.funcs") livepatch_xen_replace_world = {
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 12/18] livepatch: tests: Make them compile under ARM64
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (10 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 11/18] livepatch: tests: Move the .name value to .rodata Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 13/18] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
                   ` (6 subsequent siblings)
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, andrew.cooper3,
	Tim Deegan, Jan Beulich, Ian Jackson

We need to two things:
1) Wrap the platform-specific objcopy parameters in defines
   The input and output parameters for $(OBJCOPY) are different
   based on the platforms. As such provide them in the
   OBJCOPY_MAGIC define and use that.

2) The alternative is a bit different and there are no
   exceptions under ARM (but there are under ARM 64).
   Also use one of the first config options for the CPU
   field feature.

We are not yet attempting to build them under ARM32 so
that is still ifdefed out.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: George Dunlap <George.Dunlap@eu.citrix.com>
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Tim Deegan <tim@xen.org>
Cc: Wei Liu <wei.liu2@citrix.com>

v1: First submission
v2: Corrected description by Julien
    Add #ifeq instead of #else for ARM case.
v3: Moved 'asm(alter..)' by one space to the left.
---
 xen/test/Makefile                         |  2 +-
 xen/test/livepatch/Makefile               | 11 +++++++++--
 xen/test/livepatch/xen_hello_world_func.c |  8 +++++++-
 3 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/xen/test/Makefile b/xen/test/Makefile
index 8c53040..95c1755 100644
--- a/xen/test/Makefile
+++ b/xen/test/Makefile
@@ -1,6 +1,6 @@
 .PHONY: tests
 tests:
-ifeq ($(XEN_TARGET_ARCH),x86_64)
+ifneq $(XEN_TARGET_ARCH),arm32)
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C livepatch livepatch
 endif
 
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index 23dff1d..ce09e1d 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -1,5 +1,12 @@
 include $(XEN_ROOT)/Config.mk
 
+ifeq ($(XEN_TARGET_ARCH),x86_64)
+OBJCOPY_MAGIC := -I binary -O elf64-x86-64 -B i386:x86-64
+endif
+ifeq ($(XEN_TARGET_ARCH),arm64)
+OBJCOPY_MAGIC := -I binary -O elf64-littleaarch64 -B aarch64
+endif
+
 CODE_ADDR=$(shell nm --defined $(1) | grep $(2) | awk '{print "0x"$$1}')
 CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
 
@@ -54,7 +61,7 @@ $(LIVEPATCH): xen_hello_world_func.o xen_hello_world.o note.o
 .PHONY: note.o
 note.o:
 	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(BASEDIR)/xen-syms $@.bin
-	$(OBJCOPY) -I binary -O elf64-x86-64 -B i386:x86-64 \
+	$(OBJCOPY) $(OBJCOPY_MAGIC) \
 		   --rename-section=.data=.livepatch.depends -S $@.bin $@
 	rm -f $@.bin
 
@@ -65,7 +72,7 @@ note.o:
 .PHONY: hello_world_note.o
 hello_world_note.o: $(LIVEPATCH)
 	$(OBJCOPY) -O binary --only-section=.note.gnu.build-id $(LIVEPATCH) $@.bin
-	$(OBJCOPY)  -I binary -O elf64-x86-64 -B i386:x86-64 \
+	$(OBJCOPY) $(OBJCOPY_MAGIC) \
 		   --rename-section=.data=.livepatch.depends -S $@.bin $@
 	rm -f $@.bin
 
diff --git a/xen/test/livepatch/xen_hello_world_func.c b/xen/test/livepatch/xen_hello_world_func.c
index 03d6b84..6f53ab4 100644
--- a/xen/test/livepatch/xen_hello_world_func.c
+++ b/xen/test/livepatch/xen_hello_world_func.c
@@ -6,14 +6,17 @@
 #include <xen/types.h>
 
 #include <asm/alternative.h>
+#ifdef CONFIG_X86
 #include <asm/nops.h>
 #include <asm/uaccess.h>
 
 static unsigned long *non_canonical_addr = (unsigned long *)0xdead000000000000ULL;
+#endif
 
 /* Our replacement function for xen_extra_version. */
 const char *xen_hello_world(void)
 {
+#ifdef CONFIG_X86
     unsigned long tmp;
     int rc;
 
@@ -24,7 +27,10 @@ const char *xen_hello_world(void)
      */
     rc = __get_user(tmp, non_canonical_addr);
     BUG_ON(rc != -EFAULT);
-
+#endif
+#ifdef CONFIG_ARM_64
+    asm(ALTERNATIVE("nop", "nop", ARM64_WORKAROUND_CLEAN_CACHE));
+#endif
     return "Hello World";
 }
 
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 13/18] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (11 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 12/18] livepatch: tests: Make them compile under ARM64 Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-13  9:14   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 14/18] xen/arm32: Add an helper to invalidate all instruction caches Konrad Rzeszutek Wilk
                   ` (5 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

To use as a common way of testing alternative patching for
livepatches. Both architectures have this FEATURE and the
test-cases can piggyback on that.

Suggested-by: Julien Grall <julien.grall@arm.com>
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3: New submission
---
 xen/arch/arm/livepatch.c                  | 3 +++
 xen/include/asm-arm/alternative.h         | 2 ++
 xen/include/asm-arm/cpufeature.h          | 5 +++++
 xen/include/asm-x86/cpufeature.h          | 1 +
 xen/test/livepatch/xen_hello_world_func.c | 8 +++++---
 5 files changed, 16 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index dd94c94..da01955 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -8,6 +8,7 @@
 #include <xen/livepatch.h>
 #include <xen/vmap.h>
 
+#include <asm/cpufeature.h>
 #include <asm/livepatch.h>
 #include <asm/mm.h>
 
@@ -175,6 +176,8 @@ void __init arch_livepatch_init(void)
     end = (void *)LIVEPATCH_VMAP_END;
 
     vm_init_type(VMAP_XEN, start, end);
+
+    cpus_set_cap(LIVEPATCH_FEATURE);
 }
 
 /*
diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
index 9f88fd9..074ff63 100644
--- a/xen/include/asm-arm/alternative.h
+++ b/xen/include/asm-arm/alternative.h
@@ -165,6 +165,8 @@ static inline int apply_alternatives(void *start, size_t lenght)
     return 0;
 }
 
+#define ALTERNATIVE(oldinstr, newinstr, ...) ""
+
 #endif
 
 #endif /* __ASM_ALTERNATIVE_H */
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 66e930f..19e768b 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -40,7 +40,12 @@
 #define ARM32_WORKAROUND_766422 2
 #define ARM64_WORKAROUND_834220 3
 
+#ifdef CONFIG_LIVEPATCH
+#define LIVEPATCH_FEATURE   4
+#define ARM_NCAPS           5
+#else
 #define ARM_NCAPS           4
+#endif
 
 #ifndef __ASSEMBLY__
 
diff --git a/xen/include/asm-x86/cpufeature.h b/xen/include/asm-x86/cpufeature.h
index bcdf5d6..404dd91 100644
--- a/xen/include/asm-x86/cpufeature.h
+++ b/xen/include/asm-x86/cpufeature.h
@@ -34,6 +34,7 @@ XEN_CPUFEATURE(MFENCE_RDTSC,    (FSCAPINTS+0)*32+ 9) /* MFENCE synchronizes RDTS
 
 /* An alias of a feature we know is always going to be present. */
 #define X86_FEATURE_ALWAYS      X86_FEATURE_LM
+#define LIVEPATCH_FEATURE       X86_FEATURE_ALWAYS
 
 #if !defined(__ASSEMBLY__) && !defined(X86_FEATURES_ONLY)
 #include <xen/bitops.h>
diff --git a/xen/test/livepatch/xen_hello_world_func.c b/xen/test/livepatch/xen_hello_world_func.c
index 6f53ab4..765a871 100644
--- a/xen/test/livepatch/xen_hello_world_func.c
+++ b/xen/test/livepatch/xen_hello_world_func.c
@@ -20,7 +20,6 @@ const char *xen_hello_world(void)
     unsigned long tmp;
     int rc;
 
-    alternative(ASM_NOP8, ASM_NOP1, X86_FEATURE_LM);
     /*
      * Any BUG, or WARN_ON will contain symbol and payload name. Furthermore
      * exceptions will be caught and processed properly.
@@ -28,8 +27,11 @@ const char *xen_hello_world(void)
     rc = __get_user(tmp, non_canonical_addr);
     BUG_ON(rc != -EFAULT);
 #endif
-#ifdef CONFIG_ARM_64
-    asm(ALTERNATIVE("nop", "nop", ARM64_WORKAROUND_CLEAN_CACHE));
+#ifdef CONFIG_ARM
+    asm(ALTERNATIVE("nop", "nop", LIVEPATCH_FEATURE));
+#endif
+#ifdef CONFIG_X86
+    asm(ALTERNATIVE(ASM_NOP8, ASM_NOP1, LIVEPATCH_FEATURE));
 #endif
     return "Hello World";
 }
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 14/18] xen/arm32: Add an helper to invalidate all instruction caches
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (12 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 13/18] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
                   ` (4 subsequent siblings)
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Konrad Rzeszutek Wilk

This is exactly like commit fb9d877a9c0f3d4d15db8f6e0c5506ea641862c6
"xen/arm64: Add an helper to invalidate all instruction caches"
except it is on ARM32 side.

When we are flushing the cache we are most likley also want
to flush the branch predictor too. Hence we add this.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>

v2: First submission
v3: Squashed "xen/arm32/livepatch: Add BPICALLIS to helper to invalidate
    all instruction caches" in this patch.
---
 xen/include/asm-arm/arm32/page.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/xen/include/asm-arm/arm32/page.h b/xen/include/asm-arm/arm32/page.h
index bccdbfc..583ed75 100644
--- a/xen/include/asm-arm/arm32/page.h
+++ b/xen/include/asm-arm/arm32/page.h
@@ -30,6 +30,19 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 #define __clean_and_invalidate_dcache_one(R) STORE_CP32(R, DCCIMVAC)
 
 /*
+ * Invalidate all instruction caches in Inner Shareable domain to PoU.
+ * We also need to flush the branch predictor for ARMv7 as it may be
+ * architecturally visible to the software (see B2.2.4 in ARM DDI 0406C.b).
+ */
+static inline void invalidate_icache(void)
+{
+    asm volatile (
+        CMD_CP32(ICIALLUIS)     /* Flush I-cache. */
+        CMD_CP32(BPIALLIS)      /* Flush branch predictor. */
+        : : : "memory");
+}
+
+/*
  * Flush all hypervisor mappings from the TLB and branch predictor of
  * the local processor.
  *
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (13 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 14/18] xen/arm32: Add an helper to invalidate all instruction caches Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-13  9:21   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 16/18] livepatch: ARM32 support Konrad Rzeszutek Wilk
                   ` (3 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

Most of the WARN_ON or BUG_ON sections are properly aligned on
x86. However on ARM and on x86 assembler the macros don't include
any aligment information - hence they end up being the default
byte granularity.

On ARM32 it is paramount that the aligment is word-size (4)
otherwise if one tries to use (uint32_t*) access (such
as livepatch ELF relocations) we get a Data Abort.

Enforcing bug_frames to have the proper aligment across all
architectures and in both C and x86 makes them all the same.

Furthermore on x86 the bloat-o-meter detects that with this
change:

[konrad@char xen]$ ~/linux/scripts/bloat-o-meter xen-syms xen-syms.align4
add/remove: 0/0 grow/shrink: 3/14 up/down: 115/-1497 (-1382)
function                                     old     new   delta
mod_l1_entry                                1490    1587     +97
p2m_switch_domain_altp2m_by_id               520     533     +13
p2m_switch_vcpu_altp2m_by_id                 453     458      +5
machine_kexec                                263     261      -2
hvm_do_IRQ_dpci                              244     242      -2
sh_page_fault__guest_4                      8252    8192     -60
sh_audit_gw                                 1529    1462     -67
validate_gl3e                                337     264     -73
validate_gl4e                                449     375     -74
p2m_altp2m_lazy_copy                         730     652     -78
set_typed_p2m_entry                         1346    1259     -87
virt_to_xen_l2e                              491     365    -126
sh_x86_emulate_write__guest_4                443     288    -155
p2m_mem_access_check                        1733    1576    -157
sh_x86_emulate_cmpxchg__guest_4              512     349    -163
__get_gfn_type_access                        709     542    -167
map_pages_to_xen                            4430    4144    -286
Total: Before=1974033, After=1972651, chg -0.07%

We end up making the binary file a bit smaller.

On ARM32:
add/remove: 1/0 grow/shrink: 0/1 up/down: 384/-288 (96)
function                                     old     new   delta
gnttab_unpopulate_status_frames                -     384    +384
do_grant_table_op                          10808   10520    -288

And ARM64:
add/remove: 1/2 grow/shrink: 0/1 up/down: 4164/-4236 (-72)
function                                     old     new   delta
gnttab_map_grant_ref                           -    4164   +4164
do_grant_table_op                           9892    9836     -56
grant_map_exists                             300       -    -300
__gnttab_map_grant_ref                      3880       -   -3880

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3: First submission. Replaces the "livepatch/elf: Adjust section aligment to word"
    patch.
---
 xen/include/asm-arm/bug.h | 1 +
 xen/include/asm-x86/bug.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
index 68353e1..773d63e 100644
--- a/xen/include/asm-arm/bug.h
+++ b/xen/include/asm-arm/bug.h
@@ -52,6 +52,7 @@ struct bug_frame {
          ".popsection\n"                                                    \
          ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
          "4:\n"                                                             \
+         ".align 4\n"                                                       \
          ".long (1b - 4b)\n"                                                \
          ".long (2b - 4b)\n"                                                \
          ".long (3b - 4b)\n"                                                \
diff --git a/xen/include/asm-x86/bug.h b/xen/include/asm-x86/bug.h
index c5d2d4c..9bb4a19 100644
--- a/xen/include/asm-x86/bug.h
+++ b/xen/include/asm-x86/bug.h
@@ -98,6 +98,7 @@ extern const struct bug_frame __start_bug_frames[],
     .popsection
 
     .pushsection .bug_frames.\type, "a", @progbits
+        .p2align 2
         .L\@bf:
         .long (.L\@ud - .L\@bf) + \
                ((\line >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 16/18] livepatch: ARM32 support.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (14 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-12  7:48   ` Jan Beulich
  2016-09-11 20:35 ` [PATCH v3 17/18] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp Konrad Rzeszutek Wilk
                   ` (2 subsequent siblings)
  18 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Konrad Rzeszutek Wilk

The patch piggybacks on: livepatch: Initial ARM64 support, which
brings up all of the neccessary livepatch infrastructure pieces in.

This patch adds three major pieces:

 1) ELF relocations. ARM32 uses SHT_REL instead of SHT_RELA which
    means the adddendum had to be extracted from within the
    instruction. Which required parsing BL/BLX, B/BL<cond>,
    MOVT, and MOVW instructions.

    The code was written from scratch using the ARM ELF manual
    (and the ARM Architecture Reference Manual)

 2) Inserting an trampoline. We use the B (branch to address)
    which uses an offset that is based on the PC value: PC + imm32.
    Because we insert the branch at the start of the old function
    we have to account for the instruction already being fetched
    and subtract -8 from the delta (new_addr - old_addr). See
    ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)

 3) Allows the test-cases to be built under ARM 32.
    The "livepatch: tests: Make them compile under ARM64"
    put in the right infrastructure for it and we piggyback on it.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>

v2: First submission.
v3: Use LIVEPATCH_ARCH_RANGE instead of NEGATIVE_32MB macro
   -Use PATCH_INSN_SIZE instead of the value 4.
   -Ditch the old_ptr local variable.
   -Use 8 for evaluating the branch instead of 4. Based on ARM docs.
   -NOP patch up to sizeof(opaque) % PATCH_INSN_SIZE (so 7 instructions).
   -Don't mask 0x00FFFFF_E_ after shifting, instead mask by 0x00FFFFF_F_.
    The reason is that offset is constructed by shifting by two the insn
    (except the first two bytes) by left which meant we would have cleared
    offset[2]! - and jumped to a location that was -4 bytes off.
   -Update commit description to have -8 instead of -4 delta and also
    include reference to spec.
---
 xen/arch/arm/arm32/livepatch.c | 273 ++++++++++++++++++++++++++++++++++++++++-
 xen/arch/arm/arm64/livepatch.c |   7 ++
 xen/arch/arm/livepatch.c       |   7 --
 xen/common/Kconfig             |   2 +-
 xen/include/xen/elfstructs.h   |  24 +++-
 xen/test/Makefile              |   2 -
 xen/test/livepatch/Makefile    |   3 +
 7 files changed, 305 insertions(+), 13 deletions(-)

diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index c33b68d..56233a4 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -3,28 +3,297 @@
  */
 
 #include <xen/errno.h>
+#include <xen/kernel.h>
 #include <xen/lib.h>
 #include <xen/livepatch_elf.h>
 #include <xen/livepatch.h>
 
+#include <asm/page.h>
+#include <asm/livepatch.h>
+
 void arch_livepatch_apply_jmp(struct livepatch_func *func)
 {
+    uint32_t insn;
+    uint32_t *new_ptr;
+    unsigned int i, len;
+
+    BUILD_BUG_ON(PATCH_INSN_SIZE > sizeof(func->opaque));
+    BUILD_BUG_ON(PATCH_INSN_SIZE != sizeof(insn));
+
+    ASSERT(vmap_of_xen_text);
+
+    len = arch_livepatch_insn_len(func);
+    if ( !len )
+        return;
+
+    /* Save old ones. */
+    memcpy(func->opaque, func->old_addr, len);
+
+    if ( func->new_addr )
+    {
+        s32 delta;
+
+        /*
+         * PC is current address (old_addr) + 8 bytes. The semantics for a
+         * unconditional branch is to jump to PC + imm32 (offset).
+         *
+         * ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)
+         *
+         */
+        delta = (s32)func->new_addr - (s32)(func->old_addr + 8);
+
+        /* The arch_livepatch_symbol_ok should have caught it. */
+        ASSERT(delta >= -(s32)LIVEPATCH_ARCH_RANGE ||
+               delta < (s32)LIVEPATCH_ARCH_RANGE);
+
+        /* CPU shifts by two (left) when decoding, so we shift right by two. */
+        delta = delta >> 2;
+        /* Lets not modify the cond. */
+        delta &= 0x00FFFFFF;
+
+        insn = 0xea000000 | delta;
+    }
+    else
+        insn = 0xe1a00000; /* mov r0, r0 */
+
+    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    len = len / sizeof(uint32_t);
+
+    /* PATCH! */
+    for ( i = 0; i < len; i++ )
+        *(new_ptr + i) = insn;
+
+    clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
 }
 
 void arch_livepatch_revert_jmp(const struct livepatch_func *func)
 {
+    uint32_t *new_ptr;
+    unsigned int i, len;
+
+    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    len = arch_livepatch_insn_len(func) / sizeof(uint32_t);
+    for ( i = 0; i < len; i++ )
+    {
+        uint32_t insn;
+
+        memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), PATCH_INSN_SIZE);
+        /* PATCH! */
+        *(new_ptr + i) = insn;
+    }
+
+    clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
 }
 
 int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
 {
-    return -EOPNOTSUPP;
+    const Elf_Ehdr *hdr = elf->hdr;
+
+    if ( hdr->e_machine != EM_ARM ||
+         hdr->e_ident[EI_CLASS] != ELFCLASS32 )
+    {
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF Machine type!\n",
+                elf->name);
+        return -EOPNOTSUPP;
+    }
+
+    if ( (hdr->e_flags & EF_ARM_EABI_MASK) != EF_ARM_EABI_VER5 )
+    {
+        dprintk(XENLOG_ERR, LIVEPATCH "%s: Unsupported ELF EABI(%x)!\n",
+                elf->name, hdr->e_flags);
+        return -EOPNOTSUPP;
+    }
+
+    return 0;
+}
+
+static s32 get_addend(unsigned char type, void *dest)
+{
+    s32 addend = 0;
+
+    switch ( type ) {
+    case R_ARM_NONE:
+        /* ignore */
+        break;
+
+    case R_ARM_ABS32:
+        addend = *(u32 *)dest;
+        break;
+
+    case R_ARM_REL32:
+        addend = *(u32 *)dest;
+        break;
+
+    case R_ARM_MOVW_ABS_NC:
+    case R_ARM_MOVT_ABS:
+        addend =  (*(u32 *)dest & 0x00000FFF);
+        addend |= (*(u32 *)dest & 0x000F0000) >> 4;
+        /* Addend is to sign-extend ([19:16],[11:0]). */
+        addend = (s16)addend;
+        break;
+
+    case R_ARM_CALL:
+    case R_ARM_JUMP24:
+        /* Addend = sign_extend (insn[23:0]) << 2 */
+        addend = ((*(u32 *)dest & 0xFFFFFF) ^ 0x800000) - 0x800000;
+        addend = addend << 2;
+        break;
+    }
+
+    return addend;
+}
+
+static int perform_rel(unsigned char type, void *dest, uint32_t val, s32 addend)
+{
+
+    switch ( type ) {
+    case R_ARM_NONE:
+        /* ignore */
+        break;
+
+    case R_ARM_ABS32: /* (S + A) | T */
+        *(u32 *)dest = (val + addend);
+        break;
+
+    case R_ARM_REL32: /* ((S + A) | T) – P */
+        *(u32 *)dest = (val + addend) - (uint32_t)dest;
+        break;
+
+    case R_ARM_MOVW_ABS_NC: /* S + A */
+    case R_ARM_MOVT_ABS: /* S + A */
+        /* Clear addend if needed . */
+        if ( addend )
+            *(u32 *)dest &= 0xFFF0F000;
+
+        if ( type == R_ARM_MOVT_ABS )
+        {
+            /*
+             * Almost the same as MOVW except it uses the 16 bit
+             * high value. Putting it in insn requires shifting right by
+             * 16-bit (as we only have 16-bit for imm.
+             */
+            val &= 0xFFFF0000; /* ResultMask */
+            val = val >> 16;
+        }
+        else
+        {
+            /* MOVW loads 16 bits into the bottom half of a register. */
+            val &= 0xFFFF;
+        }
+        /* [11:0] = Result_Mask(X) & 0xFFF,[19:16] = Result_Mask(X) >> 12 */
+        *(u32 *)dest |= val & 0xFFF;
+        *(u32 *)dest |= (val >> 12) << 16;
+        break;
+
+    case R_ARM_CALL:
+    case R_ARM_JUMP24: /* (S + A) - P */
+        /* Clear the old addend. */
+        if ( addend )
+            *(u32 *)dest &= 0xFF000000;
+
+        val += addend - (uint32_t)dest;
+
+        /*
+         * arch_livepatch_verify_distance can't account of addend so we have
+         * to do the check here as well.
+         */
+        if ( (s32)val < -(s32)LIVEPATCH_ARCH_RANGE ||
+             (s32)val >= (s32)LIVEPATCH_ARCH_RANGE )
+            return -EOVERFLOW;
+
+        /* CPU always shifts insn by two, so complement it. */
+        val = val >> 2;
+        val &= 0x00FFFFFE;
+        *(u32 *)dest |= (uint32_t)val;
+        break;
+
+    default:
+         return -EOPNOTSUPP;
+    }
+
+    return 0;
+}
+
+int arch_livepatch_perform(struct livepatch_elf *elf,
+                           const struct livepatch_elf_sec *base,
+                           const struct livepatch_elf_sec *rela,
+                           bool use_rela)
+{
+    const Elf_RelA *r_a;
+    const Elf_Rel *r;
+    unsigned int symndx, i;
+    uint32_t val;
+    void *dest;
+    int rc = 0;
+
+    for ( i = 0; i < (rela->sec->sh_size / rela->sec->sh_entsize); i++ )
+    {
+        unsigned char type;
+        s32 addend = 0;
+
+        if ( use_rela )
+        {
+            r_a = rela->data + i * rela->sec->sh_entsize;
+            symndx = ELF32_R_SYM(r_a->r_info);
+            type = ELF32_R_TYPE(r_a->r_info);
+            dest = base->load_addr + r_a->r_offset; /* P */
+            addend = r_a->r_addend;
+        }
+        else
+        {
+            r = rela->data + i * rela->sec->sh_entsize;
+            symndx = ELF32_R_SYM(r->r_info);
+            type = ELF32_R_TYPE(r->r_info);
+            dest = base->load_addr + r->r_offset; /* P */
+        }
+
+        if ( symndx > elf->nsym )
+        {
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Relative symbol wants symbol@%u which is past end!\n",
+                    elf->name, symndx);
+            return -EINVAL;
+        }
+
+        if ( !use_rela )
+            addend = get_addend(type, dest);
+
+        val = elf->sym[symndx].sym->st_value; /* S */
+
+        rc = perform_rel(type, dest, val, addend);
+        switch ( rc ) {
+        case -EOVERFLOW:
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Overflow in relocation %u in %s for %s!\n",
+                    elf->name, i, rela->name, base->name);
+            break;
+
+        case -EOPNOTSUPP:
+            dprintk(XENLOG_ERR, LIVEPATCH "%s: Unhandled relocation #%x\n",
+                    elf->name, type);
+            break;
+
+        default:
+            break;
+        }
+
+        if ( rc )
+            break;
+    }
+
+    return rc;
+}
+
+int arch_livepatch_perform_rel(struct livepatch_elf *elf,
+                               const struct livepatch_elf_sec *base,
+                               const struct livepatch_elf_sec *rela)
+{
+    return arch_livepatch_perform(elf, base, rela, false);
 }
 
 int arch_livepatch_perform_rela(struct livepatch_elf *elf,
                                 const struct livepatch_elf_sec *base,
                                 const struct livepatch_elf_sec *rela)
 {
-    return -ENOSYS;
+    return arch_livepatch_perform(elf, base, rela, true);
 }
 
 /*
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index f072671..c786649 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -231,6 +231,13 @@ static int reloc_insn_imm(enum aarch64_reloc_op op, void *dest, u64 val,
     return 0;
 }
 
+int arch_livepatch_perform_rel(struct livepatch_elf *elf,
+                               const struct livepatch_elf_sec *base,
+                               const struct livepatch_elf_sec *rela)
+{
+    return -ENOSYS;
+}
+
 int arch_livepatch_perform_rela(struct livepatch_elf *elf,
                                 const struct livepatch_elf_sec *base,
                                 const struct livepatch_elf_sec *rela)
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index da01955..29fca6e 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -132,13 +132,6 @@ int arch_livepatch_symbol_check(const struct livepatch_elf *elf,
     return 0;
 }
 
-int arch_livepatch_perform_rel(struct livepatch_elf *elf,
-                               const struct livepatch_elf_sec *base,
-                               const struct livepatch_elf_sec *rela)
-{
-    return -ENOSYS;
-}
-
 int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type type)
 {
     unsigned long start = (unsigned long)va;
diff --git a/xen/common/Kconfig b/xen/common/Kconfig
index 0f26027..d4f10ca 100644
--- a/xen/common/Kconfig
+++ b/xen/common/Kconfig
@@ -217,7 +217,7 @@ config CRYPTO
 config LIVEPATCH
 	bool "Live patching support (TECH PREVIEW)"
 	default n
-	depends on !ARM_32 && HAS_BUILD_ID = "y"
+	depends on HAS_BUILD_ID = "y"
 	---help---
 	  Allows a running Xen hypervisor to be dynamically patched using
 	  binary patches without rebooting. This is primarily used to binarily
diff --git a/xen/include/xen/elfstructs.h b/xen/include/xen/elfstructs.h
index 7329987..e543212 100644
--- a/xen/include/xen/elfstructs.h
+++ b/xen/include/xen/elfstructs.h
@@ -103,6 +103,15 @@ typedef uint64_t	Elf64_Xword;
                       (ehdr).e_ident[EI_MAG2] == ELFMAG2 && \
                       (ehdr).e_ident[EI_MAG3] == ELFMAG3)
 
+/* e_flags */
+#define EF_ARM_EABI_MASK	0xff000000
+#define EF_ARM_EABI_UNKNOWN	0x00000000
+#define EF_ARM_EABI_VER1	0x01000000
+#define EF_ARM_EABI_VER2	0x02000000
+#define EF_ARM_EABI_VER3	0x03000000
+#define EF_ARM_EABI_VER4	0x04000000
+#define EF_ARM_EABI_VER5	0x05000000
+
 /* ELF Header */
 typedef struct elfhdr {
 	unsigned char	e_ident[EI_NIDENT]; /* ELF Identification */
@@ -364,9 +373,22 @@ typedef struct {
 #define R_X86_64_PLT32		4	/* 32 bit PLT address */
 
 /*
+ * ARM32 relocation types. See
+ * http://infocenter.arm.com/help/topic/com.arm.doc.ihi0044f/IHI0044F_aaelf.pdf
  * S - address of symbol.
- * A - addend for relocation (r_addend)
+ * A - addend for relocation (r_addend or need to extract from insn)
  * P - address of the dest being relocated (derieved from r_offset)
+ */
+#define R_ARM_NONE              0
+#define R_ARM_ABS32             2	/* Direct 32-bit. S+A */
+#define R_ARM_REL32             3	/* PC relative. S+A */
+#define R_ARM_CALL              28	/* SignExtend([23:0]) << 2. S+A-P */
+#define R_ARM_JUMP24            29	/* Same as R_ARM_CALL */
+#define R_ARM_MOVW_ABS_NC       43	/* SignExtend([19:16],[11:0])&0xFFFF, S+A */
+#define R_ARM_MOVT_ABS          44	/* SignExtend([19:16],[11:0))&0xFFFF0000 */
+					/*  >> 16, S+A. */
+
+/*
  * NC -  No check for overflow.
  *
  * The defines also use _PREL for PC-relative address, and _NC is No Check.
diff --git a/xen/test/Makefile b/xen/test/Makefile
index 95c1755..d91b319 100644
--- a/xen/test/Makefile
+++ b/xen/test/Makefile
@@ -1,8 +1,6 @@
 .PHONY: tests
 tests:
-ifneq $(XEN_TARGET_ARCH),arm32)
 	$(MAKE) -f $(BASEDIR)/Rules.mk -C livepatch livepatch
-endif
 
 .PHONY: clean
 clean::
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index ce09e1d..bfd63c2 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -6,6 +6,9 @@ endif
 ifeq ($(XEN_TARGET_ARCH),arm64)
 OBJCOPY_MAGIC := -I binary -O elf64-littleaarch64 -B aarch64
 endif
+ifeq ($(XEN_TARGET_ARCH),arm32)
+OBJCOPY_MAGIC := -I binary -O elf32-littlearm -B arm
+endif
 
 CODE_ADDR=$(shell nm --defined $(1) | grep $(2) | awk '{print "0x"$$1}')
 CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 17/18] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (15 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 16/18] livepatch: ARM32 support Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-11 20:35 ` [PATCH v3 18/18] livepatch: arm[32, 64], x86: NOP test-case Konrad Rzeszutek Wilk
  2016-09-16 13:35 ` [PATCH v3] Livepatch for ARM 64 and 32 Julien Grall
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Konrad Rzeszutek Wilk

It is exactly the same in both platforms.

No functional change.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>

v3: New submission.
---
 xen/arch/arm/arm32/livepatch.c | 19 +------------------
 xen/arch/arm/arm64/livepatch.c | 19 +------------------
 xen/arch/arm/livepatch.c       | 19 +++++++++++++++++++
 3 files changed, 21 insertions(+), 36 deletions(-)

diff --git a/xen/arch/arm/arm32/livepatch.c b/xen/arch/arm/arm32/livepatch.c
index 56233a4..73dcf3e 100644
--- a/xen/arch/arm/arm32/livepatch.c
+++ b/xen/arch/arm/arm32/livepatch.c
@@ -66,24 +66,7 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func)
     clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
 }
 
-void arch_livepatch_revert_jmp(const struct livepatch_func *func)
-{
-    uint32_t *new_ptr;
-    unsigned int i, len;
-
-    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
-    len = arch_livepatch_insn_len(func) / sizeof(uint32_t);
-    for ( i = 0; i < len; i++ )
-    {
-        uint32_t insn;
-
-        memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), PATCH_INSN_SIZE);
-        /* PATCH! */
-        *(new_ptr + i) = insn;
-    }
-
-    clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
-}
+/* arch_livepatch_revert_jmp shared with ARM 64. */
 
 int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
 {
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index c786649..f8fc7fb 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -53,24 +53,7 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func)
     clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr) * len);
 }
 
-void arch_livepatch_revert_jmp(const struct livepatch_func *func)
-{
-    uint32_t *new_ptr;
-    unsigned int i, len;
-
-    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
-    len = arch_livepatch_insn_len(func) / sizeof(uint32_t);
-    for ( i = 0; i < len; i++ )
-    {
-        uint32_t insn;
-
-        memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), PATCH_INSN_SIZE);
-        /* PATCH! */
-        *(new_ptr + i) = insn;
-    }
-
-    clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
-}
+/* arch_livepatch_revert_jmp shared with ARM32. */
 
 int arch_livepatch_verify_elf(const struct livepatch_elf *elf)
 {
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index 29fca6e..a14b7de 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -69,6 +69,25 @@ int arch_livepatch_verify_func(const struct livepatch_func *func)
     return 0;
 }
 
+void arch_livepatch_revert_jmp(const struct livepatch_func *func)
+{
+    uint32_t *new_ptr;
+    unsigned int i, len;
+
+    new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
+    len = arch_livepatch_insn_len(func) / sizeof(uint32_t);
+    for ( i = 0; i < len; i++ )
+    {
+        uint32_t insn;
+
+        memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), PATCH_INSN_SIZE);
+        /* PATCH! */
+        *(new_ptr + i) = insn;
+    }
+
+    clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr) * len);
+}
+
 void arch_livepatch_post_action(void)
 {
     /* arch_livepatch_revive has nuked the instruction cache. */
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* [PATCH v3 18/18] livepatch: arm[32, 64], x86: NOP test-case
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (16 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 17/18] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp Konrad Rzeszutek Wilk
@ 2016-09-11 20:35 ` Konrad Rzeszutek Wilk
  2016-09-16 13:35 ` [PATCH v3] Livepatch for ARM 64 and 32 Julien Grall
  18 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-11 20:35 UTC (permalink / raw)
  To: xen-devel, konrad, julien.grall, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich, Konrad Rzeszutek Wilk

The test-case is quite simple - we NOP the 'xen_minor_version'.
The amount of NOPs depends on the architecture.

On x86 the function is 11 bytes long:

   55                      push   %rbp      <- NOP
   48 89 e5                mov    %rsp,%rbp <- NOP
   b8 04 00 00 00          mov    $0x4,%eax <- NOP
   5d                      pop    %rbp      <- NOP
   c3                      retq

We can NOP everything but the last instruction (so 10 bytes).

On ARM64 its 8 bytes:

  52800100    mov w0, #0x8         <- NOP
  d65f03c0    ret

We can NOP the first instruction.

While on ARM32 there are 24 bytes:

  e52db004        push    {fp}
  e28db000        add     fp, sp, #0 <- NOP
  e3a00008        mov     r0, #8     <- NOP
  e24bd000        sub     sp, fp, #0 <- NOP
  e49db004        pop     {fp}
  e12fff1e        bx      lr

And we can NOP instruction 2,3, and 4.

Granted this code may be different per compiler!

Hence if anybody does run this test-case - they should
verify that the assumptions made here are correct.

Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Julien Grall <julien.grall@arm.com>
Cc: Stefano Stabellini <sstabellini@kernel.org>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>

v3: New submission.
---
 xen/test/livepatch/Makefile  | 15 +++++++++++++-
 xen/test/livepatch/xen_nop.c | 49 ++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 63 insertions(+), 1 deletion(-)
 create mode 100644 xen/test/livepatch/xen_nop.c

diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index bfd63c2..69b0cdd 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -18,6 +18,7 @@ CODE_SZ=$(shell nm --defined -S $(1) | grep $(2) | awk '{ print "0x"$$2}')
 LIVEPATCH := xen_hello_world.livepatch
 LIVEPATCH_BYE := xen_bye_world.livepatch
 LIVEPATCH_REPLACE := xen_replace_world.livepatch
+LIVEPATCH_NOP := xen_nop.livepatch
 
 default: livepatch
 
@@ -25,10 +26,12 @@ install: livepatch
 	$(INSTALL_DATA) $(LIVEPATCH) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
 	$(INSTALL_DATA) $(LIVEPATCH_BYE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
 	$(INSTALL_DATA) $(LIVEPATCH_REPLACE) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
+	$(INSTALL_DATA) $(LIVEPATCH_NOP) $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_NOP)
 uninstall:
 	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH)
 	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_BYE)
 	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_REPLACE)
+	rm -f $(DESTDIR)$(DEBUG_DIR)/$(LIVEPATCH_NOP)
 
 .PHONY: clean
 clean::
@@ -41,9 +44,13 @@ clean::
 .PHONY: config.h
 config.h: OLD_CODE_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_extra_version)
 config.h: NEW_CODE_SZ=$(call CODE_SZ,$<,xen_hello_world)
+config.h: MINOR_VERSION_SZ=$(call CODE_SZ,$(BASEDIR)/xen-syms,xen_minor_version)
+config.h: MINOR_VERSION_ADDR=$(call CODE_ADDR,$(BASEDIR)/xen-syms,xen_minor_version)
 config.h: xen_hello_world_func.o
 	(set -e; \
 	 echo "#define NEW_CODE_SZ $(NEW_CODE_SZ)"; \
+	 echo "#define MINOR_VERSION_SZ $(MINOR_VERSION_SZ)"; \
+	 echo "#define MINOR_VERSION_ADDR $(MINOR_VERSION_ADDR)"; \
 	 echo "#define OLD_CODE_SZ $(OLD_CODE_SZ)") > $@
 
 xen_hello_world.o: config.h
@@ -91,5 +98,11 @@ xen_replace_world.o: config.h
 $(LIVEPATCH_REPLACE): xen_replace_world_func.o xen_replace_world.o note.o
 	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_REPLACE) $^
 
+xen_nop.o: config.h
+
+.PHONY: $(LIVEPATCH_NOP)
+$(LIVEPATCH_NOP): xen_nop.o note.o
+	$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
+
 .PHONY: livepatch
-livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE)
+livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP)
diff --git a/xen/test/livepatch/xen_nop.c b/xen/test/livepatch/xen_nop.c
new file mode 100644
index 0000000..3827979
--- /dev/null
+++ b/xen/test/livepatch/xen_nop.c
@@ -0,0 +1,49 @@
+/*
+ * Copyright (c) 2016 Oracle and/or its affiliates. All rights reserved.
+ *
+ */
+
+#include "config.h"
+#include <xen/types.h>
+
+#include <public/sysctl.h>
+
+/*
+ * All of the .new_size and .old_addr are based on assumptions that the
+ * code for 'xen_minor_version' is compiled in specific way. Before
+ * running this test-case you MUST verify that the assumptions are
+ * correct (Hint: make debug and look in xen.s).
+ */
+struct livepatch_func __section(".livepatch.funcs") livepatch_nop = {
+    .version = LIVEPATCH_PAYLOAD_VERSION,
+    .old_size = MINOR_VERSION_SZ,
+
+#ifdef CONFIG_X86
+    .old_addr = (void *)MINOR_VERSION_ADDR,
+    /* Everything but the last instruction: "req". */
+    .new_size = MINOR_VERSION_SZ-1,
+#endif
+
+#ifdef CONFIG_ARM_64
+    .old_addr = (void *)MINOR_VERSION_ADDR,
+    /* Replace the first one: "mov w0, #0x8".  */
+    .new_size = 4,
+#endif
+
+#ifdef CONFIG_ARM_32
+    /* Skip the first instruction: "push {fp}". */
+    .old_addr = (void *)(MINOR_VERSION_ADDR + 4),
+    /* And replace the next three instructions. */
+    .new_size = 3 * 4,
+#endif
+};
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * tab-width: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
-- 
2.4.11


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

^ permalink raw reply related	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 06/18] livepatch: Initial ARM64 support.
  2016-09-11 20:35 ` [PATCH v3 06/18] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
@ 2016-09-12  7:45   ` Jan Beulich
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Beulich @ 2016-09-12  7:45 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> As compared to x86 the va of the hypervisor .text
> is locked down - we cannot modify the running pagetables
> to have the .ro flag unset. We borrow the same idea that
> alternative patching has - which is to vmap the entire
> .text region and use the alternative virtual address
> for patching.
> 
> Since we are doing vmap we may fail, hence the
> arch_livepatch_quiesce was changed (see "x86,arm:
> Change arch_livepatch_quiesce() declaration") to return
> an error value which will be bubbled in payload->rc and
> provided to the user (along with messages in the ring buffer).
> 
> The livepatch virtual address space (where the new functions
> are) needs to be close to the hypervisor virtual address
> so that the trampoline can reach it. As such we re-use
> the BOOT_RELOC_VIRT_START which is not used after bootup
> (alternatively we can also use the space after the _end to
> FIXMAP_ADDR(0), but that may be too small).
> 
> The ELF relocation engine at the start was coded from
> the "ELF for the ARM 64-bit Architecture (AArch64)"
> (http://infocenter.arm.com/help/topic/com.arm.doc.ihi0056b/IHI0056B_aaelf64.
> pdf)
> but after a while of trying to write the correct bit shifting
> and masking from scratch I ended up borrowing from Linux, the
> 'reloc_insn_imm' (Linux v4.7 arch/arm64/kernel/module.c function.
> See 257cb251925f854da435cbf79b140984413871ac "arm64: Loadable modules")
> 
> And while at it - we also utilize code from Linux to construct
> the right branch instruction (see "arm64/insn: introduce
> aarch64_insn_gen_{nop|branch_imm}() helper functions").
> 
> In the livepatch payload loading code we tweak the #ifdef to
> only exclude ARM_32. The exceptions are not part of ARM 32/64 hence
> they are still behind the #ifdef.
> 
> We also expand the MAINTAINERS file to include the arm64 and arm32
> platform specific livepatch file.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> ---
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com>
> Cc  Jan Beulich <jbeulich@suse.com>

Oops.

Non-ARM parts:
Acked-by: Jan Beulich <jbeulich@suse.com>

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 16/18] livepatch: ARM32 support.
  2016-09-11 20:35 ` [PATCH v3 16/18] livepatch: ARM32 support Konrad Rzeszutek Wilk
@ 2016-09-12  7:48   ` Jan Beulich
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Beulich @ 2016-09-12  7:48 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> The patch piggybacks on: livepatch: Initial ARM64 support, which
> brings up all of the neccessary livepatch infrastructure pieces in.
> 
> This patch adds three major pieces:
> 
>  1) ELF relocations. ARM32 uses SHT_REL instead of SHT_RELA which
>     means the adddendum had to be extracted from within the
>     instruction. Which required parsing BL/BLX, B/BL<cond>,
>     MOVT, and MOVW instructions.
> 
>     The code was written from scratch using the ARM ELF manual
>     (and the ARM Architecture Reference Manual)
> 
>  2) Inserting an trampoline. We use the B (branch to address)
>     which uses an offset that is based on the PC value: PC + imm32.
>     Because we insert the branch at the start of the old function
>     we have to account for the instruction already being fetched
>     and subtract -8 from the delta (new_addr - old_addr). See
>     ARM DDI 0406C.c, see A2.3 (pg 45) and A8.8.18 pg (pg 334,335)
> 
>  3) Allows the test-cases to be built under ARM 32.
>     The "livepatch: tests: Make them compile under ARM64"
>     put in the right infrastructure for it and we piggyback on it.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

The little bit of non-ARM parts:
Acked-by: Jan Beulich <jbeulich@suse.com>


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE]
  2016-09-11 20:35 ` [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
@ 2016-09-12 15:28   ` Jan Beulich
  2016-09-12 15:31     ` Julien Grall
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Beulich @ 2016-09-12 15:28 UTC (permalink / raw)
  To: julien.grall, sstabellini, Konrad Rzeszutek Wilk
  Cc: andrew.cooper3, Doug Goldstein, xen-devel, ross.lagerwall

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> x86 implements all of them by default - and we just
> add two extra HAS_ variables to be declared in autoconf.h.
> 
> ARM 64 only has alternative while ARM 32 has none of them.
> The ARM64 is going to be a bit funny as there is an
> ALTERNATIVE already and we end up selecting the HAS_ALTERNATIVE
> whenever the ALTERNATIVE is selected.

How about replacing ALTERNATIVE by HAS_ALTERNATIVE?

> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -45,8 +45,12 @@ config ACPI
>  config HAS_GICV3
>  	bool
>  
> +config HAS_ALTERNATIVE
> +	bool

This needlessly repeats what you add to common/Kconfig.

> @@ -22,6 +24,7 @@ config X86
>  	select NUMA
>  	select VGA
>  
> +
>  config ARCH_DEFCONFIG

Please don't.

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE]
  2016-09-12 15:28   ` Jan Beulich
@ 2016-09-12 15:31     ` Julien Grall
  2016-09-13 16:58       ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 38+ messages in thread
From: Julien Grall @ 2016-09-12 15:31 UTC (permalink / raw)
  To: Jan Beulich, sstabellini, Konrad Rzeszutek Wilk
  Cc: andrew.cooper3, Doug Goldstein, xen-devel, ross.lagerwall

Hi,

On 12/09/16 16:28, Jan Beulich wrote:
>>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
>> x86 implements all of them by default - and we just
>> add two extra HAS_ variables to be declared in autoconf.h.
>>
>> ARM 64 only has alternative while ARM 32 has none of them.
>> The ARM64 is going to be a bit funny as there is an
>> ALTERNATIVE already and we end up selecting the HAS_ALTERNATIVE
>> whenever the ALTERNATIVE is selected.
>
> How about replacing ALTERNATIVE by HAS_ALTERNATIVE?

FWIW, I was about to suggest the same.

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 07/18] livepatch: ARM/x86: Check displacement of old_addr and new_addr
  2016-09-11 20:35 ` [PATCH v3 07/18] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
@ 2016-09-12 15:36   ` Jan Beulich
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Beulich @ 2016-09-12 15:36 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> If the distance is too great we are in trouble - as our relocation

I think you mean "large" or big" here.

> distance can surely be clipped, or still have a valid width - but
> cause an overflow of distance.
> 
> On various architectures the maximum displacement for a unconditional
> branch/jump varies. ARM32 is +/- 32MB, ARM64 is +/- 128MB while x86
> for 32-bit relocations is +/- 2G.
> 
> Note: On x86 we could use the 64-bit jmpq instruction which
> would provide much bigger displacement to do a jump, but we would
> still have issues with the new function not being able to reach
> any of the old functions (as all the relocations would assume 32-bit
> displacement).

With jmpq you mean the indirect variant? That additionally
would require a register or memory location you can clobber
to load/store the address into.

> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -12,6 +12,7 @@ struct livepatch_elf_sym;
>  struct xen_sysctl_livepatch_op;
>  
>  #include <xen/elfstructs.h>
> +#include <xen/errno.h> /* For -ENOSYS or -EOVERFLOW */
>  #ifdef CONFIG_LIVEPATCH
>  
>  /*
> @@ -66,16 +67,32 @@ int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type types
>  void arch_livepatch_init(void);
>  
>  #include <public/sysctl.h> /* For struct livepatch_func. */
> -#include <asm/livepatch.h> /* For PATCH_INSN_SIZE. */
> +#include <asm/livepatch.h> /* For LIVEPATCH_ARCH_RANGE and PATCH_INSN_SIZE */
>  int arch_livepatch_verify_func(const struct livepatch_func *func);
>  
> -static inline size_t arch_livepatch_insn_len(const struct livepatch_func *func)
> +static inline
> +unsigned int arch_livepatch_insn_len(const struct livepatch_func *func)
>  {
>      if ( !func->new_addr )
>          return func->new_size;
>  
>      return PATCH_INSN_SIZE;
>  }
> +
> +static inline int arch_livepatch_verify_distance(const struct livepatch_func *func)

Just like mentioned earlier for arch_livepatch_insn_len() I don't
think the arch prefix is a good choice when the function isn't
arch-specific.

> +{
> +    long offset;
> +    long range = (long)LIVEPATCH_ARCH_RANGE;
> +
> +    if ( !func->new_addr ) /* Ignore NOPs. */
> +        return 0;
> +
> +    offset = ((long)func->old_addr - (long)func->new_addr);

Please avoid casts when they're not really needed.

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x]
  2016-09-11 20:35 ` [PATCH v3 08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x] Konrad Rzeszutek Wilk
@ 2016-09-12 15:52   ` Jan Beulich
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Beulich @ 2016-09-12 15:52 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> Those symbols are used to help final linkers to replace insn.
> The ARM ELF specification mandates that they are present
> to denote the start of certain CPU features. There are two
> variants of it - short and long format.
> 
> Either way - we can ignore these symbols.
> 
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> [x86 bits]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> 
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Cc: Ross Lagerwall <ross.lagerwall@citrix.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> Cc: Julien Grall <julien.grall@arm.com 
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> 
> v1: First submission
> v2: Update the order of symbols, fix title
>     Add {} in after the first if - per Jan's recommendation.
> v3: Add Andrew's Review tag
>     Make the function return an bool_t.

Yet it should have been bool.

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 09/18] livepatch/arm/x86: Check payload for for unwelcomed symbols.
  2016-09-11 20:35 ` [PATCH v3 09/18] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
@ 2016-09-12 15:55   ` Jan Beulich
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Beulich @ 2016-09-12 15:55 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
> @@ -117,6 +117,20 @@ bool_t arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
>      return true;
>  }
>  
> +int arch_livepatch_symbol_check(const struct livepatch_elf *elf,
> +                                const struct livepatch_elf_sym *sym)

Perhaps also better to return bool here?

> @@ -255,6 +256,14 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data)
>  
>          sym[i].sym = s;
>          sym[i].name = strtab_sec->data + delta;
> +        /* On ARM we should NEVER see $t* symbols. */

If you really mean to have such an arch-specific comment in common
code, may I recommend to use "e.g." or something similar?

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 13/18] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
  2016-09-11 20:35 ` [PATCH v3 13/18] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
@ 2016-09-13  9:14   ` Jan Beulich
  0 siblings, 0 replies; 38+ messages in thread
From: Jan Beulich @ 2016-09-13  9:14 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> --- a/xen/include/asm-x86/cpufeature.h
> +++ b/xen/include/asm-x86/cpufeature.h
> @@ -34,6 +34,7 @@ XEN_CPUFEATURE(MFENCE_RDTSC,    (FSCAPINTS+0)*32+ 9) /* MFENCE synchronizes RDTS
>  
>  /* An alias of a feature we know is always going to be present. */
>  #define X86_FEATURE_ALWAYS      X86_FEATURE_LM
> +#define LIVEPATCH_FEATURE       X86_FEATURE_ALWAYS

Didn't an earlier patch introduce asm/livepatch.h? Such a #define
would better go there, if that file already exists.

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections.
  2016-09-11 20:35 ` [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
@ 2016-09-13  9:21   ` Jan Beulich
  2016-09-13 19:26     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 38+ messages in thread
From: Jan Beulich @ 2016-09-13  9:21 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

>>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> Furthermore on x86 the bloat-o-meter detects that with this
> change:
> 
> [konrad@char xen]$ ~/linux/scripts/bloat-o-meter xen-syms xen-syms.align4
> add/remove: 0/0 grow/shrink: 3/14 up/down: 115/-1497 (-1382)
> function                                     old     new   delta
> mod_l1_entry                                1490    1587     +97
> p2m_switch_domain_altp2m_by_id               520     533     +13
> p2m_switch_vcpu_altp2m_by_id                 453     458      +5
> machine_kexec                                263     261      -2
> hvm_do_IRQ_dpci                              244     242      -2
> sh_page_fault__guest_4                      8252    8192     -60
> sh_audit_gw                                 1529    1462     -67
> validate_gl3e                                337     264     -73
> validate_gl4e                                449     375     -74
> p2m_altp2m_lazy_copy                         730     652     -78
> set_typed_p2m_entry                         1346    1259     -87
> virt_to_xen_l2e                              491     365    -126
> sh_x86_emulate_write__guest_4                443     288    -155
> p2m_mem_access_check                        1733    1576    -157
> sh_x86_emulate_cmpxchg__guest_4              512     349    -163
> __get_gfn_type_access                        709     542    -167
> map_pages_to_xen                            4430    4144    -286
> Total: Before=1974033, After=1972651, chg -0.07%
> 
> We end up making the binary file a bit smaller.

I'm fine with the change, but I'm having a very hard time buying the
above: The change you do is to code used only in assembly files _and_
is not affecting .text (and I assume the sizes above are .text ones),
yet all the functions listed above live in C ones. I can't help thinking
that there must be something else going on, or that we had an actual
problem before this.

> --- a/xen/include/asm-x86/bug.h
> +++ b/xen/include/asm-x86/bug.h
> @@ -98,6 +98,7 @@ extern const struct bug_frame __start_bug_frames[],
>      .popsection
>  
>      .pushsection .bug_frames.\type, "a", @progbits
> +        .p2align 2
>          .L\@bf:
>          .long (.L\@ud - .L\@bf) + \
>                 ((\line >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)

This ought to be accompanied by removing the enforcing of alignment
in xen.lds.S.

Jan


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE]
  2016-09-12 15:31     ` Julien Grall
@ 2016-09-13 16:58       ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-13 16:58 UTC (permalink / raw)
  To: Julien Grall
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, Doug Goldstein,
	Jan Beulich, xen-devel

On Mon, Sep 12, 2016 at 04:31:20PM +0100, Julien Grall wrote:
> Hi,
> 
> On 12/09/16 16:28, Jan Beulich wrote:
> > > > > On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> > > x86 implements all of them by default - and we just
> > > add two extra HAS_ variables to be declared in autoconf.h.
> > > 
> > > ARM 64 only has alternative while ARM 32 has none of them.
> > > The ARM64 is going to be a bit funny as there is an
> > > ALTERNATIVE already and we end up selecting the HAS_ALTERNATIVE
> > > whenever the ALTERNATIVE is selected.
> > 
> > How about replacing ALTERNATIVE by HAS_ALTERNATIVE?
> 
> FWIW, I was about to suggest the same.

Fantastic! Got a new patch queued up.


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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections.
  2016-09-13  9:21   ` Jan Beulich
@ 2016-09-13 19:26     ` Konrad Rzeszutek Wilk
  0 siblings, 0 replies; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-13 19:26 UTC (permalink / raw)
  To: Jan Beulich
  Cc: sstabellini, ross.lagerwall, andrew.cooper3, julien.grall, xen-devel

On Tue, Sep 13, 2016 at 03:21:04AM -0600, Jan Beulich wrote:
> >>> On 11.09.16 at 22:35, <konrad.wilk@oracle.com> wrote:
> > Furthermore on x86 the bloat-o-meter detects that with this
> > change:
> > 
> > [konrad@char xen]$ ~/linux/scripts/bloat-o-meter xen-syms xen-syms.align4
> > add/remove: 0/0 grow/shrink: 3/14 up/down: 115/-1497 (-1382)
> > function                                     old     new   delta
> > mod_l1_entry                                1490    1587     +97
> > p2m_switch_domain_altp2m_by_id               520     533     +13
> > p2m_switch_vcpu_altp2m_by_id                 453     458      +5
> > machine_kexec                                263     261      -2
> > hvm_do_IRQ_dpci                              244     242      -2
> > sh_page_fault__guest_4                      8252    8192     -60
> > sh_audit_gw                                 1529    1462     -67
> > validate_gl3e                                337     264     -73
> > validate_gl4e                                449     375     -74
> > p2m_altp2m_lazy_copy                         730     652     -78
> > set_typed_p2m_entry                         1346    1259     -87
> > virt_to_xen_l2e                              491     365    -126
> > sh_x86_emulate_write__guest_4                443     288    -155
> > p2m_mem_access_check                        1733    1576    -157
> > sh_x86_emulate_cmpxchg__guest_4              512     349    -163
> > __get_gfn_type_access                        709     542    -167
> > map_pages_to_xen                            4430    4144    -286
> > Total: Before=1974033, After=1972651, chg -0.07%
> > 
> > We end up making the binary file a bit smaller.
> 
> I'm fine with the change, but I'm having a very hard time buying the
> above: The change you do is to code used only in assembly files _and_

That is indeed fishy. I re-ran the test and got:

 ~/linux/scripts/bloat-o-meter /tmp/xen-syms.orig /tmp/xen-syms.p2malign 
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
function                                     old     new   delta

which is more inline with what we expect.

> is not affecting .text (and I assume the sizes above are .text ones),
> yet all the functions listed above live in C ones. I can't help thinking
> that there must be something else going on, or that we had an actual
> problem before this.

I am not sure what I managed to screw up.
> 
> > --- a/xen/include/asm-x86/bug.h
> > +++ b/xen/include/asm-x86/bug.h
> > @@ -98,6 +98,7 @@ extern const struct bug_frame __start_bug_frames[],
> >      .popsection
> >  
> >      .pushsection .bug_frames.\type, "a", @progbits
> > +        .p2align 2
> >          .L\@bf:
> >          .long (.L\@ud - .L\@bf) + \
> >                 ((\line >> BUG_LINE_LO_WIDTH) << BUG_DISP_WIDTH)
> 
> This ought to be accompanied by removing the enforcing of alignment
> in xen.lds.S.

/me nods. Done!
> 
> Jan
> 

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 02/18] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in.
  2016-09-11 20:35 ` [PATCH v3 02/18] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in Konrad Rzeszutek Wilk
@ 2016-09-16 12:50   ` Julien Grall
  0 siblings, 0 replies; 38+ messages in thread
From: Julien Grall @ 2016-09-16 12:50 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich

Hi Konrad,

On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
> If the payload had the sections mentioned but the hypervisor
> did not support some of them (say on ARM the .ex_table) - instead
> of ignoring them - it should forbid loading of such payload.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>


FWIW:

Reviewed-by: Julien Grall <julien.grall@arm.com>

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 03/18] arm/x86: change [modify, destroy]_xen_mappings to return error
  2016-09-11 20:35 ` [PATCH v3 03/18] arm/x86: change [modify, destroy]_xen_mappings to return error Konrad Rzeszutek Wilk
@ 2016-09-16 13:06   ` Julien Grall
  0 siblings, 0 replies; 38+ messages in thread
From: Julien Grall @ 2016-09-16 13:06 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini, ross.lagerwall
  Cc: andrew.cooper3, Jan Beulich

Hi Konrad,

On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
> The implementation on x86 always returns zero, but
> other platforms may return error values.
>
> Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
> Suggested-by: Julien Grall <julien.grall@arm.com>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>

For the ARM bits:

Reviewed-by: Julien Grall <julien.grall@arm.com>

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3 05/18] arm: poison initmem when it is freed.
  2016-09-11 20:35 ` [PATCH v3 05/18] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
@ 2016-09-16 13:31   ` Julien Grall
  0 siblings, 0 replies; 38+ messages in thread
From: Julien Grall @ 2016-09-16 13:31 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini, ross.lagerwall
  Cc: andrew.cooper3

Hi Konrad,

On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
> The current byte sequence is '0xcc' which makes sense on x86,
> but on ARM it is:
>
> cccccccc        stclgt  12, cr12, [ip], {204}   ; 0xcc
>
> Picking something more ARM applicable such as:
>
> efefefef        svc     0x00efefef

Whilst I agree it is much better to get a trap rather than executing a 
random instruction (though co-processor 12 is reserved and a trap should 
(?) occur), I see two problems with this choice.

Firstly, it might be possible that in the future we decide to use svc in 
the hypervisor (it is dumb but valid use case).

Secondly, AArch64 is using a different set (and therefore encoding). So 
far this encoding is not allocated, but it is not rule out that this 
encoding will not be used in the future.

So I would suggest to point initmem with:
     - AArch32: udf instruction i.e 0xe7f000f0 (see A8.8.247 in ARM DDI 
0406C.c)
     - AArch64: a break point (possibly the encoding AARCH64_BREAK_FAULT 
(see asm-arm/arm64/brk.h).

What do you think?

>
> Creates a nice crash if one executes that code:
> (XEN) CPU1: Unexpected Trap: Supervisor Call
>
> We don't have to worry about Thumb code so this instruction
> is a safe to execute.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> ---
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
> ---
>  xen/arch/arm/mm.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 07e2037..0fa5623 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -995,7 +995,7 @@ void free_init_memory(void)
>      paddr_t pa = virt_to_maddr(__init_begin);
>      unsigned long len = __init_end - __init_begin;
>      set_pte_flags_on_range(__init_begin, len, mg_rw);
> -    memset(__init_begin, 0xcc, len);
> +    memset(__init_begin, 0xef, len);

Regardless the final decision, I think we should document the meaning of 
the value in the code.

>      set_pte_flags_on_range(__init_begin, len, mg_clear);
>      init_domheap_pages(pa, pa + len);
>      printk("Freed %ldkB init memory.\n", (long)(__init_end-__init_begin)>>10);
>

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] Livepatch for ARM 64 and 32.
  2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
                   ` (17 preceding siblings ...)
  2016-09-11 20:35 ` [PATCH v3 18/18] livepatch: arm[32, 64], x86: NOP test-case Konrad Rzeszutek Wilk
@ 2016-09-16 13:35 ` Julien Grall
  2016-09-16 13:41   ` Konrad Rzeszutek Wilk
  18 siblings, 1 reply; 38+ messages in thread
From: Julien Grall @ 2016-09-16 13:35 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk, xen-devel, konrad, sstabellini, ross.lagerwall
  Cc: andrew.cooper3

Hi Konrad,

On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
> v2: [http://www.gossamer-threads.com/lists/xen/devel/444461?page=last]
>  - Addressed the two outstanding concerns: CPU bit feature to test alternative
>    test-case, and errata #843419 on some Cortex-A53

I am a bit confused. I haven't spotted any specific code for this. How 
did you handle it?

Regards,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] Livepatch for ARM 64 and 32.
  2016-09-16 13:35 ` [PATCH v3] Livepatch for ARM 64 and 32 Julien Grall
@ 2016-09-16 13:41   ` Konrad Rzeszutek Wilk
  2016-09-16 13:45     ` Julien Grall
  0 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 13:41 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, sstabellini, ross.lagerwall, andrew.cooper3

On Fri, Sep 16, 2016 at 03:35:21PM +0200, Julien Grall wrote:
> Hi Konrad,
> 
> On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
> > v2: [http://www.gossamer-threads.com/lists/xen/devel/444461?page=last]
> >  - Addressed the two outstanding concerns: CPU bit feature to test alternative
> >    test-case, and errata #843419 on some Cortex-A53
> 
> I am a bit confused. I haven't spotted any specific code for this. How did
> you handle it?

livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH

?

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] Livepatch for ARM 64 and 32.
  2016-09-16 13:41   ` Konrad Rzeszutek Wilk
@ 2016-09-16 13:45     ` Julien Grall
  2016-09-16 14:35       ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 38+ messages in thread
From: Julien Grall @ 2016-09-16 13:45 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, sstabellini, ross.lagerwall, andrew.cooper3



On 16/09/2016 15:41, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 16, 2016 at 03:35:21PM +0200, Julien Grall wrote:
>> Hi Konrad,
>>
>> On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
>>> v2: [http://www.gossamer-threads.com/lists/xen/devel/444461?page=last]
>>>  - Addressed the two outstanding concerns: CPU bit feature to test alternative
>>>    test-case, and errata #843419 on some Cortex-A53
>>
>> I am a bit confused. I haven't spotted any specific code for this. How did
>> you handle it?
>
> livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH

AFAICT, this only handles this first part (CPU bit feature) not the 
errata #843419. Did I miss anything?

Cheers,

-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] Livepatch for ARM 64 and 32.
  2016-09-16 13:45     ` Julien Grall
@ 2016-09-16 14:35       ` Konrad Rzeszutek Wilk
  2016-09-16 14:43         ` Julien Grall
  0 siblings, 1 reply; 38+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 14:35 UTC (permalink / raw)
  To: Julien Grall; +Cc: xen-devel, sstabellini, ross.lagerwall, andrew.cooper3

On Fri, Sep 16, 2016 at 03:45:12PM +0200, Julien Grall wrote:
> 
> 
> On 16/09/2016 15:41, Konrad Rzeszutek Wilk wrote:
> > On Fri, Sep 16, 2016 at 03:35:21PM +0200, Julien Grall wrote:
> > > Hi Konrad,
> > > 
> > > On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
> > > > v2: [http://www.gossamer-threads.com/lists/xen/devel/444461?page=last]
> > > >  - Addressed the two outstanding concerns: CPU bit feature to test alternative
> > > >    test-case, and errata #843419 on some Cortex-A53
> > > 
> > > I am a bit confused. I haven't spotted any specific code for this. How did
> > > you handle it?
> > 
> > livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
> 
> AFAICT, this only handles this first part (CPU bit feature) not the errata
> #843419. Did I miss anything?

My recollection is (from 
Date: Wed, 31 Aug 2016 15:49:55 +0100
From: Julien Grall <julien.grall@arm.com>
To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, xen-devel@lists.xenproject.org, konrad@kernel.org, ross.lagerwall@citrix.com,
        sstabellini@kernel.org, Andrew Cooper <andrew.cooper3@citrix.com>
Subject: Re: [PATCH v2] Livepatch for ARM 64 and 32.
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0
"

">  -  #13 "livepatch: Initial ARM64 support."
>     Need to look in erratum #843419 on some Cortex-A53 and figuring
>     out how to avoid payloads having R_AARCH64_ADR_PREL_PG_HI21 relocations.

I will not considered this has a blocker for this series. Having livepatching
on all the other boards for Xen 4.8 would still be awesome :).
"

So I moved it off to 4.9 list.

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

* Re: [PATCH v3] Livepatch for ARM 64 and 32.
  2016-09-16 14:35       ` Konrad Rzeszutek Wilk
@ 2016-09-16 14:43         ` Julien Grall
  0 siblings, 0 replies; 38+ messages in thread
From: Julien Grall @ 2016-09-16 14:43 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: xen-devel, sstabellini, ross.lagerwall, andrew.cooper3



On 16/09/2016 16:35, Konrad Rzeszutek Wilk wrote:
> On Fri, Sep 16, 2016 at 03:45:12PM +0200, Julien Grall wrote:
>>
>>
>> On 16/09/2016 15:41, Konrad Rzeszutek Wilk wrote:
>>> On Fri, Sep 16, 2016 at 03:35:21PM +0200, Julien Grall wrote:
>>>> Hi Konrad,
>>>>
>>>> On 11/09/2016 22:35, Konrad Rzeszutek Wilk wrote:
>>>>> v2: [http://www.gossamer-threads.com/lists/xen/devel/444461?page=last]
>>>>>  - Addressed the two outstanding concerns: CPU bit feature to test alternative
>>>>>    test-case, and errata #843419 on some Cortex-A53
>>>>
>>>> I am a bit confused. I haven't spotted any specific code for this. How did
>>>> you handle it?
>>>
>>> livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
>>
>> AFAICT, this only handles this first part (CPU bit feature) not the errata
>> #843419. Did I miss anything?
>
> My recollection is (from
> Date: Wed, 31 Aug 2016 15:49:55 +0100
> From: Julien Grall <julien.grall@arm.com>
> To: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>, xen-devel@lists.xenproject.org, konrad@kernel.org, ross.lagerwall@citrix.com,
>         sstabellini@kernel.org, Andrew Cooper <andrew.cooper3@citrix.com>
> Subject: Re: [PATCH v2] Livepatch for ARM 64 and 32.
> User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0
> "
>
> ">  -  #13 "livepatch: Initial ARM64 support."
>>     Need to look in erratum #843419 on some Cortex-A53 and figuring
>>     out how to avoid payloads having R_AARCH64_ADR_PREL_PG_HI21 relocations.
>
> I will not considered this has a blocker for this series. Having livepatching
> on all the other boards for Xen 4.8 would still be awesome :).
> "
>
> So I moved it off to 4.9 list.

Oh, I understood "Addressed the two outstanding concerns" as "It was 
fixed in the code". Maybe I was expecting too much to shrink down my 
todo list ^^.


-- 
Julien Grall

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

^ permalink raw reply	[flat|nested] 38+ messages in thread

end of thread, other threads:[~2016-09-16 14:44 UTC | newest]

Thread overview: 38+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-09-11 20:35 [PATCH v3] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 01/18] arm/x86: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
2016-09-12 15:28   ` Jan Beulich
2016-09-12 15:31     ` Julien Grall
2016-09-13 16:58       ` Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 02/18] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in Konrad Rzeszutek Wilk
2016-09-16 12:50   ` Julien Grall
2016-09-11 20:35 ` [PATCH v3 03/18] arm/x86: change [modify, destroy]_xen_mappings to return error Konrad Rzeszutek Wilk
2016-09-16 13:06   ` Julien Grall
2016-09-11 20:35 ` [PATCH v3 04/18] arm/mm: Introduce modify_xen_mappings Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 05/18] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
2016-09-16 13:31   ` Julien Grall
2016-09-11 20:35 ` [PATCH v3 06/18] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
2016-09-12  7:45   ` Jan Beulich
2016-09-11 20:35 ` [PATCH v3 07/18] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
2016-09-12 15:36   ` Jan Beulich
2016-09-11 20:35 ` [PATCH v3 08/18] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x] Konrad Rzeszutek Wilk
2016-09-12 15:52   ` Jan Beulich
2016-09-11 20:35 ` [PATCH v3 09/18] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
2016-09-12 15:55   ` Jan Beulich
2016-09-11 20:35 ` [PATCH v3 10/18] livepatch: Move test-cases to their own sub-directory in test Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 11/18] livepatch: tests: Move the .name value to .rodata Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 12/18] livepatch: tests: Make them compile under ARM64 Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 13/18] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
2016-09-13  9:14   ` Jan Beulich
2016-09-11 20:35 ` [PATCH v3 14/18] xen/arm32: Add an helper to invalidate all instruction caches Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 15/18] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
2016-09-13  9:21   ` Jan Beulich
2016-09-13 19:26     ` Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 16/18] livepatch: ARM32 support Konrad Rzeszutek Wilk
2016-09-12  7:48   ` Jan Beulich
2016-09-11 20:35 ` [PATCH v3 17/18] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp Konrad Rzeszutek Wilk
2016-09-11 20:35 ` [PATCH v3 18/18] livepatch: arm[32, 64], x86: NOP test-case Konrad Rzeszutek Wilk
2016-09-16 13:35 ` [PATCH v3] Livepatch for ARM 64 and 32 Julien Grall
2016-09-16 13:41   ` Konrad Rzeszutek Wilk
2016-09-16 13:45     ` Julien Grall
2016-09-16 14:35       ` Konrad Rzeszutek Wilk
2016-09-16 14:43         ` Julien Grall

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.