[PATCH v1 0/6] KVM: arm64: Fixes to fine grain traps and pKVM traps

[PATCH v1 0/6] KVM: arm64: Fixes to fine grain traps and pKVM traps

[Fuad Tabba]

This patch series has fixes, updates, and code for validating
fine grain trap register masks, as well as some fixes to feature
trapping in pKVM.

New fine grain trap (FGT) bits have been defined in the latest
Arm Architecture System Registers xml specification (2023-09)
[*], so the code is updated to reflect them. Moreover, some of
the already-defined masks overlap with res0, which this series
fixes. It also adds FGT register masks that weren't defined
earlier, as well as build time validation that the bits of the
various masks are all accounted for and without overlap.

Based on 6.7-rc4.

Cheers,
/fuad

[*] https://developer.arm.com/downloads/-/exploration-tools

Fuad Tabba (6):
  KVM: arm64: Update and fix FGT register masks
  KVM: arm64: Add bit masks for HAFGRTR_EL2
  KVM: arm64: Add build validation for trap mask values
  KVM: arm64: Calculate FGT RES0 Bits
  KVM: arm64: Trap external trace for protected VMs
  KVM: arm64: Mark features as supported for protected VMs

 arch/arm64/include/asm/kvm_arm.h              | 56 +++++++++++--------
 arch/arm64/kvm/hyp/include/hyp/switch.h       | 16 ++++++
 .../arm64/kvm/hyp/include/nvhe/fixed_config.h |  4 +-
 arch/arm64/kvm/hyp/nvhe/pkvm.c                |  4 ++
 arch/arm64/tools/sysreg                       |  5 +-
 5 files changed, 59 insertions(+), 26 deletions(-)


base-commit: 33cc938e65a98f1d29d0a18403dbbee050dcad9a
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 1/6] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

New trap bits have been defined in the latest Arm Architecture
System Registers xml specification [*]. Moreover, the existing
definitions of some of the mask and the res0 bits overlap, which
could be wrong, confusing, or potentially both.

Update the bits to represent the latest spec (as of this patch),
and ensure that the existing bits are consistent.

[*] https://developer.arm.com/downloads/-/exploration-tools

Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
 1 file changed, 24 insertions(+), 15 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b85f46a73e21..b1061647e837 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -344,36 +344,45 @@
  * Once we get to a point where the two describe the same thing, we'll
  * merge the definitions. One day.
  */
-#define __HFGRTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51))
+#define __HFGRTR_EL2_RES0	BIT(51)
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGRTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
+#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGWTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51) |	\
-				 BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
-				 GENMASK(26, 25) | BIT(21) | BIT(18) |	\
+#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
+				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
 				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
-#define __HFGWTR_EL2_MASK	GENMASK(49, 0)
-#define __HFGWTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
+#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
+				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
+				 GENMASK(24, 22) | GENMASK(20, 19) | \
+				 GENMASK(17, 16) | GENMASK(13, 11) | \
+				 GENMASK(8, 3) | GENMASK(1, 0))
+#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
 
-#define __HFGITR_EL2_RES0	GENMASK(63, 57)
-#define __HFGITR_EL2_MASK	GENMASK(54, 0)
-#define __HFGITR_EL2_nMASK	GENMASK(56, 55)
+#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
+#define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
+#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
 
 #define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
 				 GENMASK(21, 20) | BIT(8))
-#define __HDFGRTR_EL2_MASK	~__HDFGRTR_EL2_nMASK
+#define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
+				 GENMASK(41, 40) | GENMASK(37, 22) | \
+				 GENMASK(19, 9) | GENMASK(7, 0))
 #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
 
 #define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
 				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
 				 BIT(22) | BIT(9) | BIT(6))
-#define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
+#define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
+				 GENMASK(46, 44) | GENMASK(42, 41) | \
+				 GENMASK(37, 35) | GENMASK(33, 31) | \
+				 GENMASK(29, 23) | GENMASK(21, 10) | \
+				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
 /* Similar definitions for HCRX_EL2 */
-#define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
-#define __HCRX_EL2_MASK		(0)
-#define __HCRX_EL2_nMASK	(GENMASK(15, 14) | GENMASK(4, 0))
+#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
+#define __HCRX_EL2_MASK		(BIT(6))
+#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 2/6] KVM: arm64: Add bit masks for HAFGRTR_EL2

