[PATCH v5 00/10] KVM: arm64: Allow userspace to modify CTR_EL0

[PATCH v5 00/10] KVM: arm64: Allow userspace to modify CTR_EL0

[Oliver Upton]

As I'd mentioned on the list, here is my rework of Sebastian's CTR_EL0
series. Changes this time around:

 - Drop the cross-validation of the guest's CTR_EL0 with CLIDR_EL1 and
   the CCSIDR_EL1 hierarchy, instead independently checking these
   registers against the system's CTR_EL0 value.

 - Rework the idregs debugfs interface to print all VM scoped feature ID
   registers, which now includes CTR_EL0.

 - Only reset the VM scoped value of CTR_EL0 once for a VM

 - Make feature ID register accesses go through read / write helpers,
   with the intention of abstracting the layout of the registers +
   adding sanity checks to writers.

What I didn't do after all is come up with a better generic way to store
ID registers at the VM scope, but the hope is that the accessors will
make that trivial to change in the future.

Oliver Upton (5):
  KVM: arm64: Get sys_reg encoding from descriptor in
    idregs_debug_show()
  KVM: arm64: Make idregs debugfs iterator search sysreg table directly
  KVM: arm64: Use read-only helper for reading VM ID registers
  KVM: arm64: Add helper for writing ID regs
  KVM: arm64: nv: Use accessors for modifying ID registers

Sebastian Ott (5):
  KVM: arm64: unify code to prepare traps
  KVM: arm64: Treat CTR_EL0 as a VM feature ID register
  KVM: arm64: show writable masks for feature registers
  KVM: arm64: rename functions for invariant sys regs
  KVM: selftests: arm64: Test writes to CTR_EL0

 arch/arm64/include/asm/kvm_emulate.h          |  40 +--
 arch/arm64/include/asm/kvm_host.h             |  26 +-
 arch/arm64/kvm/arm.c                          |   2 +-
 arch/arm64/kvm/nested.c                       | 256 +++++++++---------
 arch/arm64/kvm/pmu-emul.c                     |   2 +-
 arch/arm64/kvm/sys_regs.c                     | 140 ++++++----
 .../selftests/kvm/aarch64/set_id_regs.c       |  16 ++
 7 files changed, 262 insertions(+), 220 deletions(-)


base-commit: 1613e604df0cd359cf2a7fbd9be7a0bcfacfabd0
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 01/10] KVM: arm64: Get sys_reg encoding from descriptor in idregs_debug_show()

[Oliver Upton]

KVM is about to add support for more VM-scoped feature ID regs that
live outside of the id_regs[] array, which means the index of the
debugfs iterator may not actually be an index into the array.

Prepare by getting the sys_reg encoding from the descriptor itself.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 22b45a15d068..ad453c7ad6cc 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3502,7 +3502,7 @@ static int idregs_debug_show(struct seq_file *s, void *v)
 		return 0;
 
 	seq_printf(s, "%20s:\t%016llx\n",
-		   desc->name, IDREG(kvm, IDX_IDREG(kvm->arch.idreg_debugfs_iter)));
+		   desc->name, IDREG(kvm, reg_to_encoding(desc)));
 
 	return 0;
 }
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 02/10] KVM: arm64: Make idregs debugfs iterator search sysreg table directly

[Oliver Upton]

CTR_EL0 complicates the existing scheme for iterating feature ID
registers, as it is not in the contiguous range that we presently
support. Just search the sysreg table for the Nth feature ID register in
anticipation of this. Yes, the debugfs interface has quadratic time
completixy now. Boo hoo.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 36 +++++++++++++++++++++++-------------
 1 file changed, 23 insertions(+), 13 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index ad453c7ad6cc..1036f865c826 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2753,8 +2753,6 @@ static struct sys_reg_desc sys_insn_descs[] = {
 	{ SYS_DESC(SYS_DC_CIGDSW), access_dcgsw },
 };
 
-static const struct sys_reg_desc *first_idreg;
-
 static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
 			struct sys_reg_params *p,
 			const struct sys_reg_desc *r)
@@ -3440,6 +3438,25 @@ static bool emulate_sys_reg(struct kvm_vcpu *vcpu,
 	return false;
 }
 
