[PATCH v7 0/7] KVM: arm64: Normalize cache configuration

[PATCH v7 0/7] KVM: arm64: Normalize cache configuration

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

V6 -> V7:
- Make the code to determine cache line more readable.
  Based on Oliver Upton's suggestion.

V5 -> V6:
- Add a comment to get_min_cache_line_size()

V4 -> V5:
- Noted why cache level existence check is unnecessary when fabricating
  CCSIDR_EL1 value.
- Removed FWB check. It is necessary as CLIDR_EL1.{LoUU, LoIUS} on the
  host are {0, 0} if FWB is enabled, and such a CLIDR_EL1 value sets
  the IDC bit of the sanitized CTR_EL0 value, which is already checked.
- Removed UNDEF injection when reading CCSIDR_EL1 with an invalid
  CSSELR_EL1 value.
- Added a check for CLIDR_EL1.{LoUU,LoC,LoUIS} values set from the
  userspace.

V3 -> V4:
- Implemented UNKNOWN system register definition for CCSIDR_EL1
- Added a comment about the relation between CCSIDR_EL1 and FEAT_CCIDX
- Squashed "Normalize cache configuration" and "Allow user to set
  CCSIDR_EL1"
  The intermediate state between them did not make much sense.
- Introduced FIELD_GET to extract CCSIDR_EL1_LineSize.

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 (6):
  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: Mask FEAT_CCIDX
  KVM: arm64: Normalize cache configuration

Marc Zyngier (1):
  arm64: Allow the definition of UNKNOWN system register fields

 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                  | 274 +++++++++++++--------
 arch/arm64/tools/gen-sysreg.awk            |  20 +-
 arch/arm64/tools/sysreg                    |  17 ++
 11 files changed, 230 insertions(+), 112 deletions(-)

-- 
2.39.0


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

[PATCH v7 1/7] arm64: Allow the definition of UNKNOWN system register fields

[Akihiko Odaki]

From: Marc Zyngier <maz@kernel.org>

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>
Signed-off-by: Akihiko Odaki <akihiko.odaki@gmail.com>
Reviewed-by: Mark Brown <broonie@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 184e58fd5631..f754265aec5f 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.39.0


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

