* [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* 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 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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
* [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* 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 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
* [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* 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
* [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] 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