+static const struct sys_reg_desc *idregs_debug_find(struct kvm *kvm, u8 pos)
+{
+	unsigned long i, idreg_idx = 0;
+
+	for (i = 0; i < ARRAY_SIZE(sys_reg_descs); i++) {
+		const struct sys_reg_desc *r = &sys_reg_descs[i];
+
+		if (!is_vm_ftr_id_reg(reg_to_encoding(r)))
+			continue;
+
+		if (idreg_idx == pos)
+			return r;
+
+		idreg_idx++;
+	}
+
+	return NULL;
+}
+
 static void *idregs_debug_start(struct seq_file *s, loff_t *pos)
 {
 	struct kvm *kvm = s->private;
@@ -3451,7 +3468,7 @@ static void *idregs_debug_start(struct seq_file *s, loff_t *pos)
 	if (test_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags) &&
 	    *iter == (u8)~0) {
 		*iter = *pos;
-		if (*iter >= KVM_ARM_ID_REG_NUM)
+		if (!idregs_debug_find(kvm, *iter))
 			iter = NULL;
 	} else {
 		iter = ERR_PTR(-EBUSY);
@@ -3468,7 +3485,7 @@ static void *idregs_debug_next(struct seq_file *s, void *v, loff_t *pos)
 
 	(*pos)++;
 
-	if ((kvm->arch.idreg_debugfs_iter + 1) < KVM_ARM_ID_REG_NUM) {
+	if (idregs_debug_find(kvm, kvm->arch.idreg_debugfs_iter + 1)) {
 		kvm->arch.idreg_debugfs_iter++;
 
 		return &kvm->arch.idreg_debugfs_iter;
@@ -3493,10 +3510,10 @@ static void idregs_debug_stop(struct seq_file *s, void *v)
 
 static int idregs_debug_show(struct seq_file *s, void *v)
 {
-	struct kvm *kvm = s->private;
 	const struct sys_reg_desc *desc;
+	struct kvm *kvm = s->private;
 
-	desc = first_idreg + kvm->arch.idreg_debugfs_iter;
+	desc = idregs_debug_find(kvm, kvm->arch.idreg_debugfs_iter);
 
 	if (!desc->name)
 		return 0;
@@ -4115,7 +4132,6 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
 
 int __init kvm_sys_reg_table_init(void)
 {
-	struct sys_reg_params params;
 	bool valid = true;
 	unsigned int i;
 	int ret = 0;
@@ -4136,12 +4152,6 @@ int __init 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]);
 
-	/* Find the first idreg (SYS_ID_PFR0_EL1) in sys_reg_descs. */
-	params = encoding_to_params(SYS_ID_PFR0_EL1);
-	first_idreg = find_reg(&params, sys_reg_descs, ARRAY_SIZE(sys_reg_descs));
-	if (!first_idreg)
-		return -EINVAL;
-
 	ret = populate_nv_trap_config();
 
 	for (i = 0; !ret && i < ARRAY_SIZE(sys_reg_descs); i++)
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 03/10] KVM: arm64: Use read-only helper for reading VM ID registers

[Oliver Upton]

IDREG() expands to the storage of a particular ID reg, which can be
useful for handling both reads and writes. However, outside of a select
few situations, the ID registers should be considered read only.

Replace current readers with a new macro that expands to the value of
the field rather than the field itself.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h | 16 +++++++++++++++-
 arch/arm64/kvm/pmu-emul.c         |  2 +-
 arch/arm64/kvm/sys_regs.c         |  6 +++---
 3 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 8170c04fde91..1201af636551 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1332,6 +1332,20 @@ static inline void kvm_hyp_reserve(void) { }
 void kvm_arm_vcpu_power_off(struct kvm_vcpu *vcpu);
 bool kvm_arm_vcpu_stopped(struct kvm_vcpu *vcpu);
 
+static inline u64 *__vm_id_reg(struct kvm_arch *ka, u32 reg)
+{
+	switch (reg) {
+	case sys_reg(3, 0, 0, 1, 0) ... sys_reg(3, 0, 0, 7, 7):
+		return &ka->id_regs[IDREG_IDX(reg)];
+	default:
+		WARN_ON_ONCE(1);
+		return NULL;
+	}
+}
+
+#define kvm_read_vm_id_reg(kvm, reg)					\
+	({ u64 __val = *__vm_id_reg(&(kvm)->arch, reg); __val; })
+
 #define __expand_field_sign_unsigned(id, fld, val)			\
 	((u64)SYS_FIELD_VALUE(id, fld, val))
 
@@ -1348,7 +1362,7 @@ bool kvm_arm_vcpu_stopped(struct kvm_vcpu *vcpu);
 
 #define get_idreg_field_unsigned(kvm, id, fld)				\
 	({								\
-		u64 __val = IDREG((kvm), SYS_##id);			\
+		u64 __val = kvm_read_vm_id_reg((kvm), SYS_##id);	\
 		FIELD_GET(id##_##fld##_MASK, __val);			\
 	})
 
diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
index a35ce10e0a9f..7848daeafd03 100644
--- a/arch/arm64/kvm/pmu-emul.c
+++ b/arch/arm64/kvm/pmu-emul.c
@@ -54,7 +54,7 @@ static u32 __kvm_pmu_event_mask(unsigned int pmuver)
 
 static u32 kvm_pmu_event_mask(struct kvm *kvm)
 {
-	u64 dfr0 = IDREG(kvm, SYS_ID_AA64DFR0_EL1);
+	u64 dfr0 = kvm_read_vm_id_reg(kvm, SYS_ID_AA64DFR0_EL1);
 	u8 pmuver = SYS_FIELD_GET(ID_AA64DFR0_EL1, PMUVer, dfr0);
 
 	return __kvm_pmu_event_mask(pmuver);
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1036f865c826..0692a109fd4d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1565,7 +1565,7 @@ static u64 kvm_read_sanitised_id_reg(struct kvm_vcpu *vcpu,
 
 static u64 read_id_reg(const struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
-	return IDREG(vcpu->kvm, reg_to_encoding(r));
+	return kvm_read_vm_id_reg(vcpu->kvm, reg_to_encoding(r));
 }
 
 static bool is_feature_id_reg(u32 encoding)
@@ -2760,7 +2760,7 @@ static bool trap_dbgdidr(struct kvm_vcpu *vcpu,
 	if (p->is_write) {
 		return ignore_write(vcpu, p);
 	} else {
-		u64 dfr = IDREG(vcpu->kvm, SYS_ID_AA64DFR0_EL1);
+		u64 dfr = kvm_read_vm_id_reg(vcpu->kvm, SYS_ID_AA64DFR0_EL1);
 		u32 el3 = kvm_has_feat(vcpu->kvm, ID_AA64PFR0_EL1, EL3, IMP);
 
 		p->regval = ((SYS_FIELD_GET(ID_AA64DFR0_EL1, WRPs, dfr) << 28) |
@@ -3519,7 +3519,7 @@ static int idregs_debug_show(struct seq_file *s, void *v)
 		return 0;
 
 	seq_printf(s, "%20s:\t%016llx\n",
-		   desc->name, IDREG(kvm, reg_to_encoding(desc)));
+		   desc->name, kvm_read_vm_id_reg(kvm, reg_to_encoding(desc)));
 
 	return 0;
 }
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 04/10] KVM: arm64: Add helper for writing ID regs

[Oliver Upton]

Replace the remaining usage of IDREG() with a new helper for setting the
value of a feature ID register, with the benefit of cramming in some
extra sanity checks.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  3 ++-
 arch/arm64/kvm/nested.c           |  4 ++--
 arch/arm64/kvm/sys_regs.c         | 17 ++++++++++++++---
 3 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 1201af636551..74e7c29364ee 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -327,7 +327,6 @@ struct kvm_arch {
 	 */
 #define IDREG_IDX(id)		(((sys_reg_CRm(id) - 1) << 3) | sys_reg_Op2(id))
 #define IDX_IDREG(idx)		sys_reg(3, 0, 0, ((idx) >> 3) + 1, (idx) & Op2_mask)
-#define IDREG(kvm, id)		((kvm)->arch.id_regs[IDREG_IDX(id)])
 #define KVM_ARM_ID_REG_NUM	(IDREG_IDX(sys_reg(3, 0, 0, 7, 7)) + 1)
 	u64 id_regs[KVM_ARM_ID_REG_NUM];
 
@@ -1346,6 +1345,8 @@ static inline u64 *__vm_id_reg(struct kvm_arch *ka, u32 reg)
 #define kvm_read_vm_id_reg(kvm, reg)					\
 	({ u64 __val = *__vm_id_reg(&(kvm)->arch, reg); __val; })
 
+void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
+
 #define __expand_field_sign_unsigned(id, fld, val)			\
 	((u64)SYS_FIELD_VALUE(id, fld, val))
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 6813c7c7f00a..5db5bc9dd290 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -203,8 +203,8 @@ int kvm_init_nv_sysregs(struct kvm *kvm)
 	}
 
 	for (int i = 0; i < KVM_ARM_ID_REG_NUM; i++)
-		kvm->arch.id_regs[i] = limit_nv_id_reg(IDX_IDREG(i),
-						       kvm->arch.id_regs[i]);
+		kvm_set_vm_id_reg(kvm, IDX_IDREG(i), limit_nv_id_reg(IDX_IDREG(i),
+								     kvm->arch.id_regs[i]));
 
 	/* VTTBR_EL2 */
 	res0 = res1 = 0;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 0692a109fd4d..8e3358905371 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1851,7 +1851,7 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 
 	ret = arm64_check_features(vcpu, rd, val);
 	if (!ret)
-		IDREG(vcpu->kvm, id) = val;
+		kvm_set_vm_id_reg(vcpu->kvm, id, val);
 
 	mutex_unlock(&vcpu->kvm->arch.config_lock);
 
@@ -1867,6 +1867,18 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 	return ret;
 }
 
+void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val)
+{
+	u64 *p = __vm_id_reg(&kvm->arch, reg);
+
+	lockdep_assert_held(&kvm->arch.config_lock);
+
+	if (KVM_BUG_ON(kvm_vm_has_ran_once(kvm) || !p, kvm))
+		return;
+
+	*p = val;
+}
+
 static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 		       u64 *val)
 {
@@ -3549,8 +3561,7 @@ static void reset_vm_ftr_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc
 	if (test_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags))
 		return;
 
-	lockdep_assert_held(&kvm->arch.config_lock);
-	IDREG(kvm, id) = reg->reset(vcpu, reg);
+	kvm_set_vm_id_reg(kvm, id, reg->reset(vcpu, reg));
 }
 
 static void reset_vcpu_ftr_id_reg(struct kvm_vcpu *vcpu,
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 05/10] KVM: arm64: nv: Use accessors for modifying ID registers

[Oliver Upton]

In the interest of abstracting away the underlying storage of feature
ID registers, rework the nested code to go through the accessors instead
of directly iterating the id_regs array.

This means we now lose the property that ID registers unknown to the
nested code get zeroed, but we really ought to be handling those
explicitly going forward.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |   1 -
 arch/arm64/kvm/nested.c           | 256 ++++++++++++++----------------
 2 files changed, 122 insertions(+), 135 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 74e7c29364ee..294c78319f58 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -326,7 +326,6 @@ struct kvm_arch {
 	 * Atomic access to multiple idregs are guarded by kvm_arch.config_lock.
 	 */
 #define IDREG_IDX(id)		(((sys_reg_CRm(id) - 1) << 3) | sys_reg_Op2(id))
-#define IDX_IDREG(idx)		sys_reg(3, 0, 0, ((idx) >> 3) + 1, (idx) & Op2_mask)
 #define KVM_ARM_ID_REG_NUM	(IDREG_IDX(sys_reg(3, 0, 0, 7, 7)) + 1)
 	u64 id_regs[KVM_ARM_ID_REG_NUM];
 
diff --git a/arch/arm64/kvm/nested.c b/arch/arm64/kvm/nested.c
index 5db5bc9dd290..44085c13e673 100644
--- a/arch/arm64/kvm/nested.c
+++ b/arch/arm64/kvm/nested.c
@@ -23,141 +23,131 @@
  * This list should get updated as new features get added to the NV
  * support, and new extension to the architecture.
  */
-static u64 limit_nv_id_reg(u32 id, u64 val)
+static void limit_nv_id_regs(struct kvm *kvm)
 {
-	u64 tmp;
-
-	switch (id) {
-	case SYS_ID_AA64ISAR0_EL1:
-		/* Support everything but TME, O.S. and Range TLBIs */
-		val &= ~(NV_FTR(ISAR0, TLB)		|
-			 NV_FTR(ISAR0, TME));
-		break;
-
-	case SYS_ID_AA64ISAR1_EL1:
-		/* Support everything but Spec Invalidation */
-		val &= ~(GENMASK_ULL(63, 56)	|
-			 NV_FTR(ISAR1, SPECRES));
-		break;
-
-	case SYS_ID_AA64PFR0_EL1:
-		/* No AMU, MPAM, S-EL2, RAS or SVE */
-		val &= ~(GENMASK_ULL(55, 52)	|
-			 NV_FTR(PFR0, AMU)	|
-			 NV_FTR(PFR0, MPAM)	|
-			 NV_FTR(PFR0, SEL2)	|
-			 NV_FTR(PFR0, RAS)	|
-			 NV_FTR(PFR0, SVE)	|
-			 NV_FTR(PFR0, EL3)	|
-			 NV_FTR(PFR0, EL2)	|
-			 NV_FTR(PFR0, EL1));
-		/* 64bit EL1/EL2/EL3 only */
-		val |= FIELD_PREP(NV_FTR(PFR0, EL1), 0b0001);
-		val |= FIELD_PREP(NV_FTR(PFR0, EL2), 0b0001);
-		val |= FIELD_PREP(NV_FTR(PFR0, EL3), 0b0001);
-		break;
-
-	case SYS_ID_AA64PFR1_EL1:
-		/* Only support SSBS */
-		val &= NV_FTR(PFR1, SSBS);
-		break;
-
-	case SYS_ID_AA64MMFR0_EL1:
-		/* Hide ECV, ExS, Secure Memory */
-		val &= ~(NV_FTR(MMFR0, ECV)		|
-			 NV_FTR(MMFR0, EXS)		|
-			 NV_FTR(MMFR0, TGRAN4_2)	|
-			 NV_FTR(MMFR0, TGRAN16_2)	|
-			 NV_FTR(MMFR0, TGRAN64_2)	|
-			 NV_FTR(MMFR0, SNSMEM));
-
-		/* Disallow unsupported S2 page sizes */
-		switch (PAGE_SIZE) {
-		case SZ_64K:
-			val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN16_2), 0b0001);
-			fallthrough;
-		case SZ_16K:
-			val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN4_2), 0b0001);
-			fallthrough;
-		case SZ_4K:
-			/* Support everything */
-			break;
-		}
-		/*
-		 * Since we can't support a guest S2 page size smaller than
-		 * the host's own page size (due to KVM only populating its
-		 * own S2 using the kernel's page size), advertise the
-		 * limitation using FEAT_GTG.
-		 */
-		switch (PAGE_SIZE) {
-		case SZ_4K:
-			val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN4_2), 0b0010);
-			fallthrough;
-		case SZ_16K:
-			val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN16_2), 0b0010);
-			fallthrough;
-		case SZ_64K:
-			val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN64_2), 0b0010);
-			break;
-		}
-		/* Cap PARange to 48bits */
-		tmp = FIELD_GET(NV_FTR(MMFR0, PARANGE), val);
-		if (tmp > 0b0101) {
-			val &= ~NV_FTR(MMFR0, PARANGE);
-			val |= FIELD_PREP(NV_FTR(MMFR0, PARANGE), 0b0101);
-		}
+	u64 val, tmp;
+
+	/* Support everything but TME, O.S. and Range TLBIs */
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64ISAR0_EL1);
+	val &= ~(NV_FTR(ISAR0, TLB)	|
+		 NV_FTR(ISAR0, TME));
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64ISAR0_EL1, val);
+
+	/* Support everything but Spec Invalidation */
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64ISAR1_EL1);
+	val &= ~(GENMASK_ULL(63, 56)	|
+		 NV_FTR(ISAR1, SPECRES));
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64ISAR1_EL1, val);
+
+	/* No AMU, MPAM, S-EL2, RAS or SVE */
+	kvm_read_vm_id_reg(kvm, SYS_ID_AA64PFR0_EL1);
+	val &= ~(GENMASK_ULL(55, 52)	|
+		 NV_FTR(PFR0, AMU)	|
+		 NV_FTR(PFR0, MPAM)	|
+		 NV_FTR(PFR0, SEL2)	|
+		 NV_FTR(PFR0, RAS)	|
+		 NV_FTR(PFR0, SVE)	|
+		 NV_FTR(PFR0, EL3)	|
+		 NV_FTR(PFR0, EL2)	|
+		 NV_FTR(PFR0, EL1));
+	/* 64bit EL1/EL2/EL3 only */
+	val |= FIELD_PREP(NV_FTR(PFR0, EL1), 0b0001);
+	val |= FIELD_PREP(NV_FTR(PFR0, EL2), 0b0001);
+	val |= FIELD_PREP(NV_FTR(PFR0, EL3), 0b0001);
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64PFR0_EL1, val);
+
+	/* Only support SSBS */
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64PFR1_EL1);
+	val &= NV_FTR(PFR1, SSBS);
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64PFR1_EL1, val);
+
+	/* Hide ECV, ExS, Secure Memory */
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64MMFR0_EL1);
+	val &= ~(NV_FTR(MMFR0, ECV)		|
+		 NV_FTR(MMFR0, EXS)		|
+		 NV_FTR(MMFR0, TGRAN4_2)	|
+		 NV_FTR(MMFR0, TGRAN16_2)	|
+		 NV_FTR(MMFR0, TGRAN64_2)	|
+		 NV_FTR(MMFR0, SNSMEM));
+
+	/* Disallow unsupported S2 page sizes */
+	switch (PAGE_SIZE) {
+	case SZ_64K:
+		val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN16_2), 0b0001);
+		fallthrough;
+	case SZ_16K:
+		val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN4_2), 0b0001);
+		fallthrough;
+	case SZ_4K:
+		/* Support everything */
 		break;
