Skip to contentSkip to conversation

KVM: arm64: Add EL2 support to FEAT_S1PIE

PATCH Superseded Marc Zyngier 2 participants 31 messages Sep 11, 2024 – Sep 12, 2024

Subsystems (3)
KVM arm64 KVM arm64

v3 of 5 Superseded Latest is v5 →

lore
Share & tools
lore

Marc Zyngier

author maintainer
#0 lore raw

v3 cover KVM: arm64: Add EL2 support to FEAT_S1PIE

This series serves a few purposes:

- Complete the S1PIE support to include EL2
- Sneak in the EL2 system register world switch

As mentioned in few of the patches, this implementation relies on a very recent fix to the architecture (D22677 in [0]).

* From v2 [2]:

- Correctly reprogram the context for AT in the fast path when S1PIE is in use

- Generalised sysreg RES0/RES1 masking, and added TCR2_EL2 RES0/RES1 bit handling

- Fix TCR2_EL1, PIR_EL1, PIRE0_EL1 access with VHE

- Add a bunch of missing registers to get_el2_to_el1_mapping()

- Correctly map {TCR2,PIR,PIRE0}_EL2 to their EL1 equivalent on NV

- Disable hierarchical permissions when S1PIE is enabled

- Make EL2 world switch directly act on the vcpu rather than an arbitrary context

- Remove SKL{0,1} from the TCR2_EL2 description

* From the initial posting [1]:

- Rebased on top of the AT support branch, which is currently sitting in kvmarm/next

- Add handling for S1 indirect permission in AT, which I'm sure will give Alexandru another king-sized headache

- Picked Mark Brown's series [3] dealing with TCRX and S1PIE visibility, and slapped an extra fix on top for good measure

- Picked up RBs from Joey, with thanks.

[0] https://developer.arm.com/documentation/102105/ka-04/
[1] https://lore.kernel.org/r/[email protected]
[2] https://lore.kernel.org/r/[email protected]
[3] https://lore.kernel.org/r/[email protected]

Marc Zyngier (21):
arm64: Drop SKL0/SKL1 from TCR2_EL2
arm64: Remove VNCR definition for PIRE0_EL2
arm64: Add encoding for PIRE0_EL2
KVM: arm64: nv: Add missing EL2->EL1 mappings in

    get_el2_to_el1_mapping()

KVM: arm64: nv: Handle CNTHCTL_EL2 specially
KVM: arm64: nv: Save/Restore vEL2 sysregs
KVM: arm64: Correctly access TCR2_EL1, PIR_EL1, PIRE0_EL1 with VHE
KVM: arm64: Extend masking facility to arbitrary registers
arm64: Define ID_AA64MMFR1_EL1.HAFDBS advertising FEAT_HAFT
KVM: arm64: Add TCR2_EL2 to the sysreg arrays
KVM: arm64: Sanitise TCR2_EL2
KVM: arm64: Add save/restore for TCR2_EL2
KVM: arm64: Add PIR{,E0}_EL2 to the sysreg arrays
KVM: arm64: Add save/restore for PIR{,E0}_EL2
KVM: arm64: Handle PIR{,E0}_EL2 traps
KVM: arm64: Sanitise ID_AA64MMFR3_EL1
KVM: arm64: Add AT fast-path support for S1PIE
KVM: arm64: Split S1 permission evaluation into direct and
hierarchical parts
KVM: arm64: Disable hierarchical permissions when S1PIE is enabled
KVM: arm64: Implement AT S1PIE support
KVM: arm64: Rely on visibility to let PIR*_ELx/TCR2_ELx UNDEF

Mark Brown (3):
KVM: arm64: Define helper for EL2 registers with custom visibility
KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests
KVM: arm64: Hide S1PIE registers from userspace when disabled for
guests

 arch/arm64/include/asm/kvm_host.h          |  34 ++-
 arch/arm64/include/asm/vncr_mapping.h      |   1 -
 arch/arm64/kvm/at.c                        | 322 +++++++++++++++++----
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |   5 +-
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c        |   2 +-
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c         | 155 +++++++++-
 arch/arm64/kvm/nested.c                    |  34 ++-
 arch/arm64/kvm/sys_regs.c                  | 129 ++++++++-
 arch/arm64/tools/sysreg                    |   8 +-
 include/kvm/arm_arch_timer.h               |   3 +
 10 files changed, 597 insertions(+), 96 deletions(-)
Signature · 1 line
2.39.2
author maintainer
#1 lore raw

PATCH v3 1/24 arm64: Drop SKL0/SKL1 from TCR2_EL2

Despite what the documentation says, TCR2_EL2.{SKL0,SKL1} do not exist, and the corresponding information is in the respective TTBRx_EL2. This is a leftover from a development version of the architecture.

This change makes TCR2_EL2 similar to TCR2_EL1 in that respect.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/tools/sysreg | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm64/tools/sysreg +1 -2
index 7ceaa1e0b4bc2..27c71fe3952f1 100644
@@ -2789,8 +2789,7 @@ Field	13	AMEC1
 Field	12	AMEC0
 Field	11	HAFT
 Field	10	PTTWI
-Field	9:8	SKL1
-Field	7:6	SKL0
+Res0	9:6
 Field	5	D128
 Field	4	AIE
 Field	3	POE
Signature · 1 line
2.39.2
reviewer Reviewed-by on v3 1/24
#2 lore raw

Re: [PATCH v3 01/24] arm64: Drop SKL0/SKL1 from TCR2_EL2

Quoted 8 lines · Marc Zyngier

Despite what the documentation says, TCR2_EL2.{SKL0,SKL1} do not exist, and the corresponding information is in the respective TTBRx_EL2. This is a leftover from a development version of the architecture.

This change makes TCR2_EL2 similar to TCR2_EL1 in that respect.

Signed-off-by Marc Zyngier <[email protected]>

---

Confirmed this should eventually be part of the Known Issues doc.

Reviewed-by Joey Gouly <[email protected]>
Quoted 20 lines
 arch/arm64/tools/sysreg | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
arch/arm64/tools/sysreg +1 -2
index 7ceaa1e0b4bc2..27c71fe3952f1 100644
@@ -2789,8 +2789,7 @@ Field	13	AMEC1
 Field	12	AMEC0
 Field	11	HAFT
 Field	10	PTTWI
-Field	9:8	SKL1
-Field	7:6	SKL0
+Res0	9:6
 Field	5	D128
 Field	4	AIE
 Field	3	POE
Signature · 1 line
2.39.2
author maintainer
#3 lore raw

PATCH v3 2/24 arm64: Remove VNCR definition for PIRE0_EL2

In reply to Marc Zyngier

As of the ARM ARM Known Issues document 102105_K.a_04_en, D22677 fixes a problem with the PIRE0_EL2 register, resulting in its removal from the VNCR page (it had no purpose being there the first place).

Follow the architecture update by removing this offset.

Reviewed-by Joey Gouly <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/vncr_mapping.h | 1 -
 1 file changed, 1 deletion(-)
arch/arm64/include/asm/vncr_mapping.h +0 -1
index df2c47c559728..9e593bb609750 100644
@@ -50,7 +50,6 @@
 #define VNCR_VBAR_EL1           0x250
 #define VNCR_TCR2_EL1		0x270
 #define VNCR_PIRE0_EL1		0x290
-#define VNCR_PIRE0_EL2		0x298
 #define VNCR_PIR_EL1		0x2A0
 #define VNCR_ICH_LR0_EL2        0x400
 #define VNCR_ICH_LR1_EL2        0x408
Signature · 1 line
2.39.2
author maintainer
#4 lore raw

PATCH v3 3/24 arm64: Add encoding for PIRE0_EL2

In reply to Marc Zyngier

PIRE0_EL2 is the equivalent of PIRE0_EL1 for the EL2&0 translation regime, and it is sorely missing from the sysreg file.

Add the sucker.

Reviewed-by Joey Gouly <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/tools/sysreg | 4 ++++
 1 file changed, 4 insertions(+)
arch/arm64/tools/sysreg +4 -0
index 27c71fe3952f1..d80859565547e 100644
@@ -2852,6 +2852,10 @@ Sysreg	PIRE0_EL12	3	5	10	2	2
 Fields	PIRx_ELx
 EndSysreg
 
+Sysreg	PIRE0_EL2	3	4	10	2	2
+Fields	PIRx_ELx
+EndSysreg
+
 Sysreg	PIR_EL1		3	0	10	2	3
 Fields	PIRx_ELx
 EndSysreg
Signature · 1 line
2.39.2
author maintainer
#5 lore raw

PATCH v3 4/24 KVM: arm64: nv: Add missing EL2->EL1 mappings in get_el2_to_el1_mapping()

In reply to Marc Zyngier

As KVM has grown a bunch of new system register for NV, it appears that we are missing them in the get_el2_to_el1_mapping() list.

Most of them are not crucial as they don't tend to be accessed via vcpu_read_sys_reg() and vcpu_write_sys_reg().

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/sys_regs.c | 9 +++++++++
 1 file changed, 9 insertions(+)
arch/arm64/kvm/sys_regs.c +9 -0
index 5ab0b27993936..46db7988d1b50 100644
@@ -102,6 +102,14 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
 		PURE_EL2_SYSREG(  RVBAR_EL2	);
 		PURE_EL2_SYSREG(  TPIDR_EL2	);
 		PURE_EL2_SYSREG(  HPFAR_EL2	);
+		PURE_EL2_SYSREG(  HCRX_EL2	);
+		PURE_EL2_SYSREG(  HFGRTR_EL2	);
+		PURE_EL2_SYSREG(  HFGWTR_EL2	);
+		PURE_EL2_SYSREG(  HFGITR_EL2	);
+		PURE_EL2_SYSREG(  HDFGRTR_EL2	);
+		PURE_EL2_SYSREG(  HDFGWTR_EL2	);
+		PURE_EL2_SYSREG(  HAFGRTR_EL2	);
+		PURE_EL2_SYSREG(  CNTVOFF_EL2	);
 		PURE_EL2_SYSREG(  CNTHCTL_EL2	);
 		MAPPED_EL2_SYSREG(SCTLR_EL2,   SCTLR_EL1,
 				  translate_sctlr_el2_to_sctlr_el1	     );

@@ -122,6 +130,7 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
 		MAPPED_EL2_SYSREG(ELR_EL2,     ELR_EL1,	    NULL	     );
 		MAPPED_EL2_SYSREG(SPSR_EL2,    SPSR_EL1,    NULL	     );
 		MAPPED_EL2_SYSREG(ZCR_EL2,     ZCR_EL1,     NULL	     );
+		MAPPED_EL2_SYSREG(CONTEXTIDR_EL2, CONTEXTIDR_EL1, NULL	     );
 	default:
 		return false;
 	}
Signature · 1 line
2.39.2
author maintainer
#6 lore raw

PATCH v3 5/24 KVM: arm64: nv: Handle CNTHCTL_EL2 specially

In reply to Marc Zyngier

