KVM ARM Archive on lore.kernel.org
 help / color / Atom feed
* [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2
@ 2019-08-28 13:38 Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level Alexandru Elisei
                   ` (15 more replies)
  0 siblings, 16 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

ARMv8.3 added support for nested virtualization, which makes it possible
for a hypervisor to run another hypervisor as a guest. Support for nested
virtualization is being worked on in KVM [1].

This patch series aims to make it possible for kvm-unit-tests to run at EL2
under KVM. The focus has been on having all the infrastructure in place to
run at EL2, and not on adding comprehensive tests for this Exception Level.
All existing tests that fulfill KVM's requirements for a nested guest (the
architecture is arm64 and they use GICv3) will be able to be run at EL2.

To keep the changes minimal, kvm-unit-tests will run with VHE enabled when
it detects that has been booted at EL2. Functions for enabling and
disabling VHE have been added, with the aim to let the user specify to
disable VHE for a given test via command-line parameters. At the moment,
only the timer test has been modified to run with VHE disabled.

The series are firmly an RFC because:
* The patches that implement KVM nested support are themselves in the RFC
  phase.
* Some tests don't complete because of bugs in the current version of the
  KVM patches. Where appropriate, I will provide fixes to allow the tests
  to finish, however those fixes are my own have not been reviewed in any
  way. Use at your own risk.

To run the tests, one obviously needs KVM with nested virtualization
support from [2]. These patches have been tested from commit
78c66132035c ("arm64: KVM: nv: Allow userspace to request
KVM_ARM_VCPU_NESTED_VIRT"), on top of which the following patches have been
cherry-picked from upstream Linux:
* b4a1583abc83 ("KVM: arm/arm64: Fix emulated ptimer irq injection")
* 16e604a437c8 ("KVM: arm/arm64: vgic: Reevaluate level sensitive
  interrupts on enable")

Without those two patches some timer tests will fail.

A version of kvmtool that knows about nested virtualization is also
needed [3]. The kvmtool --nested parameter is required for releasing a
guest at EL2. For example, to run the newly added selftest-el2 test:

lkvm -f selftest.flat -c 2 -m 128 -p el2 --nested --console serial \
	--irqchip gicv3

Summary of the patches:
* Patches 1-10 are various fixes or enhancements and can be merged without
  the rest of the series.
* Patches 11-13 add support for running at EL2. A basic selftest-el2 test
  is added that targets EL2.
* Patches 14-16 add support for disabling VHE. The timer and selftest-el2
  tests are modified to use this feature.

[1] https://www.spinics.net/lists/arm-kernel/msg736687.html
[2] git://git.kernel.org/pub/scm/linux/kernel/git/maz/arm-platforms.git kvm-arm64/nv-wip-5.2-rc5
[3] git://linux-arm.org/kvmtool.git nv/nv-wip-5.2-rc5

Alexandru Elisei (16):
  arm: selftest.c: Remove redundant check for Exception Level
  arm/arm64: psci: Don't run C code without stack or vectors
  lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h
  arm/arm64: selftest: Add prefetch abort test
  arm64: timer: Write to ICENABLER to disable timer IRQ
  arm64: timer: EOIR the interrupt after masking the timer
  arm64: timer: Test behavior when timer disabled or masked
  lib: arm/arm64: Refuse to disable the MMU with non-identity stack
    pointer
  lib: arm/arm64: Invalidate TLB before enabling MMU
  lib: Add UL and ULL definitions to linux/const.h
  lib: arm64: Run existing tests at EL2
  arm64: timer: Add test for EL2 timers
  arm64: selftest: Add basic test for EL2
  lib: arm64: Add support for disabling and re-enabling VHE
  arm64: selftest: Expand EL2 test to disable and re-enable VHE
  arm64: timer: Run tests with VHE disabled

 lib/linux/const.h             |   7 +-
 lib/arm/asm/gic-v3.h          |   1 +
 lib/arm/asm/gic.h             |   1 +
 lib/arm/asm/pgtable.h         |   1 +
 lib/arm/asm/processor.h       |   8 +
 lib/arm/asm/psci.h            |   1 +
 lib/arm64/asm/esr.h           |   5 +
 lib/arm64/asm/mmu.h           |  11 +-
 lib/arm64/asm/pgtable-hwdef.h |  55 +++++--
 lib/arm64/asm/pgtable.h       |   1 +
 lib/arm64/asm/processor.h     |  53 +++++++
 lib/arm64/asm/sysreg.h        |  28 ++++
 lib/arm/mmu.c                 |   5 +-
 lib/arm/processor.c           |  11 ++
 lib/arm/psci.c                |  43 +++++-
 lib/arm/setup.c               |   6 +
 lib/arm64/processor.c         |  69 ++++++++-
 arm/cstart.S                  |  11 ++
 arm/cstart64.S                | 221 ++++++++++++++++++++++++++-
 arm/micro-bench.c             |  17 ++-
 arm/psci.c                    |   5 +-
 arm/selftest.c                | 175 ++++++++++++++++++++--
 arm/timer.c                   | 340 +++++++++++++++++++++++++++++++++++++-----
 arm/unittests.cfg             |   8 +
 24 files changed, 1010 insertions(+), 73 deletions(-)

-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:32   ` Andrew Jones
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
                   ` (14 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

expected_regs.pstate already has PSR_MODE_EL1h set as the expected
Exception Level.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/selftest.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/arm/selftest.c b/arm/selftest.c
index 28a17f7a7531..176231f32ee1 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -213,10 +213,6 @@ static bool check_regs(struct pt_regs *regs)
 {
 	unsigned i;
 
-	/* exception handlers should always run in EL1 */
-	if (current_level() != CurrentEL_EL1)
-		return false;
-
 	for (i = 0; i < ARRAY_SIZE(regs->regs); ++i) {
 		if (regs->regs[i] != expected_regs.regs[i])
 			return false;
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:45   ` Andrew Jones
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
                   ` (13 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

The psci test performs a series of CPU_ON/CPU_OFF cycles for CPU 1. This is
done by setting the entry point for the CPU_ON call to the physical address
of the C function cpu_psci_cpu_die.

The compiler is well within its rights to use the stack when generating
code for cpu_psci_cpu_die.  However, because no stack initialization has
been done, the stack pointer is zero, as set by KVM when creating the VCPU.
This causes a data abort without a change in exception level. The VBAR_EL1
register is also zero (the KVM reset value for VBAR_EL1), the MMU is off,
and we end up trying to fetch instructions from address 0x200.

At this point, a stage 2 instruction abort is generated which is taken to
KVM. KVM interprets this as an instruction fetch from an I/O region, and
injects a prefetch abort into the guest. Prefetch abort is a synchronous
exception, and on guest return the VCPU PC will be set to VBAR_EL1 + 0x200,
which is...  0x200. The VCPU ends up in an infinite loop causing a prefetch
abort while fetching the instruction to service the said abort.

cpu_psci_cpu_die is basically a wrapper over the HVC instruction, so
provide an assembly implementation for the function which will serve as the
entry point for CPU_ON.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/cstart.S   | 7 +++++++
 arm/cstart64.S | 7 +++++++
 arm/psci.c     | 5 +++--
 3 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/arm/cstart.S b/arm/cstart.S
index 114726feab82..5d4fe4b1570b 100644
--- a/arm/cstart.S
+++ b/arm/cstart.S
@@ -7,6 +7,7 @@
  */
 #define __ASSEMBLY__
 #include <auxinfo.h>
+#include <linux/psci.h>
 #include <asm/thread_info.h>
 #include <asm/asm-offsets.h>
 #include <asm/ptrace.h>
@@ -138,6 +139,12 @@ secondary_entry:
 	blx	r0
 	b	do_idle
 
+.global asm_cpu_psci_cpu_die
+asm_cpu_psci_cpu_die:
+	ldr	r0, =PSCI_0_2_FN_CPU_OFF
+	hvc	#0
+	b	halt
+
 .globl halt
 halt:
 1:	wfi
diff --git a/arm/cstart64.S b/arm/cstart64.S
index b0e8baa1a23a..20f832fd57f7 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -7,6 +7,7 @@
  */
 #define __ASSEMBLY__
 #include <auxinfo.h>
+#include <linux/psci.h>
 #include <asm/asm-offsets.h>
 #include <asm/ptrace.h>
 #include <asm/processor.h>
@@ -128,6 +129,12 @@ secondary_entry:
 	blr	x0
 	b	do_idle
 
+.globl asm_cpu_psci_cpu_die
+asm_cpu_psci_cpu_die:
+	ldr	x0, =PSCI_0_2_FN_CPU_OFF
+	hvc	#0
+	b	halt
+
 .globl halt
 halt:
 1:	wfi
diff --git a/arm/psci.c b/arm/psci.c
index 5cb4d5c7c233..0440c4cdbc59 100644
--- a/arm/psci.c
+++ b/arm/psci.c
@@ -72,6 +72,7 @@ static int cpu_on_ret[NR_CPUS];
 static cpumask_t cpu_on_ready, cpu_on_done;
 static volatile int cpu_on_start;
 
+extern void asm_cpu_psci_cpu_die(void);
 static void cpu_on_secondary_entry(void)
 {
 	int cpu = smp_processor_id();
@@ -79,7 +80,7 @@ static void cpu_on_secondary_entry(void)
 	cpumask_set_cpu(cpu, &cpu_on_ready);
 	while (!cpu_on_start)
 		cpu_relax();
-	cpu_on_ret[cpu] = psci_cpu_on(cpus[1], __pa(cpu_psci_cpu_die));
+	cpu_on_ret[cpu] = psci_cpu_on(cpus[1], __pa(asm_cpu_psci_cpu_die));
 	cpumask_set_cpu(cpu, &cpu_on_done);
 }
 
@@ -104,7 +105,7 @@ static bool psci_cpu_on_test(void)
 	cpu_on_start = 1;
 	smp_mb();
 
-	cpu_on_ret[0] = psci_cpu_on(cpus[1], __pa(cpu_psci_cpu_die));
+	cpu_on_ret[0] = psci_cpu_on(cpus[1], __pa(asm_cpu_psci_cpu_die));
 	cpumask_set_cpu(0, &cpu_on_done);
 
 	while (!cpumask_full(&cpu_on_done))
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:47   ` Andrew Jones
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
                   ` (12 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

pgtable.h is used only by mmu.c, where it is included after alloc_page.h.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/asm/pgtable.h   | 1 +
 lib/arm64/asm/pgtable.h | 1 +
 2 files changed, 2 insertions(+)

diff --git a/lib/arm/asm/pgtable.h b/lib/arm/asm/pgtable.h
index 241dff69b38a..b01c34348321 100644
--- a/lib/arm/asm/pgtable.h
+++ b/lib/arm/asm/pgtable.h
@@ -13,6 +13,7 @@
  *
  * This work is licensed under the terms of the GNU GPL, version 2.
  */
+#include <alloc_page.h>
 
 /*
  * We can convert va <=> pa page table addresses with simple casts
diff --git a/lib/arm64/asm/pgtable.h b/lib/arm64/asm/pgtable.h
index ee0a2c88cc18..e9dd49155564 100644
--- a/lib/arm64/asm/pgtable.h
+++ b/lib/arm64/asm/pgtable.h
@@ -14,6 +14,7 @@
  * This work is licensed under the terms of the GNU GPL, version 2.
  */
 #include <alloc.h>
+#include <alloc_page.h>
 #include <asm/setup.h>
 #include <asm/page.h>
 #include <asm/pgtable-hwdef.h>
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (2 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:09   ` Mark Rutland
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 05/16] arm64: timer: Write to ICENABLER to disable timer IRQ Alexandru Elisei
                   ` (11 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

When a guest tries to execute code from MMIO memory, KVM injects an
external abort into that guest. We have now fixed the psci test to not
fetch instructions from the I/O region, and it's not that often that a
guest misbehaves in such a way. Let's expand our coverage by adding a
proper test targetting this corner case.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
The fault injection path is broken for nested guests [1]. You can use the
last patch from the thread [2] to successfully run the test at EL2.

[1] https://www.spinics.net/lists/arm-kernel/msg745391.html
[2] https://www.spinics.net/lists/arm-kernel/msg750310.html

 lib/arm64/asm/esr.h |  3 ++
 arm/selftest.c      | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 96 insertions(+), 3 deletions(-)

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_ */
diff --git a/arm/selftest.c b/arm/selftest.c
index 176231f32ee1..18cc0ad8f729 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -16,6 +16,8 @@
 #include <asm/psci.h>
 #include <asm/smp.h>
 #include <asm/barrier.h>
+#include <asm/mmu.h>
+#include <asm/pgtable.h>
 
 static void __user_psci_system_off(void)
 {
@@ -60,9 +62,38 @@ static void check_setup(int argc, char **argv)
 		report_abort("missing input");
 }
 
+extern pgd_t *mmu_idmap;
+static void prep_io_exec(void)
+{
+	pgd_t *pgd = pgd_offset(mmu_idmap, 0);
+	unsigned long sctlr;
+
+	/*
+	 * AArch64 treats all regions writable at EL0 as PXN. Clear the user bit
+	 * so we can execute code from the bottom I/O space (0G-1G) to simulate
+	 * a misbehaved guest.
+	 */
+	pgd_val(*pgd) &= ~PMD_SECT_USER;
+	flush_dcache_addr((unsigned long)pgd);
+	flush_tlb_page(0);
+
+	/* Make sure we can actually execute from a writable region */
+#ifdef __arm__
+	asm volatile("mrc p15, 0, %0, c1, c0, 0": "=r" (sctlr));
+	sctlr &= ~CR_ST;
+	asm volatile("mcr p15, 0, %0, c1, c0, 0" :: "r" (sctlr));
+#else
+	sctlr = read_sysreg(sctlr_el1);
+	sctlr &= ~SCTLR_EL1_WXN;
+	write_sysreg(sctlr, sctlr_el1);
+#endif
+	isb();
+}
+
 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
@@ -86,7 +117,7 @@ static bool svc_works;
 		"str	r1, [r0, #" xstr(S_PC) "]\n"		\
 		excptn_insn "\n"				\
 		post_insns "\n"					\
-	:: "r" (&expected_regs) : "r0", "r1")
+	:: "r" (&expected_regs) : "r0", "r1", "r2")
 
 static bool check_regs(struct pt_regs *regs)
 {
@@ -166,6 +197,32 @@ static void user_psci_system_off(struct pt_regs *regs)
 {
 	__user_psci_system_off();
 }
+
+static void check_pabt_exit(void)
+{
+	install_exception_handler(EXCPTN_PABT, NULL);
+
+	report("pabt", pabt_works);
+	exit(report_summary());
+}
+
+static void pabt_handler(struct pt_regs *regs)
+{
+	expected_regs.ARM_pc = 0;
+	pabt_works = check_regs(regs);
+
+	regs->ARM_pc = (unsigned long)&check_pabt_exit;
+}
+
+static void check_pabt(void)
+{
+	install_exception_handler(EXCPTN_PABT, pabt_handler);
+
+	prep_io_exec();
+
+	test_exception("mov r2, #0x0", "bx r2", "");
+	__builtin_unreachable();
+}
 #elif defined(__aarch64__)
 
 /*
@@ -207,7 +264,7 @@ static void user_psci_system_off(struct pt_regs *regs)
 		"stp	 x0,  x1, [x1]\n"			\
 	"1:"	excptn_insn "\n"				\
 		post_insns "\n"					\
-	:: "r" (&expected_regs) : "x0", "x1")
+	:: "r" (&expected_regs) : "x0", "x1", "x2")
 
 static bool check_regs(struct pt_regs *regs)
 {
@@ -279,6 +336,37 @@ static bool check_svc(void)
 	return svc_works;
 }
 
+static void check_pabt_exit(void)
+{
+	install_exception_handler(EL1H_SYNC, ESR_EL1_EC_IABT_EL1, NULL);
+
+	report("pabt", pabt_works);
+	exit(report_summary());
+}
+
+static void pabt_handler(struct pt_regs *regs, unsigned int esr)
+{
+	bool is_extabt;
+
+	expected_regs.pc = 0;
+	is_extabt = (esr & ESR_EL1_FSC_MASK) == ESR_EL1_FSC_EXTABT;
+	pabt_works = check_regs(regs) && is_extabt;
+
+	regs->pc = (u64)&check_pabt_exit;
+}
+
+static void check_pabt(void)
+{
+	enum vector v = check_vector_prep();
+
+	install_exception_handler(v, ESR_EL1_EC_IABT_EL1, pabt_handler);
+
+	prep_io_exec();
+
+	test_exception("mov x2, xzr", "br x2", "");
+	__builtin_unreachable();
+}
+
 static void user_psci_system_off(struct pt_regs *regs, unsigned int esr)
 {
 	__user_psci_system_off();
@@ -289,7 +377,9 @@ static void check_vectors(void *arg __unused)
 {
 	report("und", check_und());
 	report("svc", check_svc());
-	if (is_user()) {
+	if (!is_user()) {
+		check_pabt();
+	} else {
 #ifdef __arm__
 		install_exception_handler(EXCPTN_UND, user_psci_system_off);
 #else
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 05/16] arm64: timer: Write to ICENABLER to disable timer IRQ
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (3 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 06/16] arm64: timer: EOIR the interrupt after masking the timer Alexandru Elisei
                   ` (10 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

According the Generic Interrupt Controller versions 2, 3 and 4 architecture
specifications, a write of 0 to the GIC{D,R}_ISENABLER{,0} registers is
ignored; this is also how KVM emulates the corresponding register. Write
instead to the ICENABLER register when disabling the timer interrupt.

Note that fortunately for us, the timer test was still working as intended
because KVM does the sensible thing and all interrupts are disabled by
default when creating a VM.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/asm/gic-v3.h |  1 +
 lib/arm/asm/gic.h    |  1 +
 arm/timer.c          | 22 +++++++++++-----------
 3 files changed, 13 insertions(+), 11 deletions(-)

diff --git a/lib/arm/asm/gic-v3.h b/lib/arm/asm/gic-v3.h
index 347be2f9da17..0dc838b3ab2d 100644
--- a/lib/arm/asm/gic-v3.h
+++ b/lib/arm/asm/gic-v3.h
@@ -31,6 +31,7 @@
 /* Re-Distributor registers, offsets from SGI_base */
 #define GICR_IGROUPR0			GICD_IGROUPR
 #define GICR_ISENABLER0			GICD_ISENABLER
+#define GICR_ICENABLER0			GICD_ICENABLER
 #define GICR_IPRIORITYR0		GICD_IPRIORITYR
 
 #define ICC_SGI1R_AFFINITY_1_SHIFT	16
diff --git a/lib/arm/asm/gic.h b/lib/arm/asm/gic.h
index f6dfb907a7d5..a67111607bcf 100644
--- a/lib/arm/asm/gic.h
+++ b/lib/arm/asm/gic.h
@@ -15,6 +15,7 @@
 #define GICD_IIDR			0x0008
 #define GICD_IGROUPR			0x0080
 #define GICD_ISENABLER			0x0100
+#define GICD_ICENABLER			0x0180
 #define GICD_ISPENDR			0x0200
 #define GICD_ICPENDR			0x0280
 #define GICD_ISACTIVER			0x0300
diff --git a/arm/timer.c b/arm/timer.c
index f2f60192ba62..78f0dd870993 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -17,6 +17,9 @@
 #define ARCH_TIMER_CTL_ISTATUS (1 << 2)
 
 static void *gic_ispendr;
+static void *gic_isenabler;
+static void *gic_icenabler;
+
 static bool ptimer_unsupported;
 
 static void ptimer_unsupported_handler(struct pt_regs *regs, unsigned int esr)
@@ -132,19 +135,12 @@ static struct timer_info ptimer_info = {
 
 static void set_timer_irq_enabled(struct timer_info *info, bool enabled)
 {
-	u32 val = 0;
+	u32 val = 1 << PPI(info->irq);
 
 	if (enabled)
-		val = 1 << PPI(info->irq);
-
-	switch (gic_version()) {
-	case 2:
-		writel(val, gicv2_dist_base() + GICD_ISENABLER + 0);
-		break;
-	case 3:
-		writel(val, gicv3_sgi_base() + GICR_ISENABLER0);
-		break;
-	}
+		writel(val, gic_isenabler);
+	else
+		writel(val, gic_icenabler);
 }
 
 static void irq_handler(struct pt_regs *regs)
@@ -305,9 +301,13 @@ static void test_init(void)
 	switch (gic_version()) {
 	case 2:
 		gic_ispendr = gicv2_dist_base() + GICD_ISPENDR;
+		gic_isenabler = gicv2_dist_base() + GICD_ISENABLER;
+		gic_icenabler = gicv2_dist_base() + GICD_ICENABLER;
 		break;
 	case 3:
 		gic_ispendr = gicv3_sgi_base() + GICD_ISPENDR;
+		gic_isenabler = gicv3_sgi_base() + GICR_ISENABLER0;
+		gic_icenabler = gicv3_sgi_base() + GICR_ICENABLER0;
 		break;
 	}
 
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 06/16] arm64: timer: EOIR the interrupt after masking the timer
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (4 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 05/16] arm64: timer: Write to ICENABLER to disable timer IRQ Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 07/16] arm64: timer: Test behavior when timer disabled or masked Alexandru Elisei
                   ` (9 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Writing to the EOIR register before masking the HW mapped timer interrupt
can cause taking another timer interrupt immediatly after exception return.
This doesn't happen all the time, because KVM reevaluates the state of
pending HW mapped level sensitive interrupts on each guest exit. If a guest
exit occurs after masking the timer interrupt, but before the ERET, when
the extra interrupt is pending, then KVM will remove it.

Move the write after the IMASK bit has been set to prevent this from
happening.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arm/timer.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arm/timer.c b/arm/timer.c
index 78f0dd870993..7ae169bd687e 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -149,8 +149,8 @@ static void irq_handler(struct pt_regs *regs)
 	u32 irqstat = gic_read_iar();
 	u32 irqnr = gic_iar_irqnr(irqstat);
 
-	if (irqnr != GICC_INT_SPURIOUS)
-		gic_write_eoir(irqstat);
+	if (irqnr == GICC_INT_SPURIOUS)
+		return;
 
 	if (irqnr == PPI(vtimer_info.irq)) {
 		info = &vtimer_info;
@@ -162,7 +162,11 @@ static void irq_handler(struct pt_regs *regs)
 	}
 
 	info->write_ctl(ARCH_TIMER_CTL_IMASK | ARCH_TIMER_CTL_ENABLE);
+	isb();
+
 	info->irq_received = true;
+
+	gic_write_eoir(irqstat);
 }
 
 static bool gic_timer_pending(struct timer_info *info)
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 07/16] arm64: timer: Test behavior when timer disabled or masked
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (5 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 06/16] arm64: timer: EOIR the interrupt after masking the timer Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
                   ` (8 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

When the timer is disabled (the ENABLE bit is clear) or the timer interrupt
is masked at the timer level (the IMASK bit is set), timer interrupts must
not be pending or asserted by the VGIC. However, when the timer interrupt
is masked, we can still check that the timer condition is met by reading
the ISTATUS bit.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
This test was used to discover the issue fixed by the upstream patch
16e604a437c8 ("KVM: arm/arm64: vgic: Reevaluate level sensitive interrupts
on enable").

 arm/timer.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arm/timer.c b/arm/timer.c
index 7ae169bd687e..125b9f30ad3c 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -230,9 +230,17 @@ static void test_timer(struct timer_info *info)
 
 	/* Disable the timer again and prepare to take interrupts */
 	info->write_ctl(0);
+	isb();
+	info->irq_received = false;
 	set_timer_irq_enabled(info, true);
+	report("no interrupt when timer is disabled", !info->irq_received);
 	report("interrupt signal no longer pending", !gic_timer_pending(info));
 
+	info->write_ctl(ARCH_TIMER_CTL_ENABLE | ARCH_TIMER_CTL_IMASK);
+	isb();
+	report("interrupt signal not pending", !gic_timer_pending(info));
+	report("timer condition met", info->read_ctl() & ARCH_TIMER_CTL_ISTATUS);
+
 	report("latency within 10 ms", test_cval_10msec(info));
 	report("interrupt received", info->irq_received);
 
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (6 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 07/16] arm64: timer: Test behavior when timer disabled or masked Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:55   ` Andrew Jones
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
                   ` (7 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

When the MMU is off, all addresses are physical addresses. If the stack
pointer is not an identity mapped address (the virtual address is not the
same as the physical address), then we end up trying to access an invalid
memory region. This can happen if we call mmu_disable from a secondary CPU,
which has its stack allocated from the vmalloc region.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/mmu.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index 3d38c8397f5a..161f7a8e607c 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -66,8 +66,12 @@ void mmu_enable(pgd_t *pgtable)
 extern void asm_mmu_disable(void);
 void mmu_disable(void)
 {
+	unsigned long sp = current_stack_pointer;
 	int cpu = current_thread_info()->cpu;
 
+	assert_msg(__virt_to_phys(sp) == sp,
+			"Attempting to disable MMU with non-identity mapped stack");
+
 	mmu_mark_disabled(cpu);
 
 	asm_mmu_disable();
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (7 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:59   ` Andrew Jones
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h Alexandru Elisei
                   ` (6 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Let's invalidate the TLB before enabling the MMU, not after, so we don't
accidently use a stale TLB mapping. For arm64, we already do that in
asm_mmu_enable, and we issue an extra invalidation after the function
returns. Invalidate the TLB in asm_mmu_enable for arm also, and remove the
redundant call to tlb_flush_all.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/mmu.c | 1 -
 arm/cstart.S  | 4 ++++
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
index 161f7a8e607c..66a05d789386 100644
--- a/lib/arm/mmu.c
+++ b/lib/arm/mmu.c
@@ -57,7 +57,6 @@ void mmu_enable(pgd_t *pgtable)
 	struct thread_info *info = current_thread_info();
 
 	asm_mmu_enable(__pa(pgtable));
-	flush_tlb_all();
 
 	info->pgtable = pgtable;
 	mmu_mark_enabled(info->cpu);
diff --git a/arm/cstart.S b/arm/cstart.S
index 5d4fe4b1570b..316672545551 100644
--- a/arm/cstart.S
+++ b/arm/cstart.S
@@ -160,6 +160,10 @@ halt:
 .equ	NMRR,	0xff000004		@ MAIR1 (from Linux kernel)
 .globl asm_mmu_enable
 asm_mmu_enable:
+	/* TLBIALL */
+	mcr	p15, 0, r2, c8, c7, 0
+	dsb	ish
+
 	/* TTBCR */
 	mrc	p15, 0, r2, c2, c0, 2
 	orr	r2, #(1 << 31)		@ TTB_EAE
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (8 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 15:10   ` Andrew Jones
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 11/16] lib: arm64: Run existing tests at EL2 Alexandru Elisei
                   ` (5 subsequent siblings)
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

The UL macro was previously defined in lib/arm64/asm/pgtable-hwdef.h. Move
it to lib/linux/const.h so it can be used in other files. To keep things
consistent, also add an ULL macro.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/linux/const.h             | 7 +++++--
 lib/arm64/asm/pgtable-hwdef.h | 2 --
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/lib/linux/const.h b/lib/linux/const.h
index c872bfd25e13..e3c7fec3f4b8 100644
--- a/lib/linux/const.h
+++ b/lib/linux/const.h
@@ -21,7 +21,10 @@
 #define _AT(T,X)	((T)(X))
 #endif
 
-#define _BITUL(x)	(_AC(1,UL) << (x))
-#define _BITULL(x)	(_AC(1,ULL) << (x))
+#define UL(x) 		_AC(x, UL)
+#define ULL(x)		_AC(x, ULL)
+
+#define _BITUL(x)	(UL(1) << (x))
+#define _BITULL(x)	(ULL(1) << (x))
 
 #endif /* !(_LINUX_CONST_H) */
diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
index 045a3ce12645..e6f02fae4075 100644
--- a/lib/arm64/asm/pgtable-hwdef.h
+++ b/lib/arm64/asm/pgtable-hwdef.h
@@ -9,8 +9,6 @@
  * This work is licensed under the terms of the GNU GPL, version 2.
  */
 
-#define UL(x) _AC(x, UL)
-
 #define PTRS_PER_PTE		(1 << (PAGE_SHIFT - 3))
 
 /*
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 11/16] lib: arm64: Run existing tests at EL2
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (9 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 12/16] arm64: timer: Add test for EL2 timers Alexandru Elisei
                   ` (4 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Version 8.3 of the ARM architecture added support for nested
virtualization. KVM supports both VHE and non-VHE guest hypervisors running
at virtual EL2. To make the changes as minimal as possible, kvm-unit-tests
will run as a VHE guest hypervisor when booted at EL2.

To distinguish between a L1 non-VHE host calling from EL1 into the L1
hypervisor running at EL2 and a PSCI call, KVM sets the PSCI conduit for
guests that boot at EL2 to the SMC instruction. Modify existing tests to
take that into account.

Nested virtualization is enabled only for arm64 guests using GICv3 and
existing tests have been modified to run at EL2.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/asm/psci.h        |   1 +
 lib/arm64/asm/esr.h       |   2 +
 lib/arm64/asm/processor.h |   5 ++
 lib/arm64/asm/sysreg.h    |  20 ++++++++
 lib/arm/psci.c            |  43 ++++++++++++++--
 lib/arm/setup.c           |   4 ++
 lib/arm64/processor.c     |   2 +
 arm/cstart64.S            |  28 ++++++++++
 arm/micro-bench.c         |  17 ++++++-
 arm/selftest.c            |  29 +++++++++--
 arm/timer.c               | 127 +++++++++++++++++++++++++++++++++++++++-------
 11 files changed, 252 insertions(+), 26 deletions(-)

diff --git a/lib/arm/asm/psci.h b/lib/arm/asm/psci.h
index 7b956bf5987d..2ed6613fe5ea 100644
--- a/lib/arm/asm/psci.h
+++ b/lib/arm/asm/psci.h
@@ -3,6 +3,7 @@
 #include <libcflat.h>
 #include <linux/psci.h>
 
+extern void psci_init(void);
 extern int psci_invoke(unsigned long function_id, unsigned long arg0,
 		       unsigned long arg1, unsigned long arg2);
 extern int psci_cpu_on(unsigned long cpuid, unsigned long entry_point);
diff --git a/lib/arm64/asm/esr.h b/lib/arm64/asm/esr.h
index 8c351631b0a0..6d4f572923f2 100644
--- a/lib/arm64/asm/esr.h
+++ b/lib/arm64/asm/esr.h
@@ -25,6 +25,8 @@
 #define ESR_EL1_EC_ILL_ISS	(0x0E)
 #define ESR_EL1_EC_SVC32	(0x11)
 #define ESR_EL1_EC_SVC64	(0x15)
+#define ESR_EL2_EC_HVC64	(0x16)
+#define ESR_EL2_EC_SMC64	(0x17)
 #define ESR_EL1_EC_SYS64	(0x18)
 #define ESR_EL1_EC_IABT_EL0	(0x20)
 #define ESR_EL1_EC_IABT_EL1	(0x21)
diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 1d9223f728a5..4d81a4941959 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -6,6 +6,8 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 
+#include <linux/const.h>
+
 /* System Control Register (SCTLR_EL1) bits */
 #define SCTLR_EL1_EE	(1 << 25)
 #define SCTLR_EL1_WXN	(1 << 19)
@@ -16,6 +18,9 @@
 #define SCTLR_EL1_A	(1 << 1)
 #define SCTLR_EL1_M	(1 << 0)
 
+#define HCR_EL2_TGE	(1 << 27)
+#define HCR_EL2_E2H	(UL(1) << 34)
+
 #ifndef __ASSEMBLY__
 #include <asm/ptrace.h>
 #include <asm/esr.h>
diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
index a03830bceb8f..ed407f93330d 100644
--- a/lib/arm64/asm/sysreg.h
+++ b/lib/arm64/asm/sysreg.h
@@ -11,6 +11,14 @@
 #define sys_reg(op0, op1, crn, crm, op2) \
 	((((op0)&3)<<19)|((op1)<<16)|((crn)<<12)|((crm)<<8)|((op2)<<5))
 
+#define SYS_CNTP_TVAL_EL02	sys_reg(3, 5, 14, 2, 0)
+#define SYS_CNTP_CTL_EL02	sys_reg(3, 5, 14, 2, 1)
+#define SYS_CNTP_CVAL_EL02	sys_reg(3, 5, 14, 2, 2)
+
+#define SYS_CNTV_TVAL_EL02	sys_reg(3, 5, 14, 3, 0)
+#define SYS_CNTV_CTL_EL02	sys_reg(3, 5, 14, 3, 1)
+#define SYS_CNTV_CVAL_EL02	sys_reg(3, 5, 14, 3, 2)
+
 #ifdef __ASSEMBLY__
 	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
 	.equ	.L__reg_num_x\num, \num
@@ -52,5 +60,17 @@ asm(
 "	.inst	0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
 "	.endm\n"
 );
+
+#define read_sysreg_s(r) ({					\
+	u64 __val;						\
+	asm volatile("mrs_s %0, " xstr(r) :  "=r" (__val));	\
+	__val;							\
+})
+
+#define write_sysreg_s(v, r) do {				\
+	u64 __val = (u64)v;					\
+	asm volatile("msr_s " xstr(r) ", %x0" : : "rZ" (__val));\
+} while (0)
+
 #endif /* __ASSEMBLY__ */
 #endif /* _ASMARM64_SYSREG_H_ */
diff --git a/lib/arm/psci.c b/lib/arm/psci.c
index c3d399064ae3..e7855795dff3 100644
--- a/lib/arm/psci.c
+++ b/lib/arm/psci.c
@@ -11,8 +11,10 @@
 #include <asm/page.h>
 #include <asm/smp.h>
 
-__attribute__((noinline))
-int psci_invoke(unsigned long function_id, unsigned long arg0,
+static int (*psci_fn)(unsigned long, unsigned long, unsigned long,
+		unsigned long);
+
+static int psci_invoke_hvc(unsigned long function_id, unsigned long arg0,
 		unsigned long arg1, unsigned long arg2)
 {
 	asm volatile(
@@ -22,13 +24,48 @@ int psci_invoke(unsigned long function_id, unsigned long arg0,
 	return function_id;
 }
 
+#ifdef __arm__
+void psci_init(void)
+{
+	psci_fn = &psci_invoke_hvc;
+}
+
 int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
 {
-#ifdef __arm__
 	return psci_invoke(PSCI_0_2_FN_CPU_ON, cpuid, entry_point, 0);
+}
 #else
+static int psci_invoke_smc(unsigned long function_id, unsigned long arg0,
+		unsigned long arg1, unsigned long arg2)
+{
+	asm volatile(
+		"smc #0"
+	: "+r" (function_id)
+	: "r" (arg0), "r" (arg1), "r" (arg2));
+	return function_id;
+}
+
+void psci_init(void)
+{
+	if (current_level() == CurrentEL_EL2)
+		psci_fn = &psci_invoke_smc;
+	else
+		psci_fn = &psci_invoke_hvc;
+}
+
+int psci_cpu_on(unsigned long cpuid, unsigned long entry_point)
+{
 	return psci_invoke(PSCI_0_2_FN64_CPU_ON, cpuid, entry_point, 0);
+}
 #endif
+
+int psci_invoke(unsigned long function_id, unsigned long arg0,
+		unsigned long arg1, unsigned long arg2)
+{
+	/* Oh-oh, some went wrong */
+	if (!psci_fn)
+		psci_init();
+	return psci_fn(function_id, arg0, arg1, arg2);
 }
 
 extern void secondary_entry(void);
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 4f02fca85607..9253d2f886d9 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -20,6 +20,7 @@
 #include <asm/thread_info.h>
 #include <asm/setup.h>
 #include <asm/page.h>
+#include <asm/psci.h>
 #include <asm/smp.h>
 
 #include "io.h"
@@ -127,6 +128,9 @@ void setup(const void *fdt)
 	u32 fdt_size;
 	int ret;
 
+	/* make sure PSCI calls work as soon as possible */
+	psci_init();
+
 	/*
 	 * Before calling mem_init we need to move the fdt and initrd
 	 * to safe locations. We move them to construct the memory
diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
index 2a024e3f4e9d..1ff997112314 100644
--- a/lib/arm64/processor.c
+++ b/lib/arm64/processor.c
@@ -42,6 +42,8 @@ static const char *ec_names[EC_MAX] = {
 	[ESR_EL1_EC_ILL_ISS]		= "ILL_ISS",
 	[ESR_EL1_EC_SVC32]		= "SVC32",
 	[ESR_EL1_EC_SVC64]		= "SVC64",
+	[ESR_EL2_EC_HVC64]		= "HVC64",
+	[ESR_EL2_EC_SMC64]		= "SMC64",
 	[ESR_EL1_EC_SYS64]		= "SYS64",
 	[ESR_EL1_EC_IABT_EL0]		= "IABT_EL0",
 	[ESR_EL1_EC_IABT_EL1]		= "IABT_EL1",
diff --git a/arm/cstart64.S b/arm/cstart64.S
index 20f832fd57f7..d4b20267a7a6 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -52,6 +52,17 @@ start:
 	b	1b
 
 1:
+	mrs	x4, CurrentEL
+	cmp	x4, CurrentEL_EL2
+	b.ne 	1f
+	mrs	x4, mpidr_el1
+	msr	vmpidr_el2, x4
+	mrs	x4, midr_el1
+	msr	vpidr_el2, x4
+	ldr	x4, =(HCR_EL2_TGE | HCR_EL2_E2H)
+	msr	hcr_el2, x4
+	isb
+1:
 	/* set up stack */
 	mov	x4, #1
 	msr	spsel, x4
@@ -102,6 +113,17 @@ get_mmu_off:
 
 .globl secondary_entry
 secondary_entry:
+	mrs	x0, CurrentEL
+	cmp	x0, CurrentEL_EL2
+	b.ne 	1f
+	mrs	x0, mpidr_el1
+	msr	vmpidr_el2, x0
+	mrs	x0, midr_el1
+	msr	vpidr_el2, x0
+	ldr	x0, =(HCR_EL2_TGE | HCR_EL2_E2H)
+	msr	hcr_el2, x0
+	isb
+1:
 	/* Enable FP/ASIMD */
 	mov	x0, #(3 << 20)
 	msr	cpacr_el1, x0
@@ -132,6 +154,12 @@ secondary_entry:
 .globl asm_cpu_psci_cpu_die
 asm_cpu_psci_cpu_die:
 	ldr	x0, =PSCI_0_2_FN_CPU_OFF
+	mrs	x9, CurrentEL
+	cmp	x9, CurrentEL_EL2
+	b.ne	1f
+	smc	#0
+	b	halt
+1:
 	hvc	#0
 	b	halt
 
diff --git a/arm/micro-bench.c b/arm/micro-bench.c
index 4612f41001c2..5469667ddfe8 100644
--- a/arm/micro-bench.c
+++ b/arm/micro-bench.c
@@ -112,6 +112,11 @@ static void hvc_exec(void)
 	asm volatile("mov w0, #0x4b000000; hvc #0" ::: "w0");
 }
 
+static void smc_exec(void)
+{
+	asm volatile("mov w0, #0x4b000000; smc #0" ::: "w0");
+}
+
 static void mmio_read_user_exec(void)
 {
 	/*
@@ -138,6 +143,8 @@ static void eoi_exec(void)
 	write_eoir(spurious_id);
 }
 
+static void exec_select(void);
+
 struct exit_test {
 	const char *name;
 	void (*prep)(void);
@@ -146,13 +153,21 @@ struct exit_test {
 };
 
 static struct exit_test tests[] = {
-	{"hvc",			NULL,		hvc_exec,		true},
+	{"hyp_call",		exec_select,	hvc_exec,		true},
 	{"mmio_read_user",	NULL,		mmio_read_user_exec,	true},
 	{"mmio_read_vgic",	NULL,		mmio_read_vgic_exec,	true},
 	{"eoi",			NULL,		eoi_exec,		true},
 	{"ipi",			ipi_prep,	ipi_exec,		true},
 };
 
+static void exec_select(void)
+{
+	if (current_level() == CurrentEL_EL2)
+		tests[0].exec = &smc_exec;
+	else
+		tests[0].exec = &hvc_exec;
+}
+
 struct ns_time {
 	uint64_t ns;
 	uint64_t ns_frac;
diff --git a/arm/selftest.c b/arm/selftest.c
index 18cc0ad8f729..9ebf4c6051b4 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -285,13 +285,14 @@ static bool check_regs(struct pt_regs *regs)
 
 static enum vector check_vector_prep(void)
 {
-	unsigned long daif;
+	unsigned long daif, mode;
 
 	if (is_user())
 		return EL0_SYNC_64;
 
 	asm volatile("mrs %0, daif" : "=r" (daif) ::);
-	expected_regs.pstate = daif | PSR_MODE_EL1h;
+	mode = current_level() == CurrentEL_EL1 ? PSR_MODE_EL1h : PSR_MODE_EL2h;
+	expected_regs.pstate = daif | mode;
 	return EL1H_SYNC;
 }
 
@@ -307,8 +308,8 @@ 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", "");
+	/* try to read an el3 sysreg from el0/1/2 */
+	test_exception("", "mrs x0, sctlr_el3", "");
 
 	install_exception_handler(v, ESR_EL1_EC_UNKNOWN, NULL);
 
@@ -406,11 +407,29 @@ static bool psci_check(void)
 		printf("bad psci device tree node\n");
 		return false;
 	}
+	if (len != 4) {
+		printf("bad psci method\n");
+		return false;
+	}
 
-	if (len < 4 || strcmp(method->data, "hvc") != 0) {
+#ifdef __arm__
+	if (strcmp(method->data, "hvc") != 0) {
 		printf("psci method must be hvc\n");
 		return false;
 	}
+#else
+	if (current_level() == CurrentEL_EL2 &&
+	    strcmp(method->data, "smc") != 0) {
+		printf("psci method must be smc\n");
+		return false;
+	}
+
+	if (current_level() == CurrentEL_EL1 &&
+	    strcmp(method->data, "hvc") != 0) {
+		printf("psci method must be hvc\n");
+		return false;
+	}
+#endif
 
 	ver = psci_invoke(PSCI_0_2_FN_PSCI_VERSION, 0, 0, 0);
 	printf("PSCI version %d.%d\n", PSCI_VERSION_MAJOR(ver),
diff --git a/arm/timer.c b/arm/timer.c
index 125b9f30ad3c..29477ee65878 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -9,6 +9,7 @@
 #include <devicetree.h>
 #include <errata.h>
 #include <asm/processor.h>
+#include <asm/sysreg.h>
 #include <asm/gic.h>
 #include <asm/io.h>
 
@@ -63,6 +64,36 @@ static void write_vtimer_ctl(u64 val)
 	write_sysreg(val, cntv_ctl_el0);
 }
 
+static u64 read_vtimer_cval_vhe(void)
+{
+	return read_sysreg_s(SYS_CNTV_CVAL_EL02);
+}
+
+static void write_vtimer_cval_vhe(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTV_CVAL_EL02);
+}
+
+static s32 read_vtimer_tval_vhe(void)
+{
+	return read_sysreg_s(SYS_CNTV_TVAL_EL02);
+}
+
+static void write_vtimer_tval_vhe(s32 val)
+{
+	write_sysreg_s(val, SYS_CNTV_TVAL_EL02);
+}
+
+static u64 read_vtimer_ctl_vhe(void)
+{
+	return read_sysreg_s(SYS_CNTV_CTL_EL02);
+}
+
+static void write_vtimer_ctl_vhe(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTV_CTL_EL02);
+}
+
 static u64 read_ptimer_counter(void)
 {
 	return read_sysreg(cntpct_el0);
@@ -98,6 +129,36 @@ static void write_ptimer_ctl(u64 val)
 	write_sysreg(val, cntp_ctl_el0);
 }
 
+static u64 read_ptimer_cval_vhe(void)
+{
+	return read_sysreg_s(SYS_CNTP_CVAL_EL02);
+}
+
+static void write_ptimer_cval_vhe(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTP_CVAL_EL02);
+}
+
+static s32 read_ptimer_tval_vhe(void)
+{
+	return read_sysreg_s(SYS_CNTP_TVAL_EL02);
+}
+
+static void write_ptimer_tval_vhe(s32 val)
+{
+	write_sysreg_s(val, SYS_CNTP_TVAL_EL02);
+}
+
+static u64 read_ptimer_ctl_vhe(void)
+{
+	return read_sysreg_s(SYS_CNTP_CTL_EL02);
+}
+
+static void write_ptimer_ctl_vhe(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTP_CTL_EL02);
+}
+
 struct timer_info {
 	u32 irq;
 	u32 irq_flags;
@@ -133,6 +194,30 @@ static struct timer_info ptimer_info = {
 	.write_ctl = write_ptimer_ctl,
 };
 
+static struct timer_info vtimer_info_vhe = {
+	.irq_received = false,
+	.read_counter = read_vtimer_counter,
+	.read_cval = read_vtimer_cval_vhe,
+	.write_cval = write_vtimer_cval_vhe,
+	.read_tval = read_vtimer_tval_vhe,
+	.write_tval = write_vtimer_tval_vhe,
+	.read_ctl = read_vtimer_ctl_vhe,
+	.write_ctl = write_vtimer_ctl_vhe,
+};
+
+static struct timer_info ptimer_info_vhe = {
+	.irq_received = false,
+	.read_counter = read_ptimer_counter,
+	.read_cval = read_ptimer_cval_vhe,
+	.write_cval = write_ptimer_cval_vhe,
+	.read_tval = read_ptimer_tval_vhe,
+	.write_tval = write_ptimer_tval_vhe,
+	.read_ctl = read_ptimer_ctl_vhe,
+	.write_ctl = write_ptimer_ctl_vhe,
+};
+
+static struct timer_info *vtimer, *ptimer;
+
 static void set_timer_irq_enabled(struct timer_info *info, bool enabled)
 {
 	u32 val = 1 << PPI(info->irq);
@@ -152,10 +237,10 @@ static void irq_handler(struct pt_regs *regs)
 	if (irqnr == GICC_INT_SPURIOUS)
 		return;
 
-	if (irqnr == PPI(vtimer_info.irq)) {
-		info = &vtimer_info;
-	} else if (irqnr == PPI(ptimer_info.irq)) {
-		info = &ptimer_info;
+	if (irqnr == PPI(vtimer->irq)) {
+		info = vtimer;
+	} else if (irqnr == PPI(ptimer->irq)) {
+		info = ptimer;
 	} else {
 		report_info("Unexpected interrupt: %d\n", irqnr);
 		return;
@@ -263,7 +348,7 @@ static void test_timer(struct timer_info *info)
 static void test_vtimer(void)
 {
 	report_prefix_push("vtimer-busy-loop");
-	test_timer(&vtimer_info);
+	test_timer(vtimer);
 	report_prefix_pop();
 }
 
@@ -273,7 +358,7 @@ static void test_ptimer(void)
 		return;
 
 	report_prefix_push("ptimer-busy-loop");
-	test_timer(&ptimer_info);
+	test_timer(ptimer);
 	report_prefix_pop();
 }
 
@@ -284,6 +369,14 @@ static void test_init(void)
 	int node, len;
 	u32 *data;
 
+	if (current_level() == CurrentEL_EL1) {
+		vtimer = &vtimer_info;
+		ptimer = &ptimer_info;
+	} else {
+		vtimer = &vtimer_info_vhe;
+		ptimer = &ptimer_info_vhe;
+	}
+
 	node = fdt_node_offset_by_compatible(fdt, -1, "arm,armv8-timer");
 	assert(node >= 0);
 	prop = fdt_get_property(fdt, node, "interrupts", &len);
@@ -291,14 +384,14 @@ static void test_init(void)
 
 	data = (u32 *)prop->data;
 	assert(fdt32_to_cpu(data[3]) == 1);
-	ptimer_info.irq = fdt32_to_cpu(data[4]);
-	ptimer_info.irq_flags = fdt32_to_cpu(data[5]);
+	ptimer->irq = fdt32_to_cpu(data[4]);
+	ptimer->irq_flags = fdt32_to_cpu(data[5]);
 	assert(fdt32_to_cpu(data[6]) == 1);
-	vtimer_info.irq = fdt32_to_cpu(data[7]);
-	vtimer_info.irq_flags = fdt32_to_cpu(data[8]);
+	vtimer->irq = fdt32_to_cpu(data[7]);
+	vtimer->irq_flags = fdt32_to_cpu(data[8]);
 
 	install_exception_handler(EL1H_SYNC, ESR_EL1_EC_UNKNOWN, ptimer_unsupported_handler);
-	read_sysreg(cntp_ctl_el0);
+	ptimer->read_ctl();
 	install_exception_handler(EL1H_SYNC, ESR_EL1_EC_UNKNOWN, NULL);
 
 	if (ptimer_unsupported && !ERRATA(7b6b46311a85)) {
@@ -332,14 +425,14 @@ static void print_timer_info(void)
 	printf("CNTFRQ_EL0   : 0x%016lx\n", read_sysreg(cntfrq_el0));
 
 	if (!ptimer_unsupported){
-		printf("CNTPCT_EL0   : 0x%016lx\n", read_sysreg(cntpct_el0));
-		printf("CNTP_CTL_EL0 : 0x%016lx\n", read_sysreg(cntp_ctl_el0));
-		printf("CNTP_CVAL_EL0: 0x%016lx\n", read_sysreg(cntp_cval_el0));
+		printf("CNTPCT_EL0   : 0x%016lx\n", ptimer->read_counter());
+		printf("CNTP_CTL_EL0 : 0x%016lx\n", ptimer->read_ctl());
+		printf("CNTP_CVAL_EL0: 0x%016lx\n", ptimer->read_cval());
 	}
 
-	printf("CNTVCT_EL0   : 0x%016lx\n", read_sysreg(cntvct_el0));
-	printf("CNTV_CTL_EL0 : 0x%016lx\n", read_sysreg(cntv_ctl_el0));
-	printf("CNTV_CVAL_EL0: 0x%016lx\n", read_sysreg(cntv_cval_el0));
+	printf("CNTVCT_EL0   : 0x%016lx\n", vtimer->read_counter());
+	printf("CNTV_CTL_EL0 : 0x%016lx\n", vtimer->read_ctl());
+	printf("CNTV_CVAL_EL0: 0x%016lx\n", vtimer->read_cval());
 }
 
 int main(int argc, char **argv)
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 12/16] arm64: timer: Add test for EL2 timers
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (10 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 11/16] lib: arm64: Run existing tests at EL2 Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 13/16] arm64: selftest: Add basic test for EL2 Alexandru Elisei
                   ` (3 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

When VHE is available, EL2 has two extra timers, the physical and virtual
EL2 timers. Extend the timer test to include them.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
I'm seeing three failed tests when running at EL2, I'm investigating those
to figure out if it's KVM or kvm-unit-tests related.

 lib/arm64/asm/sysreg.h |   8 +++
 arm/timer.c            | 150 +++++++++++++++++++++++++++++++++++++++++++++++--
 2 files changed, 154 insertions(+), 4 deletions(-)

diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
index ed407f93330d..02054fbe2763 100644
--- a/lib/arm64/asm/sysreg.h
+++ b/lib/arm64/asm/sysreg.h
@@ -19,6 +19,14 @@
 #define SYS_CNTV_CTL_EL02	sys_reg(3, 5, 14, 3, 1)
 #define SYS_CNTV_CVAL_EL02	sys_reg(3, 5, 14, 3, 2)
 
+#define SYS_CNTHP_TVAL_EL2	sys_reg(3, 4, 14, 2, 0)
+#define SYS_CNTHP_CTL_EL2	sys_reg(3, 4, 14, 2, 1)
+#define SYS_CNTHP_CVAL_EL2	sys_reg(3, 4, 14, 2, 2)
+
+#define SYS_CNTHV_TVAL_EL2	sys_reg(3, 4, 14, 3, 0)
+#define SYS_CNTHV_CTL_EL2	sys_reg(3, 4, 14, 3, 1)
+#define SYS_CNTHV_CVAL_EL2	sys_reg(3, 4, 14, 3, 2)
+
 #ifdef __ASSEMBLY__
 	.irp	num,0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30
 	.equ	.L__reg_num_x\num, \num
diff --git a/arm/timer.c b/arm/timer.c
index 29477ee65878..faab671d0fb1 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -23,6 +23,8 @@ static void *gic_icenabler;
 
 static bool ptimer_unsupported;
 
+static int current_el;
+
 static void ptimer_unsupported_handler(struct pt_regs *regs, unsigned int esr)
 {
 	ptimer_unsupported = true;
@@ -159,6 +161,66 @@ static void write_ptimer_ctl_vhe(u64 val)
 	write_sysreg_s(val, SYS_CNTP_CTL_EL02);
 }
 
+static u64 read_hvtimer_cval(void)
+{
+	return read_sysreg_s(SYS_CNTHV_CVAL_EL2);
+}
+
+static void write_hvtimer_cval(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTHV_CVAL_EL2);
+}
+
+static s32 read_hvtimer_tval(void)
+{
+	return read_sysreg_s(SYS_CNTHV_TVAL_EL2);
+}
+
+static void write_hvtimer_tval(s32 val)
+{
+	write_sysreg_s(val, SYS_CNTHV_TVAL_EL2);
+}
+
+static u64 read_hvtimer_ctl(void)
+{
+	return read_sysreg_s(SYS_CNTHV_CTL_EL2);
+}
+
+static void write_hvtimer_ctl(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTHV_CTL_EL2);
+}
+
+static u64 read_hptimer_cval(void)
+{
+	return read_sysreg_s(SYS_CNTHP_CVAL_EL2);
+}
+
+static void write_hptimer_cval(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTHP_CVAL_EL2);
+}
+
+static s32 read_hptimer_tval(void)
+{
+	return read_sysreg_s(SYS_CNTHP_TVAL_EL2);
+}
+
+static void write_hptimer_tval(s32 val)
+{
+	write_sysreg_s(val, SYS_CNTHP_TVAL_EL2);
+}
+
+static u64 read_hptimer_ctl(void)
+{
+	return read_sysreg_s(SYS_CNTHP_CTL_EL2);
+}
+
+static void write_hptimer_ctl(u64 val)
+{
+	write_sysreg_s(val, SYS_CNTHP_CTL_EL2);
+}
+
 struct timer_info {
 	u32 irq;
 	u32 irq_flags;
@@ -216,7 +278,29 @@ static struct timer_info ptimer_info_vhe = {
 	.write_ctl = write_ptimer_ctl_vhe,
 };
 
-static struct timer_info *vtimer, *ptimer;
+static struct timer_info hvtimer_info = {
+	.irq_received = false,
+	.read_counter = read_vtimer_counter,
+	.read_cval = read_hvtimer_cval,
+	.write_cval = write_hvtimer_cval,
+	.read_tval = read_hvtimer_tval,
+	.write_tval = write_hvtimer_tval,
+	.read_ctl = read_hvtimer_ctl,
+	.write_ctl = write_hvtimer_ctl,
+};
+
+static struct timer_info hptimer_info = {
+	.irq_received = false,
+	.read_counter = read_ptimer_counter,
+	.read_cval = read_hptimer_cval,
+	.write_cval = write_hptimer_cval,
+	.read_tval = read_hptimer_tval,
+	.write_tval = write_hptimer_tval,
+	.read_ctl = read_hptimer_ctl,
+	.write_ctl = write_hptimer_ctl,
+};
+
+static struct timer_info *vtimer, *ptimer, *hvtimer, *hptimer;
 
 static void set_timer_irq_enabled(struct timer_info *info, bool enabled)
 {
@@ -241,6 +325,10 @@ static void irq_handler(struct pt_regs *regs)
 		info = vtimer;
 	} else if (irqnr == PPI(ptimer->irq)) {
 		info = ptimer;
+	} else if (current_el == CurrentEL_EL2 && irqnr == PPI(hptimer->irq)) {
+		info = hptimer;
+	} else if (current_el == CurrentEL_EL2 && irqnr == PPI(hvtimer->irq)) {
+		info = hvtimer;
 	} else {
 		report_info("Unexpected interrupt: %d\n", irqnr);
 		return;
@@ -362,6 +450,20 @@ static void test_ptimer(void)
 	report_prefix_pop();
 }
 
+static void test_hvtimer(void)
+{
+	report_prefix_push("hvtimer-busy-loop");
+	test_timer(hvtimer);
+	report_prefix_pop();
+}
+
+static void test_hptimer(void)
+{
+	report_prefix_push("hptimer-busy-loop");
+	test_timer(hptimer);
+	report_prefix_pop();
+}
+
 static void test_init(void)
 {
 	const struct fdt_property *prop;
@@ -369,12 +471,14 @@ static void test_init(void)
 	int node, len;
 	u32 *data;
 
-	if (current_level() == CurrentEL_EL1) {
+	if (current_el == CurrentEL_EL1) {
 		vtimer = &vtimer_info;
 		ptimer = &ptimer_info;
 	} else {
 		vtimer = &vtimer_info_vhe;
 		ptimer = &ptimer_info_vhe;
+		hvtimer = &hvtimer_info;
+		hptimer = &hptimer_info;
 	}
 
 	node = fdt_node_offset_by_compatible(fdt, -1, "arm,armv8-timer");
@@ -390,6 +494,19 @@ static void test_init(void)
 	vtimer->irq = fdt32_to_cpu(data[7]);
 	vtimer->irq_flags = fdt32_to_cpu(data[8]);
 
+	if (current_el == CurrentEL_EL2) {
+		assert(fdt32_to_cpu(data[9]) == 1);
+		hptimer->irq = fdt32_to_cpu(data[10]);
+		hptimer->irq_flags = fdt32_to_cpu(data[11]);
+		/* The hvtimer is not in the DT, assume KVM default. */
+		hvtimer->irq = 28;
+		/*
+		 * With VHE, accesses to the vtimer are redirected to the
+		 * hvtimer. They should have the same interrupt properties.
+		 */
+		hvtimer->irq_flags = vtimer->irq_flags;
+	}
+
 	install_exception_handler(EL1H_SYNC, ESR_EL1_EC_UNKNOWN, ptimer_unsupported_handler);
 	ptimer->read_ctl();
 	install_exception_handler(EL1H_SYNC, ESR_EL1_EC_UNKNOWN, NULL);
@@ -433,12 +550,22 @@ static void print_timer_info(void)
 	printf("CNTVCT_EL0   : 0x%016lx\n", vtimer->read_counter());
 	printf("CNTV_CTL_EL0 : 0x%016lx\n", vtimer->read_ctl());
 	printf("CNTV_CVAL_EL0: 0x%016lx\n", vtimer->read_cval());
+
+	if (current_el == CurrentEL_EL2) {
+		printf("CNTHP_CTL_EL0 : 0x%016lx\n", hptimer->read_ctl());
+		printf("CNTHP_CVAL_EL0: 0x%016lx\n", hptimer->read_cval());
+
+		printf("CNTHV_CTL_EL0 : 0x%016lx\n", hvtimer->read_ctl());
+		printf("CNTHV_CVAL_EL0: 0x%016lx\n", hvtimer->read_cval());
+	}
 }
 
 int main(int argc, char **argv)
 {
 	int i;
 
+	current_el = current_level();
+
 	test_init();
 
 	print_timer_info();
@@ -446,13 +573,28 @@ int main(int argc, char **argv)
 	if (argc == 1) {
 		test_vtimer();
 		test_ptimer();
+		if (current_el == CurrentEL_EL2) {
+			test_hvtimer();
+			test_hptimer();
+		}
 	}
 
 	for (i = 1; i < argc; ++i) {
-		if (strcmp(argv[i], "vtimer") == 0)
+		if (strcmp(argv[i], "vtimer") == 0) {
 			test_vtimer();
-		if (strcmp(argv[i], "ptimer") == 0)
+		} if (strcmp(argv[i], "ptimer") == 0) {
 			test_ptimer();
+		} if (strcmp(argv[i], "hvtimer") == 0) {
+			if (current_el == CurrentEL_EL1)
+				report_info("Skipping hvtimer tests. Boot at EL2 to enable.");
+			else
+				test_hvtimer();
+		} if (strcmp(argv[i], "hptimer") == 0) {
+			if (current_el == CurrentEL_EL1)
+				report_info("Skipping hptimer tests. Boot at EL2 to enable.");
+			else
+				test_hptimer();
+		}
 	}
 
 	return report_summary();
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 13/16] arm64: selftest: Add basic test for EL2
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (11 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 12/16] arm64: timer: Add test for EL2 timers Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
                   ` (2 subsequent siblings)
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Add a rudimentary test for EL2 that checks that we are indeed running with
VHE enabled and that we are using SMC for issuing PSCI calls.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm64/asm/processor.h |  6 ++++++
 arm/selftest.c            | 36 +++++++++++++++++++++++++++++++++++-
 arm/unittests.cfg         |  8 ++++++++
 3 files changed, 49 insertions(+), 1 deletion(-)

diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 4d81a4941959..b9058f140039 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -110,5 +110,11 @@ static inline u32 get_cntfrq(void)
 	return read_sysreg(cntfrq_el0);
 }
 
+static inline bool vhe_enabled(void)
+{
+	unsigned long hcr = read_sysreg(hcr_el2);
+	return (hcr & HCR_EL2_E2H) && (hcr & HCR_EL2_TGE);
+}
+
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASMARM64_PROCESSOR_H_ */
diff --git a/arm/selftest.c b/arm/selftest.c
index 9ebf4c6051b4..211bc8a5642b 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -19,6 +19,9 @@
 #include <asm/mmu.h>
 #include <asm/pgtable.h>
 
+#define ID_AA64MMFR1_VHE_MASK		(0xf << 8)
+#define ID_AA64MMFR1_VHE_SUPPORTED	(1 << 8)
+
 static void __user_psci_system_off(void)
 {
 	psci_system_off();
@@ -223,6 +226,11 @@ static void check_pabt(void)
 	test_exception("mov r2, #0x0", "bx r2", "");
 	__builtin_unreachable();
 }
+
+static void check_el2(void)
+{
+	report("EL2 only available on arm64", false);
+}
 #elif defined(__aarch64__)
 
 /*
@@ -372,6 +380,29 @@ static void user_psci_system_off(struct pt_regs *regs, unsigned int esr)
 {
 	__user_psci_system_off();
 }
+
+static bool vhe_supported(void)
+{
+	u64 aa64mmfr1 = read_sysreg(id_aa64mmfr1_el1);
+
+	return (aa64mmfr1 & ID_AA64MMFR1_VHE_MASK) == ID_AA64MMFR1_VHE_SUPPORTED;
+}
+
+static void check_el2_cpu(void *data __unused)
+{
+	int cpu = smp_processor_id();
+
+	report("CPU(%3d) Running at EL2", current_level() == CurrentEL_EL2, cpu);
+	report("CPU(%3d) VHE supported and enabled",
+			vhe_supported() && vhe_enabled(), cpu);
+}
+
+static bool psci_check(void);
+static void check_el2(void)
+{
+	report("PSCI conduit", psci_check());
+	on_cpus(check_el2_cpu, NULL);
+}
 #endif
 
 static void check_vectors(void *arg __unused)
@@ -423,7 +454,6 @@ static bool psci_check(void)
 		printf("psci method must be smc\n");
 		return false;
 	}
-
 	if (current_level() == CurrentEL_EL1 &&
 	    strcmp(method->data, "hvc") != 0) {
 		printf("psci method must be hvc\n");
@@ -474,6 +504,10 @@ int main(int argc, char **argv)
 		report("PSCI version", psci_check());
 		on_cpus(cpu_report, NULL);
 
+	} else if (strcmp(argv[1], "el2") == 0) {
+
+		check_el2();
+
 	} else {
 		printf("Unknown subtest\n");
 		abort();
diff --git a/arm/unittests.cfg b/arm/unittests.cfg
index 6d3df92a4e28..c632f4e75382 100644
--- a/arm/unittests.cfg
+++ b/arm/unittests.cfg
@@ -55,6 +55,14 @@ smp = $MAX_SMP
 extra_params = -append 'smp'
 groups = selftest
 
+# Test EL2 support
+[selftest-el2]
+file = selftest.flat
+smp = 2
+extra_params = -append 'el2'
+groups = selftest
+arch = arm64
+
 # Test PCI emulation
 [pci-test]
 file = pci-test.flat
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (12 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 13/16] arm64: selftest: Add basic test for EL2 Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 14:19   ` Mark Rutland
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 15/16] arm64: selftest: Expand EL2 test to disable and re-enable VHE Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 16/16] arm64: timer: Run tests with VHE disabled Alexandru Elisei
  15 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Add a function to disable VHE and another one to re-enable VHE. Both
functions work under the assumption that the CPU had VHE mode enabled at
boot.

Minimal support to run with VHE has been added to the TLB invalidate
functions and to the exception handling code.

Since we're touch the assembly enable/disable MMU code, let's take this
opportunity to replace a magic number with the proper define.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm/asm/processor.h       |   8 ++
 lib/arm64/asm/mmu.h           |  11 ++-
 lib/arm64/asm/pgtable-hwdef.h |  53 +++++++++---
 lib/arm64/asm/processor.h     |  44 +++++++++-
 lib/arm/processor.c           |  11 +++
 lib/arm/setup.c               |   2 +
 lib/arm64/processor.c         |  67 ++++++++++++++-
 arm/cstart64.S                | 186 +++++++++++++++++++++++++++++++++++++++++-
 8 files changed, 364 insertions(+), 18 deletions(-)

diff --git a/lib/arm/asm/processor.h b/lib/arm/asm/processor.h
index a8c4628da818..d5df869b2e6f 100644
--- a/lib/arm/asm/processor.h
+++ b/lib/arm/asm/processor.h
@@ -9,6 +9,11 @@
 #include <asm/sysreg.h>
 #include <asm/barrier.h>
 
+#define CTR_DMINLINE_SHIFT	16
+#define CTR_DMINLINE_MASK	(0xf << 16)
+#define CTR_DMINLINE(x)	\
+	(((x) & CTR_DMINLINE_MASK) >> CTR_DMINLINE_SHIFT)
+
 enum vector {
 	EXCPTN_RST,
 	EXCPTN_UND,
@@ -26,6 +31,9 @@ extern void install_exception_handler(enum vector v, exception_fn fn);
 
 extern void show_regs(struct pt_regs *regs);
 
+extern unsigned int dcache_line_size;
+extern void set_dcache_line_size(void);
+
 static inline unsigned long current_cpsr(void)
 {
 	unsigned long cpsr;
diff --git a/lib/arm64/asm/mmu.h b/lib/arm64/asm/mmu.h
index fa554b0c20ae..6ce6c8958910 100644
--- a/lib/arm64/asm/mmu.h
+++ b/lib/arm64/asm/mmu.h
@@ -6,6 +6,7 @@
  * This work is licensed under the terms of the GNU LGPL, version 2.
  */
 #include <asm/barrier.h>
+#include <asm/processor.h>
 
 #define PMD_SECT_UNCACHED	PMD_ATTRINDX(MT_DEVICE_nGnRE)
 #define PTE_WBWA		PTE_ATTRINDX(MT_NORMAL)
@@ -13,7 +14,10 @@
 static inline void flush_tlb_all(void)
 {
 	dsb(ishst);
-	asm("tlbi	vmalle1is");
+	if (current_level() == CurrentEL_EL2 && !cpu_el2_e2h_is_set())
+		asm("tlbi	alle2is");
+	else
+		asm("tlbi	vmalle1is");
 	dsb(ish);
 	isb();
 }
@@ -22,7 +26,10 @@ static inline void flush_tlb_page(unsigned long vaddr)
 {
 	unsigned long page = vaddr >> 12;
 	dsb(ishst);
-	asm("tlbi	vaae1is, %0" :: "r" (page));
+	if (current_level() == CurrentEL_EL2 && !cpu_el2_e2h_is_set())
+		asm("tlbi	vae2is, %0" :: "r" (page));
+	else
+		asm("tlbi	vaae1is, %0" :: "r" (page));
 	dsb(ish);
 }
 
diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
index e6f02fae4075..83364ccc28da 100644
--- a/lib/arm64/asm/pgtable-hwdef.h
+++ b/lib/arm64/asm/pgtable-hwdef.h
@@ -93,18 +93,42 @@
 /*
  * TCR flags.
  */
-#define TCR_TxSZ(x)		(((UL(64) - (x)) << 16) | ((UL(64) - (x)) << 0))
-#define TCR_IRGN_NC		((UL(0) << 8) | (UL(0) << 24))
-#define TCR_IRGN_WBWA		((UL(1) << 8) | (UL(1) << 24))
-#define TCR_IRGN_WT		((UL(2) << 8) | (UL(2) << 24))
-#define TCR_IRGN_WBnWA		((UL(3) << 8) | (UL(3) << 24))
-#define TCR_IRGN_MASK		((UL(3) << 8) | (UL(3) << 24))
-#define TCR_ORGN_NC		((UL(0) << 10) | (UL(0) << 26))
-#define TCR_ORGN_WBWA		((UL(1) << 10) | (UL(1) << 26))
-#define TCR_ORGN_WT		((UL(2) << 10) | (UL(2) << 26))
-#define TCR_ORGN_WBnWA		((UL(3) << 10) | (UL(3) << 26))
-#define TCR_ORGN_MASK		((UL(3) << 10) | (UL(3) << 26))
-#define TCR_SHARED		((UL(3) << 12) | (UL(3) << 28))
+#define TCR_T0SZ(x)		((UL(64) - (x)) << 0)
+#define TCR_T1SZ(x)		((UL(64) - (x)) << 16)
+#define TCR_TxSZ(x)		(TCR_T0SZ(x) | TCR_T1SZ(x))
+#define TCR_IRGN0_NC		(UL(0) << 8)
+#define TCR_IRGN1_NC		(UL(0) << 24)
+#define TCR_IRGN_NC		(TCR_IRGN0_NC | TCR_IRGN1_NC)
+#define TCR_IRGN0_WBWA		(UL(1) << 8)
+#define TCR_IRGN1_WBWA		(UL(1) << 24)
+#define TCR_IRGN_WBWA		(TCR_IRGN0_WBWA | TCR_IRGN1_WBWA)
+#define TCR_IRGN0_WT		(UL(2) << 8)
+#define TCR_IRGN1_WT		(UL(2) << 24)
+#define TCR_IRGN_WT		(TCR_IRGN0_WT | TCR_IRGN1_WT)
+#define TCR_IRGN0_WBnWA		(UL(3) << 8)
+#define TCR_IRGN1_WBnWA		(UL(3) << 24)
+#define TCR_IRGN_WBnWA		(TCR_IRGN0_WBnWA | TCR_IRGN1_WBnWA)
+#define TCR_IRGN0_MASK		(UL(3) << 8)
+#define TCR_IRGN1_MASK		(UL(3) << 24)
+#define TCR_IRGN_MASK		(TCR_IRGN0_MASK | TCR_IRGN1_MASK)
+#define TCR_ORGN0_NC		(UL(0) << 10)
+#define TCR_ORGN1_NC		(UL(0) << 26)
+#define TCR_ORGN_NC		(TCR_ORGN0_NC | TCR_ORGN1_NC)
+#define TCR_ORGN0_WBWA		(UL(1) << 10)
+#define TCR_ORGN1_WBWA		(UL(1) << 26)
+#define TCR_ORGN_WBWA		(TCR_ORGN0_WBWA | TCR_ORGN1_WBWA)
+#define TCR_ORGN0_WT		(UL(2) << 10)
+#define TCR_ORGN1_WT		(UL(2) << 26)
+#define TCR_ORGN_WT		(TCR_ORGN0_WT | TCR_ORGN1_WT)
+#define TCR_ORGN0_WBnWA		(UL(3) << 8)
+#define TCR_ORGN1_WBnWA		(UL(3) << 24)
+#define TCR_ORGN_WBnWA		(TCR_ORGN0_WBnWA | TCR_ORGN1_WBnWA)
+#define TCR_ORGN0_MASK		(UL(3) << 10)
+#define TCR_ORGN1_MASK		(UL(3) << 26)
+#define TCR_ORGN_MASK		(TCR_ORGN0_MASK | TCR_ORGN1_MASK)
+#define TCR_SH0_IS		(UL(3) << 12)
+#define TCR_SH1_IS		(UL(3) << 28)
+#define TCR_SHARED		(TCR_SH0_IS | TCR_SH1_IS)
 #define TCR_TG0_4K		(UL(0) << 14)
 #define TCR_TG0_64K		(UL(1) << 14)
 #define TCR_TG0_16K		(UL(2) << 14)
@@ -114,6 +138,11 @@
 #define TCR_ASID16		(UL(1) << 36)
 #define TCR_TBI0		(UL(1) << 37)
 
+#define TCR_EL1_IPS_SHIFT	32
+
+#define TCR_EL2_RES1		((UL(1) << 31) | (UL(1) << 23))
+#define TCR_EL2_PS_SHIFT	16
+
 /*
  * Memory types available.
  */
diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index b9058f140039..4d12913ca01f 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -18,8 +18,20 @@
 #define SCTLR_EL1_A	(1 << 1)
 #define SCTLR_EL1_M	(1 << 0)
 
-#define HCR_EL2_TGE	(1 << 27)
-#define HCR_EL2_E2H	(UL(1) << 34)
+#define CTR_EL0_DMINLINE_SHIFT	16
+#define CTR_EL0_DMINLINE_MASK	(0xf << 16)
+#define CTR_EL0_DMINLINE(x)	\
+	(((x) & CTR_EL0_DMINLINE_MASK) >> CTR_EL0_DMINLINE_SHIFT)
+
+#define HCR_EL2_TGE		(1 << 27)
+#define HCR_EL2_E2H_SHIFT	34
+#define HCR_EL2_E2H		(UL(1) << 34)
+
+#define SCTLR_EL2_RES1		(3 << 28 | 3 << 22 | 1 << 18 |	\
+				 1 << 16 | 1 << 11 | 3 << 4)
+#define SCTLR_EL2_I		SCTLR_EL1_I
+#define SCTLR_EL2_C		SCTLR_EL1_C
+#define SCTLR_EL2_M		SCTLR_EL1_M
 
 #ifndef __ASSEMBLY__
 #include <asm/ptrace.h>
@@ -66,6 +78,12 @@ extern void vector_handlers_default_init(vector_fn *handlers);
 extern void show_regs(struct pt_regs *regs);
 extern bool get_far(unsigned int esr, unsigned long *far);
 
+extern unsigned int dcache_line_size;
+extern void set_dcache_line_size(void);
+
+extern void disable_vhe(void);
+extern void enable_vhe(void);
+
 static inline unsigned long current_level(void)
 {
 	unsigned long el;
@@ -116,5 +134,27 @@ static inline bool vhe_enabled(void)
 	return (hcr & HCR_EL2_E2H) && (hcr & HCR_EL2_TGE);
 }
 
+static inline bool cpu_el2_e2h_is_set(void)
+{
+	return read_sysreg(hcr_el2) & HCR_EL2_E2H;
+}
+
+#define dcache_by_line_op(op, start, end)	\
+	asm volatile(	"1:\n"			\
+			"dc	" #op ", %0\n"	\
+			"add	%0, %0, %2\n"	\
+			"cmp	%0, %1\n"	\
+			"b.lo	1b\n"		\
+			"dsb	ish\n"		\
+			:: "r" (start),		\
+			   "r" (end),		\
+			   "r" (dcache_line_size))
+
+#define dcache_inval_range(start, end)		\
+	dcache_by_line_op(ivac, start, end)
+
+#define dcache_clean_inval_range(start, end)	\
+	dcache_by_line_op(civac, start, end)
+
 #endif /* !__ASSEMBLY__ */
 #endif /* _ASMARM64_PROCESSOR_H_ */
diff --git a/lib/arm/processor.c b/lib/arm/processor.c
index 773337e6d3b7..927b77041e29 100644
--- a/lib/arm/processor.c
+++ b/lib/arm/processor.c
@@ -10,6 +10,8 @@
 #include <asm/processor.h>
 #include <asm/thread_info.h>
 
+unsigned int dcache_line_size;
+
 static const char *processor_modes[] = {
 	"USER_26", "FIQ_26" , "IRQ_26" , "SVC_26" ,
 	"UK4_26" , "UK5_26" , "UK6_26" , "UK7_26" ,
@@ -145,3 +147,12 @@ bool is_user(void)
 {
 	return current_thread_info()->flags & TIF_USER_MODE;
 }
+
+void set_dcache_line_size(void)
+{
+	u32 ctr;
+
+	asm volatile("mrc p15, 0, %0, c0, c0, 1" : "=r" (ctr));
+	/* DminLine is log2 of the number of words in the smallest cache line */
+	dcache_line_size = (1 << CTR_DMINLINE(ctr)) * 4;
+}
diff --git a/lib/arm/setup.c b/lib/arm/setup.c
index 9253d2f886d9..1d4f35429740 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -22,6 +22,7 @@
 #include <asm/page.h>
 #include <asm/psci.h>
 #include <asm/smp.h>
+#include <asm/processor.h>
 
 #include "io.h"
 
@@ -64,6 +65,7 @@ static void cpu_init(void)
 	ret = dt_for_each_cpu_node(cpu_set, NULL);
 	assert(ret == 0);
 	set_cpu_online(0, true);
+	set_dcache_line_size();
 }
 
 static void mem_init(phys_addr_t freemem_start)
diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
index 1ff997112314..d343798f7613 100644
--- a/lib/arm64/processor.c
+++ b/lib/arm64/processor.c
@@ -10,6 +10,8 @@
 #include <asm/processor.h>
 #include <asm/thread_info.h>
 
+unsigned int dcache_line_size;
+
 static const char *vector_names[] = {
 	"el1t_sync",
 	"el1t_irq",
@@ -84,7 +86,10 @@ bool get_far(unsigned int esr, unsigned long *far)
 {
 	unsigned int ec = esr >> ESR_EL1_EC_SHIFT;
 
-	asm volatile("mrs %0, far_el1": "=r" (*far));
+	if (current_level() == CurrentEL_EL2 && !cpu_el2_e2h_is_set())
+		asm volatile("mrs %0, far_el2": "=r" (*far));
+	else
+		asm volatile("mrs %0, far_el1": "=r" (*far));
 
 	switch (ec) {
 	case ESR_EL1_EC_IABT_EL0:
@@ -259,3 +264,63 @@ bool is_user(void)
 {
 	return current_thread_info()->flags & TIF_USER_MODE;
 }
+
+void set_dcache_line_size(void)
+{
+	u64 ctr;
+
+	ctr = read_sysreg(ctr_el0);
+	/* DminLine is log2 of the number of words in the smallest cache line */
+	dcache_line_size = (1 << CTR_EL0_DMINLINE(ctr)) * 4;
+}
+
+extern void asm_disable_vhe(void);
+void disable_vhe(void)
+{
+	u64 sp, sp_phys, sp_base, sp_base_phys;
+
+	assert(current_level() == CurrentEL_EL2 && vhe_enabled());
+
+	sp = current_stack_pointer;
+	sp_phys = __virt_to_phys(sp);
+	sp_base = sp & THREAD_MASK;
+	sp_base_phys = sp_phys & THREAD_MASK;
+
+	/*
+	 * We will disable, then enable the MMU, make sure the exception
+	 * handling code works during the small window of time when the MMU is
+	 * off.
+	 */
+	dcache_clean_inval_range(sp_base, sp_base + THREAD_SIZE);
+	dcache_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
+	asm volatile(	"mov	sp, %0\n" : :"r" (sp_phys));
+
+	asm_disable_vhe();
+
+	dcache_clean_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
+	dcache_inval_range(sp_base, sp_base + THREAD_SIZE);
+	asm volatile(	"mov	sp, %0\n" : :"r" (sp));
+}
+
+extern void asm_enable_vhe(void);
+void enable_vhe(void)
+{
+	u64 sp, sp_phys, sp_base, sp_base_phys;
+
+	assert(current_level() == CurrentEL_EL2 && !vhe_enabled());
+
+	sp = current_stack_pointer;
+	sp_phys = __virt_to_phys(sp);
+	sp_base = sp & THREAD_MASK;
+	sp_base_phys = sp_phys & THREAD_MASK;
+
+	dcache_clean_inval_range(sp_base, sp_base + THREAD_SIZE);
+	dcache_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
+	asm volatile(	"mov	sp, %0\n" : :"r" (sp_phys));
+
+	asm_enable_vhe();
+
+	dcache_clean_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
+	dcache_inval_range(sp_base, sp_base + THREAD_SIZE);
+	asm volatile(	"mov	sp, %0\n" : :"r" (sp));
+}
diff --git a/arm/cstart64.S b/arm/cstart64.S
index d4b20267a7a6..dc9e634e2307 100644
--- a/arm/cstart64.S
+++ b/arm/cstart64.S
@@ -104,6 +104,13 @@ exceptions_init:
 
 .text
 
+exceptions_init_nvhe:
+	adrp	x0, vector_table_nvhe
+	add	x0, x0, :lo12:vector_table_nvhe
+	msr	vbar_el2, x0
+	isb
+	ret
+
 .globl get_mmu_off
 get_mmu_off:
 	adrp	x0, auxinfo
@@ -204,7 +211,7 @@ asm_mmu_enable:
 		     TCR_IRGN_WBWA | TCR_ORGN_WBWA |	\
 		     TCR_SHARED
 	mrs	x2, id_aa64mmfr0_el1
-	bfi	x1, x2, #32, #3
+	bfi	x1, x2, #TCR_EL1_IPS_SHIFT, #3
 	msr	tcr_el1, x1
 
 	/* MAIR */
@@ -229,6 +236,33 @@ asm_mmu_enable:
 
 	ret
 
+asm_mmu_enable_nvhe:
+	ic      iallu
+	tlbi    alle2is
+	dsb     ish
+
+        /* TCR */
+	ldr	x1, =TCR_EL2_RES1 | 			\
+		     TCR_T0SZ(VA_BITS) |		\
+		     TCR_TG0_64K |                      \
+		     TCR_IRGN0_WBWA | TCR_ORGN0_WBWA |	\
+		     TCR_SH0_IS
+	mrs	x2, id_aa64mmfr0_el1
+	bfi	x1, x2, #TCR_EL2_PS_SHIFT, #3
+	msr	tcr_el2, x1
+
+	/* Same MAIR and TTBR0 as in VHE mode */
+
+	/* SCTLR */
+	ldr	x1, =SCTLR_EL2_RES1 |			\
+		     SCTLR_EL2_C | 			\
+		     SCTLR_EL2_I | 			\
+		     SCTLR_EL2_M
+	msr	sctlr_el2, x1
+	isb
+
+	ret
+
 .globl asm_mmu_disable
 asm_mmu_disable:
 	mrs	x0, sctlr_el1
@@ -237,6 +271,48 @@ asm_mmu_disable:
 	isb
 	ret
 
+asm_mmu_disable_nvhe:
+	mrs	x0, sctlr_el2
+	bic	x0, x0, SCTLR_EL2_M
+	msr	sctlr_el2, x0
+	isb
+	ret
+
+.globl asm_disable_vhe
+asm_disable_vhe:
+	str	x30, [sp, #-16]!
+
+	bl	asm_mmu_disable
+	/*
+	 * This goes *before* disabling VHE because using the _EL2 registers is
+	 * always correct, but using _EL1 registers is not when VHE is off.
+	 */
+	bl	exceptions_init_nvhe
+	msr	hcr_el2, xzr
+	isb
+	bl	asm_mmu_enable_nvhe
+
+	ldr	x30, [sp], #16
+	ret
+
+.globl asm_enable_vhe
+asm_enable_vhe:
+	str	x30, [sp, #-16]!
+
+	bl	asm_mmu_disable_nvhe
+	ldr	x0, =(HCR_EL2_E2H | HCR_EL2_TGE)
+	msr	hcr_el2, x0
+	isb
+	/* This goes *after* enabling VHE because it uses the _EL1 registers */
+	bl	exceptions_init
+	/* Make asm_mmu_enable happy by having TTBR0 value in x0 */
+	mrs	x0, ttbr0_el2
+	isb
+	bl	asm_mmu_enable
+
+	ldr	x30, [sp], #16
+	ret
+
 /*
  * Vectors
  * Adapted from arch/arm64/kernel/entry.S
@@ -327,6 +403,92 @@ vector_stub	el0_irq_32,   13
 vector_stub	el0_fiq_32,   14
 vector_stub	el0_error_32, 15
 
+.macro vector_stub_nvhe, name, vec
+\name:
+	stp	 x0,  x1, [sp, #-S_FRAME_SIZE]!
+	stp	 x2,  x3, [sp,  #16]
+	stp	 x4,  x5, [sp,  #32]
+	stp	 x6,  x7, [sp,  #48]
+	stp	 x8,  x9, [sp,  #64]
+	stp	x10, x11, [sp,  #80]
+	stp	x12, x13, [sp,  #96]
+	stp	x14, x15, [sp, #112]
+	stp	x16, x17, [sp, #128]
+	stp	x18, x19, [sp, #144]
+	stp	x20, x21, [sp, #160]
+	stp	x22, x23, [sp, #176]
+	stp	x24, x25, [sp, #192]
+	stp	x26, x27, [sp, #208]
+	stp	x28, x29, [sp, #224]
+
+	str	x30, [sp, #S_LR]
+
+	.if \vec >= 8
+	mrs	x1, sp_el1
+	.else
+	add	x1, sp, #S_FRAME_SIZE
+	.endif
+	str	x1, [sp, #S_SP]
+
+	mrs	x1, elr_el2
+	mrs	x2, spsr_el2
+	stp	x1, x2, [sp, #S_PC]
+
+	mov	x0, \vec
+	mov	x1, sp
+	mrs	x2, esr_el2
+	bl	do_handle_exception
+
+	ldp	x1, x2, [sp, #S_PC]
+	msr	spsr_el2, x2
+	msr	elr_el2, x1
+
+	.if \vec >= 8
+	ldr	x1, [sp, #S_SP]
+	msr	sp_el1, x1
+	.endif
+
+	ldr	x30, [sp, #S_LR]
+
+	ldp	x28, x29, [sp, #224]
+	ldp	x26, x27, [sp, #208]
+	ldp	x24, x25, [sp, #192]
+	ldp	x22, x23, [sp, #176]
+	ldp	x20, x21, [sp, #160]
+	ldp	x18, x19, [sp, #144]
+	ldp	x16, x17, [sp, #128]
+	ldp	x14, x15, [sp, #112]
+	ldp	x12, x13, [sp,  #96]
+	ldp	x10, x11, [sp,  #80]
+	ldp	 x8,  x9, [sp,  #64]
+	ldp	 x6,  x7, [sp,  #48]
+	ldp	 x4,  x5, [sp,  #32]
+	ldp	 x2,  x3, [sp,  #16]
+	ldp	 x0,  x1, [sp], #S_FRAME_SIZE
+
+	eret
+.endm
+
+vector_stub_nvhe	el2t_sync,     0
+vector_stub_nvhe	el2t_irq,      1
+vector_stub_nvhe	el2t_fiq,      2
+vector_stub_nvhe	el2t_error,    3
+
+vector_stub_nvhe	el2h_sync,     4
+vector_stub_nvhe	el2h_irq,      5
+vector_stub_nvhe	el2h_fiq,      6
+vector_stub_nvhe	el2h_error,    7
+
+vector_stub_nvhe	el1_sync_64,   8
+vector_stub_nvhe	el1_irq_64,    9
+vector_stub_nvhe	el1_fiq_64,   10
+vector_stub_nvhe	el1_error_64, 11
+
+vector_stub_nvhe	el1_sync_32,  12
+vector_stub_nvhe	el1_irq_32,   13
+vector_stub_nvhe	el1_fiq_32,   14
+vector_stub_nvhe	el1_error_32, 15
+
 .section .text.ex
 
 .macro ventry, label
@@ -355,3 +517,25 @@ vector_table:
 	ventry	el0_irq_32			// IRQ 32-bit EL0
 	ventry	el0_fiq_32			// FIQ 32-bit EL0
 	ventry	el0_error_32			// Error 32-bit EL0
+
+.align 11
+vector_table_nvhe:
+	ventry	el2t_sync			// Synchronous EL2t
+	ventry	el2t_irq			// IRQ EL2t
+	ventry	el2t_fiq			// FIQ EL2t
+	ventry	el2t_error			// Error EL2t
+
+	ventry	el2h_sync			// Synchronous EL2h
+	ventry	el2h_irq			// IRQ EL2h
+	ventry	el2h_fiq			// FIQ EL2h
+	ventry	el2h_error			// Error EL2h
+
+	ventry	el1_sync_64			// Synchronous 64-bit EL1
+	ventry	el1_irq_64			// IRQ 64-bit EL1
+	ventry	el1_fiq_64			// FIQ 64-bit EL1
+	ventry	el1_error_64			// Error 64-bit EL1
+
+	ventry	el1_sync_32			// Synchronous 32-bit EL1
+	ventry	el1_irq_32			// IRQ 32-bit EL1
+	ventry	el1_fiq_32			// FIQ 32-bit EL1
+	ventry	el1_error_32			// Error 32-bit EL1
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 15/16] arm64: selftest: Expand EL2 test to disable and re-enable VHE
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (13 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 16/16] arm64: timer: Run tests with VHE disabled Alexandru Elisei
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
KVM doesn't deal with a guest running with VHE enabled, then disabling it
[1]. I also proposed a fix [1] for it.

[1] https://www.spinics.net/lists/arm-kernel/msg749823.html

 arm/selftest.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/arm/selftest.c b/arm/selftest.c
index 211bc8a5642b..4a6c943497ee 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -395,6 +395,18 @@ static void check_el2_cpu(void *data __unused)
 	report("CPU(%3d) Running at EL2", current_level() == CurrentEL_EL2, cpu);
 	report("CPU(%3d) VHE supported and enabled",
 			vhe_supported() && vhe_enabled(), cpu);
+
+	report_info("CPU(%3d) Disabling VHE", cpu);
+	disable_vhe();
+
+	report("CPU(%3d) Running at EL2", current_level() == CurrentEL_EL2, cpu);
+	report("CPU(%3d) VHE disabled", !vhe_enabled(), cpu);
+
+	report_info("CPU(%3d) Re-enabling VHE", cpu);
+	enable_vhe();
+
+	report("CPU(%3d) Running at EL2", current_level() == CurrentEL_EL2, cpu);
+	report("CPU(%3d) VHE enabled", vhe_enabled(), cpu);
 }
 
 static bool psci_check(void);
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* [kvm-unit-tests RFC PATCH 16/16] arm64: timer: Run tests with VHE disabled
  2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
                   ` (14 preceding siblings ...)
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 15/16] arm64: selftest: Expand EL2 test to disable and re-enable VHE Alexandru Elisei
@ 2019-08-28 13:38 ` Alexandru Elisei
  15 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 13:38 UTC (permalink / raw)
  To: kvm, kvmarm; +Cc: maz, andre.przywara, pbonzini

Disable VHE if first command line parameter is "nvhe" and then test the
timers. Just like with VHE enabled, if no other parameter is given, all
four timers are tested; otherwise, only the timers specified by the user.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 lib/arm64/asm/processor.h |  2 ++
 arm/timer.c               | 33 +++++++++++++++++++++++++++++----
 2 files changed, 31 insertions(+), 4 deletions(-)

diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 4d12913ca01f..36260a942795 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -26,6 +26,8 @@
 #define HCR_EL2_TGE		(1 << 27)
 #define HCR_EL2_E2H_SHIFT	34
 #define HCR_EL2_E2H		(UL(1) << 34)
+#define HCR_EL2_IMO		(1 << 4)
+#define HCR_EL2_FMO		(1 << 3)
 
 #define SCTLR_EL2_RES1		(3 << 28 | 3 << 22 | 1 << 18 |	\
 				 1 << 16 | 1 << 11 | 3 << 4)
diff --git a/arm/timer.c b/arm/timer.c
index faab671d0fb1..6b9d5d57a658 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -464,19 +464,34 @@ static void test_hptimer(void)
 	report_prefix_pop();
 }
 
-static void test_init(void)
+static void test_init(bool without_vhe)
 {
 	const struct fdt_property *prop;
 	const void *fdt = dt_fdt();
+	u64 hcr;
 	int node, len;
 	u32 *data;
 
+	if (without_vhe) {
+		disable_vhe();
+		hcr = read_sysreg(hcr_el2);
+		/* KVM doesn't support different IMO/FMO settings */
+		hcr |= HCR_EL2_IMO | HCR_EL2_FMO;
+		write_sysreg(hcr, hcr_el2);
+		isb();
+	}
+
 	if (current_el == CurrentEL_EL1) {
 		vtimer = &vtimer_info;
 		ptimer = &ptimer_info;
 	} else {
-		vtimer = &vtimer_info_vhe;
-		ptimer = &ptimer_info_vhe;
+		if (without_vhe) {
+			vtimer = &vtimer_info;
+			ptimer = &ptimer_info;
+		} else {
+			vtimer = &vtimer_info_vhe;
+			ptimer = &ptimer_info_vhe;
+		}
 		hvtimer = &hvtimer_info;
 		hptimer = &hptimer_info;
 	}
@@ -563,10 +578,20 @@ static void print_timer_info(void)
 int main(int argc, char **argv)
 {
 	int i;
+	bool without_vhe = false;
 
 	current_el = current_level();
 
-	test_init();
+	if (argc > 1 && strcmp(argv[1], "nvhe") == 0) {
+		if (current_el == CurrentEL_EL1)
+			report_info("Skipping EL2 tests. Boot at EL2 to enable.");
+		else
+			without_vhe = true;
+		argv = &argv[1];
+		argc--;
+	}
+
+	test_init(without_vhe);
 
 	print_timer_info();
 
-- 
2.7.4

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
@ 2019-08-28 14:09   ` Mark Rutland
  2019-08-29  8:18     ` Alexandru Elisei
  0 siblings, 1 reply; 34+ messages in thread
From: Mark Rutland @ 2019-08-28 14:09 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: maz, pbonzini, kvmarm, kvm, andre.przywara

On Wed, Aug 28, 2019 at 02:38:19PM +0100, Alexandru Elisei wrote:
> When a guest tries to execute code from MMIO memory, KVM injects an
> external abort into that guest. We have now fixed the psci test to not
> fetch instructions from the I/O region, and it's not that often that a
> guest misbehaves in such a way. Let's expand our coverage by adding a
> proper test targetting this corner case.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> The fault injection path is broken for nested guests [1]. You can use the
> last patch from the thread [2] to successfully run the test at EL2.
> 
> [1] https://www.spinics.net/lists/arm-kernel/msg745391.html
> [2] https://www.spinics.net/lists/arm-kernel/msg750310.html
> 
>  lib/arm64/asm/esr.h |  3 ++
>  arm/selftest.c      | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>  2 files changed, 96 insertions(+), 3 deletions(-)
> 
> 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_ */
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 176231f32ee1..18cc0ad8f729 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -16,6 +16,8 @@
>  #include <asm/psci.h>
>  #include <asm/smp.h>
>  #include <asm/barrier.h>
> +#include <asm/mmu.h>
> +#include <asm/pgtable.h>
>  
>  static void __user_psci_system_off(void)
>  {
> @@ -60,9 +62,38 @@ static void check_setup(int argc, char **argv)
>  		report_abort("missing input");
>  }
>  
> +extern pgd_t *mmu_idmap;
> +static void prep_io_exec(void)
> +{
> +	pgd_t *pgd = pgd_offset(mmu_idmap, 0);
> +	unsigned long sctlr;
> +
> +	/*
> +	 * AArch64 treats all regions writable at EL0 as PXN.

I didn't think that was the case, and I can't find wording to that
effect in the ARM ARM (looking at ARM DDI 0487E.a). Where is that
stated?

> Clear the user bit
> +	 * so we can execute code from the bottom I/O space (0G-1G) to simulate
> +	 * a misbehaved guest.
> +	 */
> +	pgd_val(*pgd) &= ~PMD_SECT_USER;
> +	flush_dcache_addr((unsigned long)pgd);

The virtualization extensions imply coherent page table walks, so I
don't think the cache maintenance is necessary (provided
TCR_EL1.{SH*,ORGN*,IRGN*} are configured appropriately.

> +	flush_tlb_page(0);
> +
> +	/* Make sure we can actually execute from a writable region */
> +#ifdef __arm__
> +	asm volatile("mrc p15, 0, %0, c1, c0, 0": "=r" (sctlr));
> +	sctlr &= ~CR_ST;
> +	asm volatile("mcr p15, 0, %0, c1, c0, 0" :: "r" (sctlr));
> +#else
> +	sctlr = read_sysreg(sctlr_el1);
> +	sctlr &= ~SCTLR_EL1_WXN;
> +	write_sysreg(sctlr, sctlr_el1);
> +#endif
> +	isb();
> +}

Thanks,
Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
@ 2019-08-28 14:19   ` Mark Rutland
  2019-08-29  8:36     ` Alexandru Elisei
  0 siblings, 1 reply; 34+ messages in thread
From: Mark Rutland @ 2019-08-28 14:19 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: maz, pbonzini, kvmarm, kvm, andre.przywara

On Wed, Aug 28, 2019 at 02:38:29PM +0100, Alexandru Elisei wrote:
> Add a function to disable VHE and another one to re-enable VHE. Both
> functions work under the assumption that the CPU had VHE mode enabled at
> boot.
> 
> Minimal support to run with VHE has been added to the TLB invalidate
> functions and to the exception handling code.
> 
> Since we're touch the assembly enable/disable MMU code, let's take this
> opportunity to replace a magic number with the proper define.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/arm/asm/processor.h       |   8 ++
>  lib/arm64/asm/mmu.h           |  11 ++-
>  lib/arm64/asm/pgtable-hwdef.h |  53 +++++++++---
>  lib/arm64/asm/processor.h     |  44 +++++++++-
>  lib/arm/processor.c           |  11 +++
>  lib/arm/setup.c               |   2 +
>  lib/arm64/processor.c         |  67 ++++++++++++++-
>  arm/cstart64.S                | 186 +++++++++++++++++++++++++++++++++++++++++-
>  8 files changed, 364 insertions(+), 18 deletions(-)

> +extern void asm_disable_vhe(void);
> +void disable_vhe(void)
> +{
> +	u64 sp, sp_phys, sp_base, sp_base_phys;
> +
> +	assert(current_level() == CurrentEL_EL2 && vhe_enabled());
> +
> +	sp = current_stack_pointer;
> +	sp_phys = __virt_to_phys(sp);
> +	sp_base = sp & THREAD_MASK;
> +	sp_base_phys = sp_phys & THREAD_MASK;
> +
> +	/*
> +	 * We will disable, then enable the MMU, make sure the exception
> +	 * handling code works during the small window of time when the MMU is
> +	 * off.
> +	 */
> +	dcache_clean_inval_range(sp_base, sp_base + THREAD_SIZE);
> +	dcache_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp_phys));
> +
> +	asm_disable_vhe();
> +
> +	dcache_clean_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
> +	dcache_inval_range(sp_base, sp_base + THREAD_SIZE);
> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp));
> +}

This sequence is not safe. The compiler can spill/reload at any point,
and the CPU can allocate (clean) lines into the cache while the MMU is
enabled.

I think you need to move the entire sequence to assembly, and should
perform any cache maintenance while the MMU is off.

> +extern void asm_enable_vhe(void);
> +void enable_vhe(void)
> +{
> +	u64 sp, sp_phys, sp_base, sp_base_phys;
> +
> +	assert(current_level() == CurrentEL_EL2 && !vhe_enabled());
> +
> +	sp = current_stack_pointer;
> +	sp_phys = __virt_to_phys(sp);
> +	sp_base = sp & THREAD_MASK;
> +	sp_base_phys = sp_phys & THREAD_MASK;
> +
> +	dcache_clean_inval_range(sp_base, sp_base + THREAD_SIZE);
> +	dcache_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp_phys));
> +
> +	asm_enable_vhe();
> +
> +	dcache_clean_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
> +	dcache_inval_range(sp_base, sp_base + THREAD_SIZE);
> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp));
> +}