-
-	case SYS_ID_AA64MMFR1_EL1:
-		val &= (NV_FTR(MMFR1, HCX)	|
-			NV_FTR(MMFR1, PAN)	|
-			NV_FTR(MMFR1, LO)	|
-			NV_FTR(MMFR1, HPDS)	|
-			NV_FTR(MMFR1, VH)	|
-			NV_FTR(MMFR1, VMIDBits));
-		break;
-
-	case SYS_ID_AA64MMFR2_EL1:
-		val &= ~(NV_FTR(MMFR2, BBM)	|
-			 NV_FTR(MMFR2, TTL)	|
-			 GENMASK_ULL(47, 44)	|
-			 NV_FTR(MMFR2, ST)	|
-			 NV_FTR(MMFR2, CCIDX)	|
-			 NV_FTR(MMFR2, VARange));
-
-		/* Force TTL support */
-		val |= FIELD_PREP(NV_FTR(MMFR2, TTL), 0b0001);
-		break;
-
-	case SYS_ID_AA64MMFR4_EL1:
-		val = 0;
-		if (!cpus_have_final_cap(ARM64_HAS_HCR_NV1))
-			val |= FIELD_PREP(NV_FTR(MMFR4, E2H0),
-					  ID_AA64MMFR4_EL1_E2H0_NI_NV1);
-		break;
-
-	case SYS_ID_AA64DFR0_EL1:
-		/* Only limited support for PMU, Debug, BPs and WPs */
-		val &= (NV_FTR(DFR0, PMUVer)	|
-			NV_FTR(DFR0, WRPs)	|
-			NV_FTR(DFR0, BRPs)	|
-			NV_FTR(DFR0, DebugVer));
-
-		/* Cap Debug to ARMv8.1 */
-		tmp = FIELD_GET(NV_FTR(DFR0, DebugVer), val);
-		if (tmp > 0b0111) {
-			val &= ~NV_FTR(DFR0, DebugVer);
-			val |= FIELD_PREP(NV_FTR(DFR0, DebugVer), 0b0111);
-		}
-		break;
-
-	default:
-		/* Unknown register, just wipe it clean */
-		val = 0;
+	}
+	/*
+	 * Since we can't support a guest S2 page size smaller than
+	 * the host's own page size (due to KVM only populating its
+	 * own S2 using the kernel's page size), advertise the
+	 * limitation using FEAT_GTG.
+	 */
+	switch (PAGE_SIZE) {
+	case SZ_4K:
+		val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN4_2), 0b0010);
+		fallthrough;
+	case SZ_16K:
+		val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN16_2), 0b0010);
+		fallthrough;
+	case SZ_64K:
+		val |= FIELD_PREP(NV_FTR(MMFR0, TGRAN64_2), 0b0010);
 		break;
 	}
