* [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 9:09 ` Jan Beulich
2016-09-19 9:26 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 02/16] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in Konrad Rzeszutek Wilk
` (14 subsequent siblings)
15 siblings, 2 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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.
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.
v4: Depend on "arm64: s/ALTERNATIVE/HAS_ALTERNATIVE/"
Remove the extra newline in arch/x86/Kconfig
---
xen/arch/arm/Kconfig | 3 ---
xen/arch/x86/Kconfig | 2 ++
xen/common/Kconfig | 6 ++++++
xen/common/livepatch.c | 4 +++-
4 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index b188293..2e023d1 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -42,9 +42,6 @@ config ACPI
Advanced Configuration and Power Interface (ACPI) support for Xen is
an alternative to device tree on ARM64.
-config HAS_ALTERNATIVE
- bool
-
config HAS_GICV3
bool
diff --git a/xen/arch/x86/Kconfig b/xen/arch/x86/Kconfig
index 265fd79..96ca2bf 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
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 3729409..c9ceeb0 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -638,7 +638,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 )
{
@@ -669,7 +669,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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]
2016-09-16 16:38 ` [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
@ 2016-09-19 9:09 ` Jan Beulich
2016-09-19 9:26 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Jan Beulich @ 2016-09-19 9:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: sstabellini, ross.lagerwall, Andrew Cooper, Doug Goldstein,
julien.grall, xen-devel
>>> On 16.09.16 at 18:38, <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.
>
> 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>
Relevant 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] 50+ messages in thread
* Re: [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]
2016-09-16 16:38 ` [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
2016-09-19 9:09 ` Jan Beulich
@ 2016-09-19 9:26 ` Julien Grall
2016-09-19 14:04 ` Konrad Rzeszutek Wilk
1 sibling, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 9:26 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Andrew Cooper, Doug Goldstein, Jan Beulich
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk 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.
>
> 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.
> v4: Depend on "arm64: s/ALTERNATIVE/HAS_ALTERNATIVE/"
I can't find this patch on the ML. Did you forget to send it?
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]
2016-09-19 9:26 ` Julien Grall
@ 2016-09-19 14:04 ` Konrad Rzeszutek Wilk
2016-09-19 14:09 ` Julien Grall
0 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 14:04 UTC (permalink / raw)
To: Julien Grall
Cc: sstabellini, Andrew Cooper, Doug Goldstein, ross.lagerwall,
Jan Beulich, xen-devel
On Mon, Sep 19, 2016 at 11:26:19AM +0200, Julien Grall wrote:
> Hi Konrad,
>
> On 16/09/2016 18:38, Konrad Rzeszutek Wilk 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.
> >
> > 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.
> > v4: Depend on "arm64: s/ALTERNATIVE/HAS_ALTERNATIVE/"
>
> I can't find this patch on the ML. Did you forget to send it?
Oh darn. Here it is inline:
From 13bc2e2f1082ac72e89c598443781b5141412e9a Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Tue, 13 Sep 2016 12:45:14 -0400
Subject: [PATCH] arm64: s/ALTERNATIVE/HAS_ALTERNATIVE/
No functional change. Also move the entries in the Kconfig file
to be more in alphabetical order.
Suggested-by: Jan Beulich <JBeulich@suse.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>
Cc: Doug Goldstein <cardoe@cardoe.com>
v4: New submission.
---
xen/arch/arm/Kconfig | 8 ++++----
xen/arch/arm/Makefile | 2 +-
xen/arch/arm/xen.lds.S | 2 +-
xen/include/asm-arm/alternative.h | 4 ++--
xen/include/asm-arm/cpuerrata.h | 4 ++--
5 files changed, 10 insertions(+), 10 deletions(-)
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 797c91f..b188293 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -12,8 +12,8 @@ config ARM_32
config ARM_64
def_bool y
depends on 64BIT
+ select HAS_ALTERNATIVE
select HAS_GICV3
- select ALTERNATIVE
config ARM
def_bool y
@@ -42,16 +42,16 @@ config ACPI
Advanced Configuration and Power Interface (ACPI) support for Xen is
an alternative to device tree on ARM64.
-config HAS_GICV3
+config HAS_ALTERNATIVE
bool
-config ALTERNATIVE
+config HAS_GICV3
bool
endmenu
menu "ARM errata workaround via the alternative framework"
- depends on ALTERNATIVE
+ depends on HAS_ALTERNATIVE
config ARM64_ERRATUM_827319
bool "Cortex-A53: 827319: Data cache clean instructions might cause overlapping transactions to the interconnect"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 64fdf41..61e655b 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -4,7 +4,7 @@ subdir-y += platforms
subdir-$(CONFIG_ARM_64) += efi
subdir-$(CONFIG_ACPI) += acpi
-obj-$(CONFIG_ALTERNATIVE) += alternative.o
+obj-$(CONFIG_HAS_ALTERNATIVE) += alternative.o
obj-y += bootfdt.o
obj-y += cpu.o
obj-y += cpuerrata.o
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 3c5e7ba..47b910d 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -151,7 +151,7 @@ SECTIONS
*(.initcall1.init)
__initcall_end = .;
-#ifdef CONFIG_ALTERNATIVE
+#ifdef CONFIG_HAS_ALTERNATIVE
. = ALIGN(4);
__alt_instructions = .;
*(.altinstructions)
diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
index 9f88fd9..6851217 100644
--- a/xen/include/asm-arm/alternative.h
+++ b/xen/include/asm-arm/alternative.h
@@ -5,7 +5,7 @@
#include <xen/config.h>
#include <xen/kconfig.h>
-#ifdef CONFIG_ALTERNATIVE
+#ifdef CONFIG_HAS_ALTERNATIVE
#ifndef __ASSEMBLY__
@@ -154,7 +154,7 @@ int apply_alternatives(const struct alt_instr *start, const struct alt_instr *en
#define ALTERNATIVE(oldinstr, newinstr, ...) \
_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
-#else /* !CONFIG_ALTERNATIVE */
+#else /* !CONFIG_HAS_ALTERNATIVE */
static inline void apply_alternatives_all(void)
{
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
index 5e35b4f..8c57c6a 100644
--- a/xen/include/asm-arm/cpuerrata.h
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -7,7 +7,7 @@
void check_local_cpu_errata(void);
-#ifdef CONFIG_ALTERNATIVE
+#ifdef CONFIG_HAS_ALTERNATIVE
#define CHECK_WORKAROUND_HELPER(erratum, feature, arch) \
static inline bool_t check_workaround_##erratum(void) \
@@ -27,7 +27,7 @@ static inline bool_t check_workaround_##erratum(void) \
} \
}
-#else /* CONFIG_ALTERNATIVE */
+#else /* CONFIG_HAS_ALTERNATIVE */
#define CHECK_WORKAROUND_HELPER(erratum, feature, arch) \
static inline bool_t check_workaround_##erratum(void) \
--
2.4.11
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]
2016-09-19 14:04 ` Konrad Rzeszutek Wilk
@ 2016-09-19 14:09 ` Julien Grall
2016-09-19 14:43 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:09 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: sstabellini, Andrew Cooper, Doug Goldstein, ross.lagerwall,
Jan Beulich, xen-devel
Hi Konrad,
On 19/09/2016 16:04, Konrad Rzeszutek Wilk wrote:
> On Mon, Sep 19, 2016 at 11:26:19AM +0200, Julien Grall wrote:
>> Hi Konrad,
>>
>> On 16/09/2016 18:38, Konrad Rzeszutek Wilk 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.
>>>
>>> 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.
>>> v4: Depend on "arm64: s/ALTERNATIVE/HAS_ALTERNATIVE/"
>>
>> I can't find this patch on the ML. Did you forget to send it?
>
> Oh darn. Here it is inline:
>
> From 13bc2e2f1082ac72e89c598443781b5141412e9a Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> Date: Tue, 13 Sep 2016 12:45:14 -0400
> Subject: [PATCH] arm64: s/ALTERNATIVE/HAS_ALTERNATIVE/
>
> No functional change. Also move the entries in the Kconfig file
> to be more in alphabetical order.
>
> Suggested-by: Jan Beulich <JBeulich@suse.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>
> Cc: Doug Goldstein <cardoe@cardoe.com>
>
> v4: New submission.
> ---
> xen/arch/arm/Kconfig | 8 ++++----
> xen/arch/arm/Makefile | 2 +-
> xen/arch/arm/xen.lds.S | 2 +-
> xen/include/asm-arm/alternative.h | 4 ++--
> xen/include/asm-arm/cpuerrata.h | 4 ++--
> 5 files changed, 10 insertions(+), 10 deletions(-)
>
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 797c91f..b188293 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -12,8 +12,8 @@ config ARM_32
> config ARM_64
> def_bool y
> depends on 64BIT
> + select HAS_ALTERNATIVE
> select HAS_GICV3
> - select ALTERNATIVE
>
> config ARM
> def_bool y
> @@ -42,16 +42,16 @@ config ACPI
> Advanced Configuration and Power Interface (ACPI) support for Xen is
> an alternative to device tree on ARM64.
>
> -config HAS_GICV3
> +config HAS_ALTERNATIVE
> bool
>
> -config ALTERNATIVE
> +config HAS_GICV3
> bool
Why did you invert HAS_GICV3 and HAS_ALTERNATIVE? This makes the patch
more difficult to read.
Cheers,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* Re: [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE]
2016-09-19 14:09 ` Julien Grall
@ 2016-09-19 14:43 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 14:43 UTC (permalink / raw)
To: Julien Grall
Cc: sstabellini, Andrew Cooper, Doug Goldstein, ross.lagerwall,
Jan Beulich, xen-devel
> > @@ -42,16 +42,16 @@ config ACPI
> > Advanced Configuration and Power Interface (ACPI) support for Xen is
> > an alternative to device tree on ARM64.
> >
> > -config HAS_GICV3
> > +config HAS_ALTERNATIVE
> > bool
> >
> > -config ALTERNATIVE
> > +config HAS_GICV3
> > bool
>
> Why did you invert HAS_GICV3 and HAS_ALTERNATIVE? This makes the patch more
> difficult to read.
To be in alphabetic order. But considering I remove it immediately in
the next patch (as I move it to common/Kconfig) it is kind of pointless.
>
> Cheers,
>
> --
> Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 02/16] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 03/16] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
` (13 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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.
Reviewed-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>
v3: New submission.
v4: Added Julien's Reviewed-by tag.
---
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 c9ceeb0..a4ce8c7 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -638,10 +638,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) )
@@ -668,13 +668,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 ||
@@ -693,8 +697,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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v4 03/16] arm: poison initmem when it is freed.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 01/16] arm/x86/common: Add HAS_[ALTERNATIVE|EX_TABLE] Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 02/16] livepatch: Reject payloads with .alternative or .ex_table if support is not built-in Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 9:35 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 04/16] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
` (12 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: 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
But unfortunatly that may not be a good choice either as in the future
we may want to implement support for it.
Julien suggested that we use a 4-byte insn instruction instead
of trying to work with one byte.
As such on ARM 32 we use the udf instruction (see A8.8.247
in ARM DDI 0406C.c) and on ARM 64 use the AARCH64_BREAK_FAULT
instruction (aka brk instruction).
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>
v3: New submission
v4: Instead of using 0xef, use specific insn for architectures.
---
xen/arch/arm/mm.c | 17 ++++++++++++++++-
1 file changed, 16 insertions(+), 1 deletion(-)
diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
index 07e2037..438bed7 100644
--- a/xen/arch/arm/mm.c
+++ b/xen/arch/arm/mm.c
@@ -994,8 +994,23 @@ void free_init_memory(void)
{
paddr_t pa = virt_to_maddr(__init_begin);
unsigned long len = __init_end - __init_begin;
+ uint32_t insn;
+ unsigned int i, nr = len / sizeof(insn);
+
set_pte_flags_on_range(__init_begin, len, mg_rw);
- memset(__init_begin, 0xcc, len);
+#ifdef CONFIG_ARM_32
+ /* udf instruction i.e (see A8.8.247 in ARM DDI 0406C.c) */
+ insn = 0xe7f000f0;
+#else
+ insn = AARCH64_BREAK_FAULT;
+#endif
+ for ( i = 0; i < nr; i++ )
+ *(__init_begin + i) = insn;
+
+ nr = len % sizeof(insn);
+ if ( nr )
+ memset(__init_begin + len - nr, 0xcc, nr);
+
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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 03/16] arm: poison initmem when it is freed.
2016-09-16 16:38 ` [PATCH v4 03/16] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
@ 2016-09-19 9:35 ` Julien Grall
2016-09-19 14:19 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 9:35 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Hi Konrad,
On 16/09/2016 18:38, 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
>
> Creates a nice crash if one executes that code:
> (XEN) CPU1: Unexpected Trap: Supervisor Call
>
> But unfortunatly that may not be a good choice either as in the future
s/unfortunatly/unfortunately/
> we may want to implement support for it.
>
> Julien suggested that we use a 4-byte insn instruction instead
> of trying to work with one byte.
>
> As such on ARM 32 we use the udf instruction (see A8.8.247
> in ARM DDI 0406C.c) and on ARM 64 use the AARCH64_BREAK_FAULT
> instruction (aka brk instruction).
>
> 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>
>
> v3: New submission
> v4: Instead of using 0xef, use specific insn for architectures.
> ---
> xen/arch/arm/mm.c | 17 ++++++++++++++++-
> 1 file changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> index 07e2037..438bed7 100644
> --- a/xen/arch/arm/mm.c
> +++ b/xen/arch/arm/mm.c
> @@ -994,8 +994,23 @@ void free_init_memory(void)
> {
> paddr_t pa = virt_to_maddr(__init_begin);
> unsigned long len = __init_end - __init_begin;
> + uint32_t insn;
> + unsigned int i, nr = len / sizeof(insn);
> +
> set_pte_flags_on_range(__init_begin, len, mg_rw);
> - memset(__init_begin, 0xcc, len);
> +#ifdef CONFIG_ARM_32
> + /* udf instruction i.e (see A8.8.247 in ARM DDI 0406C.c) */
> + insn = 0xe7f000f0;
> +#else
> + insn = AARCH64_BREAK_FAULT;
> +#endif
> + for ( i = 0; i < nr; i++ )
> + *(__init_begin + i) = insn;
__init_begin is char[], so you will only copy the first byte of the
instruction.
And because of nr = len / sizeof(insn) only 1/4 of the initmem will be
poisoned.
So this should be something like:
uint32_t *p = (uint32_t *)__init_begin;
for ( i = 0; i < nr; i++)
*(p + i) = insn;
> +
> + nr = len % sizeof(insn);
> + if ( nr )
> + memset(__init_begin + len - nr, 0xcc, nr);
I am wondering if we should instead align __init_end to 4-byte. With a
BUILD_BUG_ON in the code to check this assumption.
Any opinions?
> +
> 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] 50+ messages in thread* Re: [PATCH v4 03/16] arm: poison initmem when it is freed.
2016-09-19 9:35 ` Julien Grall
@ 2016-09-19 14:19 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 14:19 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ross.lagerwall, sstabellini
On Mon, Sep 19, 2016 at 11:35:57AM +0200, Julien Grall wrote:
> Hi Konrad,
>
> On 16/09/2016 18:38, 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
> >
> > Creates a nice crash if one executes that code:
> > (XEN) CPU1: Unexpected Trap: Supervisor Call
> >
> > But unfortunatly that may not be a good choice either as in the future
>
> s/unfortunatly/unfortunately/
>
> > we may want to implement support for it.
> >
> > Julien suggested that we use a 4-byte insn instruction instead
> > of trying to work with one byte.
> >
> > As such on ARM 32 we use the udf instruction (see A8.8.247
> > in ARM DDI 0406C.c) and on ARM 64 use the AARCH64_BREAK_FAULT
> > instruction (aka brk instruction).
> >
> > 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>
> >
> > v3: New submission
> > v4: Instead of using 0xef, use specific insn for architectures.
> > ---
> > xen/arch/arm/mm.c | 17 ++++++++++++++++-
> > 1 file changed, 16 insertions(+), 1 deletion(-)
> >
> > diff --git a/xen/arch/arm/mm.c b/xen/arch/arm/mm.c
> > index 07e2037..438bed7 100644
> > --- a/xen/arch/arm/mm.c
> > +++ b/xen/arch/arm/mm.c
> > @@ -994,8 +994,23 @@ void free_init_memory(void)
> > {
> > paddr_t pa = virt_to_maddr(__init_begin);
> > unsigned long len = __init_end - __init_begin;
> > + uint32_t insn;
> > + unsigned int i, nr = len / sizeof(insn);
> > +
> > set_pte_flags_on_range(__init_begin, len, mg_rw);
> > - memset(__init_begin, 0xcc, len);
> > +#ifdef CONFIG_ARM_32
> > + /* udf instruction i.e (see A8.8.247 in ARM DDI 0406C.c) */
> > + insn = 0xe7f000f0;
> > +#else
> > + insn = AARCH64_BREAK_FAULT;
> > +#endif
> > + for ( i = 0; i < nr; i++ )
> > + *(__init_begin + i) = insn;
>
> __init_begin is char[], so you will only copy the first byte of the
> instruction.
>
> And because of nr = len / sizeof(insn) only 1/4 of the initmem will be
> poisoned.
>
> So this should be something like:
>
> uint32_t *p = (uint32_t *)__init_begin;
> for ( i = 0; i < nr; i++)
> *(p + i) = insn;
>
Yes of course!
> > +
> > + nr = len % sizeof(insn);
> > + if ( nr )
> > + memset(__init_begin + len - nr, 0xcc, nr);
>
> I am wondering if we should instead align __init_end to 4-byte. With a
> BUILD_BUG_ON in the code to check this assumption.
The __init_end is already aligned:
175 . = ALIGN(STACK_SIZE);
176 __init_end = .;
So we are good there, but I do like the BUILD_BUG_ON. Let me do that.
>
> Any opinions?
>
> > +
> > 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] 50+ messages in thread
* [PATCH v4 04/16] livepatch: Initial ARM64 support.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (2 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 03/16] arm: poison initmem when it is freed Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 10:26 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
` (11 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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.
Acked-by: Jan Beulich <jbeulich@suse.com> [non-arm parts]
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.
v4: Added Jan's Ack
s/PATCH_INSN_SIZE/ARCH_PATCH_INSN_SIZE/
s/arch_livepatch_insn_len/livepatch_insn_len/
---
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 61e655b..5cee84d 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..49eb69b
--- /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(ARCH_PATCH_INSN_SIZE > sizeof(func->opaque));
+ BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != sizeof(insn));
+
+ ASSERT(vmap_of_xen_text);
+
+ len = 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 = livepatch_insn_len(func) / sizeof(uint32_t);
+ for ( i = 0; i < len; i++ )
+ {
+ uint32_t insn;
+
+ memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), ARCH_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..b4b4b6c 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 % ARCH_PATCH_INSN_SIZE )
+ return -EOPNOTSUPP;
-void arch_livepatch_apply_jmp(struct livepatch_func *func)
-{
-}
+ if ( func->old_size < ARCH_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..929c7d9
--- /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 ARCH_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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 04/16] livepatch: Initial ARM64 support.
2016-09-16 16:38 ` [PATCH v4 04/16] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
@ 2016-09-19 10:26 ` Julien Grall
2016-09-19 14:33 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 10:26 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Andrew Cooper
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
> new file mode 100644
> index 0000000..49eb69b
> --- /dev/null
> +++ b/xen/arch/arm/arm64/livepatch.c
[...]
> +int arch_livepatch_perform_rela(struct livepatch_elf *elf,
> + const struct livepatch_elf_sec *base,
> + const struct livepatch_elf_sec *rela)
> +{
[...]
> +
> + /* MOVW instruction relocations. */
> + case R_AARCH64_MOVW_UABS_G0_NC:
> + overflow_check = false;
Can you add a comment here to mention the fall-through?
> +
> + 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;
Ditto.
> +
> + 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;
Ditto.
> +
> + case R_AARCH64_MOVW_UABS_G2:
> + ovf = reloc_insn_movw(RELOC_OP_ABS, dest, val, 32,
> + AARCH64_INSN_IMM_MOVKZ);
> + 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;
Ditto.
> + case R_AARCH64_ADR_PREL_PG_HI21:
> + ovf = reloc_insn_imm(RELOC_OP_PAGE, dest, val, 12, 21,
> + AARCH64_INSN_IMM_ADR);
> + break;
[...]
> diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
> index 755f596..b4b4b6c 100644
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
[...]
> void arch_livepatch_revive(void)
> {
> + /*
> + * Nuke the instruction cache. Data cache has been cleaned before in
> + * arch_livepatch_apply_jmp.
I think you forgot to clean text region from the payload. Without that,
you may receive a crash if you have a separate cache for data and
instruction.
> + */
> + invalidate_icache();
> +
> + if ( vmap_of_xen_text )
> + vunmap(vmap_of_xen_text);
> +
> + vmap_of_xen_text = NULL;
> }
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 04/16] livepatch: Initial ARM64 support.
2016-09-19 10:26 ` Julien Grall
@ 2016-09-19 14:33 ` Konrad Rzeszutek Wilk
2016-09-20 9:40 ` Julien Grall
0 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 14:33 UTC (permalink / raw)
To: Julien Grall; +Cc: xen-devel, ross.lagerwall, Andrew Cooper, sstabellini
>
> > void arch_livepatch_revive(void)
> > {
> > + /*
> > + * Nuke the instruction cache. Data cache has been cleaned before in
> > + * arch_livepatch_apply_jmp.
>
> I think you forgot to clean text region from the payload. Without that, you
> may receive a crash if you have a separate cache for data and instruction.
Help me out here please.
Why would we need to call clean_and_invalidate_dcache_va_range on the
payload .text area (the func->new_addr and func->new_size)?
We don't modify that .text area and after this function is done
(arch_livepatch_revive) it would be very first time that code would be called.
Hence there would not be any cache remains at all?
Or did you mean the old_addr (the one we just patched?)
If we are reverting it then we just clear at func->old_addr for one
instruction? We could drop the dcache for the func->new_addr (so new
.text code), to explicitly tell the CPU to not waste cache space for
those instructions? Is that what you meant?
Anyhow did this:
diff --git a/xen/arch/arm/arm64/livepatch.c b/xen/arch/arm/arm64/livepatch.c
index 49eb69b..07f0ce7 100644
--- a/xen/arch/arm/arm64/livepatch.c
+++ b/xen/arch/arm/arm64/livepatch.c
@@ -49,7 +49,10 @@ void arch_livepatch_apply_jmp(struct livepatch_func *func)
for ( i = 0; i < len; i++ )
*(new_ptr + i) = insn;
+ /* There should not be _any_ aliasing using vmap's, but just in case. */
clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr) * len);
+ /* And definitly clear the old code. */
+ clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
}
void arch_livepatch_revert_jmp(const struct livepatch_func *func)
@@ -68,6 +71,9 @@ void arch_livepatch_revert_jmp(const struct livepatch_func *func)
*(new_ptr + i) = insn;
}
+ /* There should not be _any_ aliasing using vmap's, but just in case. */
+ clean_and_invalidate_dcache_va_range(new_ptr, sizeof(*new_ptr) * len);
+ /* And definitly clear the old code. */
clean_and_invalidate_dcache_va_range(func->old_addr, sizeof(*new_ptr) * len);
}
And added the invalidation of dcache at old_addr (so the function we
patched).
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 04/16] livepatch: Initial ARM64 support.
2016-09-19 14:33 ` Konrad Rzeszutek Wilk
@ 2016-09-20 9:40 ` Julien Grall
0 siblings, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-20 9:40 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: xen-devel, ross.lagerwall, Andrew Cooper, sstabellini
Hi Konrad,
On 19/09/2016 16:33, Konrad Rzeszutek Wilk wrote:
>>
>>> void arch_livepatch_revive(void)
>>> {
>>> + /*
>>> + * Nuke the instruction cache. Data cache has been cleaned before in
>>> + * arch_livepatch_apply_jmp.
>>
>> I think you forgot to clean text region from the payload. Without that, you
>> may receive a crash if you have a separate cache for data and instruction.
>
> Help me out here please.
>
> Why would we need to call clean_and_invalidate_dcache_va_range on the
> payload .text area (the func->new_addr and func->new_size)?
>
> We don't modify that .text area and after this function is done
> (arch_livepatch_revive) it would be very first time that code would be called.
>
> Hence there would not be any cache remains at all?
Because when you copy the payload to the memory, it will go through the
data cache (the region is cacheable). So until the data cache is
cleaned, the data may not reach the memory.
In the case the data and instruction cache are separated, you may read
invalid instruction from the memory because the data cache have not yet
synced with the memory.
>
> Or did you mean the old_addr (the one we just patched?)
We don't care about the previous function. It will never changed except
for the instruction patched.
>
> If we are reverting it then we just clear at func->old_addr for one
> instruction? We could drop the dcache for the func->new_addr (so new
> .text code), to explicitly tell the CPU to not waste cache space for
> those instructions? Is that what you meant?
The data cache should not contain any cache line of the previous
function because it only contains instructions. However the instructions
cache will contain some of them, but they will be removed by the
invalidate cache instruction in arch_live_revive.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (3 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 04/16] livepatch: Initial ARM64 support Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 9:19 ` Jan Beulich
2016-09-19 13:12 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 06/16] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x] Konrad Rzeszutek Wilk
` (10 subsequent siblings)
15 siblings, 2 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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). And "furthermore would require an register or
memory location to load/store the address to." (From Jan).
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.
v4: s/arch_livepatch_verify_distance/livepatch_verify_distance/
s/LIVEPATCH_ARCH_RANGE/ARCH_LIVEPATCH_RANGE/
v5: Updated commit description with Jan's comment
Ditch the casting of long on calculating offset.
---
docs/misc/livepatch.markdown | 14 +++++++++++++-
xen/arch/arm/arm64/livepatch.c | 1 +
xen/common/livepatch.c | 4 ++++
xen/include/asm-arm/livepatch.h | 11 +++++++++++
xen/include/asm-x86/livepatch.h | 3 +++
xen/include/xen/livepatch.h | 19 +++++++++++++++++--
6 files changed, 49 insertions(+), 3 deletions(-)
diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
index 9e72897..5baaa0a 100644
--- a/docs/misc/livepatch.markdown
+++ b/docs/misc/livepatch.markdown
@@ -1100,7 +1100,7 @@ and no .data or .bss sections.
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
@@ -1134,3 +1134,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 49eb69b..7d593b2 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 livepatch_verify_distance. */
ASSERT(insn != AARCH64_BREAK_FAULT);
new_ptr = func->old_addr - (void *)_start + vmap_of_xen_text;
diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c
index a4ce8c7..38260b9 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -540,6 +540,10 @@ static int prepare_payload(struct payload *payload,
rc = resolve_old_address(f, elf);
if ( rc )
return rc;
+
+ rc = 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 929c7d9..482d74f 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 ARCH_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 ARCH_LIVEPATCH_RANGE SZ_32M
+#else
+/* ARM64: C1.3.2 in ARM DDI 0487A.j */
+#define ARCH_LIVEPATCH_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 5e04aa1..7dfc2e7 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 ARCH_PATCH_INSN_SIZE 5
+#define ARCH_LIVEPATCH_RANGE SZ_2G
#endif /* __XEN_X86_LIVEPATCH_H__ */
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index b714fbc..6ea92b5 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
/*
@@ -68,7 +69,7 @@ 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 ARCH_PATCH_INSN_SIZE. */
+#include <asm/livepatch.h> /* For ARCH_[PATCH_INSN_SIZE, LIVEPATCH_RANGE]. */
int arch_livepatch_verify_func(const struct livepatch_func *func);
static inline size_t livepatch_insn_len(const struct livepatch_func *func)
@@ -78,6 +79,21 @@ static inline size_t livepatch_insn_len(const struct livepatch_func *func)
return ARCH_PATCH_INSN_SIZE;
}
+
+static inline int livepatch_verify_distance(const struct livepatch_func *func)
+{
+ long offset;
+ long range = (long)ARCH_LIVEPATCH_RANGE;
+
+ if ( !func->new_addr ) /* Ignore NOPs. */
+ return 0;
+
+ offset = func->old_addr - 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.
@@ -102,7 +118,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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr
2016-09-16 16:38 ` [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
@ 2016-09-19 9:19 ` Jan Beulich
2016-09-19 13:12 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Jan Beulich @ 2016-09-19 9:19 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: sstabellini, ross.lagerwall, Andrew Cooper, julien.grall,
xen-devel
>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
> If the distance is too great we are in trouble - as our relocation
s/great/big/ (or large), as mentioned before?
> @@ -68,7 +69,7 @@ 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 ARCH_PATCH_INSN_SIZE. */
> +#include <asm/livepatch.h> /* For ARCH_[PATCH_INSN_SIZE, LIVEPATCH_RANGE]. */
Perhaps better to drop the comment - it'll get unwieldy to extend it
the same way for the next addition, and it's kind of expected to
include the per-arch header here.
> @@ -78,6 +79,21 @@ static inline size_t livepatch_insn_len(const struct livepatch_func *func)
>
> return ARCH_PATCH_INSN_SIZE;
> }
> +
> +static inline int livepatch_verify_distance(const struct livepatch_func *func)
> +{
> + long offset;
> + long range = (long)ARCH_LIVEPATCH_RANGE;
So you've dropped a few casts, but left this one around. What good
does it do?
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr
2016-09-16 16:38 ` [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
2016-09-19 9:19 ` Jan Beulich
@ 2016-09-19 13:12 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-19 13:12 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Andrew Cooper, Jan Beulich
Hello Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> 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). And "furthermore would require an register or
> memory location to load/store the address to." (From Jan).
>
> On ARM the conditional branch supports even a smaller displacement
> but fortunatly we are not using that.
s/fortunatly/fortunately/
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> ---
[...]
> diff --git a/docs/misc/livepatch.markdown b/docs/misc/livepatch.markdown
> index 9e72897..5baaa0a 100644
> --- a/docs/misc/livepatch.markdown
> +++ b/docs/misc/livepatch.markdown
> @@ -1100,7 +1100,7 @@ and no .data or .bss sections.
> 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
> @@ -1134,3 +1134,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.
The opcode/encoding mentioned is wrong for AArch64. Anyway, I am not
sure why you want to mention the opcode in the documentation. I think it
would be enough to specify: "unconditional branch instruction (for the
encoding see the ARM ARM).".
> 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.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 06/16] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x]
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (4 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 05/16] livepatch: ARM/x86: Check displacement of old_addr and new_addr Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
` (9 subsequent siblings)
15 siblings, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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/
v4: s/bool_t/bool/
---
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 b4b4b6c..a87d48c 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 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 d4ac61f..efc487a 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 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 38260b9..25d9865 100644
--- a/xen/common/livepatch.c
+++ b/xen/common/livepatch.c
@@ -744,7 +744,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 6ea92b5..03abc5b 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -48,6 +48,8 @@ bool_t is_patch(const void *addr);
/* Arch hooks. */
int arch_livepatch_verify_elf(const struct livepatch_elf *elf);
+bool 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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (5 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 06/16] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x] Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 9:27 ` Jan Beulich
2016-09-16 16:38 ` [PATCH v4 08/16] livepatch: Move test-cases to their own sub-directory in test Konrad Rzeszutek Wilk
` (8 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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).
v4: Use bool instead of int for return
Update comment in common code about ARM odd symbols.
s/_check/_deny/ to make it more clear.
---
xen/arch/arm/livepatch.c | 14 ++++++++++++++
xen/arch/x86/livepatch.c | 7 +++++++
xen/common/livepatch_elf.c | 7 +++++++
xen/include/xen/livepatch.h | 2 ++
4 files changed, 30 insertions(+)
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c
index a87d48c..13d6c10 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
return true;
}
+bool arch_livepatch_symbol_deny(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 true;
+#endif
+ return false;
+}
+
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 efc487a..79be833 100644
--- a/xen/arch/x86/livepatch.c
+++ b/xen/arch/x86/livepatch.c
@@ -125,6 +125,13 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
return true;
}
+bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf,
+ const struct livepatch_elf_sym *sym)
+{
+ /* No special checks on x86. */
+ return false;
+}
+
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 79c290e..a4f6794 100644
--- a/xen/common/livepatch_elf.c
+++ b/xen/common/livepatch_elf.c
@@ -251,6 +251,13 @@ static int elf_get_sym(struct livepatch_elf *elf, const void *data)
sym[i].sym = s;
sym[i].name = strtab_sec->data + delta;
+ /* e.g. On ARM we should NEVER see $t* symbols. */
+ if ( arch_livepatch_symbol_deny(elf, &sym[i]) )
+ {
+ dprintk(XENLOG_ERR, LIVEPATCH "%s: Symbol '%s' should not be in payload!\n",
+ elf->name, sym[i].name);
+ return -EINVAL;
+ }
}
elf->nsym = nsym;
diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h
index 03abc5b..d107a75 100644
--- a/xen/include/xen/livepatch.h
+++ b/xen/include/xen/livepatch.h
@@ -50,6 +50,8 @@ bool_t is_patch(const void *addr);
int arch_livepatch_verify_elf(const struct livepatch_elf *elf);
bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
const struct livepatch_elf_sym *sym);
+bool arch_livepatch_symbol_deny(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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-16 16:38 ` [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
@ 2016-09-19 9:27 ` Jan Beulich
2016-09-19 13:33 ` Julien Grall
0 siblings, 1 reply; 50+ messages in thread
From: Jan Beulich @ 2016-09-19 9:27 UTC (permalink / raw)
To: julien.grall, sstabellini, Konrad Rzeszutek Wilk
Cc: Andrew Cooper, xen-devel, ross.lagerwall
>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
> --- a/xen/arch/arm/livepatch.c
> +++ b/xen/arch/arm/livepatch.c
> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
> return true;
> }
>
> +bool arch_livepatch_symbol_deny(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' )
I'm not sure here: Are all symbols starting with $t to be rejected, or
only $t but not e.g. $txyz? According to some other code I have
lying around it ought to be "$t" and any symbols starting with "$t.",
and would be in line with patch 6. But I guess the ARM maintainers
will know best.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 9:27 ` Jan Beulich
@ 2016-09-19 13:33 ` Julien Grall
2016-09-19 14:11 ` Jan Beulich
0 siblings, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 13:33 UTC (permalink / raw)
To: Jan Beulich, sstabellini, Konrad Rzeszutek Wilk
Cc: Andrew Cooper, xen-devel, ross.lagerwall
Hi,
On 19/09/2016 11:27, Jan Beulich wrote:
>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
>> --- a/xen/arch/arm/livepatch.c
>> +++ b/xen/arch/arm/livepatch.c
>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf,
>> return true;
>> }
>>
>> +bool arch_livepatch_symbol_deny(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' )
Please use sym->name[0] for readability. Also, you may want to check the
length of the symbol before checking the second character.
>
> I'm not sure here: Are all symbols starting with $t to be rejected, or
> only $t but not e.g. $txyz? According to some other code I have
> lying around it ought to be "$t" and any symbols starting with "$t.",
> and would be in line with patch 6. But I guess the ARM maintainers
> will know best.
Only $t and $t.* should be rejected. All the others may be valid in the
future.
Looking at the spec, I am wondering if we should also check the type and
binding of the symbols. I have the impression that the naming is
specific to STT_NOTYPE and STB_LOCAL. Any opinions?
Similar question for the previous patch (#6).
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 13:33 ` Julien Grall
@ 2016-09-19 14:11 ` Jan Beulich
2016-09-19 14:13 ` Julien Grall
0 siblings, 1 reply; 50+ messages in thread
From: Jan Beulich @ 2016-09-19 14:11 UTC (permalink / raw)
To: Julien Grall; +Cc: sstabellini, Andrew Cooper, ross.lagerwall, xen-devel
>>> On 19.09.16 at 15:33, <julien.grall@arm.com> wrote:
> Hi,
>
> On 19/09/2016 11:27, Jan Beulich wrote:
>>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
>>> --- a/xen/arch/arm/livepatch.c
>>> +++ b/xen/arch/arm/livepatch.c
>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf
> *elf,
>>> return true;
>>> }
>>>
>>> +bool arch_livepatch_symbol_deny(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' )
>
> Please use sym->name[0] for readability. Also, you may want to check the
> length of the symbol before checking the second character.
Why would the length check be needed? If the first character is $,
then looking at the second one is always valid (and it being nul will
be properly dealt with by the expression above).
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 14:11 ` Jan Beulich
@ 2016-09-19 14:13 ` Julien Grall
2016-09-19 14:48 ` Jan Beulich
0 siblings, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:13 UTC (permalink / raw)
To: Jan Beulich; +Cc: sstabellini, Andrew Cooper, ross.lagerwall, xen-devel
On 19/09/2016 16:11, Jan Beulich wrote:
>>>> On 19.09.16 at 15:33, <julien.grall@arm.com> wrote:
>> On 19/09/2016 11:27, Jan Beulich wrote:
>>>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
>>>> --- a/xen/arch/arm/livepatch.c
>>>> +++ b/xen/arch/arm/livepatch.c
>>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf
>> *elf,
>>>> return true;
>>>> }
>>>>
>>>> +bool arch_livepatch_symbol_deny(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' )
>>
>> Please use sym->name[0] for readability. Also, you may want to check the
>> length of the symbol before checking the second character.
>
> Why would the length check be needed? If the first character is $,
> then looking at the second one is always valid (and it being nul will
> be properly dealt with by the expression above).
Because you may have a payload which is not valid? Or maybe you consider
that all the payload are trusted.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 14:13 ` Julien Grall
@ 2016-09-19 14:48 ` Jan Beulich
2016-09-19 17:32 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 50+ messages in thread
From: Jan Beulich @ 2016-09-19 14:48 UTC (permalink / raw)
To: Julien Grall; +Cc: sstabellini, Andrew Cooper, ross.lagerwall, xen-devel
>>> On 19.09.16 at 16:13, <julien.grall@arm.com> wrote:
>
> On 19/09/2016 16:11, Jan Beulich wrote:
>>>>> On 19.09.16 at 15:33, <julien.grall@arm.com> wrote:
>>> On 19/09/2016 11:27, Jan Beulich wrote:
>>>>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
>>>>> --- a/xen/arch/arm/livepatch.c
>>>>> +++ b/xen/arch/arm/livepatch.c
>>>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf
>>> *elf,
>>>>> return true;
>>>>> }
>>>>>
>>>>> +bool arch_livepatch_symbol_deny(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' )
>>>
>>> Please use sym->name[0] for readability. Also, you may want to check the
>>> length of the symbol before checking the second character.
>>
>> Why would the length check be needed? If the first character is $,
>> then looking at the second one is always valid (and it being nul will
>> be properly dealt with by the expression above).
>
> Because you may have a payload which is not valid? Or maybe you consider
> that all the payload are trusted.
If all symbols' names are inside their string tables and the string
tables are both contained inside the image and have a zero byte
at their end (all of which gets verified afair), nothing bad can
happen I think.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 14:48 ` Jan Beulich
@ 2016-09-19 17:32 ` Konrad Rzeszutek Wilk
2016-09-20 7:00 ` Jan Beulich
2016-09-20 9:44 ` Julien Grall
0 siblings, 2 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 17:32 UTC (permalink / raw)
To: Jan Beulich
Cc: sstabellini, ross.lagerwall, Andrew Cooper, Julien Grall,
xen-devel
On Mon, Sep 19, 2016 at 08:48:10AM -0600, Jan Beulich wrote:
> >>> On 19.09.16 at 16:13, <julien.grall@arm.com> wrote:
>
> >
> > On 19/09/2016 16:11, Jan Beulich wrote:
> >>>>> On 19.09.16 at 15:33, <julien.grall@arm.com> wrote:
> >>> On 19/09/2016 11:27, Jan Beulich wrote:
> >>>>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
> >>>>> --- a/xen/arch/arm/livepatch.c
> >>>>> +++ b/xen/arch/arm/livepatch.c
> >>>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf
> >>> *elf,
> >>>>> return true;
> >>>>> }
> >>>>>
> >>>>> +bool arch_livepatch_symbol_deny(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' )
> >>>
> >>> Please use sym->name[0] for readability. Also, you may want to check the
> >>> length of the symbol before checking the second character.
> >>
> >> Why would the length check be needed? If the first character is $,
> >> then looking at the second one is always valid (and it being nul will
> >> be properly dealt with by the expression above).
> >
> > Because you may have a payload which is not valid? Or maybe you consider
> > that all the payload are trusted.
>
> If all symbols' names are inside their string tables and the string
> tables are both contained inside the image and have a zero byte
> at their end (all of which gets verified afair), nothing bad can
> happen I think.
Exactly. All of those checks are done so we are sure that the
sym->name[0] points to something.
Julien, I can use strlen if you prefer, so it would be like so:
bool arch_livepatch_symbol_deny(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[0] == '$' && sym->name[1] == 't' )
{
size_t len = strlen(sym->name);
if ( (len >= 3 && (sym->name[2] == '.')) || len == 2 )
return true;
}
#endif
return false;
}
Or this way:
bool arch_livepatch_symbol_deny(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[0] == '$' && sym->name[1] == 't' )
{
if ( sym->name[2] && sym->name[2] != '.' )
return false;
return true;
}
#endif
return false;
}
Both add exactly the same amount of lines of code :-)
>
> Jan
>
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 17:32 ` Konrad Rzeszutek Wilk
@ 2016-09-20 7:00 ` Jan Beulich
2016-09-20 9:44 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Jan Beulich @ 2016-09-20 7:00 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: sstabellini, ross.lagerwall, Andrew Cooper, Julien Grall,
xen-devel
>>> On 19.09.16 at 19:32, <konrad.wilk@oracle.com> wrote:
> On Mon, Sep 19, 2016 at 08:48:10AM -0600, Jan Beulich wrote:
>> >>> On 19.09.16 at 16:13, <julien.grall@arm.com> wrote:
>>
>> >
>> > On 19/09/2016 16:11, Jan Beulich wrote:
>> >>>>> On 19.09.16 at 15:33, <julien.grall@arm.com> wrote:
>> >>> On 19/09/2016 11:27, Jan Beulich wrote:
>> >>>>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
>> >>>>> --- a/xen/arch/arm/livepatch.c
>> >>>>> +++ b/xen/arch/arm/livepatch.c
>> >>>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct
> livepatch_elf
>> >>> *elf,
>> >>>>> return true;
>> >>>>> }
>> >>>>>
>> >>>>> +bool arch_livepatch_symbol_deny(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' )
>> >>>
>> >>> Please use sym->name[0] for readability. Also, you may want to check the
>> >>> length of the symbol before checking the second character.
>> >>
>> >> Why would the length check be needed? If the first character is $,
>> >> then looking at the second one is always valid (and it being nul will
>> >> be properly dealt with by the expression above).
>> >
>> > Because you may have a payload which is not valid? Or maybe you consider
>> > that all the payload are trusted.
>>
>> If all symbols' names are inside their string tables and the string
>> tables are both contained inside the image and have a zero byte
>> at their end (all of which gets verified afair), nothing bad can
>> happen I think.
>
> Exactly. All of those checks are done so we are sure that the
> sym->name[0] points to something.
>
>
> Julien, I can use strlen if you prefer, so it would be like so:
If I came across this code, I'd be tempted to immediately submit
a patch to rip out that strlen(), so if you ask me - please don't.
Jan
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols.
2016-09-19 17:32 ` Konrad Rzeszutek Wilk
2016-09-20 7:00 ` Jan Beulich
@ 2016-09-20 9:44 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-20 9:44 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, Jan Beulich
Cc: Andrew Cooper, sstabellini, xen-devel, ross.lagerwall
Hi Konrad,
On 19/09/2016 19:32, Konrad Rzeszutek Wilk wrote:
> On Mon, Sep 19, 2016 at 08:48:10AM -0600, Jan Beulich wrote:
>>>>> On 19.09.16 at 16:13, <julien.grall@arm.com> wrote:
>>
>>>
>>> On 19/09/2016 16:11, Jan Beulich wrote:
>>>>>>> On 19.09.16 at 15:33, <julien.grall@arm.com> wrote:
>>>>> On 19/09/2016 11:27, Jan Beulich wrote:
>>>>>>>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
>>>>>>> --- a/xen/arch/arm/livepatch.c
>>>>>>> +++ b/xen/arch/arm/livepatch.c
>>>>>>> @@ -117,6 +117,20 @@ bool arch_livepatch_symbol_ok(const struct livepatch_elf
>>>>> *elf,
>>>>>>> return true;
>>>>>>> }
>>>>>>>
>>>>>>> +bool arch_livepatch_symbol_deny(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' )
>>>>>
>>>>> Please use sym->name[0] for readability. Also, you may want to check the
>>>>> length of the symbol before checking the second character.
>>>>
>>>> Why would the length check be needed? If the first character is $,
>>>> then looking at the second one is always valid (and it being nul will
>>>> be properly dealt with by the expression above).
>>>
>>> Because you may have a payload which is not valid? Or maybe you consider
>>> that all the payload are trusted.
>>
>> If all symbols' names are inside their string tables and the string
>> tables are both contained inside the image and have a zero byte
>> at their end (all of which gets verified afair), nothing bad can
>> happen I think.
>
> Exactly. All of those checks are done so we are sure that the
> sym->name[0] points to something.
>
Hmmm right, sorry for the confusion.
> bool arch_livepatch_symbol_deny(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[0] == '$' && sym->name[1] == 't' )
> {
> if ( sym->name[2] && sym->name[2] != '.' )
> return false;
>
> return true;
> }
> #endif
> return false;
> }
The second one is fine by me with a small change:
if ( sym->name[2] && sym->name[2] != '.' )
return false;
return true;
Could be replaced by:
return ( !sym->name[2] || sym->name[2] == '.' );
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 08/16] livepatch: Move test-cases to their own sub-directory in test.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (6 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 07/16] livepatch/arm/x86: Check payload for for unwelcomed symbols Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-16 16:58 ` Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 09/16] livepatch: tests: Make them compile under ARM64 Konrad Rzeszutek Wilk
` (7 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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 | 67 -----------------------
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 | 67 +++++++++++++++++++++++
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, 319 insertions(+), 316 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 5cee84d..1d9051c 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 48ff843..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,alloc,load,readonly,data,contents -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,alloc,load,readonly,data,contents -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 2700f0e..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 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 = {
- .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 02f3f85..0000000
--- a/xen/arch/x86/test/xen_hello_world.c
+++ /dev/null
@@ -1,67 +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 const char hello_world_patch_this_fnc[] = "xen_extra_version";
-extern const char *xen_hello_world(void);
-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);
-};
-
-static void check_fnc(void)
-{
- printk(KERN_DEBUG "%s: Hi func called %u times\n", __func__, cnt);
- BUG_ON(cnt == 0 || cnt > 2);
-}
-
-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);
-
-LIVEPATCH_UNLOAD_HOOK(check_fnc);
-
-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 78a8f52..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 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 = {
- .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..48ff843
--- /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,alloc,load,readonly,data,contents -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,alloc,load,readonly,data,contents -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..2700f0e
--- /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 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 = {
+ .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..02f3f85
--- /dev/null
+++ b/xen/test/livepatch/xen_hello_world.c
@@ -0,0 +1,67 @@
+/*
+ * 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 const char hello_world_patch_this_fnc[] = "xen_extra_version";
+extern const char *xen_hello_world(void);
+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);
+};
+
+static void check_fnc(void)
+{
+ printk(KERN_DEBUG "%s: Hi func called %u times\n", __func__, cnt);
+ BUG_ON(cnt == 0 || cnt > 2);
+}
+
+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);
+
+LIVEPATCH_UNLOAD_HOOK(check_fnc);
+
+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..78a8f52
--- /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 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 = {
+ .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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 08/16] livepatch: Move test-cases to their own sub-directory in test.
2016-09-16 16:38 ` [PATCH v4 08/16] livepatch: Move test-cases to their own sub-directory in test Konrad Rzeszutek Wilk
@ 2016-09-16 16:58 ` Konrad Rzeszutek Wilk
0 siblings, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:58 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, Jan Beulich
On Fri, Sep 16, 2016 at 12:38:20PM -0400, Konrad Rzeszutek Wilk wrote:
> 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 | 67 -----------------------
> 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 | 67 +++++++++++++++++++++++
> 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, 319 insertions(+), 316 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
ARGH!!!
There has to be some .gitconfig parameter for this.
In the meantime please ignore this patch and instead see this one:
From 50f28785cff34a060ae528dc21493ee41ad55cdd Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Fri, 12 Aug 2016 15:27:58 -0400
Subject: [PATCH v5] livepatch: Move test-cases to their own sub-directory in
test.
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/test/Makefile | 9 +++++++++
xen/{arch/x86/test => test/livepatch}/Makefile | 0
xen/{arch/x86/test => test/livepatch}/xen_bye_world.c | 0
xen/{arch/x86/test => test/livepatch}/xen_bye_world_func.c | 0
xen/{arch/x86/test => test/livepatch}/xen_hello_world.c | 0
xen/{arch/x86/test => test/livepatch}/xen_hello_world_func.c | 0
xen/{arch/x86/test => test/livepatch}/xen_replace_world.c | 0
xen/{arch/x86/test => test/livepatch}/xen_replace_world_func.c | 0
13 files changed, 17 insertions(+), 14 deletions(-)
create mode 100644 xen/test/Makefile
rename xen/{arch/x86/test => test/livepatch}/Makefile (100%)
rename xen/{arch/x86/test => test/livepatch}/xen_bye_world.c (100%)
rename xen/{arch/x86/test => test/livepatch}/xen_bye_world_func.c (100%)
rename xen/{arch/x86/test => test/livepatch}/xen_hello_world.c (100%)
rename xen/{arch/x86/test => test/livepatch}/xen_hello_world_func.c (100%)
rename xen/{arch/x86/test => test/livepatch}/xen_replace_world.c (100%)
rename xen/{arch/x86/test => test/livepatch}/xen_replace_world_func.c (100%)
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 5cee84d..1d9051c 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/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/arch/x86/test/Makefile b/xen/test/livepatch/Makefile
similarity index 100%
rename from xen/arch/x86/test/Makefile
rename to xen/test/livepatch/Makefile
diff --git a/xen/arch/x86/test/xen_bye_world.c b/xen/test/livepatch/xen_bye_world.c
similarity index 100%
rename from xen/arch/x86/test/xen_bye_world.c
rename to xen/test/livepatch/xen_bye_world.c
diff --git a/xen/arch/x86/test/xen_bye_world_func.c b/xen/test/livepatch/xen_bye_world_func.c
similarity index 100%
rename from xen/arch/x86/test/xen_bye_world_func.c
rename to xen/test/livepatch/xen_bye_world_func.c
diff --git a/xen/arch/x86/test/xen_hello_world.c b/xen/test/livepatch/xen_hello_world.c
similarity index 100%
rename from xen/arch/x86/test/xen_hello_world.c
rename to xen/test/livepatch/xen_hello_world.c
diff --git a/xen/arch/x86/test/xen_hello_world_func.c b/xen/test/livepatch/xen_hello_world_func.c
similarity index 100%
rename from xen/arch/x86/test/xen_hello_world_func.c
rename to xen/test/livepatch/xen_hello_world_func.c
diff --git a/xen/arch/x86/test/xen_replace_world.c b/xen/test/livepatch/xen_replace_world.c
similarity index 100%
rename from xen/arch/x86/test/xen_replace_world.c
rename to xen/test/livepatch/xen_replace_world.c
diff --git a/xen/arch/x86/test/xen_replace_world_func.c b/xen/test/livepatch/xen_replace_world_func.c
similarity index 100%
rename from xen/arch/x86/test/xen_replace_world_func.c
rename to xen/test/livepatch/xen_replace_world_func.c
--
2.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread
* [PATCH v4 09/16] livepatch: tests: Make them compile under ARM64
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (7 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 08/16] livepatch: Move test-cases to their own sub-directory in test Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 13:35 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 10/16] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
` (6 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Wei Liu, Konrad Rzeszutek Wilk, George Dunlap, Andrew Cooper,
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.
v4: Rebase on top of "livepatch/tests: Make .livepatch.depends be read-only"
---
xen/test/Makefile | 2 +-
xen/test/livepatch/Makefile | 12 ++++++++++--
xen/test/livepatch/xen_hello_world_func.c | 8 +++++++-
3 files changed, 18 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 48ff843..5db4d9c 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,8 +61,9 @@ $(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,alloc,load,readonly,data,contents -S $@.bin $@
+ --rename-section=.data=.livepatch.depends -S $@.bin $@
rm -f $@.bin
#
@@ -65,7 +73,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,alloc,load,readonly,data,contents -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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 09/16] livepatch: tests: Make them compile under ARM64
2016-09-16 16:38 ` [PATCH v4 09/16] livepatch: tests: Make them compile under ARM64 Konrad Rzeszutek Wilk
@ 2016-09-19 13:35 ` Julien Grall
0 siblings, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-19 13:35 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Wei Liu, George Dunlap, Andrew Cooper, Ian Jackson, Tim Deegan,
Jan Beulich
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> /* 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));
I would prefer if you invert the order between #9 and #10 to avoid
introducing this dummy ALTERNATIVE.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 10/16] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (8 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 09/16] livepatch: tests: Make them compile under ARM64 Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 13:47 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 11/16] xen/arm32: Add an helper to invalidate all instruction caches Konrad Rzeszutek Wilk
` (5 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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
v4: Move the LIVEPATCH_FEATURE to asm-x86/livepatch.h
---
xen/arch/arm/livepatch.c | 3 +++
xen/include/asm-arm/alternative.h | 2 ++
xen/include/asm-arm/cpufeature.h | 5 +++++
xen/include/asm-x86/livepatch.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 13d6c10..b771cb7 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 6851217..cca5f17 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/livepatch.h b/xen/include/asm-x86/livepatch.h
index 7dfc2e7..00aefd2 100644
--- a/xen/include/asm-x86/livepatch.h
+++ b/xen/include/asm-x86/livepatch.h
@@ -10,6 +10,7 @@
#define ARCH_PATCH_INSN_SIZE 5
#define ARCH_LIVEPATCH_RANGE SZ_2G
+#define LIVEPATCH_FEATURE X86_FEATURE_ALWAYS
#endif /* __XEN_X86_LIVEPATCH_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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 10/16] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH
2016-09-16 16:38 ` [PATCH v4 10/16] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
@ 2016-09-19 13:47 ` Julien Grall
0 siblings, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-19 13:47 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Andrew Cooper, Jan Beulich
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> 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
> v4: Move the LIVEPATCH_FEATURE to asm-x86/livepatch.h
> ---
> xen/arch/arm/livepatch.c | 3 +++
> xen/include/asm-arm/alternative.h | 2 ++
> xen/include/asm-arm/cpufeature.h | 5 +++++
> xen/include/asm-x86/livepatch.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 13d6c10..b771cb7 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 6851217..cca5f17 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, ...) ""
> +
Why this change? Nobody should ever use ALTERNATIVE when
CONFIG_ALTERNATIVE is not enabled. This is a call to have a Xen not
patched for a buggy hardware.
Not to mention that a void ALTERNATIVE is really buggy. We use
ALTERNATIVE to switch between two set of instructions. So we always need
at least one integrated in Xen.
> #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
I would rather define the feature livepatch unconditionally to avoid the
definition of ARM_NCAPS twice.
> 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 reason to move the code later on?
> /*
> * 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));
Ah, this is why you need a nop ALTERNATIVE. My comment above still
stands, this definition of ALTERNATIVE maybe be valid for your use case,
but it is invalid for most of the others.
> +#endif
> +#ifdef CONFIG_X86
> + asm(ALTERNATIVE(ASM_NOP8, ASM_NOP1, LIVEPATCH_FEATURE));
> #endif
> return "Hello World";
> }
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 11/16] xen/arm32: Add an helper to invalidate all instruction caches
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (9 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 10/16] livepatch: x86, ARM, alternative: Expose FEATURE_LIVEPATCH Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 14:24 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
` (4 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: 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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 11/16] xen/arm32: Add an helper to invalidate all instruction caches
2016-09-16 16:38 ` [PATCH v4 11/16] xen/arm32: Add an helper to invalidate all instruction caches Konrad Rzeszutek Wilk
@ 2016-09-19 14:24 ` Julien Grall
0 siblings, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:24 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> 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");
This requires a dsb(ish); isb();
Which make me notice that I forgot to add this in the AArch64
counterpart. I will send a patch later on.
> +}
> +
> +/*
> * Flush all hypervisor mappings from the TLB and branch predictor of
> * the local processor.
> *
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (10 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 11/16] xen/arm32: Add an helper to invalidate all instruction caches Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 9:29 ` Jan Beulich
2016-09-19 14:34 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 13/16] livepatch: Initial ARM32 support Konrad Rzeszutek Wilk
` (3 subsequent siblings)
15 siblings, 2 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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:
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
function old new delta
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.
v4: Remove the .ALIGN(4) in xen.lds.S for x86 (the only place needing
that change).
Update the commit description with correct x86 results
Remove the . = ALIGN(4); in linker filer on x86 [the only file needing the change]
---
xen/arch/x86/xen.lds.S | 1 -
xen/include/asm-arm/bug.h | 1 +
xen/include/asm-x86/bug.h | 1 +
3 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index d903c31..7676de9 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -79,7 +79,6 @@ SECTIONS
.rodata : {
_srodata = .;
/* Bug frames table */
- . = ALIGN(4);
__start_bug_frames = .;
*(.bug_frames.0)
__stop_bug_frames_0 = .;
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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-16 16:38 ` [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
@ 2016-09-19 9:29 ` Jan Beulich
2016-09-19 14:34 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Jan Beulich @ 2016-09-19 9:29 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: sstabellini, ross.lagerwall, Andrew Cooper, julien.grall,
xen-devel
>>> On 16.09.16 at 18:38, <konrad.wilk@oracle.com> wrote:
> 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:
>
> add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> function old new delta
>
> 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>
x86 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] 50+ messages in thread
* Re: [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-16 16:38 ` [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
2016-09-19 9:29 ` Jan Beulich
@ 2016-09-19 14:34 ` Julien Grall
2016-09-19 14:35 ` Julien Grall
1 sibling, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:34 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Andrew Cooper, Jan Beulich
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> 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
s/aligment/alignment/
> byte granularity.
>
> On ARM32 it is paramount that the aligment is word-size (4)
Ditto
> 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
Ditto
> architectures and in both C and x86 makes them all the same.
>
> Furthermore on x86 the bloat-o-meter detects that with this
> change:
>
> add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> function old new delta
>
> 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.
> v4: Remove the .ALIGN(4) in xen.lds.S for x86 (the only place needing
> that change).
> Update the commit description with correct x86 results
> Remove the . = ALIGN(4); in linker filer on x86 [the only file needing the change]
> ---
> xen/arch/x86/xen.lds.S | 1 -
> xen/include/asm-arm/bug.h | 1 +
> xen/include/asm-x86/bug.h | 1 +
> 3 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
> index d903c31..7676de9 100644
> --- a/xen/arch/x86/xen.lds.S
> +++ b/xen/arch/x86/xen.lds.S
> @@ -79,7 +79,6 @@ SECTIONS
> .rodata : {
> _srodata = .;
> /* Bug frames table */
> - . = ALIGN(4);
> __start_bug_frames = .;
> *(.bug_frames.0)
> __stop_bug_frames_0 = .;
> 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)
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-19 14:34 ` Julien Grall
@ 2016-09-19 14:35 ` Julien Grall
2016-09-19 20:19 ` Konrad Rzeszutek Wilk
0 siblings, 1 reply; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:35 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Cc: Andrew Cooper, Jan Beulich
On 19/09/2016 16:34, Julien Grall wrote:
> Hi Konrad,
>
> On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
>> 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
>
> s/aligment/alignment/
>
>> byte granularity.
>>
>> On ARM32 it is paramount that the aligment is word-size (4)
>
> Ditto
>
>> 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
>
> Ditto
>
>> architectures and in both C and x86 makes them all the same.
>>
>> Furthermore on x86 the bloat-o-meter detects that with this
>> change:
>>
>> add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
>> function old new delta
>>
>> 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>
I forgot to mention that with those NITs fixed:
Reviewed-by: Julien Grall <julien.grall@arm.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.
>> v4: Remove the .ALIGN(4) in xen.lds.S for x86 (the only place needing
>> that change).
>> Update the commit description with correct x86 results
>> Remove the . = ALIGN(4); in linker filer on x86 [the only file
>> needing the change]
>> ---
>> xen/arch/x86/xen.lds.S | 1 -
>> xen/include/asm-arm/bug.h | 1 +
>> xen/include/asm-x86/bug.h | 1 +
>> 3 files changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
>> index d903c31..7676de9 100644
>> --- a/xen/arch/x86/xen.lds.S
>> +++ b/xen/arch/x86/xen.lds.S
>> @@ -79,7 +79,6 @@ SECTIONS
>> .rodata : {
>> _srodata = .;
>> /* Bug frames table */
>> - . = ALIGN(4);
>> __start_bug_frames = .;
>> *(.bug_frames.0)
>> __stop_bug_frames_0 = .;
>> 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)
>>
>
> Regards,
>
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread* Re: [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-19 14:35 ` Julien Grall
@ 2016-09-19 20:19 ` Konrad Rzeszutek Wilk
2016-09-19 20:26 ` Konrad Rzeszutek Wilk
2016-09-20 9:46 ` Julien Grall
0 siblings, 2 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 20:19 UTC (permalink / raw)
To: Julien Grall
Cc: sstabellini, Andrew Cooper, ross.lagerwall, Jan Beulich,
xen-devel
> > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>
> I forgot to mention that with those NITs fixed:
>
> Reviewed-by: Julien Grall <julien.grall@arm.com>
Thanks.
However I noticed (and I recall having it replaced in earlier
versions so I must have in a hurry missed it), but this:
> > > --- 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" \
[fixed up, your mailer mangled it]
This should have been:
diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
index 773d63e..affe64f 100644
--- a/xen/include/asm-arm/bug.h
+++ b/xen/include/asm-arm/bug.h
@@ -52,7 +52,7 @@ struct bug_frame {
".popsection\n" \
".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
"4:\n" \
- ".align 4\n" \
+ ".p2align 4\n" \
".long (1b - 4b)\n" \
".long (2b - 4b)\n" \
".long (3b - 4b)\n" \
So that it would be in sync with x86 version.
The end result is exactly the same - it is just that p2align is
preferred because it has the same semantics across platforms while
.align differs.
I made the change (along with your recommendations) and put your
Reviewed-by. I hope that is OK.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-19 20:19 ` Konrad Rzeszutek Wilk
@ 2016-09-19 20:26 ` Konrad Rzeszutek Wilk
2016-09-20 9:46 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-19 20:26 UTC (permalink / raw)
To: Julien Grall
Cc: sstabellini, Andrew Cooper, ross.lagerwall, Jan Beulich,
xen-devel
On Mon, Sep 19, 2016 at 04:19:55PM -0400, Konrad Rzeszutek Wilk wrote:
> > > > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
> >
> > I forgot to mention that with those NITs fixed:
> >
> > Reviewed-by: Julien Grall <julien.grall@arm.com>
>
> Thanks.
>
> However I noticed (and I recall having it replaced in earlier
> versions so I must have in a hurry missed it), but this:
>
> > > > --- 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" \
>
> [fixed up, your mailer mangled it]
> This should have been:
>
> diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
> index 773d63e..affe64f 100644
> --- a/xen/include/asm-arm/bug.h
> +++ b/xen/include/asm-arm/bug.h
> @@ -52,7 +52,7 @@ struct bug_frame {
> ".popsection\n" \
> ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
> "4:\n" \
> - ".align 4\n" \
> + ".p2align 4\n" \
Argh. '.p2align 2' !
For reference, here is the full patch:
From ecf25f594118042364367d0bfacb3ee252222046 Mon Sep 17 00:00:00 2001
From: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Date: Wed, 7 Sep 2016 11:57:05 -0400
Subject: [PATCH] bug/x86/arm: Align bug_frames sections.
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 alignment information - hence they end up being the default
byte granularity.
On ARM32 it is paramount that the alignment 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 alignment 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:
add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
function old new delta
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
Reviewed-by: Julien Grall <julien.grall@arm.com>
Acked-by: Jan Beulich <jbeulich@suse.com> [x86 parts]
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.
v4: Remove the .ALIGN(4) in xen.lds.S for x86 (the only place needing
that change).
Update the commit description with correct x86 results
Remove the . = ALIGN(4); in linker filer on x86 [the only file needing the change]
v5: Add Jan's Ack on x86 parts.
v6: Added Julien's Reviewed-by
s/aligment/alignment/
s/align 4/p2align 2/ as the align semnatics varies by platforms, while
p2align is the same across all of them.
---
xen/arch/x86/xen.lds.S | 1 -
xen/include/asm-arm/bug.h | 1 +
xen/include/asm-x86/bug.h | 1 +
3 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/xen/arch/x86/xen.lds.S b/xen/arch/x86/xen.lds.S
index d903c31..7676de9 100644
--- a/xen/arch/x86/xen.lds.S
+++ b/xen/arch/x86/xen.lds.S
@@ -79,7 +79,6 @@ SECTIONS
.rodata : {
_srodata = .;
/* Bug frames table */
- . = ALIGN(4);
__start_bug_frames = .;
*(.bug_frames.0)
__stop_bug_frames_0 = .;
diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
index 68353e1..4704e2d 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" \
+ ".p2align 2\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] 50+ messages in thread* Re: [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections.
2016-09-19 20:19 ` Konrad Rzeszutek Wilk
2016-09-19 20:26 ` Konrad Rzeszutek Wilk
@ 2016-09-20 9:46 ` Julien Grall
1 sibling, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-20 9:46 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk
Cc: sstabellini, Andrew Cooper, ross.lagerwall, Jan Beulich,
xen-devel
Hi Konrad,
On 19/09/2016 22:19, Konrad Rzeszutek Wilk wrote:
>>>> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
>>
>> I forgot to mention that with those NITs fixed:
>>
>> Reviewed-by: Julien Grall <julien.grall@arm.com>
>
> Thanks.
>
> However I noticed (and I recall having it replaced in earlier
> versions so I must have in a hurry missed it), but this:
>
>>>> --- 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" \
>
> [fixed up, your mailer mangled it]
> This should have been:
>
> diff --git a/xen/include/asm-arm/bug.h b/xen/include/asm-arm/bug.h
> index 773d63e..affe64f 100644
> --- a/xen/include/asm-arm/bug.h
> +++ b/xen/include/asm-arm/bug.h
> @@ -52,7 +52,7 @@ struct bug_frame {
> ".popsection\n" \
> ".pushsection .bug_frames." __stringify(type) ", \"a\", %progbits\n"\
> "4:\n" \
> - ".align 4\n" \
> + ".p2align 4\n" \
> ".long (1b - 4b)\n" \
> ".long (2b - 4b)\n" \
> ".long (3b - 4b)\n" \
>
>
> So that it would be in sync with x86 version.
>
> The end result is exactly the same - it is just that p2align is
> preferred because it has the same semantics across platforms while
> .align differs.
>
> I made the change (along with your recommendations) and put your
> Reviewed-by. I hope that is OK.
I am fine with that.
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 13/16] livepatch: Initial ARM32 support.
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (11 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 12/16] bug/x86/arm: Align bug_frames sections Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 14:39 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 14/16] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp Konrad Rzeszutek Wilk
` (2 subsequent siblings)
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: 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.
Acked-by: Jan Beulich <jbeulich@suse.com> [for non-ARM parts]
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.
v4: Added Jan's Ack.
s/PATCH_INSN_SIZE/ARCH_PATCH_INSN_SIZE/
s/arch_livepatch_insn_len/livepatch_insn_len/
s/LIVEPATCH_ARCH_RANGE/ARCH_LIVEPATCH_RANGE/
---
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..6ad0ce5 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(ARCH_PATCH_INSN_SIZE > sizeof(func->opaque));
+ BUILD_BUG_ON(ARCH_PATCH_INSN_SIZE != sizeof(insn));
+
+ ASSERT(vmap_of_xen_text);
+
+ len = 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)ARCH_LIVEPATCH_RANGE ||
+ delta < (s32)ARCH_LIVEPATCH_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 = livepatch_insn_len(func) / sizeof(uint32_t);
+ for ( i = 0; i < len; i++ )
+ {
+ uint32_t insn;
+
+ memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), ARCH_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)ARCH_LIVEPATCH_RANGE ||
+ (s32)val >= (s32)ARCH_LIVEPATCH_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 7d593b2..d53fe59 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 b771cb7..9c198e9 100644
--- a/xen/arch/arm/livepatch.c
+++ b/xen/arch/arm/livepatch.c
@@ -132,13 +132,6 @@ bool arch_livepatch_symbol_deny(const struct livepatch_elf *elf,
return false;
}
-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 5db4d9c..4d66a40 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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 13/16] livepatch: Initial ARM32 support.
2016-09-16 16:38 ` [PATCH v4 13/16] livepatch: Initial ARM32 support Konrad Rzeszutek Wilk
@ 2016-09-19 14:39 ` Julien Grall
0 siblings, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:39 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Hello Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> The patch piggybacks on: livepatch: Initial ARM64 support, which
> brings up all of the neccessary livepatch infrastructure pieces in.
s/neccessary/necessary/
>
> 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.
>
> Acked-by: Jan Beulich <jbeulich@suse.com> [for non-ARM parts]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
Acked-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] 50+ messages in thread
* [PATCH v4 14/16] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (12 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 13/16] livepatch: Initial ARM32 support Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 14:43 ` Julien Grall
2016-09-16 16:38 ` [PATCH v4 15/16] livepatch: arm[32, 64], x86: NOP test-case Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 16/16] livepatch: In xen_nop test-case remove the .bss and .data sections Konrad Rzeszutek Wilk
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: 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.
v4: s/arch_livepatch_insn_len/livepatch_insn_len/
s/PATCH_INSN_SIZE/ARCH_PATCH_INSN_SIZE/
---
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 6ad0ce5..606a00c 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 = livepatch_insn_len(func) / sizeof(uint32_t);
- for ( i = 0; i < len; i++ )
- {
- uint32_t insn;
-
- memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), ARCH_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 d53fe59..89d061e 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 = livepatch_insn_len(func) / sizeof(uint32_t);
- for ( i = 0; i < len; i++ )
- {
- uint32_t insn;
-
- memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), ARCH_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 9c198e9..c77030e 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 = livepatch_insn_len(func) / sizeof(uint32_t);
+ for ( i = 0; i < len; i++ )
+ {
+ uint32_t insn;
+
+ memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), ARCH_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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* Re: [PATCH v4 14/16] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp
2016-09-16 16:38 ` [PATCH v4 14/16] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp Konrad Rzeszutek Wilk
@ 2016-09-19 14:43 ` Julien Grall
0 siblings, 0 replies; 50+ messages in thread
From: Julien Grall @ 2016-09-19 14:43 UTC (permalink / raw)
To: Konrad Rzeszutek Wilk, xen-devel, konrad, ross.lagerwall,
sstabellini
Hi Konrad,
On 16/09/2016 18:38, Konrad Rzeszutek Wilk wrote:
> It is exactly the same in both platforms.
>
> No functional change.
>
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
With the small change request below:
Acked-by: Julien Grall <julien.grall@arm.com>
> ---
> Cc: Julien Grall <julien.grall@arm.com>
> Cc: Stefano Stabellini <sstabellini@kernel.org>
>
> v3: New submission.
> v4: s/arch_livepatch_insn_len/livepatch_insn_len/
> s/PATCH_INSN_SIZE/ARCH_PATCH_INSN_SIZE/
> ---
> 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 6ad0ce5..606a00c 100644
> --- a/xen/arch/arm/arm32/livepatch.c
> +++ b/xen/arch/arm/arm32/livepatch.c
[...]
> +/* arch_livepatch_revert_jmp shared with ARM 64. */
You use ARM 64 here but ...
>
> 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 d53fe59..89d061e 100644
> --- a/xen/arch/arm/arm64/livepatch.c
> +++ b/xen/arch/arm/arm64/livepatch.c
[...]
> +/* arch_livepatch_revert_jmp shared with ARM32. */
... ARM32 here. Please use either ARM 32/ARM 64 or ARM32/ARM64.
>
> 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 9c198e9..c77030e 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 = livepatch_insn_len(func) / sizeof(uint32_t);
> + for ( i = 0; i < len; i++ )
> + {
> + uint32_t insn;
> +
> + memcpy(&insn, func->opaque + (i * sizeof(uint32_t)), ARCH_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. */
>
Regards,
--
Julien Grall
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply [flat|nested] 50+ messages in thread
* [PATCH v4 15/16] livepatch: arm[32, 64], x86: NOP test-case
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (13 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 14/16] livepatch, arm[32|64]: Share arch_livepatch_revert_jmp Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-16 16:38 ` [PATCH v4 16/16] livepatch: In xen_nop test-case remove the .bss and .data sections Konrad Rzeszutek Wilk
15 siblings, 0 replies; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, 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 4d66a40..d7a4735 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
@@ -92,5 +99,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.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread* [PATCH v4 16/16] livepatch: In xen_nop test-case remove the .bss and .data sections
2016-09-16 16:38 [PATCH v4] Livepatch for ARM 64 and 32 Konrad Rzeszutek Wilk
` (14 preceding siblings ...)
2016-09-16 16:38 ` [PATCH v4 15/16] livepatch: arm[32, 64], x86: NOP test-case Konrad Rzeszutek Wilk
@ 2016-09-16 16:38 ` Konrad Rzeszutek Wilk
2016-09-19 9:32 ` Jan Beulich
15 siblings, 1 reply; 50+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-09-16 16:38 UTC (permalink / raw)
To: xen-devel, konrad, ross.lagerwall, julien.grall, sstabellini
Cc: Andrew Cooper, Jan Beulich, Konrad Rzeszutek Wilk
As they are not needed for the livepatch to work.
Also this allows us to properly test:
"livepatch: Disallow applying after an revert" as the livepatch
will now only have the minimalistic sections:
Section Headers:
[Nr] Name Type Address Offset
Size EntSize Flags Link Info Align
[ 0] NULL 0000000000000000 00000000
0000000000000000 0000000000000000 0 0 0
[ 1] .note.gnu.build-i NOTE 0000000000000000 00000040
0000000000000024 0000000000000000 A 0 0 4
[ 2] .text PROGBITS 0000000000000024 00000064
0000000000000000 0000000000000000 AX 0 0 4
[ 3] .livepatch.depend PROGBITS 0000000000000000 00000064
0000000000000024 0000000000000000 A 0 0 1
[ 4] .livepatch.funcs PROGBITS 0000000000000000 000000a0
0000000000000040 0000000000000000 WA 0 0 32
[ 5] .note.GNU-stack PROGBITS 0000000000000000 000000e0
0000000000000000 0000000000000000 X 0 0 1
[ 6] .comment PROGBITS 0000000000000000 000000e0
000000000000002d 0000000000000001 MS 0 0 1
[ 7] .shstrtab STRTAB 0000000000000000 0000010d
0000000000000071 0000000000000000 0 0 1
[ 8] .symtab SYMTAB 0000000000000000 00000400
00000000000000c0 0000000000000018 9 7 8
[ 9] .strtab STRTAB 0000000000000000 000004c0
000000000000000f 0000000000000000 0 0 1
In which only .livepatch.funcs is write-able.
Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>
---
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
v4: New submission
---
xen/test/livepatch/Makefile | 2 ++
1 file changed, 2 insertions(+)
diff --git a/xen/test/livepatch/Makefile b/xen/test/livepatch/Makefile
index d7a4735..41ad59d 100644
--- a/xen/test/livepatch/Makefile
+++ b/xen/test/livepatch/Makefile
@@ -104,6 +104,8 @@ xen_nop.o: config.h
.PHONY: $(LIVEPATCH_NOP)
$(LIVEPATCH_NOP): xen_nop.o note.o
$(LD) $(LDFLAGS) $(build_id_linker) -r -o $(LIVEPATCH_NOP) $^
+ $(OBJCOPY) --remove-section=.bss --remove-section=.data $@
+ $(OBJCOPY) --strip-unneeded $@
.PHONY: livepatch
livepatch: $(LIVEPATCH) $(LIVEPATCH_BYE) $(LIVEPATCH_REPLACE) $(LIVEPATCH_NOP)
--
2.5.5
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel
^ permalink raw reply related [flat|nested] 50+ messages in thread