Skip to contentSkip to conversation

arm/arm64 updates

GIT PULL Andrew Jones 2 participants 12 messages Oct 24, 2019 KVM

lore
Share & tools
lore
#0 lore raw

The following changes since commit ac033c9a2cb287c1bb5ebe414c63067563a05bbb:

Revert "lib: use an argument which doesn't require default argument promotion" (2019-10-23 11:03:31 +0200)

are available in the Git repository at:

https://github.com/rhdrjones/kvm-unit-tests pull-arm-oct-24-2019

for you to fetch changes up to 00d7e26501263263877465d2a74a330897de1e70:

arm: Add PL031 test (2019-10-24 14:41:01 +0200)

----------------------------------------------------------------
Alexander Graf (1):
arm: Add PL031 test

Alexandru Elisei (3):
lib: arm64: Add missing ISB in flush_tlb_page
lib: arm/arm64: Add function to clear the PTE_USER bit
arm64: Add cache code generation test

Andre Przywara (6):
arm: gic: check_acked: add test description
arm: gic: Split variable output data from test name
arm: timer: Split variable output data from test name
arm: selftest: Split variable output data from test name
arm: selftest: Make MPIDR output stable
arm: Add missing test name prefix calls

 arm/Makefile.arm64    |   1 +
 arm/Makefile.common   |   1 +
 arm/cache.c           | 122 ++++++++++++++++++++
 arm/gic.c             |  64 ++++++-----
 arm/pci-test.c        |   2 +
 arm/pl031.c           | 262 ++++++++++++++++++++++++++++++++++++++++++
 arm/psci.c            |   2 +
 arm/selftest.c        |  23 +++-
 arm/timer.c           |   3 +-
 arm/unittests.cfg     |   6 +
 lib/arm/asm/gic.h     |   1 +
 lib/arm/asm/mmu-api.h |   1 +
 lib/arm/mmu.c         |  15 +++
 lib/arm64/asm/mmu.h   |   1 +
 14 files changed, 472 insertions(+), 32 deletions(-)
 create mode 100644 arm/cache.c
 create mode 100644 arm/pl031.c
Signature · 1 line
2.21.0
#1 lore raw

[PULL 01/10] arm: gic: check_acked: add test description

From: Andre Przywara <[email protected]>

At the moment the check_acked() IRQ helper function just prints a generic "Completed" or "Timed out" message, without given a more detailed test description.

To be able to tell the different IRQ tests apart, and also to allow re-using it more easily, add a "description" parameter string, which is prefixing the output line. This gives more information on what exactly was tested.

This also splits the variable output part of the line (duration of IRQ delivery) into a separate INFO: line, to not confuse testing frameworks.

---
 arm/gic.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)
arm/gic.c +10 -7
index ed5642e74f70..2ec4070fbaf9 100644
@@ -60,7 +60,7 @@ static void stats_reset(void)
 	smp_wmb();
 }
 
-static void check_acked(cpumask_t *mask)
+static void check_acked(const char *testname, cpumask_t *mask)
 {
 	int missing = 0, extra = 0, unexpected = 0;
 	int nr_pass, cpu, i;
@@ -88,7 +88,9 @@ static void check_acked(cpumask_t *mask)
 			}
 		}
 		if (nr_pass == nr_cpus) {
-			report("Completed in %d ms", !bad, ++i * 100);
+			report("%s", !bad, testname);
+			if (i)
+				report_info("took more than %d ms", i * 100);
 			return;
 		}
 	}
@@ -105,8 +107,9 @@ static void check_acked(cpumask_t *mask)
 		}
 	}
 
-	report("Timed-out (5s). ACKS: missing=%d extra=%d unexpected=%d",
-	       false, missing, extra, unexpected);
+	report("%s", false, testname);
+	report_info("Timed-out (5s). ACKS: missing=%d extra=%d unexpected=%d",
+		    missing, extra, unexpected);
 }
 
 static void check_spurious(void)
@@ -185,7 +188,7 @@ static void ipi_test_self(void)
 	cpumask_clear(&mask);
 	cpumask_set_cpu(smp_processor_id(), &mask);
 	gic->ipi.send_self();