[Fuad Tabba]

The HAFGRTR_EL2 register isn't used yet, but it will be in future
patches. It's also the only FGT register remaining (not including
FGT2 registers).

Add the masks for completeness and future patches. The definition
is from the latest Arm Architecture System Registers xml
specification [*].

[*] https://developer.arm.com/downloads/-/exploration-tools

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

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b1061647e837..b0dc3249d5cd 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -379,6 +379,10 @@
 				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
 
+#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
+#define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
+#define __HAFGRTR_EL2_nMASK	0UL
+
 /* Similar definitions for HCRX_EL2 */
 #define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
 #define __HCRX_EL2_MASK		(BIT(6))
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 3/6] KVM: arm64: Add build validation for trap mask values

[Fuad Tabba]

These checks help ensure that all the bits are accounted for, and
that there hasn't been a transcribing error from the spec.

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

diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index f99d8af0b9af..2c6e8cbbd081 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -79,6 +79,16 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
 		clr |= ~hfg & __ ## reg ## _nMASK; 			\
 	} while(0)
 
+/*
+ * Validate the fine grain trap masks.
+ * Check that the masks do not overlap and that all bits are accounted for.
+ */
+#define CHECK_FGT_MASKS(reg)							\
+	do {									\
+		BUILD_BUG_ON((__ ## reg ## _MASK) & (__ ## reg ## _nMASK));	\
+		BUILD_BUG_ON(~((__ ## reg ## _RES0) ^ (__ ## reg ## _MASK) ^	\
+			       (__ ## reg ## _nMASK)));				\
+	} while(0)
 
 static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 {
@@ -86,6 +96,14 @@ static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
 	u64 r_clr = 0, w_clr = 0, r_set = 0, w_set = 0, tmp;
 	u64 r_val, w_val;
 
+	CHECK_FGT_MASKS(HFGRTR_EL2);
+	CHECK_FGT_MASKS(HFGWTR_EL2);
+	CHECK_FGT_MASKS(HFGITR_EL2);
+	CHECK_FGT_MASKS(HDFGRTR_EL2);
+	CHECK_FGT_MASKS(HDFGWTR_EL2);
+	CHECK_FGT_MASKS(HAFGRTR_EL2);
+	CHECK_FGT_MASKS(HCRX_EL2);
+
 	if (!cpus_have_final_cap(ARM64_HAS_FGT))
 		return;
 
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 4/6] KVM: arm64: Calculate FGT RES0 Bits

[Fuad Tabba]

All FGT reserved bits are res0, and they are the ones remaining
after accounting for all defined trap bits. Now that we have full
coverage of the trap bits, calculate the res0 bits based on the
other bits.

No functional change intended.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/include/asm/kvm_arm.h        | 19 +++++++------------
 arch/arm64/kvm/hyp/include/hyp/switch.h |  2 --
 2 files changed, 7 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index b0dc3249d5cd..44bbbb4110d3 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -344,49 +344,44 @@
  * Once we get to a point where the two describe the same thing, we'll
  * merge the definitions. One day.
  */
-#define __HFGRTR_EL2_RES0	BIT(51)
 #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
 #define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
+#define __HFGRTR_EL2_RES0	~(__HFGRTR_EL2_MASK | __HFGRTR_EL2_nMASK)
 
-#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
-				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
-				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
 #define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
 				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
 				 GENMASK(24, 22) | GENMASK(20, 19) | \
 				 GENMASK(17, 16) | GENMASK(13, 11) | \
 				 GENMASK(8, 3) | GENMASK(1, 0))
 #define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
+#define __HFGWTR_EL2_RES0	~(__HFGWTR_EL2_MASK | __HFGWTR_EL2_nMASK)
 
-#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
 #define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
 #define __HFGITR_EL2_nMASK	GENMASK(59, 55)
+#define __HFGITR_EL2_RES0	~(__HFGITR_EL2_MASK | __HFGITR_EL2_nMASK)
 
-#define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
-				 GENMASK(21, 20) | BIT(8))
 #define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
 				 GENMASK(41, 40) | GENMASK(37, 22) | \
 				 GENMASK(19, 9) | GENMASK(7, 0))
 #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
+#define __HDFGRTR_EL2_RES0	~(__HDFGRTR_EL2_MASK | __HDFGRTR_EL2_nMASK)
 
-#define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
-				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
-				 BIT(22) | BIT(9) | BIT(6))
 #define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
 				 GENMASK(46, 44) | GENMASK(42, 41) | \
 				 GENMASK(37, 35) | GENMASK(33, 31) | \
 				 GENMASK(29, 23) | GENMASK(21, 10) | \
 				 GENMASK(8, 7) | GENMASK(5, 0))
 #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
+#define __HDFGWTR_EL2_RES0	~(__HDFGWTR_EL2_MASK | __HDFGWTR_EL2_nMASK)
 
-#define __HAFGRTR_EL2_RES0	(GENMASK(63, 50) | GENMASK(16, 5))
 #define __HAFGRTR_EL2_MASK	(GENMASK(49, 17) | GENMASK(4, 0))
 #define __HAFGRTR_EL2_nMASK	0UL
+#define __HAFGRTR_EL2_RES0	~(__HAFGRTR_EL2_MASK | __HAFGRTR_EL2_nMASK)
 
 /* Similar definitions for HCRX_EL2 */
-#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
 #define __HCRX_EL2_MASK		(BIT(6))
 #define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
+#define __HCRX_EL2_RES0         ~(__HCRX_EL2_MASK | __HCRX_EL2_nMASK)
 
 /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
 #define HPFAR_MASK	(~UL(0xf))
diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
index 2c6e8cbbd081..bf045dc32996 100644
--- a/arch/arm64/kvm/hyp/include/hyp/switch.h
+++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
@@ -86,8 +86,6 @@ static inline void __activate_traps_fpsimd32(struct kvm_vcpu *vcpu)
 #define CHECK_FGT_MASKS(reg)							\
 	do {									\
 		BUILD_BUG_ON((__ ## reg ## _MASK) & (__ ## reg ## _nMASK));	\
-		BUILD_BUG_ON(~((__ ## reg ## _RES0) ^ (__ ## reg ## _MASK) ^	\
-			       (__ ## reg ## _nMASK)));				\
 	} while(0)
 
 static inline void __activate_traps_hfgxtr(struct kvm_vcpu *vcpu)
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 5/6] KVM: arm64: Trap external trace for protected VMs

[Fuad Tabba]

pKVM does not support external trace for protected VMs.

Signed-off-by: Fuad Tabba <tabba@google.com>
---
 arch/arm64/kvm/hyp/nvhe/pkvm.c | 4 ++++
 arch/arm64/tools/sysreg        | 5 ++++-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
index 9d23a51d7f75..84b5c3f387d8 100644
--- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
+++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
@@ -136,6 +136,10 @@ static void pvm_init_traps_aa64dfr0(struct kvm_vcpu *vcpu)
 			cptr_set |= CPTR_EL2_TTA;
 	}
 
+	/* Trap External Trace */
+	if (!FIELD_GET(ARM64_FEATURE_MASK(ID_AA64DFR0_EL1_ExtTrcBuff), feature_ids))
+		mdcr_clear |= MDCR_EL2_E2TB_MASK << MDCR_EL2_E2TB_SHIFT;
+
 	vcpu->arch.mdcr_el2 |= mdcr_set;
 	vcpu->arch.mdcr_el2 &= ~mdcr_clear;
 	vcpu->arch.cptr_el2 |= cptr_set;
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 96cbeeab4eec..02e1a4290fa5 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -1115,7 +1115,10 @@ Enum	63:60	HPMN0
 	0b0000	UNPREDICTABLE
 	0b0001	DEF
 EndEnum
-Res0	59:56
+UnsignedEnum	59:56	ExtTrcBuff
+	0b0000	NI
+	0b0001	IMP
+EndEnum
 UnsignedEnum	55:52	BRBE
 	0b0000	NI
 	0b0001	IMP
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH v1 6/6] KVM: arm64: Mark features as supported for protected VMs

[Fuad Tabba]

Add features supported for protected VMs to the allow list.

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

diff --git a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
index e91922daa8ca..e628541585fc 100644
--- a/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
+++ b/arch/arm64/kvm/hyp/include/nvhe/fixed_config.h
@@ -101,6 +101,7 @@
  * - Privileged Access Never
  * - SError interrupt exceptions from speculative reads
  * - Enhanced Translation Synchronization
+ * - Control for cache maintenance permission
  */
 #define PVM_ID_AA64MMFR1_ALLOW (\
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_HAFDBS) | \
@@ -108,7 +109,8 @@
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_HPDS) | \
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_PAN) | \
 	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_SpecSEI) | \