-
-	return val;
+	/* Cap PARange to 48bits */
+	tmp = FIELD_GET(NV_FTR(MMFR0, PARANGE), val);
+	if (tmp > 0b0101) {
+		val &= ~NV_FTR(MMFR0, PARANGE);
+		val |= FIELD_PREP(NV_FTR(MMFR0, PARANGE), 0b0101);
+	}
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64MMFR0_EL1, val);
+
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64MMFR1_EL1);
+	val &= (NV_FTR(MMFR1, HCX)	|
+		NV_FTR(MMFR1, PAN)	|
+		NV_FTR(MMFR1, LO)	|
+		NV_FTR(MMFR1, HPDS)	|
+		NV_FTR(MMFR1, VH)	|
+		NV_FTR(MMFR1, VMIDBits));
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64MMFR1_EL1, val);
+
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64MMFR2_EL1);
+	val &= ~(NV_FTR(MMFR2, BBM)	|
+		 NV_FTR(MMFR2, TTL)	|
+		 GENMASK_ULL(47, 44)	|
+		 NV_FTR(MMFR2, ST)	|
+		 NV_FTR(MMFR2, CCIDX)	|
+		 NV_FTR(MMFR2, VARange));
+
+	/* Force TTL support */
+	val |= FIELD_PREP(NV_FTR(MMFR2, TTL), 0b0001);
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64MMFR2_EL1, val);
+
+	val = 0;
+	if (!cpus_have_final_cap(ARM64_HAS_HCR_NV1))
+		val |= FIELD_PREP(NV_FTR(MMFR4, E2H0),
+				  ID_AA64MMFR4_EL1_E2H0_NI_NV1);
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64MMFR4_EL1, val);
+
+	/* Only limited support for PMU, Debug, BPs and WPs */
+	val = kvm_read_vm_id_reg(kvm, SYS_ID_AA64DFR0_EL1);
+	val &= (NV_FTR(DFR0, PMUVer)	|
+		NV_FTR(DFR0, WRPs)	|
+		NV_FTR(DFR0, BRPs)	|
+		NV_FTR(DFR0, DebugVer));
+
+	/* Cap Debug to ARMv8.1 */
+	tmp = FIELD_GET(NV_FTR(DFR0, DebugVer), val);
+	if (tmp > 0b0111) {
+		val &= ~NV_FTR(DFR0, DebugVer);
+		val |= FIELD_PREP(NV_FTR(DFR0, DebugVer), 0b0111);
+	}
+	kvm_set_vm_id_reg(kvm, SYS_ID_AA64DFR0_EL1, val);
 }
 
 u64 kvm_vcpu_sanitise_vncr_reg(const struct kvm_vcpu *vcpu, enum vcpu_sysreg sr)
@@ -202,9 +192,7 @@ int kvm_init_nv_sysregs(struct kvm *kvm)
 		goto out;
 	}
 
-	for (int i = 0; i < KVM_ARM_ID_REG_NUM; i++)
-		kvm_set_vm_id_reg(kvm, IDX_IDREG(i), limit_nv_id_reg(IDX_IDREG(i),
-								     kvm->arch.id_regs[i]));
+	limit_nv_id_regs(kvm);
 
 	/* VTTBR_EL2 */
 	res0 = res1 = 0;
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 06/10] KVM: arm64: unify code to prepare traps

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

There are 2 functions to calculate traps via HCR_EL2:
* kvm_init_sysreg() called via KVM_RUN (before the 1st run or when
  the pid changes)
* vcpu_reset_hcr() called via KVM_ARM_VCPU_INIT

To unify these 2 and to support traps that are dependent on the
ID register configuration, move the code from vcpu_reset_hcr()
to sys_regs.c and call it via kvm_init_sysreg().

We still have to keep the non-FWB handling stuff in vcpu_reset_hcr().
Also the initialization with HCR_GUEST_FLAGS is kept there but guarded
by !vcpu_has_run_once() to ensure that previous calculated values
don't get overwritten.

While at it rename kvm_init_sysreg() to kvm_calculate_traps() to
better reflect what it's doing.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_emulate.h | 40 +++++++---------------------
 arch/arm64/include/asm/kvm_host.h    |  2 +-
 arch/arm64/kvm/arm.c                 |  2 +-
 arch/arm64/kvm/sys_regs.c            | 34 +++++++++++++++++++++--
 4 files changed, 43 insertions(+), 35 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_emulate.h b/arch/arm64/include/asm/kvm_emulate.h
index 501e3e019c93..84dc3fac9711 100644
--- a/arch/arm64/include/asm/kvm_emulate.h
+++ b/arch/arm64/include/asm/kvm_emulate.h
@@ -69,39 +69,17 @@ static __always_inline bool vcpu_el1_is_32bit(struct kvm_vcpu *vcpu)
 
 static inline void vcpu_reset_hcr(struct kvm_vcpu *vcpu)
 {
-	vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
-	if (has_vhe() || has_hvhe())
-		vcpu->arch.hcr_el2 |= HCR_E2H;
-	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN)) {
-		/* route synchronous external abort exceptions to EL2 */
-		vcpu->arch.hcr_el2 |= HCR_TEA;
-		/* trap error record accesses */
-		vcpu->arch.hcr_el2 |= HCR_TERR;
-	}
+	if (!vcpu_has_run_once(vcpu))
+		vcpu->arch.hcr_el2 = HCR_GUEST_FLAGS;
 
-	if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB)) {
-		vcpu->arch.hcr_el2 |= HCR_FWB;
-	} else {
-		/*
-		 * For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
-		 * get set in SCTLR_EL1 such that we can detect when the guest
-		 * MMU gets turned on and do the necessary cache maintenance
-		 * then.
-		 */
+	/*
+	 * For non-FWB CPUs, we trap VM ops (HCR_EL2.TVM) until M+C
+	 * get set in SCTLR_EL1 such that we can detect when the guest
+	 * MMU gets turned on and do the necessary cache maintenance
+	 * then.
+	 */
+	if (!cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
 		vcpu->arch.hcr_el2 |= HCR_TVM;
-	}
-
-	if (cpus_have_final_cap(ARM64_HAS_EVT) &&
-	    !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE))
-		vcpu->arch.hcr_el2 |= HCR_TID4;
-	else
-		vcpu->arch.hcr_el2 |= HCR_TID2;
-
-	if (vcpu_el1_is_32bit(vcpu))
-		vcpu->arch.hcr_el2 &= ~HCR_RW;
-
-	if (kvm_has_mte(vcpu->kvm))
-		vcpu->arch.hcr_el2 |= HCR_ATA;
 }
 
 static inline unsigned long *vcpu_hcr(struct kvm_vcpu *vcpu)
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 294c78319f58..26042875d6fc 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -1120,7 +1120,7 @@ int __init populate_nv_trap_config(void);
 bool lock_all_vcpus(struct kvm *kvm);
 void unlock_all_vcpus(struct kvm *kvm);
 
-void kvm_init_sysreg(struct kvm_vcpu *);
+void kvm_calculate_traps(struct kvm_vcpu *vcpu);
 
 /* MMIO helpers */
 void kvm_mmio_write_buf(void *buf, unsigned int len, unsigned long data);
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 9996a989b52e..6b217afb4e8e 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -797,7 +797,7 @@ int kvm_arch_vcpu_run_pid_change(struct kvm_vcpu *vcpu)
 	 * This needs to happen after NV has imposed its own restrictions on
 	 * the feature set
 	 */
-	kvm_init_sysreg(vcpu);
+	kvm_calculate_traps(vcpu);
 
 	ret = kvm_timer_enable(vcpu);
 	if (ret)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 8e3358905371..a467ff4290a7 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -4069,11 +4069,33 @@ int kvm_vm_ioctl_get_reg_writable_masks(struct kvm *kvm, struct reg_mask_range *
 	return 0;
 }
 
-void kvm_init_sysreg(struct kvm_vcpu *vcpu)
+static void vcpu_set_hcr(struct kvm_vcpu *vcpu)
 {
 	struct kvm *kvm = vcpu->kvm;
 
-	mutex_lock(&kvm->arch.config_lock);
+	if (has_vhe() || has_hvhe())
+		vcpu->arch.hcr_el2 |= HCR_E2H;
+	if (cpus_have_final_cap(ARM64_HAS_RAS_EXTN)) {
+		/* route synchronous external abort exceptions to EL2 */
+		vcpu->arch.hcr_el2 |= HCR_TEA;
+		/* trap error record accesses */
+		vcpu->arch.hcr_el2 |= HCR_TERR;
+	}
+
+	if (cpus_have_final_cap(ARM64_HAS_STAGE2_FWB))
+		vcpu->arch.hcr_el2 |= HCR_FWB;
+
+	if (cpus_have_final_cap(ARM64_HAS_EVT) &&
+	    !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE))
+		vcpu->arch.hcr_el2 |= HCR_TID4;
+	else
+		vcpu->arch.hcr_el2 |= HCR_TID2;
+
+	if (vcpu_el1_is_32bit(vcpu))
+		vcpu->arch.hcr_el2 &= ~HCR_RW;
+
+	if (kvm_has_mte(vcpu->kvm))
+		vcpu->arch.hcr_el2 |= HCR_ATA;
 
 	/*
 	 * In the absence of FGT, we cannot independently trap TLBI
@@ -4082,6 +4104,14 @@ void kvm_init_sysreg(struct kvm_vcpu *vcpu)
 	 */
 	if (!kvm_has_feat(kvm, ID_AA64ISAR0_EL1, TLB, OS))
 		vcpu->arch.hcr_el2 |= HCR_TTLBOS;