Accessing CNTHCTL_EL2 is fraught with danger if running with HCR_EL2.E2H=1: half of the bits are held in CNTKCTL_EL1, and thus can be changed behind our back, while the rest lives in the CNTHCTL_EL2 shadow copy that is memory-based.

Yes, this is a lot of fun!

Make sure that we merge the two on read access, while we can write to CNTKCTL_EL1 in a more straightforward manner.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/sys_regs.c    | 28 ++++++++++++++++++++++++++++
 include/kvm/arm_arch_timer.h |  3 +++
 2 files changed, 31 insertions(+)
arch/arm64/kvm/sys_regs.c +28 -0
index 46db7988d1b50..e4ce1e1c19bef 100644
@@ -149,6 +149,21 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
 		if (!is_hyp_ctxt(vcpu))
 			goto memory_read;
 
+		/*
+		 * CNTHCTL_EL2 requires some special treatment to
+		 * account for the bits that can be set via CNTKCTL_EL1.
+		 */
+		switch (reg) {
+		case CNTHCTL_EL2:
+			if (vcpu_el2_e2h_is_set(vcpu)) {
+				val = read_sysreg_el1(SYS_CNTKCTL);
+				val &= CNTKCTL_VALID_BITS;
+				val |= __vcpu_sys_reg(vcpu, reg) & ~CNTKCTL_VALID_BITS;
+				return val;
+			}
+			break;
+		}
+
 		/*
 		 * If this register does not have an EL1 counterpart,
 		 * then read the stored EL2 version.

@@ -199,6 +214,19 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
 		 */
 		__vcpu_sys_reg(vcpu, reg) = val;
 
+		switch (reg) {
+		case CNTHCTL_EL2:
+			/*
+			 * If E2H=0, CNHTCTL_EL2 is a pure shadow register.
+			 * Otherwise, some of the bits are backed by
+			 * CNTKCTL_EL1, while the rest is kept in memory.
+			 * Yes, this is fun stuff.
+			 */
+			if (vcpu_el2_e2h_is_set(vcpu))
+				write_sysreg_el1(val, SYS_CNTKCTL);
+			return;
+		}
+
 		/* No EL1 counterpart? We're done here.? */
 		if (reg == el1r)
 			return;
include/kvm/arm_arch_timer.h +3 -0
index c819c5d16613b..fd650a8789b91 100644
@@ -147,6 +147,9 @@ u64 timer_get_cval(struct arch_timer_context *ctxt);
 void kvm_timer_cpu_up(void);
 void kvm_timer_cpu_down(void);
 
+/* CNTKCTL_EL1 valid bits as of DDI0487J.a */
+#define CNTKCTL_VALID_BITS	(BIT(17) | GENMASK_ULL(9, 0))
+
 static inline bool has_cntpoff(void)
 {
 	return (has_vhe() && cpus_have_final_cap(ARM64_HAS_ECV_CNTPOFF));
Signature · 1 line
2.39.2
author maintainer
#7 lore raw

PATCH v3 6/24 KVM: arm64: nv: Save/Restore vEL2 sysregs

In reply to Marc Zyngier

Whenever we need to restore the guest's system registers to the CPU, we now need to take care of the EL2 system registers as well. Most of them are accessed via traps only, but some have an immediate effect and also a guest running in VHE mode would expect them to be accessible via their EL1 encoding, which we do not trap.

For vEL2 we write the virtual EL2 registers with an identical format directly into their EL1 counterpart, and translate the few registers that have a different format for the same effect on the execution when running a non-VHE guest guest hypervisor.

Based on an initial patch from Andre Przywara, rewritten many times since.

Reviewed-by Alexandru Elisei <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |   5 +-
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c        |   2 +-
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c         | 137 ++++++++++++++++++++-
 3 files changed, 139 insertions(+), 5 deletions(-)
arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h +3 -2
index 4c0fdabaf8aec..dfbc0159bf0b3 100644
@@ -128,9 +128,10 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)
 	write_sysreg(ctxt_sys_reg(ctxt, TPIDRRO_EL0),	tpidrro_el0);
 }
 
-static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
+static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt,
+					      u64 mpidr)
 {
-	write_sysreg(ctxt_sys_reg(ctxt, MPIDR_EL1),	vmpidr_el2);
+	write_sysreg(mpidr,				vmpidr_el2);
 
 	if (has_vhe() ||
 	    !cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
arch/arm64/kvm/hyp/nvhe/sysreg-sr.c +1 -1
index 29305022bc048..dba101565de36 100644
@@ -28,7 +28,7 @@ void __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)
 
 void __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt)
 {
-	__sysreg_restore_el1_state(ctxt);
+	__sysreg_restore_el1_state(ctxt, ctxt_sys_reg(ctxt, MPIDR_EL1));
 	__sysreg_restore_common_state(ctxt);
 	__sysreg_restore_user_state(ctxt);
 	__sysreg_restore_el2_return_state(ctxt);
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +135 -2

166 lines not on this page · raw message

Signature · 1 line
2.39.2
author maintainer
#8 lore raw

PATCH v3 7/24 KVM: arm64: Correctly access TCR2_EL1, PIR_EL1, PIRE0_EL1 with VHE

In reply to Marc Zyngier

For code that accesses any of the guest registers for emulation purposes, it is crucial to know where the most up-to-date data is.

While this is pretty clear for nVHE (memory is the sole repository), things are a lot muddier for VHE, as depending on the SYSREGS_ON_CPU flag, registers can either be loaded on the HW or be in memory.

Even worse with NV, where the loaded state is by definition partial.

For these reasons, KVM offers the vcpu_read_sys_reg() and vcpu_write_sys_reg() primitives that always do the right thing. However, these primitive must know what register to access, and this is the role of the __vcpu_read_sys_reg_from_cpu() and __vcpu_write_sys_reg_to_cpu() helpers.

As it turns out, TCR2_EL1, PIR_EL1, PIRE0_EL1 and not described in the latter helpers, meaning that the AT code cannot use them to emulate S1PIE.

Add the three registers to the (long) list.

Fixes 86f9de9db178 ("KVM: arm64: Save/restore PIE registers")
Signed-off-by Marc Zyngier <[email protected]>
Cc Joey Gouly <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h | 6 ++++++
 1 file changed, 6 insertions(+)
arch/arm64/include/asm/kvm_host.h +6 -0
index a33f5996ca9f1..5265a1a929514 100644
@@ -1012,6 +1012,9 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
 	case TTBR0_EL1:		*val = read_sysreg_s(SYS_TTBR0_EL12);	break;
 	case TTBR1_EL1:		*val = read_sysreg_s(SYS_TTBR1_EL12);	break;
 	case TCR_EL1:		*val = read_sysreg_s(SYS_TCR_EL12);	break;
+	case TCR2_EL1:		*val = read_sysreg_s(SYS_TCR2_EL12);	break;
+	case PIR_EL1:		*val = read_sysreg_s(SYS_PIR_EL12);	break;
+	case PIRE0_EL1:		*val = read_sysreg_s(SYS_PIRE0_EL12);	break;
 	case ESR_EL1:		*val = read_sysreg_s(SYS_ESR_EL12);	break;
 	case AFSR0_EL1:		*val = read_sysreg_s(SYS_AFSR0_EL12);	break;
 	case AFSR1_EL1:		*val = read_sysreg_s(SYS_AFSR1_EL12);	break;

@@ -1058,6 +1061,9 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)
 	case TTBR0_EL1:		write_sysreg_s(val, SYS_TTBR0_EL12);	break;
 	case TTBR1_EL1:		write_sysreg_s(val, SYS_TTBR1_EL12);	break;
 	case TCR_EL1:		write_sysreg_s(val, SYS_TCR_EL12);	break;
+	case TCR2_EL1:		write_sysreg_s(val, SYS_TCR2_EL12);	break;
+	case PIR_EL1:		write_sysreg_s(val, SYS_PIR_EL12);	break;
+	case PIRE0_EL1:		write_sysreg_s(val, SYS_PIRE0_EL12);	break;
 	case ESR_EL1:		write_sysreg_s(val, SYS_ESR_EL12);	break;
 	case AFSR0_EL1:		write_sysreg_s(val, SYS_AFSR0_EL12);	break;
 	case AFSR1_EL1:		write_sysreg_s(val, SYS_AFSR1_EL12);	break;
Signature · 1 line
2.39.2
author maintainer
#9 lore raw

PATCH v3 8/24 KVM: arm64: Extend masking facility to arbitrary registers

In reply to Marc Zyngier

We currently only use the masking (RES0/RES1) facility for VNCR registers, as they are memory-based and thus easy to sanitise.

But we could apply the same thing to other registers if we:

- split the sanitisation from __VNCR_START__
- apply the sanitisation when reading from a HW register

This involves a new "marker" in the vcpu_sysreg enum, which defines the point at which the sanitisation applies (the VNCR registers being of course after this marker).

Whle we are at it, rename kvm_vcpu_sanitise_vncr_reg() to kvm_vcpu_apply_reg_masks(), which is vaguely more explicit, and harden set_sysreg_masks() against setting masks for random registers...

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h | 19 +++++++++++++------
 arch/arm64/kvm/nested.c           | 12 ++++++++----
 arch/arm64/kvm/sys_regs.c         |  3 +++
 3 files changed, 24 insertions(+), 10 deletions(-)
arch/arm64/include/asm/kvm_host.h +13 -6
index 5265a1a929514..bb65da14963d4 100644
@@ -367,7 +367,7 @@ struct kvm_arch {
 
 	u64 ctr_el0;
 
-	/* Masks for VNCR-baked sysregs */
+	/* Masks for VNCR-backed and general EL2 sysregs */
 	struct kvm_sysreg_masks	*sysreg_masks;
 
 	/*

@@ -401,6 +401,9 @@ struct kvm_vcpu_fault_info {
 	r = __VNCR_START__ + ((VNCR_ ## r) / 8),	\
 	__after_##r = __MAX__(__before_##r - 1, r)
 
+#define MARKER(m)				\
+	m, __after_##m = m - 1
+
 enum vcpu_sysreg {
 	__INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
 	MPIDR_EL1,	/* MultiProcessor Affinity Register */

@@ -481,7 +484,11 @@ enum vcpu_sysreg {
 	CNTHV_CTL_EL2,
 	CNTHV_CVAL_EL2,
 
-	__VNCR_START__,	/* Any VNCR-capable reg goes after this point */
+	/* Anything from this can be RES0/RES1 sanitised */
+	MARKER(__SANITISED_REG_START__),
+
+	/* Any VNCR-capable reg goes after this point */
+	MARKER(__VNCR_START__),
 
 	VNCR(SCTLR_EL1),/* System Control Register */
 	VNCR(ACTLR_EL1),/* Auxiliary Control Register */

@@ -537,7 +544,7 @@ struct kvm_sysreg_masks {
 	struct {
 		u64	res0;
 		u64	res1;
-	} mask[NR_SYS_REGS - __VNCR_START__];
+	} mask[NR_SYS_REGS - __SANITISED_REG_START__];
 };
 
 struct kvm_cpu_context {

@@ -977,13 +984,13 @@ static inline u64 *___ctxt_sys_reg(const struct kvm_cpu_context *ctxt, int r)
 
 #define ctxt_sys_reg(c,r)	(*__ctxt_sys_reg(c,r))
 
-u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *, enum vcpu_sysreg);
+u64 kvm_vcpu_apply_reg_masks(const struct kvm_vcpu *, enum vcpu_sysreg, u64);
 #define __vcpu_sys_reg(v,r)						\
 	(*({								\
 		const struct kvm_cpu_context *ctxt = &(v)->arch.ctxt;	\
 		u64 *__r = __ctxt_sys_reg(ctxt, (r));			\
-		if (vcpu_has_nv((v)) && (r) >= __VNCR_START__)		\
-			*__r = kvm_vcpu_sanitise_vncr_reg((v), (r));	\
+		if (vcpu_has_nv((v)) && (r) >= __SANITISED_REG_START__)	\
+			*__r = kvm_vcpu_apply_reg_masks((v), (r), *__r);\
 		__r;							\
 	}))
arch/arm64/kvm/nested.c +8 -4
index 133cc2f9530df..fd9ddd0d31400 100644
@@ -909,15 +909,15 @@ static void limit_nv_id_regs(struct kvm *kvm)
 	kvm_set_vm_id_reg(kvm, SYS_ID_AA64DFR0_EL1, val);
 }
 