-	check_acked(&mask);
+	check_acked("IPI: self", &mask);
 	report_prefix_pop();
 }
 
@@ -200,7 +203,7 @@ static void ipi_test_smp(void)
 	for (i = smp_processor_id() & 1; i < nr_cpus; i += 2)
 		cpumask_clear_cpu(i, &mask);
 	gic_ipi_send_mask(IPI_IRQ, &mask);
-	check_acked(&mask);
+	check_acked("IPI: directed", &mask);
 	report_prefix_pop();
 
 	report_prefix_push("broadcast");
@@ -208,7 +211,7 @@ static void ipi_test_smp(void)
 	cpumask_copy(&mask, &cpu_present_mask);
 	cpumask_clear_cpu(smp_processor_id(), &mask);
 	gic->ipi.send_broadcast();
-	check_acked(&mask);
+	check_acked("IPI: broadcast", &mask);
 	report_prefix_pop();
 }
 
Signature · 1 line
2.21.0
#2 lore raw

[PULL 03/10] arm: timer: Split variable output data from test name

In reply to Andrew Jones

From: Andre Przywara <[email protected]>

For some tests we mix variable diagnostic output with the test name, which leads to variable test line, confusing some higher level frameworks.

Split the output to always use the same test name for a certain test, and put diagnostic output on a separate line using the INFO: tag.

---
 arm/timer.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
arm/timer.c +2 -1
index f2f60192ba62..0b808d5da9da 100644
@@ -249,7 +249,8 @@ static void test_timer(struct timer_info *info)
 	local_irq_enable();
 	left = info->read_tval();
 	report("interrupt received after TVAL/WFI", info->irq_received);
-	report("timer has expired (%d)", left < 0, left);
+	report("timer has expired", left < 0);
+	report_info("TVAL is %d ticks", left);
 }
 
 static void test_vtimer(void)
Signature · 1 line
2.21.0
#3 lore raw

[PULL 06/10] arm: Add missing test name prefix calls

In reply to Andrew Jones

From: Andre Przywara <[email protected]>

When running the unit tests in TAP mode (./run_tests.sh -t), every single test result is printed. This works fine for most tests which use the reporting prefix feature to indicate the actual test name. However psci and pci were missing those names, so the reporting left people scratching their head what was actually tested:
...
ok 74 - invalid-function
ok 75 - affinity-info-on
ok 76 - affinity-info-off
ok 77 - cpu-on

Push a "psci" prefix before running those tests to make those report lines more descriptive.
While at it, do the same for pci, even though it is less ambigious there. Also the GIC ITARGETSR test was missing a report_prefix_pop().

---
 arm/gic.c      | 2 ++
 arm/pci-test.c | 2 ++
 arm/psci.c     | 2 ++
 3 files changed, 6 insertions(+)
arm/gic.c +2 -0
index 02d292807c9b..adb6aa464513 100644
@@ -480,6 +480,8 @@ static void test_targets(int nr_irqs)
 	test_byte_access(targetsptr + GIC_FIRST_SPI, pattern, cpu_mask);
 
 	writel(orig_targets, targetsptr + GIC_FIRST_SPI);
+
+	report_prefix_pop();
 }
 
 static void gic_test_mmio(void)
arm/pci-test.c +2 -0
index cf128ac1b032..7c3836e5cd63 100644
@@ -19,6 +19,8 @@ int main(void)
 		return report_summary();
 	}
 
+	report_prefix_push("pci");
+
 	pci_print();
 
 	ret = pci_testdev();