+}
+
+void kvm_calculate_traps(struct kvm_vcpu *vcpu)
+{
+	struct kvm *kvm = vcpu->kvm;
+
+	mutex_lock(&kvm->arch.config_lock);
+	vcpu_set_hcr(vcpu);
 
 	if (cpus_have_final_cap(ARM64_HAS_HCX)) {
 		vcpu->arch.hcrx_el2 = HCRX_GUEST_FLAGS;
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

CTR_EL0 is currently handled as an invariant register, thus
guests will be presented with the host value of that register.

Add emulation for CTR_EL0 based on a per VM value. Userspace can
switch off DIC and IDC bits and reduce DminLine and IminLine sizes.
Naturally, ensure CTR_EL0 is trapped (HCR_EL2.TID2=1) any time that a
VM's CTR_EL0 differs from hardware.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Reviewed-by: Shaoqin Huang <shahuang@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  4 ++++
 arch/arm64/kvm/sys_regs.c         | 20 ++++++++++----------
 2 files changed, 14 insertions(+), 10 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 26042875d6fc..f6de08e81d49 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -329,6 +329,8 @@ struct kvm_arch {
 #define KVM_ARM_ID_REG_NUM	(IDREG_IDX(sys_reg(3, 0, 0, 7, 7)) + 1)
 	u64 id_regs[KVM_ARM_ID_REG_NUM];
 
+	u64 ctr_el0;
+
 	/* Masks for VNCR-baked sysregs */
 	struct kvm_sysreg_masks	*sysreg_masks;
 
@@ -1335,6 +1337,8 @@ static inline u64 *__vm_id_reg(struct kvm_arch *ka, u32 reg)
 	switch (reg) {
 	case sys_reg(3, 0, 0, 1, 0) ... sys_reg(3, 0, 0, 7, 7):
 		return &ka->id_regs[IDREG_IDX(reg)];
+	case SYS_CTR_EL0:
+		return &ka->ctr_el0;
 	default:
 		WARN_ON_ONCE(1);
 		return NULL;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index a467ff4290a7..a12f3bdfb43d 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1583,6 +1583,9 @@ static bool is_feature_id_reg(u32 encoding)
  */
 static inline bool is_vm_ftr_id_reg(u32 id)
 {
+	if (id == SYS_CTR_EL0)
+		return true;
+
 	return (sys_reg_Op0(id) == 3 && sys_reg_Op1(id) == 0 &&
 		sys_reg_CRn(id) == 0 && sys_reg_CRm(id) >= 1 &&
 		sys_reg_CRm(id) < 8);
@@ -1898,7 +1901,7 @@ static bool access_ctr(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
 	if (p->is_write)
 		return write_to_read_only(vcpu, p, r);
 
-	p->regval = read_sanitised_ftr_reg(SYS_CTR_EL0);
+	p->regval = kvm_read_vm_id_reg(vcpu->kvm, SYS_CTR_EL0);
 	return true;
 }
 
@@ -2487,7 +2490,10 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	{ 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 },
+	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
+			     CTR_EL0_IDC_MASK |
+			     CTR_EL0_DminLine_MASK |
+			     CTR_EL0_IminLine_MASK),
 	{ SYS_DESC(SYS_SVCR), undef_access },
 
 	{ PMU_SYS_REG(PMCR_EL0), .access = access_pmcr, .reset = reset_pmcr,
@@ -3725,18 +3731,11 @@ FUNCTION_INVARIANT(midr_el1)
 FUNCTION_INVARIANT(revidr_el1)
 FUNCTION_INVARIANT(aidr_el1)
 
-static u64 get_ctr_el0(struct kvm_vcpu *v, const struct sys_reg_desc *r)
-{
-	((struct sys_reg_desc *)r)->val = read_sanitised_ftr_reg(SYS_CTR_EL0);
-	return ((struct sys_reg_desc *)r)->val;
-}
-
 /* ->val is filled in by kvm_sys_reg_table_init() */
 static struct sys_reg_desc invariant_sys_regs[] __ro_after_init = {
 	{ SYS_DESC(SYS_MIDR_EL1), NULL, get_midr_el1 },
 	{ SYS_DESC(SYS_REVIDR_EL1), NULL, get_revidr_el1 },
 	{ SYS_DESC(SYS_AIDR_EL1), NULL, get_aidr_el1 },
-	{ SYS_DESC(SYS_CTR_EL0), NULL, get_ctr_el0 },
 };
 
 static int get_invariant_sys_reg(u64 id, u64 __user *uaddr)
@@ -4086,7 +4085,8 @@ static void vcpu_set_hcr(struct kvm_vcpu *vcpu)
 		vcpu->arch.hcr_el2 |= HCR_FWB;
 
 	if (cpus_have_final_cap(ARM64_HAS_EVT) &&
-	    !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE))
+	    !cpus_have_final_cap(ARM64_MISMATCHED_CACHE_TYPE) &&
+	    kvm_read_vm_id_reg(kvm, SYS_CTR_EL0) == read_sanitised_ftr_reg(SYS_CTR_EL0))
 		vcpu->arch.hcr_el2 |= HCR_TID4;
 	else
 		vcpu->arch.hcr_el2 |= HCR_TID2;
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 08/10] KVM: arm64: show writable masks for feature registers

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

Instead of using ~0UL provide the actual writable mask for
non-id feature registers in the output of the
KVM_ARM_GET_REG_WRITABLE_MASKS ioctl.

This changes the mask for the CTR_EL0 and CLIDR_EL1 registers.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index a12f3bdfb43d..d8d2a7880576 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2486,7 +2486,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 
 	{ SYS_DESC(SYS_CCSIDR_EL1), access_ccsidr },
 	{ SYS_DESC(SYS_CLIDR_EL1), access_clidr, reset_clidr, CLIDR_EL1,
-	  .set_user = set_clidr },
+	  .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
 	{ 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 },
@@ -4046,20 +4046,11 @@ int kvm_vm_ioctl_get_reg_writable_masks(struct kvm *kvm, struct reg_mask_range *
 		if (!is_feature_id_reg(encoding) || !reg->set_user)
 			continue;
 
-		/*
-		 * For ID registers, we return the writable mask. Other feature
-		 * registers return a full 64bit mask. That's not necessary
-		 * compliant with a given revision of the architecture, but the
-		 * RES0/RES1 definitions allow us to do that.
-		 */
-		if (is_vm_ftr_id_reg(encoding)) {
-			if (!reg->val ||
-			    (is_aa32_id_reg(encoding) && !kvm_supports_32bit_el0()))
-				continue;
-			val = reg->val;
-		} else {
-			val = ~0UL;
+		if (!reg->val ||
+		    (is_aa32_id_reg(encoding) && !kvm_supports_32bit_el0())) {
+			continue;
 		}
+		val = reg->val;
 
 		if (put_user(val, (masks + KVM_ARM_FEATURE_ID_RANGE_INDEX(encoding))))
 			return -EFAULT;
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 09/10] KVM: arm64: rename functions for invariant sys regs

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

Invariant system id registers are populated with host values
at initialization time using their .reset function cb.

These are currently called get_* which is usually used by
the functions implementing the .get_user callback.

Change their function names to reset_* to reflect what they
are used for.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/sys_regs.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index d8d2a7880576..71a4ed58f94b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -3720,8 +3720,8 @@ id_to_sys_reg_desc(struct kvm_vcpu *vcpu, u64 id,
  */
 
 #define FUNCTION_INVARIANT(reg)						\
-	static u64 get_##reg(struct kvm_vcpu *v,			\
-			      const struct sys_reg_desc *r)		\
+	static u64 reset_##reg(struct kvm_vcpu *v,			\
+			       const struct sys_reg_desc *r)		\
 	{								\
 		((struct sys_reg_desc *)r)->val = read_sysreg(reg);	\
 		return ((struct sys_reg_desc *)r)->val;			\
@@ -3733,9 +3733,9 @@ FUNCTION_INVARIANT(aidr_el1)
 
 /* ->val is filled in by kvm_sys_reg_table_init() */
 static struct sys_reg_desc invariant_sys_regs[] __ro_after_init = {
-	{ SYS_DESC(SYS_MIDR_EL1), NULL, get_midr_el1 },
-	{ SYS_DESC(SYS_REVIDR_EL1), NULL, get_revidr_el1 },
-	{ SYS_DESC(SYS_AIDR_EL1), NULL, get_aidr_el1 },
+	{ SYS_DESC(SYS_MIDR_EL1), NULL, reset_midr_el1 },
+	{ SYS_DESC(SYS_REVIDR_EL1), NULL, reset_revidr_el1 },
+	{ SYS_DESC(SYS_AIDR_EL1), NULL, reset_aidr_el1 },
 };
 
 static int get_invariant_sys_reg(u64 id, u64 __user *uaddr)
-- 
2.45.2.627.g7a2c4fd464-goog

[PATCH v5 10/10] KVM: selftests: arm64: Test writes to CTR_EL0

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

Test that CTR_EL0 is modifiable from userspace, that changes are
visible to guests, and that they are preserved across a vCPU reset.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Reviewed-by: Eric Auger <eric.auger@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 .../testing/selftests/kvm/aarch64/set_id_regs.c  | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/tools/testing/selftests/kvm/aarch64/set_id_regs.c b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
index a7de39fa2a0a..9583c04f1228 100644
--- a/tools/testing/selftests/kvm/aarch64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/aarch64/set_id_regs.c
@@ -219,6 +219,7 @@ static void guest_code(void)
 	GUEST_REG_SYNC(SYS_ID_AA64MMFR1_EL1);
 	GUEST_REG_SYNC(SYS_ID_AA64MMFR2_EL1);
 	GUEST_REG_SYNC(SYS_ID_AA64ZFR0_EL1);
+	GUEST_REG_SYNC(SYS_CTR_EL0);
 
 	GUEST_DONE();
 }
@@ -490,11 +491,25 @@ static void test_clidr(struct kvm_vcpu *vcpu)
 	test_reg_vals[encoding_to_range_idx(SYS_CLIDR_EL1)] = clidr;
 }
 
+static void test_ctr(struct kvm_vcpu *vcpu)
+{
+	u64 ctr;
+
+	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0), &ctr);
+	ctr &= ~CTR_EL0_DIC_MASK;
+	if (ctr & CTR_EL0_IminLine_MASK)
+		ctr--;
+
+	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_CTR_EL0), ctr);
+	test_reg_vals[encoding_to_range_idx(SYS_CTR_EL0)] = ctr;
+}
+
 static void test_vcpu_ftr_id_regs(struct kvm_vcpu *vcpu)
 {
 	u64 val;
 
 	test_clidr(vcpu);
+	test_ctr(vcpu);
 
 	vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), &val);
 	val++;
