linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v3 0/7] KVM: arm64: Normalize cache configuration
@ 2022-12-18  5:14 Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation Akihiko Odaki
                   ` (6 more replies)
  0 siblings, 7 replies; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

Before this change, the cache configuration of the physical CPU was
exposed to vcpus. This is problematic because the cache configuration a
vcpu sees varies when it migrates between vcpus with different cache
configurations.

Fabricate cache configuration from the sanitized value, which holds the
CTR_EL0 value the userspace sees regardless of which physical CPU it
resides on.

V2 -> V3:
- Corrected message for patch "Normalize cache configuration"
- Split patch "Normalize cache configuration"
- Added handling for CSSELR_EL1.TnD
- Added code to ignore RES0 in CSSELR_EL1
- Replaced arm64_ftr_reg_ctrel0.sys_val with
  read_sanitised_ftr_reg(SYS_CTR_EL0)
- Fixed vcpu->arch.ccsidr initialziation
- Added CCSIDR_EL1 sanitization
- Added FWB check
- Added a comment for CACHE_TYPE_SEPARATE
- Added MTE tag cache creation code for CLIDR_EL1 fabrication
- Removed CLIDR_EL1 reset code for reset caused by guest
- Added a comment for CCSIDR2

V2: https://lore.kernel.org/lkml/20221211051700.275761-2-akihiko.odaki@daynix.com/
V1: https://lore.kernel.org/lkml/525ff263-90b3-5b12-da31-171b09f9ad1b@daynix.com/

Akihiko Odaki (7):
  arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  arm64/sysreg: Add CCSIDR2_EL1
  arm64/cache: Move CLIDR macro definitions
  KVM: arm64: Always set HCR_TID2
  KVM: arm64: Allow user to set CCSIDR_EL1
  KVM: arm64: Mask FEAT_CCIDX
  KVM: arm64: Normalize cache configuration

 arch/arm64/include/asm/cache.h             |   9 +
 arch/arm64/include/asm/kvm_arm.h           |   3 +-
 arch/arm64/include/asm/kvm_emulate.h       |   4 -
 arch/arm64/include/asm/kvm_host.h          |   6 +-
 arch/arm64/include/asm/sysreg.h            |   1 -
 arch/arm64/kernel/cacheinfo.c              |   5 -
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |   2 -
 arch/arm64/kvm/reset.c                     |   1 +
 arch/arm64/kvm/sys_regs.c                  | 246 +++++++++++++--------
 arch/arm64/tools/sysreg                    |  16 ++
 10 files changed, 182 insertions(+), 111 deletions(-)

-- 
2.38.1


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

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

* [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  2022-12-18 11:23   ` Marc Zyngier
  2022-12-18  5:14 ` [PATCH v3 2/7] arm64/sysreg: Add CCSIDR2_EL1 Akihiko Odaki
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

Convert CCSIDR_EL1 to automatic generation as per DDI0487I.a. The field
definition is for case when FEAT_CCIDX is not implemented. Fields WT,
WB, RA and WA are defined as per A.j since they are now reserved and
may have UNKNOWN values in I.a, which the file format cannot represent.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/include/asm/sysreg.h |  1 -
 arch/arm64/tools/sysreg         | 11 +++++++++++
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 7d301700d1a9..910e960661d3 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -425,7 +425,6 @@
 
 #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1, 0)
 
-#define SYS_CCSIDR_EL1			sys_reg(3, 1, 0, 0, 0)
 #define SYS_AIDR_EL1			sys_reg(3, 1, 0, 0, 7)
 
 #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 384757a7eda9..acc79b5ccf92 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -871,6 +871,17 @@ Sysreg	SCXTNUM_EL1	3	0	13	0	7
 Field	63:0	SoftwareContextNumber
 EndSysreg
 
+Sysreg	CCSIDR_EL1	3	1	0	0	0
+Res0	63:32
+Field	31:31	WT
+Field	30:30	WB
+Field	29:29	RA
+Field	28:28	WA
+Field	27:13	NumSets
+Field	12:3	Associavity
+Field	2:0	LineSize
+EndSysreg
+
 Sysreg	CLIDR_EL1	3	1	0	0	1
 Res0	63:47
 Field	46:33	Ttypen
-- 
2.38.1


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

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

* [PATCH v3 2/7] arm64/sysreg: Add CCSIDR2_EL1
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  2022-12-19 15:01   ` Mark Brown
  2022-12-18  5:14 ` [PATCH v3 3/7] arm64/cache: Move CLIDR macro definitions Akihiko Odaki
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

CCSIDR2_EL1 was added with FEAT_CCIDX.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/tools/sysreg | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index acc79b5ccf92..0a302b4e6d7a 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -898,6 +898,11 @@ Field	5:3	Ctype2
 Field	2:0	Ctype1
 EndSysreg
 
+Sysreg	CCSIDR2_EL1	3	1	0	0	2
+Res0	63:24
+Field	23:0	NumSets
+EndSysreg
+
 Sysreg	GMID_EL1	3	1	0	0	4
 Res0	63:4
 Field	3:0	BS
-- 
2.38.1


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

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

* [PATCH v3 3/7] arm64/cache: Move CLIDR macro definitions
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 2/7] arm64/sysreg: Add CCSIDR2_EL1 Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 4/7] KVM: arm64: Always set HCR_TID2 Akihiko Odaki
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

The macros are useful for KVM which needs to manage how CLIDR is exposed
to vcpu so move them to include/asm/cache.h, which KVM can refer to.

Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/include/asm/cache.h | 6 ++++++
 arch/arm64/kernel/cacheinfo.c  | 5 -----
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index c0b178d1bb4f..ab7133654a72 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -16,6 +16,12 @@
 #define CLIDR_LOC(clidr)	(((clidr) >> CLIDR_LOC_SHIFT) & 0x7)
 #define CLIDR_LOUIS(clidr)	(((clidr) >> CLIDR_LOUIS_SHIFT) & 0x7)
 
+/* Ctypen, bits[3(n - 1) + 2 : 3(n - 1)], for n = 1 to 7 */
+#define CLIDR_CTYPE_SHIFT(level)	(3 * (level - 1))
+#define CLIDR_CTYPE_MASK(level)		(7 << CLIDR_CTYPE_SHIFT(level))
+#define CLIDR_CTYPE(clidr, level)	\
+	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
+
 /*
  * Memory returned by kmalloc() may be used for DMA, so we must make
  * sure that all such allocations are cache aligned. Otherwise,
diff --git a/arch/arm64/kernel/cacheinfo.c b/arch/arm64/kernel/cacheinfo.c
index 97c42be71338..daa7b3f55997 100644
--- a/arch/arm64/kernel/cacheinfo.c
+++ b/arch/arm64/kernel/cacheinfo.c
@@ -11,11 +11,6 @@
 #include <linux/of.h>
 
 #define MAX_CACHE_LEVEL			7	/* Max 7 level supported */
-/* Ctypen, bits[3(n - 1) + 2 : 3(n - 1)], for n = 1 to 7 */
-#define CLIDR_CTYPE_SHIFT(level)	(3 * (level - 1))
-#define CLIDR_CTYPE_MASK(level)		(7 << CLIDR_CTYPE_SHIFT(level))
-#define CLIDR_CTYPE(clidr, level)	\
-	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
 
 int cache_line_size(void)
 {
-- 
2.38.1


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

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

* [PATCH v3 4/7] KVM: arm64: Always set HCR_TID2
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
                   ` (2 preceding siblings ...)
  2022-12-18  5:14 ` [PATCH v3 3/7] arm64/cache: Move CLIDR macro definitions Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 5/7] KVM: arm64: Allow user to set CCSIDR_EL1 Akihiko Odaki
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

Always set HCR_TID2 to trap CTR_EL0, CCSIDR2_EL1, CLIDR_EL1, and
CSSELR_EL1. This saves a few lines of code and allows to employ their
access trap handlers for more purposes anticipated by the old
condition for setting HCR_TID2.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/include/asm/kvm_arm.h           | 3 ++-
 arch/arm64/include/asm/kvm_emulate.h       | 4 ----
 arch/arm64/include/asm/kvm_host.h          | 2 --
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h | 2 --
 4 files changed, 2 insertions(+), 9 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 8aa8492dafc0..44be46c280c1 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -81,11 +81,12 @@
  * SWIO:	Turn set/way invalidates into set/way clean+invalidate
  * PTW:		Take a stage2 fault if a stage1 walk steps in device memory
  * TID3:	Trap EL1 reads of group 3 ID registers
+ * TID2:	Trap CTR_EL0, CCSIDR2_EL1, CLIDR_EL1, and CSSELR_EL1
  */
 #define HCR_GUEST_FLAGS (HCR_TSC | HCR_TSW | HCR_TWE | HCR_TWI | HCR_VM | \
 			 HCR_BSU_IS | HCR_FB | HCR_TACR | \
 			 HCR_AMO | HCR_SWIO | HCR_TIDCP | HCR_RW | HCR_TLOR | \
-			 HCR_FMO | HCR_IMO | HCR_PTW | HCR_TID3 )
+			 HCR_FMO | HCR_IMO | HCR_PTW | HCR_TID3 | HCR_TID2)
 #define HCR_VIRT_EXCP_MASK (HCR_VSE | HCR_VI | HCR_VF)
 #define HCR_HOST_NVHE_FLAGS (HCR_RW | HCR_API | HCR_APK | HCR_ATA)
 #define HCR_HOST_NVHE_PROTECTED_FLAGS (HCR_HOST_NVHE_FLAGS | HCR_TSC)
diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 9bdba47f7e14..30c4598d643b 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -88,10 +88,6 @@ static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 	if (vcpu_el1_is_32bit(vcpu))
 		vcpu->arch.hcr_el2 &= ~HCR_RW;
 
-	if (cpus_have_const_cap(ARM64_MISMATCHED_CACHE_TYPE) ||
-	    vcpu_el1_is_32bit(vcpu))
-		vcpu->arch.hcr_el2 |= HCR_TID2;
-
 	if (kvm_has_mte(vcpu->kvm))
 		vcpu->arch.hcr_el2 |= HCR_ATA;
 }
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 45e2136322ba..cc2ede0eaed4 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -621,7 +621,6 @@ static inline bool __vcpu_read_sys_reg_from_cpu(int reg, u64 *val)
 		return false;
 
 	switch (reg) {
-	case CSSELR_EL1:	*val = read_sysreg_s(SYS_CSSELR_EL1);	break;
 	case SCTLR_EL1:		*val = read_sysreg_s(SYS_SCTLR_EL12);	break;
 	case CPACR_EL1:		*val = read_sysreg_s(SYS_CPACR_EL12);	break;
 	case TTBR0_EL1:		*val = read_sysreg_s(SYS_TTBR0_EL12);	break;
@@ -666,7 +665,6 @@ static inline bool __vcpu_write_sys_reg_to_cpu(u64 val, int reg)
 		return false;
 
 	switch (reg) {
-	case CSSELR_EL1:	write_sysreg_s(val, SYS_CSSELR_EL1);	break;
 	case SCTLR_EL1:		write_sysreg_s(val, SYS_SCTLR_EL12);	break;
 	case CPACR_EL1:		write_sysreg_s(val, SYS_CPACR_EL12);	break;
 	case TTBR0_EL1:		write_sysreg_s(val, SYS_TTBR0_EL12);	break;
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index baa5b9b3dde5..147cb4c846c6 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -39,7 +39,6 @@ static inline bool ctxt_has_mte(struct kvm_cpu_context *ctxt)
 
 static inline void __sysreg_save_el1_state(struct kvm_cpu_context *ctxt)
 {
-	ctxt_sys_reg(ctxt, CSSELR_EL1)	= read_sysreg(csselr_el1);
 	ctxt_sys_reg(ctxt, SCTLR_EL1)	= read_sysreg_el1(SYS_SCTLR);
 	ctxt_sys_reg(ctxt, CPACR_EL1)	= read_sysreg_el1(SYS_CPACR);
 	ctxt_sys_reg(ctxt, TTBR0_EL1)	= read_sysreg_el1(SYS_TTBR0);
@@ -95,7 +94,6 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)
 static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt)
 {
 	write_sysreg(ctxt_sys_reg(ctxt, MPIDR_EL1),	vmpidr_el2);
-	write_sysreg(ctxt_sys_reg(ctxt, CSSELR_EL1),	csselr_el1);
 
 	if (has_vhe() ||
 	    !cpus_have_final_cap(ARM64_WORKAROUND_SPECULATIVE_AT)) {
-- 
2.38.1


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

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

* [PATCH v3 5/7] KVM: arm64: Allow user to set CCSIDR_EL1
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
                   ` (3 preceding siblings ...)
  2022-12-18  5:14 ` [PATCH v3 4/7] KVM: arm64: Always set HCR_TID2 Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  2022-12-18 21:16   ` Marc Zyngier
  2022-12-18  5:14 ` [PATCH v3 6/7] KVM: arm64: Mask FEAT_CCIDX Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 7/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
  6 siblings, 1 reply; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

Allow the userspace to set CCSIDR_EL1 so that if the kernel changes the
default values of CCSIDR_EL1, the userspace can restore the old values
from an old saved VM context.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/include/asm/kvm_host.h |   3 +
 arch/arm64/kvm/reset.c            |   1 +
 arch/arm64/kvm/sys_regs.c         | 116 ++++++++++++++++++++----------
 3 files changed, 83 insertions(+), 37 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index cc2ede0eaed4..cfc6930efe1b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -417,6 +417,9 @@ struct kvm_vcpu_arch {
 		u64 last_steal;
 		gpa_t base;
 	} steal;
+
+	/* Per-vcpu CCSIDR override or NULL */
+	u32 *ccsidr;
 };
 
 /*
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 5ae18472205a..7980983dbad7 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -157,6 +157,7 @@ void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu)
 	if (sve_state)
 		kvm_unshare_hyp(sve_state, sve_state + vcpu_sve_state_size(vcpu));
 	kfree(sve_state);
+	kfree(vcpu->arch.ccsidr);
 }
 
 static void kvm_vcpu_reset_sve(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index f4a7c5abcbca..f48a3cc38d24 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -87,11 +87,27 @@ static u32 cache_levels;
 /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */
 #define CSSELR_MAX 14
 
+static u8 get_min_cache_line_size(u32 csselr)
+{
+	u64 ctr_el0;
+	int field;
+
+	ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
+	field = csselr & CSSELR_EL1_InD ? CTR_EL0_IminLine_SHIFT : CTR_EL0_DminLine_SHIFT;
+
+	return cpuid_feature_extract_unsigned_field(ctr_el0, field) - 2;
+}
+
 /* Which cache CCSIDR represents depends on CSSELR value. */
-static u32 get_ccsidr(u32 csselr)
+static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
 {
+	u32 ccsidr_index = csselr & (CSSELR_EL1_Level | CSSELR_EL1_InD);
 	u32 ccsidr;
 
+	if (vcpu->arch.ccsidr && is_valid_cache(ccsidr_index) &&
+	    !(kvm_has_mte(vcpu->kvm) && (csselr & CSSELR_EL1_TnD)))
+		return vcpu->arch.ccsidr[ccsidr_index];
+
 	/* Make sure noone else changes CSSELR during this! */
 	local_irq_disable();
 	write_sysreg(csselr, csselr_el1);
@@ -102,6 +118,61 @@ static u32 get_ccsidr(u32 csselr)
 	return ccsidr;
 }
 
+static bool is_valid_cache(u32 val)
+{
+	u32 level, ctype;
+
+	if (val >= CSSELR_MAX)
+		return false;
+
+	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
+	level = (val >> 1);
+	ctype = (cache_levels >> (level * 3)) & 7;
+
+	switch (ctype) {
+	case 0: /* No cache */
+		return false;
+	case 1: /* Instruction cache only */
+		return (val & 1);
+	case 2: /* Data cache only */
+	case 4: /* Unified cache */
+		return !(val & 1);
+	case 3: /* Separate instruction and data caches */
+		return true;
+	default: /* Reserved: we can't know instruction or data. */
+		return false;
+	}
+}
+
+static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
+{
+	u8 line_size = (val & CCSIDR_EL1_LineSize) >> CCSIDR_EL1_LineSize_SHIFT;
+	u32 *ccsidr = vcpu->arch.ccsidr;
+	u32 i;
+
+	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
+		return -EINVAL;
+
+	if (!ccsidr) {
+		if (val == get_ccsidr(vcpu, csselr))
+			return 0;
+
+		ccsidr = kmalloc_array(CSSELR_MAX, sizeof(u32), GFP_KERNEL);
+		if (!ccsidr)
+			return -ENOMEM;
+
+		for (i = 0; i < CSSELR_MAX; i++)
+			if (is_valid_cache(i))
+				ccsidr[i] = get_ccsidr(vcpu, i);
+
+		vcpu->arch.ccsidr = ccsidr;
+	}
+
+	ccsidr[csselr] = val;
+
+	return 0;
+}
+
 /*
  * See note at ARMv7 ARM B1.14.4 (TL;DR: S/W ops are not easily virtualized).
  */
@@ -1300,7 +1371,7 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 		return write_to_read_only(vcpu, p, r);
 
 	csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1);
-	p->regval = get_ccsidr(csselr);
+	p->regval = get_ccsidr(vcpu, csselr);
 
 	/*
 	 * Guests should not be doing cache operations by set/way at all, and
@@ -2660,33 +2731,7 @@ static int set_invariant_sys_reg(u64 id, u64 __user *uaddr)
 	return 0;
 }
 
-static bool is_valid_cache(u32 val)
-{
-	u32 level, ctype;
-
-	if (val >= CSSELR_MAX)
-		return false;
-
-	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
-	level = (val >> 1);
-	ctype = (cache_levels >> (level * 3)) & 7;
-
-	switch (ctype) {
-	case 0: /* No cache */
-		return false;
-	case 1: /* Instruction cache only */
-		return (val & 1);
-	case 2: /* Data cache only */
-	case 4: /* Unified cache */
-		return !(val & 1);
-	case 3: /* Separate instruction and data caches */
-		return true;
-	default: /* Reserved: we can't know instruction or data. */
-		return false;
-	}
-}
-
-static int demux_c15_get(u64 id, void __user *uaddr)
+static int demux_c15_get(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
 {
 	u32 val;
 	u32 __user *uval = uaddr;
@@ -2705,13 +2750,13 @@ static int demux_c15_get(u64 id, void __user *uaddr)
 		if (!is_valid_cache(val))
 			return -ENOENT;
 
-		return put_user(get_ccsidr(val), uval);
+		return put_user(get_ccsidr(vcpu, val), uval);
 	default:
 		return -ENOENT;
 	}
 }
 
-static int demux_c15_set(u64 id, void __user *uaddr)
+static int demux_c15_set(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
 {
 	u32 val, newval;
 	u32 __user *uval = uaddr;
@@ -2733,10 +2778,7 @@ static int demux_c15_set(u64 id, void __user *uaddr)
 		if (get_user(newval, uval))
 			return -EFAULT;
 
-		/* This is also invariant: you can't change it. */
-		if (newval != get_ccsidr(val))
-			return -EINVAL;
-		return 0;
+		return set_ccsidr(vcpu, val, newval);
 	default:
 		return -ENOENT;
 	}
@@ -2773,7 +2815,7 @@ int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 	int err;
 
 	if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
-		return demux_c15_get(reg->id, uaddr);
+		return demux_c15_get(vcpu, reg->id, uaddr);
 
 	err = get_invariant_sys_reg(reg->id, uaddr);
 	if (err != -ENOENT)
@@ -2817,7 +2859,7 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 	int err;
 
 	if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
-		return demux_c15_set(reg->id, uaddr);
+		return demux_c15_set(vcpu, reg->id, uaddr);
 
 	err = set_invariant_sys_reg(reg->id, uaddr);
 	if (err != -ENOENT)
-- 
2.38.1


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

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

* [PATCH v3 6/7] KVM: arm64: Mask FEAT_CCIDX
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
                   ` (4 preceding siblings ...)
  2022-12-18  5:14 ` [PATCH v3 5/7] KVM: arm64: Allow user to set CCSIDR_EL1 Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  2022-12-18  5:14 ` [PATCH v3 7/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
  6 siblings, 0 replies; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

The CCSIDR access handler masks the associativity bits according to the
bit layout for processors without FEAT_CCIDX. KVM also assumes CCSIDR is
32-bit where it will be 64-bit if FEAT_CCIDX is enabled. Mask FEAT_CCIDX
so that these assumptions hold.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/kvm/sys_regs.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index f48a3cc38d24..a7199f34e321 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1195,6 +1195,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, struct sys_reg_desc const *r
 						      ID_DFR0_PERFMON_SHIFT,
 						      kvm_vcpu_has_pmu(vcpu) ? ID_DFR0_PERFMON_8_4 : 0);
 		break;