arm/psci.c +2 -0
index 5cb4d5c7c233..536c9b742033 100644
@@ -126,6 +126,8 @@ int main(void)
 {
 	int ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
 
+	report_prefix_push("psci");
+
 	if (nr_cpus < 2) {
 		report_skip("At least 2 cpus required");
 		goto done;
Signature · 1 line
2.21.0
#4 lore raw

[PULL 05/10] arm: selftest: Make MPIDR output stable

In reply to Andrew Jones

From: Andre Przywara <[email protected]>

At the moment the smp selftest outputs one line for each vCPU, with the CPU number and its MPIDR printed in the same test result line. For automated test frameworks this has the problem of including variable output in the test name, also the number of tests varies, depending on the number of vCPUs.

Fix this by only generating a single line of output for the SMP test, which summarises the result. We use two cpumasks, to let each vCPU report its result and completion of the test (code stolen from the GIC test).

For informational purposes we keep the one line per CPU, but prefix it with an INFO: tag, so that frameworks can ignore it.

---
 arm/selftest.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)
arm/selftest.c +12 -2
index a0c1ab8180bc..e9dc5c0cab28 100644
@@ -17,6 +17,8 @@
 #include <asm/smp.h>
 #include <asm/barrier.h>
 
+static cpumask_t ready, valid;
+
 static void __user_psci_system_off(void)
 {
 	psci_system_off();
@@ -341,8 +343,11 @@ static void cpu_report(void *data __unused)
 	uint64_t mpidr = get_mpidr();
 	int cpu = smp_processor_id();
 
-	report("CPU(%3d) mpidr=%010" PRIx64,
-		mpidr_to_cpu(mpidr) == cpu, cpu, mpidr);
+	if (mpidr_to_cpu(mpidr) == cpu)
+		cpumask_set_cpu(smp_processor_id(), &valid);
+	smp_wmb();		/* Paired with rmb in main(). */
+	cpumask_set_cpu(smp_processor_id(), &ready);
+	report_info("CPU%3d: MPIDR=%010" PRIx64, cpu, mpidr);
 }
 
 int main(int argc, char **argv)
@@ -371,6 +376,11 @@ int main(int argc, char **argv)
 
 		report("PSCI version", psci_check());
 		on_cpus(cpu_report, NULL);
+		while (!cpumask_full(&ready))
+			cpu_relax();
+		smp_rmb();		/* Paired with wmb in cpu_report(). */
+		report("MPIDR test on all CPUs", cpumask_full(&valid));
+		report_info("%d CPUs reported back", nr_cpus);
 
 	} else {
 		printf("Unknown subtest\n");
Signature · 1 line
2.21.0
#5 lore raw

[PULL 02/10] arm: gic: Split variable output data from test name

In reply to Andrew Jones

From: Andre Przywara <[email protected]>

For some tests we mix variable diagnostic output with the test name, which leads to variable test line, confusing some higher level frameworks.

Split the output to always use the same test name for a certain test, and put diagnostic output on a separate line using the INFO: tag.

---
 arm/gic.c | 45 ++++++++++++++++++++++++++-------------------
 1 file changed, 26 insertions(+), 19 deletions(-)
arm/gic.c +26 -19
index 2ec4070fbaf9..02d292807c9b 100644
@@ -353,8 +353,8 @@ static void test_typer_v2(uint32_t reg)
 {
 	int nr_gic_cpus = ((reg >> 5) & 0x7) + 1;
 
-	report("all %d CPUs have interrupts", nr_cpus == nr_gic_cpus,
-	       nr_gic_cpus);
+	report_info("nr_cpus=%d", nr_cpus);
+	report("all CPUs have interrupts", nr_cpus == nr_gic_cpus);
 }
 
 #define BYTE(reg32, byte) (((reg32) >> ((byte) * 8)) & 0xff)
@@ -370,16 +370,21 @@ static void test_typer_v2(uint32_t reg)
 static void test_byte_access(void *base_addr, u32 pattern, u32 mask)
 {
 	u32 reg = readb(base_addr + 1);
+	bool res;
 
-	report("byte reads successful (0x%08x => 0x%02x)",
-	       reg == (BYTE(pattern, 1) & (mask >> 8)),
-	       pattern & mask, reg);
+	res = (reg == (BYTE(pattern, 1) & (mask >> 8)));
+	report("byte reads successful", res);
+	if (!res)
+		report_info("byte 1 of 0x%08x => 0x%02x", pattern & mask, reg);
 
 	pattern = REPLACE_BYTE(pattern, 2, 0x1f);
 	writeb(BYTE(pattern, 2), base_addr + 2);
 	reg = readl(base_addr);
-	report("byte writes successful (0x%02x => 0x%08x)",
-	       reg == (pattern & mask), BYTE(pattern, 2), reg);
+	res = (reg == (pattern & mask));
+	report("byte writes successful", res);
+	if (!res)
+		report_info("writing 0x%02x into bytes 2 => 0x%08x",
+			    BYTE(pattern, 2), reg);
 }
 
 static void test_priorities(int nr_irqs, void *priptr)
@@ -399,15 +404,16 @@ static void test_priorities(int nr_irqs, void *priptr)
 	pri_mask = readl(first_spi);
 
 	reg = ~pri_mask;
-	report("consistent priority masking (0x%08x)",
+	report("consistent priority masking",
 	       (((reg >> 16) == (reg & 0xffff)) &&
-	        ((reg & 0xff) == ((reg >> 8) & 0xff))), pri_mask);
+	        ((reg & 0xff) == ((reg >> 8) & 0xff))));
+	report_info("priority mask is 0x%08x", pri_mask);
 
 	reg = reg & 0xff;
 	for (pri_bits = 8; reg & 1; reg >>= 1, pri_bits--)
 		;