@@ -525,6 +540,7 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
 		test_assert_id_reg_unchanged(vcpu, test_regs[i].reg);
 
 	test_assert_id_reg_unchanged(vcpu, SYS_CLIDR_EL1);
+	test_assert_id_reg_unchanged(vcpu, SYS_CTR_EL0);
 
 	ksft_test_result_pass("%s\n", __func__);
 }
-- 
2.45.2.627.g7a2c4fd464-goog

Re: [PATCH v5 01/10] KVM: arm64: Get sys_reg encoding from descriptor in idregs_debug_show()

[Sebastian Ott]

On Wed, 19 Jun 2024, Oliver Upton wrote:
> KVM is about to add support for more VM-scoped feature ID regs that
> live outside of the id_regs[] array, which means the index of the
> debugfs iterator may not actually be an index into the array.
>
> Prepare by getting the sys_reg encoding from the descriptor itself.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v5 02/10] KVM: arm64: Make idregs debugfs iterator search sysreg table directly

[Sebastian Ott]

On Wed, 19 Jun 2024, Oliver Upton wrote:
> CTR_EL0 complicates the existing scheme for iterating feature ID
> registers, as it is not in the contiguous range that we presently
> support. Just search the sysreg table for the Nth feature ID register in
> anticipation of this. Yes, the debugfs interface has quadratic time
> completixy now. Boo hoo.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v5 03/10] KVM: arm64: Use read-only helper for reading VM ID registers

[Sebastian Ott]

On Wed, 19 Jun 2024, Oliver Upton wrote:
> IDREG() expands to the storage of a particular ID reg, which can be
> useful for handling both reads and writes. However, outside of a select
> few situations, the ID registers should be considered read only.
>
> Replace current readers with a new macro that expands to the value of
> the field rather than the field itself.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v5 04/10] KVM: arm64: Add helper for writing ID regs

[Sebastian Ott]

On Wed, 19 Jun 2024, Oliver Upton wrote:
> Replace the remaining usage of IDREG() with a new helper for setting the
> value of a feature ID register, with the benefit of cramming in some
> extra sanity checks.
>
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>

Reviewed-by: Sebastian Ott <sebott@redhat.com>

Re: [PATCH v5 00/10] KVM: arm64: Allow userspace to modify CTR_EL0

[Oliver Upton]

On Wed, 19 Jun 2024 17:40:26 +0000, Oliver Upton wrote:
> As I'd mentioned on the list, here is my rework of Sebastian's CTR_EL0
> series. Changes this time around:
> 
>  - Drop the cross-validation of the guest's CTR_EL0 with CLIDR_EL1 and
>    the CCSIDR_EL1 hierarchy, instead independently checking these
>    registers against the system's CTR_EL0 value.
> 
> [...]

Applied to kvmarm/next, thanks!

[01/10] KVM: arm64: Get sys_reg encoding from descriptor in idregs_debug_show()
        https://git.kernel.org/kvmarm/kvmarm/c/4e8ff73eb7ae
[02/10] KVM: arm64: Make idregs debugfs iterator search sysreg table directly
        https://git.kernel.org/kvmarm/kvmarm/c/410db103f6eb
[03/10] KVM: arm64: Use read-only helper for reading VM ID registers
        https://git.kernel.org/kvmarm/kvmarm/c/97ca3fcc15cc
[04/10] KVM: arm64: Add helper for writing ID regs
        https://git.kernel.org/kvmarm/kvmarm/c/d7508d27dd88
[05/10] KVM: arm64: nv: Use accessors for modifying ID registers
        https://git.kernel.org/kvmarm/kvmarm/c/44241f34fac9
[06/10] KVM: arm64: unify code to prepare traps
        https://git.kernel.org/kvmarm/kvmarm/c/f1ff3fc5209a
[07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register
        https://git.kernel.org/kvmarm/kvmarm/c/2843cae26644
[08/10] KVM: arm64: show writable masks for feature registers
        https://git.kernel.org/kvmarm/kvmarm/c/bb4fa769dcdd
[09/10] KVM: arm64: rename functions for invariant sys regs
        https://git.kernel.org/kvmarm/kvmarm/c/76d36012276a
[10/10] KVM: selftests: arm64: Test writes to CTR_EL0
        https://git.kernel.org/kvmarm/kvmarm/c/11a31be88fb6

--
Best,
Oliver

Re: [PATCH v5 00/10] KVM: arm64: Allow userspace to modify CTR_EL0

[Sebastian Ott]

On Thu, 20 Jun 2024, Oliver Upton wrote:
> On Wed, 19 Jun 2024 17:40:26 +0000, Oliver Upton wrote:
>> As I'd mentioned on the list, here is my rework of Sebastian's CTR_EL0
>> series. Changes this time around:
>>
>>  - Drop the cross-validation of the guest's CTR_EL0 with CLIDR_EL1 and
>>    the CCSIDR_EL1 hierarchy, instead independently checking these
>>    registers against the system's CTR_EL0 value.
>>
>> [...]
>
> Applied to kvmarm/next, thanks!

Great, thanks a lot!

Sebastian

RE: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Shameerali Kolothum Thodi]

Hi Oliver/Sebastian,

> -----Original Message-----
> From: Oliver Upton <oliver.upton@linux.dev>
> Sent: Wednesday, June 19, 2024 6:41 PM
> To: kvmarm@lists.linux.dev
> Cc: Marc Zyngier <maz@kernel.org>; James Morse
> <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Sebastian Ott
> <sebott@redhat.com>; Shaoqin Huang <shahuang@redhat.com>; Eric Auger
> <eric.auger@redhat.com>; Oliver Upton <oliver.upton@linux.dev>
> Subject: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> register

[...] 
 
> @@ -2487,7 +2490,10 @@ static const struct sys_reg_desc sys_reg_descs[] =
> {
>  	{ 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 },
> +	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> +			     CTR_EL0_IDC_MASK |
> +			     CTR_EL0_DminLine_MASK |
> +			     CTR_EL0_IminLine_MASK),

(Sorry if this was discussed earlier, but I couldn't locate it anywhere.)

Is there a reason why we can't make the L1Ip writable as well here?
We do have hardware that reports VIPT and PIPT for L11p.

The comment here states,
https://elixir.bootlin.com/linux/v6.11-rc7/source/arch/arm64/kernel/cpufeature.c#L489

" If we have differing I-cache policies, report it as the weakest - VIPT."

Does this also mean it is safe to downgrade the PIPT to VIPT for Guest as well?

Please let me know.

Thanks,
Shameer

Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Marc Zyngier]

Hi Shameer,