[PATCH v7 2/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation

[Akihiko Odaki]

Convert CCSIDR_EL1 to automatic generation as per DDI0487I.a.

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

diff --git a/arch/arm64/include/asm/sysreg.h b/arch/arm64/include/asm/sysreg.h
index 1312fb48f18b..a2a93b3fc557 100644
--- a/arch/arm64/include/asm/sysreg.h
+++ b/arch/arm64/include/asm/sysreg.h
@@ -404,7 +404,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 f754265aec5f..45648fa89be8 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -1637,6 +1637,16 @@ Sysreg	SCXTNUM_EL1	3	0	13	0	7
 Field	63:0	SoftwareContextNumber
 EndSysreg
 
+# The bit layout for CCSIDR_EL1 depends on whether FEAT_CCIDX is implemented.
+# The following is for case when FEAT_CCIDX is not implemented.
+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
+
 Sysreg	CLIDR_EL1	3	1	0	0	1
 Res0	63:47
 Field	46:33	Ttypen
-- 
2.39.0


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

[PATCH v7 3/7] arm64/sysreg: Add CCSIDR2_EL1

[Akihiko Odaki]

CCSIDR2_EL1 is available if FEAT_CCIDX is implemented as per
DDI0487I.a.

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

diff --git a/arch/arm64/tools/sysreg b/arch/arm64/tools/sysreg
index 45648fa89be8..90b64697345f 100644
--- a/arch/arm64/tools/sysreg
+++ b/arch/arm64/tools/sysreg
@@ -1663,6 +1663,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.39.0


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

[PATCH v7 4/7] arm64/cache: Move CLIDR macro definitions

[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.39.0


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

[PATCH v7 5/7] KVM: arm64: Always set HCR_TID2

[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>
Reviewed-by: Reiji Watanabe <reijiw@google.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 0df3fc3a0173..158f2033fde9 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 35a159d131b5..374390a9212e 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -705,7 +705,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;
@@ -750,7 +749,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.39.0


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

[PATCH v7 6/7] KVM: arm64: Mask FEAT_CCIDX

[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 d5ee52d6bf73..5617de916c80 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1155,6 +1155,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu, struct sys_reg_desc const *r
 		val |= FIELD_PREP(ARM64_FEATURE_MASK(ID_DFR0_EL1_PerfMon),
 				  pmuver_to_perfmon(vcpu_pmuver(vcpu)));
 		break;
+	case SYS_ID_AA64MMFR2_EL1:
+		val &= ~ID_AA64MMFR2_EL1_CCIDX_MASK;
+		break;
+	case SYS_ID_MMFR4_EL1:
+		val &= ~ARM64_FEATURE_MASK(ID_MMFR4_EL1_CCIDX);
+		break;
 	}
 
 	return val;
@@ -1718,6 +1724,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 },
@@ -2219,6 +2226,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.39.0


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

[PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[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 and CCSIDR_EL1 are 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 |   4 +
 arch/arm64/kvm/reset.c            |   1 +
 arch/arm64/kvm/sys_regs.c         | 263 +++++++++++++++++++-----------
 4 files changed, 175 insertions(+), 96 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 374390a9212e..496602e0b299 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -252,6 +252,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 */
@@ -501,6 +502,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 e0267f672b8a..dc235ddc6172 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 5617de916c80..459e6d358dab 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,25 +82,96 @@ 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
 
+/*
+ * Returns the minimum line size for the selected cache, expressed as
+ * Log2(bytes).
+ */
+static u8 get_min_cache_line_size(bool icache)
+{
+	u64 ctr = read_sanitised_ftr_reg(SYS_CTR_EL0);
+	u8 field;
+
+	if (icache)
+		field = SYS_FIELD_GET(CTR_EL0, IminLine, ctr);
+	else
+		field = SYS_FIELD_GET(CTR_EL0, DminLine, ctr);
+
+	/*
+	 * Cache line size is represented as Log2(words) in CTR_EL0.
+	 * Log2(bytes) can be derived with the following:
+	 *
+	 * Log2(words) + 2 = Log2(bytes / 4) + 2
+	 * 		   = Log2(bytes) - 2 + 2
+	 * 		   = Log2(bytes)
+	 */
+	return 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)
+{
+	u8 line_size;
+
+	if (vcpu->arch.ccsidr)
+		return vcpu->arch.ccsidr[csselr];
+
+	line_size = get_min_cache_line_size(csselr & CSSELR_EL1_InD);
+
+	/*
+	 * 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 get_min_cache_line_size(), 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.]
+	 *
+	 * We don't check if the cache level exists as it is allowed to return
+	 * an UNKNOWN value if not.
+	 */
+	return SYS_FIELD_PREP(CCSIDR_EL1, LineSize, line_size - 4);
+}
+
+static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
 {
-	u32 ccsidr;
+	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++)
+			ccsidr[i] = get_ccsidr(vcpu, i);
+
+		vcpu->arch.ccsidr = ccsidr;
+	}
 
-	/* 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();
+	ccsidr[csselr] = val;
 
-	return ccsidr;
+	return 0;
 }
 
 /*
@@ -1391,10 +1463,78 @@ 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)) {
+		/*
+		 * 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 int set_clidr(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+		      u64 val)
+{
+	u64 ctr_el0 = read_sanitised_ftr_reg(SYS_CTR_EL0);
+	u64 idc = !CLIDR_LOC(val) || (!CLIDR_LOUIS(val) && !CLIDR_LOUU(val));
+
+	if ((val & CLIDR_EL1_RES0) || (!(ctr_el0 & CTR_EL0_IDC) && idc))
+		return -EINVAL;
+
+	__vcpu_sys_reg(vcpu, rd->reg) = val;
+
+	return 0;
+}
+
 static bool access_csselr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 			  const struct sys_reg_desc *r)
 {
@@ -1416,22 +1556,10 @@ 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);
+	csselr &= CSSELR_EL1_Level | CSSELR_EL1_InD;
+	if (csselr < CSSELR_MAX)
+		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;
 }
 
@@ -1723,7 +1851,8 @@ 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,
+	  .set_user = set_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 },
@@ -2735,7 +2864,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)
@@ -2747,7 +2875,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 },
 };
@@ -2784,33 +2911,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;
@@ -2826,16 +2927,16 @@ static int demux_c15_get(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(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;
@@ -2851,16 +2952,13 @@ static int demux_c15_set(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))
 			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;
 	}
@@ -2897,7 +2995,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)
@@ -2941,7 +3039,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)
@@ -2953,13 +3051,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)
@@ -2969,8 +3061,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++;
@@ -3072,7 +3162,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);
@@ -3089,23 +3178,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.39.0


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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Oliver Upton]

Hi Akihiko,

On Thu, Jan 12, 2023 at 11:38:52AM +0900, Akihiko Odaki wrote:
> 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 and CCSIDR_EL1 are 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>

I needed to squash in the patch below to get all of this working.
Writing back the value read for a given cache level was failing, which I
caught with the get-reg-list selftest.

Pushed the result here if you want to have a look:

  https://github.com/oupton/linux/tree/kvm-arm64/virtual-cache-geometry

--
Thanks,
Oliver

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 459e6d358dab..b6228f7d1d8d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -148,17 +148,19 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
 
 static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
 {
-	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);
+	u8 line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, val);
+	u32 cur = get_ccsidr(vcpu, csselr);
+	u8 min_line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, cur);
 	u32 *ccsidr = vcpu->arch.ccsidr;
 	u32 i;
 
-	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
+	if (cur == val)
+		return 0;
+
+	if ((val & CCSIDR_EL1_RES0) || line_size < min_line_size)
 		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;

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Marc Zyngier]

On Thu, 19 Jan 2023 19:46:16 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> Hi Akihiko,
> 
> On Thu, Jan 12, 2023 at 11:38:52AM +0900, Akihiko Odaki wrote:
> > 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 and CCSIDR_EL1 are 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>
> 
> I needed to squash in the patch below to get all of this working.
> Writing back the value read for a given cache level was failing, which I
> caught with the get-reg-list selftest.
> 
> Pushed the result here if you want to have a look:
> 
>   https://github.com/oupton/linux/tree/kvm-arm64/virtual-cache-geometry
> 
> --
> Thanks,
> Oliver
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 459e6d358dab..b6228f7d1d8d 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -148,17 +148,19 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
>  
>  static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
>  {
> -	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);
> +	u8 line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, val);
> +	u32 cur = get_ccsidr(vcpu, csselr);
> +	u8 min_line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, cur);
>  	u32 *ccsidr = vcpu->arch.ccsidr;
>  	u32 i;
>  
> -	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
> +	if (cur == val)
> +		return 0;
> +
> +	if ((val & CCSIDR_EL1_RES0) || line_size < min_line_size)
>  		return -EINVAL;

This doesn't look right. You're comparing the value userspace is
trying to set for a given level with the value that is already set for
that level, and forbid the cache line size to be smaller. It works if
no value has been set yet (you fallback to something derived from
CTR_EL0), but this fails if userspace does multiple writes.

The original check is against CTR_EL0, which makes absolute sense
because we want to check across the whole hierarchy. It is just that
the original code has two bugs:

- It fails to convert the CCSIDR_EL1.LineSize value to a number of
  words (the missing +4). Admire how the architecture is actively
  designed to be hostile to SW by providing two different formats for
  the cache line size, none of which is in... bytes.

- It passes the full CSSELR value to get_min_cache_line_size(), while
  this function wants a bool... Yes, there are times where you'd want
  a stronger type system (did anyone say Rust? ;-)

I propose that we fold something like the patch below in instead
(tested with get-reg-list).

Thanks,

	M.

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 3b3024c42e61..ac943dcb4610 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -148,11 +148,12 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
 
 static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
 {
-	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);
+	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val) + 4;
 	u32 *ccsidr = vcpu->arch.ccsidr;
 	u32 i;
 
-	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
+	if ((val & CCSIDR_EL1_RES0) ||
+	    line_size < get_min_cache_line_size(csselr & CSSELR_EL1_InD))
 		return -EINVAL;
 
 	if (!ccsidr) {

-- 
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 v7 7/7] KVM: arm64: Normalize cache configuration

[Oliver Upton]

Hey Marc,

On Sat, Jan 21, 2023 at 12:02:03PM +0000, Marc Zyngier wrote:
> On Thu, 19 Jan 2023 19:46:16 +0000, Oliver Upton <oliver.upton@linux.dev> wrote:
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 459e6d358dab..b6228f7d1d8d 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -148,17 +148,19 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
> >  
> >  static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
> >  {
> > -	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);
> > +	u8 line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, val);
> > +	u32 cur = get_ccsidr(vcpu, csselr);
> > +	u8 min_line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, cur);
> >  	u32 *ccsidr = vcpu->arch.ccsidr;
> >  	u32 i;
> >  
> > -	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
> > +	if (cur == val)
> > +		return 0;
> > +
> > +	if ((val & CCSIDR_EL1_RES0) || line_size < min_line_size)
> >  		return -EINVAL;
> 
> This doesn't look right. You're comparing the value userspace is
> trying to set for a given level with the value that is already set for
> that level, and forbid the cache line size to be smaller. It works if
> no value has been set yet (you fallback to something derived from
> CTR_EL0), but this fails if userspace does multiple writes.

Good catch, I tried to skip over the unit/field conversions by doing this
but it has the consequence of not working as expected for multiple writes.

> The original check is against CTR_EL0, which makes absolute sense
> because we want to check across the whole hierarchy. It is just that
> the original code has two bugs:
> 
> - It fails to convert the CCSIDR_EL1.LineSize value to a number of
>   words (the missing +4). Admire how the architecture is actively
>   designed to be hostile to SW by providing two different formats for
>   the cache line size, none of which is in... bytes.
> 
> - It passes the full CSSELR value to get_min_cache_line_size(), while
>   this function wants a bool... Yes, there are times where you'd want
>   a stronger type system (did anyone say Rust? ;-)

Hey now, if you say it enough times people are going to start getting
ideas ;-P

> I propose that we fold something like the patch below in instead
> (tested with get-reg-list).

Agreed, I've backed out my diff and applied yours. Pushed (with force!)
to my repo now, PTAL.

--
Thanks,
Oliver

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Akihiko Odaki]

On 2023/01/22 3:15, Oliver Upton wrote:
> Hey Marc,
> 
> On Sat, Jan 21, 2023 at 12:02:03PM +0000, Marc Zyngier wrote:
>> On Thu, 19 Jan 2023 19:46:16 +0000, Oliver Upton <oliver.upton@linux.dev> wrote:
>>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
>>> index 459e6d358dab..b6228f7d1d8d 100644
>>> --- a/arch/arm64/kvm/sys_regs.c
>>> +++ b/arch/arm64/kvm/sys_regs.c
>>> @@ -148,17 +148,19 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
>>>   
>>>   static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
>>>   {
>>> -	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);
>>> +	u8 line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, val);
>>> +	u32 cur = get_ccsidr(vcpu, csselr);
>>> +	u8 min_line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, cur);
>>>   	u32 *ccsidr = vcpu->arch.ccsidr;
>>>   	u32 i;
>>>   
>>> -	if ((val & CCSIDR_EL1_RES0) || line_size < get_min_cache_line_size(csselr))
>>> +	if (cur == val)
>>> +		return 0;
>>> +
>>> +	if ((val & CCSIDR_EL1_RES0) || line_size < min_line_size)
>>>   		return -EINVAL;
>>
>> This doesn't look right. You're comparing the value userspace is
>> trying to set for a given level with the value that is already set for
>> that level, and forbid the cache line size to be smaller. It works if
>> no value has been set yet (you fallback to something derived from
>> CTR_EL0), but this fails if userspace does multiple writes.
> 
> Good catch, I tried to skip over the unit/field conversions by doing this
> but it has the consequence of not working as expected for multiple writes.
> 
>> The original check is against CTR_EL0, which makes absolute sense
>> because we want to check across the whole hierarchy. It is just that
>> the original code has two bugs:
>>
>> - It fails to convert the CCSIDR_EL1.LineSize value to a number of
>>    words (the missing +4). Admire how the architecture is actively
>>    designed to be hostile to SW by providing two different formats for
>>    the cache line size, none of which is in... bytes.
>>
>> - It passes the full CSSELR value to get_min_cache_line_size(), while
>>    this function wants a bool... Yes, there are times where you'd want
>>    a stronger type system (did anyone say Rust? ;-)
> 
> Hey now, if you say it enough times people are going to start getting
> ideas ;-P
> 
>> I propose that we fold something like the patch below in instead
>> (tested with get-reg-list).
> 
> Agreed, I've backed out my diff and applied yours. Pushed (with force!)
> to my repo now, PTAL.
> 
> --
> Thanks,
> Oliver
> 

I was so careless that I missed two bugs and failed to test the last 
version of my patch. It is fortunate that the bugs were caught by 
careful review though we don't have a strong type system (yet). Your 
tree looks good to me.

Regards,
Akihiko Odaki

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Oliver Upton]

On Mon, Jan 23, 2023 at 02:36:39AM +0900, Akihiko Odaki wrote:

[...]

> I was so careless that I missed two bugs and failed to test the last version
> of my patch. It is fortunate that the bugs were caught by careful review
> though we don't have a strong type system (yet).

This is the exact purpose of reviews though! We make these sort of
mistakes all the time, so nothing to worry about. Thanks for your
responsiveness on this series and for quickly respinning it, very
helpful.

> Your tree looks good to me.

Thanks!

--
Best,
Oliver

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Marc Zyngier]

On Sun, 22 Jan 2023 17:36:39 +0000,
Akihiko Odaki <akihiko.odaki@gmail.com> wrote:
> 
> On 2023/01/22 3:15, Oliver Upton wrote:
> > Hey Marc,
> > 
> > On Sat, Jan 21, 2023 at 12:02:03PM +0000, Marc Zyngier wrote:
> >> On Thu, 19 Jan 2023 19:46:16 +0000, Oliver Upton <oliver.upton@linux.dev> wrote:
> >>> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> >>> index 459e6d358dab..b6228f7d1d8d 100644
> >>> --- a/arch/arm64/kvm/sys_regs.c
> >>> +++ b/arch/arm64/kvm/sys_regs.c
> >>> @@ -148,17 +148,19 @@ static u32 get_ccsidr(struct kvm_vcpu *vcpu, u32 csselr)
> >>>     static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32
> >>> val)
> >>>   {
> >>> -	u8 line_size = FIELD_GET(CCSIDR_EL1_LineSize, val);
> >>> +	u8 line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, val);
> >>> +	u32 cur = get_ccsidr(vcpu, csselr);
> >>> +	u8 min_line_size = SYS_FIELD_GET(CCSIDR_EL1, LineSize, cur);
> >>>   	u32 *ccsidr = vcpu->arch.ccsidr;
> >>>   	u32 i;
> >>>   -	if ((val & CCSIDR_EL1_RES0) || line_size <
> >>> get_min_cache_line_size(csselr))
> >>> +	if (cur == val)
> >>> +		return 0;
> >>> +
> >>> +	if ((val & CCSIDR_EL1_RES0) || line_size < min_line_size)
> >>>   		return -EINVAL;
> >> 
> >> This doesn't look right. You're comparing the value userspace is
> >> trying to set for a given level with the value that is already set for
> >> that level, and forbid the cache line size to be smaller. It works if
> >> no value has been set yet (you fallback to something derived from
> >> CTR_EL0), but this fails if userspace does multiple writes.
> > 
> > Good catch, I tried to skip over the unit/field conversions by doing this
> > but it has the consequence of not working as expected for multiple writes.
> > 
> >> The original check is against CTR_EL0, which makes absolute sense
> >> because we want to check across the whole hierarchy. It is just that
> >> the original code has two bugs:
> >> 
> >> - It fails to convert the CCSIDR_EL1.LineSize value to a number of
> >>    words (the missing +4). Admire how the architecture is actively
> >>    designed to be hostile to SW by providing two different formats for
> >>    the cache line size, none of which is in... bytes.
> >> 
> >> - It passes the full CSSELR value to get_min_cache_line_size(), while
> >>    this function wants a bool... Yes, there are times where you'd want
> >>    a stronger type system (did anyone say Rust? ;-)
> > 
> > Hey now, if you say it enough times people are going to start getting
> > ideas ;-P
> > 
> >> I propose that we fold something like the patch below in instead
> >> (tested with get-reg-list).
> > 
> > Agreed, I've backed out my diff and applied yours. Pushed (with force!)
> > to my repo now, PTAL.
> > 
> > --
> > Thanks,
> > Oliver
> > 
> 
> I was so careless that I missed two bugs and failed to test the last
> version of my patch. It is fortunate that the bugs were caught by
> careful review though we don't have a strong type system (yet). Your
> tree looks good to me.

Don't beat yourself up! You've done a great job for a first arm64
contribution, and we caught the problem during the review process,
which is what it is for.

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 v7 0/7] KVM: arm64: Normalize cache configuration

[Oliver Upton]

On Thu, 12 Jan 2023 11:38:45 +0900, Akihiko Odaki wrote:
> 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.
> 
> [...]

Applied to kvmarm/next, thanks!

[1/7] arm64: Allow the definition of UNKNOWN system register fields
      https://git.kernel.org/kvmarm/kvmarm/c/e2c0b51f1c9d
[2/7] arm64/sysreg: Convert CCSIDR_EL1 to automatic generation
      https://git.kernel.org/kvmarm/kvmarm/c/d1a0eb124c44
[3/7] arm64/sysreg: Add CCSIDR2_EL1
      https://git.kernel.org/kvmarm/kvmarm/c/8f407d6a15f3
[4/7] arm64/cache: Move CLIDR macro definitions
      https://git.kernel.org/kvmarm/kvmarm/c/805e6ec1c5e0
[5/7] KVM: arm64: Always set HCR_TID2
      https://git.kernel.org/kvmarm/kvmarm/c/8cc6dedaff42
[6/7] KVM: arm64: Mask FEAT_CCIDX
      https://git.kernel.org/kvmarm/kvmarm/c/bf48040cd9b0
[7/7] KVM: arm64: Normalize cache configuration
      https://git.kernel.org/kvmarm/kvmarm/c/7af0c2534f4c

--
Best,
Oliver

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 1571 bytes --]

On Thu, 12 Jan 2023 at 11:38:52 +0900, Akihiko Odaki wrote:
> 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 and CCSIDR_EL1 are now writable from the userspace so that
> the VMM can restore the values saved with the old kernel.

(commit 7af0c2534f4c5)

How does the VMM set the values that the old kernel would have set?

Let's say we're deploying a kernel with this change for the first time,
and we need to ensure that we provide a consistent environment to
guests, which can be live migrated back to an older host.

So for new launches, we need to provide the values that the old kernel
*would* have provided to the guest. A new launch isn't a migration;
there are no "values saved with the old kernel".

Userspace can't read the CLIDR_EL1 and CCSIDR_EL1 registers directly,
and AFAICT not everything we need to reconstitute them is in sysfs. How
is this supposed to work?

Shouldn't this change have been made as a capability that the VMM can
explicitly opt in or out of? Environments that don't do cross-CPU
migration absolutely don't care about, and actively don't *want*, the
sanitisation that this commit inflicted on us, surely?

Am I missing something?







[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Marc Zyngier]

On Thu, 09 Apr 2026 13:25:24 +0100,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> On Thu, 12 Jan 2023 at 11:38:52 +0900, Akihiko Odaki wrote:
> > 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 and CCSIDR_EL1 are now writable from the userspace so that
> > the VMM can restore the values saved with the old kernel.
> 
> (commit 7af0c2534f4c5)
> 
> How does the VMM set the values that the old kernel would have set?

By reading them at the source?

> Let's say we're deploying a kernel with this change for the first time,
> and we need to ensure that we provide a consistent environment to
> guests, which can be live migrated back to an older host.

We have never guaranteed host downgrade. It almost never works.

> So for new launches, we need to provide the values that the old kernel
> *would* have provided to the guest. A new launch isn't a migration;
> there are no "values saved with the old kernel".

And you can provide these values.

> 
> Userspace can't read the CLIDR_EL1 and CCSIDR_EL1 registers directly,
> and AFAICT not everything we need to reconstitute them is in sysfs. How
> is this supposed to work?
>
> Shouldn't this change have been made as a capability that the VMM can
> explicitly opt in or out of? Environments that don't do cross-CPU
> migration absolutely don't care about, and actively don't *want*, the
> sanitisation that this commit inflicted on us, surely?

I don't think a capability buys you anything. You want to expose
something to the guest? Make it so. You are in the favourable
situation to completely own the HW and the VMM.

The stuff we are allowing the VMM to change are not directly relevant
to the guest anyway (set/way per cache levels...), and were only
actively breaking things.

> Am I missing something?

That you had over 3 years to voice your concern, and did nothing?

	M.

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 3229 bytes --]

On Thu, 2026-04-09 at 14:36 +0100, Marc Zyngier wrote:
> On Thu, 09 Apr 2026 13:25:24 +0100,
> David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > On Thu, 12 Jan 2023 at 11:38:52 +0900, Akihiko Odaki wrote:
> > > 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 and CCSIDR_EL1 are now writable from the userspace so that
> > > the VMM can restore the values saved with the old kernel.
> > 
> > (commit 7af0c2534f4c5)
> > 
> > How does the VMM set the values that the old kernel would have set?
> 
> By reading them at the source?

Yes, but the VMM in EL0 *can't* read the source, can it?

> > Let's say we're deploying a kernel with this change for the first time,
> > and we need to ensure that we provide a consistent environment to
> > guests, which can be live migrated back to an older host.
> 
> We have never guaranteed host downgrade. It almost never works.

Huh? Host downgrade absolutely *does* work; KVM on Arm isn't a toy.
We'd never be able to ship a new kernel if we didn't know we could roll
it *back* if we needed to.

I *know* that you know perfectly well that we actively test host
downgrades prior to every rollout of a new kernel. So I'm a little
confused about what you're trying to say here, Marc.

> > So for new launches, we need to provide the values that the old kernel
> > *would* have provided to the guest. A new launch isn't a migration;
> > there are no "values saved with the old kernel".
> 
> And you can provide these values.

If I know them, sure. But I don't, because:

> > Userspace can't read the CLIDR_EL1 and CCSIDR_EL1 registers directly,
> > and AFAICT not everything we need to reconstitute them is in sysfs. How
> > is this supposed to work?
> > 
> > Shouldn't this change have been made as a capability that the VMM can
> > explicitly opt in or out of? Environments that don't do cross-CPU
> > migration absolutely don't care about, and actively don't *want*, the
> > sanitisation that this commit inflicted on us, surely?
> 
> I don't think a capability buys you anything. You want to expose
> something to the guest? Make it so. You are in the favourable
> situation to completely own the HW and the VMM.

Now that the values are writable, but userspace can't easily see *what*
they would have been in the previous kernel. So having the capability
to ask KVM to set them to those values seems like it might be useful.

> > Am I missing something?
> 
> That you had over 3 years to voice your concern, and did nothing?

I was looking more for technical input about how I might determine the
original values to retain compatibility... but yes, I have given
feedback that merely *reverting* the offending commit in previous
kernel upgrades without actually doing anything to fix it properly
wasn't the best choice.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

[PATCH] KVM: arm64: Add KVM_CAP_ARM_NATIVE_CACHE_CONFIG vcpu capability

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 8489 bytes --]

From: David Woodhouse <dwmw@amazon.co.uk>

Commit 7af0c2534f4c5 ("KVM: arm64: Normalize cache configuration")
fabricates CLIDR_EL1 and CCSIDR_EL1 values instead of using the real
hardware values. While this provides consistent values across
heterogeneous CPUs, it does cause visible changes in the CPU model
exposed to guests.

The commit claims that userspace can restore the original values, but
there is no way for userspace to obtain the real CLIDR_EL1 register
value — it is not fully reconstructible from sysfs, which lacks the
LoC, LoUU, and LoUIS fields.

Add a per-vcpu KVM_CAP_ARM_NATIVE_CACHE_CONFIG capability that reads
the real CLIDR_EL1 and all CCSIDR_EL1 values from the current physical
CPU and sets them on the vcpu.

This allows hypervisors to present the real hardware cache configuration
to guests, which is important for consistency of the environment across
kernel versions and for migration compatibility with hosts running
older kernels that exposed the real values.

Fixes: 7af0c2534f4c ("KVM: arm64: Normalize cache configuration")
Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
---
 Documentation/virt/kvm/api.rst                | 23 ++++++++
 arch/arm64/include/asm/kvm_host.h             |  1 +
 arch/arm64/kvm/arm.c                          | 17 ++++++
 arch/arm64/kvm/sys_regs.c                     | 26 ++++++++++
 include/uapi/linux/kvm.h                      |  1 +
 tools/testing/selftests/kvm/Makefile.kvm      |  1 +
 .../selftests/kvm/arm64/native_cache_config.c | 52 +++++++++++++++++++
 7 files changed, 121 insertions(+)
 create mode 100644 tools/testing/selftests/kvm/arm64/native_cache_config.c

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index e3b3bd9edeec..ee47dc07ceac 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8930,6 +8930,29 @@ no-op.
 
 ``KVM_CHECK_EXTENSION`` returns the bitmask of exits that can be disabled.
 
+7.48 KVM_CAP_ARM_NATIVE_CACHE_CONFIG
+-------------------------------------
+
+:Architecture: arm64
+:Target: vcpu
+:Parameters: none
+:Returns: 0 on success, -ENOMEM on allocation failure, -EINVAL if
+          args[0] or flags are non-zero.
+
+This per-vcpu capability reads the real CLIDR_EL1 and CCSIDR_EL1 values
+from the physical CPU on which the ioctl is executed, and sets them on
+the vcpu. This replaces the fabricated cache configuration that KVM
+provides by default.
+
+The caller should ensure the vcpu thread is pinned to the desired
+physical CPU before invoking this capability, so that the correct cache
+topology is captured. On heterogeneous systems, different physical CPUs
+may have different cache configurations.
+
+After this capability is enabled, the vcpu's CLIDR_EL1 and CCSIDR_EL1
+values can still be overridden individually via ``KVM_SET_ONE_REG`` and
+the ``KVM_REG_ARM_DEMUX`` interface.
+
 8. Other capabilities.
 ======================
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index a1bb025c641f..c9713a472c47 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1296,6 +1296,7 @@ void kvm_sys_regs_create_debugfs(struct kvm *kvm);
 void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
 
 int __init kvm_sys_reg_table_init(void);
+int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu);
 struct sys_reg_desc;
 int __init populate_sysreg_config(const struct sys_reg_desc *sr,
 				  unsigned int idx);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 326a99fea753..579583e8dc5c 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -393,6 +393,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_ARM_DISABLE_EXITS:
 		r = KVM_ARM_DISABLE_VALID_EXITS;
 		break;
+	case KVM_CAP_ARM_NATIVE_CACHE_CONFIG:
+	case KVM_CAP_ENABLE_CAP:
+		r = 1;
+		break;
 	case KVM_CAP_SET_GUEST_DEBUG2:
 		return KVM_GUESTDBG_VALID_MASK;
 	case KVM_CAP_ARM_SET_DEVICE_ADDR:
@@ -1793,6 +1797,19 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
 		r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
 		break;
 	}
+	case KVM_ENABLE_CAP: {
+		struct kvm_enable_cap cap;
+
+		r = -EFAULT;
+		if (copy_from_user(&cap, argp, sizeof(cap)))
+			break;
+
+		r = -EINVAL;
+		if (cap.cap == KVM_CAP_ARM_NATIVE_CACHE_CONFIG &&
+		    !cap.args[0] && !cap.flags)
+			r = kvm_vcpu_set_native_cache_config(vcpu);
+		break;
+	}
 	case KVM_SET_ONE_REG:
 	case KVM_GET_ONE_REG: {
 		struct kvm_one_reg reg;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1b4cacb6e918..c19d84e48f8b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -484,6 +484,32 @@ static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
 	return 0;
 }
 
+int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu)
+{
+	u32 csselr;
+
+	if (!vcpu->arch.ccsidr) {
+		vcpu->arch.ccsidr = kmalloc_array(CSSELR_MAX, sizeof(u32),
+						  GFP_KERNEL_ACCOUNT);
+		if (!vcpu->arch.ccsidr)
+			return -ENOMEM;
+	}
+
+	local_irq_disable();
+
+	__vcpu_assign_sys_reg(vcpu, CLIDR_EL1, read_sysreg(clidr_el1));
+
+	for (csselr = 0; csselr < CSSELR_MAX; csselr++) {
+		write_sysreg(csselr, csselr_el1);
+		isb();
+		vcpu->arch.ccsidr[csselr] = read_sysreg(ccsidr_el1);
+	}
+
+	local_irq_enable();
+
+	return 0;
+}
+
 static bool access_rw(struct kvm_vcpu *vcpu,
 		      struct sys_reg_params *p,
 		      const struct sys_reg_desc *r)
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 694cf699ed0a..2d8bbb4dd69b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -995,6 +995,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_S390_USER_OPEREXEC 246
 #define KVM_CAP_S390_KEYOP 247
 #define KVM_CAP_ARM_DISABLE_EXITS 248
+#define KVM_CAP_ARM_NATIVE_CACHE_CONFIG 249
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
diff --git a/tools/testing/selftests/kvm/Makefile.kvm b/tools/testing/selftests/kvm/Makefile.kvm
index ad9f9fa181a1..8e05cc8ec204 100644
--- a/tools/testing/selftests/kvm/Makefile.kvm
+++ b/tools/testing/selftests/kvm/Makefile.kvm
@@ -182,6 +182,7 @@ TEST_GEN_PROGS_arm64 += arm64/vgic_group_iidr
 TEST_GEN_PROGS_arm64 += arm64/vgic_group_v2
 TEST_GEN_PROGS_arm64 += arm64/vpmu_counter_access
 TEST_GEN_PROGS_arm64 += arm64/no-vgic-v3
+TEST_GEN_PROGS_arm64 += arm64/native_cache_config
 TEST_GEN_PROGS_arm64 += arm64/idreg-idst
 TEST_GEN_PROGS_arm64 += arm64/kvm-uuid
 TEST_GEN_PROGS_arm64 += access_tracking_perf_test
diff --git a/tools/testing/selftests/kvm/arm64/native_cache_config.c b/tools/testing/selftests/kvm/arm64/native_cache_config.c
new file mode 100644
index 000000000000..4afea32f2348
--- /dev/null
+++ b/tools/testing/selftests/kvm/arm64/native_cache_config.c
@@ -0,0 +1,52 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * native_cache_config.c - Test KVM_CAP_ARM_NATIVE_CACHE_CONFIG
+ *
+ * Verify that enabling the capability populates the vcpu's CLIDR_EL1
+ * with the real hardware value instead of the fabricated default.
+ */
+#include "test_util.h"
+#include "kvm_util.h"
+#include "processor.h"
+
+#define CLIDR_EL1_REG_ID ARM64_SYS_REG(3, 1, 0, 0, 1)
+
+int main(int argc, char *argv[])
+{
+	struct kvm_enable_cap cap = {
+		.cap = KVM_CAP_ARM_NATIVE_CACHE_CONFIG,
+	};
+	struct kvm_vcpu *vcpu;
+	struct kvm_vm *vm;
+	uint64_t clidr_before, clidr_after;
+	int r;
+
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_NATIVE_CACHE_CONFIG));
+
+	vm = vm_create_with_one_vcpu(&vcpu, NULL);
+
+	/* Read the fabricated CLIDR_EL1 */
+	clidr_before = vcpu_get_reg(vcpu, CLIDR_EL1_REG_ID);
+
+	/* Enable native cache config */
+	r = __vcpu_ioctl(vcpu, KVM_ENABLE_CAP, &cap);
+	TEST_ASSERT(!r, "KVM_ENABLE_CAP failed: %d (errno %d)", r, errno);
+
+	/* Read CLIDR_EL1 again */
+	clidr_after = vcpu_get_reg(vcpu, CLIDR_EL1_REG_ID);
+
+	pr_info("CLIDR_EL1 before: 0x%016lx\n", clidr_before);
+	pr_info("CLIDR_EL1 after:  0x%016lx\n", clidr_after);
+
+	TEST_ASSERT(clidr_after != 0,
+		    "CLIDR_EL1 should not be zero after native config");
+
+	/* Invalid: non-zero args should fail */
+	cap.args[0] = 1;
+	r = __vcpu_ioctl(vcpu, KVM_ENABLE_CAP, &cap);
+	TEST_ASSERT(r == -1 && errno == EINVAL,
+		    "Non-zero args should fail: got %d errno %d", r, errno);
+
+	kvm_vm_free(vm);
+	return 0;
+}
-- 
2.43.0



[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[Marc Zyngier]

On Thu, 09 Apr 2026 15:51:59 +0100,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> [1  <text/plain; UTF-8 (quoted-printable)>]
> On Thu, 2026-04-09 at 14:36 +0100, Marc Zyngier wrote:
> > On Thu, 09 Apr 2026 13:25:24 +0100,
> > David Woodhouse <dwmw2@infradead.org> wrote:
> > > 
> > > On Thu, 12 Jan 2023 at 11:38:52 +0900, Akihiko Odaki wrote:
> > > > 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 and CCSIDR_EL1 are now writable from the userspace so that
> > > > the VMM can restore the values saved with the old kernel.
> > > 
> > > (commit 7af0c2534f4c5)
> > > 
> > > How does the VMM set the values that the old kernel would have set?
> > 
> > By reading them at the source?
> 
> Yes, but the VMM in EL0 *can't* read the source, can it?

Again, this is designed for migration, and snapshoting the source
values gives you the expected result.

>
> > > Let's say we're deploying a kernel with this change for the first time,
> > > and we need to ensure that we provide a consistent environment to
> > > guests, which can be live migrated back to an older host.
> > 
> > We have never guaranteed host downgrade. It almost never works.
> 
> Huh? Host downgrade absolutely *does* work; KVM on Arm isn't a toy.
> We'd never be able to ship a new kernel if we didn't know we could roll
> it *back* if we needed to.

You have clearly been lucky so far, because I'd never such claim.

> 
> I *know* that you know perfectly well that we actively test host
> downgrades prior to every rollout of a new kernel. So I'm a little
> confused about what you're trying to say here, Marc.

I said exactly this: we make no effort to ensure that a guest started
on a version of the kernel can be migrated to an older version -- the
state space might be different.

> 
> > > So for new launches, we need to provide the values that the old kernel
> > > *would* have provided to the guest. A new launch isn't a migration;
> > > there are no "values saved with the old kernel".
> > 
> > And you can provide these values.
> 
> If I know them, sure. But I don't, because:
> 
> > > Userspace can't read the CLIDR_EL1 and CCSIDR_EL1 registers directly,
> > > and AFAICT not everything we need to reconstitute them is in sysfs. How
> > > is this supposed to work?
> > > 
> > > Shouldn't this change have been made as a capability that the VMM can
> > > explicitly opt in or out of? Environments that don't do cross-CPU
> > > migration absolutely don't care about, and actively don't *want*, the
> > > sanitisation that this commit inflicted on us, surely?
> > 
> > I don't think a capability buys you anything. You want to expose
> > something to the guest? Make it so. You are in the favourable
> > situation to completely own the HW and the VMM.
> 
> Now that the values are writable, but userspace can't easily see *what*
> they would have been in the previous kernel. So having the capability
> to ask KVM to set them to those values seems like it might be useful.

Well, it's only a patch away.

	M.

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

Re: [PATCH v7 7/7] KVM: arm64: Normalize cache configuration

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 2531 bytes --]

On Thu, 2026-04-09 at 16:45 +0100, Marc Zyngier wrote:
> On Thu, 09 Apr 2026 15:51:59 +0100, David Woodhouse <dwmw2@infradead.org> wrote:
> > On Thu, 2026-04-09 at 14:36 +0100, Marc Zyngier wrote:
> 
> > > We have never guaranteed host downgrade. It almost never works.
> >
> > Huh? Host downgrade absolutely *does* work; KVM on Arm isn't a toy.
> > We'd never be able to ship a new kernel if we didn't know we could roll
> > it *back* if we needed to.
> 
> You have clearly been lucky so far, because I'd never such claim.

It's not luck. We do a *lot* of testing, both within the guest checking
that what it sees is identical between old and new environments, and
repeated live upgrade/downgrade cycles between versions as well as
migration back and forth, and hibernate/resume from one to the other.

> > 
> > I *know* that you know perfectly well that we actively test host
> > downgrades prior to every rollout of a new kernel. So I'm a little
> > confused about what you're trying to say here, Marc.
> 
> I said exactly this: we make no effort to ensure that a guest started
> on a version of the kernel can be migrated to an older version -- the
> state space might be different.

You said "it almost never works". If that were the case, then I would
suggest that KVM on arm64 would be entirely unsuitable as a production
platform for virtual hosting. But thankfully it isn't my experience at
all.

Sure, we've occasionally found breakage, such as this commit and the
vGIC 'IMP_REV_1' thing being discussed in a separate thread, but those
are the exceptions rather than the norm.

(And yes, we need to do better than just reverting the offending
commits when we find them, and fix the issues properly upstream. Which
is what I'm doing now, belatedly).

> > 
> > Now that the values are writable, but userspace can't easily see *what*
> > they would have been in the previous kernel. So having the capability
> > to ask KVM to set them to those values seems like it might be useful.
> 
> Well, it's only a patch away.

Indeed so. When I asked what I was missing, I was *kind* of hoping
someone would tell me I'm wrong and that I can get the original values
from userspace either through sysfs or some other means, but in the
absence of that:
https://lore.kernel.org/lkml/7fb7b823c68e04321eb532a5b8ae21a818d4926d.camel@infradead.org/

(Lore is being insanely slow today but it *will* be there, and the
marc.info first alternative it offers does already have the message).

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH] KVM: arm64: Add KVM_CAP_ARM_NATIVE_CACHE_CONFIG vcpu capability

[Marc Zyngier]

On Thu, 09 Apr 2026 16:29:06 +0100,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> [1  <text/plain; UTF-8 (quoted-printable)>]
> From: David Woodhouse <dwmw@amazon.co.uk>
> 
> Commit 7af0c2534f4c5 ("KVM: arm64: Normalize cache configuration")
> fabricates CLIDR_EL1 and CCSIDR_EL1 values instead of using the real
> hardware values. While this provides consistent values across
> heterogeneous CPUs, it does cause visible changes in the CPU model
> exposed to guests.
> 
> The commit claims that userspace can restore the original values, but
> there is no way for userspace to obtain the real CLIDR_EL1 register
> value — it is not fully reconstructible from sysfs, which lacks the
> LoC, LoUU, and LoUIS fields.
> 
> Add a per-vcpu KVM_CAP_ARM_NATIVE_CACHE_CONFIG capability that reads
> the real CLIDR_EL1 and all CCSIDR_EL1 values from the current physical
> CPU and sets them on the vcpu.
> 
> This allows hypervisors to present the real hardware cache configuration
> to guests, which is important for consistency of the environment across
> kernel versions and for migration compatibility with hosts running
> older kernels that exposed the real values.
> 
> Fixes: 7af0c2534f4c ("KVM: arm64: Normalize cache configuration")
> Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> ---
>  Documentation/virt/kvm/api.rst                | 23 ++++++++
>  arch/arm64/include/asm/kvm_host.h             |  1 +
>  arch/arm64/kvm/arm.c                          | 17 ++++++
>  arch/arm64/kvm/sys_regs.c                     | 26 ++++++++++
>  include/uapi/linux/kvm.h                      |  1 +
>  tools/testing/selftests/kvm/Makefile.kvm      |  1 +
>  .../selftests/kvm/arm64/native_cache_config.c | 52 +++++++++++++++++++
>  7 files changed, 121 insertions(+)
>  create mode 100644 tools/testing/selftests/kvm/arm64/native_cache_config.c
> 
> diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> index e3b3bd9edeec..ee47dc07ceac 100644
> --- a/Documentation/virt/kvm/api.rst
> +++ b/Documentation/virt/kvm/api.rst
> @@ -8930,6 +8930,29 @@ no-op.
>  
>  ``KVM_CHECK_EXTENSION`` returns the bitmask of exits that can be disabled.
>  
> +7.48 KVM_CAP_ARM_NATIVE_CACHE_CONFIG
> +-------------------------------------
> +
> +:Architecture: arm64
> +:Target: vcpu
> +:Parameters: none
> +:Returns: 0 on success, -ENOMEM on allocation failure, -EINVAL if
> +          args[0] or flags are non-zero.
> +
> +This per-vcpu capability reads the real CLIDR_EL1 and CCSIDR_EL1 values
> +from the physical CPU on which the ioctl is executed, and sets them on
> +the vcpu. This replaces the fabricated cache configuration that KVM
> +provides by default.
> +
> +The caller should ensure the vcpu thread is pinned to the desired
> +physical CPU before invoking this capability, so that the correct cache
> +topology is captured. On heterogeneous systems, different physical CPUs
> +may have different cache configurations.
> +
> +After this capability is enabled, the vcpu's CLIDR_EL1 and CCSIDR_EL1
> +values can still be overridden individually via ``KVM_SET_ONE_REG`` and
> +the ``KVM_REG_ARM_DEMUX`` interface.
> +
>  8. Other capabilities.
>  ======================
>  
> diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> index a1bb025c641f..c9713a472c47 100644
> --- a/arch/arm64/include/asm/kvm_host.h
> +++ b/arch/arm64/include/asm/kvm_host.h
> @@ -1296,6 +1296,7 @@ void kvm_sys_regs_create_debugfs(struct kvm *kvm);
>  void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
>  
>  int __init kvm_sys_reg_table_init(void);
> +int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu);
>  struct sys_reg_desc;
>  int __init populate_sysreg_config(const struct sys_reg_desc *sr,
>  				  unsigned int idx);
> diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> index 326a99fea753..579583e8dc5c 100644
> --- a/arch/arm64/kvm/arm.c
> +++ b/arch/arm64/kvm/arm.c
> @@ -393,6 +393,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
>  	case KVM_CAP_ARM_DISABLE_EXITS:
>  		r = KVM_ARM_DISABLE_VALID_EXITS;
>  		break;
> +	case KVM_CAP_ARM_NATIVE_CACHE_CONFIG:
> +	case KVM_CAP_ENABLE_CAP:
> +		r = 1;
> +		break;
>  	case KVM_CAP_SET_GUEST_DEBUG2:
>  		return KVM_GUESTDBG_VALID_MASK;
>  	case KVM_CAP_ARM_SET_DEVICE_ADDR:
> @@ -1793,6 +1797,19 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
>  		r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
>  		break;
>  	}
> +	case KVM_ENABLE_CAP: {
> +		struct kvm_enable_cap cap;
> +
> +		r = -EFAULT;
> +		if (copy_from_user(&cap, argp, sizeof(cap)))
> +			break;
> +
> +		r = -EINVAL;
> +		if (cap.cap == KVM_CAP_ARM_NATIVE_CACHE_CONFIG &&
> +		    !cap.args[0] && !cap.flags)
> +			r = kvm_vcpu_set_native_cache_config(vcpu);
> +		break;
> +	}
>  	case KVM_SET_ONE_REG:
>  	case KVM_GET_ONE_REG: {
>  		struct kvm_one_reg reg;
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1b4cacb6e918..c19d84e48f8b 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -484,6 +484,32 @@ static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
>  	return 0;
>  }
>  
> +int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu)
> +{
> +	u32 csselr;
> +
> +	if (!vcpu->arch.ccsidr) {
> +		vcpu->arch.ccsidr = kmalloc_array(CSSELR_MAX, sizeof(u32),
> +						  GFP_KERNEL_ACCOUNT);
> +		if (!vcpu->arch.ccsidr)
> +			return -ENOMEM;
> +	}

Well, no.

The moment you decide to expose all of the host's crap, you really
need to put everything on the table. It means fully handling
FEAT_CCIDX, which we were careful not to expose anywhere because it is
a terrible idea.

So CCSIDR_EL1 becomes a 64bit value, is complemented with CCSIDR2_EL1,
and needs to be advertised as such through the idregs.  CCSIDR2_EL1
must be exposed to userspace and made writable, but only if the
feature exists. You also need to conditionally undef CCSIDR2_EL1
depending on the VM configuration.

The "amusing" thing is that, before we introduced the cache hierarchy
sanitisation, we would happily report something completely senseless
on CCIDX hardware. It didn't matter, because nobody can make any use
of that information, apart from EL3 firmware.

But if you want this to be a reflection of the underlying HW, then so
be it.

> +	for (csselr = 0; csselr < CSSELR_MAX; csselr++) {
> +		write_sysreg(csselr, csselr_el1);
> +		isb();
> +		vcpu->arch.ccsidr[csselr] = read_sysreg(ccsidr_el1);

That's not how the selection register works. CLIDR_EL1 tells you what
each cache level is (Instructions, Data, Unified, Tags), and that must
be combined with the index (which doesn't start at bit 0).

I also wonder how you reconcile not exposing MTE when the cache
hierarchy indicate support for tags. That clearly contradicts "report
what the HW has".

	M.

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

Re: [PATCH] KVM: arm64: Add KVM_CAP_ARM_NATIVE_CACHE_CONFIG vcpu capability

[David Woodhouse]

[-- Attachment #1: Type: text/plain, Size: 7534 bytes --]

On Thu, 2026-04-09 at 18:07 +0100, Marc Zyngier wrote:
> On Thu, 09 Apr 2026 16:29:06 +0100,
> David Woodhouse <dwmw2@infradead.org> wrote:
> > 
> > [1  <text/plain; UTF-8 (quoted-printable)>]
> > From: David Woodhouse <dwmw@amazon.co.uk>
> > 
> > Commit 7af0c2534f4c5 ("KVM: arm64: Normalize cache configuration")
> > fabricates CLIDR_EL1 and CCSIDR_EL1 values instead of using the real
> > hardware values. While this provides consistent values across
> > heterogeneous CPUs, it does cause visible changes in the CPU model
> > exposed to guests.
> > 
> > The commit claims that userspace can restore the original values, but
> > there is no way for userspace to obtain the real CLIDR_EL1 register
> > value — it is not fully reconstructible from sysfs, which lacks the
> > LoC, LoUU, and LoUIS fields.
> > 
> > Add a per-vcpu KVM_CAP_ARM_NATIVE_CACHE_CONFIG capability that reads
> > the real CLIDR_EL1 and all CCSIDR_EL1 values from the current physical
> > CPU and sets them on the vcpu.
> > 
> > This allows hypervisors to present the real hardware cache configuration
> > to guests, which is important for consistency of the environment across
> > kernel versions and for migration compatibility with hosts running
> > older kernels that exposed the real values.
> > 
> > Fixes: 7af0c2534f4c ("KVM: arm64: Normalize cache configuration")
> > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > ---
> >  Documentation/virt/kvm/api.rst                | 23 ++++++++
> >  arch/arm64/include/asm/kvm_host.h             |  1 +
> >  arch/arm64/kvm/arm.c                          | 17 ++++++
> >  arch/arm64/kvm/sys_regs.c                     | 26 ++++++++++
> >  include/uapi/linux/kvm.h                      |  1 +
> >  tools/testing/selftests/kvm/Makefile.kvm      |  1 +
> >  .../selftests/kvm/arm64/native_cache_config.c | 52 +++++++++++++++++++
> >  7 files changed, 121 insertions(+)
> >  create mode 100644 tools/testing/selftests/kvm/arm64/native_cache_config.c
> > 
> > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > index e3b3bd9edeec..ee47dc07ceac 100644
> > --- a/Documentation/virt/kvm/api.rst
> > +++ b/Documentation/virt/kvm/api.rst
> > @@ -8930,6 +8930,29 @@ no-op.
> >  
> >  ``KVM_CHECK_EXTENSION`` returns the bitmask of exits that can be disabled.
> >  
> > +7.48 KVM_CAP_ARM_NATIVE_CACHE_CONFIG
> > +-------------------------------------
> > +
> > +:Architecture: arm64
> > +:Target: vcpu
> > +:Parameters: none
> > +:Returns: 0 on success, -ENOMEM on allocation failure, -EINVAL if
> > +          args[0] or flags are non-zero.
> > +
> > +This per-vcpu capability reads the real CLIDR_EL1 and CCSIDR_EL1 values
> > +from the physical CPU on which the ioctl is executed, and sets them on
> > +the vcpu. This replaces the fabricated cache configuration that KVM
> > +provides by default.
> > +
> > +The caller should ensure the vcpu thread is pinned to the desired
> > +physical CPU before invoking this capability, so that the correct cache
> > +topology is captured. On heterogeneous systems, different physical CPUs
> > +may have different cache configurations.
> > +
> > +After this capability is enabled, the vcpu's CLIDR_EL1 and CCSIDR_EL1
> > +values can still be overridden individually via ``KVM_SET_ONE_REG`` and
> > +the ``KVM_REG_ARM_DEMUX`` interface.
> > +
> >  8. Other capabilities.
> >  ======================
> >  
> > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > index a1bb025c641f..c9713a472c47 100644
> > --- a/arch/arm64/include/asm/kvm_host.h
> > +++ b/arch/arm64/include/asm/kvm_host.h
> > @@ -1296,6 +1296,7 @@ void kvm_sys_regs_create_debugfs(struct kvm *kvm);
> >  void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
> >  
> >  int __init kvm_sys_reg_table_init(void);
> > +int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu);
> >  struct sys_reg_desc;
> >  int __init populate_sysreg_config(const struct sys_reg_desc *sr,
> >  				  unsigned int idx);
> > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > index 326a99fea753..579583e8dc5c 100644
> > --- a/arch/arm64/kvm/arm.c
> > +++ b/arch/arm64/kvm/arm.c
> > @@ -393,6 +393,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> >  	case KVM_CAP_ARM_DISABLE_EXITS:
> >  		r = KVM_ARM_DISABLE_VALID_EXITS;
> >  		break;
> > +	case KVM_CAP_ARM_NATIVE_CACHE_CONFIG:
> > +	case KVM_CAP_ENABLE_CAP:
> > +		r = 1;
> > +		break;
> >  	case KVM_CAP_SET_GUEST_DEBUG2:
> >  		return KVM_GUESTDBG_VALID_MASK;
> >  	case KVM_CAP_ARM_SET_DEVICE_ADDR:
> > @@ -1793,6 +1797,19 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> >  		r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
> >  		break;
> >  	}
> > +	case KVM_ENABLE_CAP: {
> > +		struct kvm_enable_cap cap;
> > +
> > +		r = -EFAULT;
> > +		if (copy_from_user(&cap, argp, sizeof(cap)))
> > +			break;
> > +
> > +		r = -EINVAL;
> > +		if (cap.cap == KVM_CAP_ARM_NATIVE_CACHE_CONFIG &&
> > +		    !cap.args[0] && !cap.flags)
> > +			r = kvm_vcpu_set_native_cache_config(vcpu);
> > +		break;
> > +	}
> >  	case KVM_SET_ONE_REG:
> >  	case KVM_GET_ONE_REG: {
> >  		struct kvm_one_reg reg;
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 1b4cacb6e918..c19d84e48f8b 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -484,6 +484,32 @@ static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
> >  	return 0;
> >  }
> >  
> > +int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu)
> > +{
> > +	u32 csselr;
> > +
> > +	if (!vcpu->arch.ccsidr) {
> > +		vcpu->arch.ccsidr = kmalloc_array(CSSELR_MAX, sizeof(u32),
> > +						  GFP_KERNEL_ACCOUNT);
> > +		if (!vcpu->arch.ccsidr)
> > +			return -ENOMEM;
> > +	}
> 
> Well, no.
> 
> The moment you decide to expose all of the host's crap, you really
> need to put everything on the table. It means fully handling
> FEAT_CCIDX, which we were careful not to expose anywhere because it is
> a terrible idea.

The intent here is not to "expose all of the host's crap", but to
maintain compatibility with what the kernel did before commit
7af0c2534f4c. No need to expose FEAT_CCIDX.

> > +	for (csselr = 0; csselr < CSSELR_MAX; csselr++) {
> > +		write_sysreg(csselr, csselr_el1);
> > +		isb();
> > +		vcpu->arch.ccsidr[csselr] = read_sysreg(ccsidr_el1);
> 
> That's not how the selection register works. CLIDR_EL1 tells you what
> each cache level is (Instructions, Data, Unified, Tags), and that must
> be combined with the index (which doesn't start at bit 0).

Ack, thanks. I'll rework that based on the old is_valid_cache()
function.

> I also wonder how you reconcile not exposing MTE when the cache
> hierarchy indicate support for tags. That clearly contradicts "report
> what the HW has".

If that was an issue then it would already have been an issue before
commit 7af0c2534f4 (and in kernels with that commit reverted), hosting
millions of guests today.

This isn't about introducing *new* behaviour; it's about allowing the
existing established behaviour to be maintained so that we can have a
*managed* transition to the new model (for new launches) rather than an
unconditional uncontrolled change as the kernel gets upgraded.

[-- Attachment #2: smime.p7s --]
[-- Type: application/pkcs7-signature, Size: 5069 bytes --]

Re: [PATCH] KVM: arm64: Add KVM_CAP_ARM_NATIVE_CACHE_CONFIG vcpu capability

[Marc Zyngier]

On Thu, 09 Apr 2026 18:49:09 +0100,
David Woodhouse <dwmw2@infradead.org> wrote:
> 
> [1  <text/plain; UTF-8 (quoted-printable)>]
> On Thu, 2026-04-09 at 18:07 +0100, Marc Zyngier wrote:
> > On Thu, 09 Apr 2026 16:29:06 +0100,
> > David Woodhouse <dwmw2@infradead.org> wrote:
> > > 
> > > [1  <text/plain; UTF-8 (quoted-printable)>]
> > > From: David Woodhouse <dwmw@amazon.co.uk>
> > > 
> > > Commit 7af0c2534f4c5 ("KVM: arm64: Normalize cache configuration")
> > > fabricates CLIDR_EL1 and CCSIDR_EL1 values instead of using the real
> > > hardware values. While this provides consistent values across
> > > heterogeneous CPUs, it does cause visible changes in the CPU model
> > > exposed to guests.
> > > 
> > > The commit claims that userspace can restore the original values, but
> > > there is no way for userspace to obtain the real CLIDR_EL1 register
> > > value — it is not fully reconstructible from sysfs, which lacks the
> > > LoC, LoUU, and LoUIS fields.
> > > 
> > > Add a per-vcpu KVM_CAP_ARM_NATIVE_CACHE_CONFIG capability that reads
> > > the real CLIDR_EL1 and all CCSIDR_EL1 values from the current physical
> > > CPU and sets them on the vcpu.
> > > 
> > > This allows hypervisors to present the real hardware cache configuration
> > > to guests, which is important for consistency of the environment across
> > > kernel versions and for migration compatibility with hosts running
> > > older kernels that exposed the real values.
> > > 
> > > Fixes: 7af0c2534f4c ("KVM: arm64: Normalize cache configuration")
> > > Signed-off-by: David Woodhouse <dwmw@amazon.co.uk>
> > > ---
> > >  Documentation/virt/kvm/api.rst                | 23 ++++++++
> > >  arch/arm64/include/asm/kvm_host.h             |  1 +
> > >  arch/arm64/kvm/arm.c                          | 17 ++++++
> > >  arch/arm64/kvm/sys_regs.c                     | 26 ++++++++++
> > >  include/uapi/linux/kvm.h                      |  1 +
> > >  tools/testing/selftests/kvm/Makefile.kvm      |  1 +
> > >  .../selftests/kvm/arm64/native_cache_config.c | 52 +++++++++++++++++++
> > >  7 files changed, 121 insertions(+)
> > >  create mode 100644 tools/testing/selftests/kvm/arm64/native_cache_config.c
> > > 
> > > diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
> > > index e3b3bd9edeec..ee47dc07ceac 100644
> > > --- a/Documentation/virt/kvm/api.rst
> > > +++ b/Documentation/virt/kvm/api.rst
> > > @@ -8930,6 +8930,29 @@ no-op.
> > >  
> > >  ``KVM_CHECK_EXTENSION`` returns the bitmask of exits that can be disabled.
> > >  
> > > +7.48 KVM_CAP_ARM_NATIVE_CACHE_CONFIG
> > > +-------------------------------------
> > > +
> > > +:Architecture: arm64
> > > +:Target: vcpu
> > > +:Parameters: none
> > > +:Returns: 0 on success, -ENOMEM on allocation failure, -EINVAL if
> > > +          args[0] or flags are non-zero.
> > > +
> > > +This per-vcpu capability reads the real CLIDR_EL1 and CCSIDR_EL1 values
> > > +from the physical CPU on which the ioctl is executed, and sets them on
> > > +the vcpu. This replaces the fabricated cache configuration that KVM
> > > +provides by default.
> > > +
> > > +The caller should ensure the vcpu thread is pinned to the desired
> > > +physical CPU before invoking this capability, so that the correct cache
> > > +topology is captured. On heterogeneous systems, different physical CPUs
> > > +may have different cache configurations.
> > > +
> > > +After this capability is enabled, the vcpu's CLIDR_EL1 and CCSIDR_EL1
> > > +values can still be overridden individually via ``KVM_SET_ONE_REG`` and
> > > +the ``KVM_REG_ARM_DEMUX`` interface.
> > > +
> > >  8. Other capabilities.
> > >  ======================
> > >  
> > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
> > > index a1bb025c641f..c9713a472c47 100644
> > > --- a/arch/arm64/include/asm/kvm_host.h
> > > +++ b/arch/arm64/include/asm/kvm_host.h
> > > @@ -1296,6 +1296,7 @@ void kvm_sys_regs_create_debugfs(struct kvm *kvm);
> > >  void kvm_reset_sys_regs(struct kvm_vcpu *vcpu);
> > >  
> > >  int __init kvm_sys_reg_table_init(void);
> > > +int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu);
> > >  struct sys_reg_desc;
> > >  int __init populate_sysreg_config(const struct sys_reg_desc *sr,
> > >  				  unsigned int idx);
> > > diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
> > > index 326a99fea753..579583e8dc5c 100644
> > > --- a/arch/arm64/kvm/arm.c
> > > +++ b/arch/arm64/kvm/arm.c
> > > @@ -393,6 +393,10 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
> > >  	case KVM_CAP_ARM_DISABLE_EXITS:
> > >  		r = KVM_ARM_DISABLE_VALID_EXITS;
> > >  		break;
> > > +	case KVM_CAP_ARM_NATIVE_CACHE_CONFIG:
> > > +	case KVM_CAP_ENABLE_CAP:
> > > +		r = 1;
> > > +		break;
> > >  	case KVM_CAP_SET_GUEST_DEBUG2:
> > >  		return KVM_GUESTDBG_VALID_MASK;
> > >  	case KVM_CAP_ARM_SET_DEVICE_ADDR:
> > > @@ -1793,6 +1797,19 @@ long kvm_arch_vcpu_ioctl(struct file *filp,
> > >  		r = kvm_arch_vcpu_ioctl_vcpu_init(vcpu, &init);
> > >  		break;
> > >  	}
> > > +	case KVM_ENABLE_CAP: {
> > > +		struct kvm_enable_cap cap;
> > > +
> > > +		r = -EFAULT;
> > > +		if (copy_from_user(&cap, argp, sizeof(cap)))
> > > +			break;
> > > +
> > > +		r = -EINVAL;
> > > +		if (cap.cap == KVM_CAP_ARM_NATIVE_CACHE_CONFIG &&
> > > +		    !cap.args[0] && !cap.flags)
> > > +			r = kvm_vcpu_set_native_cache_config(vcpu);
> > > +		break;
> > > +	}
> > >  	case KVM_SET_ONE_REG:
> > >  	case KVM_GET_ONE_REG: {
> > >  		struct kvm_one_reg reg;
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 1b4cacb6e918..c19d84e48f8b 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -484,6 +484,32 @@ static int set_ccsidr(struct kvm_vcpu *vcpu, u32 csselr, u32 val)
> > >  	return 0;
> > >  }
> > >  
> > > +int kvm_vcpu_set_native_cache_config(struct kvm_vcpu *vcpu)
> > > +{
> > > +	u32 csselr;
> > > +
> > > +	if (!vcpu->arch.ccsidr) {
> > > +		vcpu->arch.ccsidr = kmalloc_array(CSSELR_MAX, sizeof(u32),
> > > +						  GFP_KERNEL_ACCOUNT);
> > > +		if (!vcpu->arch.ccsidr)
> > > +			return -ENOMEM;
> > > +	}
> > 
> > Well, no.
> > 
> > The moment you decide to expose all of the host's crap, you really
> > need to put everything on the table. It means fully handling
> > FEAT_CCIDX, which we were careful not to expose anywhere because it is
> > a terrible idea.
> 
> The intent here is not to "expose all of the host's crap", but to
> maintain compatibility with what the kernel did before commit
> 7af0c2534f4c. No need to expose FEAT_CCIDX.

That's not optional. Without FEAT_CCIDX, the guest cannot interpret
the correct cache geometry.

> 
> > > +	for (csselr = 0; csselr < CSSELR_MAX; csselr++) {
> > > +		write_sysreg(csselr, csselr_el1);
> > > +		isb();
> > > +		vcpu->arch.ccsidr[csselr] = read_sysreg(ccsidr_el1);
> > 
> > That's not how the selection register works. CLIDR_EL1 tells you what
> > each cache level is (Instructions, Data, Unified, Tags), and that must
> > be combined with the index (which doesn't start at bit 0).
> 
> Ack, thanks. I'll rework that based on the old is_valid_cache()
> function.
> 
> > I also wonder how you reconcile not exposing MTE when the cache
> > hierarchy indicate support for tags. That clearly contradicts "report
> > what the HW has".
> 
> If that was an issue then it would already have been an issue before
> commit 7af0c2534f4 (and in kernels with that commit reverted), hosting
> millions of guests today.

That only means you are doing a pretty bad job at supporting
guests. And yes, this is an issue for anything that expects to see
something meaningful in CCSIDR[]. The fact that none of your guests
hit that problem only means you're lacking coverage.

From what I can read, anything from Neoverse V1 is affected.

> 
> This isn't about introducing *new* behaviour; it's about allowing the
> existing established behaviour to be maintained so that we can have a
> *managed* transition to the new model (for new launches) rather than an
> unconditional uncontrolled change as the kernel gets upgraded.

Then fully implement "show me the cache hierarchy", read it out, and
write it back with whatever level of brokenness you intend to inflict
on your guests.

But I'm not reintroducing this particular bug.

	M.

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