-	report("implements at least 4 priority bits (%d)",
-	       pri_bits >= 4, pri_bits);
+	report("implements at least 4 priority bits", pri_bits >= 4);
+	report_info("%d priority bits implemented", pri_bits);
 
 	pattern = 0;
 	writel(pattern, first_spi);
@@ -452,9 +458,9 @@ static void test_targets(int nr_irqs)
 	/* Check that bits for non implemented CPUs are RAZ/WI. */
 	if (nr_cpus < 8) {
 		writel(0xffffffff, targetsptr + GIC_FIRST_SPI);
-		report("bits for %d non-existent CPUs masked",
-		       !(readl(targetsptr + GIC_FIRST_SPI) & ~cpu_mask),
-		       8 - nr_cpus);
+		report("bits for non-existent CPUs masked",
+		       !(readl(targetsptr + GIC_FIRST_SPI) & ~cpu_mask));
+		report_info("%d non-existent CPUs", 8 - nr_cpus);
 	} else {
 		report_skip("CPU masking (all CPUs implemented)");
 	}
@@ -465,8 +471,10 @@ static void test_targets(int nr_irqs)
 	pattern = 0x0103020f;
 	writel(pattern, targetsptr + GIC_FIRST_SPI);
 	reg = readl(targetsptr + GIC_FIRST_SPI);
-	report("register content preserved (%08x => %08x)",
-	       reg == (pattern & cpu_mask), pattern & cpu_mask, reg);
+	report("register content preserved", reg == (pattern & cpu_mask));
+	if (reg != (pattern & cpu_mask))
+		report_info("writing %08x reads back as %08x",
+			    pattern & cpu_mask, reg);
 
 	/* The TARGETS registers are byte accessible. */
 	test_byte_access(targetsptr + GIC_FIRST_SPI, pattern, cpu_mask);
@@ -505,9 +513,8 @@ static void gic_test_mmio(void)
 	       test_readonly_32(gic_dist_base + GICD_IIDR, false));
 
 	reg = readl(idreg);
-	report("ICPIDR2 is read-only (0x%08x)",
-	       test_readonly_32(idreg, false),
-	       reg);
+	report("ICPIDR2 is read-only", test_readonly_32(idreg, false));
+	report_info("value of ICPIDR2: 0x%08x", reg);
 
 	test_priorities(nr_irqs, gic_dist_base + GICD_IPRIORITYR);
 
Signature · 1 line
2.21.0
#6 lore raw

[PULL 04/10] arm: selftest: Split variable output data from test name

In reply to Andrew Jones

From: Andre Przywara <[email protected]>

For some tests we mix variable diagnostic output with the test name, which leads to variable test line, confusing some higher level frameworks.

Split the output to always use the same test name for a certain test, and put diagnostic output on a separate line using the INFO: tag.

---
 arm/selftest.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)