-	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_ETS) \
+	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_ETS) | \
+	ARM64_FEATURE_MASK(ID_AA64MMFR1_EL1_CMOW) \
 	)
 
 /*
-- 
2.43.0.rc2.451.g8631bc7472-goog


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/6] KVM: arm64: Update and fix FGT register masks

[Marc Zyngier]

On Tue, 05 Dec 2023 10:22:43 +0000,
Fuad Tabba <tabba@google.com> wrote:
> 
> New trap bits have been defined in the latest Arm Architecture
> System Registers xml specification [*]. Moreover, the existing
> definitions of some of the mask and the res0 bits overlap, which

nit: s/res0/RES0/

> could be wrong, confusing, or potentially both.
> 
> Update the bits to represent the latest spec (as of this patch),
> and ensure that the existing bits are consistent.
> 
> [*] https://developer.arm.com/downloads/-/exploration-tools

Please indicate which version of the XML you are referring to (it
changes on average every 3 months).

> 
> Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> Signed-off-by: Fuad Tabba <tabba@google.com>
> ---
>  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
>  1 file changed, 24 insertions(+), 15 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index b85f46a73e21..b1061647e837 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -344,36 +344,45 @@
>   * Once we get to a point where the two describe the same thing, we'll
>   * merge the definitions. One day.
>   */
> -#define __HFGRTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51))
> +#define __HFGRTR_EL2_RES0	BIT(51)