-u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg sr)
+u64 kvm_vcpu_apply_reg_masks(const struct kvm_vcpu *vcpu,
+			     enum vcpu_sysreg sr, u64 v)
 {
-	u64 v = ctxt_sys_reg(&vcpu->arch.ctxt, sr);
 	struct kvm_sysreg_masks *masks;
 
 	masks = vcpu->kvm->arch.sysreg_masks;
 
 	if (masks) {
-		sr -= __VNCR_START__;
+		sr -= __SANITISED_REG_START__;
 
 		v &= ~masks->mask[sr].res0;
 		v |= masks->mask[sr].res1;

@@ -928,7 +928,11 @@ u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg sr)
 
 static void set_sysreg_masks(struct kvm *kvm, int sr, u64 res0, u64 res1)
 {
-	int i = sr - __VNCR_START__;
+	int i = sr - __SANITISED_REG_START__;
+
+	BUILD_BUG_ON(!__builtin_constant_p(sr));
+	BUILD_BUG_ON(sr < __SANITISED_REG_START__);
+	BUILD_BUG_ON(sr >= NR_SYS_REGS);
 
 	kvm->arch.sysreg_masks->mask[i].res0 = res0;
 	kvm->arch.sysreg_masks->mask[i].res1 = res1;
arch/arm64/kvm/sys_regs.c +3 -0
index e4ce1e1c19bef..8fce896685b9c 100644
@@ -181,6 +181,9 @@ u64 vcpu_read_sys_reg(const struct kvm_vcpu *vcpu, int reg)
 
 		/* Get the current version of the EL1 counterpart. */
 		WARN_ON(!__vcpu_read_sys_reg_from_cpu(el1r, &val));
+		if (reg >= __SANITISED_REG_START__)
+			val = kvm_vcpu_apply_reg_masks(vcpu, reg, val);
+
 		return val;
 	}
Signature · 1 line
2.39.2
author maintainer
#10 lore raw

PATCH v3 9/24 arm64: Define ID_AA64MMFR1_EL1.HAFDBS advertising FEAT_HAFT

In reply to Marc Zyngier

This definition is missing, and we are going to need it to sanitise TCR2_ELx.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/tools/sysreg | 1 +
 1 file changed, 1 insertion(+)
arch/arm64/tools/sysreg +1 -0
index d80859565547e..c1c0a45b896fb 100644
@@ -1688,6 +1688,7 @@ UnsignedEnum	3:0	HAFDBS
 	0b0000	NI
 	0b0001	AF
 	0b0010	DBM
+	0b0011	HAFT
 EndEnum
 EndSysreg
Signature · 1 line
2.39.2
author maintainer
#11 lore raw

PATCH v3 10/24 KVM: arm64: Add TCR2_EL2 to the sysreg arrays

In reply to Marc Zyngier

Add the TCR2_EL2 register to the per-vcpu sysreg register array, the sysreg descriptor array, and advertise it as mapped to TCR2_EL1 for NV purposes.

Access to this register is conditional based on ID_AA64MMFR3_EL1.TCRX being advertised.

Reviewed-by Joey Gouly <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |  1 +
 arch/arm64/kvm/sys_regs.c         | 14 ++++++++++++++
 2 files changed, 15 insertions(+)
arch/arm64/include/asm/kvm_host.h +1 -0
index bb65da14963d4..fac6c458afc79 100644
@@ -486,6 +486,7 @@ enum vcpu_sysreg {
 
 	/* Anything from this can be RES0/RES1 sanitised */
 	MARKER(__SANITISED_REG_START__),
+	TCR2_EL2,	/* Extended Translation Control Register (EL2) */
 
 	/* Any VNCR-capable reg goes after this point */
 	MARKER(__VNCR_START__),
arch/arm64/kvm/sys_regs.c +14 -0
index 8fce896685b9c..886ff9557d14c 100644
@@ -126,6 +126,7 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
 		MAPPED_EL2_SYSREG(ESR_EL2,     ESR_EL1,     NULL	     );
 		MAPPED_EL2_SYSREG(FAR_EL2,     FAR_EL1,     NULL	     );
 		MAPPED_EL2_SYSREG(MAIR_EL2,    MAIR_EL1,    NULL	     );
+		MAPPED_EL2_SYSREG(TCR2_EL2,    TCR2_EL1,    NULL	     );
 		MAPPED_EL2_SYSREG(AMAIR_EL2,   AMAIR_EL1,   NULL	     );
 		MAPPED_EL2_SYSREG(ELR_EL2,     ELR_EL1,	    NULL	     );
 		MAPPED_EL2_SYSREG(SPSR_EL2,    SPSR_EL1,    NULL	     );

@@ -448,6 +449,18 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+static bool access_tcr2_el2(struct kvm_vcpu *vcpu,
+			    struct sys_reg_params *p,
+			    const struct sys_reg_desc *r)
+{
+	if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, TCRX, IMP)) {
+		kvm_inject_undefined(vcpu);
+		return false;
+	}
+
+	return access_rw(vcpu, p, r);
+}
+
 static bool access_actlr(struct kvm_vcpu *vcpu,
 			 struct sys_reg_params *p,
 			 const struct sys_reg_desc *r)

@@ -2795,6 +2808,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(TTBR0_EL2, access_rw, reset_val, 0),
 	EL2_REG(TTBR1_EL2, access_rw, reset_val, 0),
 	EL2_REG(TCR_EL2, access_rw, reset_val, TCR_EL2_RES1),
+	EL2_REG(TCR2_EL2, access_tcr2_el2, reset_val, TCR2_EL2_RES1),
 	EL2_REG_VNCR(VTTBR_EL2, reset_val, 0),
 	EL2_REG_VNCR(VTCR_EL2, reset_val, 0),
Signature · 1 line
2.39.2
author maintainer
#12 lore raw

PATCH v3 11/24 KVM: arm64: Sanitise TCR2_EL2

In reply to Marc Zyngier

TCR2_EL2 is a bag of control bits, all of which are only valid if certain features are present, and RES0 otherwise.

Describe these constraints and register them with the masking infrastructure.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/nested.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)
arch/arm64/kvm/nested.c +22 -0
index fd9ddd0d31400..a566efd6ca4f2 100644
@@ -1183,6 +1183,28 @@ int kvm_init_nv_sysregs(struct kvm *kvm)
 		res0 |= ~(res0 | res1);
 	set_sysreg_masks(kvm, HAFGRTR_EL2, res0, res1);
 
+	/* TCR2_EL2 */
+	res0 = TCR2_EL2_RES0;
+	res1 = TCR2_EL2_RES1;
+	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, D128, IMP))
+		res0 |= (TCR2_EL2_DisCH0 | TCR2_EL2_DisCH1 | TCR2_EL2_D128);
+	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, MEC, IMP))
+		res0 |= TCR2_EL2_AMEC1 | TCR2_EL2_AMEC0;
+	if (!kvm_has_feat(kvm, ID_AA64MMFR1_EL1, HAFDBS, HAFT))
+		res0 |= TCR2_EL2_HAFT;
+	if (!kvm_has_feat(kvm, ID_AA64PFR1_EL1, THE, IMP))
+		res0 |= TCR2_EL2_PTTWI | TCR2_EL2_PnCH;
+	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, AIE, IMP))
+		res0 |= TCR2_EL2_AIE;
+	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, S1POE, IMP))
+		res0 |= TCR2_EL2_POE | TCR2_EL2_E0POE;
+	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, S1PIE, IMP))
+		res0 |= TCR2_EL2_PIE;
+	if (!kvm_has_feat(kvm, ID_AA64MMFR1_EL1, VH, IMP))
+		res0 |= (TCR2_EL2_E0POE | TCR2_EL2_D128 |
+			 TCR2_EL2_AMEC1 | TCR2_EL2_DisCH0 | TCR2_EL2_DisCH1);
+	set_sysreg_masks(kvm, TCR2_EL2, res0, res1);
+
 	/* SCTLR_EL1 */
 	res0 = SCTLR_EL1_RES0;
 	res1 = SCTLR_EL1_RES1;
Signature · 1 line
2.39.2
author maintainer
#13 lore raw

PATCH v3 12/24 KVM: arm64: Add save/restore for TCR2_EL2

In reply to Marc Zyngier

Like its EL1 equivalent, TCR2_EL2 gets context-switched. This is made conditional on FEAT_TCRX being adversised.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 7 +++++++
 1 file changed, 7 insertions(+)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +7 -0
index e0df14ead2657..5f69a1f713cfe 100644
@@ -51,6 +51,9 @@ static void __sysreg_save_vel2_state(struct kvm_vcpu *vcpu)
 		__vcpu_sys_reg(vcpu, TTBR1_EL2)	= read_sysreg_el1(SYS_TTBR1);
 		__vcpu_sys_reg(vcpu, TCR_EL2)	= read_sysreg_el1(SYS_TCR);
 
