[PATCH kvm-unit-tests 0/3] arm/arm64: selftest for pabt

[PATCH kvm-unit-tests 0/3] arm/arm64: selftest for pabt

[Andrew Jones]

Patch 3/3 is a rework of Alexandru's pabt test on top of a couple of
framework changes allowing it to be more simply and robustly implemented.

Andrew Jones (3):
  arm/arm64: Improve memory region management
  arm/arm64: selftest: Allow test_exception clobber list to be extended
  arm/arm64: selftest: Add prefetch abort test

 arm/selftest.c      | 199 ++++++++++++++++++++++++++++++++------------
 lib/arm/asm/setup.h |   8 +-
 lib/arm/mmu.c       |  24 ++----
 lib/arm/setup.c     |  56 +++++++++----
 lib/arm64/asm/esr.h |   3 +
 5 files changed, 203 insertions(+), 87 deletions(-)

-- 
2.21.1

[PATCH kvm-unit-tests 1/3] arm/arm64: Improve memory region management

[Andrew Jones]

Add expected I/O regions and provide a way to check memory region
properties of a physical address. We also bump the initial number
of regions and even prepare for a unit test to reallocate for
growth if necessary.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 lib/arm/asm/setup.h |  8 +++++--
 lib/arm/mmu.c       | 24 ++++++-------------
 lib/arm/setup.c     | 56 +++++++++++++++++++++++++++++++++------------
 3 files changed, 55 insertions(+), 33 deletions(-)

diff --git a/lib/arm/asm/setup.h b/lib/arm/asm/setup.h
index 81cac019b1d1..c8afb2493f8d 100644
--- a/lib/arm/asm/setup.h
+++ b/lib/arm/asm/setup.h
@@ -13,16 +13,20 @@
 extern u64 cpus[NR_CPUS];	/* per-cpu IDs (MPIDRs) */
 extern int nr_cpus;
 
-#define NR_MEM_REGIONS		8
 #define MR_F_PRIMARY		(1U << 0)
+#define MR_F_IO			(1U << 1)
+#define MR_F_UNKNOWN		(1U << 31)
+
 struct mem_region {
 	phys_addr_t start;
 	phys_addr_t end;
 	unsigned int flags;
 };
-extern struct mem_region mem_regions[NR_MEM_REGIONS];
+extern struct mem_region *mem_regions;
 extern phys_addr_t __phys_offset, __phys_end;
 
+extern unsigned int mem_region_get_flags(phys_addr_t paddr);
+
 #define PHYS_OFFSET		(__phys_offset)
 #define PHYS_END		(__phys_end)
 
diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index 5fb56180d334..540a1e842d5b 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -152,6 +152,7 @@ void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset,
 void *setup_mmu(phys_addr_t phys_end)
 {
 	uintptr_t code_end = (uintptr_t)&etext;
+	struct mem_region *r;
 
 	/* 0G-1G = I/O, 1G-3G = identity, 3G-4G = vmalloc */
 	if (phys_end > (3ul << 30))
@@ -163,23 +164,12 @@ void *setup_mmu(phys_addr_t phys_end)
 
 	mmu_idmap = alloc_page();
 
-	/*
-	 * mach-virt I/O regions:
-	 *   - The first 1G (arm/arm64)
-	 *   - 512M at 256G (arm64, arm uses highmem=off)
-	 *   - 512G at 512G (arm64, arm uses highmem=off)
-	 */
-	mmu_set_range_sect(mmu_idmap,
-		0, 0, (1ul << 30),
-		__pgprot(PMD_SECT_UNCACHED | PMD_SECT_USER));
-#ifdef __aarch64__
-	mmu_set_range_sect(mmu_idmap,
-		(1ul << 38), (1ul << 38), (1ul << 38) | (1ul << 29),
-		__pgprot(PMD_SECT_UNCACHED | PMD_SECT_USER));
-	mmu_set_range_sect(mmu_idmap,
-		(1ul << 39), (1ul << 39), (1ul << 40),
-		__pgprot(PMD_SECT_UNCACHED | PMD_SECT_USER));
-#endif
+	for (r = mem_regions; r->end; ++r) {
+		if (!(r->flags & MR_F_IO))
+			continue;
+		mmu_set_range_sect(mmu_idmap, r->start, r->start, r->end,
+				   __pgprot(PMD_SECT_UNCACHED | PMD_SECT_USER));
+	}
 
 	/* armv8 requires code shared between EL1 and EL0 to be read-only */
 	mmu_set_range_ptes(mmu_idmap, PHYS_OFFSET,
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 4f02fca85607..385e135f4865 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -24,6 +24,8 @@
 
 #include "io.h"
 
+#define NR_INITIAL_MEM_REGIONS 16
+
 extern unsigned long stacktop;
 
 char *initrd;
@@ -32,7 +34,8 @@ u32 initrd_size;
 u64 cpus[NR_CPUS] = { [0 ... NR_CPUS-1] = (u64)~0 };
 int nr_cpus;
 
-struct mem_region mem_regions[NR_MEM_REGIONS];
+static struct mem_region __initial_mem_regions[NR_INITIAL_MEM_REGIONS + 1];
+struct mem_region *mem_regions = __initial_mem_regions;
 phys_addr_t __phys_offset, __phys_end;
 
 int mpidr_to_cpu(uint64_t mpidr)
@@ -65,41 +68,66 @@ static void cpu_init(void)
 	set_cpu_online(0, true);
 }
 
+unsigned int mem_region_get_flags(phys_addr_t paddr)
+{
+	struct mem_region *r;
+
+	for (r = mem_regions; r->end; ++r) {
+		if (paddr >= r->start && paddr < r->end)
+			return r->flags;
+	}
+
+	return MR_F_UNKNOWN;
+}
+
 static void mem_init(phys_addr_t freemem_start)
 {
-	struct dt_pbus_reg regs[NR_MEM_REGIONS];
+	struct dt_pbus_reg regs[NR_INITIAL_MEM_REGIONS];
 	struct mem_region primary, mem = {
 		.start = (phys_addr_t)-1,
 	};
 	phys_addr_t base, top;
-	int nr_regs, i;
+	int nr_regs, nr_io = 0, i;
 
-	nr_regs = dt_get_memory_params(regs, NR_MEM_REGIONS);
+	/*
+	 * mach-virt I/O regions:
+	 *   - The first 1G (arm/arm64)
+	 *   - 512M at 256G (arm64, arm uses highmem=off)
+	 *   - 512G at 512G (arm64, arm uses highmem=off)
+	 */
+	mem_regions[nr_io++] = (struct mem_region){ 0, (1ul << 30), MR_F_IO };
+#ifdef __aarch64__
+	mem_regions[nr_io++] = (struct mem_region){ (1ul << 38), (1ul << 38) | (1ul << 29), MR_F_IO };
+	mem_regions[nr_io++] = (struct mem_region){ (1ul << 39), (1ul << 40), MR_F_IO };
+#endif
+
+	nr_regs = dt_get_memory_params(regs, NR_INITIAL_MEM_REGIONS - nr_io);
 	assert(nr_regs > 0);
 
 	primary = (struct mem_region){ 0 };
 
 	for (i = 0; i < nr_regs; ++i) {
-		mem_regions[i].start = regs[i].addr;
-		mem_regions[i].end = regs[i].addr + regs[i].size;
+		struct mem_region *r = &mem_regions[nr_io + i];
+
+		r->start = regs[i].addr;
+		r->end = regs[i].addr + regs[i].size;
 
 		/*
 		 * pick the region we're in for our primary region
 		 */
-		if (freemem_start >= mem_regions[i].start
-				&& freemem_start < mem_regions[i].end) {
-			mem_regions[i].flags |= MR_F_PRIMARY;
-			primary = mem_regions[i];
+		if (freemem_start >= r->start && freemem_start < r->end) {
+			r->flags |= MR_F_PRIMARY;
+			primary = *r;
 		}
 
 		/*
 		 * set the lowest and highest addresses found,
 		 * ignoring potential gaps
 		 */
-		if (mem_regions[i].start < mem.start)
-			mem.start = mem_regions[i].start;
-		if (mem_regions[i].end > mem.end)
-			mem.end = mem_regions[i].end;
+		if (r->start < mem.start)
+			mem.start = r->start;
+		if (r->end > mem.end)
+			mem.end = r->end;
 	}
 	assert(primary.end != 0);
 	assert(!(mem.start & ~PHYS_MASK) && !((mem.end - 1) & ~PHYS_MASK));
-- 
2.21.1

[PATCH kvm-unit-tests 2/3] arm/arm64: selftest: Allow test_exception clobber list to be extended

[Andrew Jones]

test_exception() callers may need to extend the clobber list. Also
there's no reason for the callers to need to assume that R0 and R1
are already in the list.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/selftest.c | 105 ++++++++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 54 deletions(-)

diff --git a/arm/selftest.c b/arm/selftest.c
index 89759cf9f592..6d74fa1fa4c4 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -74,24 +74,22 @@ static bool svc_works;
  * that causes an exception. The test handler will check that its
  * capture of the current register state matches the capture done
  * here.
- *
- * NOTE: update clobber list if passed insns needs more than r0,r1
  */
-#define test_exception(pre_insns, excptn_insn, post_insns)	\
-	asm volatile(						\
-		pre_insns "\n"					\
-		"mov	r0, %0\n"				\
-		"stmia	r0, { r0-lr }\n"			\
-		"mrs	r1, cpsr\n"				\
-		"str	r1, [r0, #" xstr(S_PSR) "]\n"		\
-		"mov	r1, #-1\n"				\
-		"str	r1, [r0, #" xstr(S_OLD_R0) "]\n"	\
-		"add	r1, pc, #8\n"				\
-		"str	r1, [r0, #" xstr(S_R1) "]\n"		\
-		"str	r1, [r0, #" xstr(S_PC) "]\n"		\
-		excptn_insn "\n"				\
-		post_insns "\n"					\
-	:: "r" (&expected_regs) : "r0", "r1")
+#define test_exception(pre_insns, excptn_insn, post_insns, clobbers...)	\
+	asm volatile(							\
+		pre_insns "\n"						\
+		"mov	r0, %0\n"					\
+		"stmia	r0, { r0-lr }\n"				\
+		"mrs	r1, cpsr\n"					\
+		"str	r1, [r0, #" xstr(S_PSR) "]\n"			\
+		"mov	r1, #-1\n"					\
+		"str	r1, [r0, #" xstr(S_OLD_R0) "]\n"		\
+		"add	r1, pc, #8\n"					\
+		"str	r1, [r0, #" xstr(S_R1) "]\n"			\
+		"str	r1, [r0, #" xstr(S_PC) "]\n"			\
+		excptn_insn "\n"					\
+		post_insns "\n"						\
+	:: "r" (&expected_regs) : "r0", "r1", ##clobbers)
 
 static bool check_regs(struct pt_regs *regs)
 {
@@ -119,7 +117,7 @@ static bool check_und(void)
 	install_exception_handler(EXCPTN_UND, und_handler);
 
 	/* issue an instruction to a coprocessor we don't have */
-	test_exception("", "mcr p2, 0, r0, c0, c0", "");
+	test_exception("", "mcr p2, 0, r0, c0, c0", "", "r0");
 
 	install_exception_handler(EXCPTN_UND, NULL);
 
@@ -156,7 +154,8 @@ static bool check_svc(void)
 			"push	{ r0,lr }\n",
 			"svc	#123\n",
 			"pop	{ r0,lr }\n"
-			"msr	spsr_cxsf, r0\n"
+			"msr	spsr_cxsf, r0\n",
+			"r0", "lr"
 		);
 	} else {
 		test_exception("", "svc #123", "");
@@ -178,41 +177,39 @@ static void user_psci_system_off(struct pt_regs *regs)
  * that causes an exception. The test handler will check that its
  * capture of the current register state matches the capture done
  * here.
- *
- * NOTE: update clobber list if passed insns needs more than x0,x1
  */
-#define test_exception(pre_insns, excptn_insn, post_insns)	\
-	asm volatile(						\
-		pre_insns "\n"					\
-		"mov	x1, %0\n"				\
-		"ldr	x0, [x1, #" xstr(S_PSTATE) "]\n"	\
-		"mrs	x1, nzcv\n"				\
-		"orr	w0, w0, w1\n"				\
-		"mov	x1, %0\n"				\
-		"str	w0, [x1, #" xstr(S_PSTATE) "]\n"	\
-		"mov	x0, sp\n"				\
-		"str	x0, [x1, #" xstr(S_SP) "]\n"		\
-		"adr	x0, 1f\n"				\
-		"str	x0, [x1, #" xstr(S_PC) "]\n"		\
-		"stp	 x2,  x3, [x1,  #16]\n"			\
-		"stp	 x4,  x5, [x1,  #32]\n"			\
-		"stp	 x6,  x7, [x1,  #48]\n"			\
-		"stp	 x8,  x9, [x1,  #64]\n"			\
-		"stp	x10, x11, [x1,  #80]\n"			\
-		"stp	x12, x13, [x1,  #96]\n"			\
-		"stp	x14, x15, [x1, #112]\n"			\
-		"stp	x16, x17, [x1, #128]\n"			\
-		"stp	x18, x19, [x1, #144]\n"			\
-		"stp	x20, x21, [x1, #160]\n"			\
-		"stp	x22, x23, [x1, #176]\n"			\
-		"stp	x24, x25, [x1, #192]\n"			\
-		"stp	x26, x27, [x1, #208]\n"			\
-		"stp	x28, x29, [x1, #224]\n"			\
-		"str	x30, [x1, #" xstr(S_LR) "]\n"		\
-		"stp	 x0,  x1, [x1]\n"			\
-	"1:"	excptn_insn "\n"				\
-		post_insns "\n"					\
-	:: "r" (&expected_regs) : "x0", "x1")
+#define test_exception(pre_insns, excptn_insn, post_insns, clobbers...)	\
+	asm volatile(							\
+		pre_insns "\n"						\
+		"mov	x1, %0\n"					\
+		"ldr	x0, [x1, #" xstr(S_PSTATE) "]\n"		\
+		"mrs	x1, nzcv\n"					\
+		"orr	w0, w0, w1\n"					\
+		"mov	x1, %0\n"					\
+		"str	w0, [x1, #" xstr(S_PSTATE) "]\n"		\
+		"mov	x0, sp\n"					\
+		"str	x0, [x1, #" xstr(S_SP) "]\n"			\
+		"adr	x0, 1f\n"					\
+		"str	x0, [x1, #" xstr(S_PC) "]\n"			\
+		"stp	 x2,  x3, [x1,  #16]\n"				\
+		"stp	 x4,  x5, [x1,  #32]\n"				\
+		"stp	 x6,  x7, [x1,  #48]\n"				\
+		"stp	 x8,  x9, [x1,  #64]\n"				\
+		"stp	x10, x11, [x1,  #80]\n"				\
+		"stp	x12, x13, [x1,  #96]\n"				\
+		"stp	x14, x15, [x1, #112]\n"				\
+		"stp	x16, x17, [x1, #128]\n"				\
+		"stp	x18, x19, [x1, #144]\n"				\
+		"stp	x20, x21, [x1, #160]\n"				\
+		"stp	x22, x23, [x1, #176]\n"				\
+		"stp	x24, x25, [x1, #192]\n"				\
+		"stp	x26, x27, [x1, #208]\n"				\
+		"stp	x28, x29, [x1, #224]\n"				\
+		"str	x30, [x1, #" xstr(S_LR) "]\n"			\
+		"stp	 x0,  x1, [x1]\n"				\
+	"1:"	excptn_insn "\n"					\
+		post_insns "\n"						\
+	:: "r" (&expected_regs) : "x0", "x1", ##clobbers)
 
 static bool check_regs(struct pt_regs *regs)
 {
@@ -260,7 +257,7 @@ static bool check_und(void)
 	install_exception_handler(v, ESR_EL1_EC_UNKNOWN, unknown_handler);
 
 	/* try to read an el2 sysreg from el0/1 */
-	test_exception("", "mrs x0, sctlr_el2", "");
+	test_exception("", "mrs x0, sctlr_el2", "", "x0");
 
 	install_exception_handler(v, ESR_EL1_EC_UNKNOWN, NULL);
 
-- 
2.21.1

[PATCH kvm-unit-tests 3/3] arm/arm64: selftest: Add prefetch abort test

[Andrew Jones]

When a guest tries to execute code from an invalid physical address
KVM should inject an external abort. This test is based on a test
originally posted by Alexandru Elisei. This version avoids hard
coding the invalid physical address used.

Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/selftest.c      | 94 +++++++++++++++++++++++++++++++++++++++++++++
 lib/arm64/asm/esr.h |  3 ++
 2 files changed, 97 insertions(+)

diff --git a/arm/selftest.c b/arm/selftest.c
index 6d74fa1fa4c4..4495b161cdd5 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -8,6 +8,7 @@
 #include <libcflat.h>
 #include <util.h>
 #include <devicetree.h>
+#include <vmalloc.h>
 #include <asm/setup.h>
 #include <asm/ptrace.h>
 #include <asm/asm-offsets.h>
@@ -15,6 +16,7 @@
 #include <asm/thread_info.h>
 #include <asm/psci.h>
 #include <asm/smp.h>
+#include <asm/mmu.h>
 #include <asm/barrier.h>
 
 static cpumask_t ready, valid;
@@ -65,9 +67,43 @@ static void check_setup(int argc, char **argv)
 		report_abort("missing input");
 }
 
+unsigned long check_pabt_invalid_paddr;
+static bool check_pabt_init(void)
+{
+	phys_addr_t highest_end = 0;
+	unsigned long vaddr;
+	struct mem_region *r;
+
+	/*
+	 * We need a physical address that isn't backed by anything. Without
+	 * fully parsing the device tree there's no way to be certain of any
+	 * address, but an unknown address immediately following the highest
+	 * memory region has a reasonable chance. This is because we can
+	 * assume that that memory region could have been larger, if the user
+	 * had configured more RAM, and therefore no MMIO region should be
+	 * there.
+	 */
+	for (r = mem_regions; r->end; ++r) {
+		if (r->flags & MR_F_IO)
+			continue;
+		if (r->end > highest_end)
+			highest_end = PAGE_ALIGN(r->end);
+	}
+
+	if (mem_region_get_flags(highest_end) != MR_F_UNKNOWN)
+		return false;
+
+	vaddr = (unsigned long)vmap(highest_end, PAGE_SIZE);
+	mmu_clear_user(current_thread_info()->pgtable, vaddr);
+	check_pabt_invalid_paddr = vaddr;
+
+	return true;
+}
+
 static struct pt_regs expected_regs;
 static bool und_works;
 static bool svc_works;
+static bool pabt_works;
 #if defined(__arm__)
 /*
  * Capture the current register state and execute an instruction
@@ -166,6 +202,30 @@ static bool check_svc(void)
 	return svc_works;
 }
 
+static void pabt_handler(struct pt_regs *regs)
+{
+	expected_regs.ARM_lr = expected_regs.ARM_pc;
+	expected_regs.ARM_pc = expected_regs.ARM_r9;
+
+	pabt_works = check_regs(regs);
+
+	regs->ARM_pc = regs->ARM_lr;
+}
+
+static bool check_pabt(void)
+{
+	install_exception_handler(EXCPTN_PABT, pabt_handler);
+
+	test_exception("ldr	r9, =check_pabt_invalid_paddr\n"
+		       "ldr	r9, [r9]\n",
+		       "blx	r9\n",
+		       "", "r9", "lr");
+
+	install_exception_handler(EXCPTN_PABT, NULL);
+
+	return pabt_works;
+}
+
 static void user_psci_system_off(struct pt_regs *regs)
 {
 	__user_psci_system_off();
@@ -285,6 +345,35 @@ static bool check_svc(void)
 	return svc_works;
 }
 
+static void pabt_handler(struct pt_regs *regs, unsigned int esr)
+{
+	bool is_extabt = (esr & ESR_EL1_FSC_MASK) == ESR_EL1_FSC_EXTABT;
+
+	expected_regs.regs[30] = expected_regs.pc + 4;
+	expected_regs.pc = expected_regs.regs[9];
+
+	pabt_works = check_regs(regs) && is_extabt;
+
+	regs->pc = regs->regs[30];
+}
+
+static bool check_pabt(void)
+{
+	enum vector v = check_vector_prep();
+
+	install_exception_handler(v, ESR_EL1_EC_IABT_EL1, pabt_handler);
+
+	test_exception("adrp	x9, check_pabt_invalid_paddr\n"
+		       "add	x9, x9, :lo12:check_pabt_invalid_paddr\n"
+		       "ldr	x9, [x9]\n",
+		       "blr	x9\n",
+		       "", "x9", "x30");
+
+	install_exception_handler(v, ESR_EL1_EC_IABT_EL1, NULL);
+
+	return pabt_works;
+}
+
 static void user_psci_system_off(struct pt_regs *regs, unsigned int esr)
 {
 	__user_psci_system_off();
@@ -302,6 +391,11 @@ static void check_vectors(void *arg __unused)
 		install_exception_handler(EL0_SYNC_64, ESR_EL1_EC_UNKNOWN,
 					  user_psci_system_off);
 #endif
+	} else {
+		if (!check_pabt_init())
+			report_skip("Couldn't guess an invalid physical address");
+		else
+			report(check_pabt(), "pabt");
 	}
 	exit(report_summary());
 }
diff --git a/lib/arm64/asm/esr.h b/lib/arm64/asm/esr.h
index 8e5af4d90767..8c351631b0a0 100644
--- a/lib/arm64/asm/esr.h
+++ b/lib/arm64/asm/esr.h
@@ -44,4 +44,7 @@
 #define ESR_EL1_EC_BKPT32	(0x38)
 #define ESR_EL1_EC_BRK64	(0x3C)
 
+#define ESR_EL1_FSC_MASK	(0x3F)
+#define ESR_EL1_FSC_EXTABT	(0x10)
+
 #endif /* _ASMARM64_ESR_H_ */
-- 
2.21.1

Re: [PATCH kvm-unit-tests 0/3] arm/arm64: selftest for pabt

[Paolo Bonzini]

On 13/01/20 14:00, Andrew Jones wrote:
> Patch 3/3 is a rework of Alexandru's pabt test on top of a couple of
> framework changes allowing it to be more simply and robustly implemented.
> 
> Andrew Jones (3):
>   arm/arm64: Improve memory region management
>   arm/arm64: selftest: Allow test_exception clobber list to be extended
>   arm/arm64: selftest: Add prefetch abort test
> 
>  arm/selftest.c      | 199 ++++++++++++++++++++++++++++++++------------
>  lib/arm/asm/setup.h |   8 +-
>  lib/arm/mmu.c       |  24 ++----
>  lib/arm/setup.c     |  56 +++++++++----
>  lib/arm64/asm/esr.h |   3 +
>  5 files changed, 203 insertions(+), 87 deletions(-)
> 

Looks good, are you going to send a pull request for this?

Paolo

Re: [PATCH kvm-unit-tests 0/3] arm/arm64: selftest for pabt

[Andrew Jones]

On Tue, Jan 21, 2020 at 01:24:31PM +0100, Paolo Bonzini wrote:
> On 13/01/20 14:00, Andrew Jones wrote:
> > Patch 3/3 is a rework of Alexandru's pabt test on top of a couple of
> > framework changes allowing it to be more simply and robustly implemented.
> > 
> > Andrew Jones (3):
> >   arm/arm64: Improve memory region management
> >   arm/arm64: selftest: Allow test_exception clobber list to be extended
> >   arm/arm64: selftest: Add prefetch abort test
> > 
> >  arm/selftest.c      | 199 ++++++++++++++++++++++++++++++++------------
> >  lib/arm/asm/setup.h |   8 +-
> >  lib/arm/mmu.c       |  24 ++----
> >  lib/arm/setup.c     |  56 +++++++++----
> >  lib/arm64/asm/esr.h |   3 +
> >  5 files changed, 203 insertions(+), 87 deletions(-)
> > 
> 
> Looks good, are you going to send a pull request for this?
>

Sure. I'll send in a few minutes.

Thanks,
drew 

[PATCH kvm-unit-tests 2/3] arm/arm64: selftest: Allow test_exception clobber list to be extended

[Andrew Jones]

test_exception() callers may need to extend the clobber list. Also
there's no reason for the callers to need to assume that R0 and R1
are already in the list.

Signed-off-by: Andrew Jones <drjones@redhat.com>
---
 arm/selftest.c | 105 ++++++++++++++++++++++++-------------------------
 1 file changed, 51 insertions(+), 54 deletions(-)

diff --git a/arm/selftest.c b/arm/selftest.c
index 89759cf9f592..6d74fa1fa4c4 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -74,24 +74,22 @@ static bool svc_works;
  * that causes an exception. The test handler will check that its
  * capture of the current register state matches the capture done
  * here.
- *
- * NOTE: update clobber list if passed insns needs more than r0,r1
  */
-#define test_exception(pre_insns, excptn_insn, post_insns)	\
-	asm volatile(						\
-		pre_insns "\n"					\
-		"mov	r0, %0\n"				\
-		"stmia	r0, { r0-lr }\n"			\
-		"mrs	r1, cpsr\n"				\
-		"str	r1, [r0, #" xstr(S_PSR) "]\n"		\
-		"mov	r1, #-1\n"				\
-		"str	r1, [r0, #" xstr(S_OLD_R0) "]\n"	\
-		"add	r1, pc, #8\n"				\
-		"str	r1, [r0, #" xstr(S_R1) "]\n"		\
-		"str	r1, [r0, #" xstr(S_PC) "]\n"		\
-		excptn_insn "\n"				\
-		post_insns "\n"					\
-	:: "r" (&expected_regs) : "r0", "r1")
+#define test_exception(pre_insns, excptn_insn, post_insns, clobbers...)	\
+	asm volatile(							\
+		pre_insns "\n"						\
+		"mov	r0, %0\n"					\
+		"stmia	r0, { r0-lr }\n"				\
+		"mrs	r1, cpsr\n"					\
+		"str	r1, [r0, #" xstr(S_PSR) "]\n"			\
+		"mov	r1, #-1\n"					\
+		"str	r1, [r0, #" xstr(S_OLD_R0) "]\n"		\
+		"add	r1, pc, #8\n"					\
+		"str	r1, [r0, #" xstr(S_R1) "]\n"			\
+		"str	r1, [r0, #" xstr(S_PC) "]\n"			\
+		excptn_insn "\n"					\
+		post_insns "\n"						\
+	:: "r" (&expected_regs) : "r0", "r1", ##clobbers)
 
 static bool check_regs(struct pt_regs *regs)
 {
@@ -119,7 +117,7 @@ static bool check_und(void)
 	install_exception_handler(EXCPTN_UND, und_handler);
 
 	/* issue an instruction to a coprocessor we don't have */
-	test_exception("", "mcr p2, 0, r0, c0, c0", "");
+	test_exception("", "mcr p2, 0, r0, c0, c0", "", "r0");
 
 	install_exception_handler(EXCPTN_UND, NULL);
 
@@ -156,7 +154,8 @@ static bool check_svc(void)
 			"push	{ r0,lr }\n",
 			"svc	#123\n",
 			"pop	{ r0,lr }\n"
-			"msr	spsr_cxsf, r0\n"
+			"msr	spsr_cxsf, r0\n",
+			"r0", "lr"
 		);
 	} else {
 		test_exception("", "svc #123", "");
@@ -178,41 +177,39 @@ static void user_psci_system_off(struct pt_regs *regs)
  * that causes an exception. The test handler will check that its
  * capture of the current register state matches the capture done
  * here.
- *
- * NOTE: update clobber list if passed insns needs more than x0,x1
  */
-#define test_exception(pre_insns, excptn_insn, post_insns)	\
-	asm volatile(						\
-		pre_insns "\n"					\
-		"mov	x1, %0\n"				\
-		"ldr	x0, [x1, #" xstr(S_PSTATE) "]\n"	\
-		"mrs	x1, nzcv\n"				\
-		"orr	w0, w0, w1\n"				\
-		"mov	x1, %0\n"				\
-		"str	w0, [x1, #" xstr(S_PSTATE) "]\n"	\
-		"mov	x0, sp\n"				\
-		"str	x0, [x1, #" xstr(S_SP) "]\n"		\
-		"adr	x0, 1f\n"				\
-		"str	x0, [x1, #" xstr(S_PC) "]\n"		\
-		"stp	 x2,  x3, [x1,  #16]\n"			\
-		"stp	 x4,  x5, [x1,  #32]\n"			\
-		"stp	 x6,  x7, [x1,  #48]\n"			\
-		"stp	 x8,  x9, [x1,  #64]\n"			\
-		"stp	x10, x11, [x1,  #80]\n"			\
-		"stp	x12, x13, [x1,  #96]\n"			\
-		"stp	x14, x15, [x1, #112]\n"			\
-		"stp	x16, x17, [x1, #128]\n"			\
-		"stp	x18, x19, [x1, #144]\n"			\
-		"stp	x20, x21, [x1, #160]\n"			\
-		"stp	x22, x23, [x1, #176]\n"			\
-		"stp	x24, x25, [x1, #192]\n"			\
-		"stp	x26, x27, [x1, #208]\n"			\
-		"stp	x28, x29, [x1, #224]\n"			\
-		"str	x30, [x1, #" xstr(S_LR) "]\n"		\
-		"stp	 x0,  x1, [x1]\n"			\
-	"1:"	excptn_insn "\n"				\
-		post_insns "\n"					\
-	:: "r" (&expected_regs) : "x0", "x1")
+#define test_exception(pre_insns, excptn_insn, post_insns, clobbers...)	\
+	asm volatile(							\
+		pre_insns "\n"						\
+		"mov	x1, %0\n"					\
+		"ldr	x0, [x1, #" xstr(S_PSTATE) "]\n"		\
+		"mrs	x1, nzcv\n"					\
+		"orr	w0, w0, w1\n"					\
+		"mov	x1, %0\n"					\
+		"str	w0, [x1, #" xstr(S_PSTATE) "]\n"		\
+		"mov	x0, sp\n"					\
+		"str	x0, [x1, #" xstr(S_SP) "]\n"			\
+		"adr	x0, 1f\n"					\
+		"str	x0, [x1, #" xstr(S_PC) "]\n"			\
+		"stp	 x2,  x3, [x1,  #16]\n"				\
+		"stp	 x4,  x5, [x1,  #32]\n"				\
+		"stp	 x6,  x7, [x1,  #48]\n"				\
+		"stp	 x8,  x9, [x1,  #64]\n"				\
+		"stp	x10, x11, [x1,  #80]\n"				\
+		"stp	x12, x13, [x1,  #96]\n"				\
+		"stp	x14, x15, [x1, #112]\n"				\
+		"stp	x16, x17, [x1, #128]\n"				\
+		"stp	x18, x19, [x1, #144]\n"				\
+		"stp	x20, x21, [x1, #160]\n"				\
+		"stp	x22, x23, [x1, #176]\n"				\
+		"stp	x24, x25, [x1, #192]\n"				\
+		"stp	x26, x27, [x1, #208]\n"				\
+		"stp	x28, x29, [x1, #224]\n"				\
+		"str	x30, [x1, #" xstr(S_LR) "]\n"			\
+		"stp	 x0,  x1, [x1]\n"				\
+	"1:"	excptn_insn "\n"					\
+		post_insns "\n"						\
+	:: "r" (&expected_regs) : "x0", "x1", ##clobbers)
 
 static bool check_regs(struct pt_regs *regs)
 {
@@ -260,7 +257,7 @@ static bool check_und(void)
 	install_exception_handler(v, ESR_EL1_EC_UNKNOWN, unknown_handler);
 
 	/* try to read an el2 sysreg from el0/1 */
-	test_exception("", "mrs x0, sctlr_el2", "");
+	test_exception("", "mrs x0, sctlr_el2", "", "x0");
 
 	install_exception_handler(v, ESR_EL1_EC_UNKNOWN, NULL);
 
-- 
2.21.1