Likewise.

> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index d4b20267a7a6..dc9e634e2307 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -104,6 +104,13 @@ exceptions_init:
>  
>  .text
>  
> +exceptions_init_nvhe:
> +	adrp	x0, vector_table_nvhe
> +	add	x0, x0, :lo12:vector_table_nvhe
> +	msr	vbar_el2, x0
> +	isb
> +	ret
> +
>  .globl get_mmu_off
>  get_mmu_off:
>  	adrp	x0, auxinfo
> @@ -204,7 +211,7 @@ asm_mmu_enable:
>  		     TCR_IRGN_WBWA | TCR_ORGN_WBWA |	\
>  		     TCR_SHARED
>  	mrs	x2, id_aa64mmfr0_el1
> -	bfi	x1, x2, #32, #3
> +	bfi	x1, x2, #TCR_EL1_IPS_SHIFT, #3
>  	msr	tcr_el1, x1
>  
>  	/* MAIR */
> @@ -229,6 +236,33 @@ asm_mmu_enable:
>  
>  	ret
>  
> +asm_mmu_enable_nvhe:
> +	ic      iallu
> +	tlbi    alle2is
> +	dsb     ish

why is the IC local, but the TLBI broadcast?

If this only needs ot be local, a DSB NSH will be sufficient.