arm/selftest.c +6 -3
index 28a17f7a7531..a0c1ab8180bc 100644
@@ -43,13 +43,16 @@ static void check_setup(int argc, char **argv)
 			phys_addr_t memsize = PHYS_END - PHYS_OFFSET;
 			phys_addr_t expected = ((phys_addr_t)val)*1024*1024;
 
-			report("size = %" PRIu64 " MB", memsize == expected,
-							memsize/1024/1024);
+			report("memory size matches expectation",
+			       memsize == expected);
+			report_info("found %" PRIu64 " MB", memsize/1024/1024);
 			++nr_tests;
 
 		} else if (strcmp(argv[i], "smp") == 0) {
 
-			report("nr_cpus = %d", nr_cpus == (int)val, nr_cpus);
+			report("number of CPUs matches expectation",
+			       nr_cpus == (int)val);
+			report_info("found %d CPUs", nr_cpus);
 			++nr_tests;
 		}
 
Signature · 1 line
2.21.0
#7 lore raw

[PULL 07/10] lib: arm64: Add missing ISB in flush_tlb_page

In reply to Andrew Jones

From: Alexandru Elisei <[email protected]>

Linux commit d0b7a302d58a made it abundantly clear that certain CPU implementations require an ISB after a DSB. Add the missing ISB to flush_tlb_page. No changes are required for flush_tlb_all, as the function already had the ISB.

---
 lib/arm64/asm/mmu.h | 1 +
 1 file changed, 1 insertion(+)
lib/arm64/asm/mmu.h +1 -0
index fa554b0c20ae..72d75eafc882 100644
@@ -24,6 +24,7 @@ static inline void flush_tlb_page(unsigned long vaddr)
 	dsb(ishst);
 	asm("tlbi	vaae1is, %0" :: "r" (page));
 	dsb(ish);
+	isb();
 }
 
 static inline void flush_dcache_addr(unsigned long vaddr)
Signature · 1 line
2.21.0
#8 lore raw

[PULL 09/10] arm64: Add cache code generation test

In reply to Andrew Jones

From: Alexandru Elisei <[email protected]>

Caches are a misterious creature on arm64, requiring a more hands-on approach from the programmer than on x86. When generating code, two cache maintenance operations are generally required: an invalidation for the stale instruction and a clean to the PoU (Point of Unification) for the new instruction. Fortunately, the ARM architecture has features to alleviate some of this overhead, which are advertised via the IDC and DIC bits in
CTR_EL0: if IDC is 1, then the dcache clean is not required, and if DIC is 1, the icache invalidation can be absent. KVM exposes these bits to the guest.

Until Linux v4.16.1, KVM performed an icache invalidation each time a stage 2 page was mapped. This was then optimized so that the icache invalidation was performed when the guest tried to execute code from the page for the first time. And that was optimized again when support for the DIC bit was added to KVM.

The interactions between a guest that is generating code, the stage 2 tables and the IDC and DIC bits can be subtle, especially when KVM optimizations come into play. Let's add a test that generates a few instructions and checks that KVM indeed honors those bits.

---
 arm/Makefile.arm64 |   1 +
 arm/cache.c        | 122 +++++++++++++++++++++++++++++++++++++++++++++
 arm/unittests.cfg  |   6 +++
 3 files changed, 129 insertions(+)
 create mode 100644 arm/cache.c
arm/Makefile.arm64 +1 -0
index 35de5ea333b4..6d3dc2c4a464 100644
@@ -25,6 +25,7 @@ OBJDIRS += lib/arm64
 # arm64 specific tests
 tests = $(TEST_DIR)/timer.flat
 tests += $(TEST_DIR)/micro-bench.flat
+tests += $(TEST_DIR)/cache.flat
 
 include $(SRCDIR)/$(TEST_DIR)/Makefile.common
 
arm/cache.c new file +122 -0

122 lines not on this page · raw message

arm/unittests.cfg +6 -0
index 6d3df92a4e28..daeb5a09ad39 100644
@@ -142,3 +142,9 @@ smp = 2
 groups = nodefault,micro-bench
 accel = kvm
 arch = arm64
