All of lore.kernel.org
 help / color / mirror / Atom feed
* [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64
@ 2016-05-05 16:34 Julien Grall
  2016-05-05 16:34 ` [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically Julien Grall
                   ` (17 more replies)
  0 siblings, 18 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Hello,

Some of the processor erratum will require to modify code sequence. As those
modifications may impact the performance, they should only be enabled on
affected cores. Furthermore, Xen may also want to take advantage of new
hardware features coming up with v8.1 and v8.2.

The first part of the series adds the alternative infrastructure, most of the
code is based on Linux v4.6-rc3. The rest of the series implements errata
for Cortex-A57 and Cortex-A53.

Yours sincerely,

Julien Grall (16):
  xen/arm: Makefile: Sort the entries alphabetically
  xen/arm: Include the header asm-arm/system.h in asm-arm/page.h
  xen/arm: Add macros to handle the MIDR
  xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3
  xen/arm: Add cpu_hwcap bitmap
  xen/arm64: Add an helper to invalidate all instruction caches
  xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header
  xen/arm: arm64: Reserve a brk immediate to fault on purpose
  xen/arm: arm64: Add helpers to decode and encode branch instructions
  xen/arm: Introduce alternative runtime patching
  xen/arm: Detect silicon revision and set cap bits accordingly
  xen/arm: Document the errata implemented in Xen
  xen/arm: arm64: Add Cortex-A53 cache errata workaround
  xen/arm: arm64: Add cortex-A57 erratum 832075 workaround
  xen/arm: traps: Don't inject a fault if the translation VA -> IPA
    fails
  xen/arm: arm64: Document Cortex-A57 erratum 834220

 docs/misc/arm/silicon-errata.txt  |  50 +++++++++
 xen/arch/arm/Kconfig              |  96 +++++++++++++++++
 xen/arch/arm/Makefile             |  41 +++----
 xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/arm32/Makefile       |   9 +-
 xen/arch/arm/arm64/Makefile       |  13 ++-
 xen/arch/arm/arm64/cache.S        |  51 +++++++++
 xen/arch/arm/arm64/insn.c         | 213 +++++++++++++++++++++++++++++++++++++
 xen/arch/arm/cpuerrata.c          |  52 +++++++++
 xen/arch/arm/cpufeature.c         |  50 +++++++++
 xen/arch/arm/platforms/Makefile   |   2 +-
 xen/arch/arm/setup.c              |  11 ++
 xen/arch/arm/smpboot.c            |   3 +
 xen/arch/arm/traps.c              |  37 ++++++-
 xen/arch/arm/xen.lds.S            |   7 ++
 xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/brk.h   |  39 +++++++
 xen/include/asm-arm/arm64/bug.h   |   5 +-
 xen/include/asm-arm/arm64/insn.h  |  88 ++++++++++++++++
 xen/include/asm-arm/arm64/io.h    |  21 +++-
 xen/include/asm-arm/arm64/page.h  |  13 ++-
 xen/include/asm-arm/cpuerrata.h   |   6 ++
 xen/include/asm-arm/cpufeature.h  |  47 +++++++++
 xen/include/asm-arm/insn.h        |  22 ++++
 xen/include/asm-arm/page.h        |   3 +
 xen/include/asm-arm/processor.h   |  43 +++++++-
 26 files changed, 1260 insertions(+), 44 deletions(-)
 create mode 100644 docs/misc/arm/silicon-errata.txt
 create mode 100644 xen/arch/arm/alternative.c
 create mode 100644 xen/arch/arm/arm64/insn.c
 create mode 100644 xen/arch/arm/cpuerrata.c
 create mode 100644 xen/arch/arm/cpufeature.c
 create mode 100644 xen/include/asm-arm/alternative.h
 create mode 100644 xen/include/asm-arm/arm64/brk.h
 create mode 100644 xen/include/asm-arm/arm64/insn.h
 create mode 100644 xen/include/asm-arm/cpuerrata.h
 create mode 100644 xen/include/asm-arm/insn.h

-- 
1.9.1


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

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

* [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:34   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 02/16] xen/arm: Include the header asm-arm/system.h in asm-arm/page.h Julien Grall
                   ` (16 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/Makefile           | 38 ++++++++++++++++++++------------------
 xen/arch/arm/arm32/Makefile     |  9 ++++-----
 xen/arch/arm/arm64/Makefile     | 12 +++++-------
 xen/arch/arm/platforms/Makefile |  2 +-
 4 files changed, 30 insertions(+), 31 deletions(-)

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index ead0cc0..8e0d2f6 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -4,42 +4,44 @@ subdir-y += platforms
 subdir-$(CONFIG_ARM_64) += efi
 subdir-$(CONFIG_ACPI) += acpi
 
-obj-$(EARLY_PRINTK) += early_printk.o
+obj-y += bootfdt.o
 obj-y += cpu.o
+obj-y += decode.o
+obj-y += device.o
 obj-y += domain.o
-obj-y += psci.o
-obj-y += vpsci.o
-obj-y += domctl.o
-obj-y += sysctl.o
 obj-y += domain_build.o
-obj-y += gic.o gic-v2.o
+obj-y += domctl.o
+obj-$(EARLY_PRINTK) += early_printk.o
+obj-y += gic.o
+obj-y += gic-v2.o
 obj-$(CONFIG_HAS_GICV3) += gic-v3.o
+obj-y += guestcopy.o
+obj-y += hvm.o
 obj-y += io.o
 obj-y += irq.o
 obj-y += kernel.o
 obj-y += mm.o
 obj-y += p2m.o
 obj-y += percpu.o
-obj-y += guestcopy.o
-obj-y += physdev.o
 obj-y += platform.o
 obj-y += platform_hypercall.o
+obj-y += physdev.o
+obj-y += processor.o
+obj-y += psci.o
 obj-y += setup.o
-obj-y += bootfdt.o
-obj-y += time.o
-obj-y += smpboot.o
-obj-y += smp.o
 obj-y += shutdown.o
+obj-y += smc.o
+obj-y += smp.o
+obj-y += smpboot.o
+obj-y += sysctl.o
+obj-y += time.o
 obj-y += traps.o
-obj-y += vgic.o vgic-v2.o
+obj-y += vgic.o
+obj-y += vgic-v2.o
 obj-$(CONFIG_ARM_64) += vgic-v3.o
 obj-y += vtimer.o
+obj-y += vpsci.o
 obj-y += vuart.o
-obj-y += hvm.o
-obj-y += device.o
-obj-y += decode.o
-obj-y += processor.o
-obj-y += smc.o
 obj-$(CONFIG_XSPLICE) += xsplice.o
 
 #obj-bin-y += ....o
diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
index df0e7de..b20db64 100644
--- a/xen/arch/arm/arm32/Makefile
+++ b/xen/arch/arm/arm32/Makefile
@@ -1,12 +1,11 @@
 subdir-y += lib
 
+obj-$(EARLY_PRINTK) += debug.o
+obj-y += domctl.o
+obj-y += domain.o
 obj-y += entry.o
 obj-y += proc-v7.o proc-caxx.o
-
+obj-y += smpboot.o
 obj-y += traps.o
-obj-y += domain.o
 obj-y += vfp.o
-obj-y += smpboot.o
-obj-y += domctl.o
 
-obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index c7243f5..39c6ac6 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -1,12 +1,10 @@
 subdir-y += lib
 
+obj-y += cache.o
+obj-$(EARLY_PRINTK) += debug.o
+obj-y += domctl.o
+obj-y += domain.o
 obj-y += entry.o
-
+obj-y += smpboot.o
 obj-y += traps.o
-obj-y += domain.o
 obj-y += vfp.o
-obj-y += smpboot.o
-obj-y += domctl.o
-obj-y += cache.o
-
-obj-$(EARLY_PRINTK) += debug.o
diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
index 3689eec..49fa683 100644
--- a/xen/arch/arm/platforms/Makefile
+++ b/xen/arch/arm/platforms/Makefile
@@ -3,8 +3,8 @@ obj-$(CONFIG_ARM_32) += brcm.o
 obj-$(CONFIG_ARM_32) += exynos5.o
 obj-$(CONFIG_ARM_32) += midway.o
 obj-$(CONFIG_ARM_32) += omap5.o
-obj-$(CONFIG_ARM_32) += sunxi.o
 obj-$(CONFIG_ARM_32) += rcar2.o
 obj-$(CONFIG_ARM_64) += seattle.o
+obj-$(CONFIG_ARM_32) += sunxi.o
 obj-$(CONFIG_ARM_64) += xgene-storm.o
 obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
-- 
1.9.1


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

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

* [RFC 02/16] xen/arm: Include the header asm-arm/system.h in asm-arm/page.h
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
  2016-05-05 16:34 ` [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:34   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 03/16] xen/arm: Add macros to handle the MIDR Julien Grall
                   ` (15 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

The header asm-arm/page.h makes use of the macro dsb defined in the header
asm-arm/system.h. Currently, the includer has to specify both of them.

This can be avoided by including asm-arm/system.h in asm-arm/page.h.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/page.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index a94e978..05d9f82 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -84,6 +84,7 @@
 #include <xen/errno.h>
 #include <xen/types.h>
 #include <xen/lib.h>
+#include <asm/system.h>
 
 /* WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
  * level and l4 is the root of the trie, the ARM pagetables follow ARM's
-- 
1.9.1


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

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

* [RFC 03/16] xen/arm: Add macros to handle the MIDR
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
  2016-05-05 16:34 ` [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically Julien Grall
  2016-05-05 16:34 ` [RFC 02/16] xen/arm: Include the header asm-arm/system.h in asm-arm/page.h Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:37   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 04/16] xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3 Julien Grall
                   ` (14 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Add new macros to easily get different parts of the register and to
check if a given MIDR match a CPU model range. The latter will be really
useful to handle errata later.

The macros have been imported from the header arch64/include/asm/cputype.h
in Linux v4.6-rc3

Also remove MIDR_MASK which is unused.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/processor.h | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 6789cd0..1b701c5 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -9,7 +9,40 @@
 #include <public/arch-arm.h>
 
 /* MIDR Main ID Register */
-#define MIDR_MASK    0xff0ffff0
+#define MIDR_REVISION_MASK      0xf
+#define MIDR_RESIVION(midr)     ((midr) & MIDR_REVISION_MASK)
+#define MIDR_PARTNUM_SHIFT      4
+#define MIDR_PARTNUM_MASK       (0xfff << MIDR_PARTNUM_SHIFT)
+#define MIDR_PARTNUM(midr) \
+    (((midr) & MIDR_PARTNUM_MASK) >> MIDR_PARTNUM_SHIFT)
+#define MIDR_ARCHITECTURE_SHIFT 16
+#define MIDR_ARCHITECTURE_MASK  (0xf << MIDR_ARCHITECTURE_SHIFT)
+#define MIDR_ARCHITECTURE(midr) \
+    (((midr) & MIDR_ARCHITECTURE_MASK) >> MIDR_ARCHITECTURE_SHIFT)
+#define MIDR_VARIANT_SHIFT      20
+#define MIDR_VARIANT_MASK       (0xf << MIDR_VARIANT_SHIFT)
+#define MIDR_VARIANT(midr) \
+    (((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
+#define MIDR_IMPLEMENTOR_SHIFT  24
+#define MIDR_IMPLEMENTOR_MASK   (0xff << MIDR_IMPLEMENTOR_SHIFT)
+#define MIDR_IMPLEMENTOR(midr) \
+    (((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
+
+#define MIDR_CPU_MODEL(imp, partnum)            \
+    (((imp)     << MIDR_IMPLEMENTOR_SHIFT) |    \
+     (0xf       << MIDR_ARCHITECTURE_SHIFT) |   \
+     ((partnum) << MIDR_PARTNUM_SHIFT))
+
+#define MIDR_CPU_MODEL_MASK \
+     (MIDR_IMPLEMENTOR_MASK | MIDR_PARTNUM_MASK | MIDR_ARCHITECTURE_MASK)
+
+#define MIDR_IS_CPU_MODEL_RANGE(midr, model, rv_min, rv_max)            \
+({                                                                      \
+        u32 _model = (midr) & MIDR_CPU_MODEL_MASK;                      \
+        u32 _rv = (midr) & (MIDR_REVISION_MASK | MIDR_VARIANT_MASK);    \
+                                                                        \
+        _model == (model) && _rv >= (rv_min) && _rv <= (rv_max);        \
+})
 
 /* MPIDR Multiprocessor Affinity Register */
 #define _MPIDR_UP           (30)
-- 
1.9.1


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

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

* [RFC 04/16] xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (2 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 03/16] xen/arm: Add macros to handle the MIDR Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:40   ` Julien Grall
  2016-05-05 16:34 ` [RFC 05/16] xen/arm: Add cpu_hwcap bitmap Julien Grall
                   ` (13 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Flushing the icache will required when the support Xen patching will be
added.

Also import the macro icache_line_size which is used by
flush_icache_range.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/cache.S | 45 +++++++++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/page.h |  2 ++
 2 files changed, 47 insertions(+)

diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S
index eff4e16..bc5a8f7 100644
--- a/xen/arch/arm/arm64/cache.S
+++ b/xen/arch/arm/arm64/cache.S
@@ -30,6 +30,51 @@
 	.endm
 
 /*
+ * icache_line_size - get the minimum I-cache line size from the CTR register.
+ */
+	.macro	icache_line_size, reg, tmp
+	mrs	\tmp, ctr_el0			// read CTR
+	and	\tmp, \tmp, #0xf		// cache line size encoding
+	mov	\reg, #4			// bytes per word
+	lsl	\reg, \reg, \tmp		// actual cache line size
+	.endm
+
+/*
+ *	flush_icache_range(start,end)
+ *
+ *	Ensure that the I and D caches are coherent within specified region.
+ *	This is typically used when code has been written to a memory region,
+ *	and will be executed.
+ *
+ *	- start   - virtual start address of region
+ *	- end     - virtual end address of region
+ */
+ENTRY(flush_icache_range)
+	dcache_line_size x2, x3
+	sub	x3, x2, #1
+	bic	x4, x0, x3
+1:
+	dc	cvau, x4		// clean D line to PoU
+	add	x4, x4, x2
+	cmp	x4, x1
+	b.lo	1b
+	dsb	ish
+
+	icache_line_size x2, x3
+	sub	x3, x2, #1
+	bic	x4, x0, x3
+1:
+ 	ic	ivau, x4		// invalidate I line PoU
+	add	x4, x4, x2
+	cmp	x4, x1
+	b.lo	1b
+	dsb	ish
+	isb
+	mov	x0, #0
+	ret
+ENDPROC(flush_icache_range)
+
+/*
  *	__flush_dcache_area(kaddr, size)
  *
  *	Ensure that the data held in the page kaddr is written back to the
diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
index 05d9f82..a94e826 100644
--- a/xen/include/asm-arm/page.h
+++ b/xen/include/asm-arm/page.h
@@ -328,6 +328,8 @@ static inline int clean_and_invalidate_dcache_va_range
     return 0;
 }
 
+int flush_icache_range(unsigned long start, unsigned long end);
+
 /* Macros for flushing a single small item.  The predicate is always
  * compile-time constant so this will compile down to 3 instructions in
  * the common case. */
-- 
1.9.1


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

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

* [RFC 05/16] xen/arm: Add cpu_hwcap bitmap
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (3 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 04/16] xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3 Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:53   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 06/16] xen/arm64: Add an helper to invalidate all instruction caches Julien Grall
                   ` (12 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

This will be used to know if a feature, which Xen cares, is available accross
all the CPUs.

This code is a light version of arch/arm64/kernel/cpufeature.c from
Linux v4.6-rc3.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/Makefile            |  1 +
 xen/arch/arm/cpufeature.c        | 34 ++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/cpufeature.h | 29 +++++++++++++++++++++++++++++
 3 files changed, 64 insertions(+)
 create mode 100644 xen/arch/arm/cpufeature.c

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 8e0d2f6..9122f78 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -6,6 +6,7 @@ subdir-$(CONFIG_ACPI) += acpi
 
 obj-y += bootfdt.o
 obj-y += cpu.o
+obj-y += cpufeature.o
 obj-y += decode.o
 obj-y += device.o
 obj-y += domain.o
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
new file mode 100644
index 0000000..7a1b56b
--- /dev/null
+++ b/xen/arch/arm/cpufeature.c
@@ -0,0 +1,34 @@
+/*
+ * Contains CPU feature definitions
+ *
+ * Copyright (C) 2015 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/init.h>
+#include <xen/smp.h>
+#include <asm/cpufeature.h>
+
+DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 7b519cd..2bebad1 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -35,6 +35,35 @@
 #endif
 #define cpu_has_security  (boot_cpu_feature32(security) > 0)
 
+#define ARM_NCAPS           0
+
+#ifndef __ASSEMBLY__
+
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/bitops.h>
+
+extern DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
+
+static inline bool_t cpus_have_cap(unsigned int num)
+{
+    if ( num >= ARM_NCAPS )
+        return 0;
+
+    return test_bit(num, cpu_hwcaps);
+}
+
+static inline void cpus_set_cap(unsigned int num)
+{
+    if (num >= ARM_NCAPS)
+        printk(XENLOG_WARNING "Attempt to set an illegal CPU capability (%d >= %d)\n",
+               num, ARM_NCAPS);
+    else
+        __set_bit(num, cpu_hwcaps);
+}
+
+#endif /* __ASSEMBLY__ */
+
 #endif
 /*
  * Local variables:
-- 
1.9.1


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

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

* [RFC 06/16] xen/arm64: Add an helper to invalidate all instruction caches
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (4 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 05/16] xen/arm: Add cpu_hwcap bitmap Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:50   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 07/16] xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header Julien Grall
                   ` (11 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/arm64/page.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index 29a32cf..fbdc8fb 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -24,6 +24,12 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
  * inline asm operand) */
 #define __clean_and_invalidate_dcache_one(R) "dc  civac, %" #R ";"
 
+/* Invalidate all instruction caches in Inner Shareable domain to PoU */
+static inline void invalidate_icache(void)
+{
+    asm volatile ("ic ialluis");
+}
+
 /*
  * Flush all hypervisor mappings from the TLB of the local processor.
  *
-- 
1.9.1


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

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

* [RFC 07/16] xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (5 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 06/16] xen/arm64: Add an helper to invalidate all instruction caches Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:55   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 08/16] xen/arm: arm64: Reserve a brk immediate to fault on purpose Julien Grall
                   ` (10 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

New immediates will be defined in the future. To keep track of the
immediates allocated, gather all of them in a separate header.

Also rename BRK_BUG_FRAME to BKR_BUG_FRAME_IMM.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c            |  2 +-
 xen/include/asm-arm/arm64/brk.h | 26 ++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/bug.h |  5 ++---
 3 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 xen/include/asm-arm/arm64/brk.h

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 1828ea1..c0325d5 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -1205,7 +1205,7 @@ static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
 
     switch (hsr.brk.comment)
     {
-    case BRK_BUG_FRAME:
+    case BRK_BUG_FRAME_IMM:
         if ( do_bug_frame(regs, regs->pc) )
             goto die;
 
diff --git a/xen/include/asm-arm/arm64/brk.h b/xen/include/asm-arm/arm64/brk.h
new file mode 100644
index 0000000..7867b07
--- /dev/null
+++ b/xen/include/asm-arm/arm64/brk.h
@@ -0,0 +1,26 @@
+/*
+ * Copyright (C) 2016 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ */
+
+#ifndef __ASM_ARM_ARM64_BRK
+#define __ASM_ARM_ARM64_BRK
+
+/*
+ * #imm16 values used for BRK instruction generation
+ * 0x001: xen-mode BUG() and WARN() traps
+ */
+#define BRK_BUG_FRAME_IMM   1
+
+#endif /* !__ASM_ARM_ARM64_BRK */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/include/asm-arm/arm64/bug.h b/xen/include/asm-arm/arm64/bug.h
index 42b0e4f..59f664d 100644
--- a/xen/include/asm-arm/arm64/bug.h
+++ b/xen/include/asm-arm/arm64/bug.h
@@ -2,9 +2,8 @@
 #define __ARM_ARM64_BUG_H__
 
 #include <xen/stringify.h>
+#include <asm/arm64/brk.h>
 
-#define BRK_BUG_FRAME 1
-
-#define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME)
+#define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME_IMM)
 
 #endif /* __ARM_ARM64_BUG_H__ */
-- 
1.9.1


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

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

* [RFC 08/16] xen/arm: arm64: Reserve a brk immediate to fault on purpose
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (6 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 07/16] xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09  9:58   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions Julien Grall
                   ` (9 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

It may not possible to return a proper error when encoding an
instruction. Instead, a handcrafted instruction will be returned.

Also, provide the encoding for the faulting instruction.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/include/asm-arm/arm64/brk.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/xen/include/asm-arm/arm64/brk.h b/xen/include/asm-arm/arm64/brk.h
index 7867b07..04442c4 100644
--- a/xen/include/asm-arm/arm64/brk.h
+++ b/xen/include/asm-arm/arm64/brk.h
@@ -12,8 +12,21 @@
 /*
  * #imm16 values used for BRK instruction generation
  * 0x001: xen-mode BUG() and WARN() traps
+ * 0x002: for triggering a fault on purpose (reserved)
  */
 #define BRK_BUG_FRAME_IMM   1
+#define BRK_FAULT_IMM       2
+
+/*
+ * BRK instruction encoding
+ * The #imm16 value should be placed at bits[20:5] within BRK ins
+ */
+#define AARCH64_BREAK_MON 0xd4200000
+
+/*
+ * BRK instruction for provoking a fault on purpose
+ */
+#define AARCH64_BREAK_FAULT (AARCH64_BREAK_MON | (BRK_FAULT_IMM << 5))
 
 #endif /* !__ASM_ARM_ARM64_BRK */
 /*
-- 
1.9.1


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

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

* [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (7 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 08/16] xen/arm: arm64: Reserve a brk immediate to fault on purpose Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-09 10:05   ` Stefano Stabellini
  2016-05-13 20:35   ` Konrad Rzeszutek Wilk
  2016-05-05 16:34 ` [RFC 10/16] xen/arm: Introduce alternative runtime patching Julien Grall
                   ` (8 subsequent siblings)
  17 siblings, 2 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

We may need to update branch instruction when patching Xen.

The code has been imported from the files arch/arm64/kernel/insn.c
and arch/arm64/include/asm/insn.h in Linux v4.6-rc3.

Note that only the necessary helpers have been imported.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/arm64/Makefile      |   1 +
 xen/arch/arm/arm64/insn.c        | 213 +++++++++++++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/insn.h |  72 +++++++++++++
 xen/include/asm-arm/insn.h       |  22 ++++
 4 files changed, 308 insertions(+)
 create mode 100644 xen/arch/arm/arm64/insn.c
 create mode 100644 xen/include/asm-arm/arm64/insn.h
 create mode 100644 xen/include/asm-arm/insn.h

diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
index 39c6ac6..c1fa43f 100644
--- a/xen/arch/arm/arm64/Makefile
+++ b/xen/arch/arm/arm64/Makefile
@@ -5,6 +5,7 @@ obj-$(EARLY_PRINTK) += debug.o
 obj-y += domctl.o
 obj-y += domain.o
 obj-y += entry.o
+obj-y += insn.o
 obj-y += smpboot.o
 obj-y += traps.o
 obj-y += vfp.o
diff --git a/xen/arch/arm/arm64/insn.c b/xen/arch/arm/arm64/insn.c
new file mode 100644
index 0000000..54a6b62
--- /dev/null
+++ b/xen/arch/arm/arm64/insn.c
@@ -0,0 +1,213 @@
+/*
+ * Based on Linux arch/arm64/kernel/insn.c
+ *
+ * Copyright (C) 2013 Huawei Ltd.
+ * Author: Jiang Liu <liuj97@gmail.com>
+ *
+ * Copyright (C) 2014-2016 Zi Shen Lim <zlim.lnx@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/lib.h>
+#include <xen/errno.h>
+#include <xen/sizes.h>
+#include <xen/bitops.h>
+#include <asm/insn.h>
+#include <asm/arm64/insn.h>
+
+bool aarch64_insn_is_branch_imm(u32 insn)
+{
+	return (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn) ||
+		aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn) ||
+		aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
+		aarch64_insn_is_bcond(insn));
+}
+
+static int aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,
+						u32 *maskp, int *shiftp)
+{
+	u32 mask;
+	int shift;
+
+	switch (type) {
+	case AARCH64_INSN_IMM_26:
+		mask = BIT(26) - 1;
+		shift = 0;
+		break;
+	case AARCH64_INSN_IMM_19:
+		mask = BIT(19) - 1;
+		shift = 5;
+		break;
+	case AARCH64_INSN_IMM_16:
+		mask = BIT(16) - 1;
+		shift = 5;
+		break;
+	case AARCH64_INSN_IMM_14:
+		mask = BIT(14) - 1;
+		shift = 5;
+		break;
+	case AARCH64_INSN_IMM_12:
+		mask = BIT(12) - 1;
+		shift = 10;
+		break;
+	case AARCH64_INSN_IMM_9:
+		mask = BIT(9) - 1;
+		shift = 12;
+		break;
+	case AARCH64_INSN_IMM_7:
+		mask = BIT(7) - 1;
+		shift = 15;
+		break;
+	case AARCH64_INSN_IMM_6:
+	case AARCH64_INSN_IMM_S:
+		mask = BIT(6) - 1;
+		shift = 10;
+		break;
+	case AARCH64_INSN_IMM_R:
+		mask = BIT(6) - 1;
+		shift = 16;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	*maskp = mask;
+	*shiftp = shift;
+
+	return 0;
+}
+
+#define ADR_IMM_HILOSPLIT	2
+#define ADR_IMM_SIZE		SZ_2M
+#define ADR_IMM_LOMASK		((1 << ADR_IMM_HILOSPLIT) - 1)
+#define ADR_IMM_HIMASK		((ADR_IMM_SIZE >> ADR_IMM_HILOSPLIT) - 1)
+#define ADR_IMM_LOSHIFT		29
+#define ADR_IMM_HISHIFT		5
+
+u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
+{
+	u32 immlo, immhi, mask;
+	int shift;
+
+	switch (type) {
+	case AARCH64_INSN_IMM_ADR:
+		shift = 0;
+		immlo = (insn >> ADR_IMM_LOSHIFT) & ADR_IMM_LOMASK;
+		immhi = (insn >> ADR_IMM_HISHIFT) & ADR_IMM_HIMASK;
+		insn = (immhi << ADR_IMM_HILOSPLIT) | immlo;
+		mask = ADR_IMM_SIZE - 1;
+		break;
+	default:
+		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
+			printk(XENLOG_ERR "aarch64_insn_decode_immediate: unknown immediate encoding %d\n",
+			       type);
+			return 0;
+		}
+	}
+
+	return (insn >> shift) & mask;
+}
+
+u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
+				  u32 insn, u64 imm)
+{
+	u32 immlo, immhi, mask;
+	int shift;
+
+	switch (type) {
+	case AARCH64_INSN_IMM_ADR:
+		shift = 0;
+		immlo = (imm & ADR_IMM_LOMASK) << ADR_IMM_LOSHIFT;
+		imm >>= ADR_IMM_HILOSPLIT;
+		immhi = (imm & ADR_IMM_HIMASK) << ADR_IMM_HISHIFT;
+		imm = immlo | immhi;
+		mask = ((ADR_IMM_LOMASK << ADR_IMM_LOSHIFT) |
+			(ADR_IMM_HIMASK << ADR_IMM_HISHIFT));
+		break;
+	default:
+		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
+			printk(XENLOG_ERR "aarch64_insn_encode_immediate: unknown immediate encoding %d\n",
+			       type);
+			return AARCH64_BREAK_FAULT;
+		}
+	}
+
+	/* Update the immediate field. */
+	insn &= ~(mask << shift);
+	insn |= (imm & mask) << shift;
+
+	return insn;
+}
+
+/*
+ * Decode the imm field of a branch, and return the byte offset as a
+ * signed value (so it can be used when computing a new branch
+ * target).
+ */
+s32 aarch64_get_branch_offset(u32 insn)
+{
+	s32 imm;
+
+	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn)) {
+		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
+		return (imm << 6) >> 4;
+	}
+
+	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
+	    aarch64_insn_is_bcond(insn)) {
+		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
+		return (imm << 13) >> 11;
+	}
+
+	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn)) {
+		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
+		return (imm << 18) >> 16;
+	}
+
+	/* Unhandled instruction */
+	BUG();
+}
+
+/*
+ * Encode the displacement of a branch in the imm field and return the
+ * updated instruction.
+ */
+u32 aarch64_set_branch_offset(u32 insn, s32 offset)
+{
+	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn))
+		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
+						     offset >> 2);
+
+	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
+	    aarch64_insn_is_bcond(insn))
+		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
+						     offset >> 2);
+
+	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn))
+		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
+						     offset >> 2);
+
+	/* Unhandled instruction */
+	BUG();
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */
diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
new file mode 100644
index 0000000..cfcdbe9
--- /dev/null
+++ b/xen/include/asm-arm/arm64/insn.h
@@ -0,0 +1,72 @@
+/*
+ * Copyright (C) 2013 Huawei Ltd.
+ * Author: Jiang Liu <liuj97@gmail.com>
+ *
+ * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@gmail.com>
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+#ifndef __ARCH_ARM_ARM64_INSN
+#define __ARCH_ARM_ARM64_INSN
+
+#include <xen/config.h>
+#include <xen/types.h>
+#include <xen/stdbool.h>
+
+enum aarch64_insn_imm_type {
+	AARCH64_INSN_IMM_ADR,
+	AARCH64_INSN_IMM_26,
+	AARCH64_INSN_IMM_19,
+	AARCH64_INSN_IMM_16,
+	AARCH64_INSN_IMM_14,
+	AARCH64_INSN_IMM_12,
+	AARCH64_INSN_IMM_9,
+	AARCH64_INSN_IMM_7,
+	AARCH64_INSN_IMM_6,
+	AARCH64_INSN_IMM_S,
+	AARCH64_INSN_IMM_R,
+	AARCH64_INSN_IMM_MAX
+};
+
+#define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
+static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
+{ return (code & (mask)) == (val); } \
+static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
+{ return (val); }
+
+__AARCH64_INSN_FUNCS(b,		0xFC000000, 0x14000000)
+__AARCH64_INSN_FUNCS(bl,	0xFC000000, 0x94000000)
+__AARCH64_INSN_FUNCS(cbz,	0x7F000000, 0x34000000)
+__AARCH64_INSN_FUNCS(cbnz,	0x7F000000, 0x35000000)
+__AARCH64_INSN_FUNCS(tbz,	0x7F000000, 0x36000000)
+__AARCH64_INSN_FUNCS(tbnz,	0x7F000000, 0x37000000)
+__AARCH64_INSN_FUNCS(bcond,	0xFF000010, 0x54000000)
+
+bool aarch64_insn_is_branch_imm(u32 insn);
+
+u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
+u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
+				  u32 insn, u64 imm);
+
+s32 aarch64_get_branch_offset(u32 insn);
+u32 aarch64_set_branch_offset(u32 insn, s32 offset);
+
+#endif /* !__ARCH_ARM_ARM64_INSN */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */
diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
new file mode 100644
index 0000000..fe9419c
--- /dev/null
+++ b/xen/include/asm-arm/insn.h
@@ -0,0 +1,22 @@
+#ifndef __ARCH_ARM_INSN
+#define __ARCH_ARM_INSN
+
+#include <xen/types.h>
+
+#if defined(CONFIG_ARM_32)
+# include <asm/arm32/insn.h>
+#elif defined(CONFIG_ARM_64)
+# include <asm/arm64/insn.h>
+#else
+# error "unknown ARM variant"
+#endif
+
+#endif /* !__ARCH_ARM_INSN */
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 8
+ * indent-tabs-mode: t
+ * End:
+ */
-- 
1.9.1


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

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

* [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (8 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-13 20:26   ` Konrad Rzeszutek Wilk
  2016-05-21 15:09   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly Julien Grall
                   ` (7 subsequent siblings)
  17 siblings, 2 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Some of the processor erratum will require to modify code sequence.
As those modifications may impact the performance, they should only
be enabled on affected cores. Furthermore, Xen may also want to take
advantage of new hardware features coming up with v8.1 and v8.2.

This patch adds an infrastructure to patch Xen during boot time
depending on the "features" available on the platform.

This code is based on the file arch/arm64/kernel/alternative.c in
Linux 4.6-rc3. Any references to arm64 have been dropped to make the
code as generic as possible.

Furthermore, in Xen the executable sections (.text and .init.text)
are always mapped read-only and enforced by SCTLR.WNX. So it is not
possible to directly patch Xen.

To by-pass this restriction, a temporary writeable mapping is made with
vmap. This mapping will be used to write the new instructions.

Lastly, runtime patching is currently not necessary for ARM32. So the
code is only enabled for ARM64.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/Kconfig              |   5 +
 xen/arch/arm/Makefile             |   1 +
 xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/setup.c              |   8 ++
 xen/arch/arm/xen.lds.S            |   7 ++
 xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
 xen/include/asm-arm/arm64/insn.h  |  16 +++
 7 files changed, 419 insertions(+)
 create mode 100644 xen/arch/arm/alternative.c
 create mode 100644 xen/include/asm-arm/alternative.h

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 6231cd5..9b3e66b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -14,6 +14,7 @@ config ARM_64
 	def_bool y
 	depends on 64BIT
 	select HAS_GICV3
+    select ALTERNATIVE
 
 config ARM
 	def_bool y
@@ -46,6 +47,10 @@ config ACPI
 config HAS_GICV3
 	bool
 
+# Select ALTERNATIVE if the architecture supports runtime patching
+config ALTERNATIVE
+    bool
+
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 9122f78..2d330aa 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -4,6 +4,7 @@ subdir-y += platforms
 subdir-$(CONFIG_ARM_64) += efi
 subdir-$(CONFIG_ACPI) += acpi
 
+obj-$(CONFIG_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.o
 obj-y += cpu.o
 obj-y += cpufeature.o
diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
new file mode 100644
index 0000000..0a5d1c8
--- /dev/null
+++ b/xen/arch/arm/alternative.c
@@ -0,0 +1,217 @@
+/*
+ * alternative runtime patching
+ * inspired by the x86 version
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#include <xen/config.h>
+#include <xen/init.h>
+#include <xen/types.h>
+#include <xen/kernel.h>
+#include <xen/mm.h>
+#include <xen/vmap.h>
+#include <xen/smp.h>
+#include <xen/stop_machine.h>
+#include <asm/alternative.h>
+#include <asm/atomic.h>
+#include <asm/byteorder.h>
+#include <asm/cpufeature.h>
+#include <asm/insn.h>
+#include <asm/page.h>
+
+#define __ALT_PTR(a,f)      (u32 *)((void *)&(a)->f + (a)->f)
+#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
+#define ALT_REPL_PTR(a)     __ALT_PTR(a, alt_offset)
+
+extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
+
+struct alt_region {
+    const struct alt_instr *begin;
+    const struct alt_instr *end;
+};
+
+/*
+ * Check if the target PC is within an alternative block.
+ */
+static bool_t branch_insn_requires_update(const struct alt_instr *alt,
+                                          unsigned long pc)
+{
+    unsigned long replptr;
+
+    if ( is_active_kernel_text(pc) )
+        return 1;
+
+    replptr = (unsigned long)ALT_REPL_PTR(alt);
+    if ( pc >= replptr && pc <= (replptr + alt->alt_len) )
+        return 0;
+
+    /*
+     * Branching into *another* alternate sequence is doomed, and
+     * we're not even trying to fix it up.
+     */
+    BUG();
+}
+
+static u32 get_alt_insn(const struct alt_instr *alt,
+                        const u32 *insnptr, const u32 *altinsnptr)
+{
+    u32 insn;
+
+    insn = le32_to_cpu(*altinsnptr);
+
+    if ( insn_is_branch_imm(insn) )
+    {
+        s32 offset = insn_get_branch_offset(insn);
+        unsigned long target;
+
+        target = (unsigned long)altinsnptr + offset;
+
+        /*
+         * If we're branching inside the alternate sequence,
+         * do not rewrite the instruction, as it is already
+         * correct. Otherwise, generate the new instruction.
+         */
+        if ( branch_insn_requires_update(alt, target) )
+        {
+            offset = target - (unsigned long)insnptr;
+            insn = insn_set_branch_offset(insn, offset);
+        }
+    }
+
+    return insn;
+}
+
+static int __apply_alternatives(const struct alt_region *region)
+{
+    const struct alt_instr *alt;
+    const u32 *origptr, *replptr;
+    u32 *writeptr, *writemap;
+    mfn_t text_mfn = _mfn(virt_to_mfn(_stext));
+    unsigned int text_order = get_order_from_bytes(_end - _start);
+
+    printk("Patching kernel code\n");
+
+    /*
+     * The text section is read-only. So re-map Xen to be able to patch
+     * the code.
+     */
+    writemap = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
+                      VMAP_DEFAULT);
+    if ( !writemap )
+    {
+        printk("alternatives: Unable to map the text section\n");
+        return -ENOMEM;
+    }
+
+    for ( alt = region->begin; alt < region->end; alt++ )
+    {
+        u32 insn;
+        int i, nr_inst;
+
+        if ( !cpus_have_cap(alt->cpufeature) )
+            continue;
+
+        BUG_ON(alt->alt_len != alt->orig_len);
+
+        origptr = ALT_ORIG_PTR(alt);
+        writeptr = origptr - (u32 *)_start + writemap;
+        replptr = ALT_REPL_PTR(alt);
+
+        nr_inst = alt->alt_len / sizeof(insn);
+
+        for ( i = 0; i < nr_inst; i++ )
+        {
+            insn = get_alt_insn(alt, origptr + i, replptr + i);
+            *(writeptr + i) = cpu_to_le32(insn);
+        }
+
+        /* Ensure the new instructions reached the memory and nuke */
+        clean_and_invalidate_dcache_va_range(writeptr,
+                                             (sizeof (*writeptr) * nr_inst));
+    }
+
+    /* Nuke the instruction cache */
+    invalidate_icache();
+
+    vunmap(writemap);
+
+    return 0;
+}
+
+/*
+ * We might be patching the stop_machine state machine, so implement a
+ * really simple polling protocol here.
+ */
+static int __apply_alternatives_multi_stop(void *unused)
+{
+    static int patched = 0;
+    struct alt_region region = {
+        .begin = __alt_instructions,
+        .end = __alt_instructions_end,
+    };
+
+    /* We always have a CPU 0 at this point (__init) */
+    if ( smp_processor_id() )
+    {
+        while ( !read_atomic(&patched) )
+            cpu_relax();
+        isb();
+    }
+    else
+    {
+        int ret;
+
+        BUG_ON(patched);
+        ret = __apply_alternatives(&region);
+        /* The patching is not expected to fail during boot. */
+        BUG_ON(ret != 0);
+
+        /* Barriers provided by the cache flushing */
+        write_atomic(&patched, 1);
+    }
+
+    return 0;
+}
+
+void __init apply_alternatives_all(void)
+{
+    int ret;
+
+	/* better not try code patching on a live SMP system */
+    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL, NR_CPUS);
+
+    /* stop_machine_run should never fail at this stage of the boot */
+    BUG_ON(ret);
+}
+
+int apply_alternatives(void *start, size_t length)
+{
+    struct alt_region region = {
+        .begin = start,
+        .end = start + length,
+    };
+
+    return __apply_alternatives(&region);
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-file-style: "BSD"
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index 09ff1ea..c4389ef 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -38,6 +38,7 @@
 #include <xen/vmap.h>
 #include <xen/libfdt/libfdt.h>
 #include <xen/acpi.h>
+#include <asm/alternative.h>
 #include <asm/page.h>
 #include <asm/current.h>
 #include <asm/setup.h>
@@ -834,6 +835,13 @@ void __init start_xen(unsigned long boot_phys_offset,
 
     do_initcalls();
 
+    /*
+     * It needs to be called after do_initcalls to be able to use
+     * stop_machine (tasklets initialized via an initcall).
+     * XXX: Is it too late?
+     */
+    apply_alternatives_all();
+
     /* Create initial domain 0. */
     /* The vGIC for DOM0 is exactly emulating the hardware GIC */
     config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
index 1f010bd..80d9703 100644
--- a/xen/arch/arm/xen.lds.S
+++ b/xen/arch/arm/xen.lds.S
@@ -167,6 +167,13 @@ SECTIONS
        *(.xsm_initcall.init)
        __xsm_initcall_end = .;
   } :text
+#ifdef CONFIG_ALTERNATIVE
+  .init.alternatives : {
+      __alt_instructions = .;
+      *(.altinstructions)
+      __alt_instructions_end = .;
+  }
+#endif
   __init_end_efi = .;
   . = ALIGN(STACK_SIZE);
   __init_end = .;
diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
new file mode 100644
index 0000000..7e3a610
--- /dev/null
+++ b/xen/include/asm-arm/alternative.h
@@ -0,0 +1,165 @@
+#ifndef __ASM_ALTERNATIVE_H
+#define __ASM_ALTERNATIVE_H
+
+#include <asm/cpufeature.h>
+#include <xen/config.h>
+#include <xen/kconfig.h>
+
+#ifdef CONFIG_ALTERNATIVE
+
+#ifndef __ASSEMBLY__
+
+#include <xen/init.h>
+#include <xen/types.h>
+#include <xen/stringify.h>
+
+struct alt_instr {
+	s32 orig_offset;	/* offset to original instruction */
+	s32 alt_offset;		/* offset to replacement instruction */
+	u16 cpufeature;		/* cpufeature bit set for replacement */
+	u8  orig_len;		/* size of original instruction(s) */
+	u8  alt_len;		/* size of new instruction(s), <= orig_len */
+};
+
+void __init apply_alternatives_all(void);
+int apply_alternatives(void *start, size_t length);
+
+#define ALTINSTR_ENTRY(feature)						      \
+	" .word 661b - .\n"				/* label           */ \
+	" .word 663f - .\n"				/* new instruction */ \
+	" .hword " __stringify(feature) "\n"		/* feature bit     */ \
+	" .byte 662b-661b\n"				/* source len      */ \
+	" .byte 664f-663f\n"				/* replacement len */
+
+/*
+ * alternative assembly primitive:
+ *
+ * If any of these .org directive fail, it means that insn1 and insn2
+ * don't have the same length. This used to be written as
+ *
+ * .if ((664b-663b) != (662b-661b))
+ * 	.error "Alternatives instruction length mismatch"
+ * .endif
+ *
+ * but most assemblers die if insn1 or insn2 have a .inst. This should
+ * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
+ * containing commit 4e4d08cf7399b606 or c1baaddf8861).
+ */
+#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled)	\
+	".if "__stringify(cfg_enabled)" == 1\n"				\
+	"661:\n\t"							\
+	oldinstr "\n"							\
+	"662:\n"							\
+	".pushsection .altinstructions,\"a\"\n"				\
+	ALTINSTR_ENTRY(feature)						\
+	".popsection\n"							\
+	".pushsection .altinstr_replacement, \"a\"\n"			\
+	"663:\n\t"							\
+	newinstr "\n"							\
+	"664:\n\t"							\
+	".popsection\n\t"						\
+	".org	. - (664b-663b) + (662b-661b)\n\t"			\
+	".org	. - (662b-661b) + (664b-663b)\n"			\
+	".endif\n"
+
+#define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...)	\
+	__ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
+
+#else
+
+#include <asm/asm_defns.h>
+
+.macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
+	.word \orig_offset - .
+	.word \alt_offset - .
+	.hword \feature
+	.byte \orig_len
+	.byte \alt_len
+.endm
+
+.macro alternative_insn insn1, insn2, cap, enable = 1
+	.if \enable
+661:	\insn1
+662:	.pushsection .altinstructions, "a"
+	altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
+	.popsection
+	.pushsection .altinstr_replacement, "ax"
+663:	\insn2
+664:	.popsection
+	.org	. - (664b-663b) + (662b-661b)
+	.org	. - (662b-661b) + (664b-663b)
+	.endif
+.endm
+
+/*
+ * Begin an alternative code sequence.
+ *
+ * The code that follows this macro will be assembled and linked as
+ * normal. There are no restrictions on this code.
+ */
+.macro alternative_if_not cap, enable = 1
+	.if \enable
+	.pushsection .altinstructions, "a"
+	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
+	.popsection
+661:
+	.endif
+.endm
+
+/*
+ * Provide the alternative code sequence.
+ *
+ * The code that follows this macro is assembled into a special
+ * section to be used for dynamic patching. Code that follows this
+ * macro must:
+ *
+ * 1. Be exactly the same length (in bytes) as the default code
+ *    sequence.
+ *
+ * 2. Not contain a branch target that is used outside of the
+ *    alternative sequence it is defined in (branches into an
+ *    alternative sequence are not fixed up).
+ */
+.macro alternative_else
+662:	.pushsection .altinstr_replacement, "ax"
+663:
+.endm
+
+/*
+ * Complete an alternative code sequence.
+ */
+.macro alternative_endif
+664:	.popsection
+	.org	. - (664b-663b) + (662b-661b)
+	.org	. - (662b-661b) + (664b-663b)
+.endm
+
+#define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
+	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
+
+#endif  /*  __ASSEMBLY__  */
+
+/*
+ * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature));
+ *
+ * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature, CONFIG_FOO));
+ * N.B. If CONFIG_FOO is specified, but not selected, the whole block
+ *      will be omitted, including oldinstr.
+ */
+#define ALTERNATIVE(oldinstr, newinstr, ...)   \
+	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
+
+#else /* !CONFIG_ALTERNATIVE */
+
+static inline void apply_alternatives_all(void)
+{
+}
+
+int apply_alternatives(void *start, size_t lenght)
+{
+    return 0;
+}
+
+#endif
+
+#endif /* __ASM_ALTERNATIVE_H */
diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
index cfcdbe9..6ce37be 100644
--- a/xen/include/asm-arm/arm64/insn.h
+++ b/xen/include/asm-arm/arm64/insn.h
@@ -61,6 +61,22 @@ u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
 s32 aarch64_get_branch_offset(u32 insn);
 u32 aarch64_set_branch_offset(u32 insn, s32 offset);
 
+/* Wrapper for common code */
+static inline bool insn_is_branch_imm(u32 insn)
+{
+    return aarch64_insn_is_branch_imm(insn);
+}
+
+static inline s32 insn_get_branch_offset(u32 insn)
+{
+    return aarch64_get_branch_offset(insn);
+}
+
+static inline u32 insn_set_branch_offset(u32 insn, s32 offset)
+{
+    return aarch64_set_branch_offset(insn, offset);
+}
+
 #endif /* !__ARCH_ARM_ARM64_INSN */
 /*
  * Local variables:
-- 
1.9.1


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

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

* [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (9 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 10/16] xen/arm: Introduce alternative runtime patching Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-13 20:37   ` Konrad Rzeszutek Wilk
  2016-05-05 16:34 ` [RFC 12/16] xen/arm: Document the errata implemented in Xen Julien Grall
                   ` (6 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

After each CPU has been started, we iterate through a list of CPU
errata to detect CPUs which need from hypervisor code patches.

For each bug there is a function which check if that a particular CPU is
affected. This needs to be done on every CPUs to cover heterogenous
system properly.

If a certain erratum has been detected, the capability bit will be set
and later the call to apply_alternatives() will trigger the actual code
patching.

The code is based on the file arch/arm64/kernel/cpu_errata.c in Linux
v4.6-rc3.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/Makefile            |  1 +
 xen/arch/arm/cpuerrata.c         | 26 ++++++++++++++++++++++++++
 xen/arch/arm/cpufeature.c        | 16 ++++++++++++++++
 xen/arch/arm/setup.c             |  3 +++
 xen/arch/arm/smpboot.c           |  3 +++
 xen/include/asm-arm/cpuerrata.h  |  6 ++++++
 xen/include/asm-arm/cpufeature.h | 15 +++++++++++++++
 7 files changed, 70 insertions(+)
 create mode 100644 xen/arch/arm/cpuerrata.c
 create mode 100644 xen/include/asm-arm/cpuerrata.h

diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 2d330aa..307578c 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -7,6 +7,7 @@ subdir-$(CONFIG_ACPI) += acpi
 obj-$(CONFIG_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.o
 obj-y += cpu.o
+obj-y += cpuerrata.o
 obj-y += cpufeature.o
 obj-y += decode.o
 obj-y += device.o
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
new file mode 100644
index 0000000..52d39f8
--- /dev/null
+++ b/xen/arch/arm/cpuerrata.c
@@ -0,0 +1,26 @@
+#include <xen/config.h>
+#include <asm/cpufeature.h>
+#include <asm/cpuerrata.h>
+
+#define MIDR_RANGE(model, min, max)     \
+    .matches = is_affected_midr_range,  \
+    .midr_model = model,                \
+    .midr_range_min = min,              \
+    .midr_range_max = max
+
+static bool_t __maybe_unused
+is_affected_midr_range(const struct arm_cpu_capabilities *entry)
+{
+    return MIDR_IS_CPU_MODEL_RANGE(boot_cpu_data.midr.bits, entry->midr_model,
+                                   entry->midr_range_min,
+                                   entry->midr_range_max);
+}
+
+static const struct arm_cpu_capabilities arm_errata[] = {
+    {},
+};
+
+void check_local_cpu_errata(void)
+{
+    update_cpu_capabilities(arm_errata, "enabled workaround for");
+}
diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
index 7a1b56b..aa7c5b1 100644
--- a/xen/arch/arm/cpufeature.c
+++ b/xen/arch/arm/cpufeature.c
@@ -24,6 +24,22 @@
 
 DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
 
+void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
+                             const char *info)
+{
+    int i;
+
+    for ( i = 0; caps[i].matches; i++ )
+    {
+        if ( !caps[i].matches(&caps[i]) )
+            continue;
+
+        if ( !cpus_have_cap(caps[i].capability) && caps[i].desc )
+            printk("%s: %s\n", info, caps[i].desc);
+        cpus_set_cap(caps[i].capability);
+    }
+}
+
 /*
  * Local variables:
  * mode: C
diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
index c4389ef..67eb13b 100644
--- a/xen/arch/arm/setup.c
+++ b/xen/arch/arm/setup.c
@@ -43,6 +43,7 @@
 #include <asm/current.h>
 #include <asm/setup.h>
 #include <asm/gic.h>
+#include <asm/cpuerrata.h>
 #include <asm/cpufeature.h>
 #include <asm/platform.h>
 #include <asm/procinfo.h>
@@ -171,6 +172,8 @@ static void __init processor_id(void)
     }
 
     processor_setup();
+
+    check_local_cpu_errata();
 }
 
 void dt_unreserved_regions(paddr_t s, paddr_t e,
diff --git a/xen/arch/arm/smpboot.c b/xen/arch/arm/smpboot.c
index c5109bf..3f5a77b 100644
--- a/xen/arch/arm/smpboot.c
+++ b/xen/arch/arm/smpboot.c
@@ -29,6 +29,7 @@
 #include <xen/timer.h>
 #include <xen/irq.h>
 #include <xen/console.h>
+#include <asm/cpuerrata.h>
 #include <asm/gic.h>
 #include <asm/psci.h>
 #include <asm/acpi.h>
@@ -316,6 +317,8 @@ void start_secondary(unsigned long boot_phys_offset,
     local_irq_enable();
     local_abort_enable();
 
+    check_local_cpu_errata();
+
     printk(XENLOG_DEBUG "CPU %u booted.\n", smp_processor_id());
 
     startup_cpu_idle_loop();
diff --git a/xen/include/asm-arm/cpuerrata.h b/xen/include/asm-arm/cpuerrata.h
new file mode 100644
index 0000000..8ffd7ba
--- /dev/null
+++ b/xen/include/asm-arm/cpuerrata.h
@@ -0,0 +1,6 @@
+#ifndef __ARM_CPUERRATA_H
+#define __ARM_CPUERRATA_H
+
+void check_local_cpu_errata(void);
+
+#endif /* __ARM_CPUERRATA_H */
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 2bebad1..fb57295 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -62,6 +62,21 @@ static inline void cpus_set_cap(unsigned int num)
         __set_bit(num, cpu_hwcaps);
 }
 
+struct arm_cpu_capabilities {
+    const char *desc;
+    u16 capability;
+    bool_t (*matches)(const struct arm_cpu_capabilities *);
+    union {
+        struct {    /* To be used for eratum handling only */
+            u32 midr_model;
+            u32 midr_range_min, midr_range_max;
+        };
+    };
+};
+
+void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
+                             const char *info);
+
 #endif /* __ASSEMBLY__ */
 
 #endif
-- 
1.9.1


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

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

* [RFC 12/16] xen/arm: Document the errata implemented in Xen
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (10 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-05 16:34 ` [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround Julien Grall
                   ` (5 subsequent siblings)
  17 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

The new document will help to keep track of all the erratum that Xen is
able to handle.

The text is based on the Linux doc in Documents/arm64/silicon-errata.txt.

Also list the current errata that Xen is aware of.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 docs/misc/arm/silicon-errata.txt | 45 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 45 insertions(+)
 create mode 100644 docs/misc/arm/silicon-errata.txt

diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
new file mode 100644
index 0000000..3f0d32b
--- /dev/null
+++ b/docs/misc/arm/silicon-errata.txt
@@ -0,0 +1,45 @@
+                Silicon Errata and Software Workarounds
+                =======================================
+
+It is an unfortunate fact of life that hardware is often produced with
+so-called "errata", which can cause it to deviate from the architecture
+under specific circumstances.  For hardware produced by ARM, these
+errata are broadly classified into the following categories:
+
+  Category A: A critical error without a viable workaround.
+  Category B: A significant or critical error with an acceptable
+              workaround.
+  Category C: A minor error that is not expected to occur under normal
+              operation.
+
+For more information, consult one of the "Software Developers Errata
+Notice" documents available on infocenter.arm.com (registration
+required).
+
+As far as Xen is concerned, Category B errata may require some special
+treatment in the hypervisor. For example, avoiding a particular sequence
+of code, or configuring the processor in a particular way. A less common
+situation may require similar actions in order to declassify a Category A
+erratum into a Category C erratum. These are collectively known as
+"software workarounds" and are only required in the minority of cases
+(e.g. those cases that both require a non-secure workaround *and* can
+be triggered by Linux).
+
+For software workarounds that may adversely impact systems unaffected by
+the erratum in question, a Kconfig entry is added under "ARM errata
+workarounds via the alternatives framework". These are enabled by default
+and patched in at runtime when an affected CPU is detected. For
+less-intrusive workarounds, a Kconfig option is not available and the code
+is structured (preferably with a comment) in such a way that the erratum
+will not be hit.
+
+This approach can make it slightly onerous to determine exactly which
+errata are worked around in an arbitrary hypervisor source tree, so this
+file acts as a registry of software workarounds in the Xen hypervisor and
+will be updated when new workarounds are committed and backported to
+stable hypervisors.
+
+| Implementor    | Component       | Erratum ID      | Kconfig                 |
++----------------+-----------------+-----------------+-------------------------+
+| ARM            | Cortex-A15      | #766422         | N/A                     |
+| ARM            | Cortex-A57      | #852523         | N/A                     |
-- 
1.9.1


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

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

* [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (11 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 12/16] xen/arm: Document the errata implemented in Xen Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-21 14:40   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 14/16] xen/arm: arm64: Add cortex-A57 erratum 832075 workaround Julien Grall
                   ` (4 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

The ARM errata 819472, 827319 and 824069 define the same workaround for
these hardware issues in certain Cortex-A53 parts.

The cache instructions "dc cvac" and "dc cvau" need to be upgraded to
"dc civac".

Use the alternative framework to replace those instructions only on
affected cores.

Whilst the errata affect cache instructions issued at any exception
level, it is not necessary to trap EL1/EL0 data cache instructions
access in order to upgrade them. Indeed the data cache corruption would
always be at the address used by the data cache instructions. Note that
this address could point to a shared memory between guests and the
hypervisors, however all the information present in it are be validated
before any use.

Therefore a malicious guest could only hurt itself. Note that all the
guests should implement/enable the workaround for the affected cores.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 docs/misc/arm/silicon-errata.txt |  3 ++
 xen/arch/arm/Kconfig             | 71 ++++++++++++++++++++++++++++++++++++++++
 xen/arch/arm/arm64/cache.S       |  8 ++++-
 xen/arch/arm/cpuerrata.c         | 17 ++++++++++
 xen/include/asm-arm/arm64/page.h |  7 +++-
 xen/include/asm-arm/cpufeature.h |  4 ++-
 xen/include/asm-arm/processor.h  |  6 ++++
 7 files changed, 113 insertions(+), 3 deletions(-)

diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index 3f0d32b..9a2983b 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -42,4 +42,7 @@ stable hypervisors.
 | Implementor    | Component       | Erratum ID      | Kconfig                 |
 +----------------+-----------------+-----------------+-------------------------+
 | ARM            | Cortex-A15      | #766422         | N/A                     |
+| ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319    |
+| ARM            | Cortex-A53      | #824069         | ARM64_ERRATUM_824069    |
+| ARM            | Cortex-A53      | #819472         | ARM64_ERRATUM_819472    |
 | ARM            | Cortex-A57      | #852523         | N/A                     |
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 9b3e66b..6e4c769 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -53,6 +53,77 @@ config ALTERNATIVE
 
 endmenu
 
+menu "ARM errata workaround via the alternative framework"
+	depends on ALTERNATIVE
+
+config ARM64_ERRATUM_827319
+	bool "Cortex-A53: 827319: Data cache clean instructions might cause overlapping transactions to the interconnect"
+	default y
+	depends on ARM_64
+	help
+	  This option adds an alternative code sequence to work around ARM
+	  erratum 827319 on Cortex-A53 parts up to r0p2 with an AMBA 5 CHI
+	  master interface and an L2 cache.
+
+	  Under certain conditions this erratum can cause a clean line eviction
+	  to occur at the same time as another transaction to the same address
+	  on the AMBA 5 CHI interface, which can cause data corruption if the
+	  interconnect reorders the two transactions.
+
+	  The workaround promotes data cache clean instructions to
+	  data cache clean-and-invalidate.
+	  Please note that this does not necessarily enable the workaround,
+	  as it depends on the alternative framework, which will only patch
+	  the kernel if an affected CPU is detected.
+
+	  If unsure, say Y.
+
+config ARM64_ERRATUM_824069
+	bool "Cortex-A53: 824069: Cache line might not be marked as clean after a CleanShared snoop"
+	default y
+	depends on ARM_64
+	help
+	  This option adds an alternative code sequence to work around ARM
+	  erratum 824069 on Cortex-A53 parts up to r0p2 when it is connected
+	  to a coherent interconnect.
+
+	  If a Cortex-A53 processor is executing a store or prefetch for
+	  write instruction at the same time as a processor in another
+	  cluster is executing a cache maintenance operation to the same
+	  address, then this erratum might cause a clean cache line to be
+	  incorrectly marked as dirty.
+
+	  The workaround promotes data cache clean instructions to
+	  data cache clean-and-invalidate.
+	  Please note that this option does not necessarily enable the
+	  workaround, as it depends on the alternative framework, which will
+	  only patch the kernel if an affected CPU is detected.
+
+	  If unsure, say Y.
+
+config ARM64_ERRATUM_819472
+	bool "Cortex-A53: 819472: Store exclusive instructions might cause data corruption"
+	default y
+	depends on ARM_64
+	help
+	  This option adds an alternative code sequence to work around ARM
+	  erratum 819472 on Cortex-A53 parts up to r0p1 with an L2 cache
+	  present when it is connected to a coherent interconnect.
+
+	  If the processor is executing a load and store exclusive sequence at
+	  the same time as a processor in another cluster is executing a cache
+	  maintenance operation to the same address, then this erratum might
+	  cause data corruption.
+
+	  The workaround promotes data cache clean instructions to
+	  data cache clean-and-invalidate.
+	  Please note that this does not necessarily enable the workaround,
+	  as it depends on the alternative framework, which will only patch
+	  the kernel if an affected CPU is detected.
+
+	  If unsure, say Y.
+endmenu
+
 source "common/Kconfig"
 
 source "drivers/Kconfig"
diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S
index bc5a8f7..1a0c80c 100644
--- a/xen/arch/arm/arm64/cache.S
+++ b/xen/arch/arm/arm64/cache.S
@@ -19,6 +19,8 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include <asm/alternative.h>
+
 /*
  * dcache_line_size - get the minimum D-cache line size from the CTR register.
  */
@@ -54,7 +56,11 @@ ENTRY(flush_icache_range)
 	sub	x3, x2, #1
 	bic	x4, x0, x3
 1:
-	dc	cvau, x4		// clean D line to PoU
+alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE
+	dc	cvau, x4
+alternative_else
+	dc	civac, x4
+alternative_endif
 	add	x4, x4, x2
 	cmp	x4, x1
 	b.lo	1b
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 52d39f8..211b520 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -17,6 +17,23 @@ is_affected_midr_range(const struct arm_cpu_capabilities *entry)
 }
 
 static const struct arm_cpu_capabilities arm_errata[] = {
+#if defined(CONFIG_ARM64_ERRATUM_827319) || \
+    defined(CONFIG_ARM64_ERRATUM_824069)
+    {
+        /* Cortex-A53 r0p[012] */
+        .desc = "ARM errata 827319, 824069",
+        .capability = ARM64_WORKAROUND_CLEAN_CACHE,
+        MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x02),
+    },
+#endif
+#ifdef CONFIG_ARM64_ERRATUM_819472
+    {
+        /* Cortex-A53 r0[01] */
+        .desc = "ARM erratum 819472",
+        .capability = ARM64_WORKAROUND_CLEAN_CACHE,
+        MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01),
+    },
+#endif
     {},
 };
 
diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
index fbdc8fb..79ef7bd 100644
--- a/xen/include/asm-arm/arm64/page.h
+++ b/xen/include/asm-arm/arm64/page.h
@@ -3,6 +3,8 @@
 
 #ifndef __ASSEMBLY__
 
+#include <asm/alternative.h>
+
 /* Write a pagetable entry */
 static inline void write_pte(lpae_t *p, lpae_t pte)
 {
@@ -18,7 +20,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
 #define __invalidate_dcache_one(R) "dc ivac, %" #R ";"
 
 /* Inline ASM to flush dcache on register R (may be an inline asm operand) */
-#define __clean_dcache_one(R) "dc cvac, %" #R ";"
+#define __clean_dcache_one(R)                   \
+    ALTERNATIVE("dc cvac, %" #R ";",            \
+                "dc civac, %" #R ";",           \
+                ARM64_WORKAROUND_CLEAN_CACHE)   \
 
 /* Inline ASM to clean and invalidate dcache on register R (may be an
  * inline asm operand) */
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index fb57295..474a778 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -35,7 +35,9 @@
 #endif
 #define cpu_has_security  (boot_cpu_feature32(security) > 0)
 
-#define ARM_NCAPS           0
+#define ARM64_WORKAROUND_CLEAN_CACHE    0
+
+#define ARM_NCAPS           1
 
 #ifndef __ASSEMBLY__
 
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 1b701c5..4123951 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -44,6 +44,12 @@
         _model == (model) && _rv >= (rv_min) && _rv <= (rv_max);        \
 })
 
+#define ARM_CPU_IMP_ARM             0x41
+
+#define ARM_CPU_PART_CORTEX_A53     0xD03
+
+#define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
+
 /* MPIDR Multiprocessor Affinity Register */
 #define _MPIDR_UP           (30)
 #define MPIDR_UP            (_AC(1,U) << _MPIDR_UP)
-- 
1.9.1


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

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

* [RFC 14/16] xen/arm: arm64: Add cortex-A57 erratum 832075 workaround
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (12 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-05 16:34 ` [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails Julien Grall
                   ` (3 subsequent siblings)
  17 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

The ARM erratum 832075 applies to certain revisions of Cortex-A57, one
of the workarounds is to change device loads into using load-acquire
semantics.

Use the alternative framework to enable the workaround only on affected
cores.

Whilst a guest could trigger the deadlock, it can be broken when the
processor is receiving an interrupt. As the Xen scheduler will always setup
a timer (firing to every 1ms to 300ms depending on the running time
slice) on each processor, the deadlock would last only few milliseconds
and only affects the guest time slice.

Therefore a malicious guest could only hurt itself. Note that all the
guests should implement/enable the workaround for the affected cores.

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

---
    Changes in v2:
        - Update the commit message to explain why it is not necessary
        to take care of possible deadlock from the guest.
---
 docs/misc/arm/silicon-errata.txt |  1 +
 xen/arch/arm/Kconfig             | 20 ++++++++++++++++++++
 xen/arch/arm/cpuerrata.c         |  9 +++++++++
 xen/include/asm-arm/arm64/io.h   | 21 +++++++++++++++++----
 xen/include/asm-arm/cpufeature.h |  3 ++-
 xen/include/asm-arm/processor.h  |  2 ++
 6 files changed, 51 insertions(+), 5 deletions(-)

diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index 9a2983b..ab2e5bc 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -46,3 +46,4 @@ stable hypervisors.
 | ARM            | Cortex-A53      | #824069         | ARM64_ERRATUM_824069    |
 | ARM            | Cortex-A53      | #819472         | ARM64_ERRATUM_819472    |
 | ARM            | Cortex-A57      | #852523         | N/A                     |
+| ARM            | Cortex-A57      | #832075         | ARM64_ERRATUM_832075    |
diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 6e4c769..0c3d66d 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -122,6 +122,26 @@ config ARM64_ERRATUM_819472
 	  the kernel if an affected CPU is detected.
 
 	  If unsure, say Y.
+
+config ARM64_ERRATUM_832075
+	bool "Cortex-A57: 832075: possible deadlock on mixing exclusive memory accesses with device loads"
+	default y
+	depends on ARM_64
+	help
+	  This option adds an alternative code sequence to work around ARM
+	  erratum 832075 on Cortex-A57 parts up to r1p2.
+
+	  Affected Cortex-A57 parts might deadlock when exclusive load/store
+	  instructions to Write-Back memory are mixed with Device loads.
+
+	  The workaround is to promote device loads to use Load-Acquire
+	  semantics.
+	  Please note that this does not necessarily enable the workaround,
+	  as it depends on the alternative framework, which will only patch
+	  the kernel if an affected CPU is detected.
+
+	  If unsure, say Y.
+
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
index 211b520..48fc87a 100644
--- a/xen/arch/arm/cpuerrata.c
+++ b/xen/arch/arm/cpuerrata.c
@@ -34,6 +34,15 @@ static const struct arm_cpu_capabilities arm_errata[] = {
         MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01),
     },
 #endif
+#ifdef CONFIG_ARM64_ERRATUM_832075
+    {
+        /* Cortex-A57 r0p0 - r1p2 */
+        .desc = "ARM erratum 832075",
+        .capability = ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE,
+        MIDR_RANGE(MIDR_CORTEX_A57, 0x00,
+                   (1 << MIDR_VARIANT_SHIFT) | 2),
+    },
+#endif
     {},
 };
 
diff --git a/xen/include/asm-arm/arm64/io.h b/xen/include/asm-arm/arm64/io.h
index f80156f..30bfc78 100644
--- a/xen/include/asm-arm/arm64/io.h
+++ b/xen/include/asm-arm/arm64/io.h
@@ -22,6 +22,7 @@
 
 #include <asm/system.h>
 #include <asm/byteorder.h>
+#include <asm/alternative.h>
 
 /*
  * Generic IO read/write.  These perform native-endian accesses.
@@ -49,28 +50,40 @@ static inline void __raw_writeq(u64 val, volatile void __iomem *addr)
 static inline u8 __raw_readb(const volatile void __iomem *addr)
 {
         u8 val;
-        asm volatile("ldrb %w0, [%1]" : "=r" (val) : "r" (addr));
+        asm volatile(ALTERNATIVE("ldrb %w0, [%1]",
+                                 "ldarb %w0, [%1]",
+                                 ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
+                     : "=r" (val) : "r" (addr));
         return val;
 }
 
 static inline u16 __raw_readw(const volatile void __iomem *addr)
 {
         u16 val;
-        asm volatile("ldrh %w0, [%1]" : "=r" (val) : "r" (addr));
+        asm volatile(ALTERNATIVE("ldrh %w0, [%1]",
+                                 "ldarh %w0, [%1]",
+                                 ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
+                     : "=r" (val) : "r" (addr));
         return val;
 }
 
 static inline u32 __raw_readl(const volatile void __iomem *addr)
 {
         u32 val;
-        asm volatile("ldr %w0, [%1]" : "=r" (val) : "r" (addr));
+        asm volatile(ALTERNATIVE("ldr %w0, [%1]",
+                                 "ldar %w0, [%1]",
+                                 ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
+                     : "=r" (val) : "r" (addr));
         return val;
 }
 
 static inline u64 __raw_readq(const volatile void __iomem *addr)
 {
         u64 val;
-        asm volatile("ldr %0, [%1]" : "=r" (val) : "r" (addr));
+        asm volatile(ALTERNATIVE("ldr %0, [%1]",
+                                 "ldar %0, [%1]",
+                                 ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE)
+                     : "=r" (val) : "r" (addr));
         return val;
 }
 
diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
index 474a778..78e2263 100644
--- a/xen/include/asm-arm/cpufeature.h
+++ b/xen/include/asm-arm/cpufeature.h
@@ -36,8 +36,9 @@
 #define cpu_has_security  (boot_cpu_feature32(security) > 0)
 
 #define ARM64_WORKAROUND_CLEAN_CACHE    0
+#define ARM64_WORKAROUND_DEVICE_LOAD_ACQUIRE    1
 
-#define ARM_NCAPS           1
+#define ARM_NCAPS           2
 
 #ifndef __ASSEMBLY__
 
diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
index 4123951..a2c0dbe 100644
--- a/xen/include/asm-arm/processor.h
+++ b/xen/include/asm-arm/processor.h
@@ -47,8 +47,10 @@
 #define ARM_CPU_IMP_ARM             0x41
 
 #define ARM_CPU_PART_CORTEX_A53     0xD03
+#define ARM_CPU_PART_CORTEX_A57     0xD07
 
 #define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
+#define MIDR_CORTEX_A57 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A57)
 
 /* MPIDR Multiprocessor Affinity Register */
 #define _MPIDR_UP           (30)
-- 
1.9.1


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

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

* [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (13 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 14/16] xen/arm: arm64: Add cortex-A57 erratum 832075 workaround Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-21 14:42   ` Stefano Stabellini
  2016-05-05 16:34 ` [RFC 16/16] xen/arm: arm64: Document Cortex-A57 erratum 834220 Julien Grall
                   ` (2 subsequent siblings)
  17 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

Based on ARM ARM (D4.5.3 in ARM DDI 0486A and B3.12.7 in ARM DDI 0406C.c),
a Stage 1 translation error has priority over a Stage 2 translation error.

Therefore gva_to_ipa can only fail if another vCPU is playing with the
page table.

Rather than injecting a custom fault, replay the instruction and let the
processor injecting the correct fault.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 xen/arch/arm/traps.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index c0325d5..3acdba0 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2410,7 +2410,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
 
             rc = gva_to_ipa(gva, &gpa, GV2M_READ);
             if ( rc == -EFAULT )
-                goto bad_insn_abort;
+                return; /* Try again */
         }
 
         rc = p2m_mem_access_check(gpa, gva, npfec);
@@ -2422,7 +2422,6 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
     break;
     }
 
-bad_insn_abort:
     inject_iabt_exception(regs, gva, hsr.len);
 }
 
@@ -2452,7 +2451,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
     {
         rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ);
         if ( rc == -EFAULT )
-            goto bad_data_abort;
+            return; /* Try again */
     }
 
     switch ( dabt.dfsc & 0x3f )
-- 
1.9.1


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

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

* [RFC 16/16] xen/arm: arm64: Document Cortex-A57 erratum 834220
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (14 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails Julien Grall
@ 2016-05-05 16:34 ` Julien Grall
  2016-05-05 16:38 ` [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
  2016-05-11  2:28 ` Wei Chen
  17 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:34 UTC (permalink / raw)
  To: xen-devel
  Cc: andre.przywara, Julien Grall, sstabellini, wei.chen, steve.capper

The ARM erratum applies to certain revisions of Cortex-A57. The
processor may report a Stage 2 translation fault as the result of
Stage 1 fault for load crossing a page boundary when there is a
permission fault or device memory fault at stage 1 and a translation
fault at Stage 2.

So Xen needs to check that Stage 1 translation does not generate a fault
before handling the Stage 2 fault. If it is a Stage 1 translation fault,
return to the guest to let the processor injecting the correct fault.

Only document it as this is already the behavior of the fault handlers.
Note that some optimization could be done to avoid unecessary translation
fault. This is because HPFAR_EL2 is valid for more use case. For the moment,
the code is left unmodified.

Signed-off-by: Julien Grall <julien.grall@arm.com>
---
 docs/misc/arm/silicon-errata.txt |  1 +
 xen/arch/arm/traps.c             | 30 ++++++++++++++++++++++++++++++
 2 files changed, 31 insertions(+)

diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
index ab2e5bc..1ac365d 100644
--- a/docs/misc/arm/silicon-errata.txt
+++ b/docs/misc/arm/silicon-errata.txt
@@ -47,3 +47,4 @@ stable hypervisors.
 | ARM            | Cortex-A53      | #819472         | ARM64_ERRATUM_819472    |
 | ARM            | Cortex-A57      | #852523         | N/A                     |
 | ARM            | Cortex-A57      | #832075         | ARM64_ERRATUM_832075    |
+| ARM            | Cortex-A57      | #834220         | N/A                     |
diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
index 3acdba0..bbd5309 100644
--- a/xen/arch/arm/traps.c
+++ b/xen/arch/arm/traps.c
@@ -2396,6 +2396,21 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
             .kind = hsr.iabt.s1ptw ? npfec_kind_in_gpt : npfec_kind_with_gla
         };
 
+        /*
+         * Erratum #834220: The processor may report a Stage 2
+         * translation fault as the result of Stage 1 fault for load
+         * crossing a page boundary when there is a permission fault or
+         * device memory alignment fault at Stage 1 and a translation
+         * fault at Stage 2.
+         *
+         * So Xen needs to check that the Stage 1 translation does not
+         * generate a fault before handling stage 2 fault. If it is a Stage
+         * 1 translation fault, return to the guest to let the processor
+         * injecting the correct fault.
+         *
+         * XXX: This can be optimized to avoid some unecessary
+         * translation.
+         */
         if ( hsr.iabt.s1ptw )
             gpa = get_faulting_ipa();
         else
@@ -2445,6 +2460,21 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
     info.gva = READ_SYSREG64(FAR_EL2);
 #endif
 
+    /*
+     * Erratum #834220: The processor may report a Stage 2
+     * translation fault as the result of Stage 1 fault for load
+     * crossing a page boundary when there is a permission fault or
+     * device memory alignment fault at Stage 1 and a translation
+     * fault at Stage 2.
+     *
+     * So Xen needs to check that the Stage 1 translation does not
+     * generate a fault before handling stage 2 fault. If it is a Stage
+     * 1 translation fault, return to the guest to let the processor
+     * injecting the correct fault.
+     *
+     * XXX: This can be optimized to avoid some unecessary
+     * translation.
+     */
     if ( dabt.s1ptw )
         info.gpa = get_faulting_ipa();
     else
-- 
1.9.1


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

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

* Re: [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (15 preceding siblings ...)
  2016-05-05 16:34 ` [RFC 16/16] xen/arm: arm64: Document Cortex-A57 erratum 834220 Julien Grall
@ 2016-05-05 16:38 ` Julien Grall
  2016-05-11  2:28 ` Wei Chen
  17 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-05 16:38 UTC (permalink / raw)
  To: xen-devel; +Cc: andre.przywara, sstabellini, wei.chen, steve.capper



On 05/05/16 17:34, Julien Grall wrote:
> Hello,
> 
> Some of the processor erratum will require to modify code sequence. As those
> modifications may impact the performance, they should only be enabled on
> affected cores. Furthermore, Xen may also want to take advantage of new
> hardware features coming up with v8.1 and v8.2.
> 
> The first part of the series adds the alternative infrastructure, most of the
> code is based on Linux v4.6-rc3. The rest of the series implements errata
> for Cortex-A57 and Cortex-A53.

I forgot to mention that I have pushed a branch with the series on
xenbits:

git://xenbits.xen.org/people/julieng/xen-unstable.git branch alternative-rfc

> Yours sincerely,
> 
> Julien Grall (16):
>    xen/arm: Makefile: Sort the entries alphabetically
>    xen/arm: Include the header asm-arm/system.h in asm-arm/page.h
>    xen/arm: Add macros to handle the MIDR
>    xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3
>    xen/arm: Add cpu_hwcap bitmap
>    xen/arm64: Add an helper to invalidate all instruction caches
>    xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header
>    xen/arm: arm64: Reserve a brk immediate to fault on purpose
>    xen/arm: arm64: Add helpers to decode and encode branch instructions
>    xen/arm: Introduce alternative runtime patching
>    xen/arm: Detect silicon revision and set cap bits accordingly
>    xen/arm: Document the errata implemented in Xen
>    xen/arm: arm64: Add Cortex-A53 cache errata workaround
>    xen/arm: arm64: Add cortex-A57 erratum 832075 workaround
>    xen/arm: traps: Don't inject a fault if the translation VA -> IPA
>      fails
>    xen/arm: arm64: Document Cortex-A57 erratum 834220
> 
>   docs/misc/arm/silicon-errata.txt  |  50 +++++++++
>   xen/arch/arm/Kconfig              |  96 +++++++++++++++++
>   xen/arch/arm/Makefile             |  41 +++----
>   xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/arm32/Makefile       |   9 +-
>   xen/arch/arm/arm64/Makefile       |  13 ++-
>   xen/arch/arm/arm64/cache.S        |  51 +++++++++
>   xen/arch/arm/arm64/insn.c         | 213 +++++++++++++++++++++++++++++++++++++
>   xen/arch/arm/cpuerrata.c          |  52 +++++++++
>   xen/arch/arm/cpufeature.c         |  50 +++++++++
>   xen/arch/arm/platforms/Makefile   |   2 +-
>   xen/arch/arm/setup.c              |  11 ++
>   xen/arch/arm/smpboot.c            |   3 +
>   xen/arch/arm/traps.c              |  37 ++++++-
>   xen/arch/arm/xen.lds.S            |   7 ++
>   xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
>   xen/include/asm-arm/arm64/brk.h   |  39 +++++++
>   xen/include/asm-arm/arm64/bug.h   |   5 +-
>   xen/include/asm-arm/arm64/insn.h  |  88 ++++++++++++++++
>   xen/include/asm-arm/arm64/io.h    |  21 +++-
>   xen/include/asm-arm/arm64/page.h  |  13 ++-
>   xen/include/asm-arm/cpuerrata.h   |   6 ++
>   xen/include/asm-arm/cpufeature.h  |  47 +++++++++
>   xen/include/asm-arm/insn.h        |  22 ++++
>   xen/include/asm-arm/page.h        |   3 +
>   xen/include/asm-arm/processor.h   |  43 +++++++-
>   26 files changed, 1260 insertions(+), 44 deletions(-)
>   create mode 100644 docs/misc/arm/silicon-errata.txt
>   create mode 100644 xen/arch/arm/alternative.c
>   create mode 100644 xen/arch/arm/arm64/insn.c
>   create mode 100644 xen/arch/arm/cpuerrata.c
>   create mode 100644 xen/arch/arm/cpufeature.c
>   create mode 100644 xen/include/asm-arm/alternative.h
>   create mode 100644 xen/include/asm-arm/arm64/brk.h
>   create mode 100644 xen/include/asm-arm/arm64/insn.h
>   create mode 100644 xen/include/asm-arm/cpuerrata.h
>   create mode 100644 xen/include/asm-arm/insn.h
> 

-- 
Julien Grall

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

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

* Re: [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically
  2016-05-05 16:34 ` [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically Julien Grall
@ 2016-05-09  9:34   ` Stefano Stabellini
  0 siblings, 0 replies; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:34 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/arch/arm/Makefile           | 38 ++++++++++++++++++++------------------
>  xen/arch/arm/arm32/Makefile     |  9 ++++-----
>  xen/arch/arm/arm64/Makefile     | 12 +++++-------
>  xen/arch/arm/platforms/Makefile |  2 +-
>  4 files changed, 30 insertions(+), 31 deletions(-)
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index ead0cc0..8e0d2f6 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -4,42 +4,44 @@ subdir-y += platforms
>  subdir-$(CONFIG_ARM_64) += efi
>  subdir-$(CONFIG_ACPI) += acpi
>  
> -obj-$(EARLY_PRINTK) += early_printk.o
> +obj-y += bootfdt.o
>  obj-y += cpu.o
> +obj-y += decode.o
> +obj-y += device.o
>  obj-y += domain.o
> -obj-y += psci.o
> -obj-y += vpsci.o
> -obj-y += domctl.o
> -obj-y += sysctl.o
>  obj-y += domain_build.o
> -obj-y += gic.o gic-v2.o
> +obj-y += domctl.o
> +obj-$(EARLY_PRINTK) += early_printk.o
> +obj-y += gic.o
> +obj-y += gic-v2.o
>  obj-$(CONFIG_HAS_GICV3) += gic-v3.o
> +obj-y += guestcopy.o
> +obj-y += hvm.o
>  obj-y += io.o
>  obj-y += irq.o
>  obj-y += kernel.o
>  obj-y += mm.o
>  obj-y += p2m.o
>  obj-y += percpu.o
> -obj-y += guestcopy.o
> -obj-y += physdev.o
>  obj-y += platform.o
>  obj-y += platform_hypercall.o
> +obj-y += physdev.o
> +obj-y += processor.o
> +obj-y += psci.o
>  obj-y += setup.o
> -obj-y += bootfdt.o
> -obj-y += time.o
> -obj-y += smpboot.o
> -obj-y += smp.o
>  obj-y += shutdown.o
> +obj-y += smc.o
> +obj-y += smp.o
> +obj-y += smpboot.o
> +obj-y += sysctl.o
> +obj-y += time.o
>  obj-y += traps.o
> -obj-y += vgic.o vgic-v2.o
> +obj-y += vgic.o
> +obj-y += vgic-v2.o
>  obj-$(CONFIG_ARM_64) += vgic-v3.o
>  obj-y += vtimer.o
> +obj-y += vpsci.o
>  obj-y += vuart.o
> -obj-y += hvm.o
> -obj-y += device.o
> -obj-y += decode.o
> -obj-y += processor.o
> -obj-y += smc.o
>  obj-$(CONFIG_XSPLICE) += xsplice.o
>  
>  #obj-bin-y += ....o
> diff --git a/xen/arch/arm/arm32/Makefile b/xen/arch/arm/arm32/Makefile
> index df0e7de..b20db64 100644
> --- a/xen/arch/arm/arm32/Makefile
> +++ b/xen/arch/arm/arm32/Makefile
> @@ -1,12 +1,11 @@
>  subdir-y += lib
>  
> +obj-$(EARLY_PRINTK) += debug.o
> +obj-y += domctl.o
> +obj-y += domain.o
>  obj-y += entry.o
>  obj-y += proc-v7.o proc-caxx.o
> -
> +obj-y += smpboot.o
>  obj-y += traps.o
> -obj-y += domain.o
>  obj-y += vfp.o
> -obj-y += smpboot.o
> -obj-y += domctl.o
>  
> -obj-$(EARLY_PRINTK) += debug.o
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index c7243f5..39c6ac6 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -1,12 +1,10 @@
>  subdir-y += lib
>  
> +obj-y += cache.o
> +obj-$(EARLY_PRINTK) += debug.o
> +obj-y += domctl.o
> +obj-y += domain.o
>  obj-y += entry.o
> -
> +obj-y += smpboot.o
>  obj-y += traps.o
> -obj-y += domain.o
>  obj-y += vfp.o
> -obj-y += smpboot.o
> -obj-y += domctl.o
> -obj-y += cache.o
> -
> -obj-$(EARLY_PRINTK) += debug.o
> diff --git a/xen/arch/arm/platforms/Makefile b/xen/arch/arm/platforms/Makefile
> index 3689eec..49fa683 100644
> --- a/xen/arch/arm/platforms/Makefile
> +++ b/xen/arch/arm/platforms/Makefile
> @@ -3,8 +3,8 @@ obj-$(CONFIG_ARM_32) += brcm.o
>  obj-$(CONFIG_ARM_32) += exynos5.o
>  obj-$(CONFIG_ARM_32) += midway.o
>  obj-$(CONFIG_ARM_32) += omap5.o
> -obj-$(CONFIG_ARM_32) += sunxi.o
>  obj-$(CONFIG_ARM_32) += rcar2.o
>  obj-$(CONFIG_ARM_64) += seattle.o
> +obj-$(CONFIG_ARM_32) += sunxi.o
>  obj-$(CONFIG_ARM_64) += xgene-storm.o
>  obj-$(CONFIG_ARM_64) += xilinx-zynqmp.o
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 02/16] xen/arm: Include the header asm-arm/system.h in asm-arm/page.h
  2016-05-05 16:34 ` [RFC 02/16] xen/arm: Include the header asm-arm/system.h in asm-arm/page.h Julien Grall
@ 2016-05-09  9:34   ` Stefano Stabellini
  0 siblings, 0 replies; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:34 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> The header asm-arm/page.h makes use of the macro dsb defined in the header
> asm-arm/system.h. Currently, the includer has to specify both of them.
> 
> This can be avoided by including asm-arm/system.h in asm-arm/page.h.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/include/asm-arm/page.h | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index a94e978..05d9f82 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -84,6 +84,7 @@
>  #include <xen/errno.h>
>  #include <xen/types.h>
>  #include <xen/lib.h>
> +#include <asm/system.h>
>  
>  /* WARNING!  Unlike the Intel pagetable code, where l1 is the lowest
>   * level and l4 is the root of the trie, the ARM pagetables follow ARM's
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 03/16] xen/arm: Add macros to handle the MIDR
  2016-05-05 16:34 ` [RFC 03/16] xen/arm: Add macros to handle the MIDR Julien Grall
@ 2016-05-09  9:37   ` Stefano Stabellini
  0 siblings, 0 replies; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> Add new macros to easily get different parts of the register and to
> check if a given MIDR match a CPU model range. The latter will be really
> useful to handle errata later.
> 
> The macros have been imported from the header arch64/include/asm/cputype.h
                                                 ^ arch/arm64

Aside from this:

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


> in Linux v4.6-rc3
> 
> Also remove MIDR_MASK which is unused.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/include/asm-arm/processor.h | 35 ++++++++++++++++++++++++++++++++++-
>  1 file changed, 34 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 6789cd0..1b701c5 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -9,7 +9,40 @@
>  #include <public/arch-arm.h>
>  
>  /* MIDR Main ID Register */
> -#define MIDR_MASK    0xff0ffff0
> +#define MIDR_REVISION_MASK      0xf
> +#define MIDR_RESIVION(midr)     ((midr) & MIDR_REVISION_MASK)
> +#define MIDR_PARTNUM_SHIFT      4
> +#define MIDR_PARTNUM_MASK       (0xfff << MIDR_PARTNUM_SHIFT)
> +#define MIDR_PARTNUM(midr) \
> +    (((midr) & MIDR_PARTNUM_MASK) >> MIDR_PARTNUM_SHIFT)
> +#define MIDR_ARCHITECTURE_SHIFT 16
> +#define MIDR_ARCHITECTURE_MASK  (0xf << MIDR_ARCHITECTURE_SHIFT)
> +#define MIDR_ARCHITECTURE(midr) \
> +    (((midr) & MIDR_ARCHITECTURE_MASK) >> MIDR_ARCHITECTURE_SHIFT)
> +#define MIDR_VARIANT_SHIFT      20
> +#define MIDR_VARIANT_MASK       (0xf << MIDR_VARIANT_SHIFT)
> +#define MIDR_VARIANT(midr) \
> +    (((midr) & MIDR_VARIANT_MASK) >> MIDR_VARIANT_SHIFT)
> +#define MIDR_IMPLEMENTOR_SHIFT  24
> +#define MIDR_IMPLEMENTOR_MASK   (0xff << MIDR_IMPLEMENTOR_SHIFT)
> +#define MIDR_IMPLEMENTOR(midr) \
> +    (((midr) & MIDR_IMPLEMENTOR_MASK) >> MIDR_IMPLEMENTOR_SHIFT)
> +
> +#define MIDR_CPU_MODEL(imp, partnum)            \
> +    (((imp)     << MIDR_IMPLEMENTOR_SHIFT) |    \
> +     (0xf       << MIDR_ARCHITECTURE_SHIFT) |   \
> +     ((partnum) << MIDR_PARTNUM_SHIFT))
> +
> +#define MIDR_CPU_MODEL_MASK \
> +     (MIDR_IMPLEMENTOR_MASK | MIDR_PARTNUM_MASK | MIDR_ARCHITECTURE_MASK)
> +
> +#define MIDR_IS_CPU_MODEL_RANGE(midr, model, rv_min, rv_max)            \
> +({                                                                      \
> +        u32 _model = (midr) & MIDR_CPU_MODEL_MASK;                      \
> +        u32 _rv = (midr) & (MIDR_REVISION_MASK | MIDR_VARIANT_MASK);    \
> +                                                                        \
> +        _model == (model) && _rv >= (rv_min) && _rv <= (rv_max);        \
> +})
>  
>  /* MPIDR Multiprocessor Affinity Register */
>  #define _MPIDR_UP           (30)
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 04/16] xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3
  2016-05-05 16:34 ` [RFC 04/16] xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3 Julien Grall
@ 2016-05-09  9:40   ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-09  9:40 UTC (permalink / raw)
  To: xen-devel; +Cc: andre.przywara, sstabellini, wei.chen, steve.capper



On 05/05/2016 17:34, Julien Grall wrote:
> Flushing the icache will required when the support Xen patching will be
> added.
>
> Also import the macro icache_line_size which is used by
> flush_icache_range.
>
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Hmmm, I forgot to drop this patch from the series. Please ignore it.

Cheers,

> ---
>   xen/arch/arm/arm64/cache.S | 45 +++++++++++++++++++++++++++++++++++++++++++++
>   xen/include/asm-arm/page.h |  2 ++
>   2 files changed, 47 insertions(+)
>
> diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S
> index eff4e16..bc5a8f7 100644
> --- a/xen/arch/arm/arm64/cache.S
> +++ b/xen/arch/arm/arm64/cache.S
> @@ -30,6 +30,51 @@
>   	.endm
>
>   /*
> + * icache_line_size - get the minimum I-cache line size from the CTR register.
> + */
> +	.macro	icache_line_size, reg, tmp
> +	mrs	\tmp, ctr_el0			// read CTR
> +	and	\tmp, \tmp, #0xf		// cache line size encoding
> +	mov	\reg, #4			// bytes per word
> +	lsl	\reg, \reg, \tmp		// actual cache line size
> +	.endm
> +
> +/*
> + *	flush_icache_range(start,end)
> + *
> + *	Ensure that the I and D caches are coherent within specified region.
> + *	This is typically used when code has been written to a memory region,
> + *	and will be executed.
> + *
> + *	- start   - virtual start address of region
> + *	- end     - virtual end address of region
> + */
> +ENTRY(flush_icache_range)
> +	dcache_line_size x2, x3
> +	sub	x3, x2, #1
> +	bic	x4, x0, x3
> +1:
> +	dc	cvau, x4		// clean D line to PoU
> +	add	x4, x4, x2
> +	cmp	x4, x1
> +	b.lo	1b
> +	dsb	ish
> +
> +	icache_line_size x2, x3
> +	sub	x3, x2, #1
> +	bic	x4, x0, x3
> +1:
> + 	ic	ivau, x4		// invalidate I line PoU
> +	add	x4, x4, x2
> +	cmp	x4, x1
> +	b.lo	1b
> +	dsb	ish
> +	isb
> +	mov	x0, #0
> +	ret
> +ENDPROC(flush_icache_range)
> +
> +/*
>    *	__flush_dcache_area(kaddr, size)
>    *
>    *	Ensure that the data held in the page kaddr is written back to the
> diff --git a/xen/include/asm-arm/page.h b/xen/include/asm-arm/page.h
> index 05d9f82..a94e826 100644
> --- a/xen/include/asm-arm/page.h
> +++ b/xen/include/asm-arm/page.h
> @@ -328,6 +328,8 @@ static inline int clean_and_invalidate_dcache_va_range
>       return 0;
>   }
>
> +int flush_icache_range(unsigned long start, unsigned long end);
> +
>   /* Macros for flushing a single small item.  The predicate is always
>    * compile-time constant so this will compile down to 3 instructions in
>    * the common case. */
>

-- 
Julien Grall

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

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

* Re: [RFC 06/16] xen/arm64: Add an helper to invalidate all instruction caches
  2016-05-05 16:34 ` [RFC 06/16] xen/arm64: Add an helper to invalidate all instruction caches Julien Grall
@ 2016-05-09  9:50   ` Stefano Stabellini
  0 siblings, 0 replies; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:50 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/include/asm-arm/arm64/page.h | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
> index 29a32cf..fbdc8fb 100644
> --- a/xen/include/asm-arm/arm64/page.h
> +++ b/xen/include/asm-arm/arm64/page.h
> @@ -24,6 +24,12 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
>   * inline asm operand) */
>  #define __clean_and_invalidate_dcache_one(R) "dc  civac, %" #R ";"
>  
> +/* Invalidate all instruction caches in Inner Shareable domain to PoU */
> +static inline void invalidate_icache(void)
> +{
> +    asm volatile ("ic ialluis");
> +}
> +
>  /*
>   * Flush all hypervisor mappings from the TLB of the local processor.
>   *
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 05/16] xen/arm: Add cpu_hwcap bitmap
  2016-05-05 16:34 ` [RFC 05/16] xen/arm: Add cpu_hwcap bitmap Julien Grall
@ 2016-05-09  9:53   ` Stefano Stabellini
  2016-05-09 10:02     ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:53 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> This will be used to know if a feature, which Xen cares, is available accross
> all the CPUs.
> 
> This code is a light version of arch/arm64/kernel/cpufeature.c from
> Linux v4.6-rc3.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/Makefile            |  1 +
>  xen/arch/arm/cpufeature.c        | 34 ++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/cpufeature.h | 29 +++++++++++++++++++++++++++++
>  3 files changed, 64 insertions(+)
>  create mode 100644 xen/arch/arm/cpufeature.c
> 
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 8e0d2f6..9122f78 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -6,6 +6,7 @@ subdir-$(CONFIG_ACPI) += acpi
>  
>  obj-y += bootfdt.o
>  obj-y += cpu.o
> +obj-y += cpufeature.o
>  obj-y += decode.o
>  obj-y += device.o
>  obj-y += domain.o
> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> new file mode 100644
> index 0000000..7a1b56b
> --- /dev/null
> +++ b/xen/arch/arm/cpufeature.c
> @@ -0,0 +1,34 @@
> +/*
> + * Contains CPU feature definitions
> + *
> + * Copyright (C) 2015 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/config.h>
> +#include <xen/types.h>
> +#include <xen/init.h>
> +#include <xen/smp.h>
> +#include <asm/cpufeature.h>
> +
> +DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index 7b519cd..2bebad1 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -35,6 +35,35 @@
>  #endif
>  #define cpu_has_security  (boot_cpu_feature32(security) > 0)
>  
> +#define ARM_NCAPS           0
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/types.h>
> +#include <xen/lib.h>
> +#include <xen/bitops.h>
> +
> +extern DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
> +
> +static inline bool_t cpus_have_cap(unsigned int num)
> +{
> +    if ( num >= ARM_NCAPS )
> +        return 0;
> +
> +    return test_bit(num, cpu_hwcaps);
> +}
> +
> +static inline void cpus_set_cap(unsigned int num)
> +{
> +    if (num >= ARM_NCAPS)
> +        printk(XENLOG_WARNING "Attempt to set an illegal CPU capability (%d >= %d)\n",
> +               num, ARM_NCAPS);
> +    else
> +        __set_bit(num, cpu_hwcaps);
> +}
> +
> +#endif /* __ASSEMBLY__ */

Why everything static line? Wouldn't it better to add the functions to
cpufeature.c? They are not going to be called too many times, right?


>  #endif
>  /*
>   * Local variables:
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 07/16] xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header
  2016-05-05 16:34 ` [RFC 07/16] xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header Julien Grall
@ 2016-05-09  9:55   ` Stefano Stabellini
  0 siblings, 0 replies; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:55 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> New immediates will be defined in the future. To keep track of the
> immediates allocated, gather all of them in a separate header.
> 
> Also rename BRK_BUG_FRAME to BKR_BUG_FRAME_IMM.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/arch/arm/traps.c            |  2 +-
>  xen/include/asm-arm/arm64/brk.h | 26 ++++++++++++++++++++++++++
>  xen/include/asm-arm/arm64/bug.h |  5 ++---
>  3 files changed, 29 insertions(+), 4 deletions(-)
>  create mode 100644 xen/include/asm-arm/arm64/brk.h
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index 1828ea1..c0325d5 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -1205,7 +1205,7 @@ static void do_trap_brk(struct cpu_user_regs *regs, const union hsr hsr)
>  
>      switch (hsr.brk.comment)
>      {
> -    case BRK_BUG_FRAME:
> +    case BRK_BUG_FRAME_IMM:
>          if ( do_bug_frame(regs, regs->pc) )
>              goto die;
>  
> diff --git a/xen/include/asm-arm/arm64/brk.h b/xen/include/asm-arm/arm64/brk.h
> new file mode 100644
> index 0000000..7867b07
> --- /dev/null
> +++ b/xen/include/asm-arm/arm64/brk.h
> @@ -0,0 +1,26 @@
> +/*
> + * Copyright (C) 2016 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + */
> +
> +#ifndef __ASM_ARM_ARM64_BRK
> +#define __ASM_ARM_ARM64_BRK
> +
> +/*
> + * #imm16 values used for BRK instruction generation
> + * 0x001: xen-mode BUG() and WARN() traps
> + */
> +#define BRK_BUG_FRAME_IMM   1
> +
> +#endif /* !__ASM_ARM_ARM64_BRK */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/include/asm-arm/arm64/bug.h b/xen/include/asm-arm/arm64/bug.h
> index 42b0e4f..59f664d 100644
> --- a/xen/include/asm-arm/arm64/bug.h
> +++ b/xen/include/asm-arm/arm64/bug.h
> @@ -2,9 +2,8 @@
>  #define __ARM_ARM64_BUG_H__
>  
>  #include <xen/stringify.h>
> +#include <asm/arm64/brk.h>
>  
> -#define BRK_BUG_FRAME 1
> -
> -#define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME)
> +#define BUG_INSTR "brk " __stringify(BRK_BUG_FRAME_IMM)
>  
>  #endif /* __ARM_ARM64_BUG_H__ */
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 08/16] xen/arm: arm64: Reserve a brk immediate to fault on purpose
  2016-05-05 16:34 ` [RFC 08/16] xen/arm: arm64: Reserve a brk immediate to fault on purpose Julien Grall
@ 2016-05-09  9:58   ` Stefano Stabellini
  0 siblings, 0 replies; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09  9:58 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> It may not possible to return a proper error when encoding an
> instruction. Instead, a handcrafted instruction will be returned.
> 
> Also, provide the encoding for the faulting instruction.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Acked-by: Stefano Stabellini <sstabellini@kernel.org>


>  xen/include/asm-arm/arm64/brk.h | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
> 
> diff --git a/xen/include/asm-arm/arm64/brk.h b/xen/include/asm-arm/arm64/brk.h
> index 7867b07..04442c4 100644
> --- a/xen/include/asm-arm/arm64/brk.h
> +++ b/xen/include/asm-arm/arm64/brk.h
> @@ -12,8 +12,21 @@
>  /*
>   * #imm16 values used for BRK instruction generation
>   * 0x001: xen-mode BUG() and WARN() traps
> + * 0x002: for triggering a fault on purpose (reserved)
>   */
>  #define BRK_BUG_FRAME_IMM   1
> +#define BRK_FAULT_IMM       2
> +
> +/*
> + * BRK instruction encoding
> + * The #imm16 value should be placed at bits[20:5] within BRK ins
> + */
> +#define AARCH64_BREAK_MON 0xd4200000
> +
> +/*
> + * BRK instruction for provoking a fault on purpose
> + */
> +#define AARCH64_BREAK_FAULT (AARCH64_BREAK_MON | (BRK_FAULT_IMM << 5))
>  
>  #endif /* !__ASM_ARM_ARM64_BRK */
>  /*
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 05/16] xen/arm: Add cpu_hwcap bitmap
  2016-05-09  9:53   ` Stefano Stabellini
@ 2016-05-09 10:02     ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-09 10:02 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 09/05/2016 10:53, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
>> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
>> index 7b519cd..2bebad1 100644
>> --- a/xen/include/asm-arm/cpufeature.h
>> +++ b/xen/include/asm-arm/cpufeature.h
>> @@ -35,6 +35,35 @@
>>   #endif
>>   #define cpu_has_security  (boot_cpu_feature32(security) > 0)
>>
>> +#define ARM_NCAPS           0
>> +
>> +#ifndef __ASSEMBLY__
>> +
>> +#include <xen/types.h>
>> +#include <xen/lib.h>
>> +#include <xen/bitops.h>
>> +
>> +extern DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>> +
>> +static inline bool_t cpus_have_cap(unsigned int num)
>> +{
>> +    if ( num >= ARM_NCAPS )
>> +        return 0;
>> +
>> +    return test_bit(num, cpu_hwcaps);
>> +}
>> +
>> +static inline void cpus_set_cap(unsigned int num)
>> +{
>> +    if (num >= ARM_NCAPS)
>> +        printk(XENLOG_WARNING "Attempt to set an illegal CPU capability (%d >= %d)\n",
>> +               num, ARM_NCAPS);
>> +    else
>> +        __set_bit(num, cpu_hwcaps);
>> +}
>> +
>> +#endif /* __ASSEMBLY__ */
>
> Why everything static line? Wouldn't it better to add the functions to
> cpufeature.c? They are not going to be called too many times, right?

This code is based on Linux where the 2 functions were already static 
inline.

I agree that cpus_set_cap won't be called often (mostly CPU bring up), 
however cpus_have_cap could be used at runtime to check whether the 
AArch32 mode is present...

But I think those 2 functions should be kept together.

Note that today, we always use the boot CPU data to know if a feature 
exists (see include/asm-arm/cpufeatures.h).

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions
  2016-05-05 16:34 ` [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions Julien Grall
@ 2016-05-09 10:05   ` Stefano Stabellini
  2016-05-09 13:04     ` Julien Grall
  2016-05-13 20:35   ` Konrad Rzeszutek Wilk
  1 sibling, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-09 10:05 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> We may need to update branch instruction when patching Xen.
> 
> The code has been imported from the files arch/arm64/kernel/insn.c
> and arch/arm64/include/asm/insn.h in Linux v4.6-rc3.
> 
> Note that only the necessary helpers have been imported.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/arm64/Makefile      |   1 +
>  xen/arch/arm/arm64/insn.c        | 213 +++++++++++++++++++++++++++++++++++++++
>  xen/include/asm-arm/arm64/insn.h |  72 +++++++++++++
>  xen/include/asm-arm/insn.h       |  22 ++++
>  4 files changed, 308 insertions(+)
>  create mode 100644 xen/arch/arm/arm64/insn.c
>  create mode 100644 xen/include/asm-arm/arm64/insn.h
>  create mode 100644 xen/include/asm-arm/insn.h
> 
> diff --git a/xen/arch/arm/arm64/Makefile b/xen/arch/arm/arm64/Makefile
> index 39c6ac6..c1fa43f 100644
> --- a/xen/arch/arm/arm64/Makefile
> +++ b/xen/arch/arm/arm64/Makefile
> @@ -5,6 +5,7 @@ obj-$(EARLY_PRINTK) += debug.o
>  obj-y += domctl.o
>  obj-y += domain.o
>  obj-y += entry.o
> +obj-y += insn.o
>  obj-y += smpboot.o
>  obj-y += traps.o
>  obj-y += vfp.o
> diff --git a/xen/arch/arm/arm64/insn.c b/xen/arch/arm/arm64/insn.c
> new file mode 100644
> index 0000000..54a6b62
> --- /dev/null
> +++ b/xen/arch/arm/arm64/insn.c
> @@ -0,0 +1,213 @@
> +/*
> + * Based on Linux arch/arm64/kernel/insn.c
> + *
> + * Copyright (C) 2013 Huawei Ltd.
> + * Author: Jiang Liu <liuj97@gmail.com>
> + *
> + * Copyright (C) 2014-2016 Zi Shen Lim <zlim.lnx@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#include <xen/config.h>
> +#include <xen/types.h>
> +#include <xen/lib.h>
> +#include <xen/errno.h>
> +#include <xen/sizes.h>
> +#include <xen/bitops.h>
> +#include <asm/insn.h>
> +#include <asm/arm64/insn.h>
> +
> +bool aarch64_insn_is_branch_imm(u32 insn)
> +{
> +	return (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn) ||
> +		aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn) ||
> +		aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
> +		aarch64_insn_is_bcond(insn));
> +}
> +
> +static int aarch64_get_imm_shift_mask(enum aarch64_insn_imm_type type,
> +						u32 *maskp, int *shiftp)
> +{
> +	u32 mask;
> +	int shift;
> +
> +	switch (type) {
> +	case AARCH64_INSN_IMM_26:
> +		mask = BIT(26) - 1;
> +		shift = 0;
> +		break;
> +	case AARCH64_INSN_IMM_19:
> +		mask = BIT(19) - 1;
> +		shift = 5;
> +		break;
> +	case AARCH64_INSN_IMM_16:
> +		mask = BIT(16) - 1;
> +		shift = 5;
> +		break;
> +	case AARCH64_INSN_IMM_14:
> +		mask = BIT(14) - 1;
> +		shift = 5;
> +		break;
> +	case AARCH64_INSN_IMM_12:
> +		mask = BIT(12) - 1;
> +		shift = 10;
> +		break;
> +	case AARCH64_INSN_IMM_9:
> +		mask = BIT(9) - 1;
> +		shift = 12;
> +		break;
> +	case AARCH64_INSN_IMM_7:
> +		mask = BIT(7) - 1;
> +		shift = 15;
> +		break;
> +	case AARCH64_INSN_IMM_6:
> +	case AARCH64_INSN_IMM_S:
> +		mask = BIT(6) - 1;
> +		shift = 10;
> +		break;
> +	case AARCH64_INSN_IMM_R:
> +		mask = BIT(6) - 1;
> +		shift = 16;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	*maskp = mask;
> +	*shiftp = shift;
> +
> +	return 0;
> +}
> +
> +#define ADR_IMM_HILOSPLIT	2
> +#define ADR_IMM_SIZE		SZ_2M
> +#define ADR_IMM_LOMASK		((1 << ADR_IMM_HILOSPLIT) - 1)
> +#define ADR_IMM_HIMASK		((ADR_IMM_SIZE >> ADR_IMM_HILOSPLIT) - 1)
> +#define ADR_IMM_LOSHIFT		29
> +#define ADR_IMM_HISHIFT		5
> +
> +u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn)
> +{
> +	u32 immlo, immhi, mask;
> +	int shift;
> +
> +	switch (type) {
> +	case AARCH64_INSN_IMM_ADR:
> +		shift = 0;
> +		immlo = (insn >> ADR_IMM_LOSHIFT) & ADR_IMM_LOMASK;
> +		immhi = (insn >> ADR_IMM_HISHIFT) & ADR_IMM_HIMASK;
> +		insn = (immhi << ADR_IMM_HILOSPLIT) | immlo;
> +		mask = ADR_IMM_SIZE - 1;
> +		break;
> +	default:
> +		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
> +			printk(XENLOG_ERR "aarch64_insn_decode_immediate: unknown immediate encoding %d\n",
> +			       type);
> +			return 0;
> +		}
> +	}
> +
> +	return (insn >> shift) & mask;
> +}
> +
> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
> +				  u32 insn, u64 imm)
> +{
> +	u32 immlo, immhi, mask;
> +	int shift;

Here Linux checks for insn == AARCH64_BREAK_FAULT. Shouldn't we do the
same?


> +	switch (type) {
> +	case AARCH64_INSN_IMM_ADR:
> +		shift = 0;
> +		immlo = (imm & ADR_IMM_LOMASK) << ADR_IMM_LOSHIFT;
> +		imm >>= ADR_IMM_HILOSPLIT;
> +		immhi = (imm & ADR_IMM_HIMASK) << ADR_IMM_HISHIFT;
> +		imm = immlo | immhi;
> +		mask = ((ADR_IMM_LOMASK << ADR_IMM_LOSHIFT) |
> +			(ADR_IMM_HIMASK << ADR_IMM_HISHIFT));
> +		break;
> +	default:
> +		if (aarch64_get_imm_shift_mask(type, &mask, &shift) < 0) {
> +			printk(XENLOG_ERR "aarch64_insn_encode_immediate: unknown immediate encoding %d\n",
> +			       type);
> +			return AARCH64_BREAK_FAULT;
> +		}
> +	}
> +
> +	/* Update the immediate field. */
> +	insn &= ~(mask << shift);
> +	insn |= (imm & mask) << shift;
> +
> +	return insn;
> +}
> +
> +/*
> + * Decode the imm field of a branch, and return the byte offset as a
> + * signed value (so it can be used when computing a new branch
> + * target).
> + */
> +s32 aarch64_get_branch_offset(u32 insn)
> +{
> +	s32 imm;
> +
> +	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn)) {
> +		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_26, insn);
> +		return (imm << 6) >> 4;
> +	}
> +
> +	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
> +	    aarch64_insn_is_bcond(insn)) {
> +		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_19, insn);
> +		return (imm << 13) >> 11;
> +	}
> +
> +	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn)) {
> +		imm = aarch64_insn_decode_immediate(AARCH64_INSN_IMM_14, insn);
> +		return (imm << 18) >> 16;
> +	}
> +
> +	/* Unhandled instruction */
> +	BUG();
> +}
> +
> +/*
> + * Encode the displacement of a branch in the imm field and return the
> + * updated instruction.
> + */
> +u32 aarch64_set_branch_offset(u32 insn, s32 offset)
> +{
> +	if (aarch64_insn_is_b(insn) || aarch64_insn_is_bl(insn))
> +		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_26, insn,
> +						     offset >> 2);
> +
> +	if (aarch64_insn_is_cbz(insn) || aarch64_insn_is_cbnz(insn) ||
> +	    aarch64_insn_is_bcond(insn))
> +		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_19, insn,
> +						     offset >> 2);
> +
> +	if (aarch64_insn_is_tbz(insn) || aarch64_insn_is_tbnz(insn))
> +		return aarch64_insn_encode_immediate(AARCH64_INSN_IMM_14, insn,
> +						     offset >> 2);
> +
> +	/* Unhandled instruction */
> +	BUG();
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 8
> + * indent-tabs-mode: t
> + * End:
> + */
> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
> new file mode 100644
> index 0000000..cfcdbe9
> --- /dev/null
> +++ b/xen/include/asm-arm/arm64/insn.h
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright (C) 2013 Huawei Ltd.
> + * Author: Jiang Liu <liuj97@gmail.com>
> + *
> + * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ARCH_ARM_ARM64_INSN
> +#define __ARCH_ARM_ARM64_INSN
> +
> +#include <xen/config.h>
> +#include <xen/types.h>
> +#include <xen/stdbool.h>
> +
> +enum aarch64_insn_imm_type {
> +	AARCH64_INSN_IMM_ADR,
> +	AARCH64_INSN_IMM_26,
> +	AARCH64_INSN_IMM_19,
> +	AARCH64_INSN_IMM_16,
> +	AARCH64_INSN_IMM_14,
> +	AARCH64_INSN_IMM_12,
> +	AARCH64_INSN_IMM_9,
> +	AARCH64_INSN_IMM_7,
> +	AARCH64_INSN_IMM_6,
> +	AARCH64_INSN_IMM_S,
> +	AARCH64_INSN_IMM_R,
> +	AARCH64_INSN_IMM_MAX
> +};
> +
> +#define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
> +static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
> +{ return (code & (mask)) == (val); } \
> +static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
> +{ return (val); }
> +
> +__AARCH64_INSN_FUNCS(b,		0xFC000000, 0x14000000)
> +__AARCH64_INSN_FUNCS(bl,	0xFC000000, 0x94000000)
> +__AARCH64_INSN_FUNCS(cbz,	0x7F000000, 0x34000000)
> +__AARCH64_INSN_FUNCS(cbnz,	0x7F000000, 0x35000000)
> +__AARCH64_INSN_FUNCS(tbz,	0x7F000000, 0x36000000)
> +__AARCH64_INSN_FUNCS(tbnz,	0x7F000000, 0x37000000)
> +__AARCH64_INSN_FUNCS(bcond,	0xFF000010, 0x54000000)
> +
> +bool aarch64_insn_is_branch_imm(u32 insn);
> +
> +u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
> +				  u32 insn, u64 imm);
> +
> +s32 aarch64_get_branch_offset(u32 insn);
> +u32 aarch64_set_branch_offset(u32 insn, s32 offset);
> +
> +#endif /* !__ARCH_ARM_ARM64_INSN */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 8
> + * indent-tabs-mode: t
> + * End:
> + */
> diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
> new file mode 100644
> index 0000000..fe9419c
> --- /dev/null
> +++ b/xen/include/asm-arm/insn.h
> @@ -0,0 +1,22 @@
> +#ifndef __ARCH_ARM_INSN
> +#define __ARCH_ARM_INSN
> +
> +#include <xen/types.h>
> +
> +#if defined(CONFIG_ARM_32)
> +# include <asm/arm32/insn.h>