Thanks,
Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level Alexandru Elisei
@ 2019-08-28 14:32   ` Andrew Jones
  2019-08-28 15:39     ` Alexandru Elisei
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 14:32 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 02:38:16PM +0100, Alexandru Elisei wrote:
> expected_regs.pstate already has PSR_MODE_EL1h set as the expected
> Exception Level.

That's true for selftest-vectors-kernel, but not for
selftest-vectors-user.

Thanks,
drew

> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/selftest.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/arm/selftest.c b/arm/selftest.c
> index 28a17f7a7531..176231f32ee1 100644
> --- a/arm/selftest.c
> +++ b/arm/selftest.c
> @@ -213,10 +213,6 @@ static bool check_regs(struct pt_regs *regs)
>  {
>  	unsigned i;
>  
> -	/* exception handlers should always run in EL1 */
> -	if (current_level() != CurrentEL_EL1)
> -		return false;
> -
>  	for (i = 0; i < ARRAY_SIZE(regs->regs); ++i) {
>  		if (regs->regs[i] != expected_regs.regs[i])
>  			return false;
> -- 
> 2.7.4
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
@ 2019-08-28 14:45   ` Andrew Jones
  2019-08-28 15:14     ` Alexandru Elisei
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 14:45 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 02:38:17PM +0100, Alexandru Elisei wrote:
> The psci test performs a series of CPU_ON/CPU_OFF cycles for CPU 1. This is
> done by setting the entry point for the CPU_ON call to the physical address
> of the C function cpu_psci_cpu_die.
> 
> The compiler is well within its rights to use the stack when generating
> code for cpu_psci_cpu_die.  However, because no stack initialization has
> been done, the stack pointer is zero, as set by KVM when creating the VCPU.
> This causes a data abort without a change in exception level. The VBAR_EL1
> register is also zero (the KVM reset value for VBAR_EL1), the MMU is off,
> and we end up trying to fetch instructions from address 0x200.
> 
> At this point, a stage 2 instruction abort is generated which is taken to
> KVM. KVM interprets this as an instruction fetch from an I/O region, and
> injects a prefetch abort into the guest. Prefetch abort is a synchronous
> exception, and on guest return the VCPU PC will be set to VBAR_EL1 + 0x200,
> which is...  0x200. The VCPU ends up in an infinite loop causing a prefetch
> abort while fetching the instruction to service the said abort.
> 
> cpu_psci_cpu_die is basically a wrapper over the HVC instruction, so
> provide an assembly implementation for the function which will serve as the
> entry point for CPU_ON.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arm/cstart.S   | 7 +++++++
>  arm/cstart64.S | 7 +++++++
>  arm/psci.c     | 5 +++--
>  3 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/arm/cstart.S b/arm/cstart.S
> index 114726feab82..5d4fe4b1570b 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -7,6 +7,7 @@
>   */
>  #define __ASSEMBLY__
>  #include <auxinfo.h>
> +#include <linux/psci.h>
>  #include <asm/thread_info.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/ptrace.h>
> @@ -138,6 +139,12 @@ secondary_entry:
>  	blx	r0
>  	b	do_idle
>  
> +.global asm_cpu_psci_cpu_die
> +asm_cpu_psci_cpu_die:
> +	ldr	r0, =PSCI_0_2_FN_CPU_OFF
> +	hvc	#0
> +	b	halt

Shouldn't we load PSCI_POWER_STATE_TYPE_POWER_DOWN into r1 and
zero out r2 and r3, as cpu_psci_cpu_die() does? And maybe we
should just do a 'b .' here instead of 'b halt' in order to
avoid confusion as to how we ended up in halt(), if the psci
invocation were to ever fail.

> +
>  .globl halt
>  halt:
>  1:	wfi
> diff --git a/arm/cstart64.S b/arm/cstart64.S
> index b0e8baa1a23a..20f832fd57f7 100644
> --- a/arm/cstart64.S
> +++ b/arm/cstart64.S
> @@ -7,6 +7,7 @@
>   */
>  #define __ASSEMBLY__
>  #include <auxinfo.h>
> +#include <linux/psci.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/ptrace.h>
>  #include <asm/processor.h>
> @@ -128,6 +129,12 @@ secondary_entry:
>  	blr	x0
>  	b	do_idle
>  
> +.globl asm_cpu_psci_cpu_die
> +asm_cpu_psci_cpu_die:
> +	ldr	x0, =PSCI_0_2_FN_CPU_OFF
> +	hvc	#0
> +	b	halt

Same as above

> +
>  .globl halt
>  halt:
>  1:	wfi
> diff --git a/arm/psci.c b/arm/psci.c
> index 5cb4d5c7c233..0440c4cdbc59 100644
> --- a/arm/psci.c
> +++ b/arm/psci.c
> @@ -72,6 +72,7 @@ static int cpu_on_ret[NR_CPUS];
>  static cpumask_t cpu_on_ready, cpu_on_done;
>  static volatile int cpu_on_start;
>  
> +extern void asm_cpu_psci_cpu_die(void);
>  static void cpu_on_secondary_entry(void)
>  {
>  	int cpu = smp_processor_id();
> @@ -79,7 +80,7 @@ static void cpu_on_secondary_entry(void)
>  	cpumask_set_cpu(cpu, &cpu_on_ready);
>  	while (!cpu_on_start)
>  		cpu_relax();
> -	cpu_on_ret[cpu] = psci_cpu_on(cpus[1], __pa(cpu_psci_cpu_die));
> +	cpu_on_ret[cpu] = psci_cpu_on(cpus[1], __pa(asm_cpu_psci_cpu_die));
>  	cpumask_set_cpu(cpu, &cpu_on_done);
>  }
>  
> @@ -104,7 +105,7 @@ static bool psci_cpu_on_test(void)
>  	cpu_on_start = 1;
>  	smp_mb();
>  
> -	cpu_on_ret[0] = psci_cpu_on(cpus[1], __pa(cpu_psci_cpu_die));
> +	cpu_on_ret[0] = psci_cpu_on(cpus[1], __pa(asm_cpu_psci_cpu_die));
>  	cpumask_set_cpu(0, &cpu_on_done);
>  
>  	while (!cpumask_full(&cpu_on_done))
> -- 
> 2.7.4
>

Thanks,
drew 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
@ 2019-08-28 14:47   ` Andrew Jones
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 14:47 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 02:38:18PM +0100, Alexandru Elisei wrote:
> pgtable.h is used only by mmu.c, where it is included after alloc_page.h.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/arm/asm/pgtable.h   | 1 +
>  lib/arm64/asm/pgtable.h | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/lib/arm/asm/pgtable.h b/lib/arm/asm/pgtable.h
> index 241dff69b38a..b01c34348321 100644
> --- a/lib/arm/asm/pgtable.h
> +++ b/lib/arm/asm/pgtable.h
> @@ -13,6 +13,7 @@
>   *
>   * This work is licensed under the terms of the GNU GPL, version 2.
>   */
> +#include <alloc_page.h>
>  
>  /*
>   * We can convert va <=> pa page table addresses with simple casts
> diff --git a/lib/arm64/asm/pgtable.h b/lib/arm64/asm/pgtable.h
> index ee0a2c88cc18..e9dd49155564 100644
> --- a/lib/arm64/asm/pgtable.h
> +++ b/lib/arm64/asm/pgtable.h
> @@ -14,6 +14,7 @@
>   * This work is licensed under the terms of the GNU GPL, version 2.
>   */
>  #include <alloc.h>
> +#include <alloc_page.h>
>  #include <asm/setup.h>
>  #include <asm/page.h>
>  #include <asm/pgtable-hwdef.h>
> -- 
> 2.7.4
>

Reviewed-by: Andrew Jones <drjones@redhat.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
@ 2019-08-28 14:55   ` Andrew Jones
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 14:55 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 02:38:23PM +0100, Alexandru Elisei wrote:
> When the MMU is off, all addresses are physical addresses. If the stack
> pointer is not an identity mapped address (the virtual address is not the
> same as the physical address), then we end up trying to access an invalid
> memory region. This can happen if we call mmu_disable from a secondary CPU,
> which has its stack allocated from the vmalloc region.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/arm/mmu.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
> index 3d38c8397f5a..161f7a8e607c 100644
> --- a/lib/arm/mmu.c
> +++ b/lib/arm/mmu.c
> @@ -66,8 +66,12 @@ void mmu_enable(pgd_t *pgtable)
>  extern void asm_mmu_disable(void);
>  void mmu_disable(void)
>  {
> +	unsigned long sp = current_stack_pointer;
>  	int cpu = current_thread_info()->cpu;
>  
> +	assert_msg(__virt_to_phys(sp) == sp,
> +			"Attempting to disable MMU with non-identity mapped stack");
> +
>  	mmu_mark_disabled(cpu);
>  
>  	asm_mmu_disable();
> -- 
> 2.7.4
>

Reviewed-by: Andrew Jones <drjones@redhat.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
@ 2019-08-28 14:59   ` Andrew Jones
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 14:59 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 02:38:24PM +0100, Alexandru Elisei wrote:
> Let's invalidate the TLB before enabling the MMU, not after, so we don't
> accidently use a stale TLB mapping. For arm64, we already do that in
> asm_mmu_enable, and we issue an extra invalidation after the function
> returns. Invalidate the TLB in asm_mmu_enable for arm also, and remove the
> redundant call to tlb_flush_all.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/arm/mmu.c | 1 -
>  arm/cstart.S  | 4 ++++
>  2 files changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/arm/mmu.c b/lib/arm/mmu.c
> index 161f7a8e607c..66a05d789386 100644
> --- a/lib/arm/mmu.c
> +++ b/lib/arm/mmu.c
> @@ -57,7 +57,6 @@ void mmu_enable(pgd_t *pgtable)
>  	struct thread_info *info = current_thread_info();
>  
>  	asm_mmu_enable(__pa(pgtable));
> -	flush_tlb_all();
>  
>  	info->pgtable = pgtable;
>  	mmu_mark_enabled(info->cpu);
> diff --git a/arm/cstart.S b/arm/cstart.S
> index 5d4fe4b1570b..316672545551 100644
> --- a/arm/cstart.S
> +++ b/arm/cstart.S
> @@ -160,6 +160,10 @@ halt:
>  .equ	NMRR,	0xff000004		@ MAIR1 (from Linux kernel)
>  .globl asm_mmu_enable
>  asm_mmu_enable:
> +	/* TLBIALL */
> +	mcr	p15, 0, r2, c8, c7, 0
> +	dsb	ish
> +
>  	/* TTBCR */
>  	mrc	p15, 0, r2, c2, c0, 2
>  	orr	r2, #(1 << 31)		@ TTB_EAE
> -- 
> 2.7.4
>

Reviewed-by: Andrew Jones <drjones@redhat.com>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h
  2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h Alexandru Elisei
@ 2019-08-28 15:10   ` Andrew Jones
  2019-08-28 15:46     ` Alexandru Elisei
  0 siblings, 1 reply; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 15:10 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 02:38:25PM +0100, Alexandru Elisei wrote:
> The UL macro was previously defined in lib/arm64/asm/pgtable-hwdef.h. Move
> it to lib/linux/const.h so it can be used in other files. To keep things
> consistent, also add an ULL macro.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  lib/linux/const.h             | 7 +++++--
>  lib/arm64/asm/pgtable-hwdef.h | 2 --
>  2 files changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/linux/const.h b/lib/linux/const.h
> index c872bfd25e13..e3c7fec3f4b8 100644
> --- a/lib/linux/const.h
> +++ b/lib/linux/const.h
> @@ -21,7 +21,10 @@
>  #define _AT(T,X)	((T)(X))
>  #endif
>  
> -#define _BITUL(x)	(_AC(1,UL) << (x))
> -#define _BITULL(x)	(_AC(1,ULL) << (x))
> +#define UL(x) 		_AC(x, UL)
> +#define ULL(x)		_AC(x, ULL)
> +
> +#define _BITUL(x)	(UL(1) << (x))
> +#define _BITULL(x)	(ULL(1) << (x))

I don't mind this, but if we want to keep this file consistent with
Linux's include/uapi/linux/const.h, which is actually the goal, then we
should be adding _UL and _ULL instead. But in that case we'd probably
want to leave the define below.

Thanks,
drew

>  
>  #endif /* !(_LINUX_CONST_H) */
> diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
> index 045a3ce12645..e6f02fae4075 100644
> --- a/lib/arm64/asm/pgtable-hwdef.h
> +++ b/lib/arm64/asm/pgtable-hwdef.h
> @@ -9,8 +9,6 @@
>   * This work is licensed under the terms of the GNU GPL, version 2.
>   */
>  
> -#define UL(x) _AC(x, UL)
> -
>  #define PTRS_PER_PTE		(1 << (PAGE_SHIFT - 3))
>  
>  /*
> -- 
> 2.7.4
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors
  2019-08-28 14:45   ` Andrew Jones
@ 2019-08-28 15:14     ` Alexandru Elisei
  2019-09-02 14:55       ` Alexandru Elisei
  0 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 15:14 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On 8/28/19 3:45 PM, Andrew Jones wrote:
> On Wed, Aug 28, 2019 at 02:38:17PM +0100, Alexandru Elisei wrote:
>> The psci test performs a series of CPU_ON/CPU_OFF cycles for CPU 1. This is
>> done by setting the entry point for the CPU_ON call to the physical address
>> of the C function cpu_psci_cpu_die.
>>
>> The compiler is well within its rights to use the stack when generating
>> code for cpu_psci_cpu_die.  However, because no stack initialization has
>> been done, the stack pointer is zero, as set by KVM when creating the VCPU.
>> This causes a data abort without a change in exception level. The VBAR_EL1
>> register is also zero (the KVM reset value for VBAR_EL1), the MMU is off,
>> and we end up trying to fetch instructions from address 0x200.
>>
>> At this point, a stage 2 instruction abort is generated which is taken to
>> KVM. KVM interprets this as an instruction fetch from an I/O region, and
>> injects a prefetch abort into the guest. Prefetch abort is a synchronous
>> exception, and on guest return the VCPU PC will be set to VBAR_EL1 + 0x200,
>> which is...  0x200. The VCPU ends up in an infinite loop causing a prefetch
>> abort while fetching the instruction to service the said abort.
>>
>> cpu_psci_cpu_die is basically a wrapper over the HVC instruction, so
>> provide an assembly implementation for the function which will serve as the
>> entry point for CPU_ON.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  arm/cstart.S   | 7 +++++++
>>  arm/cstart64.S | 7 +++++++
>>  arm/psci.c     | 5 +++--
>>  3 files changed, 17 insertions(+), 2 deletions(-)
>>
>> diff --git a/arm/cstart.S b/arm/cstart.S
>> index 114726feab82..5d4fe4b1570b 100644
>> --- a/arm/cstart.S
>> +++ b/arm/cstart.S
>> @@ -7,6 +7,7 @@
>>   */
>>  #define __ASSEMBLY__
>>  #include <auxinfo.h>
>> +#include <linux/psci.h>
>>  #include <asm/thread_info.h>
>>  #include <asm/asm-offsets.h>
>>  #include <asm/ptrace.h>
>> @@ -138,6 +139,12 @@ secondary_entry:
>>  	blx	r0
>>  	b	do_idle
>>  
>> +.global asm_cpu_psci_cpu_die
>> +asm_cpu_psci_cpu_die:
>> +	ldr	r0, =PSCI_0_2_FN_CPU_OFF
>> +	hvc	#0
>> +	b	halt
> Shouldn't we load PSCI_POWER_STATE_TYPE_POWER_DOWN into r1 and
> zero out r2 and r3, as cpu_psci_cpu_die() does? And maybe we
> should just do a 'b .' here instead of 'b halt' in order to
> avoid confusion as to how we ended up in halt(), if the psci
> invocation were to ever fail.

Not really, I'm not really sure where the extra parameter in cpu_psci_cpu_die
comes from. I have looked at ARM DEN 0022D, section 5.1.3, and the CPU_OFFcall
has exactly one parameter, the function id. I've also looked at how KVM handles
this call, and it doesn't use anything else other than the function id. Please
correct me if I missed something.

As for zero'ing the extra registers, this is not part of the SMC calling
convention, this is just something that the C code for psci does. The SMC
calling convention states that registers 0-3 will be modified after the call, so
having 4 arguments to the psci_invoke function will tell the compiler to
save/restore the registers in the caller.

As for doing 'b .' instead of branching to halt, that's a good idea, I'll do
that. But it will only be useful if the last CPU_OFF call fails.

Thanks,
Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level
  2019-08-28 14:32   ` Andrew Jones
@ 2019-08-28 15:39     ` Alexandru Elisei
  0 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 15:39 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, Andre Przywara, pbonzini, kvmarm

On 8/28/19 3:32 PM, Andrew Jones wrote:
> On Wed, Aug 28, 2019 at 02:38:16PM +0100, Alexandru Elisei wrote:
>> expected_regs.pstate already has PSR_MODE_EL1h set as the expected
>> Exception Level.
> That's true for selftest-vectors-kernel, but not for
> selftest-vectors-user.

Oops, that's true. This patch is definitely wrong, I'll drop it.

Thanks,
Alex
>
> Thanks,
> drew
>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  arm/selftest.c | 4 ----
>>  1 file changed, 4 deletions(-)
>>
>> diff --git a/arm/selftest.c b/arm/selftest.c
>> index 28a17f7a7531..176231f32ee1 100644
>> --- a/arm/selftest.c
>> +++ b/arm/selftest.c
>> @@ -213,10 +213,6 @@ static bool check_regs(struct pt_regs *regs)
>>  {
>>      unsigned i;
>>
>> -    /* exception handlers should always run in EL1 */
>> -    if (current_level() != CurrentEL_EL1)
>> -            return false;
>> -
>>      for (i = 0; i < ARRAY_SIZE(regs->regs); ++i) {
>>              if (regs->regs[i] != expected_regs.regs[i])
>>                      return false;
>> --
>> 2.7.4
>>
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h
  2019-08-28 15:10   ` Andrew Jones
@ 2019-08-28 15:46     ` Alexandru Elisei
  2019-08-28 16:19       ` Andrew Jones
  0 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-28 15:46 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On 8/28/19 4:10 PM, Andrew Jones wrote:
> On Wed, Aug 28, 2019 at 02:38:25PM +0100, Alexandru Elisei wrote:
>> The UL macro was previously defined in lib/arm64/asm/pgtable-hwdef.h. Move
>> it to lib/linux/const.h so it can be used in other files. To keep things
>> consistent, also add an ULL macro.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  lib/linux/const.h             | 7 +++++--
>>  lib/arm64/asm/pgtable-hwdef.h | 2 --
>>  2 files changed, 5 insertions(+), 4 deletions(-)
>>
>> diff --git a/lib/linux/const.h b/lib/linux/const.h
>> index c872bfd25e13..e3c7fec3f4b8 100644
>> --- a/lib/linux/const.h
>> +++ b/lib/linux/const.h
>> @@ -21,7 +21,10 @@
>>  #define _AT(T,X)	((T)(X))
>>  #endif
>>  
>> -#define _BITUL(x)	(_AC(1,UL) << (x))
>> -#define _BITULL(x)	(_AC(1,ULL) << (x))
>> +#define UL(x) 		_AC(x, UL)
>> +#define ULL(x)		_AC(x, ULL)
>> +
>> +#define _BITUL(x)	(UL(1) << (x))
>> +#define _BITULL(x)	(ULL(1) << (x))
> I don't mind this, but if we want to keep this file consistent with
> Linux's include/uapi/linux/const.h, which is actually the goal, then we
> should be adding _UL and _ULL instead. But in that case we'd probably
> want to leave the define below.
>
> Thanks,
> drew

Hm... The next patch needs the UL define. Consistency is good, so if we want to
keep it consistent with include/uapi/linux/const.h, then I will change the
defines (and the uses) to _UL and _ULL, if that's fine with you.

Thanks,
Alex
>
>>  
>>  #endif /* !(_LINUX_CONST_H) */
>> diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
>> index 045a3ce12645..e6f02fae4075 100644
>> --- a/lib/arm64/asm/pgtable-hwdef.h
>> +++ b/lib/arm64/asm/pgtable-hwdef.h
>> @@ -9,8 +9,6 @@
>>   * This work is licensed under the terms of the GNU GPL, version 2.
>>   */
>>  
>> -#define UL(x) _AC(x, UL)
>> -
>>  #define PTRS_PER_PTE		(1 << (PAGE_SHIFT - 3))
>>  
>>  /*
>> -- 
>> 2.7.4
>>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h
  2019-08-28 15:46     ` Alexandru Elisei
@ 2019-08-28 16:19       ` Andrew Jones
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2019-08-28 16:19 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Wed, Aug 28, 2019 at 04:46:04PM +0100, Alexandru Elisei wrote:
> On 8/28/19 4:10 PM, Andrew Jones wrote:
> > On Wed, Aug 28, 2019 at 02:38:25PM +0100, Alexandru Elisei wrote:
> >> The UL macro was previously defined in lib/arm64/asm/pgtable-hwdef.h. Move
> >> it to lib/linux/const.h so it can be used in other files. To keep things
> >> consistent, also add an ULL macro.
> >>
> >> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> >> ---
> >>  lib/linux/const.h             | 7 +++++--
> >>  lib/arm64/asm/pgtable-hwdef.h | 2 --
> >>  2 files changed, 5 insertions(+), 4 deletions(-)
> >>
> >> diff --git a/lib/linux/const.h b/lib/linux/const.h
> >> index c872bfd25e13..e3c7fec3f4b8 100644
> >> --- a/lib/linux/const.h
> >> +++ b/lib/linux/const.h
> >> @@ -21,7 +21,10 @@
> >>  #define _AT(T,X)	((T)(X))
> >>  #endif
> >>  
> >> -#define _BITUL(x)	(_AC(1,UL) << (x))
> >> -#define _BITULL(x)	(_AC(1,ULL) << (x))
> >> +#define UL(x) 		_AC(x, UL)
> >> +#define ULL(x)		_AC(x, ULL)
> >> +
> >> +#define _BITUL(x)	(UL(1) << (x))
> >> +#define _BITULL(x)	(ULL(1) << (x))
> > I don't mind this, but if we want to keep this file consistent with
> > Linux's include/uapi/linux/const.h, which is actually the goal, then we
> > should be adding _UL and _ULL instead. But in that case we'd probably
> > want to leave the define below.
> >
> > Thanks,
> > drew
> 
> Hm... The next patch needs the UL define. Consistency is good, so if we want to
> keep it consistent with include/uapi/linux/const.h, then I will change the
> defines (and the uses) to _UL and _ULL, if that's fine with you.

Yeah, I think that's best.

Thanks,
drew

> 
> Thanks,
> Alex
> >
> >>  
> >>  #endif /* !(_LINUX_CONST_H) */
> >> diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
> >> index 045a3ce12645..e6f02fae4075 100644
> >> --- a/lib/arm64/asm/pgtable-hwdef.h
> >> +++ b/lib/arm64/asm/pgtable-hwdef.h
> >> @@ -9,8 +9,6 @@
> >>   * This work is licensed under the terms of the GNU GPL, version 2.
> >>   */
> >>  
> >> -#define UL(x) _AC(x, UL)
> >> -
> >>  #define PTRS_PER_PTE		(1 << (PAGE_SHIFT - 3))
> >>  
> >>  /*
> >> -- 
> >> 2.7.4
> >>
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test
  2019-08-28 14:09   ` Mark Rutland
@ 2019-08-29  8:18     ` Alexandru Elisei
  2019-08-29 10:19       ` Mark Rutland
  0 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-29  8:18 UTC (permalink / raw)
  To: Mark Rutland; +Cc: maz, pbonzini, kvmarm, kvm, andre.przywara

On 8/28/19 3:09 PM, Mark Rutland wrote:
> On Wed, Aug 28, 2019 at 02:38:19PM +0100, Alexandru Elisei wrote:
>> When a guest tries to execute code from MMIO memory, KVM injects an
>> external abort into that guest. We have now fixed the psci test to not
>> fetch instructions from the I/O region, and it's not that often that a
>> guest misbehaves in such a way. Let's expand our coverage by adding a
>> proper test targetting this corner case.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>> The fault injection path is broken for nested guests [1]. You can use the
>> last patch from the thread [2] to successfully run the test at EL2.
>>
>> [1] https://www.spinics.net/lists/arm-kernel/msg745391.html
>> [2] https://www.spinics.net/lists/arm-kernel/msg750310.html
>>
>>  lib/arm64/asm/esr.h |  3 ++
>>  arm/selftest.c      | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++--
>>  2 files changed, 96 insertions(+), 3 deletions(-)
>>
>> 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_ */
>> diff --git a/arm/selftest.c b/arm/selftest.c
>> index 176231f32ee1..18cc0ad8f729 100644
>> --- a/arm/selftest.c
>> +++ b/arm/selftest.c
>> @@ -16,6 +16,8 @@
>>  #include <asm/psci.h>
>>  #include <asm/smp.h>
>>  #include <asm/barrier.h>
>> +#include <asm/mmu.h>
>> +#include <asm/pgtable.h>
>>  
>>  static void __user_psci_system_off(void)
>>  {
>> @@ -60,9 +62,38 @@ static void check_setup(int argc, char **argv)
>>  		report_abort("missing input");
>>  }
>>  
>> +extern pgd_t *mmu_idmap;
>> +static void prep_io_exec(void)
>> +{
>> +	pgd_t *pgd = pgd_offset(mmu_idmap, 0);
>> +	unsigned long sctlr;
>> +
>> +	/*
>> +	 * AArch64 treats all regions writable at EL0 as PXN.
> I didn't think that was the case, and I can't find wording to that
> effect in the ARM ARM (looking at ARM DDI 0487E.a). Where is that
> stated?

It's in ARM DDI 0487E.a, table D5-33, footnote c: "Not executable, because
AArch64 execution treats all regions writable at EL0 as being PXN". I'll update
the comment to include the quote.

>
>> Clear the user bit
>> +	 * so we can execute code from the bottom I/O space (0G-1G) to simulate
>> +	 * a misbehaved guest.
>> +	 */
>> +	pgd_val(*pgd) &= ~PMD_SECT_USER;
>> +	flush_dcache_addr((unsigned long)pgd);
> The virtualization extensions imply coherent page table walks, so I
> don't think the cache maintenance is necessary (provided
> TCR_EL1.{SH*,ORGN*,IRGN*} are configured appropriately.

I was following the pattern from lib/arm/mmu.c. You are correct, and Linux
doesn't do any dcache maintenance either (judging by looking at both set_pte
(for arm64) and various implementations for set_pte_ext (for armv7)).

For future reference, ARM DDI 0487E.a, in section D13.2.72, states about the
ID_MMFR3_EL1 register:

"CohWalk, bits [23:20]

Coherent Walk. Indicates whether Translation table updates require a clean to
the Point of Unification. Defined values are:
0b0000 Updates to the translation tables require a clean to the Point of
Unification to ensure visibility by subsequent translation table walks.
0b0001 Updates to the translation tables do not require a clean to the Point of
Unification to ensure visibility by subsequent translation table walks.

In Armv8-A the only permitted value is 0b0001."

For armv7, ARM DDI 0406C.d states in section B3.3.1 Translation table walks:

"If an implementation includes the Multiprocessing Extensions, translation table
walks must access data or unified caches, or data and unified caches, of other
agents participating in the coherency protocol, according to the shareability
attributes described in the  TTBR. These shareability attributes must be
consistent with the shareability attributes for the translation tables themselves."

and in section B1.7 that virtualization extensions require the multiprocessing
extensions.

So the dcache maintenance operations are not needed, I'll remove them, thank you
for pointing this out.

Thanks,
Alex
>
>> +	flush_tlb_page(0);
>> +
>> +	/* Make sure we can actually execute from a writable region */
>> +#ifdef __arm__
>> +	asm volatile("mrc p15, 0, %0, c1, c0, 0": "=r" (sctlr));
>> +	sctlr &= ~CR_ST;
>> +	asm volatile("mcr p15, 0, %0, c1, c0, 0" :: "r" (sctlr));
>> +#else
>> +	sctlr = read_sysreg(sctlr_el1);
>> +	sctlr &= ~SCTLR_EL1_WXN;
>> +	write_sysreg(sctlr, sctlr_el1);
>> +#endif
>> +	isb();
>> +}
> Thanks,
> Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE
  2019-08-28 14:19   ` Mark Rutland
@ 2019-08-29  8:36     ` Alexandru Elisei
  0 siblings, 0 replies; 34+ messages in thread
From: Alexandru Elisei @ 2019-08-29  8:36 UTC (permalink / raw)
  To: Mark Rutland; +Cc: maz, pbonzini, kvmarm, kvm, andre.przywara

On 8/28/19 3:19 PM, Mark Rutland wrote:
> On Wed, Aug 28, 2019 at 02:38:29PM +0100, Alexandru Elisei wrote:
>> Add a function to disable VHE and another one to re-enable VHE. Both
>> functions work under the assumption that the CPU had VHE mode enabled at
>> boot.
>>
>> Minimal support to run with VHE has been added to the TLB invalidate
>> functions and to the exception handling code.
>>
>> Since we're touch the assembly enable/disable MMU code, let's take this
>> opportunity to replace a magic number with the proper define.
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>>  lib/arm/asm/processor.h       |   8 ++
>>  lib/arm64/asm/mmu.h           |  11 ++-
>>  lib/arm64/asm/pgtable-hwdef.h |  53 +++++++++---
>>  lib/arm64/asm/processor.h     |  44 +++++++++-
>>  lib/arm/processor.c           |  11 +++
>>  lib/arm/setup.c               |   2 +
>>  lib/arm64/processor.c         |  67 ++++++++++++++-
>>  arm/cstart64.S                | 186 +++++++++++++++++++++++++++++++++++++++++-
>>  8 files changed, 364 insertions(+), 18 deletions(-)
>> +extern void asm_disable_vhe(void);
>> +void disable_vhe(void)
>> +{
>> +	u64 sp, sp_phys, sp_base, sp_base_phys;
>> +
>> +	assert(current_level() == CurrentEL_EL2 && vhe_enabled());
>> +
>> +	sp = current_stack_pointer;
>> +	sp_phys = __virt_to_phys(sp);
>> +	sp_base = sp & THREAD_MASK;
>> +	sp_base_phys = sp_phys & THREAD_MASK;
>> +
>> +	/*
>> +	 * We will disable, then enable the MMU, make sure the exception
>> +	 * handling code works during the small window of time when the MMU is
>> +	 * off.
>> +	 */
>> +	dcache_clean_inval_range(sp_base, sp_base + THREAD_SIZE);
>> +	dcache_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
>> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp_phys));
>> +
>> +	asm_disable_vhe();
>> +
>> +	dcache_clean_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
>> +	dcache_inval_range(sp_base, sp_base + THREAD_SIZE);
>> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp));
>> +}
> This sequence is not safe. The compiler can spill/reload at any point,
> and the CPU can allocate (clean) lines into the cache while the MMU is
> enabled.
>
> I think you need to move the entire sequence to assembly, and should
> perform any cache maintenance while the MMU is off.