+
+# Cache emulation tests
+[cache]
+file = cache.flat
+arch = arm64
+groups = cache
Signature · 1 line
2.21.0
#9 lore raw

[PULL 08/10] lib: arm/arm64: Add function to clear the PTE_USER bit

In reply to Andrew Jones

From: Alexandru Elisei <[email protected]>

The PTE_USER bit (AP[1]) in a page entry means that lower privilege levels (EL0, on arm64, or PL0, on arm) can read and write from that memory location [1][2]. On arm64, it also implies PXN (Privileged execute-never) when is set [3]. Add a function to clear the bit which we can use when we want to execute code from that page or the prevent access from lower exception levels.

Make it available to arm too, in case someone needs it at some point.

[1] ARM DDI 0406C.d, Table B3-6
[2] ARM DDI 0487E.a, table D5-28
[3] ARM DDI 0487E.a, table D5-33

---
 lib/arm/asm/mmu-api.h |  1 +
 lib/arm/mmu.c         | 15 +++++++++++++++
 2 files changed, 16 insertions(+)
lib/arm/asm/mmu-api.h +1 -0
index df3ccf7bc7e0..8fe85ba31ec9 100644
@@ -22,4 +22,5 @@ extern void mmu_set_range_sect(pgd_t *pgtable, uintptr_t virt_offset,
 extern void mmu_set_range_ptes(pgd_t *pgtable, uintptr_t virt_offset,
 			       phys_addr_t phys_start, phys_addr_t phys_end,
 			       pgprot_t prot);
+extern void mmu_clear_user(unsigned long vaddr);
 #endif
lib/arm/mmu.c +15 -0
index 3d38c8397f5a..78db22e6af14 100644
@@ -217,3 +217,18 @@ unsigned long __phys_to_virt(phys_addr_t addr)
 	assert(!mmu_enabled() || __virt_to_phys(addr) == addr);
 	return addr;
 }
+
+void mmu_clear_user(unsigned long vaddr)
+{
+	pgd_t *pgtable;
+	pteval_t *pte;
+
+	if (!mmu_enabled())
+		return;
+
+	pgtable = current_thread_info()->pgtable;
+	pte = get_pte(pgtable, vaddr);
+
+	*pte &= ~PTE_USER;
+	flush_tlb_page(vaddr);
+}
Signature · 1 line
2.21.0
#10 lore raw

[PULL 10/10] arm: Add PL031 test

In reply to Andrew Jones

From: Alexander Graf <[email protected]>

This patch adds a unit test for the PL031 RTC that is used in the virt machine. It just pokes basic functionality. I've mostly written it to familiarize myself with the device, but I suppose having the test around does not hurt, as it also exercises the GIC SPI interrupt path.

---
 arm/Makefile.common |   1 +
 arm/pl031.c         | 262 ++++++++++++++++++++++++++++++++++++++++++++
 lib/arm/asm/gic.h   |   1 +
 3 files changed, 264 insertions(+)
 create mode 100644 arm/pl031.c
arm/Makefile.common +1 -0
index f0c4b5d7620c..b8988f214d3b 100644
@@ -11,6 +11,7 @@ tests-common += $(TEST_DIR)/pmu.flat
 tests-common += $(TEST_DIR)/gic.flat
 tests-common += $(TEST_DIR)/psci.flat
 tests-common += $(TEST_DIR)/sieve.flat
+tests-common += $(TEST_DIR)/pl031.flat
 
 tests-all = $(tests-common) $(tests)
 all: directories $(tests-all)
arm/pl031.c new file +262 -0

262 lines not on this page · raw message

lib/arm/asm/gic.h +1 -0
index f6dfb907a7d5..1fc10a096259 100644
@@ -41,6 +41,7 @@
 #include <asm/gic-v3.h>
 
 #define PPI(irq)			((irq) + 16)
+#define SPI(irq)			((irq) + GIC_FIRST_SPI)
 
 #ifndef __ASSEMBLY__
 #include <asm/cpumask.h>
Signature · 1 line
2.21.0