This is missing


> +#elif defined(CONFIG_ARM_64)
> +# include <asm/arm64/insn.h>
> +#else
> +# error "unknown ARM variant"
> +#endif
> +
> +#endif /* !__ARCH_ARM_INSN */
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 8
> + * indent-tabs-mode: t
> + * End:
> + */
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions
  2016-05-09 10:05   ` Stefano Stabellini
@ 2016-05-09 13:04     ` Julien Grall
  2016-05-23 10:52       ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-09 13:04 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 09/05/16 11:05, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
>> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>> +				  u32 insn, u64 imm)
>> +{
>> +	u32 immlo, immhi, mask;
>> +	int shift;
>
> Here Linux checks for insn == AARCH64_BREAK_FAULT. Shouldn't we do the
> same?

I am not sure why I removed this check. I will re-add on the next version.

>> diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
>> new file mode 100644
>> index 0000000..fe9419c
>> --- /dev/null
>> +++ b/xen/include/asm-arm/insn.h
>> @@ -0,0 +1,22 @@
>> +#ifndef __ARCH_ARM_INSN
>> +#define __ARCH_ARM_INSN
>> +
>> +#include <xen/types.h>
>> +
>> +#if defined(CONFIG_ARM_32)
>> +# include <asm/arm32/insn.h>
>
> This is missing

I will drop the include as I don't plan to implement instruction 
patching for ARM32.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64
  2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
                   ` (16 preceding siblings ...)
  2016-05-05 16:38 ` [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
@ 2016-05-11  2:28 ` Wei Chen
  17 siblings, 0 replies; 55+ messages in thread
From: Wei Chen @ 2016-05-11  2:28 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel

Hi Julien,

I have created a dummy CPU feature to test alternative runtime patching.
It seems working well:

Before enable dummy CPU feature:
(XEN) I/O virtualisation disabled
(XEN) Patching kernel code
(XEN) Testing alternative branch....
(XEN) Hello, this is the original code...
(XEN) *** LOADING DOMAIN 0 ***
(XEN) Loading kernel from boot module @ 00000083fc3e7000

After enable dummy CPU feature:
(XEN) I/O virtualisation disabled
(XEN) Setting Dummy CPU Feature to test Alternative patching...
(XEN) Patching kernel code
(XEN) Testing alternative branch....
(XEN) Hello, this is the patched code...
(XEN) *** LOADING DOMAIN 0 ***


Tested-by: Wei Chen <Wei.Chen@linaro.org>

Regards.

On 6 May 2016 at 00:34, Julien Grall <julien.grall@arm.com> wrote:
> Hello,
>
> Some of the processor erratum will require to modify code sequence. As those
> modifications may impact the performance, they should only be enabled on
> affected cores. Furthermore, Xen may also want to take advantage of new
> hardware features coming up with v8.1 and v8.2.
>
> The first part of the series adds the alternative infrastructure, most of the
> code is based on Linux v4.6-rc3. The rest of the series implements errata
> for Cortex-A57 and Cortex-A53.
>
> Yours sincerely,
>
> Julien Grall (16):
>   xen/arm: Makefile: Sort the entries alphabetically
>   xen/arm: Include the header asm-arm/system.h in asm-arm/page.h
>   xen/arm: Add macros to handle the MIDR
>   xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3
>   xen/arm: Add cpu_hwcap bitmap
>   xen/arm64: Add an helper to invalidate all instruction caches
>   xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header
>   xen/arm: arm64: Reserve a brk immediate to fault on purpose
>   xen/arm: arm64: Add helpers to decode and encode branch instructions
>   xen/arm: Introduce alternative runtime patching
>   xen/arm: Detect silicon revision and set cap bits accordingly
>   xen/arm: Document the errata implemented in Xen
>   xen/arm: arm64: Add Cortex-A53 cache errata workaround
>   xen/arm: arm64: Add cortex-A57 erratum 832075 workaround
>   xen/arm: traps: Don't inject a fault if the translation VA -> IPA
>     fails
>   xen/arm: arm64: Document Cortex-A57 erratum 834220
>
>  docs/misc/arm/silicon-errata.txt  |  50 +++++++++
>  xen/arch/arm/Kconfig              |  96 +++++++++++++++++
>  xen/arch/arm/Makefile             |  41 +++----
>  xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/arm32/Makefile       |   9 +-
>  xen/arch/arm/arm64/Makefile       |  13 ++-
>  xen/arch/arm/arm64/cache.S        |  51 +++++++++
>  xen/arch/arm/arm64/insn.c         | 213 +++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/cpuerrata.c          |  52 +++++++++
>  xen/arch/arm/cpufeature.c         |  50 +++++++++
>  xen/arch/arm/platforms/Makefile   |   2 +-
>  xen/arch/arm/setup.c              |  11 ++
>  xen/arch/arm/smpboot.c            |   3 +
>  xen/arch/arm/traps.c              |  37 ++++++-
>  xen/arch/arm/xen.lds.S            |   7 ++
>  xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
>  xen/include/asm-arm/arm64/brk.h   |  39 +++++++
>  xen/include/asm-arm/arm64/bug.h   |   5 +-
>  xen/include/asm-arm/arm64/insn.h  |  88 ++++++++++++++++
>  xen/include/asm-arm/arm64/io.h    |  21 +++-
>  xen/include/asm-arm/arm64/page.h  |  13 ++-
>  xen/include/asm-arm/cpuerrata.h   |   6 ++
>  xen/include/asm-arm/cpufeature.h  |  47 +++++++++
>  xen/include/asm-arm/insn.h        |  22 ++++
>  xen/include/asm-arm/page.h        |   3 +
>  xen/include/asm-arm/processor.h   |  43 +++++++-
>  26 files changed, 1260 insertions(+), 44 deletions(-)
>  create mode 100644 docs/misc/arm/silicon-errata.txt
>  create mode 100644 xen/arch/arm/alternative.c
>  create mode 100644 xen/arch/arm/arm64/insn.c
>  create mode 100644 xen/arch/arm/cpuerrata.c
>  create mode 100644 xen/arch/arm/cpufeature.c
>  create mode 100644 xen/include/asm-arm/alternative.h
>  create mode 100644 xen/include/asm-arm/arm64/brk.h
>  create mode 100644 xen/include/asm-arm/arm64/insn.h
>  create mode 100644 xen/include/asm-arm/cpuerrata.h
>  create mode 100644 xen/include/asm-arm/insn.h
>
> --
> 1.9.1
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-05 16:34 ` [RFC 10/16] xen/arm: Introduce alternative runtime patching Julien Grall
@ 2016-05-13 20:26   ` Konrad Rzeszutek Wilk
  2016-05-14 18:02     ` Julien Grall
  2016-05-21 15:09   ` Stefano Stabellini
  1 sibling, 1 reply; 55+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-13 20:26 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, May 05, 2016 at 05:34:19PM +0100, Julien Grall wrote:
> Some of the processor erratum will require to modify code sequence.
> As those modifications may impact the performance, they should only
> be enabled on affected cores. Furthermore, Xen may also want to take
> advantage of new hardware features coming up with v8.1 and v8.2.
> 
> This patch adds an infrastructure to patch Xen during boot time
> depending on the "features" available on the platform.
> 
> This code is based on the file arch/arm64/kernel/alternative.c in
> Linux 4.6-rc3. Any references to arm64 have been dropped to make the
> code as generic as possible.
> 
> Furthermore, in Xen the executable sections (.text and .init.text)
> are always mapped read-only and enforced by SCTLR.WNX. So it is not
> possible to directly patch Xen.

Is it not possible to temporarily 'unenforce' the SCTLR.WNX?

> 
> To by-pass this restriction, a temporary writeable mapping is made with
> vmap. This mapping will be used to write the new instructions.
> 
> Lastly, runtime patching is currently not necessary for ARM32. So the
> code is only enabled for ARM64.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/Kconfig              |   5 +
>  xen/arch/arm/Makefile             |   1 +
>  xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/setup.c              |   8 ++
>  xen/arch/arm/xen.lds.S            |   7 ++
>  xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
>  xen/include/asm-arm/arm64/insn.h  |  16 +++
>  7 files changed, 419 insertions(+)
>  create mode 100644 xen/arch/arm/alternative.c
>  create mode 100644 xen/include/asm-arm/alternative.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 6231cd5..9b3e66b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -14,6 +14,7 @@ config ARM_64
>  	def_bool y
>  	depends on 64BIT
>  	select HAS_GICV3
> +    select ALTERNATIVE

Hard tabs, not spaces.
>  
>  config ARM
>  	def_bool y
> @@ -46,6 +47,10 @@ config ACPI
>  config HAS_GICV3
>  	bool
>  
> +# Select ALTERNATIVE if the architecture supports runtime patching
> +config ALTERNATIVE
> +    bool

Ditto. 
> +
>  endmenu
>  
>  source "common/Kconfig"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 9122f78..2d330aa 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -4,6 +4,7 @@ subdir-y += platforms
>  subdir-$(CONFIG_ARM_64) += efi
>  subdir-$(CONFIG_ACPI) += acpi
>  
> +obj-$(CONFIG_ALTERNATIVE) += alternative.o

This probably needs to be alternative.init.o ?
>  obj-y += bootfdt.o
>  obj-y += cpu.o
>  obj-y += cpufeature.o
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> new file mode 100644
> index 0000000..0a5d1c8
> --- /dev/null
> +++ b/xen/arch/arm/alternative.c
> @@ -0,0 +1,217 @@
> +/*
> + * alternative runtime patching
> + * inspired by the x86 version
> + *
> + * Copyright (C) 2014 ARM Ltd.

Not 2016?

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/config.h>
> +#include <xen/init.h>
> +#include <xen/types.h>
> +#include <xen/kernel.h>
> +#include <xen/mm.h>
> +#include <xen/vmap.h>
> +#include <xen/smp.h>
> +#include <xen/stop_machine.h>
> +#include <asm/alternative.h>
> +#include <asm/atomic.h>
> +#include <asm/byteorder.h>
> +#include <asm/cpufeature.h>
> +#include <asm/insn.h>
> +#include <asm/page.h>
> +
> +#define __ALT_PTR(a,f)      (u32 *)((void *)&(a)->f + (a)->f)
> +#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
> +#define ALT_REPL_PTR(a)     __ALT_PTR(a, alt_offset)
> +
> +extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
> +
> +struct alt_region {
> +    const struct alt_instr *begin;
> +    const struct alt_instr *end;
> +};
> +
> +/*
> + * Check if the target PC is within an alternative block.
> + */
> +static bool_t branch_insn_requires_update(const struct alt_instr *alt,

__init?
> +                                          unsigned long pc)
> +{
> +    unsigned long replptr;
> +
> +    if ( is_active_kernel_text(pc) )
> +        return 1;
> +
> +    replptr = (unsigned long)ALT_REPL_PTR(alt);
> +    if ( pc >= replptr && pc <= (replptr + alt->alt_len) )
> +        return 0;
> +
> +    /*
> +     * Branching into *another* alternate sequence is doomed, and
> +     * we're not even trying to fix it up.
> +     */
> +    BUG();
> +}
> +
> +static u32 get_alt_insn(const struct alt_instr *alt,
> +                        const u32 *insnptr, const u32 *altinsnptr)
> +{
> +    u32 insn;
> +
> +    insn = le32_to_cpu(*altinsnptr);
> +
> +    if ( insn_is_branch_imm(insn) )
> +    {
> +        s32 offset = insn_get_branch_offset(insn);
> +        unsigned long target;
> +
> +        target = (unsigned long)altinsnptr + offset;
> +
> +        /*
> +         * If we're branching inside the alternate sequence,
> +         * do not rewrite the instruction, as it is already
> +         * correct. Otherwise, generate the new instruction.
> +         */
> +        if ( branch_insn_requires_update(alt, target) )
> +        {
> +            offset = target - (unsigned long)insnptr;
> +            insn = insn_set_branch_offset(insn, offset);
> +        }
> +    }
> +
> +    return insn;
> +}
> +
> +static int __apply_alternatives(const struct alt_region *region)

__init?
> +{
> +    const struct alt_instr *alt;
> +    const u32 *origptr, *replptr;
> +    u32 *writeptr, *writemap;
> +    mfn_t text_mfn = _mfn(virt_to_mfn(_stext));
> +    unsigned int text_order = get_order_from_bytes(_end - _start);
> +
> +    printk("Patching kernel code\n");
> +
> +    /*
> +     * The text section is read-only. So re-map Xen to be able to patch
> +     * the code.
> +     */
> +    writemap = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
> +                      VMAP_DEFAULT);
> +    if ( !writemap )
> +    {
> +        printk("alternatives: Unable to map the text section\n");
> +        return -ENOMEM;
> +    }
> +
> +    for ( alt = region->begin; alt < region->end; alt++ )
> +    {
> +        u32 insn;
> +        int i, nr_inst;
> +
> +        if ( !cpus_have_cap(alt->cpufeature) )
> +            continue;
> +
> +        BUG_ON(alt->alt_len != alt->orig_len);
> +
> +        origptr = ALT_ORIG_PTR(alt);
> +        writeptr = origptr - (u32 *)_start + writemap;
> +        replptr = ALT_REPL_PTR(alt);
> +
> +        nr_inst = alt->alt_len / sizeof(insn);
> +
> +        for ( i = 0; i < nr_inst; i++ )
> +        {
> +            insn = get_alt_insn(alt, origptr + i, replptr + i);
> +            *(writeptr + i) = cpu_to_le32(insn);
> +        }
> +
> +        /* Ensure the new instructions reached the memory and nuke */
> +        clean_and_invalidate_dcache_va_range(writeptr,
> +                                             (sizeof (*writeptr) * nr_inst));
> +    }
> +
> +    /* Nuke the instruction cache */
> +    invalidate_icache();
> +
> +    vunmap(writemap);
> +
> +    return 0;
> +}
> +
> +/*
> + * We might be patching the stop_machine state machine, so implement a
> + * really simple polling protocol here.
> + */
> +static int __apply_alternatives_multi_stop(void *unused)

__init ?
> +{
> +    static int patched = 0;
> +    struct alt_region region = {

const ?
> +        .begin = __alt_instructions,
> +        .end = __alt_instructions_end,
> +    };
> +
> +    /* We always have a CPU 0 at this point (__init) */
> +    if ( smp_processor_id() )
> +    {
> +        while ( !read_atomic(&patched) )
> +            cpu_relax();
> +        isb();
> +    }
> +    else
> +    {
> +        int ret;
> +
> +        BUG_ON(patched);
> +        ret = __apply_alternatives(&region);
> +        /* The patching is not expected to fail during boot. */
> +        BUG_ON(ret != 0);
> +
> +        /* Barriers provided by the cache flushing */
> +        write_atomic(&patched, 1);
> +    }
> +
> +    return 0;
> +}
> +
> +void __init apply_alternatives_all(void)
> +{
> +    int ret;
> +
> +	/* better not try code patching on a live SMP system */
> +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL, NR_CPUS);

NR_CPUS? Not 'num_online_cpus' ?

And I am bit confused. The comment says that 'stop_machine' state machine
may be patched, but here you use the stop_machine_run which is part of
stop_machine?!

> +
> +    /* stop_machine_run should never fail at this stage of the boot */
> +    BUG_ON(ret);
> +}
> +
> +int apply_alternatives(void *start, size_t length)

const void *start?
> +{

const ?
> +    struct alt_region region = {
> +        .begin = start,
> +        .end = start + length,
> +    };
> +
> +    return __apply_alternatives(&region);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 09ff1ea..c4389ef 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -38,6 +38,7 @@
>  #include <xen/vmap.h>
>  #include <xen/libfdt/libfdt.h>
>  #include <xen/acpi.h>
> +#include <asm/alternative.h>
>  #include <asm/page.h>
>  #include <asm/current.h>
>  #include <asm/setup.h>
> @@ -834,6 +835,13 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      do_initcalls();
>  
> +    /*
> +     * It needs to be called after do_initcalls to be able to use
> +     * stop_machine (tasklets initialized via an initcall).
> +     * XXX: Is it too late?

What else is going to run in initcalls? Why can't this patching be done
when you only have one CPU?

> +     */
> +    apply_alternatives_all();
> +
>      /* Create initial domain 0. */
>      /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>      config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1f010bd..80d9703 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -167,6 +167,13 @@ SECTIONS
>         *(.xsm_initcall.init)
>         __xsm_initcall_end = .;
>    } :text
> +#ifdef CONFIG_ALTERNATIVE
> +  .init.alternatives : {
> +      __alt_instructions = .;
> +      *(.altinstructions)
> +      __alt_instructions_end = .;
> +  }
> +#endif
>    __init_end_efi = .;
>    . = ALIGN(STACK_SIZE);
>    __init_end = .;
> diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
> new file mode 100644
> index 0000000..7e3a610
> --- /dev/null
> +++ b/xen/include/asm-arm/alternative.h
> @@ -0,0 +1,165 @@
> +#ifndef __ASM_ALTERNATIVE_H
> +#define __ASM_ALTERNATIVE_H
> +
> +#include <asm/cpufeature.h>
> +#include <xen/config.h>
> +#include <xen/kconfig.h>
> +
> +#ifdef CONFIG_ALTERNATIVE
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/init.h>
> +#include <xen/types.h>
> +#include <xen/stringify.h>
> +
> +struct alt_instr {
> +	s32 orig_offset;	/* offset to original instruction */
> +	s32 alt_offset;		/* offset to replacement instruction */
> +	u16 cpufeature;		/* cpufeature bit set for replacement */
> +	u8  orig_len;		/* size of original instruction(s) */
> +	u8  alt_len;		/* size of new instruction(s), <= orig_len */
> +};
> +
> +void __init apply_alternatives_all(void);
> +int apply_alternatives(void *start, size_t length);
> +
> +#define ALTINSTR_ENTRY(feature)						      \
> +	" .word 661b - .\n"				/* label           */ \
> +	" .word 663f - .\n"				/* new instruction */ \
> +	" .hword " __stringify(feature) "\n"		/* feature bit     */ \
> +	" .byte 662b-661b\n"				/* source len      */ \
> +	" .byte 664f-663f\n"				/* replacement len */
> +
> +/*
> + * alternative assembly primitive:
> + *
> + * If any of these .org directive fail, it means that insn1 and insn2
> + * don't have the same length. This used to be written as
> + *
> + * .if ((664b-663b) != (662b-661b))
> + * 	.error "Alternatives instruction length mismatch"
> + * .endif
> + *
> + * but most assemblers die if insn1 or insn2 have a .inst. This should
> + * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
> + * containing commit 4e4d08cf7399b606 or c1baaddf8861).
> + */
> +#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled)	\
> +	".if "__stringify(cfg_enabled)" == 1\n"				\
> +	"661:\n\t"							\
> +	oldinstr "\n"							\
> +	"662:\n"							\
> +	".pushsection .altinstructions,\"a\"\n"				\
> +	ALTINSTR_ENTRY(feature)						\
> +	".popsection\n"							\
> +	".pushsection .altinstr_replacement, \"a\"\n"			\
> +	"663:\n\t"							\
> +	newinstr "\n"							\
> +	"664:\n\t"							\
> +	".popsection\n\t"						\
> +	".org	. - (664b-663b) + (662b-661b)\n\t"			\
> +	".org	. - (662b-661b) + (664b-663b)\n"			\
> +	".endif\n"
> +
> +#define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...)	\
> +	__ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
> +
> +#else
> +
> +#include <asm/asm_defns.h>
> +
> +.macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
> +	.word \orig_offset - .
> +	.word \alt_offset - .
> +	.hword \feature
> +	.byte \orig_len
> +	.byte \alt_len
> +.endm
> +
> +.macro alternative_insn insn1, insn2, cap, enable = 1
> +	.if \enable
> +661:	\insn1
> +662:	.pushsection .altinstructions, "a"
> +	altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
> +	.popsection
> +	.pushsection .altinstr_replacement, "ax"
> +663:	\insn2
> +664:	.popsection
> +	.org	. - (664b-663b) + (662b-661b)
> +	.org	. - (662b-661b) + (664b-663b)
> +	.endif
> +.endm
> +
> +/*
> + * Begin an alternative code sequence.
> + *
> + * The code that follows this macro will be assembled and linked as
> + * normal. There are no restrictions on this code.
> + */
> +.macro alternative_if_not cap, enable = 1
> +	.if \enable
> +	.pushsection .altinstructions, "a"
> +	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
> +	.popsection
> +661:
> +	.endif
> +.endm
> +
> +/*
> + * Provide the alternative code sequence.
> + *
> + * The code that follows this macro is assembled into a special
> + * section to be used for dynamic patching. Code that follows this
> + * macro must:
> + *
> + * 1. Be exactly the same length (in bytes) as the default code
> + *    sequence.
> + *
> + * 2. Not contain a branch target that is used outside of the
> + *    alternative sequence it is defined in (branches into an
> + *    alternative sequence are not fixed up).
> + */
> +.macro alternative_else
> +662:	.pushsection .altinstr_replacement, "ax"
> +663:
> +.endm
> +
> +/*
> + * Complete an alternative code sequence.
> + */
> +.macro alternative_endif
> +664:	.popsection
> +	.org	. - (664b-663b) + (662b-661b)
> +	.org	. - (662b-661b) + (664b-663b)
> +.endm
> +
> +#define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
> +	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
> +
> +#endif  /*  __ASSEMBLY__  */
> +
> +/*
> + * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature));
> + *
> + * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature, CONFIG_FOO));
> + * N.B. If CONFIG_FOO is specified, but not selected, the whole block
> + *      will be omitted, including oldinstr.
> + */
> +#define ALTERNATIVE(oldinstr, newinstr, ...)   \
> +	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> +
> +#else /* !CONFIG_ALTERNATIVE */
> +
> +static inline void apply_alternatives_all(void)
> +{
> +}
> +
> +int apply_alternatives(void *start, size_t lenght)
> +{
> +    return 0;
> +}
> +
> +#endif
> +
> +#endif /* __ASM_ALTERNATIVE_H */
> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
> index cfcdbe9..6ce37be 100644
> --- a/xen/include/asm-arm/arm64/insn.h
> +++ b/xen/include/asm-arm/arm64/insn.h
> @@ -61,6 +61,22 @@ u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>  s32 aarch64_get_branch_offset(u32 insn);
>  u32 aarch64_set_branch_offset(u32 insn, s32 offset);
>  
> +/* Wrapper for common code */
> +static inline bool insn_is_branch_imm(u32 insn)
> +{
> +    return aarch64_insn_is_branch_imm(insn);
> +}
> +
> +static inline s32 insn_get_branch_offset(u32 insn)
> +{
> +    return aarch64_get_branch_offset(insn);
> +}
> +
> +static inline u32 insn_set_branch_offset(u32 insn, s32 offset)
> +{
> +    return aarch64_set_branch_offset(insn, offset);
> +}
> +
>  #endif /* !__ARCH_ARM_ARM64_INSN */
>  /*
>   * Local variables:
> -- 
> 1.9.1
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

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

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

* Re: [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions
  2016-05-05 16:34 ` [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions Julien Grall
  2016-05-09 10:05   ` Stefano Stabellini
@ 2016-05-13 20:35   ` Konrad Rzeszutek Wilk
  2016-05-14 10:49     ` Julien Grall
  1 sibling, 1 reply; 55+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-13 20:35 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
> new file mode 100644
> index 0000000..cfcdbe9
> --- /dev/null
> +++ b/xen/include/asm-arm/arm64/insn.h
> @@ -0,0 +1,72 @@
> +/*
> + * Copyright (C) 2013 Huawei Ltd.
> + * Author: Jiang Liu <liuj97@gmail.com>
> + *
> + * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@gmail.com>
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +#ifndef __ARCH_ARM_ARM64_INSN
> +#define __ARCH_ARM_ARM64_INSN
> +
> +#include <xen/config.h>
> +#include <xen/types.h>
> +#include <xen/stdbool.h>
> +
> +enum aarch64_insn_imm_type {
> +	AARCH64_INSN_IMM_ADR,
> +	AARCH64_INSN_IMM_26,
> +	AARCH64_INSN_IMM_19,
> +	AARCH64_INSN_IMM_16,
> +	AARCH64_INSN_IMM_14,
> +	AARCH64_INSN_IMM_12,
> +	AARCH64_INSN_IMM_9,
> +	AARCH64_INSN_IMM_7,
> +	AARCH64_INSN_IMM_6,
> +	AARCH64_INSN_IMM_S,
> +	AARCH64_INSN_IMM_R,
> +	AARCH64_INSN_IMM_MAX
> +};
> +
> +#define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
> +static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
> +{ return (code & (mask)) == (val); } \
> +static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
> +{ return (val); }
> +
> +__AARCH64_INSN_FUNCS(b,		0xFC000000, 0x14000000)

This looks odd here but in the file the aligment is OK.
> +__AARCH64_INSN_FUNCS(bl,	0xFC000000, 0x94000000)
> +__AARCH64_INSN_FUNCS(cbz,	0x7F000000, 0x34000000)
> +__AARCH64_INSN_FUNCS(cbnz,	0x7F000000, 0x35000000)
> +__AARCH64_INSN_FUNCS(tbz,	0x7F000000, 0x36000000)
> +__AARCH64_INSN_FUNCS(tbnz,	0x7F000000, 0x37000000)
> +__AARCH64_INSN_FUNCS(bcond,	0xFF000010, 0x54000000)
> +
> +bool aarch64_insn_is_branch_imm(u32 insn);
> +
> +u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
> +				  u32 insn, u64 imm);

While this is off. I think you have tabs here instead of spaces?
> +
> +s32 aarch64_get_branch_offset(u32 insn);
> +u32 aarch64_set_branch_offset(u32 insn, s32 offset);
> +

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

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

* Re: [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly
  2016-05-05 16:34 ` [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly Julien Grall
@ 2016-05-13 20:37   ` Konrad Rzeszutek Wilk
  2016-05-14 18:04     ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-13 20:37 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> index 7a1b56b..aa7c5b1 100644
> --- a/xen/arch/arm/cpufeature.c
> +++ b/xen/arch/arm/cpufeature.c
> @@ -24,6 +24,22 @@
>  
>  DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>  
> +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> +                             const char *info)
> +{
> +    int i;
> +
> +    for ( i = 0; caps[i].matches; i++ )
> +    {
> +        if ( !caps[i].matches(&caps[i]) )
> +            continue;
> +
> +        if ( !cpus_have_cap(caps[i].capability) && caps[i].desc )
> +            printk("%s: %s\n", info, caps[i].desc);

dprintk?


> +        cpus_set_cap(caps[i].capability);
> +    }
> +}
> +
>  /*
>   * Local variables:
>   * mode: C

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

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

* Re: [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions
  2016-05-13 20:35   ` Konrad Rzeszutek Wilk
@ 2016-05-14 10:49     ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-14 10:49 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

Hi Konrad,

On 13/05/2016 21:35, Konrad Rzeszutek Wilk wrote:
>> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
>> new file mode 100644
>> index 0000000..cfcdbe9
>> --- /dev/null
>> +++ b/xen/include/asm-arm/arm64/insn.h
>> @@ -0,0 +1,72 @@
>> +/*
>> + * Copyright (C) 2013 Huawei Ltd.
>> + * Author: Jiang Liu <liuj97@gmail.com>
>> + *
>> + * Copyright (C) 2014 Zi Shen Lim <zlim.lnx@gmail.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +#ifndef __ARCH_ARM_ARM64_INSN
>> +#define __ARCH_ARM_ARM64_INSN
>> +
>> +#include <xen/config.h>
>> +#include <xen/types.h>
>> +#include <xen/stdbool.h>
>> +
>> +enum aarch64_insn_imm_type {
>> +	AARCH64_INSN_IMM_ADR,
>> +	AARCH64_INSN_IMM_26,
>> +	AARCH64_INSN_IMM_19,
>> +	AARCH64_INSN_IMM_16,
>> +	AARCH64_INSN_IMM_14,
>> +	AARCH64_INSN_IMM_12,
>> +	AARCH64_INSN_IMM_9,
>> +	AARCH64_INSN_IMM_7,
>> +	AARCH64_INSN_IMM_6,
>> +	AARCH64_INSN_IMM_S,
>> +	AARCH64_INSN_IMM_R,
>> +	AARCH64_INSN_IMM_MAX
>> +};
>> +
>> +#define	__AARCH64_INSN_FUNCS(abbr, mask, val)	\
>> +static always_inline bool_t aarch64_insn_is_##abbr(u32 code) \
>> +{ return (code & (mask)) == (val); } \
>> +static always_inline u32 aarch64_insn_get_##abbr##_value(void) \
>> +{ return (val); }
>> +
>> +__AARCH64_INSN_FUNCS(b,		0xFC000000, 0x14000000)
>
> This looks odd here but in the file the aligment is OK.
>> +__AARCH64_INSN_FUNCS(bl,	0xFC000000, 0x94000000)
>> +__AARCH64_INSN_FUNCS(cbz,	0x7F000000, 0x34000000)
>> +__AARCH64_INSN_FUNCS(cbnz,	0x7F000000, 0x35000000)
>> +__AARCH64_INSN_FUNCS(tbz,	0x7F000000, 0x36000000)
>> +__AARCH64_INSN_FUNCS(tbnz,	0x7F000000, 0x37000000)
>> +__AARCH64_INSN_FUNCS(bcond,	0xFF000010, 0x54000000)
>> +
>> +bool aarch64_insn_is_branch_imm(u32 insn);
>> +
>> +u64 aarch64_insn_decode_immediate(enum aarch64_insn_imm_type type, u32 insn);
>> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>> +				  u32 insn, u64 imm);
>
> While this is off. I think you have tabs here instead of spaces?

The code is a verbatim copy from Linux, so I would prefer to not modify 
the code for future port/bug fixing.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-13 20:26   ` Konrad Rzeszutek Wilk
@ 2016-05-14 18:02     ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-14 18:02 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

Hi Konrad,

On 13/05/2016 21:26, Konrad Rzeszutek Wilk wrote:
> On Thu, May 05, 2016 at 05:34:19PM +0100, Julien Grall wrote:
>> Some of the processor erratum will require to modify code sequence.
>> As those modifications may impact the performance, they should only
>> be enabled on affected cores. Furthermore, Xen may also want to take
>> advantage of new hardware features coming up with v8.1 and v8.2.
>>
>> This patch adds an infrastructure to patch Xen during boot time
>> depending on the "features" available on the platform.
>>
>> This code is based on the file arch/arm64/kernel/alternative.c in
>> Linux 4.6-rc3. Any references to arm64 have been dropped to make the
>> code as generic as possible.
>>
>> Furthermore, in Xen the executable sections (.text and .init.text)
>> are always mapped read-only and enforced by SCTLR.WNX. So it is not
>> possible to directly patch Xen.
>
> Is it not possible to temporarily 'unenforce' the SCTLR.WNX?

The problem is not because of SCTLR.WNX but because the entries are 
change from read-write to read-only afterwards.

The ARM ARM (see D4-1732 in ARM DDI 0487A.i) recommends the use of 
break-before-make if either the old or new entry is writable when the 
page table is shared between multiples processor. This is to avoid 
possible TLB conflicts on platform have aggressive prefetcher.

The break-before-make approach requires the following steps:
    - Replacing the old entry with an invalid entry
    - Invalidate the entry by flush the TLBs
    - Write the new entry

However, with the current approach, secondary CPUs runs at the same, so 
even with the break-before-make approach it might be possible to hit a 
data abort if the CPU execute code on an entry that is currently been 
changed.

It would be possible to make the CPUs spinning outside of the 
executable, but I think this make the code more complicate. Note that 
you would have the same issue with XSplice.

You may wonder why Xen is patched when all the CPUs are online. This is 
because in big.LITTLE environment, big and little cores will have 
different errata. The only way to know the list of errata is to probe 
each CPU, and therefore bring them up.

I hope this gives you more insights on why I choose this approach. I can 
update the commit message with it.

>
>>
>> To by-pass this restriction, a temporary writeable mapping is made with
>> vmap. This mapping will be used to write the new instructions.
>>
>> Lastly, runtime patching is currently not necessary for ARM32. So the
>> code is only enabled for ARM64.
>>
>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>> ---
>>   xen/arch/arm/Kconfig              |   5 +
>>   xen/arch/arm/Makefile             |   1 +
>>   xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
>>   xen/arch/arm/setup.c              |   8 ++
>>   xen/arch/arm/xen.lds.S            |   7 ++
>>   xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
>>   xen/include/asm-arm/arm64/insn.h  |  16 +++
>>   7 files changed, 419 insertions(+)
>>   create mode 100644 xen/arch/arm/alternative.c
>>   create mode 100644 xen/include/asm-arm/alternative.h
>>
>> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
>> index 6231cd5..9b3e66b 100644
>> --- a/xen/arch/arm/Kconfig
>> +++ b/xen/arch/arm/Kconfig
>> @@ -14,6 +14,7 @@ config ARM_64
>>   	def_bool y
>>   	depends on 64BIT
>>   	select HAS_GICV3
>> +    select ALTERNATIVE
>
> Hard tabs, not spaces.

Right, I will change in the next version.

>>
>>   config ARM
>>   	def_bool y
>> @@ -46,6 +47,10 @@ config ACPI
>>   config HAS_GICV3
>>   	bool
>>
>> +# Select ALTERNATIVE if the architecture supports runtime patching
>> +config ALTERNATIVE
>> +    bool
>
> Ditto.

Ok.

>> +
>>   endmenu
>>
>>   source "common/Kconfig"
>> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
>> index 9122f78..2d330aa 100644
>> --- a/xen/arch/arm/Makefile
>> +++ b/xen/arch/arm/Makefile
>> @@ -4,6 +4,7 @@ subdir-y += platforms
>>   subdir-$(CONFIG_ARM_64) += efi
>>   subdir-$(CONFIG_ACPI) += acpi
>>
>> +obj-$(CONFIG_ALTERNATIVE) += alternative.o
>
> This probably needs to be alternative.init.o ?
>>   obj-y += bootfdt.o
>>   obj-y += cpu.o
>>   obj-y += cpufeature.o
>> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
>> new file mode 100644
>> index 0000000..0a5d1c8
>> --- /dev/null
>> +++ b/xen/arch/arm/alternative.c
>> @@ -0,0 +1,217 @@
>> +/*
>> + * alternative runtime patching
>> + * inspired by the x86 version
>> + *
>> + * Copyright (C) 2014 ARM Ltd.
>
> Not 2016?

The code has been taken from Linux. I was not sure what to do here. I 
will update with 2014-2016.

>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
>> + * GNU General Public License for more details.
>> + *
>> + * You should have received a copy of the GNU General Public License
>> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>> + */
>> +
>> +#include <xen/config.h>
>> +#include <xen/init.h>
>> +#include <xen/types.h>
>> +#include <xen/kernel.h>
>> +#include <xen/mm.h>
>> +#include <xen/vmap.h>
>> +#include <xen/smp.h>
>> +#include <xen/stop_machine.h>
>> +#include <asm/alternative.h>
>> +#include <asm/atomic.h>
>> +#include <asm/byteorder.h>
>> +#include <asm/cpufeature.h>
>> +#include <asm/insn.h>
>> +#include <asm/page.h>
>> +
>> +#define __ALT_PTR(a,f)      (u32 *)((void *)&(a)->f + (a)->f)
>> +#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
>> +#define ALT_REPL_PTR(a)     __ALT_PTR(a, alt_offset)
>> +
>> +extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
>> +
>> +struct alt_region {
>> +    const struct alt_instr *begin;
>> +    const struct alt_instr *end;
>> +};
>> +
>> +/*
>> + * Check if the target PC is within an alternative block.
>> + */
>> +static bool_t branch_insn_requires_update(const struct alt_instr *alt,
>
> __init?

No, xSplice will likely contain some alternatives, which Xen will need 
to apply.

I have in mind invasive errata such as the one implemented in patch #13 
and #14. Furthermore, new ARMv8.1 features will requires some to add 
alternatives in spinlock and atomic helpers.

>> +{
>> +    static int patched = 0;
>> +    struct alt_region region = {
>
> const ?

sure.

>> +        .begin = __alt_instructions,
>> +        .end = __alt_instructions_end,
>> +    };
>> +
>> +    /* We always have a CPU 0 at this point (__init) */
>> +    if ( smp_processor_id() )
>> +    {
>> +        while ( !read_atomic(&patched) )
>> +            cpu_relax();
>> +        isb();
>> +    }
>> +    else
>> +    {
>> +        int ret;
>> +
>> +        BUG_ON(patched);
>> +        ret = __apply_alternatives(&region);
>> +        /* The patching is not expected to fail during boot. */
>> +        BUG_ON(ret != 0);
>> +
>> +        /* Barriers provided by the cache flushing */
>> +        write_atomic(&patched, 1);
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +void __init apply_alternatives_all(void)
>> +{
>> +    int ret;
>> +
>> +	/* better not try code patching on a live SMP system */
>> +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL, NR_CPUS);
>
> NR_CPUS? Not 'num_online_cpus' ?

The 3rd argument of stop_machine_run is the CPU where the callback will 
be executed. If NR_CPUS is passed, the callback will be executed on all 
the CPUs.

>
> And I am bit confused. The comment says that 'stop_machine' state machine
> may be patched, but here you use the stop_machine_run which is part of
> stop_machine?!

Yes, hence the callback __apply_alternatives_multi_stop is executed on 
every online CPUs. CPU0 will patch the code whilst the other will spin 
until the code has been patched.

>
>> +
>> +    /* stop_machine_run should never fail at this stage of the boot */
>> +    BUG_ON(ret);
>> +}
>> +
>> +int apply_alternatives(void *start, size_t length)
>
> const void *start?

Could be.

>> +{
>
> const ?

Yes.

>> +    struct alt_region region = {
>> +        .begin = start,
>> +        .end = start + length,
>> +    };
>> +
>> +    return __apply_alternatives(&region);
>> +}
>> +
>> +/*
>> + * Local variables:
>> + * mode: C
>> + * c-file-style: "BSD"
>> + * c-basic-offset: 4
>> + * indent-tabs-mode: nil
>> + * End:
>> + */
>> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
>> index 09ff1ea..c4389ef 100644
>> --- a/xen/arch/arm/setup.c
>> +++ b/xen/arch/arm/setup.c
>> @@ -38,6 +38,7 @@
>>   #include <xen/vmap.h>
>>   #include <xen/libfdt/libfdt.h>
>>   #include <xen/acpi.h>
>> +#include <asm/alternative.h>
>>   #include <asm/page.h>
>>   #include <asm/current.h>
>>   #include <asm/setup.h>
>> @@ -834,6 +835,13 @@ void __init start_xen(unsigned long boot_phys_offset,
>>
>>       do_initcalls();
>>
>> +    /*
>> +     * It needs to be called after do_initcalls to be able to use
>> +     * stop_machine (tasklets initialized via an initcall).
>> +     * XXX: Is it too late?
>
> What else is going to run in initcalls? Why can't this patching be done
> when you only have one CPU?

See my answer above. Xen needs to know the list of errata that affect 
each CPU before patching.

Regards,
-- 
Julien Grall

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

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

* Re: [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly
  2016-05-13 20:37   ` Konrad Rzeszutek Wilk
@ 2016-05-14 18:04     ` Julien Grall
  2016-05-16 13:50       ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-14 18:04 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

Hi Konrad,

On 13/05/2016 21:37, Konrad Rzeszutek Wilk wrote:
>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>> index 7a1b56b..aa7c5b1 100644
>> --- a/xen/arch/arm/cpufeature.c
>> +++ b/xen/arch/arm/cpufeature.c
>> @@ -24,6 +24,22 @@
>>
>>   DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>>
>> +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>> +                             const char *info)
>> +{
>> +    int i;
>> +
>> +    for ( i = 0; caps[i].matches; i++ )
>> +    {
>> +        if ( !caps[i].matches(&caps[i]) )
>> +            continue;
>> +
>> +        if ( !cpus_have_cap(caps[i].capability) && caps[i].desc )
>> +            printk("%s: %s\n", info, caps[i].desc);
>
> dprintk?

No, this message is useful to know the list of "features" available for 
the platform.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly
  2016-05-14 18:04     ` Julien Grall
@ 2016-05-16 13:50       ` Konrad Rzeszutek Wilk
  2016-05-16 13:54         ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-05-16 13:50 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Sat, May 14, 2016 at 07:04:19PM +0100, Julien Grall wrote:
> Hi Konrad,
> 
> On 13/05/2016 21:37, Konrad Rzeszutek Wilk wrote:
> >>diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
> >>index 7a1b56b..aa7c5b1 100644
> >>--- a/xen/arch/arm/cpufeature.c
> >>+++ b/xen/arch/arm/cpufeature.c
> >>@@ -24,6 +24,22 @@
> >>
> >>  DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
> >>
> >>+void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
> >>+                             const char *info)
> >>+{
> >>+    int i;
> >>+
> >>+    for ( i = 0; caps[i].matches; i++ )
> >>+    {
> >>+        if ( !caps[i].matches(&caps[i]) )
> >>+            continue;
> >>+
> >>+        if ( !cpus_have_cap(caps[i].capability) && caps[i].desc )
> >>+            printk("%s: %s\n", info, caps[i].desc);
> >
> >dprintk?
> 
> No, this message is useful to know the list of "features" available for the
> platform.

What about level? INFO?
> 
> Regards,
> 
> -- 
> Julien Grall

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

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

* Re: [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly
  2016-05-16 13:50       ` Konrad Rzeszutek Wilk
@ 2016-05-16 13:54         ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-16 13:54 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

Hi Konrad,

On 16/05/16 14:50, Konrad Rzeszutek Wilk wrote:
> On Sat, May 14, 2016 at 07:04:19PM +0100, Julien Grall wrote:
>> Hi Konrad,
>>
>> On 13/05/2016 21:37, Konrad Rzeszutek Wilk wrote:
>>>> diff --git a/xen/arch/arm/cpufeature.c b/xen/arch/arm/cpufeature.c
>>>> index 7a1b56b..aa7c5b1 100644
>>>> --- a/xen/arch/arm/cpufeature.c
>>>> +++ b/xen/arch/arm/cpufeature.c
>>>> @@ -24,6 +24,22 @@
>>>>
>>>>   DECLARE_BITMAP(cpu_hwcaps, ARM_NCAPS);
>>>>
>>>> +void update_cpu_capabilities(const struct arm_cpu_capabilities *caps,
>>>> +                             const char *info)
>>>> +{
>>>> +    int i;
>>>> +
>>>> +    for ( i = 0; caps[i].matches; i++ )
>>>> +    {
>>>> +        if ( !caps[i].matches(&caps[i]) )
>>>> +            continue;
>>>> +
>>>> +        if ( !cpus_have_cap(caps[i].capability) && caps[i].desc )
>>>> +            printk("%s: %s\n", info, caps[i].desc);
>>>
>>> dprintk?
>>
>> No, this message is useful to know the list of "features" available for the
>> platform.
>
> What about level? INFO?

INFO would be fine. I keep forgetting that the default is WARNING.

I will fix it in the next version.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround
  2016-05-05 16:34 ` [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround Julien Grall
@ 2016-05-21 14:40   ` Stefano Stabellini
  2016-05-23 13:39     ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-21 14:40 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> The ARM errata 819472, 827319 and 824069 define the same workaround for
> these hardware issues in certain Cortex-A53 parts.
> 
> The cache instructions "dc cvac" and "dc cvau" need to be upgraded to
> "dc civac".
> 
> Use the alternative framework to replace those instructions only on
> affected cores.
> 
> Whilst the errata affect cache instructions issued at any exception
> level, it is not necessary to trap EL1/EL0 data cache instructions
> access in order to upgrade them. Indeed the data cache corruption would
> always be at the address used by the data cache instructions. Note that
> this address could point to a shared memory between guests and the
> hypervisors, however all the information present in it are be validated
> before any use.
> 
> Therefore a malicious guest could only hurt itself. Note that all the
> guests should implement/enable the workaround for the affected cores.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  docs/misc/arm/silicon-errata.txt |  3 ++
>  xen/arch/arm/Kconfig             | 71 ++++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/arm64/cache.S       |  8 ++++-
>  xen/arch/arm/cpuerrata.c         | 17 ++++++++++
>  xen/include/asm-arm/arm64/page.h |  7 +++-
>  xen/include/asm-arm/cpufeature.h |  4 ++-
>  xen/include/asm-arm/processor.h  |  6 ++++
>  7 files changed, 113 insertions(+), 3 deletions(-)
> 
> diff --git a/docs/misc/arm/silicon-errata.txt b/docs/misc/arm/silicon-errata.txt
> index 3f0d32b..9a2983b 100644
> --- a/docs/misc/arm/silicon-errata.txt
> +++ b/docs/misc/arm/silicon-errata.txt
> @@ -42,4 +42,7 @@ stable hypervisors.
>  | Implementor    | Component       | Erratum ID      | Kconfig                 |
>  +----------------+-----------------+-----------------+-------------------------+
>  | ARM            | Cortex-A15      | #766422         | N/A                     |
> +| ARM            | Cortex-A53      | #827319         | ARM64_ERRATUM_827319    |
> +| ARM            | Cortex-A53      | #824069         | ARM64_ERRATUM_824069    |
> +| ARM            | Cortex-A53      | #819472         | ARM64_ERRATUM_819472    |
>  | ARM            | Cortex-A57      | #852523         | N/A                     |
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 9b3e66b..6e4c769 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -53,6 +53,77 @@ config ALTERNATIVE
>  
>  endmenu
>  
> +menu "ARM errata workaround via the alternative framework"
> +	depends on ALTERNATIVE
> +
> +config ARM64_ERRATUM_827319
> +	bool "Cortex-A53: 827319: Data cache clean instructions might cause overlapping transactions to the interconnect"
> +	default y
> +	depends on ARM_64
> +	help
> +	  This option adds an alternative code sequence to work around ARM
> +	  erratum 827319 on Cortex-A53 parts up to r0p2 with an AMBA 5 CHI
> +	  master interface and an L2 cache.
> +
> +	  Under certain conditions this erratum can cause a clean line eviction
> +	  to occur at the same time as another transaction to the same address
> +	  on the AMBA 5 CHI interface, which can cause data corruption if the
> +	  interconnect reorders the two transactions.
> +
> +	  The workaround promotes data cache clean instructions to
> +	  data cache clean-and-invalidate.
> +	  Please note that this does not necessarily enable the workaround,
> +	  as it depends on the alternative framework, which will only patch
> +	  the kernel if an affected CPU is detected.
> +
> +	  If unsure, say Y.
> +
> +config ARM64_ERRATUM_824069
> +	bool "Cortex-A53: 824069: Cache line might not be marked as clean after a CleanShared snoop"
> +	default y
> +	depends on ARM_64
> +	help
> +	  This option adds an alternative code sequence to work around ARM
> +	  erratum 824069 on Cortex-A53 parts up to r0p2 when it is connected
> +	  to a coherent interconnect.
> +
> +	  If a Cortex-A53 processor is executing a store or prefetch for
> +	  write instruction at the same time as a processor in another
> +	  cluster is executing a cache maintenance operation to the same
> +	  address, then this erratum might cause a clean cache line to be
> +	  incorrectly marked as dirty.
> +
> +	  The workaround promotes data cache clean instructions to
> +	  data cache clean-and-invalidate.
> +	  Please note that this option does not necessarily enable the
> +	  workaround, as it depends on the alternative framework, which will
> +	  only patch the kernel if an affected CPU is detected.
> +
> +	  If unsure, say Y.
> +
> +config ARM64_ERRATUM_819472
> +	bool "Cortex-A53: 819472: Store exclusive instructions might cause data corruption"
> +	default y
> +	depends on ARM_64
> +	help
> +	  This option adds an alternative code sequence to work around ARM
> +	  erratum 819472 on Cortex-A53 parts up to r0p1 with an L2 cache
> +	  present when it is connected to a coherent interconnect.
> +
> +	  If the processor is executing a load and store exclusive sequence at
> +	  the same time as a processor in another cluster is executing a cache
> +	  maintenance operation to the same address, then this erratum might
> +	  cause data corruption.
> +
> +	  The workaround promotes data cache clean instructions to
> +	  data cache clean-and-invalidate.
> +	  Please note that this does not necessarily enable the workaround,
> +	  as it depends on the alternative framework, which will only patch
> +	  the kernel if an affected CPU is detected.
> +
> +	  If unsure, say Y.
> +endmenu

I am not sure whether we want to go into this level of configuration.
You have already introduced ALTERNATIVE, so somebody which might not
want any workarounds can disable them all. Otherwise do you have use
cases for allowing people to only enable some and not all?

If you are a distro you need to enable them all. If you are in the
embedded space and you are working with one specific SoC, you can
manually disable any workarounds you don't want.


>  source "common/Kconfig"
>  
>  source "drivers/Kconfig"
> diff --git a/xen/arch/arm/arm64/cache.S b/xen/arch/arm/arm64/cache.S
> index bc5a8f7..1a0c80c 100644
> --- a/xen/arch/arm/arm64/cache.S
> +++ b/xen/arch/arm/arm64/cache.S
> @@ -19,6 +19,8 @@
>   * along with this program.  If not, see <http://www.gnu.org/licenses/>.
>   */
>  
> +#include <asm/alternative.h>
> +
>  /*
>   * dcache_line_size - get the minimum D-cache line size from the CTR register.
>   */
> @@ -54,7 +56,11 @@ ENTRY(flush_icache_range)
>  	sub	x3, x2, #1
>  	bic	x4, x0, x3
>  1:
> -	dc	cvau, x4		// clean D line to PoU
> +alternative_if_not ARM64_WORKAROUND_CLEAN_CACHE
> +	dc	cvau, x4
> +alternative_else
> +	dc	civac, x4
> +alternative_endif
>  	add	x4, x4, x2
>  	cmp	x4, x1
>  	b.lo	1b
> diff --git a/xen/arch/arm/cpuerrata.c b/xen/arch/arm/cpuerrata.c
> index 52d39f8..211b520 100644
> --- a/xen/arch/arm/cpuerrata.c
> +++ b/xen/arch/arm/cpuerrata.c
> @@ -17,6 +17,23 @@ is_affected_midr_range(const struct arm_cpu_capabilities *entry)
>  }
>  
>  static const struct arm_cpu_capabilities arm_errata[] = {
> +#if defined(CONFIG_ARM64_ERRATUM_827319) || \
> +    defined(CONFIG_ARM64_ERRATUM_824069)
> +    {
> +        /* Cortex-A53 r0p[012] */
> +        .desc = "ARM errata 827319, 824069",
> +        .capability = ARM64_WORKAROUND_CLEAN_CACHE,
> +        MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x02),
> +    },
> +#endif
> +#ifdef CONFIG_ARM64_ERRATUM_819472
> +    {
> +        /* Cortex-A53 r0[01] */
> +        .desc = "ARM erratum 819472",
> +        .capability = ARM64_WORKAROUND_CLEAN_CACHE,
> +        MIDR_RANGE(MIDR_CORTEX_A53, 0x00, 0x01),
> +    },
> +#endif
>      {},
>  };
>  
> diff --git a/xen/include/asm-arm/arm64/page.h b/xen/include/asm-arm/arm64/page.h
> index fbdc8fb..79ef7bd 100644
> --- a/xen/include/asm-arm/arm64/page.h
> +++ b/xen/include/asm-arm/arm64/page.h
> @@ -3,6 +3,8 @@
>  
>  #ifndef __ASSEMBLY__
>  
> +#include <asm/alternative.h>
> +
>  /* Write a pagetable entry */
>  static inline void write_pte(lpae_t *p, lpae_t pte)
>  {
> @@ -18,7 +20,10 @@ static inline void write_pte(lpae_t *p, lpae_t pte)
>  #define __invalidate_dcache_one(R) "dc ivac, %" #R ";"
>  
>  /* Inline ASM to flush dcache on register R (may be an inline asm operand) */
> -#define __clean_dcache_one(R) "dc cvac, %" #R ";"
> +#define __clean_dcache_one(R)                   \
> +    ALTERNATIVE("dc cvac, %" #R ";",            \
> +                "dc civac, %" #R ";",           \
> +                ARM64_WORKAROUND_CLEAN_CACHE)   \
>  
>  /* Inline ASM to clean and invalidate dcache on register R (may be an
>   * inline asm operand) */
> diff --git a/xen/include/asm-arm/cpufeature.h b/xen/include/asm-arm/cpufeature.h
> index fb57295..474a778 100644
> --- a/xen/include/asm-arm/cpufeature.h
> +++ b/xen/include/asm-arm/cpufeature.h
> @@ -35,7 +35,9 @@
>  #endif
>  #define cpu_has_security  (boot_cpu_feature32(security) > 0)
>  
> -#define ARM_NCAPS           0
> +#define ARM64_WORKAROUND_CLEAN_CACHE    0
> +
> +#define ARM_NCAPS           1
>  
>  #ifndef __ASSEMBLY__
>  
> diff --git a/xen/include/asm-arm/processor.h b/xen/include/asm-arm/processor.h
> index 1b701c5..4123951 100644
> --- a/xen/include/asm-arm/processor.h
> +++ b/xen/include/asm-arm/processor.h
> @@ -44,6 +44,12 @@
>          _model == (model) && _rv >= (rv_min) && _rv <= (rv_max);        \
>  })
>  
> +#define ARM_CPU_IMP_ARM             0x41
> +
> +#define ARM_CPU_PART_CORTEX_A53     0xD03
> +
> +#define MIDR_CORTEX_A53 MIDR_CPU_MODEL(ARM_CPU_IMP_ARM, ARM_CPU_PART_CORTEX_A53)
> +
>  /* MPIDR Multiprocessor Affinity Register */
>  #define _MPIDR_UP           (30)
>  #define MPIDR_UP            (_AC(1,U) << _MPIDR_UP)
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails
  2016-05-05 16:34 ` [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails Julien Grall
@ 2016-05-21 14:42   ` Stefano Stabellini
  2016-05-21 14:51     ` Stefano Stabellini
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-21 14:42 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> Based on ARM ARM (D4.5.3 in ARM DDI 0486A and B3.12.7 in ARM DDI 0406C.c),
> a Stage 1 translation error has priority over a Stage 2 translation error.
> 
> Therefore gva_to_ipa can only fail if another vCPU is playing with the
> page table.
> 
> Rather than injecting a custom fault, replay the instruction and let the
> processor injecting the correct fault.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>

Couldn't a guest purposely cause a DoS in the hypervisor this way?


>  xen/arch/arm/traps.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> index c0325d5..3acdba0 100644
> --- a/xen/arch/arm/traps.c
> +++ b/xen/arch/arm/traps.c
> @@ -2410,7 +2410,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
>  
>              rc = gva_to_ipa(gva, &gpa, GV2M_READ);
>              if ( rc == -EFAULT )
> -                goto bad_insn_abort;
> +                return; /* Try again */
>          }
>  
>          rc = p2m_mem_access_check(gpa, gva, npfec);
> @@ -2422,7 +2422,6 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
>      break;
>      }
>  
> -bad_insn_abort:
>      inject_iabt_exception(regs, gva, hsr.len);
>  }
>  
> @@ -2452,7 +2451,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
>      {
>          rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ);
>          if ( rc == -EFAULT )
> -            goto bad_data_abort;
> +            return; /* Try again */
>      }
>  
>      switch ( dabt.dfsc & 0x3f )
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails
  2016-05-21 14:42   ` Stefano Stabellini
@ 2016-05-21 14:51     ` Stefano Stabellini
  2016-05-23 13:45       ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-21 14:51 UTC (permalink / raw)
  To: Stefano Stabellini
  Cc: andre.przywara, Julien Grall, steve.capper, wei.chen, xen-devel

On Sat, 21 May 2016, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
> > Based on ARM ARM (D4.5.3 in ARM DDI 0486A and B3.12.7 in ARM DDI 0406C.c),
> > a Stage 1 translation error has priority over a Stage 2 translation error.
> > 
> > Therefore gva_to_ipa can only fail if another vCPU is playing with the
> > page table.
> > 
> > Rather than injecting a custom fault, replay the instruction and let the
> > processor injecting the correct fault.
> > 
> > Signed-off-by: Julien Grall <julien.grall@arm.com>
> 
> Couldn't a guest purposely cause a DoS in the hypervisor this way?

Just double-checking. I am pretty sure it cannot, because the replayed
instruction won't cause another hypervisor trap the second time around.


> >  xen/arch/arm/traps.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
> > 
> > diff --git a/xen/arch/arm/traps.c b/xen/arch/arm/traps.c
> > index c0325d5..3acdba0 100644
> > --- a/xen/arch/arm/traps.c
> > +++ b/xen/arch/arm/traps.c
> > @@ -2410,7 +2410,7 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
> >  
> >              rc = gva_to_ipa(gva, &gpa, GV2M_READ);
> >              if ( rc == -EFAULT )
> > -                goto bad_insn_abort;
> > +                return; /* Try again */
> >          }
> >  
> >          rc = p2m_mem_access_check(gpa, gva, npfec);
> > @@ -2422,7 +2422,6 @@ static void do_trap_instr_abort_guest(struct cpu_user_regs *regs,
> >      break;
> >      }
> >  
> > -bad_insn_abort:
> >      inject_iabt_exception(regs, gva, hsr.len);
> >  }
> >  
> > @@ -2452,7 +2451,7 @@ static void do_trap_data_abort_guest(struct cpu_user_regs *regs,
> >      {
> >          rc = gva_to_ipa(info.gva, &info.gpa, GV2M_READ);
> >          if ( rc == -EFAULT )
> > -            goto bad_data_abort;
> > +            return; /* Try again */
> >      }
> >  
> >      switch ( dabt.dfsc & 0x3f )
> > -- 
> > 1.9.1
> > 
> 

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-05 16:34 ` [RFC 10/16] xen/arm: Introduce alternative runtime patching Julien Grall
  2016-05-13 20:26   ` Konrad Rzeszutek Wilk
@ 2016-05-21 15:09   ` Stefano Stabellini
  2016-05-23 11:11     ` Julien Grall
  1 sibling, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-21 15:09 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, sstabellini, steve.capper, wei.chen, xen-devel

On Thu, 5 May 2016, Julien Grall wrote:
> Some of the processor erratum will require to modify code sequence.
> As those modifications may impact the performance, they should only
> be enabled on affected cores. Furthermore, Xen may also want to take
> advantage of new hardware features coming up with v8.1 and v8.2.
> 
> This patch adds an infrastructure to patch Xen during boot time
> depending on the "features" available on the platform.
> 
> This code is based on the file arch/arm64/kernel/alternative.c in
> Linux 4.6-rc3. Any references to arm64 have been dropped to make the
> code as generic as possible.
> 
> Furthermore, in Xen the executable sections (.text and .init.text)
> are always mapped read-only and enforced by SCTLR.WNX. So it is not
> possible to directly patch Xen.
> 
> To by-pass this restriction, a temporary writeable mapping is made with
> vmap. This mapping will be used to write the new instructions.
> 
> Lastly, runtime patching is currently not necessary for ARM32. So the
> code is only enabled for ARM64.
> 
> Signed-off-by: Julien Grall <julien.grall@arm.com>
> ---
>  xen/arch/arm/Kconfig              |   5 +
>  xen/arch/arm/Makefile             |   1 +
>  xen/arch/arm/alternative.c        | 217 ++++++++++++++++++++++++++++++++++++++
>  xen/arch/arm/setup.c              |   8 ++
>  xen/arch/arm/xen.lds.S            |   7 ++
>  xen/include/asm-arm/alternative.h | 165 +++++++++++++++++++++++++++++
>  xen/include/asm-arm/arm64/insn.h  |  16 +++
>  7 files changed, 419 insertions(+)
>  create mode 100644 xen/arch/arm/alternative.c
>  create mode 100644 xen/include/asm-arm/alternative.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 6231cd5..9b3e66b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -14,6 +14,7 @@ config ARM_64
>  	def_bool y
>  	depends on 64BIT
>  	select HAS_GICV3
> +    select ALTERNATIVE
>  
>  config ARM
>  	def_bool y
> @@ -46,6 +47,10 @@ config ACPI
>  config HAS_GICV3
>  	bool
>  
> +# Select ALTERNATIVE if the architecture supports runtime patching
> +config ALTERNATIVE
> +    bool
> +
>  endmenu
>  
>  source "common/Kconfig"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 9122f78..2d330aa 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -4,6 +4,7 @@ subdir-y += platforms
>  subdir-$(CONFIG_ARM_64) += efi
>  subdir-$(CONFIG_ACPI) += acpi
>  
> +obj-$(CONFIG_ALTERNATIVE) += alternative.o
>  obj-y += bootfdt.o
>  obj-y += cpu.o
>  obj-y += cpufeature.o
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> new file mode 100644
> index 0000000..0a5d1c8
> --- /dev/null
> +++ b/xen/arch/arm/alternative.c
> @@ -0,0 +1,217 @@
> +/*
> + * alternative runtime patching
> + * inspired by the x86 version
> + *
> + * Copyright (C) 2014 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <xen/config.h>
> +#include <xen/init.h>
> +#include <xen/types.h>
> +#include <xen/kernel.h>
> +#include <xen/mm.h>
> +#include <xen/vmap.h>
> +#include <xen/smp.h>
> +#include <xen/stop_machine.h>
> +#include <asm/alternative.h>
> +#include <asm/atomic.h>
> +#include <asm/byteorder.h>
> +#include <asm/cpufeature.h>
> +#include <asm/insn.h>
> +#include <asm/page.h>
> +
> +#define __ALT_PTR(a,f)      (u32 *)((void *)&(a)->f + (a)->f)
> +#define ALT_ORIG_PTR(a)     __ALT_PTR(a, orig_offset)
> +#define ALT_REPL_PTR(a)     __ALT_PTR(a, alt_offset)
> +
> +extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
> +
> +struct alt_region {
> +    const struct alt_instr *begin;
> +    const struct alt_instr *end;
> +};
> +
> +/*
> + * Check if the target PC is within an alternative block.
> + */
> +static bool_t branch_insn_requires_update(const struct alt_instr *alt,
> +                                          unsigned long pc)
> +{
> +    unsigned long replptr;
> +
> +    if ( is_active_kernel_text(pc) )
> +        return 1;
> +
> +    replptr = (unsigned long)ALT_REPL_PTR(alt);
> +    if ( pc >= replptr && pc <= (replptr + alt->alt_len) )
> +        return 0;
> +
> +    /*
> +     * Branching into *another* alternate sequence is doomed, and
> +     * we're not even trying to fix it up.
> +     */
> +    BUG();
> +}
> +
> +static u32 get_alt_insn(const struct alt_instr *alt,
> +                        const u32 *insnptr, const u32 *altinsnptr)
> +{
> +    u32 insn;
> +
> +    insn = le32_to_cpu(*altinsnptr);
> +
> +    if ( insn_is_branch_imm(insn) )
> +    {
> +        s32 offset = insn_get_branch_offset(insn);
> +        unsigned long target;
> +
> +        target = (unsigned long)altinsnptr + offset;
> +
> +        /*
> +         * If we're branching inside the alternate sequence,
> +         * do not rewrite the instruction, as it is already
> +         * correct. Otherwise, generate the new instruction.
> +         */
> +        if ( branch_insn_requires_update(alt, target) )
> +        {
> +            offset = target - (unsigned long)insnptr;
> +            insn = insn_set_branch_offset(insn, offset);
> +        }
> +    }
> +
> +    return insn;
> +}
> +
> +static int __apply_alternatives(const struct alt_region *region)
> +{
> +    const struct alt_instr *alt;
> +    const u32 *origptr, *replptr;
> +    u32 *writeptr, *writemap;
> +    mfn_t text_mfn = _mfn(virt_to_mfn(_stext));
> +    unsigned int text_order = get_order_from_bytes(_end - _start);
> +
> +    printk("Patching kernel code\n");
> +
> +    /*
> +     * The text section is read-only. So re-map Xen to be able to patch
> +     * the code.
> +     */
> +    writemap = __vmap(&text_mfn, 1 << text_order, 1, 1, PAGE_HYPERVISOR,
> +                      VMAP_DEFAULT);
> +    if ( !writemap )
> +    {
> +        printk("alternatives: Unable to map the text section\n");
> +        return -ENOMEM;
> +    }
> +
> +    for ( alt = region->begin; alt < region->end; alt++ )
> +    {
> +        u32 insn;
> +        int i, nr_inst;
> +
> +        if ( !cpus_have_cap(alt->cpufeature) )
> +            continue;
> +
> +        BUG_ON(alt->alt_len != alt->orig_len);
> +
> +        origptr = ALT_ORIG_PTR(alt);
> +        writeptr = origptr - (u32 *)_start + writemap;
> +        replptr = ALT_REPL_PTR(alt);
> +
> +        nr_inst = alt->alt_len / sizeof(insn);
> +
> +        for ( i = 0; i < nr_inst; i++ )
> +        {
> +            insn = get_alt_insn(alt, origptr + i, replptr + i);
> +            *(writeptr + i) = cpu_to_le32(insn);
> +        }
> +
> +        /* Ensure the new instructions reached the memory and nuke */
> +        clean_and_invalidate_dcache_va_range(writeptr,
> +                                             (sizeof (*writeptr) * nr_inst));
> +    }
> +
> +    /* Nuke the instruction cache */
> +    invalidate_icache();
> +
> +    vunmap(writemap);
> +
> +    return 0;
> +}
> +
> +/*
> + * We might be patching the stop_machine state machine, so implement a
> + * really simple polling protocol here.
> + */
> +static int __apply_alternatives_multi_stop(void *unused)
> +{
> +    static int patched = 0;
> +    struct alt_region region = {
> +        .begin = __alt_instructions,
> +        .end = __alt_instructions_end,
> +    };
> +
> +    /* We always have a CPU 0 at this point (__init) */
> +    if ( smp_processor_id() )
> +    {
> +        while ( !read_atomic(&patched) )
> +            cpu_relax();
> +        isb();
> +    }
> +    else
> +    {
> +        int ret;
> +
> +        BUG_ON(patched);
> +        ret = __apply_alternatives(&region);
> +        /* The patching is not expected to fail during boot. */
> +        BUG_ON(ret != 0);
> +
> +        /* Barriers provided by the cache flushing */
> +        write_atomic(&patched, 1);
> +    }
> +
> +    return 0;
> +}
> +
> +void __init apply_alternatives_all(void)
> +{
> +    int ret;
> +
> +	/* better not try code patching on a live SMP system */
> +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL, NR_CPUS);

Why not just call stop_machine_run, passing 0 as the cpu to run
__apply_alternatives_multi_stop on? Given that you already have
secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
missing?


> +    /* stop_machine_run should never fail at this stage of the boot */
> +    BUG_ON(ret);
> +}
> +
> +int apply_alternatives(void *start, size_t length)
> +{
> +    struct alt_region region = {
> +        .begin = start,
> +        .end = start + length,
> +    };
> +
> +    return __apply_alternatives(&region);
> +}
> +
> +/*
> + * Local variables:
> + * mode: C
> + * c-file-style: "BSD"
> + * c-basic-offset: 4
> + * indent-tabs-mode: nil
> + * End:
> + */
> diff --git a/xen/arch/arm/setup.c b/xen/arch/arm/setup.c
> index 09ff1ea..c4389ef 100644
> --- a/xen/arch/arm/setup.c
> +++ b/xen/arch/arm/setup.c
> @@ -38,6 +38,7 @@
>  #include <xen/vmap.h>
>  #include <xen/libfdt/libfdt.h>
>  #include <xen/acpi.h>
> +#include <asm/alternative.h>
>  #include <asm/page.h>
>  #include <asm/current.h>
>  #include <asm/setup.h>
> @@ -834,6 +835,13 @@ void __init start_xen(unsigned long boot_phys_offset,
>  
>      do_initcalls();
>  
> +    /*
> +     * It needs to be called after do_initcalls to be able to use
> +     * stop_machine (tasklets initialized via an initcall).
> +     * XXX: Is it too late?
> +     */
> +    apply_alternatives_all();
> +
>      /* Create initial domain 0. */
>      /* The vGIC for DOM0 is exactly emulating the hardware GIC */
>      config.gic_version = XEN_DOMCTL_CONFIG_GIC_NATIVE;
> diff --git a/xen/arch/arm/xen.lds.S b/xen/arch/arm/xen.lds.S
> index 1f010bd..80d9703 100644
> --- a/xen/arch/arm/xen.lds.S
> +++ b/xen/arch/arm/xen.lds.S
> @@ -167,6 +167,13 @@ SECTIONS
>         *(.xsm_initcall.init)
>         __xsm_initcall_end = .;
>    } :text
> +#ifdef CONFIG_ALTERNATIVE
> +  .init.alternatives : {
> +      __alt_instructions = .;
> +      *(.altinstructions)
> +      __alt_instructions_end = .;
> +  }
> +#endif
>    __init_end_efi = .;
>    . = ALIGN(STACK_SIZE);
>    __init_end = .;
> diff --git a/xen/include/asm-arm/alternative.h b/xen/include/asm-arm/alternative.h
> new file mode 100644
> index 0000000..7e3a610
> --- /dev/null
> +++ b/xen/include/asm-arm/alternative.h
> @@ -0,0 +1,165 @@
> +#ifndef __ASM_ALTERNATIVE_H
> +#define __ASM_ALTERNATIVE_H
> +
> +#include <asm/cpufeature.h>
> +#include <xen/config.h>
> +#include <xen/kconfig.h>
> +
> +#ifdef CONFIG_ALTERNATIVE
> +
> +#ifndef __ASSEMBLY__
> +
> +#include <xen/init.h>
> +#include <xen/types.h>
> +#include <xen/stringify.h>
> +
> +struct alt_instr {
> +	s32 orig_offset;	/* offset to original instruction */
> +	s32 alt_offset;		/* offset to replacement instruction */
> +	u16 cpufeature;		/* cpufeature bit set for replacement */
> +	u8  orig_len;		/* size of original instruction(s) */
> +	u8  alt_len;		/* size of new instruction(s), <= orig_len */
> +};
> +
> +void __init apply_alternatives_all(void);
> +int apply_alternatives(void *start, size_t length);
> +
> +#define ALTINSTR_ENTRY(feature)						      \
> +	" .word 661b - .\n"				/* label           */ \
> +	" .word 663f - .\n"				/* new instruction */ \
> +	" .hword " __stringify(feature) "\n"		/* feature bit     */ \
> +	" .byte 662b-661b\n"				/* source len      */ \
> +	" .byte 664f-663f\n"				/* replacement len */
> +
> +/*
> + * alternative assembly primitive:
> + *
> + * If any of these .org directive fail, it means that insn1 and insn2
> + * don't have the same length. This used to be written as
> + *
> + * .if ((664b-663b) != (662b-661b))
> + * 	.error "Alternatives instruction length mismatch"
> + * .endif
> + *
> + * but most assemblers die if insn1 or insn2 have a .inst. This should
> + * be fixed in a binutils release posterior to 2.25.51.0.2 (anything
> + * containing commit 4e4d08cf7399b606 or c1baaddf8861).
> + */
> +#define __ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg_enabled)	\
> +	".if "__stringify(cfg_enabled)" == 1\n"				\
> +	"661:\n\t"							\
> +	oldinstr "\n"							\
> +	"662:\n"							\
> +	".pushsection .altinstructions,\"a\"\n"				\
> +	ALTINSTR_ENTRY(feature)						\
> +	".popsection\n"							\
> +	".pushsection .altinstr_replacement, \"a\"\n"			\
> +	"663:\n\t"							\
> +	newinstr "\n"							\
> +	"664:\n\t"							\
> +	".popsection\n\t"						\
> +	".org	. - (664b-663b) + (662b-661b)\n\t"			\
> +	".org	. - (662b-661b) + (664b-663b)\n"			\
> +	".endif\n"
> +
> +#define _ALTERNATIVE_CFG(oldinstr, newinstr, feature, cfg, ...)	\
> +	__ALTERNATIVE_CFG(oldinstr, newinstr, feature, IS_ENABLED(cfg))
> +
> +#else
> +
> +#include <asm/asm_defns.h>
> +
> +.macro altinstruction_entry orig_offset alt_offset feature orig_len alt_len
> +	.word \orig_offset - .
> +	.word \alt_offset - .
> +	.hword \feature
> +	.byte \orig_len
> +	.byte \alt_len
> +.endm
> +
> +.macro alternative_insn insn1, insn2, cap, enable = 1
> +	.if \enable
> +661:	\insn1
> +662:	.pushsection .altinstructions, "a"
> +	altinstruction_entry 661b, 663f, \cap, 662b-661b, 664f-663f
> +	.popsection
> +	.pushsection .altinstr_replacement, "ax"
> +663:	\insn2
> +664:	.popsection
> +	.org	. - (664b-663b) + (662b-661b)
> +	.org	. - (662b-661b) + (664b-663b)
> +	.endif
> +.endm
> +
> +/*
> + * Begin an alternative code sequence.
> + *
> + * The code that follows this macro will be assembled and linked as
> + * normal. There are no restrictions on this code.
> + */
> +.macro alternative_if_not cap, enable = 1
> +	.if \enable
> +	.pushsection .altinstructions, "a"
> +	altinstruction_entry 661f, 663f, \cap, 662f-661f, 664f-663f
> +	.popsection
> +661:
> +	.endif
> +.endm
> +
> +/*
> + * Provide the alternative code sequence.
> + *
> + * The code that follows this macro is assembled into a special
> + * section to be used for dynamic patching. Code that follows this
> + * macro must:
> + *
> + * 1. Be exactly the same length (in bytes) as the default code
> + *    sequence.
> + *
> + * 2. Not contain a branch target that is used outside of the
> + *    alternative sequence it is defined in (branches into an
> + *    alternative sequence are not fixed up).
> + */
> +.macro alternative_else
> +662:	.pushsection .altinstr_replacement, "ax"
> +663:
> +.endm
> +
> +/*
> + * Complete an alternative code sequence.
> + */
> +.macro alternative_endif
> +664:	.popsection
> +	.org	. - (664b-663b) + (662b-661b)
> +	.org	. - (662b-661b) + (664b-663b)
> +.endm
> +
> +#define _ALTERNATIVE_CFG(insn1, insn2, cap, cfg, ...)	\
> +	alternative_insn insn1, insn2, cap, IS_ENABLED(cfg)
> +
> +#endif  /*  __ASSEMBLY__  */
> +
> +/*
> + * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature));
> + *
> + * Usage: asm(ALTERNATIVE(oldinstr, newinstr, feature, CONFIG_FOO));
> + * N.B. If CONFIG_FOO is specified, but not selected, the whole block
> + *      will be omitted, including oldinstr.
> + */
> +#define ALTERNATIVE(oldinstr, newinstr, ...)   \
> +	_ALTERNATIVE_CFG(oldinstr, newinstr, __VA_ARGS__, 1)
> +
> +#else /* !CONFIG_ALTERNATIVE */
> +
> +static inline void apply_alternatives_all(void)
> +{
> +}
> +
> +int apply_alternatives(void *start, size_t lenght)
> +{
> +    return 0;
> +}
> +
> +#endif
> +
> +#endif /* __ASM_ALTERNATIVE_H */
> diff --git a/xen/include/asm-arm/arm64/insn.h b/xen/include/asm-arm/arm64/insn.h
> index cfcdbe9..6ce37be 100644
> --- a/xen/include/asm-arm/arm64/insn.h
> +++ b/xen/include/asm-arm/arm64/insn.h
> @@ -61,6 +61,22 @@ u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>  s32 aarch64_get_branch_offset(u32 insn);
>  u32 aarch64_set_branch_offset(u32 insn, s32 offset);
>  
> +/* Wrapper for common code */
> +static inline bool insn_is_branch_imm(u32 insn)
> +{
> +    return aarch64_insn_is_branch_imm(insn);
> +}
> +
> +static inline s32 insn_get_branch_offset(u32 insn)
> +{
> +    return aarch64_get_branch_offset(insn);
> +}
> +
> +static inline u32 insn_set_branch_offset(u32 insn, s32 offset)
> +{
> +    return aarch64_set_branch_offset(insn, offset);
> +}
> +
>  #endif /* !__ARCH_ARM_ARM64_INSN */
>  /*
>   * Local variables:
> -- 
> 1.9.1
> 

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

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

* Re: [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions
  2016-05-09 13:04     ` Julien Grall
@ 2016-05-23 10:52       ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-23 10:52 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 09/05/16 14:04, Julien Grall wrote:
> On 09/05/16 11:05, Stefano Stabellini wrote:
>> On Thu, 5 May 2016, Julien Grall wrote:
>>> +u32 aarch64_insn_encode_immediate(enum aarch64_insn_imm_type type,
>>> +                  u32 insn, u64 imm)
>>> +{
>>> +    u32 immlo, immhi, mask;
>>> +    int shift;
>>
>> Here Linux checks for insn == AARCH64_BREAK_FAULT. Shouldn't we do the
>> same?
>
> I am not sure why I removed this check. I will re-add on the next version.

Actually the patch was based on Linux 4.6-rc3 (see in commit message) 
which does not contain the check. It has been added in 4.6-rc4.

I will re-sync the code with Linux 4.6.

>
>>> diff --git a/xen/include/asm-arm/insn.h b/xen/include/asm-arm/insn.h
>>> new file mode 100644
>>> index 0000000..fe9419c
>>> --- /dev/null
>>> +++ b/xen/include/asm-arm/insn.h
>>> @@ -0,0 +1,22 @@
>>> +#ifndef __ARCH_ARM_INSN
>>> +#define __ARCH_ARM_INSN
>>> +
>>> +#include <xen/types.h>
>>> +
>>> +#if defined(CONFIG_ARM_32)
>>> +# include <asm/arm32/insn.h>
>>
>> This is missing
>
> I will drop the include as I don't plan to implement instruction
> patching for ARM32.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-21 15:09   ` Stefano Stabellini
@ 2016-05-23 11:11     ` Julien Grall
  2016-05-30 14:45       ` Stefano Stabellini
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-23 11:11 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 21/05/16 16:09, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
>> +void __init apply_alternatives_all(void)
>> +{
>> +    int ret;
>> +
>> +	/* better not try code patching on a live SMP system */
>> +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL, NR_CPUS);
>
> Why not just call stop_machine_run, passing 0 as the cpu to run
> __apply_alternatives_multi_stop on? Given that you already have
> secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> missing?

Someone as to call __apply_alternatives_multi_stop on secondary CPUs. 
apply_alternatives is called by the boot CPU, the rest are spinning 
within the idle_loop.

The function stop_machine_run will stop all the CPUs, and call 
__apply_alternatives_multi_stop on each of them.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround
  2016-05-21 14:40   ` Stefano Stabellini
@ 2016-05-23 13:39     ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-23 13:39 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hello Stefano,

On 21/05/16 15:40, Stefano Stabellini wrote:
> On Thu, 5 May 2016, Julien Grall wrote:
>> +	bool "Cortex-A53: 819472: Store exclusive instructions might cause data corruption"
>> +	default y
>> +	depends on ARM_64
>> +	help
>> +	  This option adds an alternative code sequence to work around ARM
>> +	  erratum 819472 on Cortex-A53 parts up to r0p1 with an L2 cache
>> +	  present when it is connected to a coherent interconnect.
>> +
>> +	  If the processor is executing a load and store exclusive sequence at
>> +	  the same time as a processor in another cluster is executing a cache
>> +	  maintenance operation to the same address, then this erratum might
>> +	  cause data corruption.
>> +
>> +	  The workaround promotes data cache clean instructions to
>> +	  data cache clean-and-invalidate.
>> +	  Please note that this does not necessarily enable the workaround,
>> +	  as it depends on the alternative framework, which will only patch
>> +	  the kernel if an affected CPU is detected.
>> +
>> +	  If unsure, say Y.
>> +endmenu
>
> I am not sure whether we want to go into this level of configuration.
> You have already introduced ALTERNATIVE, so somebody which might not
> want any workarounds can disable them all. Otherwise do you have use
> cases for allowing people to only enable some and not all?

Alternative is not selectable by the user.

> If you are a distro you need to enable them all. If you are in the
> embedded space and you are working with one specific SoC, you can
> manually disable any workarounds you don't want.

The main purpose of KConfig is to be able to disable manually certain 
options without hacking the code.

I also find the Kconfig very helpful for describing each erratum and the 
impact to hypervisor.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails
  2016-05-21 14:51     ` Stefano Stabellini
@ 2016-05-23 13:45       ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-05-23 13:45 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 21/05/16 15:51, Stefano Stabellini wrote:
> On Sat, 21 May 2016, Stefano Stabellini wrote:
>> On Thu, 5 May 2016, Julien Grall wrote:
>>> Based on ARM ARM (D4.5.3 in ARM DDI 0486A and B3.12.7 in ARM DDI 0406C.c),
>>> a Stage 1 translation error has priority over a Stage 2 translation error.
>>>
>>> Therefore gva_to_ipa can only fail if another vCPU is playing with the
>>> page table.
>>>
>>> Rather than injecting a custom fault, replay the instruction and let the
>>> processor injecting the correct fault.
>>>
>>> Signed-off-by: Julien Grall <julien.grall@arm.com>
>>
>> Couldn't a guest purposely cause a DoS in the hypervisor this way?
>
> Just double-checking. I am pretty sure it cannot, because the replayed
> instruction won't cause another hypervisor trap the second time around.

Before returning to the guest vCPU, Xen is handling any pending softirqs 
(see leave_hypervisor_tail). It might be possible to have the vCPU 
rescheduled.

So even if the replay cause another hypervisor trap, it will only impact 
its timeslice.

I will update the commit message to explain why it is not possible.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-23 11:11     ` Julien Grall
@ 2016-05-30 14:45       ` Stefano Stabellini
  2016-05-30 16:42         ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-30 14:45 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel

On Mon, 23 May 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 21/05/16 16:09, Stefano Stabellini wrote:
> > On Thu, 5 May 2016, Julien Grall wrote:
> > > +void __init apply_alternatives_all(void)
> > > +{
> > > +    int ret;
> > > +
> > > +	/* better not try code patching on a live SMP system */
> > > +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
> > > NR_CPUS);
> > 
> > Why not just call stop_machine_run, passing 0 as the cpu to run
> > __apply_alternatives_multi_stop on? Given that you already have
> > secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> > missing?
> 
> Someone as to call __apply_alternatives_multi_stop on secondary CPUs.

Why? Secondary cpus would be just left spinning at the beginning of the
function. You might as well leave them spinning in
stopmachine_wait_state.

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-30 14:45       ` Stefano Stabellini
@ 2016-05-30 16:42         ` Julien Grall
  2016-05-31  9:21           ` Stefano Stabellini
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-30 16:42 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 30/05/2016 15:45, Stefano Stabellini wrote:
> On Mon, 23 May 2016, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 21/05/16 16:09, Stefano Stabellini wrote:
>>> On Thu, 5 May 2016, Julien Grall wrote:
>>>> +void __init apply_alternatives_all(void)
>>>> +{
>>>> +    int ret;
>>>> +
>>>> +	/* better not try code patching on a live SMP system */
>>>> +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
>>>> NR_CPUS);
>>>
>>> Why not just call stop_machine_run, passing 0 as the cpu to run
>>> __apply_alternatives_multi_stop on? Given that you already have
>>> secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
>>> missing?
>>
>> Someone as to call __apply_alternatives_multi_stop on secondary CPUs.
>
> Why? Secondary cpus would be just left spinning at the beginning of the
> function. You might as well leave them spinning in
> stopmachine_wait_state.
>

Because, we may need to patch the stop_machine state machine 
(spinlock,...). So the safest place whilst the code is patched is 
__apply_alternatives_multi_stop.

Note that there is a comment on top of __apply_alternatives_multi_stop 
to explain the reason.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-30 16:42         ` Julien Grall
@ 2016-05-31  9:21           ` Stefano Stabellini
  2016-05-31 10:24             ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Stefano Stabellini @ 2016-05-31  9:21 UTC (permalink / raw)
  To: Julien Grall
  Cc: Stefano Stabellini, wei.chen, andre.przywara, steve.capper, xen-devel

On Mon, 30 May 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 30/05/2016 15:45, Stefano Stabellini wrote:
> > On Mon, 23 May 2016, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 21/05/16 16:09, Stefano Stabellini wrote:
> > > > On Thu, 5 May 2016, Julien Grall wrote:
> > > > > +void __init apply_alternatives_all(void)
> > > > > +{
> > > > > +    int ret;
> > > > > +
> > > > > +	/* better not try code patching on a live SMP system */
> > > > > +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
> > > > > NR_CPUS);
> > > > 
> > > > Why not just call stop_machine_run, passing 0 as the cpu to run
> > > > __apply_alternatives_multi_stop on? Given that you already have
> > > > secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> > > > missing?
> > > 
> > > Someone as to call __apply_alternatives_multi_stop on secondary CPUs.
> > 
> > Why? Secondary cpus would be just left spinning at the beginning of the
> > function. You might as well leave them spinning in
> > stopmachine_wait_state.
> > 
> 
> Because, we may need to patch the stop_machine state machine (spinlock,...).
> So the safest place whilst the code is patched is
> __apply_alternatives_multi_stop.
> 
> Note that there is a comment on top of __apply_alternatives_multi_stop to
> explain the reason.

Have you tried patching stop_machine? What if you need to patch
__apply_alternatives_multi_stop? :-)

I'll refer to Konrad on whether this is a good approach or not.

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-31  9:21           ` Stefano Stabellini
@ 2016-05-31 10:24             ` Julien Grall
  2016-06-02 14:46               ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-05-31 10:24 UTC (permalink / raw)
  To: Stefano Stabellini; +Cc: andre.przywara, steve.capper, wei.chen, xen-devel

Hi Stefano,

On 31/05/16 10:21, Stefano Stabellini wrote:
> On Mon, 30 May 2016, Julien Grall wrote:
>> Hi Stefano,
>>
>> On 30/05/2016 15:45, Stefano Stabellini wrote:
>>> On Mon, 23 May 2016, Julien Grall wrote:
>>>> Hi Stefano,
>>>>
>>>> On 21/05/16 16:09, Stefano Stabellini wrote:
>>>>> On Thu, 5 May 2016, Julien Grall wrote:
>>>>>> +void __init apply_alternatives_all(void)
>>>>>> +{
>>>>>> +    int ret;
>>>>>> +
>>>>>> +	/* better not try code patching on a live SMP system */
>>>>>> +    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
>>>>>> NR_CPUS);
>>>>>
>>>>> Why not just call stop_machine_run, passing 0 as the cpu to run
>>>>> __apply_alternatives_multi_stop on? Given that you already have
>>>>> secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
>>>>> missing?
>>>>
>>>> Someone as to call __apply_alternatives_multi_stop on secondary CPUs.
>>>
>>> Why? Secondary cpus would be just left spinning at the beginning of the
>>> function. You might as well leave them spinning in
>>> stopmachine_wait_state.
>>>
>>
>> Because, we may need to patch the stop_machine state machine (spinlock,...).
>> So the safest place whilst the code is patched is
>> __apply_alternatives_multi_stop.
>>
>> Note that there is a comment on top of __apply_alternatives_multi_stop to
>> explain the reason.
>
> Have you tried patching stop_machine? What if you need to patch
> __apply_alternatives_multi_stop? :-)

We have to define a safe place where the CPUs could spin. I.e the 
instructions will not be patched.
Whilst stop_machine may not be patched today, it is common code and we 
don't know how this will evolve in the future.

By spinning in __apply_alternatives_multi_stop we protect us against 
modification in the common code and tricky bug (yes it might be 
difficult to hit and debug).

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-05-31 10:24             ` Julien Grall
@ 2016-06-02 14:46               ` Konrad Rzeszutek Wilk
  2016-06-02 15:04                 ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-06-02 14:46 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel

On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
> Hi Stefano,
> 
> On 31/05/16 10:21, Stefano Stabellini wrote:
> >On Mon, 30 May 2016, Julien Grall wrote:
> >>Hi Stefano,
> >>
> >>On 30/05/2016 15:45, Stefano Stabellini wrote:
> >>>On Mon, 23 May 2016, Julien Grall wrote:
> >>>>Hi Stefano,
> >>>>
> >>>>On 21/05/16 16:09, Stefano Stabellini wrote:
> >>>>>On Thu, 5 May 2016, Julien Grall wrote:
> >>>>>>+void __init apply_alternatives_all(void)
> >>>>>>+{
> >>>>>>+    int ret;
> >>>>>>+
> >>>>>>+	/* better not try code patching on a live SMP system */
> >>>>>>+    ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
> >>>>>>NR_CPUS);
> >>>>>
> >>>>>Why not just call stop_machine_run, passing 0 as the cpu to run
> >>>>>__apply_alternatives_multi_stop on? Given that you already have
> >>>>>secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> >>>>>missing?
> >>>>
> >>>>Someone as to call __apply_alternatives_multi_stop on secondary CPUs.
> >>>
> >>>Why? Secondary cpus would be just left spinning at the beginning of the
> >>>function. You might as well leave them spinning in
> >>>stopmachine_wait_state.
> >>>
> >>
> >>Because, we may need to patch the stop_machine state machine (spinlock,...).
> >>So the safest place whilst the code is patched is
> >>__apply_alternatives_multi_stop.
> >>
> >>Note that there is a comment on top of __apply_alternatives_multi_stop to
> >>explain the reason.
> >
> >Have you tried patching stop_machine? What if you need to patch
> >__apply_alternatives_multi_stop? :-)
> 
> We have to define a safe place where the CPUs could spin. I.e the
> instructions will not be patched.
> Whilst stop_machine may not be patched today, it is common code and we don't
> know how this will evolve in the future.

Right, which is why livepatching persued a different path as it may
be patched. And instead choose a simpler mechanism to trigger IPIs and
either patched it from idle_loop() or from the VMX assembler code.


> 
> By spinning in __apply_alternatives_multi_stop we protect us against
> modification in the common code and tricky bug (yes it might be difficult to
> hit and debug).

I feel that you are moving down the stack to try to make the
impact of doing modifications have no impact (or as low as possible).

And it would work now, but I am concerned that in the future it may be
not soo future proof.

Would it perhaps make sense to make some of the livepatching mechanism
be exposed as general code? And use it for alternative asm as well?


> 
> Regards,
> 
> -- 
> Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-06-02 14:46               ` Konrad Rzeszutek Wilk
@ 2016-06-02 15:04                 ` Julien Grall
  2016-06-02 15:14                   ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-06-02 15:04 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel

Hi Konrad,

On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:
> On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
>> On 31/05/16 10:21, Stefano Stabellini wrote:
>>> On Mon, 30 May 2016, Julien Grall wrote:
>> By spinning in __apply_alternatives_multi_stop we protect us against
>> modification in the common code and tricky bug (yes it might be difficult to
>> hit and debug).
>
> I feel that you are moving down the stack to try to make the
> impact of doing modifications have no impact (or as low as possible).
>
> And it would work now, but I am concerned that in the future it may be
> not soo future proof.

Can you detail here?

> Would it perhaps make sense to make some of the livepatching mechanism
> be exposed as general code? And use it for alternative asm as well?

The code to sync the CPU looks very similar to stop_machine, so why 
would we want to get yet another mechanism to sync the CPUs and execute 
a specific function?

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-06-02 15:04                 ` Julien Grall
@ 2016-06-02 15:14                   ` Julien Grall
  2016-06-06 14:17                     ` Konrad Rzeszutek Wilk
  0 siblings, 1 reply; 55+ messages in thread
From: Julien Grall @ 2016-06-02 15:14 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel



On 02/06/16 16:04, Julien Grall wrote:
> Hi Konrad,
>
> On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:
>> On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
>>> On 31/05/16 10:21, Stefano Stabellini wrote:
>>>> On Mon, 30 May 2016, Julien Grall wrote:
>>> By spinning in __apply_alternatives_multi_stop we protect us against
>>> modification in the common code and tricky bug (yes it might be
>>> difficult to
>>> hit and debug).
>>
>> I feel that you are moving down the stack to try to make the
>> impact of doing modifications have no impact (or as low as possible).
>>
>> And it would work now, but I am concerned that in the future it may be
>> not soo future proof.
>
> Can you detail here?
>
>> Would it perhaps make sense to make some of the livepatching mechanism
>> be exposed as general code? And use it for alternative asm as well?
>
> The code to sync the CPU looks very similar to stop_machine, so why
> would we want to get yet another mechanism to sync the CPUs and execute
> a specific function?

I forgot to mention that apply_alternatives_all is only called one time 
during the boot and before CPU0 goes in idle_loop. If we have to patch 
afterward, we would use apply_alternatives which consider that all the 
CPUs have been parked somewhere else.

Regards,

-- 
Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-06-02 15:14                   ` Julien Grall
@ 2016-06-06 14:17                     ` Konrad Rzeszutek Wilk
  2016-06-06 14:18                       ` Julien Grall
  0 siblings, 1 reply; 55+ messages in thread
From: Konrad Rzeszutek Wilk @ 2016-06-06 14:17 UTC (permalink / raw)
  To: Julien Grall
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel

On Thu, Jun 02, 2016 at 04:14:04PM +0100, Julien Grall wrote:
> 
> 
> On 02/06/16 16:04, Julien Grall wrote:
> >Hi Konrad,
> >
> >On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:
> >>On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
> >>>On 31/05/16 10:21, Stefano Stabellini wrote:
> >>>>On Mon, 30 May 2016, Julien Grall wrote:
> >>>By spinning in __apply_alternatives_multi_stop we protect us against
> >>>modification in the common code and tricky bug (yes it might be
> >>>difficult to
> >>>hit and debug).
> >>
> >>I feel that you are moving down the stack to try to make the
> >>impact of doing modifications have no impact (or as low as possible).
> >>
> >>And it would work now, but I am concerned that in the future it may be
> >>not soo future proof.
> >
> >Can you detail here?
> >
> >>Would it perhaps make sense to make some of the livepatching mechanism
> >>be exposed as general code? And use it for alternative asm as well?
> >
> >The code to sync the CPU looks very similar to stop_machine, so why
> >would we want to get yet another mechanism to sync the CPUs and execute
> >a specific function?
> 
> I forgot to mention that apply_alternatives_all is only called one time
> during the boot and before CPU0 goes in idle_loop. If we have to patch
> afterward, we would use apply_alternatives which consider that all the CPUs
> have been parked somewhere else.

Oh, in that case please just mention that in the commit description.

> 
> Regards,
> 
> -- 
> Julien Grall

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

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

* Re: [RFC 10/16] xen/arm: Introduce alternative runtime patching
  2016-06-06 14:17                     ` Konrad Rzeszutek Wilk
@ 2016-06-06 14:18                       ` Julien Grall
  0 siblings, 0 replies; 55+ messages in thread
From: Julien Grall @ 2016-06-06 14:18 UTC (permalink / raw)
  To: Konrad Rzeszutek Wilk
  Cc: andre.przywara, Stefano Stabellini, steve.capper, wei.chen, xen-devel

Hi Konrad,

On 06/06/16 15:17, Konrad Rzeszutek Wilk wrote:
> On Thu, Jun 02, 2016 at 04:14:04PM +0100, Julien Grall wrote:
>>
>>
>> On 02/06/16 16:04, Julien Grall wrote:
>>> Hi Konrad,
>>>
>>> On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:
>>>> On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
>>>>> On 31/05/16 10:21, Stefano Stabellini wrote:
>>>>>> On Mon, 30 May 2016, Julien Grall wrote:
>>>>> By spinning in __apply_alternatives_multi_stop we protect us against
>>>>> modification in the common code and tricky bug (yes it might be
>>>>> difficult to
>>>>> hit and debug).
>>>>
>>>> I feel that you are moving down the stack to try to make the
>>>> impact of doing modifications have no impact (or as low as possible).
>>>>
>>>> And it would work now, but I am concerned that in the future it may be
>>>> not soo future proof.
>>>
>>> Can you detail here?
>>>
>>>> Would it perhaps make sense to make some of the livepatching mechanism
>>>> be exposed as general code? And use it for alternative asm as well?
>>>
>>> The code to sync the CPU looks very similar to stop_machine, so why
>>> would we want to get yet another mechanism to sync the CPUs and execute
>>> a specific function?
>>
>> I forgot to mention that apply_alternatives_all is only called one time
>> during the boot and before CPU0 goes in idle_loop. If we have to patch
>> afterward, we would use apply_alternatives which consider that all the CPUs
>> have been parked somewhere else.
>
> Oh, in that case please just mention that in the commit description.

Will do in the next version.

Cheers,

-- 
Julien Grall

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

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

end of thread, other threads:[~2016-06-06 14:18 UTC | newest]

Thread overview: 55+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-05-05 16:34 [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
2016-05-05 16:34 ` [RFC 01/16] xen/arm: Makefile: Sort the entries alphabetically Julien Grall
2016-05-09  9:34   ` Stefano Stabellini
2016-05-05 16:34 ` [RFC 02/16] xen/arm: Include the header asm-arm/system.h in asm-arm/page.h Julien Grall
2016-05-09  9:34   ` Stefano Stabellini
2016-05-05 16:34 ` [RFC 03/16] xen/arm: Add macros to handle the MIDR Julien Grall
2016-05-09  9:37   ` Stefano Stabellini
2016-05-05 16:34 ` [RFC 04/16] xen/arm: arm64: Import flush_icache_range from Linux v4.6-rc3 Julien Grall
2016-05-09  9:40   ` Julien Grall
2016-05-05 16:34 ` [RFC 05/16] xen/arm: Add cpu_hwcap bitmap Julien Grall
2016-05-09  9:53   ` Stefano Stabellini
2016-05-09 10:02     ` Julien Grall
2016-05-05 16:34 ` [RFC 06/16] xen/arm64: Add an helper to invalidate all instruction caches Julien Grall
2016-05-09  9:50   ` Stefano Stabellini
2016-05-05 16:34 ` [RFC 07/16] xen/arm: arm64: Move the define BRK_BUG_FRAME into a separate header Julien Grall
2016-05-09  9:55   ` Stefano Stabellini
2016-05-05 16:34 ` [RFC 08/16] xen/arm: arm64: Reserve a brk immediate to fault on purpose Julien Grall
2016-05-09  9:58   ` Stefano Stabellini
2016-05-05 16:34 ` [RFC 09/16] xen/arm: arm64: Add helpers to decode and encode branch instructions Julien Grall
2016-05-09 10:05   ` Stefano Stabellini
2016-05-09 13:04     ` Julien Grall
2016-05-23 10:52       ` Julien Grall
2016-05-13 20:35   ` Konrad Rzeszutek Wilk
2016-05-14 10:49     ` Julien Grall
2016-05-05 16:34 ` [RFC 10/16] xen/arm: Introduce alternative runtime patching Julien Grall
2016-05-13 20:26   ` Konrad Rzeszutek Wilk
2016-05-14 18:02     ` Julien Grall
2016-05-21 15:09   ` Stefano Stabellini
2016-05-23 11:11     ` Julien Grall
2016-05-30 14:45       ` Stefano Stabellini
2016-05-30 16:42         ` Julien Grall
2016-05-31  9:21           ` Stefano Stabellini
2016-05-31 10:24             ` Julien Grall
2016-06-02 14:46               ` Konrad Rzeszutek Wilk
2016-06-02 15:04                 ` Julien Grall
2016-06-02 15:14                   ` Julien Grall
2016-06-06 14:17                     ` Konrad Rzeszutek Wilk
2016-06-06 14:18                       ` Julien Grall
2016-05-05 16:34 ` [RFC 11/16] xen/arm: Detect silicon revision and set cap bits accordingly Julien Grall
2016-05-13 20:37   ` Konrad Rzeszutek Wilk
2016-05-14 18:04     ` Julien Grall
2016-05-16 13:50       ` Konrad Rzeszutek Wilk
2016-05-16 13:54         ` Julien Grall
2016-05-05 16:34 ` [RFC 12/16] xen/arm: Document the errata implemented in Xen Julien Grall
2016-05-05 16:34 ` [RFC 13/16] xen/arm: arm64: Add Cortex-A53 cache errata workaround Julien Grall
2016-05-21 14:40   ` Stefano Stabellini
2016-05-23 13:39     ` Julien Grall
2016-05-05 16:34 ` [RFC 14/16] xen/arm: arm64: Add cortex-A57 erratum 832075 workaround Julien Grall
2016-05-05 16:34 ` [RFC 15/16] xen/arm: traps: Don't inject a fault if the translation VA -> IPA fails Julien Grall
2016-05-21 14:42   ` Stefano Stabellini
2016-05-21 14:51     ` Stefano Stabellini
2016-05-23 13:45       ` Julien Grall
2016-05-05 16:34 ` [RFC 16/16] xen/arm: arm64: Document Cortex-A57 erratum 834220 Julien Grall
2016-05-05 16:38 ` [RFC 00/16] xen/arm: Introduce alternative runtime patching for ARM64 Julien Grall
2016-05-11  2:28 ` Wei Chen

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