On Mon, 09 Sep 2024 16:19:54 +0100,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> Hi Oliver/Sebastian,
> 
> > -----Original Message-----
> > From: Oliver Upton <oliver.upton@linux.dev>
> > Sent: Wednesday, June 19, 2024 6:41 PM
> > To: kvmarm@lists.linux.dev
> > Cc: Marc Zyngier <maz@kernel.org>; James Morse
> > <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Sebastian Ott
> > <sebott@redhat.com>; Shaoqin Huang <shahuang@redhat.com>; Eric Auger
> > <eric.auger@redhat.com>; Oliver Upton <oliver.upton@linux.dev>
> > Subject: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> > register
> 
> [...] 
>  
> > @@ -2487,7 +2490,10 @@ static const struct sys_reg_desc sys_reg_descs[] =
> > {
> >  	{ 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 },
> > +	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> > +			     CTR_EL0_IDC_MASK |
> > +			     CTR_EL0_DminLine_MASK |
> > +			     CTR_EL0_IminLine_MASK),
> 
> (Sorry if this was discussed earlier, but I couldn't locate it anywhere.)
> 
> Is there a reason why we can't make the L1Ip writable as well here?
> We do have hardware that reports VIPT and PIPT for L11p.
> 
> The comment here states,
> https://elixir.bootlin.com/linux/v6.11-rc7/source/arch/arm64/kernel/cpufeature.c#L489
> 
> " If we have differing I-cache policies, report it as the weakest - VIPT."
> 
> Does this also mean it is safe to downgrade the PIPT to VIPT for Guest as well?

It should be safe, as a PIPT CMO always does at least the same as
VIPT, and potentially more if there is aliasing.

Thanks,

	M.

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

Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Oliver Upton]

On Mon, Sep 09, 2024 at 05:28:48PM +0100, Marc Zyngier wrote:
> Hi Shameer,
> 
> On Mon, 09 Sep 2024 16:19:54 +0100,
> Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> > 
> > Hi Oliver/Sebastian,
> > 
> > > -----Original Message-----
> > > From: Oliver Upton <oliver.upton@linux.dev>
> > > Sent: Wednesday, June 19, 2024 6:41 PM
> > > To: kvmarm@lists.linux.dev
> > > Cc: Marc Zyngier <maz@kernel.org>; James Morse
> > > <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> > > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Sebastian Ott
> > > <sebott@redhat.com>; Shaoqin Huang <shahuang@redhat.com>; Eric Auger
> > > <eric.auger@redhat.com>; Oliver Upton <oliver.upton@linux.dev>
> > > Subject: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> > > register
> > 
> > [...] 
> >  
> > > @@ -2487,7 +2490,10 @@ static const struct sys_reg_desc sys_reg_descs[] =
> > > {
> > >  	{ 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 },
> > > +	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> > > +			     CTR_EL0_IDC_MASK |
> > > +			     CTR_EL0_DminLine_MASK |
> > > +			     CTR_EL0_IminLine_MASK),
> > 
> > (Sorry if this was discussed earlier, but I couldn't locate it anywhere.)
> > 
> > Is there a reason why we can't make the L1Ip writable as well here?
> > We do have hardware that reports VIPT and PIPT for L11p.
> > 
> > The comment here states,
> > https://elixir.bootlin.com/linux/v6.11-rc7/source/arch/arm64/kernel/cpufeature.c#L489
> > 
> > " If we have differing I-cache policies, report it as the weakest - VIPT."
> > 
> > Does this also mean it is safe to downgrade the PIPT to VIPT for Guest as well?
> 
> It should be safe, as a PIPT CMO always does at least the same as
> VIPT, and potentially more if there is aliasing.

+1, there was no particular reason why this wasn't handled before.

We should be careful to only allow userspace to select VIPT or PIPT
(where permissible), and not necessarily any value lower than what's
reported by hardware.

-- 
Thanks,
Oliver

RE: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Oliver Upton <oliver.upton@linux.dev>
> Sent: Monday, September 9, 2024 5:57 PM
> To: Marc Zyngier <maz@kernel.org>
> Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> kvmarm@lists.linux.dev; Sebastian Ott <sebott@redhat.com>; James Morse
> <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Shaoqin Huang
> <shahuang@redhat.com>; Eric Auger <eric.auger@redhat.com>; Wangzhou
> (B) <wangzhou1@hisilicon.com>
> Subject: Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> register
> 
> On Mon, Sep 09, 2024 at 05:28:48PM +0100, Marc Zyngier wrote:
> > Hi Shameer,
> >
> > On Mon, 09 Sep 2024 16:19:54 +0100,
> > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> wrote:
> > >
> > > Hi Oliver/Sebastian,
> > >
> > > > -----Original Message-----
> > > > From: Oliver Upton <oliver.upton@linux.dev>
> > > > Sent: Wednesday, June 19, 2024 6:41 PM
> > > > To: kvmarm@lists.linux.dev
> > > > Cc: Marc Zyngier <maz@kernel.org>; James Morse
> > > > <james.morse@arm.com>; Suzuki K Poulose
> <suzuki.poulose@arm.com>;
> > > > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Sebastian
> > > > Ott <sebott@redhat.com>; Shaoqin Huang <shahuang@redhat.com>;
> Eric
> > > > Auger <eric.auger@redhat.com>; Oliver Upton
> > > > <oliver.upton@linux.dev>
> > > > Subject: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM
> > > > feature ID register
> > >
> > > [...]
> > >
> > > > @@ -2487,7 +2490,10 @@ static const struct sys_reg_desc
> > > > sys_reg_descs[] = {
> > > >  	{ 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 },
> > > > +	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> > > > +			     CTR_EL0_IDC_MASK |
> > > > +			     CTR_EL0_DminLine_MASK |
> > > > +			     CTR_EL0_IminLine_MASK),
> > >
> > > (Sorry if this was discussed earlier, but I couldn't locate it
> > > anywhere.)
> > >
> > > Is there a reason why we can't make the L1Ip writable as well here?
> > > We do have hardware that reports VIPT and PIPT for L11p.
> > >
> > > The comment here states,
> > > https://elixir.bootlin.com/linux/v6.11-rc7/source/arch/arm64/kernel/
> > > cpufeature.c#L489
> > >
> > > " If we have differing I-cache policies, report it as the weakest - VIPT."
> > >
> > > Does this also mean it is safe to downgrade the PIPT to VIPT for Guest as
> well?
> >
> > It should be safe, as a PIPT CMO always does at least the same as
> > VIPT, and potentially more if there is aliasing.
> 
> +1, there was no particular reason why this wasn't handled before.
> 
> We should be careful to only allow userspace to select VIPT or PIPT (where
> permissible), and not necessarily any value lower than what's reported by
> hardware.

VIPT 0b10
PIPT 0b11

Ok. Just to clarify that " not necessarily any value lower than what's reported by
hardware" means userspace can set PIPT if hardware supports VIPT?

Based on this,
" If we have differing I-cache policies, report it as the weakest - VIPT." , I was thinking
the other way around(see "safe to downgrade PIPT to VIPT"). But Marc also
seems to suggest PIPT CMO ends up doing atleast same as VIPT and more, so it looks like
the other way. If that's the case, what does that "report it as the weakest" means for host?
 
(I will send out a patch, once the above is clarified)

Thanks,
Shameer

Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Marc Zyngier]

On Mon, 09 Sep 2024 18:16:55 +0100,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> 
> 
> > -----Original Message-----
> > From: Oliver Upton <oliver.upton@linux.dev>
> > Sent: Monday, September 9, 2024 5:57 PM
> > To: Marc Zyngier <maz@kernel.org>
> > Cc: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>;
> > kvmarm@lists.linux.dev; Sebastian Ott <sebott@redhat.com>; James Morse
> > <james.morse@arm.com>; Suzuki K Poulose <suzuki.poulose@arm.com>;
> > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Shaoqin Huang
> > <shahuang@redhat.com>; Eric Auger <eric.auger@redhat.com>; Wangzhou
> > (B) <wangzhou1@hisilicon.com>
> > Subject: Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> > register
> > 
> > On Mon, Sep 09, 2024 at 05:28:48PM +0100, Marc Zyngier wrote:
> > > Hi Shameer,
> > >
> > > On Mon, 09 Sep 2024 16:19:54 +0100,
> > > Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> > wrote:
> > > >
> > > > Hi Oliver/Sebastian,
> > > >
> > > > > -----Original Message-----
> > > > > From: Oliver Upton <oliver.upton@linux.dev>
> > > > > Sent: Wednesday, June 19, 2024 6:41 PM
> > > > > To: kvmarm@lists.linux.dev
> > > > > Cc: Marc Zyngier <maz@kernel.org>; James Morse
> > > > > <james.morse@arm.com>; Suzuki K Poulose
> > <suzuki.poulose@arm.com>;
> > > > > yuzenghui <yuzenghui@huawei.com>; kvm@vger.kernel.org; Sebastian
> > > > > Ott <sebott@redhat.com>; Shaoqin Huang <shahuang@redhat.com>;
> > Eric
> > > > > Auger <eric.auger@redhat.com>; Oliver Upton
> > > > > <oliver.upton@linux.dev>
> > > > > Subject: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM
> > > > > feature ID register
> > > >
> > > > [...]
> > > >
> > > > > @@ -2487,7 +2490,10 @@ static const struct sys_reg_desc
> > > > > sys_reg_descs[] = {
> > > > >  	{ 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 },
> > > > > +	ID_WRITABLE(CTR_EL0, CTR_EL0_DIC_MASK |
> > > > > +			     CTR_EL0_IDC_MASK |
> > > > > +			     CTR_EL0_DminLine_MASK |
> > > > > +			     CTR_EL0_IminLine_MASK),
> > > >
> > > > (Sorry if this was discussed earlier, but I couldn't locate it
> > > > anywhere.)
> > > >
> > > > Is there a reason why we can't make the L1Ip writable as well here?
> > > > We do have hardware that reports VIPT and PIPT for L11p.
> > > >
> > > > The comment here states,
> > > > https://elixir.bootlin.com/linux/v6.11-rc7/source/arch/arm64/kernel/
> > > > cpufeature.c#L489
> > > >
> > > > " If we have differing I-cache policies, report it as the weakest - VIPT."
> > > >
> > > > Does this also mean it is safe to downgrade the PIPT to VIPT for Guest as
> > well?
> > >
> > > It should be safe, as a PIPT CMO always does at least the same as
> > > VIPT, and potentially more if there is aliasing.
> > 
> > +1, there was no particular reason why this wasn't handled before.
> > 
> > We should be careful to only allow userspace to select VIPT or PIPT (where
> > permissible), and not necessarily any value lower than what's reported by
> > hardware.
> 
> VIPT 0b10
> PIPT 0b11
> 
> Ok. Just to clarify that " not necessarily any value lower than what's reported by
> hardware" means userspace can set PIPT if hardware supports VIPT?

No, exactly the opposite. If the HW advertises VIPT, we can set
anything else. If the HW advertises PIPT, we can lie to the guest and
set VIPT. And if the HW advertises any of the two reserved values, it
can but in hell... ;-)