+		if (ctxt_has_tcrx(&vcpu->arch.ctxt))
+			__vcpu_sys_reg(vcpu, TCR2_EL2) = read_sysreg_el1(SYS_TCR2);
+
 		/*
 		 * The EL1 view of CNTKCTL_EL1 has a bunch of RES0 bits where
 		 * the interesting CNTHCTL_EL2 bits live. So preserve these

@@ -108,6 +111,10 @@ static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)
 		write_sysreg_el1(val, SYS_TCR);
 	}
 
+	if (ctxt_has_tcrx(&vcpu->arch.ctxt))
+		write_sysreg_el1(__vcpu_sys_reg(vcpu, TCR2_EL2), SYS_TCR2);
+
+
 	write_sysreg_el1(__vcpu_sys_reg(vcpu, ESR_EL2),		SYS_ESR);
 	write_sysreg_el1(__vcpu_sys_reg(vcpu, AFSR0_EL2),	SYS_AFSR0);
 	write_sysreg_el1(__vcpu_sys_reg(vcpu, AFSR1_EL2),	SYS_AFSR1);
Signature · 1 line
2.39.2
author maintainer
#14 lore raw

PATCH v3 13/24 KVM: arm64: Add PIR{,E0}_EL2 to the sysreg arrays

In reply to Marc Zyngier

Add the FEAT_S1PIE EL2 registers to the per-vcpu sysreg register array.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h | 2 ++
 arch/arm64/kvm/sys_regs.c         | 2 ++
 2 files changed, 4 insertions(+)
arch/arm64/include/asm/kvm_host.h +2 -0
index fac6c458afc79..34318842bd2da 100644
@@ -465,6 +465,8 @@ enum vcpu_sysreg {
 	TTBR0_EL2,	/* Translation Table Base Register 0 (EL2) */
 	TTBR1_EL2,	/* Translation Table Base Register 1 (EL2) */
 	TCR_EL2,	/* Translation Control Register (EL2) */
+	PIRE0_EL2,	/* Permission Indirection Register 0 (EL2) */
+	PIR_EL2,	/* Permission Indirection Register 1 (EL2) */
 	SPSR_EL2,	/* EL2 saved program status register */
 	ELR_EL2,	/* EL2 exception link register */
 	AFSR0_EL2,	/* Auxiliary Fault Status Register 0 (EL2) */
arch/arm64/kvm/sys_regs.c +2 -0
index 886ff9557d14c..c109f9c8c5c64 100644
@@ -127,6 +127,8 @@ static bool get_el2_to_el1_mapping(unsigned int reg,
 		MAPPED_EL2_SYSREG(FAR_EL2,     FAR_EL1,     NULL	     );
 		MAPPED_EL2_SYSREG(MAIR_EL2,    MAIR_EL1,    NULL	     );
 		MAPPED_EL2_SYSREG(TCR2_EL2,    TCR2_EL1,    NULL	     );
+		MAPPED_EL2_SYSREG(PIR_EL2,     PIR_EL1,     NULL	     );
+		MAPPED_EL2_SYSREG(PIRE0_EL2,   PIRE0_EL1,   NULL	     );
 		MAPPED_EL2_SYSREG(AMAIR_EL2,   AMAIR_EL1,   NULL	     );
 		MAPPED_EL2_SYSREG(ELR_EL2,     ELR_EL1,	    NULL	     );
 		MAPPED_EL2_SYSREG(SPSR_EL2,    SPSR_EL1,    NULL	     );
Signature · 1 line
2.39.2
author maintainer
#15 lore raw

PATCH v3 14/24 KVM: arm64: Add save/restore for PIR{,E0}_EL2

In reply to Marc Zyngier

Like their EL1 equivalent, the EL2-specific FEAT_S1PIE registers are context-switched. This is made conditional on both FEAT_TCRX and FEAT_S1PIE being adversised.

Note that this change only makes sense if read together with the issue D22677 contained in 102105_K.a_04_en.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)
arch/arm64/kvm/hyp/vhe/sysreg-sr.c +13 -2
index 5f69a1f713cfe..005175c10b4a9 100644
@@ -51,9 +51,15 @@ static void __sysreg_save_vel2_state(struct kvm_vcpu *vcpu)
 		__vcpu_sys_reg(vcpu, TTBR1_EL2)	= read_sysreg_el1(SYS_TTBR1);
 		__vcpu_sys_reg(vcpu, TCR_EL2)	= read_sysreg_el1(SYS_TCR);
 
-		if (ctxt_has_tcrx(&vcpu->arch.ctxt))
+		if (ctxt_has_tcrx(&vcpu->arch.ctxt)) {
 			__vcpu_sys_reg(vcpu, TCR2_EL2) = read_sysreg_el1(SYS_TCR2);
 
+			if (ctxt_has_s1pie(&vcpu->arch.ctxt)) {
+				__vcpu_sys_reg(vcpu, PIRE0_EL2) = read_sysreg_el1(SYS_PIRE0);
+				__vcpu_sys_reg(vcpu, PIR_EL2) = read_sysreg_el1(SYS_PIR);
+			}
+		}
+
 		/*
 		 * The EL1 view of CNTKCTL_EL1 has a bunch of RES0 bits where
 		 * the interesting CNTHCTL_EL2 bits live. So preserve these

@@ -111,9 +117,14 @@ static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)
 		write_sysreg_el1(val, SYS_TCR);
 	}
 
-	if (ctxt_has_tcrx(&vcpu->arch.ctxt))
+	if (ctxt_has_tcrx(&vcpu->arch.ctxt)) {
 		write_sysreg_el1(__vcpu_sys_reg(vcpu, TCR2_EL2), SYS_TCR2);
 
+		if (ctxt_has_s1pie(&vcpu->arch.ctxt)) {
+			write_sysreg_el1(__vcpu_sys_reg(vcpu, PIR_EL2), SYS_PIR);
+			write_sysreg_el1(__vcpu_sys_reg(vcpu, PIRE0_EL2), SYS_PIRE0);
+		}
+	}
 
 	write_sysreg_el1(__vcpu_sys_reg(vcpu, ESR_EL2),		SYS_ESR);
 	write_sysreg_el1(__vcpu_sys_reg(vcpu, AFSR0_EL2),	SYS_AFSR0);
Signature · 1 line
2.39.2
author maintainer
#16 lore raw

PATCH v3 15/24 KVM: arm64: Handle PIR{,E0}_EL2 traps

In reply to Marc Zyngier

Add the FEAT_S1PIE EL2 registers the sysreg descriptor array so that they can be handled as a trap.

Access to these registers is conditional based on ID_AA64MMFR3_EL1.S1PIE being advertised.

Similarly to other other changes, PIRE0_EL2 is guaranteed to trap thanks to the D22677 update to the architecture.

Reviewed-by Joey Gouly <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/sys_regs.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)
arch/arm64/kvm/sys_regs.c +14 -0
index c109f9c8c5c64..81c7099a7a825 100644
@@ -361,6 +361,18 @@ static bool access_rw(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+static bool check_s1pie_access_rw(struct kvm_vcpu *vcpu,
+				  struct sys_reg_params *p,
+				  const struct sys_reg_desc *r)
+{
+	if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) {
+		kvm_inject_undefined(vcpu);
+		return false;
+	}
+
+	return access_rw(vcpu, p, r);
+}
+
 /*
  * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized).
  */

@@ -2842,6 +2854,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(HPFAR_EL2, access_rw, reset_val, 0),
 
 	EL2_REG(MAIR_EL2, access_rw, reset_val, 0),
+	EL2_REG(PIRE0_EL2, check_s1pie_access_rw, reset_val, 0),
+	EL2_REG(PIR_EL2, check_s1pie_access_rw, reset_val, 0),
 	EL2_REG(AMAIR_EL2, access_rw, reset_val, 0),
 
 	EL2_REG(VBAR_EL2, access_rw, reset_val, 0),
Signature · 1 line
2.39.2
author maintainer
#17 lore raw

PATCH v3 16/24 KVM: arm64: Sanitise ID_AA64MMFR3_EL1

In reply to Marc Zyngier

Add the missing sanitisation of ID_AA64MMFR3_EL1, making sure we solely expose S1PIE and TCRX (we currently don't support anything else).

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/sys_regs.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
arch/arm64/kvm/sys_regs.c +5 -1
index 81c7099a7a825..ffffcbaf80da4 100644
@@ -1623,6 +1623,9 @@ static u64 __kvm_read_sanitised_id_reg(const struct kvm_vcpu *vcpu,
 	case SYS_ID_AA64MMFR2_EL1:
 		val &= ~ID_AA64MMFR2_EL1_CCIDX_MASK;
 		break;
+	case SYS_ID_AA64MMFR3_EL1:
+		val &= ID_AA64MMFR3_EL1_TCRX | ID_AA64MMFR3_EL1_S1PIE;
+		break;
 	case SYS_ID_MMFR4_EL1:
 		val &= ~ARM64_FEATURE_MASK(ID_MMFR4_EL1_CCIDX);
 		break;

@@ -2485,7 +2488,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 					ID_AA64MMFR2_EL1_IDS |
 					ID_AA64MMFR2_EL1_NV |
 					ID_AA64MMFR2_EL1_CCIDX)),
-	ID_SANITISED(ID_AA64MMFR3_EL1),
+	ID_WRITABLE(ID_AA64MMFR3_EL1, (ID_AA64MMFR3_EL1_TCRX	|
+				       ID_AA64MMFR3_EL1_S1PIE)),
 	ID_SANITISED(ID_AA64MMFR4_EL1),
 	ID_UNALLOCATED(7,5),
 	ID_UNALLOCATED(7,6),
Signature · 1 line
2.39.2
author maintainer
#18 lore raw

PATCH v3 17/24 KVM: arm64: Add AT fast-path support for S1PIE

In reply to Marc Zyngier

Emulating AT using AT instructions requires that the live state matches the translation regime the AT instruction targets.

If targeting the EL1&0 translation regime and that S1PIE is supported, we also need to restore that state (covering TCR2_EL1, PIR_EL1, and PIRE0_EL1).

Add the required system register switcheroo.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/at.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)
arch/arm64/kvm/at.c +24 -0
index 39f0e87a340e8..73b2ee662f371 100644
@@ -412,6 +412,9 @@ struct mmu_config {
 	u64	ttbr1;
 	u64	tcr;
 	u64	mair;
+	u64	tcr2;
+	u64	pir;
+	u64	pire0;
 	u64	sctlr;
 	u64	vttbr;
 	u64	vtcr;

@@ -424,6 +427,13 @@ static void __mmu_config_save(struct mmu_config *config)
 	config->ttbr1	= read_sysreg_el1(SYS_TTBR1);
 	config->tcr	= read_sysreg_el1(SYS_TCR);
 	config->mair	= read_sysreg_el1(SYS_MAIR);
+	if (cpus_have_final_cap(ARM64_HAS_TCR2)) {
+		config->tcr2	= read_sysreg_el1(SYS_TCR2);
+		if (cpus_have_final_cap(ARM64_HAS_S1PIE)) {
+			config->pir	= read_sysreg_el1(SYS_PIR);
+			config->pire0	= read_sysreg_el1(SYS_PIRE0);
+		}
+	}
 	config->sctlr	= read_sysreg_el1(SYS_SCTLR);
 	config->vttbr	= read_sysreg(vttbr_el2);
 	config->vtcr	= read_sysreg(vtcr_el2);

@@ -444,6 +454,13 @@ static void __mmu_config_restore(struct mmu_config *config)
 	write_sysreg_el1(config->ttbr1,	SYS_TTBR1);
 	write_sysreg_el1(config->tcr,	SYS_TCR);
 	write_sysreg_el1(config->mair,	SYS_MAIR);
+	if (cpus_have_final_cap(ARM64_HAS_TCR2)) {
+		write_sysreg_el1(config->tcr2, SYS_TCR2);
+		if (cpus_have_final_cap(ARM64_HAS_S1PIE)) {
+			write_sysreg_el1(config->pir, SYS_PIR);
+			write_sysreg_el1(config->pire0, SYS_PIRE0);
+		}
+	}
 	write_sysreg_el1(config->sctlr,	SYS_SCTLR);
 	write_sysreg(config->vttbr,	vttbr_el2);
 	write_sysreg(config->vtcr,	vtcr_el2);

@@ -914,6 +931,13 @@ static u64 __kvm_at_s1e01_fast(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
 	write_sysreg_el1(vcpu_read_sys_reg(vcpu, TTBR1_EL1),	SYS_TTBR1);
 	write_sysreg_el1(vcpu_read_sys_reg(vcpu, TCR_EL1),	SYS_TCR);
 	write_sysreg_el1(vcpu_read_sys_reg(vcpu, MAIR_EL1),	SYS_MAIR);
+	if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, TCRX, IMP)) {
+		write_sysreg_el1(vcpu_read_sys_reg(vcpu, TCR2_EL1), SYS_TCR2);
+		if (kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) {
+			write_sysreg_el1(vcpu_read_sys_reg(vcpu, PIR_EL1), SYS_PIR);
+			write_sysreg_el1(vcpu_read_sys_reg(vcpu, PIRE0_EL1), SYS_PIRE0);
+		}
+	}
 	write_sysreg_el1(vcpu_read_sys_reg(vcpu, SCTLR_EL1),	SYS_SCTLR);
 	__load_stage2(mmu, mmu->arch);