+	case SYS_ID_AA64MMFR2_EL1:
+		val &= ~ID_AA64MMFR2_EL1_CCIDX_MASK;
+		break;
+	case SYS_ID_MMFR4_EL1:
+		val &= ~ARM64_FEATURE_MASK(ID_MMFR4_CCIDX);
+		break;
 	}
 
 	return val;
@@ -1676,6 +1682,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 
 	{ SYS_DESC(SYS_CCSIDR_EL1), access_ccsidr },
 	{ SYS_DESC(SYS_CLIDR_EL1), access_clidr },
+	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
 	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
 	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
 	{ SYS_DESC(SYS_CTR_EL0), access_ctr },
@@ -2177,6 +2184,10 @@ static const struct sys_reg_desc cp15_regs[] = {
 
 	{ Op1(1), CRn( 0), CRm( 0), Op2(0), access_ccsidr },
 	{ Op1(1), CRn( 0), CRm( 0), Op2(1), access_clidr },
+
+	/* CCSIDR2 */
+	{ Op1(1), CRn( 0), CRm( 0),  Op2(2), undef_access },
+
 	{ Op1(2), CRn( 0), CRm( 0), Op2(0), access_csselr, NULL, CSSELR_EL1 },
 };
 
-- 
2.38.1


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

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