If we are now using the current architectural definition for RES0, can
we please move over to the generated values, as indicated in the above
comment?

>  #define __HFGRTR_EL2_MASK	GENMASK(49, 0)
> -#define __HFGRTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> +#define __HFGRTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
>  
> -#define __HFGWTR_EL2_RES0	(GENMASK(63, 56) | GENMASK(53, 51) |	\
> -				 BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> -				 GENMASK(26, 25) | BIT(21) | BIT(18) |	\
> +#define __HFGWTR_EL2_RES0	(BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> +				 BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
>  				 GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> -#define __HFGWTR_EL2_MASK	GENMASK(49, 0)
> -#define __HFGWTR_EL2_nMASK	(GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> +#define __HFGWTR_EL2_MASK	(GENMASK(49, 47) | GENMASK(45, 43) | \
> +				 BIT(41) | GENMASK(39, 29) | BIT(27) | \
> +				 GENMASK(24, 22) | GENMASK(20, 19) | \
> +				 GENMASK(17, 16) | GENMASK(13, 11) | \
> +				 GENMASK(8, 3) | GENMASK(1, 0))
> +#define __HFGWTR_EL2_nMASK	(GENMASK(63, 52) | BIT(50))
>  
> -#define __HFGITR_EL2_RES0	GENMASK(63, 57)
> -#define __HFGITR_EL2_MASK	GENMASK(54, 0)
> -#define __HFGITR_EL2_nMASK	GENMASK(56, 55)
> +#define __HFGITR_EL2_RES0	(BIT(63) | BIT(61))
> +#define __HFGITR_EL2_MASK	(BIT(62) | BIT(60) | GENMASK(54, 0))
> +#define __HFGITR_EL2_nMASK	GENMASK(59, 55)
>  
>  #define __HDFGRTR_EL2_RES0	(BIT(49) | BIT(42) | GENMASK(39, 38) |	\
>  				 GENMASK(21, 20) | BIT(8))
> -#define __HDFGRTR_EL2_MASK	~__HDFGRTR_EL2_nMASK
> +#define __HDFGRTR_EL2_MASK	(BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> +				 GENMASK(41, 40) | GENMASK(37, 22) | \
> +				 GENMASK(19, 9) | GENMASK(7, 0))
>  #define __HDFGRTR_EL2_nMASK	GENMASK(62, 59)
>  
>  #define __HDFGWTR_EL2_RES0	(BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
>  				 BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
>  				 BIT(22) | BIT(9) | BIT(6))
> -#define __HDFGWTR_EL2_MASK	~__HDFGWTR_EL2_nMASK
> +#define __HDFGWTR_EL2_MASK	(GENMASK(57, 52) | GENMASK(50, 48) | \
> +				 GENMASK(46, 44) | GENMASK(42, 41) | \
> +				 GENMASK(37, 35) | GENMASK(33, 31) | \
> +				 GENMASK(29, 23) | GENMASK(21, 10) | \
> +				 GENMASK(8, 7) | GENMASK(5, 0))
>  #define __HDFGWTR_EL2_nMASK	GENMASK(62, 60)
>  
>  /* Similar definitions for HCRX_EL2 */
> -#define __HCRX_EL2_RES0		(GENMASK(63, 16) | GENMASK(13, 12))
> -#define __HCRX_EL2_MASK		(0)
> -#define __HCRX_EL2_nMASK	(GENMASK(15, 14) | GENMASK(4, 0))
> +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> +#define __HCRX_EL2_MASK		(BIT(6))
> +#define __HCRX_EL2_nMASK	(GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
>  
>  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
>  #define HPFAR_MASK	(~UL(0xf))