You are correct. I was being extra careful here. The fact is, the stack isn't
used at all when the MMU is off. The only way the stack would get used if an
exception is triggered, and that can only happen if we mess up our code. I'll
remove the stack pointer swap, and make sure that interrupts are disabled.

>
>> +extern void asm_enable_vhe(void);
>> +void enable_vhe(void)
>> +{
>> +	u64 sp, sp_phys, sp_base, sp_base_phys;
>> +
>> +	assert(current_level() == CurrentEL_EL2 && !vhe_enabled());
>> +
>> +	sp = current_stack_pointer;
>> +	sp_phys = __virt_to_phys(sp);
>> +	sp_base = sp & THREAD_MASK;
>> +	sp_base_phys = sp_phys & THREAD_MASK;
>> +
>> +	dcache_clean_inval_range(sp_base, sp_base + THREAD_SIZE);
>> +	dcache_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
>> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp_phys));
>> +
>> +	asm_enable_vhe();
>> +
>> +	dcache_clean_inval_range(sp_base_phys, sp_base_phys + THREAD_SIZE);
>> +	dcache_inval_range(sp_base, sp_base + THREAD_SIZE);
>> +	asm volatile(	"mov	sp, %0\n" : :"r" (sp));
>> +}
> Likewise.
>
>> diff --git a/arm/cstart64.S b/arm/cstart64.S
>> index d4b20267a7a6..dc9e634e2307 100644
>> --- a/arm/cstart64.S
>> +++ b/arm/cstart64.S
>> @@ -104,6 +104,13 @@ exceptions_init:
>>  
>>  .text
>>  
>> +exceptions_init_nvhe:
>> +	adrp	x0, vector_table_nvhe
>> +	add	x0, x0, :lo12:vector_table_nvhe
>> +	msr	vbar_el2, x0
>> +	isb
>> +	ret
>> +
>>  .globl get_mmu_off
>>  get_mmu_off:
>>  	adrp	x0, auxinfo
>> @@ -204,7 +211,7 @@ asm_mmu_enable:
>>  		     TCR_IRGN_WBWA | TCR_ORGN_WBWA |	\
>>  		     TCR_SHARED
>>  	mrs	x2, id_aa64mmfr0_el1
>> -	bfi	x1, x2, #32, #3
>> +	bfi	x1, x2, #TCR_EL1_IPS_SHIFT, #3
>>  	msr	tcr_el1, x1
>>  
>>  	/* MAIR */
>> @@ -229,6 +236,33 @@ asm_mmu_enable:
>>  
>>  	ret
>>  
>> +asm_mmu_enable_nvhe:
>> +	ic      iallu
>> +	tlbi    alle2is
>> +	dsb     ish
> why is the IC local, but the TLBI broadcast?
>
> If this only needs ot be local, a DSB NSH will be sufficient.