* [PATCH v3 7/7] KVM: arm64: Normalize cache configuration
  2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
                   ` (5 preceding siblings ...)
  2022-12-18  5:14 ` [PATCH v3 6/7] KVM: arm64: Mask FEAT_CCIDX Akihiko Odaki
@ 2022-12-18  5:14 ` Akihiko Odaki
  6 siblings, 0 replies; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18  5:14 UTC (permalink / raw)
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin, Akihiko Odaki

Before this change, the cache configuration of the physical CPU was
exposed to vcpus. This is problematic because the cache configuration a
vcpu sees varies when it migrates between vcpus with different cache
configurations.

Fabricate cache configuration from the sanitized value, which holds the
CTR_EL0 value the userspace sees regardless of which physical CPU it
resides on.

CLIDR_EL1 is now writable from the userspace so that the VMM can
restore the values saved with the old kernel.

Suggested-by: Marc Zyngier <maz@kernel.org>
Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
---
 arch/arm64/include/asm/cache.h    |   3 +
 arch/arm64/include/asm/kvm_host.h |   1 +
 arch/arm64/kvm/sys_regs.c         | 183 +++++++++++++++---------------
 3 files changed, 96 insertions(+), 91 deletions(-)

diff --git a/arch/arm64/include/asm/cache.h b/arch/arm64/include/asm/cache.h
index ab7133654a72..a51e6e8f3171 100644
--- a/arch/arm64/include/asm/cache.h
+++ b/arch/arm64/include/asm/cache.h
@@ -22,6 +22,9 @@
 #define CLIDR_CTYPE(clidr, level)	\
 	(((clidr) & CLIDR_CTYPE_MASK(level)) >> CLIDR_CTYPE_SHIFT(level))
 
+/* Ttypen, bits [2(n - 1) + 34 : 2(n - 1) + 33], for n = 1 to 7 */
+#define CLIDR_TTYPE_SHIFT(level)	(2 * ((level) - 1) + CLIDR_EL1_Ttypen_SHIFT)
+
 /*
  * Memory returned by kmalloc() may be used for DMA, so we must make
  * sure that all such allocations are cache aligned. Otherwise,
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index cfc6930efe1b..27abf81c6910 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -178,6 +178,7 @@ struct kvm_vcpu_fault_info {
 enum vcpu_sysreg {
 	__INVALID_SYSREG__,   /* 0 is reserved as an invalid value */
 	MPIDR_EL1,	/* MultiProcessor Affinity Register */
+	CLIDR_EL1,	/* Cache Level ID Register */
 	CSSELR_EL1,	/* Cache Size Selection Register */
 	SCTLR_EL1,	/* System Control Register */
 	ACTLR_EL1,	/* Auxiliary Control Register */
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index a7199f34e321..9fd0b28e29bd 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -11,6 +11,7 @@
 
 #include <linux/bitfield.h>
 #include <linux/bsearch.h>
+#include <linux/cacheinfo.h>
 #include <linux/kvm_host.h>
 #include <linux/mm.h>
 #include <linux/printk.h>
@@ -81,9 +82,6 @@ void vcpu_write_sys_reg(struct kvm_vcpu *vcpu, u64 val, int reg)
 	 __vcpu_sys_reg(vcpu, reg) = val;
 }
 
-/* 3 bits per cache level, as per CLIDR, but non-existent caches always 0 */
-static u32 cache_levels;
-
 /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */
 #define CSSELR_MAX 14
 
@@ -101,47 +99,36 @@ static u8 get_min_cache_line_size(u32 csselr)
 /* Which cache CCSIDR represents depends on CSSELR value. */
 static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
 {
-	u32 ccsidr_index = csselr & (CSSELR_EL1_Level | CSSELR_EL1_InD);
-	u32 ccsidr;
-
-	if (vcpu->arch.ccsidr && is_valid_cache(ccsidr_index) &&
-	    !(kvm_has_mte(vcpu->kvm) && (csselr & CSSELR_EL1_TnD)))
-		return vcpu->arch.ccsidr[ccsidr_index];
-
-	/* Make sure noone else changes CSSELR during this! */
-	local_irq_disable();
-	write_sysreg(csselr, csselr_el1);
-	isb();
-	ccsidr = read_sysreg(ccsidr_el1);
-	local_irq_enable();
-
-	return ccsidr;
-}
-
-static bool is_valid_cache(u32 val)
-{
-	u32 level, ctype;
+	u64 ctr_el0;
+	int field;
 
-	if (val >= CSSELR_MAX)
-		return false;
+	if (vcpu->arch.ccsidr)
+		return vcpu->arch.ccsidr[csselr];
 
-	/* Bottom bit is Instruction or Data bit.  Next 3 bits are level. */
-	level = (val >> 1);
-	ctype = (cache_levels >> (level * 3)) & 7;
+	ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
+	field = csselr & CSSELR_EL1_InD ? CTR_EL0_IminLine_SHIFT : CTR_EL0_DminLine_SHIFT;
 
-	switch (ctype) {
-	case 0: /* No cache */
-		return false;
-	case 1: /* Instruction cache only */
-		return (val & 1);
-	case 2: /* Data cache only */
-	case 4: /* Unified cache */
-		return !(val & 1);
-	case 3: /* Separate instruction and data caches */
-		return true;
-	default: /* Reserved: we can't know instruction or data. */
-		return false;
-	}
+	/*
+	 * Fabricate a CCSIDR value as the overriding value does not exist.
+	 * The real CCSIDR value will not be used as it can vary by the
+	 * physical CPU which the vcpu currently resides in.
+	 *
+	 * The line size is determined with arm64_ftr_reg_ctrel0.sys_val, which
+	 * should be valid for all CPUs even if they have different cache
+	 * configuration.
+	 *
+	 * The associativity bits are cleared, meaning the geometry of all data
+	 * and unified caches (which are guaranteed to be PIPT and thus
+	 * non-aliasing) are 1 set and 1 way.
+	 * Guests should not be doing cache operations by set/way at all, and
+	 * for this reason, we trap them and attempt to infer the intent, so
+	 * that we can flush the entire guest's address space at the appropriate
+	 * time. The exposed geometry minimizes the number of the traps.
+	 * [If guests should attempt to infer aliasing properties from the
+	 * geometry (which is not permitted by the architecture), they would
+	 * only do so for virtually indexed caches.]
+	 */
+	return get_min_cache_line_size(csselr) << CCSIDR_EL1_LineSize_SHIFT;
 }
 
 static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
@@ -162,8 +149,7 @@ static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
 			return -ENOMEM;
 
 		for (i = 0; i < CSSELR_MAX; i++)
-			if (is_valid_cache(i))
-				ccsidr[i] = get_ccsidr(vcpu, i);
+			ccsidr[i] = get_ccsidr(vcpu, i);
 
 		vcpu->arch.ccsidr = ccsidr;
 	}
@@ -1352,10 +1338,64 @@ static bool access_clidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	if (p->is_write)
 		return write_to_read_only(vcpu, p, r);
 
-	p->regval = read_sysreg(clidr_el1);
+	p->regval = __vcpu_sys_reg(vcpu, r->reg);
 	return true;
 }
 
+/*
+ * Fabricate a CLIDR_EL1 value instead of using the real value, which can vary
+ * by the physical CPU which the vcpu currently resides in.
+ */
+static void reset_clidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
+{
+	u64 ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
+	u64 clidr;
+	u8 loc;
+
+	if ((ctr_el0 & CTR_EL0_IDC) || cpus_have_const_cap(ARM64_HAS_STAGE2_FWB)) {
+		/*
+		 * Data cache clean to the PoU is not required so LoUU and LoUIS
+		 * will not be set and a unified cache, which will be marked as
+		 * LoC, will be added.
+		 *
+		 * If not DIC, let the unified cache L2 so that an instruction
+		 * cache can be added as L1 later.
+		 */
+		loc = (ctr_el0 & CTR_EL0_DIC) ? 1 : 2;
+		clidr = CACHE_TYPE_UNIFIED << CLIDR_CTYPE_SHIFT(loc);
+	} else {
+		/*
+		 * Data cache clean to the PoU is required so let L1 have a data
+		 * cache and mark it as LoUU and LoUIS. As L1 has a data cache,
+		 * it can be marked as LoC too.
+		 */
+		loc = 1;
+		clidr = 1 << CLIDR_LOUU_SHIFT;
+		clidr |= 1 << CLIDR_LOUIS_SHIFT;
+		clidr |= CACHE_TYPE_DATA << CLIDR_CTYPE_SHIFT(1);
+	}
+
+	/*
+	 * Instruction cache invalidation to the PoU is required so let L1 have
+	 * an instruction cache. If L1 already has a data cache, it will be
+	 * CACHE_TYPE_SEPARATE.
+	 */
+	if (!(ctr_el0 & CTR_EL0_DIC))
+		clidr |= CACHE_TYPE_INST << CLIDR_CTYPE_SHIFT(1);
+
+	clidr |= loc << CLIDR_LOC_SHIFT;
+
+	/*
+	 * Add tag cache unified to data cache. Allocation tags and data are
+	 * unified in a cache line so that it looks valid even if there is only
+	 * one cache line.
+	 */
+	if (kvm_has_mte(vcpu->kvm))
+		clidr |= 2 << CLIDR_TTYPE_SHIFT(loc);
+
+	__vcpu_sys_reg(vcpu, r->reg) = clidr;
+}
+
 static bool access_csselr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 			  const struct sys_reg_desc *r)
 {
@@ -1377,22 +1417,12 @@ static bool access_ccsidr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 		return write_to_read_only(vcpu, p, r);
 
 	csselr = vcpu_read_sys_reg(vcpu, CSSELR_EL1);
+	csselr &= CSSELR_EL1_Level | CSSELR_EL1_InD;
+	if (csselr >= CSSELR_MAX)
+		return undef_access(vcpu, p, r);
+
 	p->regval = get_ccsidr(vcpu, csselr);
 
-	/*
-	 * Guests should not be doing cache operations by set/way at all, and
-	 * for this reason, we trap them and attempt to infer the intent, so
-	 * that we can flush the entire guest's address space at the appropriate
-	 * time.
-	 * To prevent this trapping from causing performance problems, let's
-	 * expose the geometry of all data and unified caches (which are
-	 * guaranteed to be PIPT and thus non-aliasing) as 1 set and 1 way.
-	 * [If guests should attempt to infer aliasing properties from the
-	 * geometry (which is not permitted by the architecture), they would
-	 * only do so for virtually indexed caches.]
-	 */
-	if (!(csselr & 1)) // data or unified cache
-		p->regval &= ~GENMASK(27, 3);
 	return true;
 }
 
@@ -1681,7 +1711,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ SYS_DESC(SYS_CNTKCTL_EL1), NULL, reset_val, CNTKCTL_EL1, 0},
 
 	{ SYS_DESC(SYS_CCSIDR_EL1), access_ccsidr },
-	{ SYS_DESC(SYS_CLIDR_EL1), access_clidr },
+	{ SYS_DESC(SYS_CLIDR_EL1), access_clidr, reset_clidr, CLIDR_EL1 },
 	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
 	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
 	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
@@ -2693,7 +2723,6 @@ id_to_sys_reg_desc(struct kvm_vcpu *vcpu, u64 id,
 
 FUNCTION_INVARIANT(midr_el1)
 FUNCTION_INVARIANT(revidr_el1)
-FUNCTION_INVARIANT(clidr_el1)
 FUNCTION_INVARIANT(aidr_el1)
 
 static void get_ctr_el0(struct kvm_vcpu *v, const struct sys_reg_desc *r)
@@ -2705,7 +2734,6 @@ static void get_ctr_el0(struct kvm_vcpu *v, const struct sys_reg_desc *r)
 static struct sys_reg_desc invariant_sys_regs[] = {
 	{ SYS_DESC(SYS_MIDR_EL1), NULL, get_midr_el1 },
 	{ SYS_DESC(SYS_REVIDR_EL1), NULL, get_revidr_el1 },
-	{ SYS_DESC(SYS_CLIDR_EL1), NULL, get_clidr_el1 },
 	{ SYS_DESC(SYS_AIDR_EL1), NULL, get_aidr_el1 },
 	{ SYS_DESC(SYS_CTR_EL0), NULL, get_ctr_el0 },
 };
@@ -2758,7 +2786,7 @@ static int demux_c15_get(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
 			return -ENOENT;
 		val = (id & KVM_REG_ARM_DEMUX_VAL_MASK)
 			>> KVM_REG_ARM_DEMUX_VAL_SHIFT;
-		if (!is_valid_cache(val))
+		if (val >= CSSELR_MAX)
 			return -ENOENT;
 
 		return put_user(get_ccsidr(vcpu, val), uval);
@@ -2783,7 +2811,7 @@ static int demux_c15_set(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
 			return -ENOENT;
 		val = (id & KVM_REG_ARM_DEMUX_VAL_MASK)
 			>> KVM_REG_ARM_DEMUX_VAL_SHIFT;
-		if (!is_valid_cache(val))
+		if (val >= CSSELR_MAX)
 			return -ENOENT;
 
 		if (get_user(newval, uval))
@@ -2882,13 +2910,7 @@ int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg
 
 static unsigned int num_demux_regs(void)
 {
-	unsigned int i, count = 0;
-
-	for (i = 0; i < CSSELR_MAX; i++)
-		if (is_valid_cache(i))
-			count++;
-
-	return count;
+	return CSSELR_MAX;
 }
 
 static int write_demux_regids(u64 __user *uindices)
@@ -2898,8 +2920,6 @@ static int write_demux_regids(u64 __user *uindices)
 
 	val |= KVM_REG_ARM_DEMUX_ID_CCSIDR;
 	for (i = 0; i < CSSELR_MAX; i++) {
-		if (!is_valid_cache(i))
-			continue;
 		if (put_user(val | i, uindices))
 			return -EFAULT;
 		uindices++;
@@ -3001,7 +3021,6 @@ int kvm_sys_reg_table_init(void)
 {
 	bool valid = true;
 	unsigned int i;
-	struct sys_reg_desc clidr;
 
 	/* Make sure tables are unique and in order. */
 	valid &= check_sysreg_table(sys_reg_descs, ARRAY_SIZE(sys_reg_descs), false);
@@ -3018,23 +3037,5 @@ int kvm_sys_reg_table_init(void)
 	for (i = 0; i < ARRAY_SIZE(invariant_sys_regs); i++)
 		invariant_sys_regs[i].reset(NULL, &invariant_sys_regs[i]);
 
-	/*
-	 * CLIDR format is awkward, so clean it up.  See ARM B4.1.20:
-	 *
-	 *   If software reads the Cache Type fields from Ctype1
-	 *   upwards, once it has seen a value of 0b000, no caches
-	 *   exist at further-out levels of the hierarchy. So, for
-	 *   example, if Ctype3 is the first Cache Type field with a
-	 *   value of 0b000, the values of Ctype4 to Ctype7 must be
-	 *   ignored.
-	 */
-	get_clidr_el1(NULL, &clidr); /* Ugly... */
-	cache_levels = clidr.val;
-	for (i = 0; i < 7; i++)
-		if (((cache_levels >> (i*3)) & 7) == 0)
-			break;
-	/* Clear all higher bits. */
-	cache_levels &= (1 << (i*3))-1;
-
 	return 0;
 }
-- 
2.38.1


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

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

* Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-18  5:14 ` [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation Akihiko Odaki
@ 2022-12-18 11:23   ` Marc Zyngier
  2022-12-18 11:35     ` Akihiko Odaki
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2022-12-18 11:23 UTC (permalink / raw)
  To: Akihiko Odaki
  Cc: linux-kernel, kvmarm, kvmarm, linux-arm-kernel, Mathieu Poirier,
	Oliver Upton, Suzuki K Poulose, Alexandru Elisei, James Morse,
	Will Deacon, Catalin Marinas, asahi, Alyssa Rosenzweig,
	Sven Peter, Hector Martin

On Sun, 18 Dec 2022 05:14:06 +0000,
Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> Convert CCSIDR_EL1 to automatic generation as per DDI0487I.a. The field
> definition is for case when FEAT_CCIDX is not implemented. Fields WT,
> WB, RA and WA are defined as per A.j since they are now reserved and
> may have UNKNOWN values in I.a, which the file format cannot represent.
>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  arch/arm64/include/asm/sysreg.h |  1 -
>  arch/arm64/tools/sysreg         | 11 +++++++++++
>  2 files changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> index 7d301700d1a9..910e960661d3 100644
> --- a/arch/arm64/include/asm/sysreg.h
> +++ b/arch/arm64/include/asm/sysreg.h
> @@ -425,7 +425,6 @@
>  
>  #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1, 0)
>  
> -#define SYS_CCSIDR_EL1			sys_reg(3, 1, 0, 0, 0)
>  #define SYS_AIDR_EL1			sys_reg(3, 1, 0, 0, 7)
>  
>  #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> index 384757a7eda9..acc79b5ccf92 100644
> --- a/arch/arm64/tools/sysreg
> +++ b/arch/arm64/tools/sysreg
> @@ -871,6 +871,17 @@ Sysreg	SCXTNUM_EL1	3	0	13	0	7
>  Field	63:0	SoftwareContextNumber
>  EndSysreg
>  
> +Sysreg	CCSIDR_EL1	3	1	0	0	0
> +Res0	63:32
> +Field	31:31	WT
> +Field	30:30	WB
> +Field	29:29	RA
> +Field	28:28	WA

For fields described as a single bit, the tool supports simply
indicating the bit number (28 rather than 28:28).

However, I strongly recommend against describing fields that have been
dropped from the architecture.  This only happens when these fields
are never used by any implementation, so describing them is at best
useless.

> +Field	27:13	NumSets
> +Field	12:3	Associavity
> +Field	2:0	LineSize
> +EndSysreg
> +

I don't think we have a good solution for overlapping fields that
depend on other factors, either contextual (such as a mode that
changes the layout of a sysreg), or architecture warts such as
FEAT_CCIDX (which changes the layout of a well-known sysreg).

At least, put a comment here that indicates the context of the
description.

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

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

* Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-18 11:23   ` Marc Zyngier
@ 2022-12-18 11:35     ` Akihiko Odaki
  2022-12-18 13:11       ` Marc Zyngier
  0 siblings, 1 reply; 16+ messages in thread
From: Akihiko Odaki @ 2022-12-18 11:35 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: linux-kernel, kvmarm, kvmarm, linux-arm-kernel, Mathieu Poirier,
	Oliver Upton, Suzuki K Poulose, Alexandru Elisei, James Morse,
	Will Deacon, Catalin Marinas, asahi, Alyssa Rosenzweig,
	Sven Peter, Hector Martin

On 2022/12/18 20:23, Marc Zyngier wrote:
> On Sun, 18 Dec 2022 05:14:06 +0000,
> Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
>>
>> Convert CCSIDR_EL1 to automatic generation as per DDI0487I.a. The field
>> definition is for case when FEAT_CCIDX is not implemented. Fields WT,
>> WB, RA and WA are defined as per A.j since they are now reserved and
>> may have UNKNOWN values in I.a, which the file format cannot represent.
>>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   arch/arm64/include/asm/sysreg.h |  1 -
>>   arch/arm64/tools/sysreg         | 11 +++++++++++
>>   2 files changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
>> index 7d301700d1a9..910e960661d3 100644
>> --- a/arch/arm64/include/asm/sysreg.h
>> +++ b/arch/arm64/include/asm/sysreg.h
>> @@ -425,7 +425,6 @@
>>   
>>   #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1, 0)
>>   
>> -#define SYS_CCSIDR_EL1			sys_reg(3, 1, 0, 0, 0)
>>   #define SYS_AIDR_EL1			sys_reg(3, 1, 0, 0, 7)
>>   
>>   #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
>> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
>> index 384757a7eda9..acc79b5ccf92 100644
>> --- a/arch/arm64/tools/sysreg
>> +++ b/arch/arm64/tools/sysreg
>> @@ -871,6 +871,17 @@ Sysreg	SCXTNUM_EL1	3	0	13	0	7
>>   Field	63:0	SoftwareContextNumber
>>   EndSysreg
>>   
>> +Sysreg	CCSIDR_EL1	3	1	0	0	0
>> +Res0	63:32
>> +Field	31:31	WT
>> +Field	30:30	WB
>> +Field	29:29	RA
>> +Field	28:28	WA
> 
> For fields described as a single bit, the tool supports simply
> indicating the bit number (28 rather than 28:28).
> 
> However, I strongly recommend against describing fields that have been
> dropped from the architecture.  This only happens when these fields
> are never used by any implementation, so describing them is at best
> useless.

arch/arm64/tools/gen-sysreg.awk does not allow a hole and requires all 
bits are described hence these descriptions. If you have an alternative 
idea I'd like to hear.

> 
>> +Field	27:13	NumSets
>> +Field	12:3	Associavity
>> +Field	2:0	LineSize
>> +EndSysreg
>> +
> 
> I don't think we have a good solution for overlapping fields that
> depend on other factors, either contextual (such as a mode that
> changes the layout of a sysreg), or architecture warts such as
> FEAT_CCIDX (which changes the layout of a well-known sysreg).
> 
> At least, put a comment here that indicates the context of the
> description.

Sounds good. I'll do so with the next version.

Regards,
Akihiko Odaki

> 
> Thanks,
> 
> 	M.
> 

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

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

* Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-18 11:35     ` Akihiko Odaki
@ 2022-12-18 13:11       ` Marc Zyngier
  2022-12-19 15:00         ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2022-12-18 13:11 UTC (permalink / raw)
  To: Akihiko Odaki
  Cc: linux-kernel, kvmarm, kvmarm, linux-arm-kernel, Mathieu Poirier,
	Oliver Upton, Suzuki K Poulose, Alexandru Elisei, James Morse,
	Will Deacon, Catalin Marinas, asahi, Alyssa Rosenzweig,
	Sven Peter, Hector Martin

On Sun, 18 Dec 2022 11:35:12 +0000,
Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> On 2022/12/18 20:23, Marc Zyngier wrote:
> > On Sun, 18 Dec 2022 05:14:06 +0000,
> > Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> >> 
> >> Convert CCSIDR_EL1 to automatic generation as per DDI0487I.a. The field
> >> definition is for case when FEAT_CCIDX is not implemented. Fields WT,
> >> WB, RA and WA are defined as per A.j since they are now reserved and
> >> may have UNKNOWN values in I.a, which the file format cannot represent.
> >> 
> >> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> >> ---
> >>   arch/arm64/include/asm/sysreg.h |  1 -
> >>   arch/arm64/tools/sysreg         | 11 +++++++++++
> >>   2 files changed, 11 insertions(+), 1 deletion(-)
> >> 
> >> diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
> >> index 7d301700d1a9..910e960661d3 100644
> >> --- a/arch/arm64/include/asm/sysreg.h
> >> +++ b/arch/arm64/include/asm/sysreg.h
> >> @@ -425,7 +425,6 @@
> >>     #define SYS_CNTKCTL_EL1			sys_reg(3, 0, 14, 1,
> >> 0)
> >>   -#define SYS_CCSIDR_EL1			sys_reg(3, 1, 0, 0, 0)
> >>   #define SYS_AIDR_EL1			sys_reg(3, 1, 0, 0, 7)
> >>     #define SYS_RNDR_EL0			sys_reg(3, 3, 2, 4, 0)
> >> diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
> >> index 384757a7eda9..acc79b5ccf92 100644
> >> --- a/arch/arm64/tools/sysreg
> >> +++ b/arch/arm64/tools/sysreg
> >> @@ -871,6 +871,17 @@ Sysreg	SCXTNUM_EL1	3	0	13	0	7
> >>   Field	63:0	SoftwareContextNumber
> >>   EndSysreg
> >>   +Sysreg	CCSIDR_EL1	3	1	0	0	0
> >> +Res0	63:32
> >> +Field	31:31	WT
> >> +Field	30:30	WB
> >> +Field	29:29	RA
> >> +Field	28:28	WA
> > 
> > For fields described as a single bit, the tool supports simply
> > indicating the bit number (28 rather than 28:28).
> > 
> > However, I strongly recommend against describing fields that have been
> > dropped from the architecture.  This only happens when these fields
> > are never used by any implementation, so describing them is at best
> > useless.
> 
> arch/arm64/tools/gen-sysreg.awk does not allow a hole and requires all
> bits are described hence these descriptions. If you have an
> alternative idea I'd like to hear.

I'd simply suggest creating an UNKNOWN field encompassing bits
[21:28]. Alternatively, feel free to try the patch below, which allows
you to describe these 4 bits as "Unkn	31:28", similar to Res0/Res1.

>
> > 
> >> +Field	27:13	NumSets
> >> +Field	12:3	Associavity

Also, you may want to fix the typo here (Associativity).

Thanks,

	M.

From 3112be25ec785de4c92d11d5964d54f216a2289c Mon Sep 17 00:00:00 2001
From: Marc Zyngier <maz@kernel.org>
Date: Sun, 18 Dec 2022 12:55:23 +0000
Subject: [PATCH] arm64: Allow the definition of UNKNOWN system register fields

The CCSIDR_EL1 register contains an UNKNOWN field (which replaces
fields that were actually defined in previous revisions of the
architecture).

Define an 'Unkn' field type modeled after the Res0/Res1 types
to allow such description. This allows the generation of

  #define CCSIDR_EL1_UNKN     (UL(0) | GENMASK_ULL(31, 28))

which may have its use one day. Hopefully the architecture doesn't
add too many of those in the future.

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/tools/gen-sysreg.awk | 20 +++++++++++++++++++-
 arch/arm64/tools/sysreg         |  2 ++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/tools/gen-sysreg.awk b/arch/arm64/tools/gen-sysreg.awk
index c350164a3955..e1df4b956596 100755
--- a/arch/arm64/tools/gen-sysreg.awk
+++ b/arch/arm64/tools/gen-sysreg.awk
@@ -98,6 +98,7 @@ END {
 
 	res0 = "UL(0)"
 	res1 = "UL(0)"
+	unkn = "UL(0)"
 
 	next_bit = 63
 
@@ -112,11 +113,13 @@ END {
 
 	define(reg "_RES0", "(" res0 ")")
 	define(reg "_RES1", "(" res1 ")")
+	define(reg "_UNKN", "(" unkn ")")
 	print ""
 
 	reg = null
 	res0 = null
 	res1 = null
+	unkn = null
 
 	next
 }
@@ -134,6 +137,7 @@ END {
 
 	res0 = "UL(0)"
 	res1 = "UL(0)"
+	unkn = "UL(0)"
 
 	define("REG_" reg, "S" op0 "_" op1 "_C" crn "_C" crm "_" op2)
 	define("SYS_" reg, "sys_reg(" op0 ", " op1 ", " crn ", " crm ", " op2 ")")
@@ -161,7 +165,9 @@ END {
 		define(reg "_RES0", "(" res0 ")")
 	if (res1 != null)
 		define(reg "_RES1", "(" res1 ")")
-	if (res0 != null || res1 != null)
+	if (unkn != null)
+		define(reg "_UNKN", "(" unkn ")")
+	if (res0 != null || res1 != null || unkn != null)
 		print ""
 
 	reg = null
@@ -172,6 +178,7 @@ END {
 	op2 = null
 	res0 = null
 	res1 = null
+	unkn = null
 
 	next
 }
@@ -190,6 +197,7 @@ END {
         next_bit = 0
 	res0 = null
 	res1 = null
+	unkn = null
 
 	next
 }
@@ -215,6 +223,16 @@ END {
 	next
 }
 
+/^Unkn/ && (block == "Sysreg" || block == "SysregFields") {
+	expect_fields(2)
+	parse_bitdef(reg, "UNKN", $2)
+	field = "UNKN_" msb "_" lsb
+
+	unkn = unkn " | GENMASK_ULL(" msb ", " lsb ")"
+
+	next
+}
+
 /^Field/ && (block == "Sysreg" || block == "SysregFields") {
 	expect_fields(3)
 	field = $3
diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index bd5fceb26c54..472f68f020d9 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -15,6 +15,8 @@
 
 # Res1	<msb>[:<lsb>]
 
+# Unkn	<msb>[:<lsb>]
+
 # Field	<msb>[:<lsb>]	<name>
 
 # Enum	<msb>[:<lsb>]	<name>
-- 
2.34.1


-- 
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

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

* Re: [PATCH v3 5/7] KVM: arm64: Allow user to set CCSIDR_EL1
  2022-12-18  5:14 ` [PATCH v3 5/7] KVM: arm64: Allow user to set CCSIDR_EL1 Akihiko Odaki
@ 2022-12-18 21:16   ` Marc Zyngier
  0 siblings, 0 replies; 16+ messages in thread
From: Marc Zyngier @ 2022-12-18 21:16 UTC (permalink / raw)
  To: Akihiko Odaki
  Cc: linux-kernel, kvmarm, kvmarm, linux-arm-kernel, Mathieu Poirier,
	Oliver Upton, Suzuki K Poulose, Alexandru Elisei, James Morse,
	Will Deacon, Catalin Marinas, asahi, Alyssa Rosenzweig,
	Sven Peter, Hector Martin

On Sun, 18 Dec 2022 05:14:10 +0000,
Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> Allow the userspace to set CCSIDR_EL1 so that if the kernel changes the
> default values of CCSIDR_EL1, the userspace can restore the old values
> from an old saved VM context.
> 
> Suggested-by: Marc Zyngier <maz@kernel.org>
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
> ---
>  arch/arm64/include/asm/kvm_host.h |   3 +
>  arch/arm64/kvm/reset.c            |   1 +
>  arch/arm64/kvm/sys_regs.c         | 116 ++++++++++++++++++++----------
>  3 files changed, 83 insertions(+), 37 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index cc2ede0eaed4..cfc6930efe1b 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -417,6 +417,9 @@ struct kvm_vcpu_arch {
>  		u64 last_steal;
>  		gpa_t base;
>  	} steal;
> +
> +	/* Per-vcpu CCSIDR override or NULL */
> +	u32 *ccsidr;
>  };
>  
>  /*
> diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
> index 5ae18472205a..7980983dbad7 100644
> --- a/arch/arm64/kvm/reset.c
> +++ b/arch/arm64/kvm/reset.c
> @@ -157,6 +157,7 @@ void kvm_arm_vcpu_destroy(struct kvm_vcpu *vcpu)
>  	if (sve_state)
>  		kvm_unshare_hyp(sve_state, sve_state + vcpu_sve_state_size(vcpu));
>  	kfree(sve_state);
> +	kfree(vcpu->arch.ccsidr);
>  }
>  
>  static void kvm_vcpu_reset_sve(struct kvm_vcpu *vcpu)
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f4a7c5abcbca..f48a3cc38d24 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -87,11 +87,27 @@ static u32 cache_levels;
>  /* CSSELR values; used to index KVM_REG_ARM_DEMUX_ID_CCSIDR */
>  #define CSSELR_MAX 14
>  
> +static u8 get_min_cache_line_size(u32 csselr)
> +{
> +	u64 ctr_el0;
> +	int field;
> +
> +	ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
> +	field = csselr & CSSELR_EL1_InD ? CTR_EL0_IminLine_SHIFT : CTR_EL0_DminLine_SHIFT;
> +
> +	return cpuid_feature_extract_unsigned_field(ctr_el0, field) - 2;
> +}
> +
>  /* Which cache CCSIDR represents depends on CSSELR value. */
> -static u32 get_ccsidr(u32 csselr)
> +static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
>  {
> +	u32 ccsidr_index = csselr & (CSSELR_EL1_Level | CSSELR_EL1_InD);
>  	u32 ccsidr;
>  
> +	if (vcpu->arch.ccsidr && is_valid_cache(ccsidr_index) &&
> +	    !(kvm_has_mte(vcpu->kvm) && (csselr & CSSELR_EL1_TnD)))
> +		return vcpu->arch.ccsidr[ccsidr_index];
> +

I really don't understand this logic. If the requested cache level is
invalid, or the MTE setup doesn't match, you return something that is
the part of the HW hierarchy, despite having a userspace-provided
hierarchy.

The other problem I can see here is that you're still relying on the
host CLIDR_EL1 (aka cache_levels), while restoring a guest cache
hierarchy must include a CLIDR_EL1. Otherwise, you cannot really
evaluate the validity of that hierarchy, nor return consistent
results.

I was expecting something like (totally untested, but you'll get what
I mean):

	if (vcpu->arch.cssidr) {
		if (!is_valid_cache(vcpu, csselr))
			return 0; // UNKNOWN value

		return vcpu->arch.ccsidr[ccsidr_index];
	}

and with is_valid_cache() written as:

bool is_valid_cache(struct kvm_vcpu *vcpu, u64 csselr)
{
	u64 clidr = __vcpu_sys_reg(vcpu, CLIDR_EL1);
	u64 idx = FIELD_GET(CSSELR_EL1_Level, csselr);
	u64 ttype = FIELD_GET(GENMASK(CLIDR_EL1_Ttypen_SHIFT + idx * 2 + 1,
				      CLIDR_EL1_Ttypen_SHIFT + idx * 2),
			      clidr);
	u64 ctype = FIELD_GET(CLIDR_EL1_Ctype1 << (idx * 3), clidr);

	// !MTE or InD make TnD RES0
	if (!kvm_has_mte(vcpu->kvm) || (csselr & CSSELR_EL1_InD))
		csselr &= ~CSSELR_EL1_TnD;

	// If TnD is set, the cache level must be purely for tags
	if (csselr & CSSELR_EL1_TnD)
		return (ttype == 0b01);

	// Otherwise, check for a match against the InD value
	switch (ctype) {
	case 0: /* No cache */
		return false;
	case 1: /* Instruction cache only */
		return (csselr & CSSELR_EL1_InD);
	case 2: /* Data cache only */
	case 4: /* Unified cache */
		return !(csselr & CSSELR_EL1_InD);
	case 3: /* Separate instruction and data caches */
		return true;
	default: /* Reserved: we can't know instruction or data. */
		return false;
	}
}

which implies that CLIDR_EL1 isn't an invariant anymore. You have that
in your last patch, but this needs to be brought in this one.

It should be validated on userspace write, making sure that
LoU/LoUIS/LoC are compatible with the state of CTR+FWB+CLIDR on the
host.

And then cache_levels disappears totally here.

[...]

> +static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
> +{
> +	u8 line_size = (val & CCSIDR_EL1_LineSize) >> CCSIDR_EL1_LineSize_SHIFT;

Better written as

	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);

> +	u32 *ccsidr = vcpu->arch.ccsidr;
> +	u32 i;
> +
> +	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
> +		return -EINVAL;
> +
> +	if (!ccsidr) {
> +		if (val == get_ccsidr(vcpu, csselr))
> +			return 0;
> +
> +		ccsidr = kmalloc_array(CSSELR_MAX, sizeof(u32), GFP_KERNEL);
> +		if (!ccsidr)
> +			return -ENOMEM;
> +
> +		for (i = 0; i < CSSELR_MAX; i++)
> +			if (is_valid_cache(i))
> +				ccsidr[i] = get_ccsidr(vcpu, i);
> +
> +		vcpu->arch.ccsidr = ccsidr;
> +	}
> +
> +	ccsidr[csselr] = val;
> +
> +	return 0;
> +}

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

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

* Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-18 13:11       ` Marc Zyngier
@ 2022-12-19 15:00         ` Mark Brown
  2022-12-19 15:27           ` Marc Zyngier
  0 siblings, 1 reply; 16+ messages in thread
From: Mark Brown @ 2022-12-19 15:00 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Akihiko Odaki, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin


[-- Attachment #1.1: Type: text/plain, Size: 1353 bytes --]

On Sun, Dec 18, 2022 at 01:11:01PM +0000, Marc Zyngier wrote:
> Akihiko Odaki <akihiko.odaki@daynix.com> wrote:

> > arch/arm64/tools/gen-sysreg.awk does not allow a hole and requires all
> > bits are described hence these descriptions. If you have an
> > alternative idea I'd like to hear.

> I'd simply suggest creating an UNKNOWN field encompassing bits
> [21:28]. Alternatively, feel free to try the patch below, which allows
> you to describe these 4 bits as "Unkn	31:28", similar to Res0/Res1.

I agree, where practical we should add new field types and other
features as needed rather than trying to shoehorn things into what the
tool currently supports.  It is very much a work in progress which can't
fully represent everything in the spec yet.  For things like the
registers with multiple possible views it's much more effort which
shouldn't get in the way of progress on features but with something like
this just updating the tool so we can match the architecture spec is the
right thing.

> Define an 'Unkn' field type modeled after the Res0/Res1 types
> to allow such description. This allows the generation of

I'd be tempted to spell out Unknown fully since Unkn is not such a
common abbreviation but I can see the desire to keep the name shorter
and it doesn't really matter so either way:

Reviewed-by: Mark Brown <broonie@kernel.org>

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

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

* Re: [PATCH v3 2/7] arm64/sysreg: Add CCSIDR2_EL1
  2022-12-18  5:14 ` [PATCH v3 2/7] arm64/sysreg: Add CCSIDR2_EL1 Akihiko Odaki
@ 2022-12-19 15:01   ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2022-12-19 15:01 UTC (permalink / raw)
  To: Akihiko Odaki
  Cc: Marc Zyngier, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin


[-- Attachment #1.1: Type: text/plain, Size: 245 bytes --]

On Sun, Dec 18, 2022 at 02:14:07PM +0900, Akihiko Odaki wrote:
> CCSIDR2_EL1 was added with FEAT_CCIDX.
> 
> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>

Reviewed-by: Mark Brown <broonie@kernel.org>

This matches DDI0487I.a.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

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

* Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-19 15:00         ` Mark Brown
@ 2022-12-19 15:27           ` Marc Zyngier
  2022-12-19 17:52             ` Mark Brown
  0 siblings, 1 reply; 16+ messages in thread
From: Marc Zyngier @ 2022-12-19 15:27 UTC (permalink / raw)
  To: Mark Brown
  Cc: Akihiko Odaki, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin

On Mon, 19 Dec 2022 15:00:15 +0000,
Mark Brown <broonie@kernel.org> wrote:
> 
> [1  <text/plain; us-ascii (7bit)>]
> On Sun, Dec 18, 2022 at 01:11:01PM +0000, Marc Zyngier wrote:
> > Akihiko Odaki <akihiko.odaki@daynix.com> wrote:
> 
> > > arch/arm64/tools/gen-sysreg.awk does not allow a hole and requires all
> > > bits are described hence these descriptions. If you have an
> > > alternative idea I'd like to hear.
> 
> > I'd simply suggest creating an UNKNOWN field encompassing bits
> > [21:28]. Alternatively, feel free to try the patch below, which allows
> > you to describe these 4 bits as "Unkn	31:28", similar to Res0/Res1.
> 
> I agree, where practical we should add new field types and other
> features as needed rather than trying to shoehorn things into what the
> tool currently supports.  It is very much a work in progress which can't
> fully represent everything in the spec yet.  For things like the
> registers with multiple possible views it's much more effort which
> shouldn't get in the way of progress on features but with something like
> this just updating the tool so we can match the architecture spec is the
> right thing.

I was tempted to add a Namespace tag that wouldn't generate the sysreg
#defines, but only generate the fields with a feature-specific
namespace. For example:

Sysreg	CCSIDR_EL1	3	1	0	0	0
Res0	63:32
Unkn	31:28
Field	27:13	NumSets
Field	12:3	Associativity
Field	2:0	LineSize
EndSysreg

Namespace CCIDX CCSIDR_EL1
Res0	63:56
Field	55:32	NumSets
Res0	31:25
Field	24:3	Associativity
Field	2:0	LineSize
EndSysreg

the later generating:

#define CCIDR_EL1_CCIDX_RES0		(GENMASK(63, 56) | GENMASK(31, 25))
#define	CCIDR_EL1_CCIDX_NumSets		GENMASK(55, 32)
#define	CCIDR_EL1_CCIDX_Associativity	GENMASK(24, 3)
#define CCIDR_EL1_CCIDX_LineSize	GENMASK(2, 0)

Thoughts?

> 
> > Define an 'Unkn' field type modeled after the Res0/Res1 types
> > to allow such description. This allows the generation of
> 
> I'd be tempted to spell out Unknown fully since Unkn is not such a
> common abbreviation but I can see the desire to keep the name shorter
> and it doesn't really matter so either way:
> 
> Reviewed-by: Mark Brown <broonie@kernel.org>

Yeah, this stuff is write-only most of the time, and I like my fields
aligned if at all possible.

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

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

* Re: [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
  2022-12-19 15:27           ` Marc Zyngier
@ 2022-12-19 17:52             ` Mark Brown
  0 siblings, 0 replies; 16+ messages in thread
From: Mark Brown @ 2022-12-19 17:52 UTC (permalink / raw)
  To: Marc Zyngier
  Cc: Akihiko Odaki, linux-kernel, kvmarm, kvmarm, linux-arm-kernel,
	Mathieu Poirier, Oliver Upton, Suzuki K Poulose, Alexandru Elisei,
	James Morse, Will Deacon, Catalin Marinas, asahi,
	Alyssa Rosenzweig, Sven Peter, Hector Martin


[-- Attachment #1.1: Type: text/plain, Size: 1952 bytes --]

On Mon, Dec 19, 2022 at 03:27:17PM +0000, Marc Zyngier wrote:
> Mark Brown <broonie@kernel.org> wrote:

> > fully represent everything in the spec yet.  For things like the
> > registers with multiple possible views it's much more effort which
> > shouldn't get in the way of progress on features but with something like
> > this just updating the tool so we can match the architecture spec is the
> > right thing.

> I was tempted to add a Namespace tag that wouldn't generate the sysreg
> #defines, but only generate the fields with a feature-specific
> namespace. For example:

I think this is roughly where we'd end up - I was using the term view
when thinking about it but that's just bikeshed.

> Sysreg	CCSIDR_EL1	3	1	0	0	0
> Res0	63:32
> Unkn	31:28
> Field	27:13	NumSets
> Field	12:3	Associativity
> Field	2:0	LineSize
> EndSysreg
> 
> Namespace CCIDX CCSIDR_EL1
> Res0	63:56
> Field	55:32	NumSets
> Res0	31:25
> Field	24:3	Associativity
> Field	2:0	LineSize
> EndSysreg

Yeah, something like that.  I think we also want a way to label bits in
the root register as only existing in namespaces/views for things where
there's no default (eg, where a feature adds two views at once or things
have been there since the base architecture), and I wasn't sure if it
made sense to nest the declaration of the views inside the Sysreg (I'm
tempted to think it's more trouble than it's worth especially on the
tooling side).

I also wanted to go through and do an audit of all the current registers
to make sure there were no nasty cases that'd complicate things.  I
don't think there'd be anything but...

> the later generating:

> #define CCIDR_EL1_CCIDX_RES0		(GENMASK(63, 56) | GENMASK(31, 25))
> #define	CCIDR_EL1_CCIDX_NumSets		GENMASK(55, 32)
> #define	CCIDR_EL1_CCIDX_Associativity	GENMASK(24, 3)
> #define CCIDR_EL1_CCIDX_LineSize	GENMASK(2, 0)

> Thoughts?

Definitely that for the output.

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

[-- Attachment #2: Type: text/plain, Size: 176 bytes --]

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

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

end of thread, other threads:[~2022-12-19 19:22 UTC | newest]

Thread overview: 16+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-12-18  5:14 [PATCH v3 0/7] KVM: arm64: Normalize cache configuration Akihiko Odaki
2022-12-18  5:14 ` [PATCH v3 1/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation Akihiko Odaki
2022-12-18 11:23   ` Marc Zyngier
2022-12-18 11:35     ` Akihiko Odaki
2022-12-18 13:11       ` Marc Zyngier
2022-12-19 15:00         ` Mark Brown
2022-12-19 15:27           ` Marc Zyngier
2022-12-19 17:52             ` Mark Brown
2022-12-18  5:14 ` [PATCH v3 2/7] arm64/sysreg: Add CCSIDR2_EL1 Akihiko Odaki
2022-12-19 15:01   ` Mark Brown
2022-12-18  5:14 ` [PATCH v3 3/7] arm64/cache: Move CLIDR macro definitions Akihiko Odaki
2022-12-18  5:14 ` [PATCH v3 4/7] KVM: arm64: Always set HCR_TID2 Akihiko Odaki
2022-12-18  5:14 ` [PATCH v3 5/7] KVM: arm64: Allow user to set CCSIDR_EL1 Akihiko Odaki
2022-12-18 21:16   ` Marc Zyngier
2022-12-18  5:14 ` [PATCH v3 6/7] KVM: arm64: Mask FEAT_CCIDX Akihiko Odaki
2022-12-18  5:14 ` [PATCH v3 7/7] KVM: arm64: Normalize cache configuration Akihiko Odaki

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