Signature · 1 line
2.39.2
author maintainer
#19 lore raw

PATCH v3 18/24 KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts

In reply to Marc Zyngier

The AArch64.S1DirectBasePermissions() pseudocode deals with both direct and hierarchical S1 permission evaluation. While this is probably convenient in the pseudocode, we would like a bit more flexibility to slot things like indirect permissions.

To that effect, split the two permission check parts out of handle_at_slow() and into their own functions. The permissions are passed around in a new s1_perms type that contains the individual permissions across the flow.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/at.c | 164 ++++++++++++++++++++++++++------------------
 1 file changed, 99 insertions(+), 65 deletions(-)
arch/arm64/kvm/at.c +99 -65
index 73b2ee662f371..d6ee3a5c30bc2 100644
@@ -47,6 +47,10 @@ struct s1_walk_result {
 	bool	failed;
 };
 
+struct s1_perms {
+	bool	ur, uw, ux, pr, pw, px;
+};
+
 static void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool ptw, bool s2)
 {
 	wr->fst		= fst;

@@ -764,111 +768,141 @@ static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)
 	return sctlr & SCTLR_EL1_EPAN;
 }
 
-static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
+static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu,
+					  struct s1_walk_info *wi,
+					  struct s1_walk_result *wr,
+					  struct s1_perms *s1p)
 {
-	bool perm_fail, ur, uw, ux, pr, pw, px;
-	struct s1_walk_result wr = {};
-	struct s1_walk_info wi = {};
-	int ret, idx;
-
-	ret = setup_s1_walk(vcpu, op, &wi, &wr, vaddr);
-	if (ret)
-		goto compute_par;
-
-	if (wr.level == S1_MMU_DISABLED)
-		goto compute_par;
-
-	idx = srcu_read_lock(&vcpu->kvm->srcu);
-
-	ret = walk_s1(vcpu, &wi, &wr, vaddr);
-
-	srcu_read_unlock(&vcpu->kvm->srcu, idx);
-
-	if (ret)
-		goto compute_par;
-
-	/* FIXME: revisit when adding indirect permission support */
-	/* AArch64.S1DirectBasePermissions() */
-	if (wi.regime != TR_EL2) {
-		switch (FIELD_GET(PTE_USER | PTE_RDONLY, wr.desc)) {
+	/* Non-hierarchical part of AArch64.S1DirectBasePermissions() */
+	if (wi->regime != TR_EL2) {
+		switch (FIELD_GET(PTE_USER | PTE_RDONLY, wr->desc)) {
 		case 0b00:
-			pr = pw = true;
-			ur = uw = false;
+			s1p->pr = s1p->pw = true;
+			s1p->ur = s1p->uw = false;
 			break;
 		case 0b01:
-			pr = pw = ur = uw = true;
+			s1p->pr = s1p->pw = s1p->ur = s1p->uw = true;
 			break;
 		case 0b10:
-			pr = true;
-			pw = ur = uw = false;
+			s1p->pr = true;
+			s1p->pw = s1p->ur = s1p->uw = false;
 			break;
 		case 0b11:
-			pr = ur = true;
-			pw = uw = false;
+			s1p->pr = s1p->ur = true;
+			s1p->pw = s1p->uw = false;
 			break;
 		}
 
-		switch (wr.APTable) {
+		/* We don't use px for anything yet, but hey... */
+		s1p->px = !((wr->desc & PTE_PXN) || s1p->uw);
+		s1p->ux = !(wr->desc & PTE_UXN);
+	} else {
+		s1p->ur = s1p->uw = s1p->ux = false;
+
+		if (!(wr->desc & PTE_RDONLY)) {
+			s1p->pr = s1p->pw = true;
+		} else {
+			s1p->pr = true;
+			s1p->pw = false;
+		}
+
+		/* XN maps to UXN */
+		s1p->px = !(wr->desc & PTE_UXN);
+	}
+}
+
+static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
+						struct s1_walk_info *wi,
+						struct s1_walk_result *wr,
+						struct s1_perms *s1p)
+{
+	/* Hierarchical part of AArch64.S1DirectBasePermissions() */
+	if (wi->regime != TR_EL2) {
+		switch (wr->APTable) {
 		case 0b00:
 			break;
 		case 0b01:
-			ur = uw = false;
+			s1p->ur = s1p->uw = false;
 			break;
 		case 0b10:
-			pw = uw = false;
+			s1p->pw = s1p->uw = false;
 			break;
 		case 0b11:
-			pw = ur = uw = false;
+			s1p->pw = s1p->ur = s1p->uw = false;
 			break;
 		}
 
-		/* We don't use px for anything yet, but hey... */
-		px = !((wr.desc & PTE_PXN) || wr.PXNTable || uw);
-		ux = !((wr.desc & PTE_UXN) || wr.UXNTable);
-
-		if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
-			bool pan;
-
-			pan = *vcpu_cpsr(vcpu) & PSR_PAN_BIT;
-			pan &= ur || uw || (pan3_enabled(vcpu, wi.regime) && ux);
-			pw &= !pan;
-			pr &= !pan;
-		}
+		s1p->px &= !wr->PXNTable;
+		s1p->ux &= !wr->UXNTable;
 	} else {
-		ur = uw = ux = false;
+		if (wr->APTable & BIT(1))
+			s1p->pw = false;
 
-		if (!(wr.desc & PTE_RDONLY)) {
-			pr = pw = true;
-		} else {
-			pr = true;
-			pw = false;
-		}
+		/* XN maps to UXN */
+		s1p->px &= !wr->UXNTable;
+	}
+}
 
-		if (wr.APTable & BIT(1))
-			pw = false;
+static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,
+				   struct s1_walk_info *wi,
+				   struct s1_walk_result *wr,
+				   struct s1_perms *s1p)
+{
+	compute_s1_direct_permissions(vcpu, wi, wr, s1p);
+	compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
 
-		/* XN maps to UXN */
-		px = !((wr.desc & PTE_UXN) || wr.UXNTable);
+	if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
+		bool pan;
+
+		pan = *vcpu_cpsr(vcpu) & PSR_PAN_BIT;
+		pan &= s1p->ur || s1p->uw || (pan3_enabled(vcpu, wi->regime) && s1p->ux);
+		s1p->pw &= !pan;
+		s1p->pr &= !pan;
 	}
