kvm.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs
@ 2021-06-08 14:11 Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 01/13] KVM: arm64: Remove trailing whitespace in comments Fuad Tabba
                   ` (14 more replies)
  0 siblings, 15 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Hi,

This patch series adds support for restricting CPU features for protected VMs
in KVM [1].

Various feature configurations are allowed in KVM/arm64. Supporting all
these features in pKVM is difficult, as it either involves moving much of
the handling code to EL2, which adds bloat and results in a less verifiable
trusted code base. Or it involves leaving the code handling at EL1, which
risks having an untrusted host kernel feeding wrong information to the EL2
and to the protected guests.

This series attempts to mitigate this by reducing the configuration space,
providing a reduced amount of feature support at EL2 with the least amount of
compromise of protected guests' capabilities.

This is done by restricting CPU features exposed to protected guests through
feature registers. These restrictions are enforced by trapping register
accesses as well as instructions associated with these features, and injecting
an undefined exception into the guest if it attempts to use a restricted
feature.

The features being restricted (only for protected VMs in protected mode) are
the following:
- Debug, Trace, and DoubleLock
- Performance Monitoring (PMU)
- Statistical Profiling (SPE)
- Scalable Vector Extension (SVE)
- Memory Partitioning and Monitoring (MPAM)
- Activity Monitoring (AMU)
- Memory Tagging (MTE)
- Limited Ordering Regions (LOR)
- AArch32 State
- Generic Interrupt Controller (GIC) (depending on rVIC support)
- Nested Virtualization (NV)
- Reliability, Availability, and Serviceability (RAS) above V1
- Implementation-defined Features

This series is based on kvmarm/next and Will's patches for an Initial pKVM user
ABI [1]. You can find the applied series here [2].

Cheers,
/fuad

[1] https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/

For more details about pKVM, please refer to Will's talk at KVM Forum 2020:
https://www.youtube.com/watch?v=edqJSzsDRxk

[2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v1

To: kvmarm@lists.cs.columbia.edu
Cc: Marc Zyngier <maz@kernel.org>
Cc: Will Deacon <will@kernel.org>
Cc: James Morse <james.morse@arm.com>
Cc: Alexandru Elisei <alexandru.elisei@arm.com>
Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
Cc: Mark Rutland <mark.rutland@arm.com>
Cc: Christoffer Dall <christoffer.dall@arm.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
Cc: Quentin Perret <qperret@google.com>
Cc: kvm@vger.kernel.org
Cc: linux-arm-kernel@lists.infradead.org
Cc: kernel-team@android.com

Fuad Tabba (13):
  KVM: arm64: Remove trailing whitespace in comments
  KVM: arm64: MDCR_EL2 is a 64-bit register
  KVM: arm64: Fix name of HCR_TACR to match the spec
  KVM: arm64: Refactor sys_regs.h,c for nVHE reuse
  KVM: arm64: Restore mdcr_el2 from vcpu
  KVM: arm64: Add feature register flag definitions
  KVM: arm64: Add config register bit definitions
  KVM: arm64: Guest exit handlers for nVHE hyp
  KVM: arm64: Add trap handlers for protected VMs
  KVM: arm64: Move sanitized copies of CPU features
  KVM: arm64: Trap access to pVM restricted features
  KVM: arm64: Handle protected guests at 32 bits
  KVM: arm64: Check vcpu features at pVM creation

 arch/arm64/include/asm/kvm_arm.h        |  34 +-
 arch/arm64/include/asm/kvm_asm.h        |   2 +-
 arch/arm64/include/asm/kvm_host.h       |   2 +-
 arch/arm64/include/asm/kvm_hyp.h        |   4 +
 arch/arm64/include/asm/sysreg.h         |   6 +
 arch/arm64/kvm/arm.c                    |   4 +
 arch/arm64/kvm/debug.c                  |   5 +-
 arch/arm64/kvm/hyp/include/hyp/switch.h |  42 ++
 arch/arm64/kvm/hyp/nvhe/Makefile        |   2 +-
 arch/arm64/kvm/hyp/nvhe/debug-sr.c      |   2 +-
 arch/arm64/kvm/hyp/nvhe/mem_protect.c   |   6 -
 arch/arm64/kvm/hyp/nvhe/switch.c        | 114 +++++-
 arch/arm64/kvm/hyp/nvhe/sys_regs.c      | 501 ++++++++++++++++++++++++
 arch/arm64/kvm/hyp/vhe/debug-sr.c       |   2 +-
 arch/arm64/kvm/pkvm.c                   |  31 ++
 arch/arm64/kvm/sys_regs.c               |  62 +--
 arch/arm64/kvm/sys_regs.h               |  35 ++
 17 files changed, 782 insertions(+), 72 deletions(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c


base-commit: 35b256a5eebe3ac715b4ea6234aa4236a10d1a88
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 01/13] KVM: arm64: Remove trailing whitespace in comments
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 02/13] KVM: arm64: MDCR_EL2 is a 64-bit register Fuad Tabba
                   ` (13 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Editing this file later, and my editor always cleans up trailing
whitespace. Removing it earler for clearer future patches.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/sys_regs.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1a7968ad078c..15c247fc9f0c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -318,14 +318,14 @@ static bool trap_dbgauthstatus_el1(struct kvm_vcpu *vcpu,
 /*
  * We want to avoid world-switching all the DBG registers all the
  * time:
- * 
+ *
  * - If we've touched any debug register, it is likely that we're
  *   going to touch more of them. It then makes sense to disable the
  *   traps and start doing the save/restore dance
  * - If debug is active (DBG_MDSCR_KDE or DBG_MDSCR_MDE set), it is
  *   then mandatory to save/restore the registers, as the guest
  *   depends on them.
- * 
+ *
  * For this, we use a DIRTY bit, indicating the guest has modified the
  * debug registers, used as follow:
  *
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 02/13] KVM: arm64: MDCR_EL2 is a 64-bit register
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 01/13] KVM: arm64: Remove trailing whitespace in comments Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 03/13] KVM: arm64: Fix name of HCR_TACR to match the spec Fuad Tabba
                   ` (12 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Fix the places in KVM that treat MDCR_EL2 as a 32-bit register.
More recent features (e.g., FEAT_SPEv1p2) use bits above 31.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h   | 20 ++++++++++----------
 arch/arm64/include/asm/kvm_asm.h   |  2 +-
 arch/arm64/include/asm/kvm_host.h  |  2 +-
 arch/arm64/kvm/debug.c             |  5 +++--
 arch/arm64/kvm/hyp/nvhe/debug-sr.c |  2 +-
 arch/arm64/kvm/hyp/vhe/debug-sr.c  |  2 +-
 6 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 692c9049befa..25d8a61888e4 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -280,18 +280,18 @@
 /* Hyp Debug Configuration Register bits */
 #define MDCR_EL2_E2TB_MASK	(UL(0x3))
 #define MDCR_EL2_E2TB_SHIFT	(UL(24))
-#define MDCR_EL2_TTRF		(1 << 19)
-#define MDCR_EL2_TPMS		(1 << 14)
+#define MDCR_EL2_TTRF		(UL(1) << 19)
+#define MDCR_EL2_TPMS		(UL(1) << 14)
 #define MDCR_EL2_E2PB_MASK	(UL(0x3))
 #define MDCR_EL2_E2PB_SHIFT	(UL(12))
-#define MDCR_EL2_TDRA		(1 << 11)
-#define MDCR_EL2_TDOSA		(1 << 10)
-#define MDCR_EL2_TDA		(1 << 9)
-#define MDCR_EL2_TDE		(1 << 8)
-#define MDCR_EL2_HPME		(1 << 7)
-#define MDCR_EL2_TPM		(1 << 6)
-#define MDCR_EL2_TPMCR		(1 << 5)
-#define MDCR_EL2_HPMN_MASK	(0x1F)
+#define MDCR_EL2_TDRA		(UL(1) << 11)
+#define MDCR_EL2_TDOSA		(UL(1) << 10)
+#define MDCR_EL2_TDA		(UL(1) << 9)
+#define MDCR_EL2_TDE		(UL(1) << 8)
+#define MDCR_EL2_HPME		(UL(1) << 7)
+#define MDCR_EL2_TPM		(UL(1) << 6)
+#define MDCR_EL2_TPMCR		(UL(1) << 5)
+#define MDCR_EL2_HPMN_MASK	(UL(0x1F))
 
 /* For compatibility with fault code shared with 32-bit */
 #define FSC_FAULT	ESR_ELx_FSC_FAULT
diff --git a/arch/arm64/include/asm/kvm_asm.h b/arch/arm64/include/asm/kvm_asm.h
index 5e9b33cbac51..d88a5550552c 100644
--- a/arch/arm64/include/asm/kvm_asm.h
+++ b/arch/arm64/include/asm/kvm_asm.h
@@ -209,7 +209,7 @@ extern u64 __vgic_v3_read_vmcr(void);
 extern void __vgic_v3_write_vmcr(u32 vmcr);
 extern void __vgic_v3_init_lrs(void);
 
-extern u32 __kvm_get_mdcr_el2(void);
+extern u64 __kvm_get_mdcr_el2(void);
 
 #define __KVM_EXTABLE(from, to)						\
 	"	.pushsection	__kvm_ex_table, \"a\"\n"		\
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 5645af2a1431..45fdd0b7063f 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -286,7 +286,7 @@ struct kvm_vcpu_arch {
 
 	/* HYP configuration */
 	u64 hcr_el2;
-	u32 mdcr_el2;
+	u64 mdcr_el2;
 
 	/* Exception Information */
 	struct kvm_vcpu_fault_info fault;
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index d5e79d7ee6e9..f7385bfbc9e4 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -21,7 +21,7 @@
 				DBG_MDSCR_KDE | \
 				DBG_MDSCR_MDE)
 
-static DEFINE_PER_CPU(u32, mdcr_el2);
+static DEFINE_PER_CPU(u64, mdcr_el2);
 
 /**
  * save/restore_guest_debug_regs
@@ -154,7 +154,8 @@ void kvm_arm_reset_debug_ptr(struct kvm_vcpu *vcpu)
 
 void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
 {
-	unsigned long mdscr, orig_mdcr_el2 = vcpu->arch.mdcr_el2;
+	unsigned long mdscr;
+	u64 orig_mdcr_el2 = vcpu->arch.mdcr_el2;
 
 	trace_kvm_arm_setup_debug(vcpu, vcpu->guest_debug);
 
diff --git a/arch/arm64/kvm/hyp/nvhe/debug-sr.c b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
index 7d3f25868cae..df361d839902 100644
--- a/arch/arm64/kvm/hyp/nvhe/debug-sr.c
+++ b/arch/arm64/kvm/hyp/nvhe/debug-sr.c
@@ -109,7 +109,7 @@ void __debug_switch_to_host(struct kvm_vcpu *vcpu)
 	__debug_switch_to_host_common(vcpu);
 }
 
-u32 __kvm_get_mdcr_el2(void)
+u64 __kvm_get_mdcr_el2(void)
 {
 	return read_sysreg(mdcr_el2);
 }
diff --git a/arch/arm64/kvm/hyp/vhe/debug-sr.c b/arch/arm64/kvm/hyp/vhe/debug-sr.c
index f1e2e5a00933..289689b2682d 100644
--- a/arch/arm64/kvm/hyp/vhe/debug-sr.c
+++ b/arch/arm64/kvm/hyp/vhe/debug-sr.c
@@ -20,7 +20,7 @@ void __debug_switch_to_host(struct kvm_vcpu *vcpu)
 	__debug_switch_to_host_common(vcpu);
 }
 
-u32 __kvm_get_mdcr_el2(void)
+u64 __kvm_get_mdcr_el2(void)
 {
 	return read_sysreg(mdcr_el2);
 }
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 03/13] KVM: arm64: Fix name of HCR_TACR to match the spec
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 01/13] KVM: arm64: Remove trailing whitespace in comments Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 02/13] KVM: arm64: MDCR_EL2 is a 64-bit register Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 04/13] KVM: arm64: Refactor sys_regs.h,c for nVHE reuse Fuad Tabba
                   ` (11 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Makes it easier to grep and to cross-check with the Arm Architecture
Reference Manual.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 25d8a61888e4..d140e3c4c34f 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -33,7 +33,7 @@
 #define HCR_TPU		(UL(1) << 24)
 #define HCR_TPC		(UL(1) << 23)
 #define HCR_TSW		(UL(1) << 22)
-#define HCR_TAC		(UL(1) << 21)
+#define HCR_TACR	(UL(1) << 21)
 #define HCR_TIDCP	(UL(1) << 20)
 #define HCR_TSC		(UL(1) << 19)
 #define HCR_TID3	(UL(1) << 18)
@@ -60,7 +60,7 @@
  * The bits we set in HCR:
  * TLOR:	Trap LORegion register accesses
  * RW:		64bit by default, can be overridden for 32bit VMs
- * TAC:		Trap ACTLR
+ * TACR:	Trap ACTLR
  * TSC:		Trap SMC
  * TSW:		Trap cache operations by set/way
  * TWE:		Trap WFE
@@ -75,7 +75,7 @@
  * PTW:		Take a stage2 fault if a stage1 walk steps in device memory
  */
 #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
-			 HCR_BSU_IS | HCR_FB | HCR_TAC | \
+			 HCR_BSU_IS | HCR_FB | HCR_TACR | \
 			 HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
 			 HCR_FMO | HCR_IMO | HCR_PTW )
 #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 04/13] KVM: arm64: Refactor sys_regs.h,c for nVHE reuse
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (2 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 03/13] KVM: arm64: Fix name of HCR_TACR to match the spec Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 05/13] KVM: arm64: Restore mdcr_el2 from vcpu Fuad Tabba
                   ` (10 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Refactor sys_regs.h and sys_regs.c to make it easier to reuse
common code. It will be used in nVHE in a later patch.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/sys_regs.c | 58 ++++++++++-----------------------------
 arch/arm64/kvm/sys_regs.h | 35 +++++++++++++++++++++++
 2 files changed, 50 insertions(+), 43 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 15c247fc9f0c..73d09bbd173c 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -44,10 +44,6 @@
  * 64bit interface.
  */
 
-#define reg_to_encoding(x)						\
-	sys_reg((u32)(x)->Op0, (u32)(x)->Op1,				\
-		(u32)(x)->CRn, (u32)(x)->CRm, (u32)(x)->Op2)
-
 static bool read_from_write_only(struct kvm_vcpu *vcpu,
 				 struct sys_reg_params *params,
 				 const struct sys_reg_desc *r)
@@ -1026,8 +1022,6 @@ static bool access_arch_timer(struct kvm_vcpu *vcpu,
 	return true;
 }
 
-#define FEATURE(x)	(GENMASK_ULL(x##_SHIFT + 3, x##_SHIFT))
-
 /* Read a sanitised cpufeature ID register by sys_reg_desc */
 static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		struct sys_reg_desc const *r, bool raz)
@@ -1038,33 +1032,33 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 	switch (id) {
 	case SYS_ID_AA64PFR0_EL1:
 		if (!vcpu_has_sve(vcpu))
-			val &= ~FEATURE(ID_AA64PFR0_SVE);
-		val &= ~FEATURE(ID_AA64PFR0_AMU);
-		val &= ~FEATURE(ID_AA64PFR0_CSV2);
-		val |= FIELD_PREP(FEATURE(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
-		val &= ~FEATURE(ID_AA64PFR0_CSV3);
-		val |= FIELD_PREP(FEATURE(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
+			val &= ~SYS_FEATURE(ID_AA64PFR0_SVE);
+		val &= ~SYS_FEATURE(ID_AA64PFR0_AMU);
+		val &= ~SYS_FEATURE(ID_AA64PFR0_CSV2);
+		val |= FIELD_PREP(SYS_FEATURE(ID_AA64PFR0_CSV2), (u64)vcpu->kvm->arch.pfr0_csv2);
+		val &= ~SYS_FEATURE(ID_AA64PFR0_CSV3);
+		val |= FIELD_PREP(SYS_FEATURE(ID_AA64PFR0_CSV3), (u64)vcpu->kvm->arch.pfr0_csv3);
 		break;
 	case SYS_ID_AA64PFR1_EL1:
-		val &= ~FEATURE(ID_AA64PFR1_MTE);
+		val &= ~SYS_FEATURE(ID_AA64PFR1_MTE);
 		break;
 	case SYS_ID_AA64ISAR1_EL1:
 		if (!vcpu_has_ptrauth(vcpu))
-			val &= ~(FEATURE(ID_AA64ISAR1_APA) |
-				 FEATURE(ID_AA64ISAR1_API) |
-				 FEATURE(ID_AA64ISAR1_GPA) |
-				 FEATURE(ID_AA64ISAR1_GPI));
+			val &= ~(SYS_FEATURE(ID_AA64ISAR1_APA) |
+				 SYS_FEATURE(ID_AA64ISAR1_API) |
+				 SYS_FEATURE(ID_AA64ISAR1_GPA) |
+				 SYS_FEATURE(ID_AA64ISAR1_GPI));
 		break;
 	case SYS_ID_AA64DFR0_EL1:
 		/* Limit debug to ARMv8.0 */
-		val &= ~FEATURE(ID_AA64DFR0_DEBUGVER);
-		val |= FIELD_PREP(FEATURE(ID_AA64DFR0_DEBUGVER), 6);
+		val &= ~SYS_FEATURE(ID_AA64DFR0_DEBUGVER);
+		val |= FIELD_PREP(SYS_FEATURE(ID_AA64DFR0_DEBUGVER), 6);
 		/* Limit guests to PMUv3 for ARMv8.4 */
 		val = cpuid_feature_cap_perfmon_field(val,
 						      ID_AA64DFR0_PMUVER_SHIFT,
 						      kvm_vcpu_has_pmu(vcpu) ? ID_AA64DFR0_PMUVER_8_4 : 0);
 		/* Hide SPE from guests */
-		val &= ~FEATURE(ID_AA64DFR0_PMSVER);
+		val &= ~SYS_FEATURE(ID_AA64DFR0_PMSVER);
 		break;
 	case SYS_ID_DFR0_EL1:
 		/* Limit guests to PMUv3 for ARMv8.4 */
@@ -2082,23 +2076,6 @@ static int check_sysreg_table(const struct sys_reg_desc *table, unsigned int n,
 	return 0;
 }
 
-static int match_sys_reg(const void *key, const void *elt)
-{
-	const unsigned long pval = (unsigned long)key;
-	const struct sys_reg_desc *r = elt;
-
-	return pval - reg_to_encoding(r);
-}
-
-static const struct sys_reg_desc *find_reg(const struct sys_reg_params *params,
-					 const struct sys_reg_desc table[],
-					 unsigned int num)
-{
-	unsigned long pval = reg_to_encoding(params);
-
-	return bsearch((void *)pval, table, num, sizeof(table[0]), match_sys_reg);
-}
-
 int kvm_handle_cp14_load_store(struct kvm_vcpu *vcpu)
 {
 	kvm_inject_undefined(vcpu);
@@ -2341,13 +2318,8 @@ int kvm_handle_sys_reg(struct kvm_vcpu *vcpu)
 
 	trace_kvm_handle_sys_reg(esr);
 
-	params.Op0 = (esr >> 20) & 3;
-	params.Op1 = (esr >> 14) & 0x7;
-	params.CRn = (esr >> 10) & 0xf;
-	params.CRm = (esr >> 1) & 0xf;
-	params.Op2 = (esr >> 17) & 0x7;
+	params = esr_sys64_to_params(esr);
 	params.regval = vcpu_get_reg(vcpu, Rt);
-	params.is_write = !(esr & 1);
 
 	ret = emulate_sys_reg(vcpu, &params);
 
diff --git a/arch/arm64/kvm/sys_regs.h b/arch/arm64/kvm/sys_regs.h
index 9d0621417c2a..f7cde4436f32 100644
--- a/arch/arm64/kvm/sys_regs.h
+++ b/arch/arm64/kvm/sys_regs.h
@@ -11,6 +11,12 @@
 #ifndef __ARM64_KVM_SYS_REGS_LOCAL_H__
 #define __ARM64_KVM_SYS_REGS_LOCAL_H__
 
+#include <linux/bsearch.h>
+
+#define reg_to_encoding(x)						\
+	sys_reg((u32)(x)->Op0, (u32)(x)->Op1,				\
+		(u32)(x)->CRn, (u32)(x)->CRm, (u32)(x)->Op2)
+
 struct sys_reg_params {
 	u8	Op0;
 	u8	Op1;
@@ -21,6 +27,14 @@ struct sys_reg_params {
 	bool	is_write;
 };
 
+#define esr_sys64_to_params(esr)                                               \
+	((struct sys_reg_params){ .Op0 = ((esr) >> 20) & 3,                    \
+				  .Op1 = ((esr) >> 14) & 0x7,                  \
+				  .CRn = ((esr) >> 10) & 0xf,                  \
+				  .CRm = ((esr) >> 1) & 0xf,                   \
+				  .Op2 = ((esr) >> 17) & 0x7,                  \
+				  .is_write = !((esr)&1) })
+
 struct sys_reg_desc {
 	/* Sysreg string for debug */
 	const char *name;
@@ -152,6 +166,24 @@ static inline int cmp_sys_reg(const struct sys_reg_desc *i1,
 	return i1->Op2 - i2->Op2;
 }
 
+static inline int match_sys_reg(const void *key, const void *elt)
+{
+	const unsigned long pval = (unsigned long)key;
+	const struct sys_reg_desc *r = elt;
+
+	return pval - reg_to_encoding(r);
+}
+
+static inline const struct sys_reg_desc *
+find_reg(const struct sys_reg_params *params, const struct sys_reg_desc table[],
+	 unsigned int num)
+{
+	unsigned long pval = reg_to_encoding(params);
+
+	return __inline_bsearch((void *)pval, table, num, sizeof(table[0]),
+				match_sys_reg);
+}
+
 const struct sys_reg_desc *find_reg_by_id(u64 id,
 					  struct sys_reg_params *params,
 					  const struct sys_reg_desc table[],
@@ -170,4 +202,7 @@ const struct sys_reg_desc *find_reg_by_id(u64 id,
 	CRn(sys_reg_CRn(reg)), CRm(sys_reg_CRm(reg)),	\
 	Op2(sys_reg_Op2(reg))
 
+/* Extract the feature specified from the feature id register. */
+#define SYS_FEATURE(x)	(GENMASK_ULL(x##_SHIFT + 3, x##_SHIFT))
+
 #endif /* __ARM64_KVM_SYS_REGS_LOCAL_H__ */
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 05/13] KVM: arm64: Restore mdcr_el2 from vcpu
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (3 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 04/13] KVM: arm64: Refactor sys_regs.h,c for nVHE reuse Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 06/13] KVM: arm64: Add feature register flag definitions Fuad Tabba
                   ` (9 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

On deactivating traps, restore the value of mdcr_el2 from the
vcpu context, rather than directly reading the hardware register.
Currently, the two values are the same, i.e., the hardware
register and the vcpu one. A future patch will be changing the
value of mdcr_el2 on activating traps, and this ensures that its
value will be restored.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/switch.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index f7af9688c1f7..430b5bae8761 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -73,7 +73,7 @@ static void __deactivate_traps(struct kvm_vcpu *vcpu)
 
 	___deactivate_traps(vcpu);
 
-	mdcr_el2 = read_sysreg(mdcr_el2);
+	mdcr_el2 = vcpu->arch.mdcr_el2;
 
 	if (cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
 		u64 val;
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 06/13] KVM: arm64: Add feature register flag definitions
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (4 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 05/13] KVM: arm64: Restore mdcr_el2 from vcpu Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 07/13] KVM: arm64: Add config register bit definitions Fuad Tabba
                   ` (8 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Add feature register flag definitions to clarify which features
might be toggled.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/sysreg.h | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 65d15700a168..52e48b9226f6 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -789,6 +789,10 @@
 #define ID_AA64PFR0_FP_SUPPORTED	0x0
 #define ID_AA64PFR0_ASIMD_NI		0xf
 #define ID_AA64PFR0_ASIMD_SUPPORTED	0x0
+#define ID_AA64PFR0_EL3_64BIT_ONLY	0x1
+#define ID_AA64PFR0_EL3_32BIT_64BIT	0x2
+#define ID_AA64PFR0_EL2_64BIT_ONLY	0x1
+#define ID_AA64PFR0_EL2_32BIT_64BIT	0x2
 #define ID_AA64PFR0_EL1_64BIT_ONLY	0x1
 #define ID_AA64PFR0_EL1_32BIT_64BIT	0x2
 #define ID_AA64PFR0_EL0_64BIT_ONLY	0x1
@@ -854,6 +858,7 @@
 #define ID_AA64MMFR0_TGRAN64_SUPPORTED	0x0
 #define ID_AA64MMFR0_TGRAN16_NI		0x0
 #define ID_AA64MMFR0_TGRAN16_SUPPORTED	0x1
+#define ID_AA64MMFR0_PARANGE_40		0x2
 #define ID_AA64MMFR0_PARANGE_48		0x5
 #define ID_AA64MMFR0_PARANGE_52		0x6
 
@@ -901,6 +906,7 @@
 #define ID_AA64MMFR2_CNP_SHIFT		0
 
 /* id_aa64dfr0 */
+#define ID_AA64DFR0_MTPMU_SHIFT		48
 #define ID_AA64DFR0_TRBE_SHIFT		44
 #define ID_AA64DFR0_TRACE_FILT_SHIFT	40
 #define ID_AA64DFR0_DOUBLELOCK_SHIFT	36
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 07/13] KVM: arm64: Add config register bit definitions
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (5 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 06/13] KVM: arm64: Add feature register flag definitions Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 08/13] KVM: arm64: Guest exit handlers for nVHE hyp Fuad Tabba
                   ` (7 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Add hardware configuration register bit definitions for HCR_EL2
and MDCR_EL2. Future patches toggle these hyp configuration
register bits to trap on certain accesses.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index d140e3c4c34f..5bb26be69c3f 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -12,7 +12,11 @@
 #include <asm/types.h>
 
 /* Hyp Configuration Register (HCR) bits */
+#define HCR_TID5	(UL(1) << 58)
+#define HCR_DCT		(UL(1) << 57)
 #define HCR_ATA		(UL(1) << 56)
+#define HCR_AMVOFFEN	(UL(1) << 51)
+#define HCR_FIEN	(UL(1) << 47)
 #define HCR_FWB		(UL(1) << 46)
 #define HCR_API		(UL(1) << 41)
 #define HCR_APK		(UL(1) << 40)
@@ -280,7 +284,11 @@
 /* Hyp Debug Configuration Register bits */
 #define MDCR_EL2_E2TB_MASK	(UL(0x3))
 #define MDCR_EL2_E2TB_SHIFT	(UL(24))
+#define MDCR_EL2_MTPME		(UL(1) << 28)
+#define MDCR_EL2_TDCC		(UL(1) << 27)
+#define MDCR_EL2_HCCD		(UL(1) << 23)
 #define MDCR_EL2_TTRF		(UL(1) << 19)
+#define MDCR_EL2_HPMD		(UL(1) << 17)
 #define MDCR_EL2_TPMS		(UL(1) << 14)
 #define MDCR_EL2_E2PB_MASK	(UL(0x3))
 #define MDCR_EL2_E2PB_SHIFT	(UL(12))
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 08/13] KVM: arm64: Guest exit handlers for nVHE hyp
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (6 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 07/13] KVM: arm64: Add config register bit definitions Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 09/13] KVM: arm64: Add trap handlers for protected VMs Fuad Tabba
                   ` (6 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Add an array of pointers to handlers for various trap reasons in
nVHE code.

The current code selects how to fixup a guest on exit based on a
series of if/else statements. Future patches will also require
different handling for guest exists. Create an array of handlers
to consolidate them.

No functional change intended as the array isn't populated yet.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 19 ++++++++++++++
 arch/arm64/kvm/hyp/nvhe/switch.c        | 35 +++++++++++++++++++++++++
 2 files changed, 54 insertions(+)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index e4a2f295a394..f5d3d1da0aec 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -405,6 +405,18 @@ static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
 	return true;
 }
 
+typedef int (*exit_handle_fn)(struct kvm_vcpu *);
+
+exit_handle_fn kvm_get_nvhe_exit_handler(struct kvm_vcpu *vcpu);
+
+static exit_handle_fn kvm_get_hyp_exit_handler(struct kvm_vcpu *vcpu)
+{
+	if (is_nvhe_hyp_code())
+		return kvm_get_nvhe_exit_handler(vcpu);
+	else
+		return NULL;
+}
+
 /*
  * Return true when we were able to fixup the guest exit and should return to
  * the guest, false when we should restore the host state and return to the
@@ -412,6 +424,8 @@ static inline bool __hyp_handle_ptrauth(struct kvm_vcpu *vcpu)
  */
 static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 {
+	exit_handle_fn exit_handler;
+
 	if (ARM_EXCEPTION_CODE(*exit_code) != ARM_EXCEPTION_IRQ)
 		vcpu->arch.fault.esr_el2 = read_sysreg_el2(SYS_ESR);
 
@@ -492,6 +506,11 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 			goto guest;
 	}
 
+	/* Check if there's an exit handler and allow it to handle the exit. */
+	exit_handler = kvm_get_hyp_exit_handler(vcpu);
+	if (exit_handler && exit_handler(vcpu))
+		goto guest;
+
 exit:
 	/* Return to the host kernel and handle the exit */
 	return false;
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 430b5bae8761..967a3ad74fbd 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -165,6 +165,41 @@ static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
 		write_sysreg(pmu->events_host, pmcntenset_el0);
 }
 
+typedef int (*exit_handle_fn)(struct kvm_vcpu *);
+
+static exit_handle_fn hyp_exit_handlers[] = {
+	[0 ... ESR_ELx_EC_MAX]		= NULL,
+	[ESR_ELx_EC_WFx]		= NULL,
+	[ESR_ELx_EC_CP15_32]		= NULL,
+	[ESR_ELx_EC_CP15_64]		= NULL,
+	[ESR_ELx_EC_CP14_MR]		= NULL,
+	[ESR_ELx_EC_CP14_LS]		= NULL,
+	[ESR_ELx_EC_CP14_64]		= NULL,
+	[ESR_ELx_EC_HVC32]		= NULL,
+	[ESR_ELx_EC_SMC32]		= NULL,
+	[ESR_ELx_EC_HVC64]		= NULL,
+	[ESR_ELx_EC_SMC64]		= NULL,
+	[ESR_ELx_EC_SYS64]		= NULL,
+	[ESR_ELx_EC_SVE]		= NULL,
+	[ESR_ELx_EC_IABT_LOW]		= NULL,
+	[ESR_ELx_EC_DABT_LOW]		= NULL,
+	[ESR_ELx_EC_SOFTSTP_LOW]	= NULL,
+	[ESR_ELx_EC_WATCHPT_LOW]	= NULL,
+	[ESR_ELx_EC_BREAKPT_LOW]	= NULL,
+	[ESR_ELx_EC_BKPT32]		= NULL,
+	[ESR_ELx_EC_BRK64]		= NULL,
+	[ESR_ELx_EC_FP_ASIMD]		= NULL,
+	[ESR_ELx_EC_PAC]		= NULL,
+};
+
+exit_handle_fn kvm_get_nvhe_exit_handler(struct kvm_vcpu *vcpu)
+{
+	u32 esr = kvm_vcpu_get_esr(vcpu);
+	u8 esr_ec = ESR_ELx_EC(esr);
+
+	return hyp_exit_handlers[esr_ec];
+}
+
 /* Switch to the guest for legacy non-VHE systems */
 int __kvm_vcpu_run(struct kvm_vcpu *vcpu)
 {
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 09/13] KVM: arm64: Add trap handlers for protected VMs
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (7 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 08/13] KVM: arm64: Guest exit handlers for nVHE hyp Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 10/13] KVM: arm64: Move sanitized copies of CPU features Fuad Tabba
                   ` (5 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Add trap handlers for protected VMs. These are mainly for Sys64
and debug traps.

No functional change intended as these are not hooked in yet.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_hyp.h   |   4 +
 arch/arm64/kvm/arm.c               |   4 +
 arch/arm64/kvm/hyp/nvhe/Makefile   |   2 +-
 arch/arm64/kvm/hyp/nvhe/sys_regs.c | 496 +++++++++++++++++++++++++++++
 4 files changed, 505 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c

diff --git a/arch/arm64/include/asm/kvm_hyp.h b/arch/arm64/include/asm/kvm_hyp.h
index 9d60b3006efc..23d4e5aac41d 100644
--- a/arch/arm64/include/asm/kvm_hyp.h
+++ b/arch/arm64/include/asm/kvm_hyp.h
@@ -115,7 +115,11 @@ int __pkvm_init(phys_addr_t phys, unsigned long size, unsigned long nr_cpus,
 void __noreturn __host_enter(struct kvm_cpu_context *host_ctxt);
 #endif
 
+extern u64 kvm_nvhe_sym(id_aa64pfr0_el1_sys_val);
+extern u64 kvm_nvhe_sym(id_aa64pfr1_el1_sys_val);
+extern u64 kvm_nvhe_sym(id_aa64dfr0_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64mmfr0_el1_sys_val);
 extern u64 kvm_nvhe_sym(id_aa64mmfr1_el1_sys_val);
+extern u64 kvm_nvhe_sym(id_aa64mmfr2_el1_sys_val);
 
 #endif /* __ARM64_KVM_HYP_H__ */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index d71da6089822..a56ff3a6d2c0 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -1751,8 +1751,12 @@ static int kvm_hyp_init_protection(u32 hyp_va_bits)
 	void *addr = phys_to_virt(hyp_mem_base);
 	int ret;
 
+	kvm_nvhe_sym(id_aa64pfr0_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64PFR0_EL1);
+	kvm_nvhe_sym(id_aa64pfr1_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64PFR1_EL1);
+	kvm_nvhe_sym(id_aa64dfr0_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64DFR0_EL1);
 	kvm_nvhe_sym(id_aa64mmfr0_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64MMFR0_EL1);
 	kvm_nvhe_sym(id_aa64mmfr1_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64MMFR1_EL1);
+	kvm_nvhe_sym(id_aa64mmfr2_el1_sys_val) = read_sanitised_ftr_reg(SYS_ID_AA64MMFR2_EL1);
 
 	ret = create_hyp_mappings(addr, addr + hyp_mem_size, PAGE_HYP);
 	if (ret)
diff --git a/arch/arm64/kvm/hyp/nvhe/Makefile b/arch/arm64/kvm/hyp/nvhe/Makefile
index 5df6193fc430..a23f417a0c20 100644
--- a/arch/arm64/kvm/hyp/nvhe/Makefile
+++ b/arch/arm64/kvm/hyp/nvhe/Makefile
@@ -14,7 +14,7 @@ lib-objs := $(addprefix ../../../lib/, $(lib-objs))
 
 obj-y := timer-sr.o sysreg-sr.o debug-sr.o switch.o tlb.o hyp-init.o host.o \
 	 hyp-main.o hyp-smp.o psci-relay.o early_alloc.o stub.o page_alloc.o \
-	 cache.o setup.o mm.o mem_protect.o
+	 cache.o setup.o mm.o mem_protect.o sys_regs.o
 obj-y += ../vgic-v3-sr.o ../aarch32.o ../vgic-v2-cpuif-proxy.o ../entry.o \
 	 ../fpsimd.o ../hyp-entry.o ../exception.o ../pgtable.o
 obj-y += $(lib-objs)
diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
new file mode 100644
index 000000000000..890c96315e55
--- /dev/null
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -0,0 +1,496 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2021 Google LLC
+ * Author: Fuad Tabba <tabba@google.com>
+ */
+
+#include <linux/kvm_host.h>
+
+#include <asm/kvm_asm.h>
+#include <asm/kvm_emulate.h>
+#include <asm/kvm_mmu.h>
+
+#include <hyp/adjust_pc.h>
+
+#include "../../sys_regs.h"
+
+u64 id_aa64pfr0_el1_sys_val;
+u64 id_aa64pfr1_el1_sys_val;
+u64 id_aa64dfr0_el1_sys_val;
+u64 id_aa64mmfr2_el1_sys_val;
+
+/*
+ * Inject an undefined exception to the guest.
+ */
+static void inject_undef(struct kvm_vcpu *vcpu)
+{
+	u32 esr = (ESR_ELx_EC_UNKNOWN << ESR_ELx_EC_SHIFT);
+
+	vcpu->arch.flags |= (KVM_ARM64_EXCEPT_AA64_EL1 |
+			     KVM_ARM64_EXCEPT_AA64_ELx_SYNC |
+			     KVM_ARM64_PENDING_EXCEPTION);
+
+	__kvm_adjust_pc(vcpu);
+
+	write_sysreg_el1(esr, SYS_ESR);
+	write_sysreg_el1(read_sysreg_el2(SYS_ELR), SYS_ELR);
+	write_sysreg_el2(*vcpu_pc(vcpu), SYS_ELR);
+	write_sysreg_el2(*vcpu_cpsr(vcpu), SYS_SPSR);
+}
+
+/*
+ * Accessor for undefined accesses.
+ */
+static bool undef_access(struct kvm_vcpu *vcpu,
+			 struct sys_reg_params *p,
+			 const struct sys_reg_desc *r)
+{
+	inject_undef(vcpu);
+	return false;
+}
+
+/*
+ * Accessors for feature registers.
+ *
+ * If access is allowed, set the regval to the protected VM's view of the
+ * register and return true.
+ * Otherwise, inject an undefined exception and return false.
+ */
+
+/* Accessor for ID_AA64PFR0_EL1. */
+static bool pvm_access_id_aa64pfr0(struct kvm_vcpu *vcpu,
+				   struct sys_reg_params *p,
+				   const struct sys_reg_desc *r)
+{
+	u64 clear_mask;
+	u64 set_mask;
+	u64 val = id_aa64pfr0_el1_sys_val;
+	const struct kvm *kvm = (const struct kvm *) kern_hyp_va(vcpu->kvm);
+
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/*
+	 * No support for:
+	 * - AArch32 state for protected VMs
+	 * - GIC CPU Interface
+	 * - ARMv8.4-RAS (restricted to v1)
+	 * - Scalable Vectors
+	 * - Memory Partitioning and Monitoring
+	 * - Activity Monitoring
+	 * - Secure EL2 (not relevant to non-secure guests)
+	 */
+	clear_mask = SYS_FEATURE(ID_AA64PFR0_EL0) |
+		     SYS_FEATURE(ID_AA64PFR0_EL1) |
+		     SYS_FEATURE(ID_AA64PFR0_EL2) |
+		     SYS_FEATURE(ID_AA64PFR0_EL3) |
+		     SYS_FEATURE(ID_AA64PFR0_GIC) |
+		     SYS_FEATURE(ID_AA64PFR0_RAS) |
+		     SYS_FEATURE(ID_AA64PFR0_SVE) |
+		     SYS_FEATURE(ID_AA64PFR0_MPAM) |
+		     SYS_FEATURE(ID_AA64PFR0_AMU) |
+		     SYS_FEATURE(ID_AA64PFR0_SEL2) |
+		     SYS_FEATURE(ID_AA64PFR0_CSV2) |
+		     SYS_FEATURE(ID_AA64PFR0_CSV3);
+
+	set_mask = (ID_AA64PFR0_EL0_64BIT_ONLY << ID_AA64PFR0_EL0_SHIFT) |
+		   (ID_AA64PFR0_EL1_64BIT_ONLY << ID_AA64PFR0_EL1_SHIFT) |
+		   (ID_AA64PFR0_EL2_64BIT_ONLY << ID_AA64PFR0_EL2_SHIFT);
+
+	/* Only set EL3 handling if EL3 exists. */
+	if (val & SYS_FEATURE(ID_AA64PFR0_EL3))
+		set_mask |=
+			(ID_AA64PFR0_EL3_64BIT_ONLY << ID_AA64PFR0_EL3_SHIFT);
+
+	/* RAS restricted to v1 (0x1). */
+	if (val & SYS_FEATURE(ID_AA64PFR0_RAS))
+		set_mask |= FIELD_PREP(SYS_FEATURE(ID_AA64PFR0_RAS), 1);
+
+	/* Check whether Spectre and Meltdown are mitigated. */
+	set_mask |= FIELD_PREP(SYS_FEATURE(ID_AA64PFR0_CSV2),
+			       (u64)kvm->arch.pfr0_csv2);
+	set_mask |= FIELD_PREP(SYS_FEATURE(ID_AA64PFR0_CSV3),
+			       (u64)kvm->arch.pfr0_csv3);
+
+	p->regval = (val & ~clear_mask) | set_mask;
+	return true;
+}
+
+/* Accessor for ID_AA64PFR1_EL1. */
+static bool pvm_access_id_aa64pfr1(struct kvm_vcpu *vcpu,
+				   struct sys_reg_params *p,
+				   const struct sys_reg_desc *r)
+{
+	u64 clear_mask;
+	u64 val = id_aa64pfr1_el1_sys_val;
+
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/*
+	 * No support for:
+	 * - ARMv8.4-RAS (restricted to v1)
+	 * - Memory Partitioning and Monitoring
+	 * - Memory Tagging
+	 */
+	clear_mask = SYS_FEATURE(ID_AA64PFR1_RASFRAC) |
+		     SYS_FEATURE(ID_AA64PFR1_MPAMFRAC) |
+		     SYS_FEATURE(ID_AA64PFR1_MTE);
+
+	p->regval = val & ~clear_mask;
+	return true;
+}
+
+/* Accessor for ID_AA64ZFR0_EL1. */
+static bool pvm_access_id_aa64zfr0(struct kvm_vcpu *vcpu,
+				   struct sys_reg_params *p,
+				   const struct sys_reg_desc *r)
+{
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/* No support for Scalable Vectors */
+	p->regval = 0;
+	return true;
+}
+
+/* Accessor for ID_AA64DFR0_EL1. */
+static bool pvm_access_id_aa64dfr0(struct kvm_vcpu *vcpu,
+				   struct sys_reg_params *p,
+				   const struct sys_reg_desc *r)
+{
+	u64 clear_mask;
+	u64 val = id_aa64dfr0_el1_sys_val;
+
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/*
+	 * No support for:
+	 * - Debug: includes breakpoints, and watchpoints.
+	 * Note: not supporting debug at all is not arch-compliant.
+	 * - OS Double Lock
+	 * - Trace and Self Hosted Trace
+	 * - Performance Monitoring
+	 * - Statistical Profiling
+	 */
+	clear_mask = SYS_FEATURE(ID_AA64DFR0_DEBUGVER) |
+		     SYS_FEATURE(ID_AA64DFR0_BRPS) |
+		     SYS_FEATURE(ID_AA64DFR0_WRPS) |
+		     SYS_FEATURE(ID_AA64DFR0_CTX_CMPS) |
+		     SYS_FEATURE(ID_AA64DFR0_DOUBLELOCK) |
+		     SYS_FEATURE(ID_AA64DFR0_TRACEVER) |
+		     SYS_FEATURE(ID_AA64DFR0_TRACE_FILT) |
+		     SYS_FEATURE(ID_AA64DFR0_PMUVER) |
+		     SYS_FEATURE(ID_AA64DFR0_MTPMU) |
+		     SYS_FEATURE(ID_AA64DFR0_PMSVER);
+
+	p->regval = val & ~clear_mask;
+	return true;
+}
+
+/* Accessor for ID_AA64MMFR0_EL1. */
+static bool pvm_access_id_aa64mmfr0(struct kvm_vcpu *vcpu,
+				    struct sys_reg_params *p,
+				    const struct sys_reg_desc *r)
+{
+	u64 clear_mask;
+	u64 set_mask;
+	u64 val = id_aa64mmfr0_el1_sys_val;
+
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/*
+	 * No support for:
+	 * - Nested Virtualization
+	 *
+	 * Only support for:
+	 * - 4KB granule
+	 * - 40-bit IPA
+	 */
+	clear_mask = SYS_FEATURE(ID_AA64MMFR0_ECV) |
+		     SYS_FEATURE(ID_AA64MMFR0_FGT) |
+		     SYS_FEATURE(ID_AA64MMFR0_TGRAN4_2) |
+		     SYS_FEATURE(ID_AA64MMFR0_TGRAN64_2) |
+		     SYS_FEATURE(ID_AA64MMFR0_TGRAN16_2) |
+		     SYS_FEATURE(ID_AA64MMFR0_TGRAN4) |
+		     SYS_FEATURE(ID_AA64MMFR0_TGRAN16) |
+		     SYS_FEATURE(ID_AA64MMFR0_PARANGE);
+
+	set_mask = SYS_FEATURE(ID_AA64MMFR0_TGRAN64) |
+		   (ID_AA64MMFR0_PARANGE_40 << ID_AA64MMFR0_PARANGE_SHIFT);
+
+	p->regval = (val & ~clear_mask) | set_mask;
+	return true;
+}
+
+/* Accessor for ID_AA64MMFR1_EL1. */
+static bool pvm_access_id_aa64mmfr1(struct kvm_vcpu *vcpu,
+				    struct sys_reg_params *p,
+				    const struct sys_reg_desc *r)
+{
+	u64 clear_mask;
+	u64 val = id_aa64mmfr1_el1_sys_val;
+
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/*
+	 * No support for:
+	 * - Nested Virtualization
+	 * - Limited Ordering Regions
+	 */
+	clear_mask = SYS_FEATURE(ID_AA64MMFR1_TWED) |
+		     SYS_FEATURE(ID_AA64MMFR1_XNX) |
+		     SYS_FEATURE(ID_AA64MMFR1_VHE) |
+		     SYS_FEATURE(ID_AA64MMFR1_LOR);
+
+	p->regval = val & ~clear_mask;
+	return true;
+}
+
+/* Accessor for ID_AA64MMFR2_EL1. */
+static bool pvm_access_id_aa64mmfr2(struct kvm_vcpu *vcpu,
+				    struct sys_reg_params *p,
+				    const struct sys_reg_desc *r)
+{
+	u64 clear_mask;
+	u64 val = id_aa64mmfr2_el1_sys_val;
+
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/*
+	 * No support for:
+	 * - Nested Virtualization
+	 * - Small translation tables
+	 * - 64-bit format of CCSIDR_EL1
+	 * - 52-bit VAs
+	 * - AArch32 state for protected VMs
+	 */
+	clear_mask = SYS_FEATURE(ID_AA64MMFR2_EVT) |
+		     SYS_FEATURE(ID_AA64MMFR2_FWB) |
+		     SYS_FEATURE(ID_AA64MMFR2_NV) |
+		     SYS_FEATURE(ID_AA64MMFR2_ST) |
+		     SYS_FEATURE(ID_AA64MMFR2_CCIDX) |
+		     SYS_FEATURE(ID_AA64MMFR2_LVA) |
+		     SYS_FEATURE(ID_AA64MMFR2_LSM);
+
+	p->regval = val & ~clear_mask;
+	return true;
+}
+
+/*
+ * Accessor for AArch32 Processor Feature Registers.
+ *
+ * The value of these registers is "unknown" according to the spec if AArch32
+ * isn't supported.
+ */
+static bool pvm_access_id_aarch32(struct kvm_vcpu *vcpu,
+				  struct sys_reg_params *p,
+				  const struct sys_reg_desc *r)
+{
+	if (p->is_write)
+		return undef_access(vcpu, p, r);
+
+	/* Use 0 for architecturally "unknown" values. */
+	p->regval = 0;
+	return true;
+}
+
+/* Mark the specified system register as an AArch32 feature register. */
+#define AARCH32(REG) { SYS_DESC(REG), .access = pvm_access_id_aarch32 }
+
+/* Mark the specified system register as not being handled in hyp. */
+#define HOST_HANDLED(REG) { SYS_DESC(REG), .access = NULL }
+
+/*
+ * Architected system registers.
+ * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2
+ *
+ * NOTE: Anything not explicitly listed here will be *restricted by default*,
+ * i.e., it will lead to injecting an exception into the guest.
+ */
+static const struct sys_reg_desc pvm_sys_reg_descs[] = {
+	/* Cache maintenance by set/way operations are restricted. */
+
+	/* Debug and Trace Registers are all restricted */
+
+	/* AArch64 mappings of the AArch32 ID registers */
+	/* CRm=1 */
+	AARCH32(SYS_ID_PFR0_EL1),
+	AARCH32(SYS_ID_PFR1_EL1),
+	AARCH32(SYS_ID_DFR0_EL1),
+	AARCH32(SYS_ID_AFR0_EL1),
+	AARCH32(SYS_ID_MMFR0_EL1),
+	AARCH32(SYS_ID_MMFR1_EL1),
+	AARCH32(SYS_ID_MMFR2_EL1),
+	AARCH32(SYS_ID_MMFR3_EL1),
+
+	/* CRm=2 */
+	AARCH32(SYS_ID_ISAR0_EL1),
+	AARCH32(SYS_ID_ISAR1_EL1),
+	AARCH32(SYS_ID_ISAR2_EL1),
+	AARCH32(SYS_ID_ISAR3_EL1),
+	AARCH32(SYS_ID_ISAR4_EL1),
+	AARCH32(SYS_ID_ISAR5_EL1),
+	AARCH32(SYS_ID_MMFR4_EL1),
+	AARCH32(SYS_ID_ISAR6_EL1),
+
+	/* CRm=3 */
+	AARCH32(SYS_MVFR0_EL1),
+	AARCH32(SYS_MVFR1_EL1),
+	AARCH32(SYS_MVFR2_EL1),
+	AARCH32(SYS_ID_PFR2_EL1),
+	AARCH32(SYS_ID_DFR1_EL1),
+	AARCH32(SYS_ID_MMFR5_EL1),
+
+	/* AArch64 ID registers */
+	/* CRm=4 */
+	{ SYS_DESC(SYS_ID_AA64PFR0_EL1), .access = pvm_access_id_aa64pfr0 },
+	{ SYS_DESC(SYS_ID_AA64PFR1_EL1), .access = pvm_access_id_aa64pfr1 },
+	{ SYS_DESC(SYS_ID_AA64ZFR0_EL1), .access = pvm_access_id_aa64zfr0 },
+	{ SYS_DESC(SYS_ID_AA64DFR0_EL1), .access = pvm_access_id_aa64dfr0 },
+	HOST_HANDLED(SYS_ID_AA64DFR1_EL1),
+	HOST_HANDLED(SYS_ID_AA64AFR0_EL1),
+	HOST_HANDLED(SYS_ID_AA64AFR1_EL1),
+	HOST_HANDLED(SYS_ID_AA64ISAR0_EL1),
+	HOST_HANDLED(SYS_ID_AA64ISAR1_EL1),
+	{ SYS_DESC(SYS_ID_AA64MMFR0_EL1), .access = pvm_access_id_aa64mmfr0 },
+	{ SYS_DESC(SYS_ID_AA64MMFR1_EL1), .access = pvm_access_id_aa64mmfr1 },
+	{ SYS_DESC(SYS_ID_AA64MMFR2_EL1), .access = pvm_access_id_aa64mmfr2 },
+
+	HOST_HANDLED(SYS_SCTLR_EL1),
+	HOST_HANDLED(SYS_ACTLR_EL1),
+	HOST_HANDLED(SYS_CPACR_EL1),
+
+	HOST_HANDLED(SYS_RGSR_EL1),
+	HOST_HANDLED(SYS_GCR_EL1),
+
+	/* Scalable Vector Registers are restricted. */
+
+	HOST_HANDLED(SYS_TTBR0_EL1),
+	HOST_HANDLED(SYS_TTBR1_EL1),
+	HOST_HANDLED(SYS_TCR_EL1),
+
+	HOST_HANDLED(SYS_APIAKEYLO_EL1),
+	HOST_HANDLED(SYS_APIAKEYHI_EL1),
+	HOST_HANDLED(SYS_APIBKEYLO_EL1),
+	HOST_HANDLED(SYS_APIBKEYHI_EL1),
+	HOST_HANDLED(SYS_APDAKEYLO_EL1),
+	HOST_HANDLED(SYS_APDAKEYHI_EL1),
+	HOST_HANDLED(SYS_APDBKEYLO_EL1),
+	HOST_HANDLED(SYS_APDBKEYHI_EL1),
+	HOST_HANDLED(SYS_APGAKEYLO_EL1),
+	HOST_HANDLED(SYS_APGAKEYHI_EL1),
+
+	HOST_HANDLED(SYS_AFSR0_EL1),
+	HOST_HANDLED(SYS_AFSR1_EL1),
+	HOST_HANDLED(SYS_ESR_EL1),
+
+	HOST_HANDLED(SYS_ERRIDR_EL1),
+	HOST_HANDLED(SYS_ERRSELR_EL1),
+	HOST_HANDLED(SYS_ERXFR_EL1),
+	HOST_HANDLED(SYS_ERXCTLR_EL1),
+	HOST_HANDLED(SYS_ERXSTATUS_EL1),
+	HOST_HANDLED(SYS_ERXADDR_EL1),
+	HOST_HANDLED(SYS_ERXMISC0_EL1),
+	HOST_HANDLED(SYS_ERXMISC1_EL1),
+
+	HOST_HANDLED(SYS_TFSR_EL1),
+	HOST_HANDLED(SYS_TFSRE0_EL1),
+
+	HOST_HANDLED(SYS_FAR_EL1),
+	HOST_HANDLED(SYS_PAR_EL1),
+
+	/* Performance Monitoring Registers are restricted. */
+
+	HOST_HANDLED(SYS_MAIR_EL1),
+	HOST_HANDLED(SYS_AMAIR_EL1),
+
+	/* Limited Ordering Regions Registers are restricted. */
+
+	HOST_HANDLED(SYS_VBAR_EL1),
+	HOST_HANDLED(SYS_DISR_EL1),
+
+	/* GIC CPU Interface registers are restricted. */
+
+	HOST_HANDLED(SYS_CONTEXTIDR_EL1),
+	HOST_HANDLED(SYS_TPIDR_EL1),
+
+	HOST_HANDLED(SYS_SCXTNUM_EL1),
+
+	HOST_HANDLED(SYS_CNTKCTL_EL1),
+
+	HOST_HANDLED(SYS_CCSIDR_EL1),
+	HOST_HANDLED(SYS_CLIDR_EL1),
+	HOST_HANDLED(SYS_CSSELR_EL1),
+	HOST_HANDLED(SYS_CTR_EL0),
+
+	/* Performance Monitoring Registers are restricted. */
+
+	HOST_HANDLED(SYS_TPIDR_EL0),
+	HOST_HANDLED(SYS_TPIDRRO_EL0),
+
+	HOST_HANDLED(SYS_SCXTNUM_EL0),
+
+	/* Activity Monitoring Registers are restricted. */
+
+	HOST_HANDLED(SYS_CNTP_TVAL_EL0),
+	HOST_HANDLED(SYS_CNTP_CTL_EL0),
+	HOST_HANDLED(SYS_CNTP_CVAL_EL0),
+
+	/* Performance Monitoring Registers are restricted. */
+
+	HOST_HANDLED(SYS_DACR32_EL2),
+	HOST_HANDLED(SYS_IFSR32_EL2),
+	HOST_HANDLED(SYS_FPEXC32_EL2),
+};
+
+/*
+ * Handler for protected VM MSR, MRS or System instruction execution in AArch64.
+ *
+ * Return 1 if handled, or 0 if not.
+ */
+int kvm_handle_pvm_sys64(struct kvm_vcpu *vcpu)
+{
+	const struct sys_reg_desc *r;
+	struct sys_reg_params params;
+	unsigned long esr = kvm_vcpu_get_esr(vcpu);
+	int Rt = kvm_vcpu_sys_get_rt(vcpu);
+
+	params = esr_sys64_to_params(esr);
+	params.regval = vcpu_get_reg(vcpu, Rt);
+
+	r = find_reg(&params, pvm_sys_reg_descs, ARRAY_SIZE(pvm_sys_reg_descs));
+
+	/* Undefined access (RESTRICTED). */
+	if (r == NULL) {
+		inject_undef(vcpu);
+		return 1;
+	}
+
+	/* Handled by the host (HOST_HANDLED) */
+	if (r->access == NULL)
+		return 0;
+
+	/* Handled by hyp: skip instruction if instructed to do so. */
+	if (r->access(vcpu, &params, r))
+		__kvm_skip_instr(vcpu);
+
+	vcpu_set_reg(vcpu, Rt, params.regval);
+	return 1;
+}
+
+/*
+ * Handler for protected VM restricted exceptions.
+ *
+ * Inject an undefined exception into the guest and return 1 to indicate that
+ * it was handled.
+ */
+int kvm_handle_pvm_restricted(struct kvm_vcpu *vcpu)
+{
+	inject_undef(vcpu);
+	return 1;
+}
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 10/13] KVM: arm64: Move sanitized copies of CPU features
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (8 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 09/13] KVM: arm64: Add trap handlers for protected VMs Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 11/13] KVM: arm64: Trap access to pVM restricted features Fuad Tabba
                   ` (4 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Move the sanitized copies of the CPU feature registers to the
recently created sys_regs.c. This consolidates all copies in a
more relevant file.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/mem_protect.c | 6 ------
 arch/arm64/kvm/hyp/nvhe/sys_regs.c    | 5 +++++
 2 files changed, 5 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/mem_protect.c b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
index 4b60c0056c04..de734d29e938 100644
--- a/arch/arm64/kvm/hyp/nvhe/mem_protect.c
+++ b/arch/arm64/kvm/hyp/nvhe/mem_protect.c
@@ -26,12 +26,6 @@ struct host_kvm host_kvm;
 static struct hyp_pool host_s2_mem;
 static struct hyp_pool host_s2_dev;
 
-/*
- * Copies of the host's CPU features registers holding sanitized values.
- */
-u64 id_aa64mmfr0_el1_sys_val;
-u64 id_aa64mmfr1_el1_sys_val;
-
 static const u8 pkvm_hyp_id = 1;
 
 static void *host_s2_zalloc_pages_exact(size_t size)
diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
index 890c96315e55..998b1b48b089 100644
--- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
+++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
@@ -14,9 +14,14 @@
 
 #include "../../sys_regs.h"
 
+/*
+ * Copies of the host's CPU features registers holding sanitized values.
+ */
 u64 id_aa64pfr0_el1_sys_val;
 u64 id_aa64pfr1_el1_sys_val;
 u64 id_aa64dfr0_el1_sys_val;
+u64 id_aa64mmfr0_el1_sys_val;
+u64 id_aa64mmfr1_el1_sys_val;
 u64 id_aa64mmfr2_el1_sys_val;
 
 /*
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 11/13] KVM: arm64: Trap access to pVM restricted features
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (9 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 10/13] KVM: arm64: Move sanitized copies of CPU features Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 12/13] KVM: arm64: Handle protected guests at 32 bits Fuad Tabba
                   ` (3 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Trap accesses to restricted features for VMs running in protected
mode.

Access to feature registers are emulated, and only supported
features are exposed to protected VMs.

Accesses to restricted registers as well as restricted
instructions are trapped, and an undefined exception is injected
into the protected guest.

Only affects the functionality of protected VMs. Otherwise,
should not affect non-protected VMs when KVM is running in
protected mode.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/hyp/switch.h |   3 +
 arch/arm64/kvm/hyp/nvhe/switch.c        | 105 ++++++++++++++++++++----
 2 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index f5d3d1da0aec..d9f087ed6e02 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -33,6 +33,9 @@
 extern struct exception_table_entry __start___kvm_ex_table;
 extern struct exception_table_entry __stop___kvm_ex_table;
 
+int kvm_handle_pvm_sys64(struct kvm_vcpu *vcpu);
+int kvm_handle_pvm_restricted(struct kvm_vcpu *vcpu);
+
 /* Check whether the FP regs were dirtied while in the host-side run loop: */
 static inline bool update_fp_enabled(struct kvm_vcpu *vcpu)
 {
diff --git a/arch/arm64/kvm/hyp/nvhe/switch.c b/arch/arm64/kvm/hyp/nvhe/switch.c
index 967a3ad74fbd..48d5f780fe64 100644
--- a/arch/arm64/kvm/hyp/nvhe/switch.c
+++ b/arch/arm64/kvm/hyp/nvhe/switch.c
@@ -34,12 +34,63 @@ DEFINE_PER_CPU(struct kvm_host_data, kvm_host_data);
 DEFINE_PER_CPU(struct kvm_cpu_context, kvm_hyp_ctxt);
 DEFINE_PER_CPU(unsigned long, kvm_hyp_vector);
 
+/*
+ * Set EL2 configuration registers to trap restricted register accesses and
+ * instructions for protected VMs.
+ *
+ * Should be called right before vcpu entry to restrict its impact only to the
+ * protected guest.
+ */
+static void __activate_traps_pvm(struct kvm_vcpu *vcpu)
+{
+	u64 mdcr;
+	u64 hcr;
+	u64 cptr;
+
+	if (!kvm_vm_is_protected(kern_hyp_va(vcpu->kvm)))
+		return;
+
+	mdcr = read_sysreg(mdcr_el2);
+	hcr = read_sysreg(hcr_el2);
+	cptr = read_sysreg(cptr_el2);
+
+	hcr |= HCR_TID3 |			/* Feature Registers */
+	       HCR_TLOR |			/* LOR */
+	       HCR_RW |				/* AArch64 EL1 only */
+	       HCR_TERR |			/* RAS */
+	       HCR_ATA | HCR_TID5 |		/* Memory Tagging */
+	       HCR_TACR | HCR_TIDCP | HCR_TID1; /* Implementation defined */
+
+	hcr &= ~(HCR_DCT |	/* Memory Tagging */
+		 HCR_FIEN |	/* RAS */
+		 HCR_AMVOFFEN);	/* Disables AMU registers virtualization */
+
+	/* Debug and Trace */
+	mdcr |= MDCR_EL2_TDRA | MDCR_EL2_TDA | MDCR_EL2_TDE |
+		MDCR_EL2_TDOSA | MDCR_EL2_TDCC | MDCR_EL2_TTRF |
+		MDCR_EL2_TPM | MDCR_EL2_TPMCR |
+		MDCR_EL2_TPMS; /* SPE */
+
+	mdcr &= ~(MDCR_EL2_HPME | MDCR_EL2_MTPME |		/* PMU */
+		  (MDCR_EL2_E2PB_MASK << MDCR_EL2_E2PB_SHIFT)); /* SPE */
+
+	cptr |= CPTR_EL2_TTA |	/* Trace */
+		CPTR_EL2_TAM |	/* AMU */
+		CPTR_EL2_TZ;	/* SVE */
+
+	/*  __deactivate_traps() restores these registers. */
+	write_sysreg(mdcr, mdcr_el2);
+	write_sysreg(hcr, hcr_el2);
+	write_sysreg(cptr, cptr_el2);
+}
+
 static void __activate_traps(struct kvm_vcpu *vcpu)
 {
 	u64 val;
 
 	___activate_traps(vcpu);
 	__activate_traps_common(vcpu);
+	__activate_traps_pvm(vcpu);
 
 	val = CPTR_EL2_DEFAULT;
 	val |= CPTR_EL2_TTA | CPTR_EL2_TAM;
@@ -165,30 +216,56 @@ static void __pmu_switch_to_host(struct kvm_cpu_context *host_ctxt)
 		write_sysreg(pmu->events_host, pmcntenset_el0);
 }
 
+/**
+ * Handle system register accesses for protected VMs.
+ *
+ * Return 1 if handled, or 0 if not.
+ */
+static int handle_pvm_sys64(struct kvm_vcpu *vcpu)
+{
+	if (kvm_vm_is_protected(kern_hyp_va(vcpu->kvm)))
+		return kvm_handle_pvm_sys64(vcpu);
+	else
+		return 0;
+}
+
+/**
+ * Handle restricted feature accesses for protected VMs.
+ *
+ * Return 1 if handled, or 0 if not.
+ */
+static int handle_pvm_restricted(struct kvm_vcpu *vcpu)
+{
+	if (kvm_vm_is_protected(kern_hyp_va(vcpu->kvm)))
+		return kvm_handle_pvm_restricted(vcpu);
+	else
+		return 0;
+}
+
 typedef int (*exit_handle_fn)(struct kvm_vcpu *);
 
 static exit_handle_fn hyp_exit_handlers[] = {
-	[0 ... ESR_ELx_EC_MAX]		= NULL,
+	[0 ... ESR_ELx_EC_MAX]		= handle_pvm_restricted,
 	[ESR_ELx_EC_WFx]		= NULL,
-	[ESR_ELx_EC_CP15_32]		= NULL,
-	[ESR_ELx_EC_CP15_64]		= NULL,
-	[ESR_ELx_EC_CP14_MR]		= NULL,
-	[ESR_ELx_EC_CP14_LS]		= NULL,
-	[ESR_ELx_EC_CP14_64]		= NULL,
+	[ESR_ELx_EC_CP15_32]		= handle_pvm_restricted,
+	[ESR_ELx_EC_CP15_64]		= handle_pvm_restricted,
+	[ESR_ELx_EC_CP14_MR]		= handle_pvm_restricted,
+	[ESR_ELx_EC_CP14_LS]		= handle_pvm_restricted,
+	[ESR_ELx_EC_CP14_64]		= handle_pvm_restricted,
 	[ESR_ELx_EC_HVC32]		= NULL,
 	[ESR_ELx_EC_SMC32]		= NULL,
 	[ESR_ELx_EC_HVC64]		= NULL,
 	[ESR_ELx_EC_SMC64]		= NULL,
-	[ESR_ELx_EC_SYS64]		= NULL,
-	[ESR_ELx_EC_SVE]		= NULL,
+	[ESR_ELx_EC_SYS64]		= handle_pvm_sys64,
+	[ESR_ELx_EC_SVE]		= handle_pvm_restricted,
 	[ESR_ELx_EC_IABT_LOW]		= NULL,
 	[ESR_ELx_EC_DABT_LOW]		= NULL,
-	[ESR_ELx_EC_SOFTSTP_LOW]	= NULL,
-	[ESR_ELx_EC_WATCHPT_LOW]	= NULL,
-	[ESR_ELx_EC_BREAKPT_LOW]	= NULL,
-	[ESR_ELx_EC_BKPT32]		= NULL,
-	[ESR_ELx_EC_BRK64]		= NULL,
-	[ESR_ELx_EC_FP_ASIMD]		= NULL,
+	[ESR_ELx_EC_SOFTSTP_LOW]	= handle_pvm_restricted,
+	[ESR_ELx_EC_WATCHPT_LOW]	= handle_pvm_restricted,
+	[ESR_ELx_EC_BREAKPT_LOW]	= handle_pvm_restricted,
+	[ESR_ELx_EC_BKPT32]		= handle_pvm_restricted,
+	[ESR_ELx_EC_BRK64]		= handle_pvm_restricted,
+	[ESR_ELx_EC_FP_ASIMD]		= handle_pvm_restricted,
 	[ESR_ELx_EC_PAC]		= NULL,
 };
 
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 12/13] KVM: arm64: Handle protected guests at 32 bits
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (10 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 11/13] KVM: arm64: Trap access to pVM restricted features Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 14:11 ` [PATCH v1 13/13] KVM: arm64: Check vcpu features at pVM creation Fuad Tabba
                   ` (2 subsequent siblings)
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Protected KVM does not support protected AArch32 guests. However,
it is possible for the guest to force run AArch32, potentially
causing problems. Add an extra check so that if the hypervisor
catches the guest doing that, it can prevent the guest from
running again by resetting vcpu->arch.target and returning
ARM_EXCEPTION_IL.

Adapted from commit 22f553842b14 ("KVM: arm64: Handle Asymmetric
AArch32 systems")

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/include/hyp/switch.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index d9f087ed6e02..672801f79579 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -447,6 +447,26 @@ static inline bool fixup_guest_exit(struct kvm_vcpu *vcpu, u64 *exit_code)
 			write_sysreg_el2(read_sysreg_el2(SYS_ELR) - 4, SYS_ELR);
 	}
 
+	/*
+	 * Protected VMs are not allowed to run in AArch32. The check below is
+	 * based on the one in kvm_arch_vcpu_ioctl_run().
+	 * The ARMv8 architecture doesn't give the hypervisor a mechanism to
+	 * prevent a guest from dropping to AArch32 EL0 if implemented by the
+	 * CPU. If the hypervisor spots a guest in such a state ensure it is
+	 * handled, and don't trust the host to spot or fix it.
+	 */
+	if (unlikely(is_nvhe_hyp_code() &&
+		     kvm_vm_is_protected(kern_hyp_va(vcpu->kvm)) &&
+		     vcpu_mode_is_32bit(vcpu))) {
+		/*
+		 * As we have caught the guest red-handed, decide that it isn't
+		 * fit for purpose anymore by making the vcpu invalid.
+		 */
+		vcpu->arch.target = -1;
+		*exit_code = ARM_EXCEPTION_IL;
+		goto exit;
+	}
+
 	/*
 	 * We're using the raw exception code in order to only process
 	 * the trap if no SError is pending. We will come back to the
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* [PATCH v1 13/13] KVM: arm64: Check vcpu features at pVM creation
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (11 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 12/13] KVM: arm64: Handle protected guests at 32 bits Fuad Tabba
@ 2021-06-08 14:11 ` Fuad Tabba
  2021-06-08 15:07 ` [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Andrew Jones
  2021-06-11 12:44 ` Alexandru Elisei
  14 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-08 14:11 UTC (permalink / raw)
  To: kvmarm
  Cc: maz, will, james.morse, alexandru.elisei, suzuki.poulose,
	mark.rutland, christoffer.dall, pbonzini, qperret, kvm,
	linux-arm-kernel, kernel-team, tabba

Check that a protected VM is not setting any of the unsupported
features when it's created.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/pkvm.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/arch/arm64/kvm/pkvm.c b/arch/arm64/kvm/pkvm.c
index cf624350fb27..5e58d604faec 100644
--- a/arch/arm64/kvm/pkvm.c
+++ b/arch/arm64/kvm/pkvm.c
@@ -88,10 +88,41 @@ static void pkvm_teardown_firmware_slot(struct kvm *kvm)
 	kvm->arch.pkvm.firmware_slot = NULL;
 }
 
+/*
+ * Check that no unsupported features are enabled for the protected VM's vcpus.
+ *
+ * Return 0 if all features enabled for all vcpus are supported, or -EINVAL if
+ * one or more vcpus has one or more unsupported features.
+ */
+static int pkvm_check_features(struct kvm *kvm)
+{
+	int i;
+	const struct kvm_vcpu *vcpu;
+
+	kvm_for_each_vcpu(i, vcpu, kvm) {
+		/*
+		 * No support for:
+		 * - AArch32 state for protected VMs
+		 * - Performance Monitoring
+		 * - Scalable Vectors
+		 */
+		if (test_bit(KVM_ARM_VCPU_EL1_32BIT, vcpu->arch.features) ||
+		    test_bit(KVM_ARM_VCPU_PMU_V3, vcpu->arch.features) ||
+		    test_bit(KVM_ARM_VCPU_SVE, vcpu->arch.features))
+			return -EINVAL;
+	}
+
+	return 0;
+}
+
 static int pkvm_enable(struct kvm *kvm, u64 slotid)
 {
 	int ret;
 
+	ret = pkvm_check_features(kvm);
+	if (ret)
+		return ret;
+
 	ret = pkvm_init_firmware_slot(kvm, slotid);
 	if (ret)
 		return ret;
-- 
2.32.0.rc1.229.g3e70b5a671-goog


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

* Re: [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (12 preceding siblings ...)
  2021-06-08 14:11 ` [PATCH v1 13/13] KVM: arm64: Check vcpu features at pVM creation Fuad Tabba
@ 2021-06-08 15:07 ` Andrew Jones
  2021-06-09 15:22   ` Fuad Tabba
  2021-06-11 12:44 ` Alexandru Elisei
  14 siblings, 1 reply; 18+ messages in thread
From: Andrew Jones @ 2021-06-08 15:07 UTC (permalink / raw)
  To: Fuad Tabba
  Cc: kvmarm, kernel-team, kvm, maz, pbonzini, will, linux-arm-kernel

On Tue, Jun 08, 2021 at 03:11:28PM +0100, Fuad Tabba wrote:
> Hi,
> 
> This patch series adds support for restricting CPU features for protected VMs
> in KVM [1].
> 
> Various feature configurations are allowed in KVM/arm64. Supporting all
> these features in pKVM is difficult, as it either involves moving much of
> the handling code to EL2, which adds bloat and results in a less verifiable
> trusted code base. Or it involves leaving the code handling at EL1, which
> risks having an untrusted host kernel feeding wrong information to the EL2
> and to the protected guests.
> 
> This series attempts to mitigate this by reducing the configuration space,
> providing a reduced amount of feature support at EL2 with the least amount of
> compromise of protected guests' capabilities.
> 
> This is done by restricting CPU features exposed to protected guests through
> feature registers. These restrictions are enforced by trapping register
> accesses as well as instructions associated with these features, and injecting
> an undefined exception into the guest if it attempts to use a restricted
> feature.
> 
> The features being restricted (only for protected VMs in protected mode) are
> the following:
> - Debug, Trace, and DoubleLock
> - Performance Monitoring (PMU)
> - Statistical Profiling (SPE)
> - Scalable Vector Extension (SVE)
> - Memory Partitioning and Monitoring (MPAM)
> - Activity Monitoring (AMU)
> - Memory Tagging (MTE)
> - Limited Ordering Regions (LOR)
> - AArch32 State
> - Generic Interrupt Controller (GIC) (depending on rVIC support)
> - Nested Virtualization (NV)
> - Reliability, Availability, and Serviceability (RAS) above V1
> - Implementation-defined Features

Hi Fuad,

I see this series takes the approach we currently have in KVM of masking
features we don't want to expose to the guest. This approach adds yet
another "reject list" to be maintained as hardware evolves. I'd rather see
that we first change KVM to using an accept list, i.e. mask everything and
then only set what we want to enable. Mimicking that new accept list in
pKVM, where much less would be enabled, would reduce the amount of
maintenance needed.

Thanks,
drew

> 
> This series is based on kvmarm/next and Will's patches for an Initial pKVM user
> ABI [1]. You can find the applied series here [2].
> 
> Cheers,
> /fuad
> 
> [1] https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/
> 
> For more details about pKVM, please refer to Will's talk at KVM Forum 2020:
> https://www.youtube.com/watch?v=edqJSzsDRxk
> 
> [2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v1
> 
> To: kvmarm@lists.cs.columbia.edu
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Will Deacon <will@kernel.org>
> Cc: James Morse <james.morse@arm.com>
> Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> Cc: Mark Rutland <mark.rutland@arm.com>
> Cc: Christoffer Dall <christoffer.dall@arm.com>
> Cc: Paolo Bonzini <pbonzini@redhat.com>
> Cc: Quentin Perret <qperret@google.com>
> Cc: kvm@vger.kernel.org
> Cc: linux-arm-kernel@lists.infradead.org
> Cc: kernel-team@android.com
> 
> Fuad Tabba (13):
>   KVM: arm64: Remove trailing whitespace in comments
>   KVM: arm64: MDCR_EL2 is a 64-bit register
>   KVM: arm64: Fix name of HCR_TACR to match the spec
>   KVM: arm64: Refactor sys_regs.h,c for nVHE reuse
>   KVM: arm64: Restore mdcr_el2 from vcpu
>   KVM: arm64: Add feature register flag definitions
>   KVM: arm64: Add config register bit definitions
>   KVM: arm64: Guest exit handlers for nVHE hyp
>   KVM: arm64: Add trap handlers for protected VMs
>   KVM: arm64: Move sanitized copies of CPU features
>   KVM: arm64: Trap access to pVM restricted features
>   KVM: arm64: Handle protected guests at 32 bits
>   KVM: arm64: Check vcpu features at pVM creation
> 
>  arch/arm64/include/asm/kvm_arm.h        |  34 +-
>  arch/arm64/include/asm/kvm_asm.h        |   2 +-
>  arch/arm64/include/asm/kvm_host.h       |   2 +-
>  arch/arm64/include/asm/kvm_hyp.h        |   4 +
>  arch/arm64/include/asm/sysreg.h         |   6 +
>  arch/arm64/kvm/arm.c                    |   4 +
>  arch/arm64/kvm/debug.c                  |   5 +-
>  arch/arm64/kvm/hyp/include/hyp/switch.h |  42 ++
>  arch/arm64/kvm/hyp/nvhe/Makefile        |   2 +-
>  arch/arm64/kvm/hyp/nvhe/debug-sr.c      |   2 +-
>  arch/arm64/kvm/hyp/nvhe/mem_protect.c   |   6 -
>  arch/arm64/kvm/hyp/nvhe/switch.c        | 114 +++++-
>  arch/arm64/kvm/hyp/nvhe/sys_regs.c      | 501 ++++++++++++++++++++++++
>  arch/arm64/kvm/hyp/vhe/debug-sr.c       |   2 +-
>  arch/arm64/kvm/pkvm.c                   |  31 ++
>  arch/arm64/kvm/sys_regs.c               |  62 +--
>  arch/arm64/kvm/sys_regs.h               |  35 ++
>  17 files changed, 782 insertions(+), 72 deletions(-)
>  create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
> 
> 
> base-commit: 35b256a5eebe3ac715b4ea6234aa4236a10d1a88
> -- 
> 2.32.0.rc1.229.g3e70b5a671-goog
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> 


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

* Re: [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs
  2021-06-08 15:07 ` [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Andrew Jones
@ 2021-06-09 15:22   ` Fuad Tabba
  0 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-09 15:22 UTC (permalink / raw)
  To: Andrew Jones
  Cc: open list:KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64),
	kernel-team, kvm, Marc Zyngier, pbonzini, Will Deacon,
	moderated list:ARM64 PORT (AARCH64 ARCHITECTURE)

Hi Drew,

> I see this series takes the approach we currently have in KVM of masking
> features we don't want to expose to the guest. This approach adds yet
> another "reject list" to be maintained as hardware evolves. I'd rather see
> that we first change KVM to using an accept list, i.e. mask everything and
> then only set what we want to enable. Mimicking that new accept list in
> pKVM, where much less would be enabled, would reduce the amount of
> maintenance needed.

Good point. I agree that having an allow list is preferable to having
a block list. The way this patch series handles system register
accesses is actually an allow list. However, as it is now, features
being exposed to protected guests via the feature registers take the
block list approach. I will fix that to ensure that instead it exposes
a list of allowed features, rather than hiding restricted ones. As you
suggest, this would reduce the amount of maintenance as hardware
evolves and is better for security as well.

As for changing KVM first, I think that that's orthogonal to what this
series is trying to accomplish. Features in pKVM are not controlled or
negotiable by userspace, as it is a fixed virtual platform. When KVM
changes to use allow lists instead, it shouldn't conflict with how
this series works, especially if I fix it to use an allow list
instead.

Thanks for your feedback.

Cheers,
/fuad


> Thanks,
> drew
>
> >
> > This series is based on kvmarm/next and Will's patches for an Initial pKVM user
> > ABI [1]. You can find the applied series here [2].
> >
> > Cheers,
> > /fuad
> >
> > [1] https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/
> >
> > For more details about pKVM, please refer to Will's talk at KVM Forum 2020:
> > https://www.youtube.com/watch?v=edqJSzsDRxk
> >
> > [2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v1
> >
> > To: kvmarm@lists.cs.columbia.edu
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Will Deacon <will@kernel.org>
> > Cc: James Morse <james.morse@arm.com>
> > Cc: Alexandru Elisei <alexandru.elisei@arm.com>
> > Cc: Suzuki K Poulose <suzuki.poulose@arm.com>
> > Cc: Mark Rutland <mark.rutland@arm.com>
> > Cc: Christoffer Dall <christoffer.dall@arm.com>
> > Cc: Paolo Bonzini <pbonzini@redhat.com>
> > Cc: Quentin Perret <qperret@google.com>
> > Cc: kvm@vger.kernel.org
> > Cc: linux-arm-kernel@lists.infradead.org
> > Cc: kernel-team@android.com
> >
> > Fuad Tabba (13):
> >   KVM: arm64: Remove trailing whitespace in comments
> >   KVM: arm64: MDCR_EL2 is a 64-bit register
> >   KVM: arm64: Fix name of HCR_TACR to match the spec
> >   KVM: arm64: Refactor sys_regs.h,c for nVHE reuse
> >   KVM: arm64: Restore mdcr_el2 from vcpu
> >   KVM: arm64: Add feature register flag definitions
> >   KVM: arm64: Add config register bit definitions
> >   KVM: arm64: Guest exit handlers for nVHE hyp
> >   KVM: arm64: Add trap handlers for protected VMs
> >   KVM: arm64: Move sanitized copies of CPU features
> >   KVM: arm64: Trap access to pVM restricted features
> >   KVM: arm64: Handle protected guests at 32 bits
> >   KVM: arm64: Check vcpu features at pVM creation
> >
> >  arch/arm64/include/asm/kvm_arm.h        |  34 +-
> >  arch/arm64/include/asm/kvm_asm.h        |   2 +-
> >  arch/arm64/include/asm/kvm_host.h       |   2 +-
> >  arch/arm64/include/asm/kvm_hyp.h        |   4 +
> >  arch/arm64/include/asm/sysreg.h         |   6 +
> >  arch/arm64/kvm/arm.c                    |   4 +
> >  arch/arm64/kvm/debug.c                  |   5 +-
> >  arch/arm64/kvm/hyp/include/hyp/switch.h |  42 ++
> >  arch/arm64/kvm/hyp/nvhe/Makefile        |   2 +-
> >  arch/arm64/kvm/hyp/nvhe/debug-sr.c      |   2 +-
> >  arch/arm64/kvm/hyp/nvhe/mem_protect.c   |   6 -
> >  arch/arm64/kvm/hyp/nvhe/switch.c        | 114 +++++-
> >  arch/arm64/kvm/hyp/nvhe/sys_regs.c      | 501 ++++++++++++++++++++++++
> >  arch/arm64/kvm/hyp/vhe/debug-sr.c       |   2 +-
> >  arch/arm64/kvm/pkvm.c                   |  31 ++
> >  arch/arm64/kvm/sys_regs.c               |  62 +--
> >  arch/arm64/kvm/sys_regs.h               |  35 ++
> >  17 files changed, 782 insertions(+), 72 deletions(-)
> >  create mode 100644 arch/arm64/kvm/hyp/nvhe/sys_regs.c
> >
> >
> > base-commit: 35b256a5eebe3ac715b4ea6234aa4236a10d1a88
> > --
> > 2.32.0.rc1.229.g3e70b5a671-goog
> >
> > _______________________________________________
> > kvmarm mailing list
> > kvmarm@lists.cs.columbia.edu
> > https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
> >
>

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

* Re: [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs
  2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
                   ` (13 preceding siblings ...)
  2021-06-08 15:07 ` [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Andrew Jones
@ 2021-06-11 12:44 ` Alexandru Elisei
  2021-06-13 16:12   ` Fuad Tabba
  14 siblings, 1 reply; 18+ messages in thread
From: Alexandru Elisei @ 2021-06-11 12:44 UTC (permalink / raw)
  To: Fuad Tabba, kvmarm
  Cc: maz, will, james.morse, suzuki.poulose, mark.rutland,
	christoffer.dall, pbonzini, qperret, kvm, linux-arm-kernel,
	kernel-team

Hi,

On 6/8/21 3:11 PM, Fuad Tabba wrote:
> Hi,
>
> This patch series adds support for restricting CPU features for protected VMs
> in KVM [1].
>
> Various feature configurations are allowed in KVM/arm64. Supporting all
> these features in pKVM is difficult, as it either involves moving much of
> the handling code to EL2, which adds bloat and results in a less verifiable
> trusted code base. Or it involves leaving the code handling at EL1, which
> risks having an untrusted host kernel feeding wrong information to the EL2
> and to the protected guests.
>
> This series attempts to mitigate this by reducing the configuration space,
> providing a reduced amount of feature support at EL2 with the least amount of
> compromise of protected guests' capabilities.
>
> This is done by restricting CPU features exposed to protected guests through
> feature registers. These restrictions are enforced by trapping register
> accesses as well as instructions associated with these features, and injecting
> an undefined exception into the guest if it attempts to use a restricted
> feature.
>
> The features being restricted (only for protected VMs in protected mode) are
> the following:
> - Debug, Trace, and DoubleLock
> - Performance Monitoring (PMU)
> - Statistical Profiling (SPE)
> - Scalable Vector Extension (SVE)
> - Memory Partitioning and Monitoring (MPAM)
> - Activity Monitoring (AMU)
> - Memory Tagging (MTE)
> - Limited Ordering Regions (LOR)
> - AArch32 State
> - Generic Interrupt Controller (GIC) (depending on rVIC support)
> - Nested Virtualization (NV)
> - Reliability, Availability, and Serviceability (RAS) above V1
> - Implementation-defined Features
>
> This series is based on kvmarm/next and Will's patches for an Initial pKVM user
> ABI [1]. You can find the applied series here [2].

Since this is implementing the kernel side of an RFC userspace ABI, I'm going to
treat the series as an RFC also and not go into the individual patches.

What strikes me as odd is the fact that, as far as I can tell, you're duplicating
part of the kvm/sys_regs.c and kvm/handle_exit.c functionality in the nvhe code.
Why was this approach chosen instead of reusing the existing functions and adding
extra code to handle the protected VM case?

Another example of this is detecting when a host dropped to 32bit EL0, the comment
says that you don't trust the host to make the check. What exactly do you trust
the host to do at what point? I don't see this explained anywhere, it's possible
I've missed it.

I also think that registers that mostly don't change during the lifetime of the VM
(HCR_EL2, CPTR_EL2, MDCR_EL2) can be set by host when the VM becomes protected,
instead of fiddling with them at each world switch. Was there a particular reason
for changing them in __activate_traps_pvm() or was this just an implementation choice?

Thanks,

Alex

> Cheers,
> /fuad
>
> [1] https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/
>
> For more details about pKVM, please refer to Will's talk at KVM Forum 2020:
> https://www.youtube.com/watch?v=edqJSzsDRxk
>
> [2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v1

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

* Re: [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs
  2021-06-11 12:44 ` Alexandru Elisei
@ 2021-06-13 16:12   ` Fuad Tabba
  0 siblings, 0 replies; 18+ messages in thread
From: Fuad Tabba @ 2021-06-13 16:12 UTC (permalink / raw)
  To: Alexandru Elisei
  Cc: open list:KERNEL VIRTUAL MACHINE FOR ARM64 (KVM/arm64),
	Marc Zyngier, Will Deacon, James Morse, Suzuki K Poulose,
	Mark Rutland, christoffer.dall, Paolo Bonzini, Quentin Perret,
	KVM, moderated list:ARM64 PORT (AARCH64 ARCHITECTURE),
	kernel-team

Hi Alex,

On Fri, Jun 11, 2021 at 1:43 PM Alexandru Elisei
<alexandru.elisei@arm.com> wrote:
>
> Hi,
>
> On 6/8/21 3:11 PM, Fuad Tabba wrote:
> > Hi,
> >
> > This patch series adds support for restricting CPU features for protected VMs
> > in KVM [1].
> >
> > Various feature configurations are allowed in KVM/arm64. Supporting all
> > these features in pKVM is difficult, as it either involves moving much of
> > the handling code to EL2, which adds bloat and results in a less verifiable
> > trusted code base. Or it involves leaving the code handling at EL1, which
> > risks having an untrusted host kernel feeding wrong information to the EL2
> > and to the protected guests.
> >
> > This series attempts to mitigate this by reducing the configuration space,
> > providing a reduced amount of feature support at EL2 with the least amount of
> > compromise of protected guests' capabilities.
> >
> > This is done by restricting CPU features exposed to protected guests through
> > feature registers. These restrictions are enforced by trapping register
> > accesses as well as instructions associated with these features, and injecting
> > an undefined exception into the guest if it attempts to use a restricted
> > feature.
> >
> > The features being restricted (only for protected VMs in protected mode) are
> > the following:
> > - Debug, Trace, and DoubleLock
> > - Performance Monitoring (PMU)
> > - Statistical Profiling (SPE)
> > - Scalable Vector Extension (SVE)
> > - Memory Partitioning and Monitoring (MPAM)
> > - Activity Monitoring (AMU)
> > - Memory Tagging (MTE)
> > - Limited Ordering Regions (LOR)
> > - AArch32 State
> > - Generic Interrupt Controller (GIC) (depending on rVIC support)
> > - Nested Virtualization (NV)
> > - Reliability, Availability, and Serviceability (RAS) above V1
> > - Implementation-defined Features
> >
> > This series is based on kvmarm/next and Will's patches for an Initial pKVM user
> > ABI [1]. You can find the applied series here [2].
>
> Since this is implementing the kernel side of an RFC userspace ABI, I'm going to
> treat the series as an RFC also and not go into the individual patches.
>
> What strikes me as odd is the fact that, as far as I can tell, you're duplicating
> part of the kvm/sys_regs.c and kvm/handle_exit.c functionality in the nvhe code.
> Why was this approach chosen instead of reusing the existing functions and adding
> extra code to handle the protected VM case?
>
> Another example of this is detecting when a host dropped to 32bit EL0, the comment
> says that you don't trust the host to make the check. What exactly do you trust
> the host to do at what point? I don't see this explained anywhere, it's possible
> I've missed it.

You're right. I haven't explained this in the patch series or provided
enough context other than a link to Will's presentation [1].

The idea is that protected VMs are protected from the host Linux
kernel (and from other VMs), where the host does not have access to
guest memory even if compromised. This patch series does not cover
that part yet. It is a part of, and builds on, other concurrent work
in order to get us there eventually [2].

Once everything falls into place, the host should not even have access
to a protected guest's state or anything that would enable it to
manipulate it (e.g., vcpu register context and el2 system registers),
only hyp would have that access. If the host could access that state,
then it might be able to get around the protection provided.
Therefore, anything that is sensitive and that would require such
access needs to happen at hyp, hence the code in nvhe running only at
hyp.

> I also think that registers that mostly don't change during the lifetime of the VM
> (HCR_EL2, CPTR_EL2, MDCR_EL2) can be set by host when the VM becomes protected,
> instead of fiddling with them at each world switch. Was there a particular reason
> for changing them in __activate_traps_pvm() or was this just an implementation choice?

You're right that they don't change during the lifetime of a VM, but
protected VMs can coexist with non-protected VMs. The values of these
registers are different between the two (different trapping to control
protection as well as enabled features). Thus the new
__activate_traps_pvm(), which would activate traps specifically for
protected VMs.

Thank you,
/fuad

[1]
https://mirrors.edge.kernel.org/pub/linux/kernel/people/will/slides/kvmforum-2020-edited.pdf

[2] Some of the work on protected KVM:
https://lore.kernel.org/kvmarm/20201202184122.26046-1-dbrazdil@google.com/
https://lore.kernel.org/kvmarm/20210602094347.3730846-1-qperret@google.com/
https://lore.kernel.org/kvmarm/20210608114518.748712-1-qperret@google.com/
https://lore.kernel.org/kvmarm/20210322175639.801566-1-maz@kernel.org/
https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/






> Thanks,
>
> Alex
>
> > Cheers,
> > /fuad
> >
> > [1] https://lore.kernel.org/kvmarm/20210603183347.1695-1-will@kernel.org/
> >
> > For more details about pKVM, please refer to Will's talk at KVM Forum 2020:
> > https://www.youtube.com/watch?v=edqJSzsDRxk
> >
> > [2] https://android-kvm.googlesource.com/linux/+/refs/heads/tabba/el2_fixed_feature_v1

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

end of thread, other threads:[~2021-06-13 16:12 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-08 14:11 [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 01/13] KVM: arm64: Remove trailing whitespace in comments Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 02/13] KVM: arm64: MDCR_EL2 is a 64-bit register Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 03/13] KVM: arm64: Fix name of HCR_TACR to match the spec Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 04/13] KVM: arm64: Refactor sys_regs.h,c for nVHE reuse Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 05/13] KVM: arm64: Restore mdcr_el2 from vcpu Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 06/13] KVM: arm64: Add feature register flag definitions Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 07/13] KVM: arm64: Add config register bit definitions Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 08/13] KVM: arm64: Guest exit handlers for nVHE hyp Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 09/13] KVM: arm64: Add trap handlers for protected VMs Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 10/13] KVM: arm64: Move sanitized copies of CPU features Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 11/13] KVM: arm64: Trap access to pVM restricted features Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 12/13] KVM: arm64: Handle protected guests at 32 bits Fuad Tabba
2021-06-08 14:11 ` [PATCH v1 13/13] KVM: arm64: Check vcpu features at pVM creation Fuad Tabba
2021-06-08 15:07 ` [PATCH v1 00/13] KVM: arm64: Fixed features for protected VMs Andrew Jones
2021-06-09 15:22   ` Fuad Tabba
2021-06-11 12:44 ` Alexandru Elisei
2021-06-13 16:12   ` Fuad Tabba

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).