That's a good question. This part was directly copied from the existing function
asm_mmu_enable. As far as I can tell, the TLBI and DSB should be local when
enabling the MMU. We should only need the inner shareable versions of the
operations (followed by an isb) when we modify the translation table used by all
CPUs.

>
> Thanks,
> Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test
  2019-08-29  8:18     ` Alexandru Elisei
@ 2019-08-29 10:19       ` Mark Rutland
  0 siblings, 0 replies; 34+ messages in thread
From: Mark Rutland @ 2019-08-29 10:19 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: maz, pbonzini, kvmarm, kvm, andre.przywara

On Thu, Aug 29, 2019 at 09:18:35AM +0100, Alexandru Elisei wrote:
> On 8/28/19 3:09 PM, Mark Rutland wrote:
> > On Wed, Aug 28, 2019 at 02:38:19PM +0100, Alexandru Elisei wrote:
> >> +	/*
> >> +	 * AArch64 treats all regions writable at EL0 as PXN.
> > I didn't think that was the case, and I can't find wording to that
> > effect in the ARM ARM (looking at ARM DDI 0487E.a). Where is that
> > stated?
> 
> It's in ARM DDI 0487E.a, table D5-33, footnote c: "Not executable, because
> AArch64 execution treats all regions writable at EL0 as being PXN". I'll update
> the comment to include the quote.

Interesting; I was not aware of that particular behaviour. Thanks
for the pointer!

Mark.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors
  2019-08-28 15:14     ` Alexandru Elisei
@ 2019-09-02 14:55       ` Alexandru Elisei
  2019-09-03  6:37         ` Andrew Jones
  0 siblings, 1 reply; 34+ messages in thread
From: Alexandru Elisei @ 2019-09-02 14:55 UTC (permalink / raw)
  To: Andrew Jones; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On 8/28/19 4:14 PM, Alexandru Elisei wrote:

> On 8/28/19 3:45 PM, Andrew Jones wrote:
>> On Wed, Aug 28, 2019 at 02:38:17PM +0100, Alexandru Elisei wrote:
>>> The psci test performs a series of CPU_ON/CPU_OFF cycles for CPU 1. This is
>>> done by setting the entry point for the CPU_ON call to the physical address
>>> of the C function cpu_psci_cpu_die.
>>>
>>> The compiler is well within its rights to use the stack when generating
>>> code for cpu_psci_cpu_die.  However, because no stack initialization has
>>> been done, the stack pointer is zero, as set by KVM when creating the VCPU.
>>> This causes a data abort without a change in exception level. The VBAR_EL1
>>> register is also zero (the KVM reset value for VBAR_EL1), the MMU is off,
>>> and we end up trying to fetch instructions from address 0x200.
>>>
>>> At this point, a stage 2 instruction abort is generated which is taken to
>>> KVM. KVM interprets this as an instruction fetch from an I/O region, and
>>> injects a prefetch abort into the guest. Prefetch abort is a synchronous
>>> exception, and on guest return the VCPU PC will be set to VBAR_EL1 + 0x200,
>>> which is...  0x200. The VCPU ends up in an infinite loop causing a prefetch
>>> abort while fetching the instruction to service the said abort.
>>>
>>> cpu_psci_cpu_die is basically a wrapper over the HVC instruction, so
>>> provide an assembly implementation for the function which will serve as the
>>> entry point for CPU_ON.
>>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>>  arm/cstart.S   | 7 +++++++
>>>  arm/cstart64.S | 7 +++++++
>>>  arm/psci.c     | 5 +++--
>>>  3 files changed, 17 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/arm/cstart.S b/arm/cstart.S
>>> index 114726feab82..5d4fe4b1570b 100644
>>> --- a/arm/cstart.S
>>> +++ b/arm/cstart.S
>>> @@ -7,6 +7,7 @@
>>>   */
>>>  #define __ASSEMBLY__
>>>  #include <auxinfo.h>
>>> +#include <linux/psci.h>
>>>  #include <asm/thread_info.h>
>>>  #include <asm/asm-offsets.h>
>>>  #include <asm/ptrace.h>
>>> @@ -138,6 +139,12 @@ secondary_entry:
>>>  	blx	r0
>>>  	b	do_idle
>>>  
>>> +.global asm_cpu_psci_cpu_die
>>> +asm_cpu_psci_cpu_die:
>>> +	ldr	r0, =PSCI_0_2_FN_CPU_OFF
>>> +	hvc	#0
>>> +	b	halt
>> Shouldn't we load PSCI_POWER_STATE_TYPE_POWER_DOWN into r1 and
>> zero out r2 and r3, as cpu_psci_cpu_die() does? And maybe we
>> should just do a 'b .' here instead of 'b halt' in order to
>> avoid confusion as to how we ended up in halt(), if the psci
>> invocation were to ever fail.
> Not really, I'm not really sure where the extra parameter in cpu_psci_cpu_die
> comes from. I have looked at ARM DEN 0022D, section 5.1.3, and the CPU_OFFcall
> has exactly one parameter, the function id. I've also looked at how KVM handles
> this call, and it doesn't use anything else other than the function id. Please
> correct me if I missed something.