+}
+
+static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
+{
+	struct s1_walk_result wr = {};
+	struct s1_walk_info wi = {};
+	struct s1_perms s1p = {};
+	bool perm_fail = false;
+	int ret, idx;
+
+	ret = setup_s1_walk(vcpu, op, &wi, &wr, vaddr);
+	if (ret)
+		goto compute_par;
+
+	if (wr.level == S1_MMU_DISABLED)
+		goto compute_par;
+
+	idx = srcu_read_lock(&vcpu->kvm->srcu);
+
+	ret = walk_s1(vcpu, &wi, &wr, vaddr);
+
+	srcu_read_unlock(&vcpu->kvm->srcu, idx);
+
+	if (ret)
+		goto compute_par;
 
-	perm_fail = false;
+	compute_s1_permissions(vcpu, op, &wi, &wr, &s1p);
 
 	switch (op) {
 	case OP_AT_S1E1RP:
 	case OP_AT_S1E1R:
 	case OP_AT_S1E2R:
-		perm_fail = !pr;
+		perm_fail = !s1p.pr;
 		break;
 	case OP_AT_S1E1WP:
 	case OP_AT_S1E1W:
 	case OP_AT_S1E2W:
-		perm_fail = !pw;
+		perm_fail = !s1p.pw;
 		break;
 	case OP_AT_S1E0R:
-		perm_fail = !ur;
+		perm_fail = !s1p.ur;
 		break;
 	case OP_AT_S1E0W:
-		perm_fail = !uw;
+		perm_fail = !s1p.uw;
 		break;
 	case OP_AT_S1E1A:
 	case OP_AT_S1E2A:
Signature · 1 line
2.39.2
reviewer
#20 lore raw

Re: [PATCH v3 18/24] KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts

Quoted 106 lines · Marc Zyngier

The AArch64.S1DirectBasePermissions() pseudocode deals with both direct and hierarchical S1 permission evaluation. While this is probably convenient in the pseudocode, we would like a bit more flexibility to slot things like indirect permissions.

To that effect, split the two permission check parts out of handle_at_slow() and into their own functions. The permissions are passed around in a new s1_perms type that contains the individual permissions across the flow.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/at.c | 164 ++++++++++++++++++++++++++------------------
 1 file changed, 99 insertions(+), 65 deletions(-)
arch/arm64/kvm/at.c +31 -34
index 73b2ee662f371..d6ee3a5c30bc2 100644
@@ -47,6 +47,10 @@ struct s1_walk_result {
 	bool	failed;
 };
 
+struct s1_perms {
+	bool	ur, uw, ux, pr, pw, px;
+};
+
 static void fail_s1_walk(struct s1_walk_result *wr, u8 fst, bool ptw, bool s2)
 {
 	wr->fst		= fst;

@@ -764,111 +768,141 @@ static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)
 	return sctlr & SCTLR_EL1_EPAN;
 }
 
-static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
+static void compute_s1_direct_permissions(struct kvm_vcpu *vcpu,
+					  struct s1_walk_info *wi,
+					  struct s1_walk_result *wr,
+					  struct s1_perms *s1p)
 {
-	bool perm_fail, ur, uw, ux, pr, pw, px;
-	struct s1_walk_result wr = {};
-	struct s1_walk_info wi = {};
-	int ret, idx;
-
-	ret = setup_s1_walk(vcpu, op, &wi, &wr, vaddr);
-	if (ret)
-		goto compute_par;
-
-	if (wr.level == S1_MMU_DISABLED)
-		goto compute_par;
-
-	idx = srcu_read_lock(&vcpu->kvm->srcu);
-
-	ret = walk_s1(vcpu, &wi, &wr, vaddr);
-
-	srcu_read_unlock(&vcpu->kvm->srcu, idx);
-
-	if (ret)
-		goto compute_par;
-
-	/* FIXME: revisit when adding indirect permission support */
-	/* AArch64.S1DirectBasePermissions() */
-	if (wi.regime != TR_EL2) {
-		switch (FIELD_GET(PTE_USER | PTE_RDONLY, wr.desc)) {
+	/* Non-hierarchical part of AArch64.S1DirectBasePermissions() */
+	if (wi->regime != TR_EL2) {
+		switch (FIELD_GET(PTE_USER | PTE_RDONLY, wr->desc)) {
 		case 0b00:
-			pr = pw = true;
-			ur = uw = false;
+			s1p->pr = s1p->pw = true;
+			s1p->ur = s1p->uw = false;
 			break;
 		case 0b01:
-			pr = pw = ur = uw = true;
+			s1p->pr = s1p->pw = s1p->ur = s1p->uw = true;
 			break;
 		case 0b10:
-			pr = true;
-			pw = ur = uw = false;
+			s1p->pr = true;
+			s1p->pw = s1p->ur = s1p->uw = false;
 			break;
 		case 0b11:
-			pr = ur = true;
-			pw = uw = false;
+			s1p->pr = s1p->ur = true;
+			s1p->pw = s1p->uw = false;
 			break;
 		}
 
-		switch (wr.APTable) {
+		/* We don't use px for anything yet, but hey... */
+		s1p->px = !((wr->desc & PTE_PXN) || s1p->uw);
+		s1p->ux = !(wr->desc & PTE_UXN);
+	} else {
+		s1p->ur = s1p->uw = s1p->ux = false;
+
+		if (!(wr->desc & PTE_RDONLY)) {
+			s1p->pr = s1p->pw = true;
+		} else {
+			s1p->pr = true;
+			s1p->pw = false;
+		}
+

arch/arm64/kvm/at.c static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 818

Quoted 10 lines
arch/arm64/kvm/at.c +10 -0
+		/* XN maps to UXN */
+		s1p->px = !(wr->desc & PTE_UXN);
+	}
+}
+
+static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
+						struct s1_walk_info *wi,
+						struct s1_walk_result *wr,
+						struct s1_perms *s1p)
+{

How about:

	if (wi->hpd)
		return;
Quoted 124 lines
arch/arm64/kvm/at.c +58 -31
+	/* Hierarchical part of AArch64.S1DirectBasePermissions() */
+	if (wi->regime != TR_EL2) {
+		switch (wr->APTable) {
 		case 0b00:
 			break;
 		case 0b01:
-			ur = uw = false;
+			s1p->ur = s1p->uw = false;
 			break;
 		case 0b10:
-			pw = uw = false;
+			s1p->pw = s1p->uw = false;
 			break;
 		case 0b11:
-			pw = ur = uw = false;
+			s1p->pw = s1p->ur = s1p->uw = false;
 			break;
 		}
 
-		/* We don't use px for anything yet, but hey... */
-		px = !((wr.desc & PTE_PXN) || wr.PXNTable || uw);
-		ux = !((wr.desc & PTE_UXN) || wr.UXNTable);
-
-		if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
-			bool pan;
-
-			pan = *vcpu_cpsr(vcpu) & PSR_PAN_BIT;
-			pan &= ur || uw || (pan3_enabled(vcpu, wi.regime) && ux);
-			pw &= !pan;
-			pr &= !pan;
-		}
+		s1p->px &= !wr->PXNTable;
+		s1p->ux &= !wr->UXNTable;
 	} else {
-		ur = uw = ux = false;
+		if (wr->APTable & BIT(1))
+			s1p->pw = false;
 
-		if (!(wr.desc & PTE_RDONLY)) {
-			pr = pw = true;
-		} else {
-			pr = true;
-			pw = false;
-		}
+		/* XN maps to UXN */
+		s1p->px &= !wr->UXNTable;
+	}
+}
 
-		if (wr.APTable & BIT(1))
-			pw = false;
+static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,
+				   struct s1_walk_info *wi,
+				   struct s1_walk_result *wr,
+				   struct s1_perms *s1p)
+{
+	compute_s1_direct_permissions(vcpu, wi, wr, s1p);
+	compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
 
-		/* XN maps to UXN */
-		px = !((wr.desc & PTE_UXN) || wr.UXNTable);
+	if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
+		bool pan;
+
+		pan = *vcpu_cpsr(vcpu) & PSR_PAN_BIT;
+		pan &= s1p->ur || s1p->uw || (pan3_enabled(vcpu, wi->regime) && s1p->ux);
+		s1p->pw &= !pan;
+		s1p->pr &= !pan;
 	}
+}
+
+static u64 handle_at_slow(struct kvm_vcpu *vcpu, u32 op, u64 vaddr)
+{
+	struct s1_walk_result wr = {};
+	struct s1_walk_info wi = {};
+	struct s1_perms s1p = {};
+	bool perm_fail = false;
+	int ret, idx;
+
+	ret = setup_s1_walk(vcpu, op, &wi, &wr, vaddr);
+	if (ret)
+		goto compute_par;
+
+	if (wr.level == S1_MMU_DISABLED)
+		goto compute_par;
+
+	idx = srcu_read_lock(&vcpu->kvm->srcu);
+
+	ret = walk_s1(vcpu, &wi, &wr, vaddr);
+
+	srcu_read_unlock(&vcpu->kvm->srcu, idx);
+
+	if (ret)
+		goto compute_par;
 
-	perm_fail = false;
+	compute_s1_permissions(vcpu, op, &wi, &wr, &s1p);
 
 	switch (op) {
 	case OP_AT_S1E1RP:
 	case OP_AT_S1E1R:
 	case OP_AT_S1E2R:
-		perm_fail = !pr;
+		perm_fail = !s1p.pr;
 		break;
 	case OP_AT_S1E1WP:
 	case OP_AT_S1E1W:
 	case OP_AT_S1E2W:
-		perm_fail = !pw;
+		perm_fail = !s1p.pw;
 		break;
 	case OP_AT_S1E0R:
-		perm_fail = !ur;
+		perm_fail = !s1p.ur;
 		break;
 	case OP_AT_S1E0W:
-		perm_fail = !uw;
+		perm_fail = !s1p.uw;
 		break;
 	case OP_AT_S1E1A:
 	case OP_AT_S1E2A:
Signature · 1 line
2.39.2
author maintainer
#21 lore raw

Re: [PATCH v3 18/24] KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts

Quoted 2 lines · Joey Gouly

On Wed, Sep 11, 2024 at 02:51:45PM +0100, Marc Zyngier wrote:

[...]

Quoted 10 lines

arch/arm64/kvm/at.c static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 818

Quoted 5 lines
arch/arm64/kvm/at.c +5 -0
+static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
+						struct s1_walk_info *wi,
+						struct s1_walk_result *wr,
+						struct s1_perms *s1p)
+{

How about:

	if (wi->hpd)
		return;

I was hoping not to add anything like this, because all the table bits are 0 (we simply don't collect them), and thus don't have any effect.

Or did you spot any edge case where that would result in in a different set of permissions?

Thanks,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
reviewer
#22 lore raw

Re: [PATCH v3 18/24] KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts

Quoted 15 lines · Marc Zyngier
Quoted 2 lines · Joey Gouly

On Wed, Sep 11, 2024 at 02:51:45PM +0100, Marc Zyngier wrote:

[...]

Quoted 8 lines

arch/arm64/kvm/at.c static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime) · line 818

Quoted 5 lines
arch/arm64/kvm/at.c +5 -0
+static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
+						struct s1_walk_info *wi,
+						struct s1_walk_result *wr,
+						struct s1_perms *s1p)
+{

How about:

Quoted 5 lines
Quoted 2 lines
	if (wi->hpd)
		return;

I was hoping not to add anything like this, because all the table bits are 0 (we simply don't collect them), and thus don't have any effect.

I just thought it was more obvious that they wouldn't apply in this case, don't feel super strongly about it.

Quoted 8 lines

Or did you spot any edge case where that would result in in a different set of permissions?

Thanks,

	M.

Thanks,
Joey

author maintainer
#23 lore raw

Re: [PATCH v3 18/24] KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts

Quoted 17 lines · Joey Gouly
Quoted 15 lines · Marc Zyngier
Quoted 2 lines · Joey Gouly

On Wed, Sep 11, 2024 at 02:51:45PM +0100, Marc Zyngier wrote:

[...]

Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
+						struct s1_walk_info *wi,
+						struct s1_walk_result *wr,
+						struct s1_perms *s1p)
+{

How about:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
	if (wi->hpd)
		return;

I was hoping not to add anything like this, because all the table bits are 0 (we simply don't collect them), and thus don't have any effect.

I just thought it was more obvious that they wouldn't apply in this case, don't feel super strongly about it.

If you want it to be obvious, how about this instead?

arch/arm64/kvm/at.c +2 -1
index 8a5e1c4682619..fb9de5fc2cc26 100644
@@ -985,7 +985,8 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,
 	else
 		compute_s1_indirect_permissions(vcpu, wi, wr, s1p);
 
-	compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
+	if (!wi->hpd)
+		compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
 
 	if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
 		bool pan;

Thanks,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
reviewer
#24 lore raw

Re: [PATCH v3 18/24] KVM: arm64: Split S1 permission evaluation into direct and hierarchical parts

Quoted 36 lines · Marc Zyngier
Quoted 17 lines · Joey Gouly
Quoted 15 lines · Marc Zyngier
Quoted 2 lines · Joey Gouly

On Wed, Sep 11, 2024 at 02:51:45PM +0100, Marc Zyngier wrote:

[...]

Quoted 8 lines

Review

Quoted 5 lines
(quoted diff) +5 -0
+static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
+						struct s1_walk_info *wi,
+						struct s1_walk_result *wr,
+						struct s1_perms *s1p)
+{

How about:

Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
	if (wi->hpd)
		return;

I was hoping not to add anything like this, because all the table bits are 0 (we simply don't collect them), and thus don't have any effect.

I just thought it was more obvious that they wouldn't apply in this case, don't feel super strongly about it.

If you want it to be obvious, how about this instead?

arch/arm64/kvm/at.c +0 -0
index 8a5e1c4682619..fb9de5fc2cc26 100644

arch/arm64/kvm/at.c static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op, · line 992

Quoted 9 lines
arch/arm64/kvm/at.c +2 -1
@@ -985,7 +985,8 @@ static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,
 	else
 		compute_s1_indirect_permissions(vcpu, wi, wr, s1p);
 
-	compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
+	if (!wi->hpd)
+		compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
 
 	if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
 		bool pan;

That's even better, if you're happy to include it.

Quoted 4 lines

Thanks,

	M.

Thanks,

	J.
Quoted 3 lines
Signature · 2 lines
-- 
Without deviation from the norm, progress is not possible.
author maintainer
#25 lore raw

PATCH v3 19/24 KVM: arm64: Disable hierarchical permissions when S1PIE is enabled

In reply to Marc Zyngier

S1PIE implicitly disables hierarchical permissions, as specified in R_JHSVW, by making TCR_ELx.HPDn RES1.

Add a predicate for S1PIE being enabled for a given translation regime, and emulate this behaviour by forcing the hpd field to true if S1PIE is enabled for that translation regime.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/at.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)
arch/arm64/kvm/at.c +19 -0
index d6ee3a5c30bc2..9a922054d76cf 100644
@@ -91,6 +91,23 @@ static enum trans_regime compute_translation_regime(struct kvm_vcpu *vcpu, u32 o
 	}
 }
 
+static bool s1pie_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)
+{
+	if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP))
+		return false;
+
+	switch (regime) {
+	case TR_EL2:
+	case TR_EL20:
+		return vcpu_read_sys_reg(vcpu, TCR2_EL2) & TCR2_EL2_PIE;
+	case TR_EL10:
+		return  (__vcpu_sys_reg(vcpu, HCRX_EL2) & HCRX_EL2_TCR2En) &&
+			(__vcpu_sys_reg(vcpu, TCR2_EL1) & TCR2_EL1x_PIE);
+	default:
+		BUG();
+	}
+}
+
 static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi,
 			 struct s1_walk_result *wr, u64 va)
 {

@@ -184,6 +201,8 @@ static int setup_s1_walk(struct kvm_vcpu *vcpu, u32 op, struct s1_walk_info *wi,
 		    (va55 ?
 		     FIELD_GET(TCR_HPD1, tcr) :
 		     FIELD_GET(TCR_HPD0, tcr)));
+	/* R_JHSVW */
+	wi->hpd |= s1pie_enabled(vcpu, wi->regime);
 
 	/* Someone was silly enough to encode TG0/TG1 differently */
 	if (va55) {
Signature · 1 line
2.39.2
author maintainer
#26 lore raw

PATCH v3 20/24 KVM: arm64: Implement AT S1PIE support

In reply to Marc Zyngier

It doesn't take much effort to imple,emt S1PIE support in AT. This is only a matter of using the AArch64.S1IndirectBasePermissions() encodings for the permission, ignoring GCS which has no impact on AT, and enforce FEAT_PAN3 being enabled as this is a requirement of FEAT_S1PIE.

Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/at.c | 119 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 118 insertions(+), 1 deletion(-)
arch/arm64/kvm/at.c +118 -1
index 9a922054d76cf..8a5e1c4682619 100644
@@ -779,6 +779,9 @@ static bool pan3_enabled(struct kvm_vcpu *vcpu, enum trans_regime regime)
 	if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR1_EL1, PAN, PAN3))
 		return false;
 