> Based on this,
> " If we have differing I-cache policies, report it as the weakest - VIPT." , I was thinking
> the other way around(see "safe to downgrade PIPT to VIPT"). But Marc also
> seems to suggest PIPT CMO ends up doing atleast same as VIPT and more, so it looks like
> the other way. If that's the case, what does that "report it as the weakest" means for host?

"report it as the weakest" means that if you have a machine with two
sets of CPUs, one implementing PIPT and the other VIPT, you report the
crappiest of the two because SW cannot know which CPU it is running
on.

With that, SW running with VIPT on PIPT will do a bit extra work, but
will still work fine.

	M.

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

Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Oliver Upton]

On Mon, Sep 09, 2024 at 05:16:55PM +0000, Shameerali Kolothum Thodi wrote:
> > > It should be safe, as a PIPT CMO always does at least the same as
> > > VIPT, and potentially more if there is aliasing.
> > 
> > +1, there was no particular reason why this wasn't handled before.
> > 
> > We should be careful to only allow userspace to select VIPT or PIPT (where
> > permissible), and not necessarily any value lower than what's reported by
> > hardware.
> 
> VIPT 0b10
> PIPT 0b11
> 
> Ok. Just to clarify that " not necessarily any value lower than what's reported by
> hardware" means userspace can set PIPT if hardware supports VIPT?

By that I meant we disallow userspace from selecting AIVIVT (0b01) and
VPIPT (0b00). The former is reserved in ARMv8, and I don't think anyone
has ever built the latter.

> Based on this,
> " If we have differing I-cache policies, report it as the weakest - VIPT." , I was thinking
> the other way around(see "safe to downgrade PIPT to VIPT"). But Marc also
> seems to suggest PIPT CMO ends up doing atleast same as VIPT and more, so it looks like
> the other way. If that's the case, what does that "report it as the weakest" means for host?

PIPT is the non-aliasing flavor of I$. Using a VIPT software model on
PIPT will lead to overinvalidating, but still correct. Cannot do it the
other way around.

-- 
Thanks,
Oliver

Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Marc Zyngier]

On Mon, 09 Sep 2024 18:50:00 +0100,
Marc Zyngier <maz@kernel.org> wrote:
> 
> > Ok. Just to clarify that " not necessarily any value lower than what's reported by
> > hardware" means userspace can set PIPT if hardware supports VIPT?
> 
> No, exactly the opposite. If the HW advertises VIPT, we can set

s/can/can't/, dammit!

> anything else. If the HW advertises PIPT, we can lie to the guest and
> set VIPT. And if the HW advertises any of the two reserved values, it
> can but in hell... ;-)

	M.

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

RE: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Shameerali Kolothum Thodi]

> -----Original Message-----
> From: Oliver Upton <oliver.upton@linux.dev>
> Sent: Monday, September 9, 2024 6:51 PM
> To: Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com>
> Cc: Marc Zyngier <maz@kernel.org>; kvmarm@lists.linux.dev; Sebastian Ott
> <sebott@redhat.com>; James Morse <james.morse@arm.com>; Suzuki K
> Poulose <suzuki.poulose@arm.com>; yuzenghui <yuzenghui@huawei.com>;
> kvm@vger.kernel.org; Shaoqin Huang <shahuang@redhat.com>; Eric Auger
> <eric.auger@redhat.com>; Wangzhou (B) <wangzhou1@hisilicon.com>
> Subject: Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID
> register
> 
> On Mon, Sep 09, 2024 at 05:16:55PM +0000, Shameerali Kolothum Thodi
> wrote:
> > > > It should be safe, as a PIPT CMO always does at least the same as
> > > > VIPT, and potentially more if there is aliasing.
> > >
> > > +1, there was no particular reason why this wasn't handled before.
> > >
> > > We should be careful to only allow userspace to select VIPT or PIPT
> (where
> > > permissible), and not necessarily any value lower than what's reported by
> > > hardware.
> >
> > VIPT 0b10
> > PIPT 0b11
> >
> > Ok. Just to clarify that " not necessarily any value lower than what's
> reported by
> > hardware" means userspace can set PIPT if hardware supports VIPT?
> 
> By that I meant we disallow userspace from selecting AIVIVT (0b01) and
> VPIPT (0b00). The former is reserved in ARMv8, and I don't think anyone
> has ever built the latter.
> 
> > Based on this,
> > " If we have differing I-cache policies, report it as the weakest - VIPT." , I
> was thinking
> > the other way around(see "safe to downgrade PIPT to VIPT"). But Marc also
> > seems to suggest PIPT CMO ends up doing atleast same as VIPT and more,
> so it looks like
> > the other way. If that's the case, what does that "report it as the weakest"
> means for host?
> 
> PIPT is the non-aliasing flavor of I$. Using a VIPT software model on
> PIPT will lead to overinvalidating, but still correct. Cannot do it the
> other way around.

Ok. Thanks both. I will send out a patch soon.

And now to the elephant in the room, handling MIDR differences and associated errata
management 😊

Marc, you mentioned about a  prototype solution you have a while back[0],
has that been shared public?

Also not sure ARM has any plans to make this a specification soon.

I was thinking of handling this in userspace for now by ignoring the MIDR write
error on Migration and keeping the host MIDR value for destination VM. This is
for use cases where we know that the hosts doesn't have any MIDR based errata or
errata difference doesn't matter and assuming the user knows what they are doing.

Please let me know, if there are any other suggestions or work in progress on this one.

Thanks,
Shameer
[0] https://lore.kernel.org/linux-arm-kernel/867ct8mnel.wl-maz@kernel.org/

Re: [PATCH v5 07/10] KVM: arm64: Treat CTR_EL0 as a VM feature ID register

[Marc Zyngier]

On Tue, 10 Sep 2024 08:16:49 +0100,
Shameerali Kolothum Thodi <shameerali.kolothum.thodi@huawei.com> wrote:
> 
> And now to the elephant in the room, handling MIDR differences and associated errata
> management 😊
> 
> Marc, you mentioned about a  prototype solution you have a while back[0],
> has that been shared public?

Nah, and I didn't have any time to get back to it. But maybe that's a
KVM Forum hackathon project! To be honest, this has little to do with
KVM itself. It is mostly guest enlightenment and a bit of
infrastructure to communicate the constraints back to the guest.

> Also not sure ARM has any plans to make this a specification soon.

Probably not until we come up with a decent prototype that solves the
problem.

> I was thinking of handling this in userspace for now by ignoring the MIDR write
> error on Migration and keeping the host MIDR value for destination VM. This is
> for use cases where we know that the hosts doesn't have any MIDR based errata or
> errata difference doesn't matter and assuming the user knows what they are doing.

If you know of *any* host that doesn't have errata to be mitigated in
the guest, send it to me, and I'll find you one pretty quickly! :D

But yes, you can always play that game (which is what I do for
big-little by letting the guest migrating between CPU types). But it
is ultimately doomed.

> Please let me know, if there are any other suggestions or work in progress on this one.

I don't have much free time at the moment, but I'll do what I can.

Thanks,

	M.

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