Did some digging, apparently the power state parameter was required for the very
first version of PSCI. ARM DEN 0022D states that it has been removed in PSCIv0.2:

"7.3 Changes in PSCIv0.2 from first proposal

[..]

Removed power_state parameter for CPU_OFF."

The kvm-unit-tests implementation of psci uses fixed function ids (as opposed to
first psci version, where the ids were taken from the DT), so I think that we
can drop the PSCI_POWER_STATE_TYPE_POWER_DOWN parameter in cpu_psci_cpu_die
altogether. What do you think?

Thanks,
Alex
> As for zero'ing the extra registers, this is not part of the SMC calling
> convention, this is just something that the C code for psci does. The SMC
> calling convention states that registers 0-3 will be modified after the call, so
> having 4 arguments to the psci_invoke function will tell the compiler to
> save/restore the registers in the caller.
>
> As for doing 'b .' instead of branching to halt, that's a good idea, I'll do
> that. But it will only be useful if the last CPU_OFF call fails.
>
> Thanks,
> Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

* Re: [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors
  2019-09-02 14:55       ` Alexandru Elisei
@ 2019-09-03  6:37         ` Andrew Jones
  0 siblings, 0 replies; 34+ messages in thread
From: Andrew Jones @ 2019-09-03  6:37 UTC (permalink / raw)
  To: Alexandru Elisei; +Cc: kvm, maz, andre.przywara, pbonzini, kvmarm

On Mon, Sep 02, 2019 at 03:55:48PM +0100, Alexandru Elisei wrote:
> On 8/28/19 4:14 PM, Alexandru Elisei wrote:
> 
> > On 8/28/19 3:45 PM, Andrew Jones wrote:
> >> On Wed, Aug 28, 2019 at 02:38:17PM +0100, Alexandru Elisei wrote:
> >>> The psci test performs a series of CPU_ON/CPU_OFF cycles for CPU 1. This is
> >>> done by setting the entry point for the CPU_ON call to the physical address
> >>> of the C function cpu_psci_cpu_die.
> >>>
> >>> The compiler is well within its rights to use the stack when generating
> >>> code for cpu_psci_cpu_die.  However, because no stack initialization has
> >>> been done, the stack pointer is zero, as set by KVM when creating the VCPU.
> >>> This causes a data abort without a change in exception level. The VBAR_EL1
> >>> register is also zero (the KVM reset value for VBAR_EL1), the MMU is off,
> >>> and we end up trying to fetch instructions from address 0x200.
> >>>
> >>> At this point, a stage 2 instruction abort is generated which is taken to
> >>> KVM. KVM interprets this as an instruction fetch from an I/O region, and
> >>> injects a prefetch abort into the guest. Prefetch abort is a synchronous
> >>> exception, and on guest return the VCPU PC will be set to VBAR_EL1 + 0x200,
> >>> which is...  0x200. The VCPU ends up in an infinite loop causing a prefetch
> >>> abort while fetching the instruction to service the said abort.
> >>>
> >>> cpu_psci_cpu_die is basically a wrapper over the HVC instruction, so
> >>> provide an assembly implementation for the function which will serve as the
> >>> entry point for CPU_ON.
> >>>
> >>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> >>> ---
> >>>  arm/cstart.S   | 7 +++++++
> >>>  arm/cstart64.S | 7 +++++++
> >>>  arm/psci.c     | 5 +++--
> >>>  3 files changed, 17 insertions(+), 2 deletions(-)
> >>>
> >>> diff --git a/arm/cstart.S b/arm/cstart.S
> >>> index 114726feab82..5d4fe4b1570b 100644
> >>> --- a/arm/cstart.S
> >>> +++ b/arm/cstart.S
> >>> @@ -7,6 +7,7 @@
> >>>   */
> >>>  #define __ASSEMBLY__
> >>>  #include <auxinfo.h>
> >>> +#include <linux/psci.h>
> >>>  #include <asm/thread_info.h>
> >>>  #include <asm/asm-offsets.h>
> >>>  #include <asm/ptrace.h>
> >>> @@ -138,6 +139,12 @@ secondary_entry:
> >>>  	blx	r0
> >>>  	b	do_idle
> >>>  
> >>> +.global asm_cpu_psci_cpu_die
> >>> +asm_cpu_psci_cpu_die:
> >>> +	ldr	r0, =PSCI_0_2_FN_CPU_OFF
> >>> +	hvc	#0
> >>> +	b	halt
> >> Shouldn't we load PSCI_POWER_STATE_TYPE_POWER_DOWN into r1 and
> >> zero out r2 and r3, as cpu_psci_cpu_die() does? And maybe we
> >> should just do a 'b .' here instead of 'b halt' in order to
> >> avoid confusion as to how we ended up in halt(), if the psci
> >> invocation were to ever fail.
> > Not really, I'm not really sure where the extra parameter in cpu_psci_cpu_die
> > comes from. I have looked at ARM DEN 0022D, section 5.1.3, and the CPU_OFFcall
> > has exactly one parameter, the function id. I've also looked at how KVM handles
> > this call, and it doesn't use anything else other than the function id. Please
> > correct me if I missed something.
> 
> Did some digging, apparently the power state parameter was required for the very
> first version of PSCI. ARM DEN 0022D states that it has been removed in PSCIv0.2:
> 
> "7.3 Changes in PSCIv0.2 from first proposal
> 
> [..]
> 
> Removed power_state parameter for CPU_OFF."
> 
> The kvm-unit-tests implementation of psci uses fixed function ids (as opposed to
> first psci version, where the ids were taken from the DT), so I think that we
> can drop the PSCI_POWER_STATE_TYPE_POWER_DOWN parameter in cpu_psci_cpu_die
> altogether. What do you think?

Sounds good to me. Thanks for the digging.

drew

> 
> Thanks,
> Alex
> > As for zero'ing the extra registers, this is not part of the SMC calling
> > convention, this is just something that the C code for psci does. The SMC
> > calling convention states that registers 0-3 will be modified after the call, so
> > having 4 arguments to the psci_invoke function will tell the compiler to
> > save/restore the registers in the caller.
> >
> > As for doing 'b .' instead of branching to halt, that's a good idea, I'll do
> > that. But it will only be useful if the last CPU_OFF call fails.
> >
> > Thanks,
> > Alex
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

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

end of thread, back to index

Thread overview: 34+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-28 13:38 [kvm-unit-tests RFC PATCH 00/16] arm64: Run at EL2 Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 01/16] arm: selftest.c: Remove redundant check for Exception Level Alexandru Elisei
2019-08-28 14:32   ` Andrew Jones
2019-08-28 15:39     ` Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 02/16] arm/arm64: psci: Don't run C code without stack or vectors Alexandru Elisei
2019-08-28 14:45   ` Andrew Jones
2019-08-28 15:14     ` Alexandru Elisei
2019-09-02 14:55       ` Alexandru Elisei
2019-09-03  6:37         ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 03/16] lib: arm/arm64: Add missing include for alloc_page.h in pgtable.h Alexandru Elisei
2019-08-28 14:47   ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 04/16] arm/arm64: selftest: Add prefetch abort test Alexandru Elisei
2019-08-28 14:09   ` Mark Rutland
2019-08-29  8:18     ` Alexandru Elisei
2019-08-29 10:19       ` Mark Rutland
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 05/16] arm64: timer: Write to ICENABLER to disable timer IRQ Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 06/16] arm64: timer: EOIR the interrupt after masking the timer Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 07/16] arm64: timer: Test behavior when timer disabled or masked Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 08/16] lib: arm/arm64: Refuse to disable the MMU with non-identity stack pointer Alexandru Elisei
2019-08-28 14:55   ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 09/16] lib: arm/arm64: Invalidate TLB before enabling MMU Alexandru Elisei
2019-08-28 14:59   ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 10/16] lib: Add UL and ULL definitions to linux/const.h Alexandru Elisei
2019-08-28 15:10   ` Andrew Jones
2019-08-28 15:46     ` Alexandru Elisei
2019-08-28 16:19       ` Andrew Jones
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 11/16] lib: arm64: Run existing tests at EL2 Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 12/16] arm64: timer: Add test for EL2 timers Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 13/16] arm64: selftest: Add basic test for EL2 Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 14/16] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
2019-08-28 14:19   ` Mark Rutland
2019-08-29  8:36     ` Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 15/16] arm64: selftest: Expand EL2 test to disable and re-enable VHE Alexandru Elisei
2019-08-28 13:38 ` [kvm-unit-tests RFC PATCH 16/16] arm64: timer: Run tests with VHE disabled Alexandru Elisei

KVM ARM Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/kvmarm/0 kvmarm/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 kvmarm kvmarm/ https://lore.kernel.org/kvmarm \
		kvmarm@lists.cs.columbia.edu kvmarm@archiver.kernel.org
	public-inbox-index kvmarm

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/edu.columbia.cs.lists.kvmarm


AGPL code for this site: git clone https://public-inbox.org/ public-inbox