+	if (s1pie_enabled(vcpu, regime))
+		return true;
+
 	if (regime == TR_EL10)
 		sctlr = vcpu_read_sys_reg(vcpu, SCTLR_EL1);
 	else

@@ -862,12 +865,126 @@ static void compute_s1_hierarchical_permissions(struct kvm_vcpu *vcpu,
 	}
 }
 
+#define pi_idx(v, r, i)	((__vcpu_sys_reg((v), (r)) >> ((i) * 4)) & 0xf)
+
+#define set_priv_perms(p, r, w, x)	\
+	do {				\
+		(p)->pr = (r);		\
+		(p)->pw = (w);		\
+		(p)->px = (x);		\
+	} while (0)
+
+#define set_unpriv_perms(p, r, w, x)	\
+	do {				\
+		(p)->ur = (r);		\
+		(p)->uw = (w);		\
+		(p)->ux = (x);		\
+	} while (0)
+
+/* Similar to AArch64.S1IndirectBasePermissions(), without GCS  */
+#define set_perms(w, p, ip)						\
+	do {								\
+		switch ((ip)) {						\
+		case 0b0000:						\
+			set_ ## w ## _perms((p), false, false, false);	\
+			break;						\
+		case 0b0001:						\
+			set_ ## w ## _perms((p), true , false, false);	\
+			break;						\
+		case 0b0010:						\
+			set_ ## w ## _perms((p), false, false, true );	\
+			break;						\
+		case 0b0011:						\
+			set_ ## w ## _perms((p), true , false, true );	\
+			break;						\
+		case 0b0100:						\
+			set_ ## w ## _perms((p), false, false, false);	\
+			break;						\
+		case 0b0101:						\
+			set_ ## w ## _perms((p), true , true , false);	\
+			break;						\
+		case 0b0110:						\
+			set_ ## w ## _perms((p), true , true , true );	\
+			break;						\
+		case 0b0111:						\
+			set_ ## w ## _perms((p), true , true , true );	\
+			break;						\
+		case 0b1000:						\
+			set_ ## w ## _perms((p), true , false, false);	\
+			break;						\
+		case 0b1001:						\
+			set_ ## w ## _perms((p), true , false, false);	\
+			break;						\
+		case 0b1010:						\
+			set_ ## w ## _perms((p), true , false, true );	\
+			break;						\
+		case 0b1011:						\
+			set_ ## w ## _perms((p), false, false, false);	\
+			break;						\
+		case 0b1100:						\
+			set_ ## w ## _perms((p), true , true , false);	\
+			break;						\
+		case 0b1101:						\
+			set_ ## w ## _perms((p), false, false, false);	\
+			break;						\
+		case 0b1110:						\
+			set_ ## w ## _perms((p), true , true , true );	\
+			break;						\
+		case 0b1111:						\
+			set_ ## w ## _perms((p), false, false, false);	\
+			break;						\
+		}							\
+	} while (0)
+
+static void compute_s1_indirect_permissions(struct kvm_vcpu *vcpu,
+					    struct s1_walk_info *wi,
+					    struct s1_walk_result *wr,
+					    struct s1_perms *s1p)
+{
+	u8 up, pp, idx;
+
+	idx = (FIELD_GET(GENMASK(54, 53), wr->desc) << 2	|
+	       FIELD_GET(BIT(51), wr->desc) << 1		|
+	       FIELD_GET(BIT(6), wr->desc));
+
+	switch (wi->regime) {
+	case TR_EL10:
+		pp = pi_idx(vcpu, PIR_EL1, idx);
+		up = pi_idx(vcpu, PIRE0_EL1, idx);
+		break;
+	case TR_EL20:
+		pp = pi_idx(vcpu, PIR_EL2, idx);
+		up = pi_idx(vcpu, PIRE0_EL2, idx);
+		break;
+	case TR_EL2:
+		pp = pi_idx(vcpu, PIR_EL2, idx);
+		up = 0;
+		break;
+	}
+
+	set_perms(priv, s1p, pp);
+
+	if (wi->regime != TR_EL2)
+		set_perms(unpriv, s1p, up);
+	else
+		set_unpriv_perms(s1p, false, false, false);
+
+	if (s1p->px && s1p->uw) {
+		set_priv_perms(s1p, false, false, false);
+		set_unpriv_perms(s1p, false, false, false);
+	}
+}
+
 static void compute_s1_permissions(struct kvm_vcpu *vcpu, u32 op,
 				   struct s1_walk_info *wi,
 				   struct s1_walk_result *wr,
 				   struct s1_perms *s1p)
 {
-	compute_s1_direct_permissions(vcpu, wi, wr, s1p);
+	if (!s1pie_enabled(vcpu, wi->regime))
+		compute_s1_direct_permissions(vcpu, wi, wr, s1p);
+	else
+		compute_s1_indirect_permissions(vcpu, wi, wr, s1p);
+
 	compute_s1_hierarchical_permissions(vcpu, wi, wr, s1p);
 
 	if (op == OP_AT_S1E1RP || op == OP_AT_S1E1WP) {
Signature · 1 line
2.39.2
author maintainer
#27 lore raw

PATCH v3 21/24 KVM: arm64: Define helper for EL2 registers with custom visibility

In reply to Marc Zyngier

From: Mark Brown <[email protected]>

In preparation for adding more visibility filtering for EL2 registers add a helper macro like EL2_REG() which allows specification of a custom visibility operation.

Signed-off-by Mark Brown <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/sys_regs.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
arch/arm64/kvm/sys_regs.c +11 -2
index ffffcbaf80da4..41063079ed941 100644
@@ -2151,6 +2151,15 @@ static bool bad_redir_trap(struct kvm_vcpu *vcpu,
 	.val = v,				\
 }
 
+#define EL2_REG_FILTERED(name, acc, rst, v, filter) {	\
+	SYS_DESC(SYS_##name),			\
+	.access = acc,				\
+	.reset = rst,				\
+	.reg = name,				\
+	.visibility = filter,			\
+	.val = v,				\
+}
+
 #define EL2_REG_VNCR(name, rst, v)	EL2_REG(name, bad_vncr_trap, rst, v)
 #define EL2_REG_REDIR(name, rst, v)	EL2_REG(name, bad_redir_trap, rst, v)
 

@@ -2818,8 +2827,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG_VNCR(HFGITR_EL2, reset_val, 0),
 	EL2_REG_VNCR(HACR_EL2, reset_val, 0),
 
-	{ SYS_DESC(SYS_ZCR_EL2), .access = access_zcr_el2, .reset = reset_val,
-	  .visibility = sve_el2_visibility, .reg = ZCR_EL2 },
+	EL2_REG_FILTERED(ZCR_EL2, access_zcr_el2, reset_val, 0,
+			 sve_el2_visibility),
 
 	EL2_REG_VNCR(HCRX_EL2, reset_val, 0),
Signature · 1 line
2.39.2
author maintainer
#28 lore raw

PATCH v3 22/24 KVM: arm64: Hide TCR2_EL1 from userspace when disabled for guests

In reply to Marc Zyngier

From: Mark Brown <[email protected]>

When the guest does not support FEAT_TCR2 we should not allow any access to it in order to ensure that we do not create spurious issues with guest migration. Add a visibility operation for it.

Fixes fbff56068232 ("KVM: arm64: Save/restore TCR2_EL1")
Signed-off-by Mark Brown <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 arch/arm64/kvm/sys_regs.c         | 29 ++++++++++++++++++++++++++---
 2 files changed, 29 insertions(+), 3 deletions(-)
arch/arm64/include/asm/kvm_host.h +3 -0
index 34318842bd2da..cc9082e9ecf8b 100644
@@ -1489,4 +1489,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
 		(pa + pi + pa3) == 1;					\
 	})
 
+#define kvm_has_tcr2(k)				\
+	(kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP))
+
 #endif /* __ARM64_KVM_HOST_H__ */