The main driver for not following the latest and greatest version
published in the architecture (including the FGT2 stuff) was to limit
the amount of work we need to do in NV.

Can you please update the FGT tables in emulated-nested.c so that we
have a matching description of the added bits? This also overlaps with
the work Joey is doing on POE, so there is scope for collaboration
between you two here.

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH v1 1/6] KVM: arm64: Update and fix FGT register masks

[Fuad Tabba]

Hi Marc,

On Tue, Dec 5, 2023 at 11:20 AM Marc Zyngier <maz@kernel.org> wrote:
>
> On Tue, 05 Dec 2023 10:22:43 +0000,
> Fuad Tabba <tabba@google.com> wrote:
> >
> > New trap bits have been defined in the latest Arm Architecture
> > System Registers xml specification [*]. Moreover, the existing
> > definitions of some of the mask and the res0 bits overlap, which
>
> nit: s/res0/RES0/

Ack

> > could be wrong, confusing, or potentially both.
> >
> > Update the bits to represent the latest spec (as of this patch),
> > and ensure that the existing bits are consistent.
> >
> > [*] https://developer.arm.com/downloads/-/exploration-tools
>
> Please indicate which version of the XML you are referring to (it
> changes on average every 3 months).

Ack

> >
> > Fixes: 0fd76865006d ("KVM: arm64: Add nPIR{E0}_EL1 to HFG traps")
> > Signed-off-by: Fuad Tabba <tabba@google.com>
> > ---
> >  arch/arm64/include/asm/kvm_arm.h | 39 ++++++++++++++++++++------------
> >  1 file changed, 24 insertions(+), 15 deletions(-)
> >
> > diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> > index b85f46a73e21..b1061647e837 100644
> > --- a/arch/arm64/include/asm/kvm_arm.h
> > +++ b/arch/arm64/include/asm/kvm_arm.h
> > @@ -344,36 +344,45 @@
> >   * Once we get to a point where the two describe the same thing, we'll
> >   * merge the definitions. One day.
> >   */
> > -#define __HFGRTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51))
> > +#define __HFGRTR_EL2_RES0    BIT(51)
>
> If we are now using the current architectural definition for RES0, can
> we please move over to the generated values, as indicated in the above
> comment?

I'd missed the generated definitions. I thought that the comment was
referring to the read and write version of the registers. Will fix
that.

