* [kvm-unit-tests RFC PATCH v3 1/7] lib: Add _UL and _ULL definitions to linux/const.h
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 2/7] lib: arm64: Run existing tests at EL2 Alexandru Elisei
` (5 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm
Cc: pbonzini, drjones, maz, andre.przywara, Laurent Vivier,
Thomas Huth, David Hildenbrand
There is an UL macro defined in lib/arm64/asm/pgtable-hwdef.h. Replace
it with the _UL macro and put it in lib/linux/const.h, where it can be
used in other files. To keep things consistent with Linux's
include/uapi/linux/const.h file, also add an _ULL macro.
Cc: Drew Jones <drjones@redhat.com>
Cc: Laurent Vivier <lvivier@redhat.com>
Cc: Thomas Huth <thuth@redhat.com>
Cc: David Hildenbrand <david@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
lib/linux/const.h | 7 ++++--
lib/asm-generic/page.h | 2 +-
lib/arm/asm/page.h | 2 +-
lib/arm/asm/pgtable-hwdef.h | 22 ++++++++++---------
lib/arm/asm/thread_info.h | 3 ++-
lib/arm64/asm/page.h | 2 +-
lib/arm64/asm/pgtable-hwdef.h | 50 +++++++++++++++++++++----------------------
lib/x86/asm/page.h | 2 +-
8 files changed, 48 insertions(+), 42 deletions(-)
diff --git a/lib/linux/const.h b/lib/linux/const.h
index c872bfd25e13..1453a886d041 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/asm-generic/page.h b/lib/asm-generic/page.h
index 5ed086129657..0b495ad090b7 100644
--- a/lib/asm-generic/page.h
+++ b/lib/asm-generic/page.h
@@ -12,7 +12,7 @@
#include <linux/const.h>
#define PAGE_SHIFT 12
-#define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT)
+#define PAGE_SIZE (_UL(1) << PAGE_SHIFT)
#define PAGE_MASK (~(PAGE_SIZE-1))
#ifndef __ASSEMBLY__
diff --git a/lib/arm/asm/page.h b/lib/arm/asm/page.h
index 039c9f7b3d49..37c9f2f12eb0 100644
--- a/lib/arm/asm/page.h
+++ b/lib/arm/asm/page.h
@@ -9,7 +9,7 @@
#include <linux/const.h>
#define PAGE_SHIFT 12
-#define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT)
+#define PAGE_SIZE (_UL(1) << PAGE_SHIFT)
#define PAGE_MASK (~(PAGE_SIZE-1))
#ifndef __ASSEMBLY__
diff --git a/lib/arm/asm/pgtable-hwdef.h b/lib/arm/asm/pgtable-hwdef.h
index 4107e188014a..66288ebb717c 100644
--- a/lib/arm/asm/pgtable-hwdef.h
+++ b/lib/arm/asm/pgtable-hwdef.h
@@ -9,9 +9,11 @@
* This work is licensed under the terms of the GNU GPL, version 2.
*/
+#include <linux/const.h>
+
#define PTRS_PER_PGD 4
#define PGDIR_SHIFT 30
-#define PGDIR_SIZE (_AC(1,UL) << PGDIR_SHIFT)
+#define PGDIR_SIZE (_UL(1) << PGDIR_SHIFT)
#define PGDIR_MASK (~((1 << PGDIR_SHIFT) - 1))
#define PGD_VALID (_AT(pgdval_t, 1) << 0)
@@ -20,7 +22,7 @@
#define PTRS_PER_PMD 512
#define PMD_SHIFT 21
-#define PMD_SIZE (_AC(1,UL) << PMD_SHIFT)
+#define PMD_SIZE (_UL(1) << PMD_SHIFT)
#define PMD_MASK (~((1 << PMD_SHIFT) - 1))
#define L_PMD_SECT_VALID (_AT(pmdval_t, 1) << 0)
@@ -109,14 +111,14 @@
* 40-bit physical address supported.
*/
#define PHYS_MASK_SHIFT (40)
-#define PHYS_MASK ((_AC(1, ULL) << PHYS_MASK_SHIFT) - 1)
+#define PHYS_MASK ((_ULL(1) << PHYS_MASK_SHIFT) - 1)
-#define TTBCR_IRGN0_WBWA (_AC(1, UL) << 8)
-#define TTBCR_ORGN0_WBWA (_AC(1, UL) << 10)
-#define TTBCR_SH0_SHARED (_AC(3, UL) << 12)
-#define TTBCR_IRGN1_WBWA (_AC(1, UL) << 24)
-#define TTBCR_ORGN1_WBWA (_AC(1, UL) << 26)
-#define TTBCR_SH1_SHARED (_AC(3, UL) << 28)
-#define TTBCR_EAE (_AC(1, UL) << 31)
+#define TTBCR_IRGN0_WBWA (_UL(1) << 8)
+#define TTBCR_ORGN0_WBWA (_UL(1) << 10)
+#define TTBCR_SH0_SHARED (_UL(3) << 12)
+#define TTBCR_IRGN1_WBWA (_UL(1) << 24)
+#define TTBCR_ORGN1_WBWA (_UL(1) << 26)
+#define TTBCR_SH1_SHARED (_UL(3) << 28)
+#define TTBCR_EAE (_UL(1) << 31)
#endif /* _ASMARM_PGTABLE_HWDEF_H_ */
diff --git a/lib/arm/asm/thread_info.h b/lib/arm/asm/thread_info.h
index 80ab3954a6b0..7ff1570e1381 100644
--- a/lib/arm/asm/thread_info.h
+++ b/lib/arm/asm/thread_info.h
@@ -7,6 +7,7 @@
*
* This work is licensed under the terms of the GNU GPL, version 2.
*/
+#include <linux/const.h>
#include <asm/page.h>
#define MIN_THREAD_SHIFT 14 /* THREAD_SIZE == 16K */
@@ -16,7 +17,7 @@
#define THREAD_MASK PAGE_MASK
#else
#define THREAD_SHIFT MIN_THREAD_SHIFT
-#define THREAD_SIZE (_AC(1,UL) << THREAD_SHIFT)
+#define THREAD_SIZE (_UL(1) << THREAD_SHIFT)
#define THREAD_MASK (~(THREAD_SIZE-1))
#endif
diff --git a/lib/arm64/asm/page.h b/lib/arm64/asm/page.h
index 46af552b91c7..07b3b61c2474 100644
--- a/lib/arm64/asm/page.h
+++ b/lib/arm64/asm/page.h
@@ -16,7 +16,7 @@
#define VA_BITS 42
#define PAGE_SHIFT 16
-#define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT)
+#define PAGE_SIZE (_UL(1) << PAGE_SHIFT)
#define PAGE_MASK (~(PAGE_SIZE-1))
#ifndef __ASSEMBLY__
diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
index 33524899e5fa..a434a01b24fc 100644
--- a/lib/arm64/asm/pgtable-hwdef.h
+++ b/lib/arm64/asm/pgtable-hwdef.h
@@ -9,7 +9,7 @@
* This work is licensed under the terms of the GNU GPL, version 2.
*/
-#define UL(x) _AC(x, UL)
+#include <linux/const.h>
#define PTRS_PER_PTE (1 << (PAGE_SHIFT - 3))
@@ -18,7 +18,7 @@
* (depending on the configuration, this level can be 0, 1 or 2).
*/
#define PGDIR_SHIFT ((PAGE_SHIFT - 3) * PGTABLE_LEVELS + 3)
-#define PGDIR_SIZE (_AC(1, UL) << PGDIR_SHIFT)
+#define PGDIR_SIZE (_UL(1) << PGDIR_SHIFT)
#define PGDIR_MASK (~(PGDIR_SIZE-1))
#define PTRS_PER_PGD (1 << (VA_BITS - PGDIR_SHIFT))
@@ -27,14 +27,14 @@
/* From include/asm-generic/pgtable-nopmd.h */
#define PMD_SHIFT PGDIR_SHIFT
#define PTRS_PER_PMD 1
-#define PMD_SIZE (UL(1) << PMD_SHIFT)
+#define PMD_SIZE (_UL(1) << PMD_SHIFT)
#define PMD_MASK (~(PMD_SIZE-1))
/*
* Section address mask and size definitions.
*/
#define SECTION_SHIFT PMD_SHIFT
-#define SECTION_SIZE (_AC(1, UL) << SECTION_SHIFT)
+#define SECTION_SIZE (_UL(1) << SECTION_SHIFT)
#define SECTION_MASK (~(SECTION_SIZE-1))
/*
@@ -93,31 +93,31 @@
* Highest possible physical address supported.
*/
#define PHYS_MASK_SHIFT (48)
-#define PHYS_MASK ((UL(1) << PHYS_MASK_SHIFT) - 1)
+#define PHYS_MASK ((_UL(1) << PHYS_MASK_SHIFT) - 1)
/*
* 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_TG0_4K (UL(0) << 14)
-#define TCR_TG0_64K (UL(1) << 14)
-#define TCR_TG0_16K (UL(2) << 14)
-#define TCR_TG1_16K (UL(1) << 30)
-#define TCR_TG1_4K (UL(2) << 30)
-#define TCR_TG1_64K (UL(3) << 30)
-#define TCR_ASID16 (UL(1) << 36)
-#define TCR_TBI0 (UL(1) << 37)
+#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_TG0_4K (_UL(0) << 14)
+#define TCR_TG0_64K (_UL(1) << 14)
+#define TCR_TG0_16K (_UL(2) << 14)
+#define TCR_TG1_16K (_UL(1) << 30)
+#define TCR_TG1_4K (_UL(2) << 30)
+#define TCR_TG1_64K (_UL(3) << 30)
+#define TCR_ASID16 (_UL(1) << 36)
+#define TCR_TBI0 (_UL(1) << 37)
/*
* Memory types available.
diff --git a/lib/x86/asm/page.h b/lib/x86/asm/page.h
index 073580a80f0c..5d3af06d14b4 100644
--- a/lib/x86/asm/page.h
+++ b/lib/x86/asm/page.h
@@ -14,7 +14,7 @@ typedef unsigned long pteval_t;
typedef unsigned long pgd_t;
#define PAGE_SHIFT 12
-#define PAGE_SIZE (_AC(1,UL) << PAGE_SHIFT)
+#define PAGE_SIZE (_UL(1) << PAGE_SHIFT)
#define PAGE_MASK (~(PAGE_SIZE-1))
#ifndef __ASSEMBLY__
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [kvm-unit-tests RFC PATCH v3 2/7] lib: arm64: Run existing tests at EL2
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 1/7] lib: Add _UL and _ULL definitions to linux/const.h Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 3/7] arm64: timer: Add test for EL2 timers Alexandru Elisei
` (4 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, drjones, maz, andre.przywara
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 virtual EL2 and a regular 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 all
existing tests have been modified to run at EL2.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
In order to run the prefetch abort test, the patch proposed by me at [1]
needs to be applied to the host.
[1] https://www.spinics.net/lists/arm-kernel/msg750310.html
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 | 38 +++++++++++---
arm/timer.c | 127 +++++++++++++++++++++++++++++++++++++++-------
11 files changed, 260 insertions(+), 27 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 fd508c02f30d..7e9f76d73f1b 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)
+
#define CTR_EL0_DMINLINE_SHIFT 16
#define CTR_EL0_DMINLINE_MASK (0xf << 16)
#define CTR_EL0_DMINLINE(x) \
diff --git a/lib/arm64/asm/sysreg.h b/lib/arm64/asm/sysreg.h
index a03830bceb8f..d625d8638407 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
@@ -38,6 +46,17 @@
asm volatile("msr " xstr(r) ", %x0" : : "rZ" (__val)); \
} while (0)
+#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)
+
asm(
" .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\n"
" .equ .L__reg_num_x\\num, \\num\n"
@@ -52,5 +71,6 @@ asm(
" .inst 0xd5000000|(\\sreg)|(.L__reg_num_\\rt)\n"
" .endm\n"
);
+
#endif /* __ASSEMBLY__ */
#endif /* _ASMARM64_SYSREG_H_ */
diff --git a/lib/arm/psci.c b/lib/arm/psci.c
index 936c83948b6a..f0c571cfc5a3 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 54fc19a20942..2f23cb07f4fc 100644
--- a/lib/arm/setup.c
+++ b/lib/arm/setup.c
@@ -21,6 +21,7 @@
#include <asm/setup.h>
#include <asm/page.h>
#include <asm/processor.h>
+#include <asm/psci.h>
#include <asm/smp.h>
#include "io.h"
@@ -129,6 +130,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 f28066d40145..2030a7a09107 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 7e7f8b2e8f0b..20f0dd8cc499 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 .
+1:
hvc #0
b .
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 11dd432f4e6f..1538e0e68483 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -224,6 +224,7 @@ static void check_pabt(void)
__builtin_unreachable();
}
#elif defined(__aarch64__)
+static unsigned long expected_level;
/*
* Capture the current register state and execute an instruction
@@ -270,8 +271,7 @@ static bool check_regs(struct pt_regs *regs)
{
unsigned i;
- /* exception handlers should always run in EL1 */
- if (current_level() != CurrentEL_EL1)
+ if (current_level() != expected_level)
return false;
for (i = 0; i < ARRAY_SIZE(regs->regs); ++i) {
@@ -295,7 +295,11 @@ static enum vector check_vector_prep(void)
return EL0_SYNC_64;
asm volatile("mrs %0, daif" : "=r" (daif) ::);
- expected_regs.pstate = daif | PSR_MODE_EL1h;
+ expected_regs.pstate = daif;
+ if (current_level() == CurrentEL_EL1)
+ expected_regs.pstate |= PSR_MODE_EL1h;
+ else
+ expected_regs.pstate |= PSR_MODE_EL2h;
return EL1H_SYNC;
}
@@ -311,8 +315,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);
@@ -433,11 +437,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),
@@ -465,6 +487,10 @@ int main(int argc, char **argv)
if (argc < 2)
report_abort("no test specified");
+#if defined(__aarch64__)
+ expected_level = current_level();
+#endif
+
report_prefix_push(argv[1]);
if (strcmp(argv[1], "setup") == 0) {
diff --git a/arm/timer.c b/arm/timer.c
index a0b57afd4fe4..0dc27e99b3b6 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;
@@ -264,7 +349,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();
}
@@ -274,7 +359,7 @@ static void test_ptimer(void)
return;
report_prefix_push("ptimer-busy-loop");
- test_timer(&ptimer_info);
+ test_timer(ptimer);
report_prefix_pop();
}
@@ -285,6 +370,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);
@@ -292,14 +385,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)) {
@@ -333,14 +426,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [kvm-unit-tests RFC PATCH v3 3/7] arm64: timer: Add test for EL2 timers
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 1/7] lib: Add _UL and _ULL definitions to linux/const.h Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 2/7] lib: arm64: Run existing tests at EL2 Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 4/7] arm64: selftest: Add basic test for EL2 Alexandru Elisei
` (3 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, drjones, maz, andre.przywara
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>
---
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 d625d8638407..0bf7839cbc4e 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 0dc27e99b3b6..88de84bc1bcf 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;
@@ -363,6 +451,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;
@@ -370,12 +472,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");
@@ -391,6 +495,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);
@@ -434,12 +551,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();
@@ -447,13 +574,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [kvm-unit-tests RFC PATCH v3 4/7] arm64: selftest: Add basic test for EL2
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
` (2 preceding siblings ...)
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 3/7] arm64: timer: Add test for EL2 timers Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
` (2 subsequent siblings)
6 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, drjones, maz, andre.przywara
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 | 35 ++++++++++++++++++++++++++++++++++-
arm/unittests.cfg | 8 ++++++++
3 files changed, 48 insertions(+), 1 deletion(-)
diff --git a/lib/arm64/asm/processor.h b/lib/arm64/asm/processor.h
index 7e9f76d73f1b..4bbd82d9bfde 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -116,5 +116,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 1538e0e68483..a30e101a4920 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -223,6 +223,11 @@ static void check_pabt(void)
test_exception("ldr r2, =" xstr(PABT_ADDR), "bx r2", "");
__builtin_unreachable();
}
+
+static void check_el2(void)
+{
+ report(false, "EL2 only available on arm64");
+}
#elif defined(__aarch64__)
static unsigned long expected_level;
@@ -402,6 +407,31 @@ static void user_psci_system_off(struct pt_regs *regs, unsigned int esr)
{
__user_psci_system_off();
}
+
+#define ID_AA64MMFR1_VHE_MASK (0xf << 8)
+#define ID_AA64MMFR1_VHE_SUPPORTED (1 << 8)
+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(current_level() == CurrentEL_EL2, "CPU(%3d) Running at EL2", cpu);
+ report(vhe_supported() && vhe_enabled(),
+ "CPU(%3d) VHE supported and enabled", cpu);
+}
+
+static bool psci_check(void);
+static void check_el2(void)
+{
+ report(psci_check(), "PSCI conduit");
+ on_cpus(check_el2_cpu, NULL);
+}
#endif
static void check_vectors(void *arg __unused)
@@ -453,7 +483,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");
@@ -516,6 +545,10 @@ int main(int argc, char **argv)
report(cpumask_full(&valid), "MPIDR test on all CPUs");
report_info("%d CPUs reported back", nr_cpus);
+ } 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 daeb5a09ad39..4b2054ad1e36 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
^ permalink raw reply related [flat|nested] 12+ messages in thread* [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
` (3 preceding siblings ...)
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 4/7] arm64: selftest: Add basic test for EL2 Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
2020-01-30 17:40 ` Marc Zyngier
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 6/7] arm64: selftest: Expand EL2 test to disable and re-enable VHE Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 7/7] arm64: timer: Run tests with VHE disabled Alexandru Elisei
6 siblings, 1 reply; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, drjones, maz, andre.przywara
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/arm64/asm/mmu.h | 11 ++-
lib/arm64/asm/pgtable-hwdef.h | 53 ++++++++---
lib/arm64/asm/processor.h | 19 +++-
lib/arm64/processor.c | 37 +++++++-
arm/cstart64.S | 204 ++++++++++++++++++++++++++++++++++++++++--
5 files changed, 300 insertions(+), 24 deletions(-)
diff --git a/lib/arm64/asm/mmu.h b/lib/arm64/asm/mmu.h
index 72d75eafc882..e800835658a0 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);
isb();
}
diff --git a/lib/arm64/asm/pgtable-hwdef.h b/lib/arm64/asm/pgtable-hwdef.h
index a434a01b24fc..9eb180de26cf 100644
--- a/lib/arm64/asm/pgtable-hwdef.h
+++ b/lib/arm64/asm/pgtable-hwdef.h
@@ -98,18 +98,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)
@@ -119,6 +143,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 4bbd82d9bfde..70a5261dfe97 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -18,8 +18,15 @@
#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 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
#define CTR_EL0_DMINLINE_SHIFT 16
#define CTR_EL0_DMINLINE_MASK (0xf << 16)
@@ -72,6 +79,9 @@ extern void show_regs(struct pt_regs *regs);
extern bool get_far(unsigned int esr, unsigned long *far);
extern void init_dcache_line_size(void);
+extern void disable_vhe(void);
+extern void enable_vhe(void);
+
static inline unsigned long current_level(void)
{
unsigned long el;
@@ -122,5 +132,10 @@ 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;
+}
+
#endif /* !__ASSEMBLY__ */
#endif /* _ASMARM64_PROCESSOR_H_ */
diff --git a/lib/arm64/processor.c b/lib/arm64/processor.c
index 2030a7a09107..45ae8ec649af 100644
--- a/lib/arm64/processor.c
+++ b/lib/arm64/processor.c
@@ -86,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:
@@ -270,3 +273,35 @@ void init_dcache_line_size(void)
/* DminLine is log2 of the number of words in the smallest cache line */
dcache_line_size = 1 << (CTR_EL0_DMINLINE(ctr) + 2);
}
+
+extern void asm_disable_vhe(void);
+void disable_vhe(void)
+{
+ u64 daif;
+
+ assert(current_level() == CurrentEL_EL2 && vhe_enabled());
+
+ /*
+ * Make sure we don't take any exceptions while we run with the MMU off.
+ * On secondaries the stack is allocated from the vmalloc area, which
+ * means it isn't identity mapped, and the exception handling code uses
+ * it. Taking an exception in this case would be very bad.
+ */
+ asm volatile("mrs %0, daif" : "=r" (daif) : : "memory");
+ local_irq_disable();
+ asm_disable_vhe();
+ asm volatile("msr daif, %0" : : "r" (daif) : "memory");
+}
+
+extern void asm_enable_vhe(void);
+void enable_vhe(void)
+{
+ u64 daif;
+
+ assert(current_level() == CurrentEL_EL2 && !vhe_enabled());
+
+ asm volatile("mrs %0, daif" : "=r" (daif) : : "memory");
+ local_irq_disable();
+ asm_enable_vhe();
+ asm volatile("msr daif, %0" : : "r" (daif) : "memory");
+}
diff --git a/arm/cstart64.S b/arm/cstart64.S
index 20f0dd8cc499..1a1515453c0a 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
@@ -203,7 +210,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 */
@@ -228,6 +235,41 @@ asm_mmu_enable:
ret
+asm_mmu_enable_nvhe:
+ tlbi alle2
+ dsb nsh
+
+ /* 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 */
+ ldr x1, =MAIR(0x00, MT_DEVICE_nGnRnE) | \
+ MAIR(0x04, MT_DEVICE_nGnRE) | \
+ MAIR(0x0c, MT_DEVICE_GRE) | \
+ MAIR(0x44, MT_NORMAL_NC) | \
+ MAIR(0xff, MT_NORMAL)
+ msr mair_el1, x1
+
+ msr ttbr0_el1, x0
+ isb
+
+ /* SCTLR */
+ ldr x1, =SCTLR_EL2_RES1 | \
+ SCTLR_EL2_C | \
+ SCTLR_EL2_I | \
+ SCTLR_EL2_M
+ msr sctlr_el2, x1
+ isb
+
+ ret
+
/* Taken with small changes from arch/arm64/incluse/asm/assembler.h */
.macro dcache_by_line_op op, domain, start, end, tmp1, tmp2
adrp \tmp1, dcache_line_size
@@ -242,21 +284,61 @@ asm_mmu_enable:
dsb \domain
.endm
+clean_inval_cache:
+ adrp x0, __phys_offset
+ ldr x0, [x0, :lo12:__phys_offset]
+ adrp x1, __phys_end
+ ldr x1, [x1, :lo12:__phys_end]
+ dcache_by_line_op civac, sy, x0, x1, x2, x3
+ isb
+ ret
+
.globl asm_mmu_disable
asm_mmu_disable:
mrs x0, sctlr_el1
bic x0, x0, SCTLR_EL1_M
msr sctlr_el1, x0
isb
+ b clean_inval_cache
- /* Clean + invalidate the entire memory */
- adrp x0, __phys_offset
- ldr x0, [x0, :lo12:__phys_offset]
- adrp x1, __phys_end
- ldr x1, [x1, :lo12:__phys_end]
- dcache_by_line_op civac, sy, x0, x1, x2, x3
+asm_mmu_disable_nvhe:
+ mrs x0, sctlr_el2
+ bic x0, x0, SCTLR_EL2_M
+ msr sctlr_el2, x0
+ isb
+ b clean_inval_cache
+
+.globl asm_disable_vhe
+asm_disable_vhe:
+ str x30, [sp, #-16]!
+
+ bl asm_mmu_disable
+ msr hcr_el2, xzr
+ isb
+ bl exceptions_init_nvhe
+ /* Make asm_mmu_enable_nvhe happy by having TTBR0 value in x0. */
+ mrs x0, ttbr0_el1
+ 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
+ bl exceptions_init
+ /* Make asm_mmu_enable happy by having TTBR0 value in x0. */
+ mrs x0, ttbr0_el1
isb
+ bl asm_mmu_enable
+ ldr x30, [sp], #16
ret
/*
@@ -349,6 +431,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
@@ -377,3 +545,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
^ permalink raw reply related [flat|nested] 12+ messages in thread* Re: [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
@ 2020-01-30 17:40 ` Marc Zyngier
2020-01-31 9:52 ` Alexandru Elisei
0 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2020-01-30 17:40 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, pbonzini, drjones, andre.przywara
Hi Alexandru,
On 2020-01-02 13:46, 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.
I've been using this test case to debug my NV code... only to realize
after a few hours of banging my head on the wall that it is the test
that needed debugging, see below... ;-)
>
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
> lib/arm64/asm/mmu.h | 11 ++-
> lib/arm64/asm/pgtable-hwdef.h | 53 ++++++++---
> lib/arm64/asm/processor.h | 19 +++-
> lib/arm64/processor.c | 37 +++++++-
> arm/cstart64.S | 204
> ++++++++++++++++++++++++++++++++++++++++--
> 5 files changed, 300 insertions(+), 24 deletions(-)
[...]
> --- 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
> @@ -203,7 +210,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 */
> @@ -228,6 +235,41 @@ asm_mmu_enable:
>
> ret
>
> +asm_mmu_enable_nvhe:
Note the "_nvhe" suffix, which implies that...
> + tlbi alle2
> + dsb nsh
> +
> + /* 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 */
> + ldr x1, =MAIR(0x00, MT_DEVICE_nGnRnE) | \
> + MAIR(0x04, MT_DEVICE_nGnRE) | \
> + MAIR(0x0c, MT_DEVICE_GRE) | \
> + MAIR(0x44, MT_NORMAL_NC) | \
> + MAIR(0xff, MT_NORMAL)
> + msr mair_el1, x1
... this should be mair_el2...
> +
> + msr ttbr0_el1, x0
... and this should be ttbr0_el2.
> + isb
> +
> + /* SCTLR */
> + ldr x1, =SCTLR_EL2_RES1 | \
> + SCTLR_EL2_C | \
> + SCTLR_EL2_I | \
> + SCTLR_EL2_M
> + msr sctlr_el2, x1
> + isb
> +
> + ret
> +
> /* Taken with small changes from arch/arm64/incluse/asm/assembler.h */
> .macro dcache_by_line_op op, domain, start, end, tmp1, tmp2
> adrp \tmp1, dcache_line_size
> @@ -242,21 +284,61 @@ asm_mmu_enable:
> dsb \domain
> .endm
>
> +clean_inval_cache:
> + adrp x0, __phys_offset
> + ldr x0, [x0, :lo12:__phys_offset]
> + adrp x1, __phys_end
> + ldr x1, [x1, :lo12:__phys_end]
> + dcache_by_line_op civac, sy, x0, x1, x2, x3
> + isb
> + ret
> +
> .globl asm_mmu_disable
> asm_mmu_disable:
> mrs x0, sctlr_el1
> bic x0, x0, SCTLR_EL1_M
> msr sctlr_el1, x0
> isb
> + b clean_inval_cache
>
> - /* Clean + invalidate the entire memory */
> - adrp x0, __phys_offset
> - ldr x0, [x0, :lo12:__phys_offset]
> - adrp x1, __phys_end
> - ldr x1, [x1, :lo12:__phys_end]
> - dcache_by_line_op civac, sy, x0, x1, x2, x3
> +asm_mmu_disable_nvhe:
> + mrs x0, sctlr_el2
> + bic x0, x0, SCTLR_EL2_M
> + msr sctlr_el2, x0
> + isb
> + b clean_inval_cache
> +
> +.globl asm_disable_vhe
> +asm_disable_vhe:
> + str x30, [sp, #-16]!
> +
> + bl asm_mmu_disable
> + msr hcr_el2, xzr
> + isb
At this stage, VHE is off...
> + bl exceptions_init_nvhe
> + /* Make asm_mmu_enable_nvhe happy by having TTBR0 value in x0. */
> + mrs x0, ttbr0_el1
... so this is going to sample the wrong TTBR. It really should be
TTBR0_EL2!
> + isb
nit: this ISB is useless, as you will have a dependency on x0 anyway.
With these fixes (and a few more terrible hacks to synchronize HCR_EL2
on ARMv8.4-NV), I can run this test reliably.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE
2020-01-30 17:40 ` Marc Zyngier
@ 2020-01-31 9:52 ` Alexandru Elisei
2020-01-31 11:26 ` Marc Zyngier
0 siblings, 1 reply; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-31 9:52 UTC (permalink / raw)
To: Marc Zyngier; +Cc: kvm, pbonzini, drjones, andre.przywara
Hi,
Thank you for testing the patches!
On 1/30/20 5:40 PM, Marc Zyngier wrote:
> Hi Alexandru,
>
> On 2020-01-02 13:46, 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.
>
> I've been using this test case to debug my NV code... only to realize
> after a few hours of banging my head on the wall that it is the test
> that needed debugging, see below... ;-)
>
>>
>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>> ---
>> lib/arm64/asm/mmu.h | 11 ++-
>> lib/arm64/asm/pgtable-hwdef.h | 53 ++++++++---
>> lib/arm64/asm/processor.h | 19 +++-
>> lib/arm64/processor.c | 37 +++++++-
>> arm/cstart64.S | 204 ++++++++++++++++++++++++++++++++++++++++--
>> 5 files changed, 300 insertions(+), 24 deletions(-)
>
> [...]
>
>> --- 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
>> @@ -203,7 +210,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 */
>> @@ -228,6 +235,41 @@ asm_mmu_enable:
>>
>> ret
>>
>> +asm_mmu_enable_nvhe:
>
> Note the "_nvhe" suffix, which implies that...
>
>> + tlbi alle2
>> + dsb nsh
>> +
>> + /* 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 */
>> + ldr x1, =MAIR(0x00, MT_DEVICE_nGnRnE) | \
>> + MAIR(0x04, MT_DEVICE_nGnRE) | \
>> + MAIR(0x0c, MT_DEVICE_GRE) | \
>> + MAIR(0x44, MT_NORMAL_NC) | \
>> + MAIR(0xff, MT_NORMAL)
>> + msr mair_el1, x1
>
> ... this should be mair_el2...
>
>> +
>> + msr ttbr0_el1, x0
>
> ... and this should be ttbr0_el2.
The code is definitely confusing, but not because it's wrong, but because it's
doing something useless. From DDI 04876E.a, page D13-3374, the pseudocode for
writing to ttbr0_el1:
[..] elsif PSTATE.EL == EL2 then if HCR_EL2.E2H == '1' then TTBR0_EL2
= X[t];
else TTBR0_EL1 = X[t]; [..]
We want to use the same ttbr0_el2 and mair_el2 values that we were using when VHE
was on. We programmed those values when VHE was on, so we actually wrote them to
ttbr0_el2 and mair_el2. We don't need to write them again now, in fact, all the
previous versions of the series didn't even have the above useless writes (I
assume it was a copy-and-paste mistake when I split the fixes from the el2 patches).
>
>> + isb
>> +
>> + /* SCTLR */
>> + ldr x1, =SCTLR_EL2_RES1 | \
>> + SCTLR_EL2_C | \
>> + SCTLR_EL2_I | \
>> + SCTLR_EL2_M
>> + msr sctlr_el2, x1
>> + isb
>> +
>> + ret
>> +
>> /* Taken with small changes from arch/arm64/incluse/asm/assembler.h */
>> .macro dcache_by_line_op op, domain, start, end, tmp1, tmp2
>> adrp \tmp1, dcache_line_size
>> @@ -242,21 +284,61 @@ asm_mmu_enable:
>> dsb \domain
>> .endm
>>
>> +clean_inval_cache:
>> + adrp x0, __phys_offset
>> + ldr x0, [x0, :lo12:__phys_offset]
>> + adrp x1, __phys_end
>> + ldr x1, [x1, :lo12:__phys_end]
>> + dcache_by_line_op civac, sy, x0, x1, x2, x3
>> + isb
>> + ret
>> +
>> .globl asm_mmu_disable
>> asm_mmu_disable:
>> mrs x0, sctlr_el1
>> bic x0, x0, SCTLR_EL1_M
>> msr sctlr_el1, x0
>> isb
>> + b clean_inval_cache
>>
>> - /* Clean + invalidate the entire memory */
>> - adrp x0, __phys_offset
>> - ldr x0, [x0, :lo12:__phys_offset]
>> - adrp x1, __phys_end
>> - ldr x1, [x1, :lo12:__phys_end]
>> - dcache_by_line_op civac, sy, x0, x1, x2, x3
>> +asm_mmu_disable_nvhe:
>> + mrs x0, sctlr_el2
>> + bic x0, x0, SCTLR_EL2_M
>> + msr sctlr_el2, x0
>> + isb
>> + b clean_inval_cache
>> +
>> +.globl asm_disable_vhe
>> +asm_disable_vhe:
>> + str x30, [sp, #-16]!
>> +
>> + bl asm_mmu_disable
>> + msr hcr_el2, xzr
>> + isb
>
> At this stage, VHE is off...
>
>> + bl exceptions_init_nvhe
>> + /* Make asm_mmu_enable_nvhe happy by having TTBR0 value in x0. */
>> + mrs x0, ttbr0_el1
>
> ... so this is going to sample the wrong TTBR. It really should be
> TTBR0_EL2!
Not really, asm_mmu_enable has one parameter, the PA for the translation tables in
register x0, and we are going to use the same translation tables with VHE off that
we were using with VHE on. Hence the read.//It could have easily been mrs
x0,ttbr0_el2, since they have the same value, which we want to reuse.
I think this confusion stems from the fact that I'm trying to write the registers
again in asm_mmu_enable_nvhe, when we don't have to. And writing to the wrong
registers makes the confusion even worse.
>
>> + isb
>
> nit: this ISB is useless, as you will have a dependency on x0 anyway.
True, I'll remove it.
Thanks,
Alex
>
> With these fixes (and a few more terrible hacks to synchronize HCR_EL2
> on ARMv8.4-NV), I can run this test reliably.
>
> Thanks,
>
> M.
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE
2020-01-31 9:52 ` Alexandru Elisei
@ 2020-01-31 11:26 ` Marc Zyngier
2020-01-31 11:43 ` Alexandru Elisei
0 siblings, 1 reply; 12+ messages in thread
From: Marc Zyngier @ 2020-01-31 11:26 UTC (permalink / raw)
To: Alexandru Elisei; +Cc: kvm, pbonzini, drjones, andre.przywara
Hi Alex,
On 2020-01-31 09:52, Alexandru Elisei wrote:
> Hi,
>
> Thank you for testing the patches!
>
> On 1/30/20 5:40 PM, Marc Zyngier wrote:
>> Hi Alexandru,
>>
>> On 2020-01-02 13:46, 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.
>>
>> I've been using this test case to debug my NV code... only to realize
>> after a few hours of banging my head on the wall that it is the test
>> that needed debugging, see below... ;-)
>>
>>>
>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>> ---
>>> lib/arm64/asm/mmu.h | 11 ++-
>>> lib/arm64/asm/pgtable-hwdef.h | 53 ++++++++---
>>> lib/arm64/asm/processor.h | 19 +++-
>>> lib/arm64/processor.c | 37 +++++++-
>>> arm/cstart64.S | 204
>>> ++++++++++++++++++++++++++++++++++++++++--
>>> 5 files changed, 300 insertions(+), 24 deletions(-)
>>
>> [...]
>>
>>> --- 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
>>> @@ -203,7 +210,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 */
>>> @@ -228,6 +235,41 @@ asm_mmu_enable:
>>>
>>> ret
>>>
>>> +asm_mmu_enable_nvhe:
>>
>> Note the "_nvhe" suffix, which implies that...
>>
>>> + tlbi alle2
>>> + dsb nsh
>>> +
>>> + /* 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 */
>>> + ldr x1, =MAIR(0x00, MT_DEVICE_nGnRnE) | \
>>> + MAIR(0x04, MT_DEVICE_nGnRE) | \
>>> + MAIR(0x0c, MT_DEVICE_GRE) | \
>>> + MAIR(0x44, MT_NORMAL_NC) | \
>>> + MAIR(0xff, MT_NORMAL)
>>> + msr mair_el1, x1
>>
>> ... this should be mair_el2...
>>
>>> +
>>> + msr ttbr0_el1, x0
>>
>> ... and this should be ttbr0_el2.
>
> The code is definitely confusing, but not because it's wrong, but
> because it's
> doing something useless. From DDI 04876E.a, page D13-3374, the
> pseudocode for
> writing to ttbr0_el1:
>
> [..] elsif PSTATE.EL == EL2 then if HCR_EL2.E2H == '1' then
> TTBR0_EL2
> = X[t];
>
> else TTBR0_EL1 = X[t]; [..]
>
> We want to use the same ttbr0_el2 and mair_el2 values that we were
> using when VHE
> was on. We programmed those values when VHE was on, so we actually
> wrote them to
> ttbr0_el2 and mair_el2. We don't need to write them again now, in fact,
> all the
> previous versions of the series didn't even have the above useless
> writes (I
> assume it was a copy-and-paste mistake when I split the fixes from the
> el2 patches).
Fair enough. You're just propagating a dummy value, which is going to
bite you later.
>
>>
>>> + isb
>>> +
>>> + /* SCTLR */
>>> + ldr x1, =SCTLR_EL2_RES1 | \
>>> + SCTLR_EL2_C | \
>>> + SCTLR_EL2_I | \
>>> + SCTLR_EL2_M
>>> + msr sctlr_el2, x1
>>> + isb
>>> +
>>> + ret
>>> +
>>> /* Taken with small changes from arch/arm64/incluse/asm/assembler.h
>>> */
>>> .macro dcache_by_line_op op, domain, start, end, tmp1, tmp2
>>> adrp \tmp1, dcache_line_size
>>> @@ -242,21 +284,61 @@ asm_mmu_enable:
>>> dsb \domain
>>> .endm
>>>
>>> +clean_inval_cache:
>>> + adrp x0, __phys_offset
>>> + ldr x0, [x0, :lo12:__phys_offset]
>>> + adrp x1, __phys_end
>>> + ldr x1, [x1, :lo12:__phys_end]
>>> + dcache_by_line_op civac, sy, x0, x1, x2, x3
>>> + isb
>>> + ret
>>> +
>>> .globl asm_mmu_disable
>>> asm_mmu_disable:
>>> mrs x0, sctlr_el1
>>> bic x0, x0, SCTLR_EL1_M
>>> msr sctlr_el1, x0
>>> isb
>>> + b clean_inval_cache
>>>
>>> - /* Clean + invalidate the entire memory */
>>> - adrp x0, __phys_offset
>>> - ldr x0, [x0, :lo12:__phys_offset]
>>> - adrp x1, __phys_end
>>> - ldr x1, [x1, :lo12:__phys_end]
>>> - dcache_by_line_op civac, sy, x0, x1, x2, x3
>>> +asm_mmu_disable_nvhe:
>>> + mrs x0, sctlr_el2
>>> + bic x0, x0, SCTLR_EL2_M
>>> + msr sctlr_el2, x0
>>> + isb
>>> + b clean_inval_cache
>>> +
>>> +.globl asm_disable_vhe
>>> +asm_disable_vhe:
>>> + str x30, [sp, #-16]!
>>> +
>>> + bl asm_mmu_disable
>>> + msr hcr_el2, xzr
>>> + isb
>>
>> At this stage, VHE is off...
>>
>>> + bl exceptions_init_nvhe
>>> + /* Make asm_mmu_enable_nvhe happy by having TTBR0 value in x0.
>>> */
>>> + mrs x0, ttbr0_el1
>>
>> ... so this is going to sample the wrong TTBR. It really should be
>> TTBR0_EL2!
>
> Not really, asm_mmu_enable has one parameter, the PA for the
> translation tables in
> register x0, and we are going to use the same translation tables with
> VHE off that
> we were using with VHE on. Hence the read.//It could have easily been
> mrs
> x0,ttbr0_el2, since they have the same value, which we want to reuse.
I'm sorry, but if your reasoning that above that VHE's TTBR0_EL1 is the
same as nVHE's TTBR0_EL2 appears correct (they are accessing the same HW
register), this particular read of TTBR0_EL1 is *not* an EL2 register at
all. VHE is off, and you are reading an uninitialized EL1 register (and
it's easy to spot in KVM, as it has the magic poison value).
> I think this confusion stems from the fact that I'm trying to write
> the registers
> again in asm_mmu_enable_nvhe, when we don't have to. And writing to the
> wrong
> registers makes the confusion even worse.
I don't mind the extra writes, or even the confusion. But the above
looks
totally wrong.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
^ permalink raw reply [flat|nested] 12+ messages in thread* Re: [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE
2020-01-31 11:26 ` Marc Zyngier
@ 2020-01-31 11:43 ` Alexandru Elisei
0 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-31 11:43 UTC (permalink / raw)
To: Marc Zyngier; +Cc: kvm, pbonzini, drjones, andre.przywara
Hi,
On 1/31/20 11:26 AM, Marc Zyngier wrote:
> Hi Alex,
>
> On 2020-01-31 09:52, Alexandru Elisei wrote:
>> Hi,
>>
>> Thank you for testing the patches!
>>
>> On 1/30/20 5:40 PM, Marc Zyngier wrote:
>>> Hi Alexandru,
>>>
>>> On 2020-01-02 13:46, 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.
>>>
>>> I've been using this test case to debug my NV code... only to realize
>>> after a few hours of banging my head on the wall that it is the test
>>> that needed debugging, see below... ;-)
>>>
>>>>
>>>> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
>>>> ---
>>>> lib/arm64/asm/mmu.h | 11 ++-
>>>> lib/arm64/asm/pgtable-hwdef.h | 53 ++++++++---
>>>> lib/arm64/asm/processor.h | 19 +++-
>>>> lib/arm64/processor.c | 37 +++++++-
>>>> arm/cstart64.S | 204 ++++++++++++++++++++++++++++++++++++++++--
>>>> 5 files changed, 300 insertions(+), 24 deletions(-)
>>>
>>> [...]
>>>
>>>> --- 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
>>>> @@ -203,7 +210,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 */
>>>> @@ -228,6 +235,41 @@ asm_mmu_enable:
>>>>
>>>> ret
>>>>
>>>> +asm_mmu_enable_nvhe:
>>>
>>> Note the "_nvhe" suffix, which implies that...
>>>
>>>> + tlbi alle2
>>>> + dsb nsh
>>>> +
>>>> + /* 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 */
>>>> + ldr x1, =MAIR(0x00, MT_DEVICE_nGnRnE) | \
>>>> + MAIR(0x04, MT_DEVICE_nGnRE) | \
>>>> + MAIR(0x0c, MT_DEVICE_GRE) | \
>>>> + MAIR(0x44, MT_NORMAL_NC) | \
>>>> + MAIR(0xff, MT_NORMAL)
>>>> + msr mair_el1, x1
>>>
>>> ... this should be mair_el2...
>>>
>>>> +
>>>> + msr ttbr0_el1, x0
>>>
>>> ... and this should be ttbr0_el2.
>>
>> The code is definitely confusing, but not because it's wrong, but because it's
>> doing something useless. From DDI 04876E.a, page D13-3374, the pseudocode for
>> writing to ttbr0_el1:
>>
>> [..] elsif PSTATE.EL == EL2 then if HCR_EL2.E2H == '1' then
>> TTBR0_EL2
>> = X[t];
>>
>> else TTBR0_EL1 = X[t]; [..]
>>
>> We want to use the same ttbr0_el2 and mair_el2 values that we were
>> using when VHE
>> was on. We programmed those values when VHE was on, so we actually wrote them to
>> ttbr0_el2 and mair_el2. We don't need to write them again now, in fact, all the
>> previous versions of the series didn't even have the above useless writes (I
>> assume it was a copy-and-paste mistake when I split the fixes from the
>> el2 patches).
>
> Fair enough. You're just propagating a dummy value, which is going to
> bite you later.
You're correct, I'm going to remove the dummy writes to EL1 registers.
>
>>
>>>
>>>> + isb
>>>> +
>>>> + /* SCTLR */
>>>> + ldr x1, =SCTLR_EL2_RES1 | \
>>>> + SCTLR_EL2_C | \
>>>> + SCTLR_EL2_I | \
>>>> + SCTLR_EL2_M
>>>> + msr sctlr_el2, x1
>>>> + isb
>>>> +
>>>> + ret
>>>> +
>>>> /* Taken with small changes from arch/arm64/incluse/asm/assembler.h */
>>>> .macro dcache_by_line_op op, domain, start, end, tmp1, tmp2
>>>> adrp \tmp1, dcache_line_size
>>>> @@ -242,21 +284,61 @@ asm_mmu_enable:
>>>> dsb \domain
>>>> .endm
>>>>
>>>> +clean_inval_cache:
>>>> + adrp x0, __phys_offset
>>>> + ldr x0, [x0, :lo12:__phys_offset]
>>>> + adrp x1, __phys_end
>>>> + ldr x1, [x1, :lo12:__phys_end]
>>>> + dcache_by_line_op civac, sy, x0, x1, x2, x3
>>>> + isb
>>>> + ret
>>>> +
>>>> .globl asm_mmu_disable
>>>> asm_mmu_disable:
>>>> mrs x0, sctlr_el1
>>>> bic x0, x0, SCTLR_EL1_M
>>>> msr sctlr_el1, x0
>>>> isb
>>>> + b clean_inval_cache
>>>>
>>>> - /* Clean + invalidate the entire memory */
>>>> - adrp x0, __phys_offset
>>>> - ldr x0, [x0, :lo12:__phys_offset]
>>>> - adrp x1, __phys_end
>>>> - ldr x1, [x1, :lo12:__phys_end]
>>>> - dcache_by_line_op civac, sy, x0, x1, x2, x3
>>>> +asm_mmu_disable_nvhe:
>>>> + mrs x0, sctlr_el2
>>>> + bic x0, x0, SCTLR_EL2_M
>>>> + msr sctlr_el2, x0
>>>> + isb
>>>> + b clean_inval_cache
>>>> +
>>>> +.globl asm_disable_vhe
>>>> +asm_disable_vhe:
>>>> + str x30, [sp, #-16]!
>>>> +
>>>> + bl asm_mmu_disable
>>>> + msr hcr_el2, xzr
>>>> + isb
>>>
>>> At this stage, VHE is off...
>>>
>>>> + bl exceptions_init_nvhe
>>>> + /* Make asm_mmu_enable_nvhe happy by having TTBR0 value in x0. */
>>>> + mrs x0, ttbr0_el1
>>>
>>> ... so this is going to sample the wrong TTBR. It really should be
>>> TTBR0_EL2!
>>
>> Not really, asm_mmu_enable has one parameter, the PA for the
>> translation tables in
>> register x0, and we are going to use the same translation tables with
>> VHE off that
>> we were using with VHE on. Hence the read.//It could have easily been mrs
>> x0,ttbr0_el2, since they have the same value, which we want to reuse.
>
> I'm sorry, but if your reasoning that above that VHE's TTBR0_EL1 is the
> same as nVHE's TTBR0_EL2 appears correct (they are accessing the same HW
> register), this particular read of TTBR0_EL1 is *not* an EL2 register at
> all. VHE is off, and you are reading an uninitialized EL1 register (and
> it's easy to spot in KVM, as it has the magic poison value).
You're totally right here, I'm reading an EL1 register with VHE off, so I'm
definitely going to get a garbage value.The instruction should be mrs x0,ttbr0_el2.
Thanks,
Alex
>
>> I think this confusion stems from the fact that I'm trying to write
>> the registers
>> again in asm_mmu_enable_nvhe, when we don't have to. And writing to the wrong
>> registers makes the confusion even worse.
>
> I don't mind the extra writes, or even the confusion. But the above looks
> totally wrong.
>
> Thanks,
>
> M.
^ permalink raw reply [flat|nested] 12+ messages in thread
* [kvm-unit-tests RFC PATCH v3 6/7] arm64: selftest: Expand EL2 test to disable and re-enable VHE
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
` (4 preceding siblings ...)
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 5/7] lib: arm64: Add support for disabling and re-enabling VHE Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 7/7] arm64: timer: Run tests with VHE disabled Alexandru Elisei
6 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, drjones, maz, andre.przywara
Check that disabling VHE, then re-enabling it again, works.
Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
Disabling VHE is broken with version v1 of the NV patches; to run the tests
I use this patch:
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -333,7 +333,8 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
* to reverse-translate virtual EL2 system registers for a
* non-VHE guest hypervisor.
*/
- __vcpu_sys_reg(vcpu, reg) = val;
+ if (reg != HCR_EL2)
+ __vcpu_sys_reg(vcpu, reg) = val;
switch (reg) {
case ELR_EL2:
@@ -370,7 +371,17 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
return;
memory_write:
- __vcpu_sys_reg(vcpu, reg) = val;
+ if (reg == HCR_EL2 && vcpu_el2_e2h_is_set(vcpu) && !(val & HCR_E2H)) {
+ preempt_disable();
+ kvm_arch_vcpu_put(vcpu);
+
+ __vcpu_sys_reg(vcpu, reg) = val;
+
+ kvm_arch_vcpu_load(vcpu, smp_processor_id());
+ preempt_enable();
+ } else {
+ __vcpu_sys_reg(vcpu, reg) = val;
+ }
}
/* 3 bits per cache level, as per CLIDR, but non-existent caches always 0 */
arm/selftest.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arm/selftest.c b/arm/selftest.c
index a30e101a4920..b7f2a83be65f 100644
--- a/arm/selftest.c
+++ b/arm/selftest.c
@@ -424,6 +424,14 @@ static void check_el2_cpu(void *data __unused)
report(current_level() == CurrentEL_EL2, "CPU(%3d) Running at EL2", cpu);
report(vhe_supported() && vhe_enabled(),
"CPU(%3d) VHE supported and enabled", cpu);
+
+ disable_vhe();
+ report(current_level() == CurrentEL_EL2 && vhe_supported() && !vhe_enabled(),
+ "CPU(%3d) VHE disable", cpu);
+
+ enable_vhe();
+ report(current_level() == CurrentEL_EL2 && vhe_supported() && vhe_enabled(),
+ "CPU(%3d) VHE re-enable", cpu);
}
static bool psci_check(void);
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread* [kvm-unit-tests RFC PATCH v3 7/7] arm64: timer: Run tests with VHE disabled
2020-01-02 13:46 [kvm-unit-tests RFC PATCH v3 0/7] arm64: Run at EL2 Alexandru Elisei
` (5 preceding siblings ...)
2020-01-02 13:46 ` [kvm-unit-tests RFC PATCH v3 6/7] arm64: selftest: Expand EL2 test to disable and re-enable VHE Alexandru Elisei
@ 2020-01-02 13:46 ` Alexandru Elisei
6 siblings, 0 replies; 12+ messages in thread
From: Alexandru Elisei @ 2020-01-02 13:46 UTC (permalink / raw)
To: kvm; +Cc: pbonzini, drjones, maz, andre.przywara
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 70a5261dfe97..45a3176629e7 100644
--- a/lib/arm64/asm/processor.h
+++ b/lib/arm64/asm/processor.h
@@ -21,6 +21,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 88de84bc1bcf..b35af189f857 100644
--- a/arm/timer.c
+++ b/arm/timer.c
@@ -465,19 +465,34 @@ static void test_hptimer(void)
report_prefix_pop();
}
-static void test_init(void)
+static void test_init(bool nvhe)
{
const struct fdt_property *prop;
const void *fdt = dt_fdt();
+ u64 hcr;
int node, len;
u32 *data;
+ if (nvhe) {
+ 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 (nvhe) {
+ vtimer = &vtimer_info;
+ ptimer = &ptimer_info;
+ } else {
+ vtimer = &vtimer_info_vhe;
+ ptimer = &ptimer_info_vhe;
+ }
hvtimer = &hvtimer_info;
hptimer = &hptimer_info;
}
@@ -564,10 +579,20 @@ static void print_timer_info(void)
int main(int argc, char **argv)
{
int i;
+ bool nvhe = 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
+ nvhe = true;
+ argv = &argv[1];
+ argc--;
+ }
+
+ test_init(nvhe);
print_timer_info();
--
2.7.4
^ permalink raw reply related [flat|nested] 12+ messages in thread