arch/arm64/kvm/sys_regs.c +26 -3
index 41063079ed941..11fcc796be334 100644
@@ -2334,6 +2334,27 @@ static bool access_zcr_el2(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+static unsigned int tcr2_visibility(const struct kvm_vcpu *vcpu,
+				    const struct sys_reg_desc *rd)
+{
+	if (kvm_has_tcr2(vcpu->kvm))
+		return 0;
+
+	return REG_HIDDEN;
+}
+
+static unsigned int tcr2_el2_visibility(const struct kvm_vcpu *vcpu,
+				    const struct sys_reg_desc *rd)
+{
+	unsigned int r;
+
+	r = el2_visibility(vcpu, rd);
+	if (r)
+		return r;
+
+	return tcr2_visibility(vcpu, rd);
+}
+
 /*
  * Architected system registers.
  * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2

@@ -2518,7 +2539,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_TTBR0_EL1), access_vm_reg, reset_unknown, TTBR0_EL1 },
 	{ SYS_DESC(SYS_TTBR1_EL1), access_vm_reg, reset_unknown, TTBR1_EL1 },
 	{ SYS_DESC(SYS_TCR_EL1), access_vm_reg, reset_val, TCR_EL1, 0 },
-	{ SYS_DESC(SYS_TCR2_EL1), access_vm_reg, reset_val, TCR2_EL1, 0 },
+	{ SYS_DESC(SYS_TCR2_EL1), access_vm_reg, reset_val, TCR2_EL1, 0,
+	  .visibility = tcr2_visibility },
 
 	PTRAUTH_KEY(APIA),
 	PTRAUTH_KEY(APIB),

@@ -2835,7 +2857,8 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(TTBR0_EL2, access_rw, reset_val, 0),
 	EL2_REG(TTBR1_EL2, access_rw, reset_val, 0),
 	EL2_REG(TCR_EL2, access_rw, reset_val, TCR_EL2_RES1),
-	EL2_REG(TCR2_EL2, access_tcr2_el2, reset_val, TCR2_EL2_RES1),
+	EL2_REG_FILTERED(TCR2_EL2, access_tcr2_el2, reset_val, TCR2_EL2_RES1,
+			 tcr2_el2_visibility),
 	EL2_REG_VNCR(VTTBR_EL2, reset_val, 0),
 	EL2_REG_VNCR(VTCR_EL2, reset_val, 0),
 

@@ -4694,7 +4717,7 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
 		if (kvm_has_feat(kvm, ID_AA64ISAR2_EL1, MOPS, IMP))
 			vcpu->arch.hcrx_el2 |= (HCRX_EL2_MSCEn | HCRX_EL2_MCE2);
 
-		if (kvm_has_feat(kvm, ID_AA64MMFR3_EL1, TCRX, IMP))
+		if (kvm_has_tcr2(kvm))
 			vcpu->arch.hcrx_el2 |= HCRX_EL2_TCR2En;
 	}
Signature · 1 line
2.39.2
author maintainer
#29 lore raw

PATCH v3 23/24 KVM: arm64: Hide S1PIE registers from userspace when disabled for guests

In reply to Marc Zyngier

From: Mark Brown <[email protected]>

When the guest does not support S1PIE we should not allow any access to the system registers it adds in order to ensure that we do not create spurious issues with guest migration. Add a visibility operation for these registers.

Fixes 86f9de9db178 ("KVM: arm64: Save/restore PIE registers")
Signed-off-by Mark Brown <[email protected]>
Link https://lore.kernel.org/r/[email protected] open here
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/include/asm/kvm_host.h |  3 +++
 arch/arm64/kvm/sys_regs.c         | 35 ++++++++++++++++++++++++++-----
 2 files changed, 33 insertions(+), 5 deletions(-)
arch/arm64/include/asm/kvm_host.h +3 -0
index cc9082e9ecf8b..326d2f601d693 100644
@@ -1492,4 +1492,7 @@ void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
 #define kvm_has_tcr2(k)				\
 	(kvm_has_feat((k), ID_AA64MMFR3_EL1, TCRX, IMP))
 
+#define kvm_has_s1pie(k)				\
+	(kvm_has_feat((k), ID_AA64MMFR3_EL1, S1PIE, IMP))
+
 #endif /* __ARM64_KVM_HOST_H__ */
arch/arm64/kvm/sys_regs.c +30 -5
index 11fcc796be334..d8fb894832776 100644
@@ -2355,6 +2355,27 @@ static unsigned int tcr2_el2_visibility(const struct kvm_vcpu *vcpu,
 	return tcr2_visibility(vcpu, rd);
 }
 
+static unsigned int s1pie_visibility(const struct kvm_vcpu *vcpu,
+				     const struct sys_reg_desc *rd)
+{
+	if (kvm_has_s1pie(vcpu->kvm))
+		return 0;
+
+	return REG_HIDDEN;
+}
+
+static unsigned int s1pie_el2_visibility(const struct kvm_vcpu *vcpu,
+					 const struct sys_reg_desc *rd)
+{
+	unsigned int r;
+
+	r = el2_visibility(vcpu, rd);
+	if (r)
+		return r;
+
+	return s1pie_visibility(vcpu, rd);
+}
+
 /*
  * Architected system registers.
  * Important: Must be sorted ascending by Op0, Op1, CRn, CRm, Op2

@@ -2592,8 +2613,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_PMMIR_EL1), trap_raz_wi },
 
 	{ SYS_DESC(SYS_MAIR_EL1), access_vm_reg, reset_unknown, MAIR_EL1 },
-	{ SYS_DESC(SYS_PIRE0_EL1), NULL, reset_unknown, PIRE0_EL1 },
-	{ SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1 },
+	{ SYS_DESC(SYS_PIRE0_EL1), NULL, reset_unknown, PIRE0_EL1,
+	  .visibility = s1pie_visibility },
+	{ SYS_DESC(SYS_PIR_EL1), NULL, reset_unknown, PIR_EL1,
+	  .visibility = s1pie_visibility },
 	{ SYS_DESC(SYS_AMAIR_EL1), access_vm_reg, reset_amair_el1, AMAIR_EL1 },
 
 	{ SYS_DESC(SYS_LORSA_EL1), trap_loregion },

@@ -2890,8 +2913,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(HPFAR_EL2, access_rw, reset_val, 0),
 
 	EL2_REG(MAIR_EL2, access_rw, reset_val, 0),
-	EL2_REG(PIRE0_EL2, check_s1pie_access_rw, reset_val, 0),
-	EL2_REG(PIR_EL2, check_s1pie_access_rw, reset_val, 0),
+	EL2_REG_FILTERED(PIRE0_EL2, check_s1pie_access_rw, reset_val, 0,
+			 s1pie_el2_visibility),
+	EL2_REG_FILTERED(PIR_EL2, check_s1pie_access_rw, reset_val, 0,
+			 s1pie_el2_visibility),
 	EL2_REG(AMAIR_EL2, access_rw, reset_val, 0),
 
 	EL2_REG(VBAR_EL2, access_rw, reset_val, 0),

@@ -4766,7 +4791,7 @@ void kvm_calculate_traps(struct kvm_vcpu *vcpu)
 		kvm->arch.fgu[HFGITR_GROUP] |= (HFGITR_EL2_ATS1E1RP |
 						HFGITR_EL2_ATS1E1WP);
 
-	if (!kvm_has_feat(kvm, ID_AA64MMFR3_EL1, S1PIE, IMP))
+	if (!kvm_has_s1pie(kvm))
 		kvm->arch.fgu[HFGxTR_GROUP] |= (HFGxTR_EL2_nPIRE0_EL1 |
 						HFGxTR_EL2_nPIR_EL1);
Signature · 1 line
2.39.2
author maintainer
#30 lore raw

PATCH v3 24/24 KVM: arm64: Rely on visibility to let PIR*_ELx/TCR2_ELx UNDEF

In reply to Marc Zyngier

With a visibility defined for these registers, there is no need to check again for S1PIE or TCRX being implemented as perform_access() already handles it.

Reviewed-by Mark Brown <[email protected]>
Signed-off-by Marc Zyngier <[email protected]>
---
 arch/arm64/kvm/sys_regs.c | 36 +++---------------------------------
 1 file changed, 3 insertions(+), 33 deletions(-)
arch/arm64/kvm/sys_regs.c +3 -33
index d8fb894832776..e467cfb91549b 100644
@@ -361,18 +361,6 @@ static bool access_rw(struct kvm_vcpu *vcpu,
 	return true;
 }
 
-static bool check_s1pie_access_rw(struct kvm_vcpu *vcpu,
-				  struct sys_reg_params *p,
-				  const struct sys_reg_desc *r)
-{
-	if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, S1PIE, IMP)) {
-		kvm_inject_undefined(vcpu);
-		return false;
-	}
-
-	return access_rw(vcpu, p, r);
-}
-
 /*
  * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized).
  */

@@ -439,12 +427,6 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
 	bool was_enabled = vcpu_has_cache_enabled(vcpu);
 	u64 val, mask, shift;
 
-	if (reg_to_encoding(r) == SYS_TCR2_EL1 &&
-	    !kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, TCRX, IMP)) {
-		kvm_inject_undefined(vcpu);
-		return false;
-	}
-
 	BUG_ON(!p->is_write);
 
 	get_access_mask(r, &mask, &shift);

@@ -463,18 +445,6 @@ static bool access_vm_reg(struct kvm_vcpu *vcpu,
 	return true;
 }
 
-static bool access_tcr2_el2(struct kvm_vcpu *vcpu,
-			    struct sys_reg_params *p,
-			    const struct sys_reg_desc *r)
-{
-	if (!kvm_has_feat(vcpu->kvm, ID_AA64MMFR3_EL1, TCRX, IMP)) {
-		kvm_inject_undefined(vcpu);
-		return false;
-	}
-
-	return access_rw(vcpu, p, r);
-}
-
 static bool access_actlr(struct kvm_vcpu *vcpu,
 			 struct sys_reg_params *p,
 			 const struct sys_reg_desc *r)

@@ -2880,7 +2850,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(TTBR0_EL2, access_rw, reset_val, 0),
 	EL2_REG(TTBR1_EL2, access_rw, reset_val, 0),
 	EL2_REG(TCR_EL2, access_rw, reset_val, TCR_EL2_RES1),
-	EL2_REG_FILTERED(TCR2_EL2, access_tcr2_el2, reset_val, TCR2_EL2_RES1,
+	EL2_REG_FILTERED(TCR2_EL2, access_rw, reset_val, TCR2_EL2_RES1,
 			 tcr2_el2_visibility),
 	EL2_REG_VNCR(VTTBR_EL2, reset_val, 0),
 	EL2_REG_VNCR(VTCR_EL2, reset_val, 0),

@@ -2913,9 +2883,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	EL2_REG(HPFAR_EL2, access_rw, reset_val, 0),
 
 	EL2_REG(MAIR_EL2, access_rw, reset_val, 0),
-	EL2_REG_FILTERED(PIRE0_EL2, check_s1pie_access_rw, reset_val, 0,
+	EL2_REG_FILTERED(PIRE0_EL2, access_rw, reset_val, 0,
 			 s1pie_el2_visibility),
-	EL2_REG_FILTERED(PIR_EL2, check_s1pie_access_rw, reset_val, 0,
+	EL2_REG_FILTERED(PIR_EL2, access_rw, reset_val, 0,
 			 s1pie_el2_visibility),
 	EL2_REG(AMAIR_EL2, access_rw, reset_val, 0),
Signature · 1 line
2.39.2