> >  #define __HFGRTR_EL2_MASK    GENMASK(49, 0)
> > -#define __HFGRTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > +#define __HFGRTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> >
> > -#define __HFGWTR_EL2_RES0    (GENMASK(63, 56) | GENMASK(53, 51) |    \
> > -                              BIT(46) | BIT(42) | BIT(40) | BIT(28) | \
> > -                              GENMASK(26, 25) | BIT(21) | BIT(18) |  \
> > +#define __HFGWTR_EL2_RES0    (BIT(51) | BIT(46) | BIT(42) | BIT(40) | \
> > +                              BIT(28) | GENMASK(26, 25) | BIT(21) | BIT(18) | \
> >                                GENMASK(15, 14) | GENMASK(10, 9) | BIT(2))
> > -#define __HFGWTR_EL2_MASK    GENMASK(49, 0)
> > -#define __HFGWTR_EL2_nMASK   (GENMASK(58, 57) | GENMASK(55, 54) | BIT(50))
> > +#define __HFGWTR_EL2_MASK    (GENMASK(49, 47) | GENMASK(45, 43) | \
> > +                              BIT(41) | GENMASK(39, 29) | BIT(27) | \
> > +                              GENMASK(24, 22) | GENMASK(20, 19) | \
> > +                              GENMASK(17, 16) | GENMASK(13, 11) | \
> > +                              GENMASK(8, 3) | GENMASK(1, 0))
> > +#define __HFGWTR_EL2_nMASK   (GENMASK(63, 52) | BIT(50))
> >
> > -#define __HFGITR_EL2_RES0    GENMASK(63, 57)
> > -#define __HFGITR_EL2_MASK    GENMASK(54, 0)
> > -#define __HFGITR_EL2_nMASK   GENMASK(56, 55)
> > +#define __HFGITR_EL2_RES0    (BIT(63) | BIT(61))
> > +#define __HFGITR_EL2_MASK    (BIT(62) | BIT(60) | GENMASK(54, 0))
> > +#define __HFGITR_EL2_nMASK   GENMASK(59, 55)
> >
> >  #define __HDFGRTR_EL2_RES0   (BIT(49) | BIT(42) | GENMASK(39, 38) |  \
> >                                GENMASK(21, 20) | BIT(8))
> > -#define __HDFGRTR_EL2_MASK   ~__HDFGRTR_EL2_nMASK
> > +#define __HDFGRTR_EL2_MASK   (BIT(63) | GENMASK(58, 50) | GENMASK(48, 43) | \
> > +                              GENMASK(41, 40) | GENMASK(37, 22) | \
> > +                              GENMASK(19, 9) | GENMASK(7, 0))
> >  #define __HDFGRTR_EL2_nMASK  GENMASK(62, 59)
> >
> >  #define __HDFGWTR_EL2_RES0   (BIT(63) | GENMASK(59, 58) | BIT(51) | BIT(47) | \
> >                                BIT(43) | GENMASK(40, 38) | BIT(34) | BIT(30) | \
> >                                BIT(22) | BIT(9) | BIT(6))
> > -#define __HDFGWTR_EL2_MASK   ~__HDFGWTR_EL2_nMASK
> > +#define __HDFGWTR_EL2_MASK   (GENMASK(57, 52) | GENMASK(50, 48) | \
> > +                              GENMASK(46, 44) | GENMASK(42, 41) | \
> > +                              GENMASK(37, 35) | GENMASK(33, 31) | \
> > +                              GENMASK(29, 23) | GENMASK(21, 10) | \
> > +                              GENMASK(8, 7) | GENMASK(5, 0))
> >  #define __HDFGWTR_EL2_nMASK  GENMASK(62, 60)
> >
> >  /* Similar definitions for HCRX_EL2 */
> > -#define __HCRX_EL2_RES0              (GENMASK(63, 16) | GENMASK(13, 12))
> > -#define __HCRX_EL2_MASK              (0)
> > -#define __HCRX_EL2_nMASK     (GENMASK(15, 14) | GENMASK(4, 0))
> > +#define __HCRX_EL2_RES0         (GENMASK(63, 25) | GENMASK(13, 12))
> > +#define __HCRX_EL2_MASK              (BIT(6))
> > +#define __HCRX_EL2_nMASK     (GENMASK(24, 14) | GENMASK(11, 7) | GENMASK(5, 0))
> >
> >  /* Hyp Prefetch Fault Address Register (HPFAR/HDFAR) */
> >  #define HPFAR_MASK   (~UL(0xf))
>
> The main driver for not following the latest and greatest version
> published in the architecture (including the FGT2 stuff) was to limit
> the amount of work we need to do in NV.
>
> Can you please update the FGT tables in emulated-nested.c so that we
> have a matching description of the added bits? This also overlaps with
> the work Joey is doing on POE, so there is scope for collaboration
> between you two here.

Will do.

Cheers,
/fuad

> Thanks,
>
>         M.
>
> --
> Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel