[PATCH v4 0/5] KVM: arm64: Writable MIDR/REVIDR (and associated baggage)

[PATCH v4 0/5] KVM: arm64: Writable MIDR/REVIDR (and associated baggage)

[Oliver Upton]

This is hopefully the last take on the writable MIDR/REVIDR/AIDR series.

I've restructured things in order to work around the existing
shortcomings, have a stable-worthy backport for SMIDR_EL1, and avoid
breaking ABI.

I personally hate using a capability for this, but given the subtlety of
the issue (and our habit of breaking save/restore) it is probably best
to put userspace in the driver's seat.

v3: https://lore.kernel.org/kvmarm/20250218163443.32836-1-sebott@redhat.com/

v3 -> v4:
 - Trap SMIDR_EL1 accesses so they actually UNDEF
 - Snapshot MIDR, REVIDR, and AIDR from the boot CPU
 - Require buy-in from userspace to make ID registers writable

Oliver Upton (2):
  KVM: arm64: Set HCR_EL2.TID1 unconditionally
  KVM: arm64: Load VPIDR_EL2 with the VM's MIDR_EL1 value

Sebastian Ott (3):
  KVM: arm64: Maintain per-VM copy of implementation ID regs
  KVM: arm64: Allow userspace to change the implementation ID registers
  KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR

 Documentation/virt/kvm/api.rst                |  18 ++
 arch/arm64/include/asm/kvm_arm.h              |   4 +-
 arch/arm64/include/asm/kvm_host.h             |  11 +
 arch/arm64/kvm/arm.c                          |   9 +
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h    |  14 +-
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c           |   4 +-
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c            |  28 +--
 arch/arm64/kvm/sys_regs.c                     | 219 +++++++++++-------
 include/uapi/linux/kvm.h                      |   1 +
 .../testing/selftests/kvm/arm64/set_id_regs.c |  37 ++-
 10 files changed, 229 insertions(+), 116 deletions(-)


base-commit: 2014c95afecee3e76ca4a56956a936e23283f05b
-- 
2.39.5

[PATCH v4 1/5] KVM: arm64: Set HCR_EL2.TID1 unconditionally

[Oliver Upton]

commit 90807748ca3a ("KVM: arm64: Hide SME system registers from
guests") added trap handling for SMIDR_EL1, treating it as UNDEFINED as
KVM does not support SME. This is right for the most part, however KVM
needs to set HCR_EL2.TID1 to _actually_ trap the register.

Unfortunately, this comes with some collateral damage as TID1 forces
REVIDR_EL1 and AIDR_EL1 to trap as well. KVM has long treated these
registers as "invariant" which is an awful term for the following:

 - Userspace sees the boot CPU values on all vCPUs

 - The guest sees the hardware values of the CPU on which a vCPU is
   scheduled

Keep the plates spinning by adding trap handling for the affected
registers and repaint all of the "invariant" crud into terms of
identifying an implementation. Yes, at this point we only need to
set TID1 on SME hardware, but REVIDR_EL1 and AIDR_EL1 are about to
become mutable anyway.

Cc: Mark Brown <broonie@kernel.org>
Cc: stable@vger.kernel.org
Fixes: 90807748ca3a ("KVM: arm64: Hide SME system registers from guests")
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_arm.h |   4 +-
 arch/arm64/kvm/sys_regs.c        | 175 ++++++++++++++++---------------
 2 files changed, 94 insertions(+), 85 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
index 8d94a6c0ed5c..b01c01e55de5 100644
--- a/arch/arm64/include/asm/kvm_arm.h
+++ b/arch/arm64/include/asm/kvm_arm.h
@@ -92,12 +92,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
+ * TID1:	Trap REVIDR_EL1, AIDR_EL1, and SMIDR_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_TID1)
 #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)
 #define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index f6cd1ea7fb55..f25a157622e3 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2483,6 +2483,93 @@ static bool access_mdcr(struct kvm_vcpu *vcpu,
 	return true;
 }
 
+/*
+ * For historical (ahem ABI) reasons, KVM treats MIDR_EL1, REVIDR_EL1, and
+ * AIDR_EL1 as "invariant" registers, meaning userspace cannot change them.
+ * The values made visible to userspace were the register values of the boot
+ * CPU.
+ *
+ * At the same time, reads from these registers at EL1 previously were not
+ * trapped, allowing the guest to read the actual hardware value. On big-little
+ * machines, this means the VM can see different values depending on where a
+ * given vCPU got scheduled.
+ *
+ * These registers are now trapped as collateral damage from SME, and what
+ * follows attempts to give a user / guest view consistent with the existing
+ * ABI.
+ */
+static bool access_imp_id_reg(struct kvm_vcpu *vcpu,
+			      struct sys_reg_params *p,
+			      const struct sys_reg_desc *r)
+{
+	if (p->is_write)
+		return write_to_read_only(vcpu, p, r);
+
+	switch (reg_to_encoding(r)) {
+	case SYS_REVIDR_EL1:
+		p->regval = read_sysreg(revidr_el1);
+		break;
+	case SYS_AIDR_EL1:
+		p->regval = read_sysreg(aidr_el1);
+		break;
+	default:
+		WARN_ON_ONCE(1);
+	}
+
+	return true;
+}
+
+static u64 __ro_after_init boot_cpu_midr_val;
+static u64 __ro_after_init boot_cpu_revidr_val;
+static u64 __ro_after_init boot_cpu_aidr_val;
+
+static void init_imp_id_regs(void)
+{
+	boot_cpu_midr_val = read_sysreg(midr_el1);
+	boot_cpu_revidr_val = read_sysreg(revidr_el1);
+	boot_cpu_aidr_val = read_sysreg(aidr_el1);
+}
+
+static int get_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
+			  u64 *val)
+{
+	switch (reg_to_encoding(r)) {
+	case SYS_MIDR_EL1:
+		*val = boot_cpu_midr_val;
+		break;
+	case SYS_REVIDR_EL1:
+		*val = boot_cpu_revidr_val;
+		break;
+	case SYS_AIDR_EL1:
+		*val = boot_cpu_aidr_val;
+		break;
+	default:
+		WARN_ON_ONCE(1);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
+			  u64 val)
+{
+	u64 expected;
+	int ret;
+
+	ret = get_imp_id_reg(vcpu, r, &expected);
+	if (ret)
+		return ret;
+
+	return (expected == val) ? 0 : -EINVAL;
+}
+
+#define IMPLEMENTATION_ID(reg) {			\
+	SYS_DESC(SYS_##reg),				\
+	.access = access_imp_id_reg,			\
+	.get_user = get_imp_id_reg,			\
+	.set_user = set_imp_id_reg,			\
+}
 
 /*
  * Architected system registers.
@@ -2532,7 +2619,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 
 	{ SYS_DESC(SYS_DBGVCR32_EL2), undef_access, reset_val, DBGVCR32_EL2, 0 },
 
+	IMPLEMENTATION_ID(MIDR_EL1),
 	{ SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
+	IMPLEMENTATION_ID(REVIDR_EL1),
 
 	/*
 	 * ID regs: all ID_SANITISED() entries here must have corresponding
@@ -2804,6 +2893,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	  .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
 	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
 	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
+	IMPLEMENTATION_ID(AIDR_EL1),
 	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
 	ID_FILTERED(CTR_EL0, ctr_el0,
 		    CTR_EL0_DIC_MASK |
@@ -4568,65 +4658,6 @@ id_to_sys_reg_desc(struct kvm_vcpu *vcpu, u64 id,
 	return r;
 }
 
-/*
- * These are the invariant sys_reg registers: we let the guest see the
- * host versions of these, so they're part of the guest state.
- *
- * A future CPU may provide a mechanism to present different values to
- * the guest, or a future kvm may trap them.
- */
-
-#define FUNCTION_INVARIANT(reg)						\
-	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;			\
-	}
-
-FUNCTION_INVARIANT(midr_el1)
-FUNCTION_INVARIANT(revidr_el1)
-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, 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)
-{
-	const struct sys_reg_desc *r;
-
-	r = get_reg_by_id(id, invariant_sys_regs,
-			  ARRAY_SIZE(invariant_sys_regs));
-	if (!r)
-		return -ENOENT;
-
-	return put_user(r->val, uaddr);
-}
-
-static int set_invariant_sys_reg(u64 id, u64 __user *uaddr)
-{
-	const struct sys_reg_desc *r;
-	u64 val;
-
-	r = get_reg_by_id(id, invariant_sys_regs,
-			  ARRAY_SIZE(invariant_sys_regs));
-	if (!r)
-		return -ENOENT;
-
-	if (get_user(val, uaddr))
-		return -EFAULT;
-
-	/* This is what we mean by invariant: you can't change it. */
-	if (r->val != val)
-		return -EINVAL;
-
-	return 0;
-}
-
 static int demux_c15_get(struct kvm_vcpu *vcpu, u64 id, void __user *uaddr)
 {
 	u32 val;
@@ -4708,15 +4739,10 @@ int kvm_sys_reg_get_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,
 int kvm_arm_sys_reg_get_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 {
 	void __user *uaddr = (void __user *)(unsigned long)reg->addr;
-	int err;
 
 	if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
 		return demux_c15_get(vcpu, reg->id, uaddr);
 
-	err = get_invariant_sys_reg(reg->id, uaddr);
-	if (err != -ENOENT)
-		return err;
-
 	return kvm_sys_reg_get_user(vcpu, reg,
 				    sys_reg_descs, ARRAY_SIZE(sys_reg_descs));
 }
@@ -4752,15 +4778,10 @@ int kvm_sys_reg_set_user(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg,
 int kvm_arm_sys_reg_set_reg(struct kvm_vcpu *vcpu, const struct kvm_one_reg *reg)
 {
 	void __user *uaddr = (void __user *)(unsigned long)reg->addr;
-	int err;
 
 	if ((reg->id & KVM_REG_ARM_COPROC_MASK) == KVM_REG_ARM_DEMUX)
 		return demux_c15_set(vcpu, reg->id, uaddr);
 
-	err = set_invariant_sys_reg(reg->id, uaddr);
-	if (err != -ENOENT)
-		return err;
-
 	return kvm_sys_reg_set_user(vcpu, reg,
 				    sys_reg_descs, ARRAY_SIZE(sys_reg_descs));
 }
@@ -4849,23 +4870,14 @@ static int walk_sys_regs(struct kvm_vcpu *vcpu, u64 __user *uind)
 
 unsigned long kvm_arm_num_sys_reg_descs(struct kvm_vcpu *vcpu)
 {
-	return ARRAY_SIZE(invariant_sys_regs)
-		+ num_demux_regs()
+	return num_demux_regs()
 		+ walk_sys_regs(vcpu, (u64 __user *)NULL);
 }
 
 int kvm_arm_copy_sys_reg_indices(struct kvm_vcpu *vcpu, u64 __user *uindices)
 {
-	unsigned int i;
 	int err;
 
-	/* Then give them all the invariant registers' indices. */
-	for (i = 0; i < ARRAY_SIZE(invariant_sys_regs); i++) {
-		if (put_user(sys_reg_to_index(&invariant_sys_regs[i]), uindices))
-			return -EFAULT;
-		uindices++;
-	}
-
 	err = walk_sys_regs(vcpu, uindices);
 	if (err < 0)
 		return err;
@@ -5091,15 +5103,12 @@ int __init kvm_sys_reg_table_init(void)
 	valid &= check_sysreg_table(cp14_64_regs, ARRAY_SIZE(cp14_64_regs), true);
 	valid &= check_sysreg_table(cp15_regs, ARRAY_SIZE(cp15_regs), true);
 	valid &= check_sysreg_table(cp15_64_regs, ARRAY_SIZE(cp15_64_regs), true);
-	valid &= check_sysreg_table(invariant_sys_regs, ARRAY_SIZE(invariant_sys_regs), false);
 	valid &= check_sysreg_table(sys_insn_descs, ARRAY_SIZE(sys_insn_descs), false);
 
 	if (!valid)
 		return -EINVAL;
 
-	/* We abuse the reset function to overwrite the table itself. */
-	for (i = 0; i < ARRAY_SIZE(invariant_sys_regs); i++)
-		invariant_sys_regs[i].reset(NULL, &invariant_sys_regs[i]);
+	init_imp_id_regs();
 
 	ret = populate_nv_trap_config();
 
-- 
2.39.5

Re: [PATCH v4 1/5] KVM: arm64: Set HCR_EL2.TID1 unconditionally

[Marc Zyngier]

On Tue, 25 Feb 2025 00:53:57 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> commit 90807748ca3a ("KVM: arm64: Hide SME system registers from
> guests") added trap handling for SMIDR_EL1, treating it as UNDEFINED as
> KVM does not support SME. This is right for the most part, however KVM
> needs to set HCR_EL2.TID1 to _actually_ trap the register.
> 
> Unfortunately, this comes with some collateral damage as TID1 forces
> REVIDR_EL1 and AIDR_EL1 to trap as well. KVM has long treated these
> registers as "invariant" which is an awful term for the following:
> 
>  - Userspace sees the boot CPU values on all vCPUs
> 
>  - The guest sees the hardware values of the CPU on which a vCPU is
>    scheduled
> 
> Keep the plates spinning by adding trap handling for the affected
> registers and repaint all of the "invariant" crud into terms of
> identifying an implementation. Yes, at this point we only need to
> set TID1 on SME hardware, but REVIDR_EL1 and AIDR_EL1 are about to
> become mutable anyway.
> 
> Cc: Mark Brown <broonie@kernel.org>
> Cc: stable@vger.kernel.org
> Fixes: 90807748ca3a ("KVM: arm64: Hide SME system registers from guests")
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  arch/arm64/include/asm/kvm_arm.h |   4 +-
>  arch/arm64/kvm/sys_regs.c        | 175 ++++++++++++++++---------------
>  2 files changed, 94 insertions(+), 85 deletions(-)
> 
> diff --git a/arch/arm64/include/asm/kvm_arm.h b/arch/arm64/include/asm/kvm_arm.h
> index 8d94a6c0ed5c..b01c01e55de5 100644
> --- a/arch/arm64/include/asm/kvm_arm.h
> +++ b/arch/arm64/include/asm/kvm_arm.h
> @@ -92,12 +92,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
> + * TID1:	Trap REVIDR_EL1, AIDR_EL1, and SMIDR_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_TID1)
>  #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)
>  #define HCR_HOST_VHE_FLAGS (HCR_RW | HCR_TGE | HCR_E2H)
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f6cd1ea7fb55..f25a157622e3 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2483,6 +2483,93 @@ static bool access_mdcr(struct kvm_vcpu *vcpu,
>  	return true;
>  }
>  
> +/*
> + * For historical (ahem ABI) reasons, KVM treats MIDR_EL1, REVIDR_EL1, and
> + * AIDR_EL1 as "invariant" registers, meaning userspace cannot change them.

For consistency, using the past tense would make a lot more sense.

> + * The values made visible to userspace were the register values of the boot
> + * CPU.
> + *
> + * At the same time, reads from these registers at EL1 previously were not
> + * trapped, allowing the guest to read the actual hardware value. On big-little
> + * machines, this means the VM can see different values depending on where a
> + * given vCPU got scheduled.
> + *
> + * These registers are now trapped as collateral damage from SME, and what
> + * follows attempts to give a user / guest view consistent with the existing
> + * ABI.
> + */
> +static bool access_imp_id_reg(struct kvm_vcpu *vcpu,
> +			      struct sys_reg_params *p,
> +			      const struct sys_reg_desc *r)
> +{
> +	if (p->is_write)
> +		return write_to_read_only(vcpu, p, r);
> +
> +	switch (reg_to_encoding(r)) {
> +	case SYS_REVIDR_EL1:
> +		p->regval = read_sysreg(revidr_el1);
> +		break;
> +	case SYS_AIDR_EL1:
> +		p->regval = read_sysreg(aidr_el1);
> +		break;
> +	default:
> +		WARN_ON_ONCE(1);
> +	}
> +
> +	return true;
> +}
> +
> +static u64 __ro_after_init boot_cpu_midr_val;
> +static u64 __ro_after_init boot_cpu_revidr_val;
> +static u64 __ro_after_init boot_cpu_aidr_val;
> +
> +static void init_imp_id_regs(void)
> +{
> +	boot_cpu_midr_val = read_sysreg(midr_el1);
> +	boot_cpu_revidr_val = read_sysreg(revidr_el1);
> +	boot_cpu_aidr_val = read_sysreg(aidr_el1);
> +}
> +
> +static int get_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
> +			  u64 *val)
> +{
> +	switch (reg_to_encoding(r)) {
> +	case SYS_MIDR_EL1:
> +		*val = boot_cpu_midr_val;
> +		break;
> +	case SYS_REVIDR_EL1:
> +		*val = boot_cpu_revidr_val;
> +		break;
> +	case SYS_AIDR_EL1:
> +		*val = boot_cpu_aidr_val;
> +		break;
> +	default:
> +		WARN_ON_ONCE(1);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
> +			  u64 val)
> +{
> +	u64 expected;
> +	int ret;
> +
> +	ret = get_imp_id_reg(vcpu, r, &expected);
> +	if (ret)
> +		return ret;
> +
> +	return (expected == val) ? 0 : -EINVAL;
> +}
> +
> +#define IMPLEMENTATION_ID(reg) {			\
> +	SYS_DESC(SYS_##reg),				\
> +	.access = access_imp_id_reg,			\
> +	.get_user = get_imp_id_reg,			\
> +	.set_user = set_imp_id_reg,			\
> +}
>  
>  /*
>   * Architected system registers.
> @@ -2532,7 +2619,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  
>  	{ SYS_DESC(SYS_DBGVCR32_EL2), undef_access, reset_val, DBGVCR32_EL2, 0 },
>  
> +	IMPLEMENTATION_ID(MIDR_EL1),
>  	{ SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
> +	IMPLEMENTATION_ID(REVIDR_EL1),
>  
>  	/*
>  	 * ID regs: all ID_SANITISED() entries here must have corresponding
> @@ -2804,6 +2893,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	  .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
>  	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
>  	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
> +	IMPLEMENTATION_ID(AIDR_EL1),
>  	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
>  	ID_FILTERED(CTR_EL0, ctr_el0,
>  		    CTR_EL0_DIC_MASK |

I don't think this is enough. You also need to augment the cp15[]
table to handle trapping for the 32bit versions of REVIDR/AIDR.

Thanks,

	M.

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

[PATCH v4 2/5] KVM: arm64: Maintain per-VM copy of implementation ID regs

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

Get ready to allow changes to the implementation ID registers by
tracking the VM-wide values.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/include/asm/kvm_host.h |  9 +++++++
 arch/arm64/kvm/sys_regs.c         | 43 ++++++++++++++++---------------
 2 files changed, 31 insertions(+), 21 deletions(-)

diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 7cfa024de4e3..bc5e8a5620ac 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -373,6 +373,9 @@ 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 midr_el1;
+	u64 revidr_el1;
+	u64 aidr_el1;
 	u64 ctr_el0;
 
 	/* Masks for VNCR-backed and general EL2 sysregs */
@@ -1471,6 +1474,12 @@ static inline u64 *__vm_id_reg(struct kvm_arch *ka, u32 reg)
 		return &ka->id_regs[IDREG_IDX(reg)];
 	case SYS_CTR_EL0:
 		return &ka->ctr_el0;
+	case SYS_MIDR_EL1:
+		return &ka->midr_el1;
+	case SYS_REVIDR_EL1:
+		return &ka->revidr_el1;
+	case SYS_AIDR_EL1:
+		return &ka->aidr_el1;
 	default:
 		WARN_ON_ONCE(1);
 		return NULL;
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index f25a157622e3..de66ffab3a2b 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1653,15 +1653,24 @@ static bool is_feature_id_reg(u32 encoding)
  * Return true if the register's (Op0, Op1, CRn, CRm, Op2) is
  * (3, 0, 0, crm, op2), where 1<=crm<8, 0<=op2<8, which is the range of ID
  * registers KVM maintains on a per-VM basis.
+ *
+ * Additionally, the implementation ID registers and CTR_EL0 are handled as
+ * per-VM registers.
  */
 static inline bool is_vm_ftr_id_reg(u32 id)
 {
-	if (id == SYS_CTR_EL0)
+	switch (id) {
+	case SYS_CTR_EL0:
+	case SYS_MIDR_EL1:
+	case SYS_REVIDR_EL1:
+	case SYS_AIDR_EL1:
 		return true;
+	default:
+		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);
 
-	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);
+	}
 }
 
 static inline bool is_vcpu_ftr_id_reg(u32 id)
@@ -2530,36 +2539,27 @@ static void init_imp_id_regs(void)
 	boot_cpu_aidr_val = read_sysreg(aidr_el1);
 }
 
-static int get_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
-			  u64 *val)
+static u64 reset_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 {
 	switch (reg_to_encoding(r)) {
 	case SYS_MIDR_EL1:
-		*val = boot_cpu_midr_val;
-		break;
+		return boot_cpu_midr_val;
 	case SYS_REVIDR_EL1:
-		*val = boot_cpu_revidr_val;
-		break;
+		return boot_cpu_revidr_val;
 	case SYS_AIDR_EL1:
-		*val = boot_cpu_aidr_val;
-		break;
+		return boot_cpu_aidr_val;
 	default:
-		WARN_ON_ONCE(1);
-		return -EINVAL;
+		KVM_BUG_ON(1, vcpu->kvm);
+		return 0;
 	}
-
-	return 0;
 }
 
 static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
 			  u64 val)
 {
 	u64 expected;
-	int ret;
 
-	ret = get_imp_id_reg(vcpu, r, &expected);
-	if (ret)
-		return ret;
+	expected = read_id_reg(vcpu, r);
 
 	return (expected == val) ? 0 : -EINVAL;
 }
@@ -2567,8 +2567,9 @@ static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
 #define IMPLEMENTATION_ID(reg) {			\
 	SYS_DESC(SYS_##reg),				\
 	.access = access_imp_id_reg,			\
-	.get_user = get_imp_id_reg,			\
+	.get_user = get_id_reg,				\
 	.set_user = set_imp_id_reg,			\
+	.reset = reset_imp_id_reg,			\
 }
 
 /*
-- 
2.39.5

[PATCH v4 3/5] KVM: arm64: Load VPIDR_EL2 with the VM's MIDR_EL1 value

[Oliver Upton]

Userspace will soon be able to change the value of MIDR_EL1. Prepare by
loading VPIDR_EL2 with the guest value for non-nested VMs.

Since VPIDR_EL2 is set for any VM, get rid of the NV-specific cleanup of
reloading the hardware value on vcpu_put(). And for nVHE, load the
hardware value before switching to the host.

Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  8 ++++++-
 arch/arm64/kvm/hyp/nvhe/sysreg-sr.c        |  4 +++-
 arch/arm64/kvm/hyp/vhe/sysreg-sr.c         | 28 ++++++++--------------
 3 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index 76ff095c6b6e..6e6d13580377 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -43,6 +43,11 @@ static inline u64 *ctxt_mdscr_el1(struct kvm_cpu_context *ctxt)
 	return &ctxt_sys_reg(ctxt, MDSCR_EL1);
 }
 
+static inline u64 ctxt_midr_el1(struct kvm_cpu_context *ctxt)
+{
+	return read_cpuid_id();
+}
+
 static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
 {
 	*ctxt_mdscr_el1(ctxt)	= read_sysreg(mdscr_el1);
@@ -168,8 +173,9 @@ static inline void __sysreg_restore_user_state(struct kvm_cpu_context *ctxt)
 }
 
 static inline void __sysreg_restore_el1_state(struct kvm_cpu_context *ctxt,
-					      u64 mpidr)
+					      u64 midr, u64 mpidr)
 {
+	write_sysreg(midr,				vpidr_el2);
 	write_sysreg(mpidr,				vmpidr_el2);
 
 	if (has_vhe() ||
diff --git a/arch/arm64/kvm/hyp/nvhe/sysreg-sr.c b/arch/arm64/kvm/hyp/nvhe/sysreg-sr.c
index dba101565de3..3cc613cce5f5 100644
--- a/arch/arm64/kvm/hyp/nvhe/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/nvhe/sysreg-sr.c
@@ -28,7 +28,9 @@ void __sysreg_save_state_nvhe(struct kvm_cpu_context *ctxt)
 
 void __sysreg_restore_state_nvhe(struct kvm_cpu_context *ctxt)
 {
-	__sysreg_restore_el1_state(ctxt, ctxt_sys_reg(ctxt, MPIDR_EL1));
+	u64 midr = ctxt_midr_el1(ctxt);
+
+	__sysreg_restore_el1_state(ctxt, midr, ctxt_sys_reg(ctxt, MPIDR_EL1));
 	__sysreg_restore_common_state(ctxt);
 	__sysreg_restore_user_state(ctxt);
 	__sysreg_restore_el2_return_state(ctxt);
diff --git a/arch/arm64/kvm/hyp/vhe/sysreg-sr.c b/arch/arm64/kvm/hyp/vhe/sysreg-sr.c
index 90b018e06f2c..3814b0b2c937 100644
--- a/arch/arm64/kvm/hyp/vhe/sysreg-sr.c
+++ b/arch/arm64/kvm/hyp/vhe/sysreg-sr.c
@@ -87,11 +87,12 @@ static void __sysreg_restore_vel2_state(struct kvm_vcpu *vcpu)
 	write_sysreg(__vcpu_sys_reg(vcpu, PAR_EL1),	par_el1);
 	write_sysreg(__vcpu_sys_reg(vcpu, TPIDR_EL1),	tpidr_el1);
 
-	write_sysreg(__vcpu_sys_reg(vcpu, MPIDR_EL1),		vmpidr_el2);
-	write_sysreg_el1(__vcpu_sys_reg(vcpu, MAIR_EL2),	SYS_MAIR);
-	write_sysreg_el1(__vcpu_sys_reg(vcpu, VBAR_EL2),	SYS_VBAR);
-	write_sysreg_el1(__vcpu_sys_reg(vcpu, CONTEXTIDR_EL2),	SYS_CONTEXTIDR);
-	write_sysreg_el1(__vcpu_sys_reg(vcpu, AMAIR_EL2),	SYS_AMAIR);
+	write_sysreg(ctxt_midr_el1(&vcpu->arch.ctxt),			vpidr_el2);
+	write_sysreg(__vcpu_sys_reg(vcpu, MPIDR_EL1),			vmpidr_el2);
+	write_sysreg_el1(__vcpu_sys_reg(vcpu, MAIR_EL2),		SYS_MAIR);
+	write_sysreg_el1(__vcpu_sys_reg(vcpu, VBAR_EL2),		SYS_VBAR);
+	write_sysreg_el1(__vcpu_sys_reg(vcpu, CONTEXTIDR_EL2),		SYS_CONTEXTIDR);
+	write_sysreg_el1(__vcpu_sys_reg(vcpu, AMAIR_EL2),		SYS_AMAIR);
 
 	if (vcpu_el2_e2h_is_set(vcpu)) {
 		/*
@@ -191,7 +192,7 @@ void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu)
 {
 	struct kvm_cpu_context *guest_ctxt = &vcpu->arch.ctxt;
 	struct kvm_cpu_context *host_ctxt;
-	u64 mpidr;
+	u64 midr, mpidr;
 
 	host_ctxt = host_data_ptr(host_ctxt);
 	__sysreg_save_user_state(host_ctxt);
@@ -220,23 +221,18 @@ void __vcpu_load_switch_sysregs(struct kvm_vcpu *vcpu)
 		__sysreg_restore_vel2_state(vcpu);
 	} else {
 		if (vcpu_has_nv(vcpu)) {
-			/*
-			 * Use the guest hypervisor's VPIDR_EL2 when in a
-			 * nested state. The hardware value of MIDR_EL1 gets
-			 * restored on put.
-			 */
-			write_sysreg(ctxt_sys_reg(guest_ctxt, VPIDR_EL2), vpidr_el2);
-
 			/*
 			 * As we're restoring a nested guest, set the value
 			 * provided by the guest hypervisor.
 			 */
+			midr = ctxt_sys_reg(guest_ctxt, VPIDR_EL2);
 			mpidr = ctxt_sys_reg(guest_ctxt, VMPIDR_EL2);
 		} else {
+			midr = ctxt_midr_el1(guest_ctxt);
 			mpidr = ctxt_sys_reg(guest_ctxt, MPIDR_EL1);
 		}
 
-		__sysreg_restore_el1_state(guest_ctxt, mpidr);
+		__sysreg_restore_el1_state(guest_ctxt, midr, mpidr);
 	}
 
 	vcpu_set_flag(vcpu, SYSREGS_ON_CPU);
@@ -271,9 +267,5 @@ void __vcpu_put_switch_sysregs(struct kvm_vcpu *vcpu)
 	/* Restore host user state */
 	__sysreg_restore_user_state(host_ctxt);
 
-	/* If leaving a nesting guest, restore MIDR_EL1 default view */
-	if (vcpu_has_nv(vcpu))
-		write_sysreg(read_cpuid_id(),	vpidr_el2);
-
 	vcpu_clear_flag(vcpu, SYSREGS_ON_CPU);
 }
-- 
2.39.5

[PATCH v4 4/5] KVM: arm64: Allow userspace to change the implementation ID registers

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

KVM's treatment of the ID registers that describe the implementation
(MIDR, REVIDR, and AIDR) is interesting, to say the least. On the
userspace-facing end of it, KVM presents the values of the boot CPU on
all vCPUs and treats them as invariant. On the guest side of things KVM
presents the hardware values of the local CPU, which can change during
CPU migration in a big-little system.

While one may call this fragile, there is at least some degree of
predictability around it. For example, if a VMM wanted to present
big-little to a guest, it could affine vCPUs accordingly to the correct
clusters.

All of this makes a giant mess out of adding support for making these
implementation ID registers writable. Avoid breaking the rather subtle
ABI around the old way of doing things by requiring opt-in from
userspace to make the registers writable.

When the cap is enabled, allow userspace to set MIDR, REVIDR, and AIDR
to any non-reserved value and present those values consistently across
all vCPUs.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
[oliver: changelog, capability]
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 Documentation/virt/kvm/api.rst             | 18 +++++++++
 arch/arm64/include/asm/kvm_host.h          |  2 +
 arch/arm64/kvm/arm.c                       |  9 +++++
 arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  8 +++-
 arch/arm64/kvm/sys_regs.c                  | 45 +++++++++++++++++++---
 include/uapi/linux/kvm.h                   |  1 +
 6 files changed, 77 insertions(+), 6 deletions(-)

diff --git a/Documentation/virt/kvm/api.rst b/Documentation/virt/kvm/api.rst
index 0d1c3a820ce6..718e67373809 100644
--- a/Documentation/virt/kvm/api.rst
+++ b/Documentation/virt/kvm/api.rst
@@ -8258,6 +8258,24 @@ KVM exits with the register state of either the L1 or L2 guest
 depending on which executed at the time of an exit. Userspace must
 take care to differentiate between these cases.
 
+7.37 KVM_CAP_ARM_WRITABLE_IMP_ID_REGS
+-------------------------------------
+
+:Architectures: arm64
+:Target: VM
+:Parameters: None
+:Returns: 0 on success, -EBUSY if vCPUs have been created before enabling this
+          capability.
+
+This capability changes the behavior of the registers that identify a PE
+implementation of the Arm architecture: MIDR_EL1, REVIDR_EL1, and AIDR_EL1.
+By default, these registers are visible to userspace but treated as invariant.
+
+When this capability is enabled, KVM allows userspace to change the
+aforementioned registers before the first KVM_RUN. These registers are VM
+scoped, meaning that the same set of values are presented on all vCPUs in a
+given VM.
+
 8. Other capabilities.
 ======================
 
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index bc5e8a5620ac..8cb9d824fdb3 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -334,6 +334,8 @@ struct kvm_arch {
 #define KVM_ARCH_FLAG_FGU_INITIALIZED			8
 	/* SVE exposed to guest */
 #define KVM_ARCH_FLAG_GUEST_HAS_SVE			9
+	/* MIDR_EL1, REVIDR_EL1, and AIDR_EL1 are writable from userspace */
+#define KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS		10
 	unsigned long flags;
 
 	/* VM-wide vCPU feature set */
diff --git a/arch/arm64/kvm/arm.c b/arch/arm64/kvm/arm.c
index 646e806c6ca6..e32b1a622937 100644
--- a/arch/arm64/kvm/arm.c
+++ b/arch/arm64/kvm/arm.c
@@ -125,6 +125,14 @@ int kvm_vm_ioctl_enable_cap(struct kvm *kvm,
 		}
 		mutex_unlock(&kvm->slots_lock);
 		break;
+	case KVM_CAP_ARM_WRITABLE_IMP_ID_REGS:
+		mutex_lock(&kvm->lock);
+		if (!kvm->created_vcpus) {
+			r = 0;
+			set_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &kvm->arch.flags);
+		}
+		mutex_unlock(&kvm->lock);
+		break;
 	default:
 		break;
 	}
@@ -313,6 +321,7 @@ int kvm_vm_ioctl_check_extension(struct kvm *kvm, long ext)
 	case KVM_CAP_ARM_SYSTEM_SUSPEND:
 	case KVM_CAP_IRQFD_RESAMPLE:
 	case KVM_CAP_COUNTER_OFFSET:
+	case KVM_CAP_ARM_WRITABLE_IMP_ID_REGS:
 		r = 1;
 		break;
 	case KVM_CAP_SET_GUEST_DEBUG2:
diff --git a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
index 6e6d13580377..b9cff893bbe0 100644
--- a/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
+++ b/arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h
@@ -45,7 +45,13 @@ static inline u64 *ctxt_mdscr_el1(struct kvm_cpu_context *ctxt)
 
 static inline u64 ctxt_midr_el1(struct kvm_cpu_context *ctxt)
 {
-	return read_cpuid_id();
+	struct kvm *kvm = kern_hyp_va(ctxt_to_vcpu(ctxt)->kvm);
+
+	if (!(ctxt_is_guest(ctxt) &&
+	      test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &kvm->arch.flags)))
+		return read_cpuid_id();
+
+	return kvm_read_vm_id_reg(kvm, SYS_MIDR_EL1);
 }
 
 static inline void __sysreg_save_common_state(struct kvm_cpu_context *ctxt)
diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index de66ffab3a2b..a83fd8a86e98 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -2514,6 +2514,17 @@ static bool access_imp_id_reg(struct kvm_vcpu *vcpu,
 	if (p->is_write)
 		return write_to_read_only(vcpu, p, r);
 
+	/*
+	 * Return the VM-scoped implementation ID register values if userspace
+	 * has made them writable.
+	 */
+	if (test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &vcpu->kvm->arch.flags))
+		return access_id_reg(vcpu, p, r);
+
+	/*
+	 * Otherwise, fall back to the old behavior of returning the value of
+	 * the current CPU.
+	 */
 	switch (reg_to_encoding(r)) {
 	case SYS_REVIDR_EL1:
 		p->regval = read_sysreg(revidr_el1);
@@ -2557,19 +2568,43 @@ static u64 reset_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r)
 static int set_imp_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *r,
 			  u64 val)
 {
+	struct kvm *kvm = vcpu->kvm;
 	u64 expected;
 
+	guard(mutex)(&kvm->arch.config_lock);
+
 	expected = read_id_reg(vcpu, r);
+	if (expected == val)
+		return 0;
 
-	return (expected == val) ? 0 : -EINVAL;
+	if (!test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &kvm->arch.flags))
+		return -EINVAL;
+
+	/*
+	 * Once the VM has started the ID registers are immutable. Reject the
+	 * write if userspace tries to change it.
+	 */
+	if (kvm_vm_has_ran_once(kvm))
+		return -EBUSY;
+
+	/*
+	 * Any value is allowed for the implementation ID registers so long as
+	 * it is within the writable mask.
+	 */
+	if ((val & r->val) != val)
+		return -EINVAL;
+
+	kvm_set_vm_id_reg(kvm, reg_to_encoding(r), val);
+	return 0;
 }
 
-#define IMPLEMENTATION_ID(reg) {			\
+#define IMPLEMENTATION_ID(reg, mask) {			\
 	SYS_DESC(SYS_##reg),				\
 	.access = access_imp_id_reg,			\
 	.get_user = get_id_reg,				\
 	.set_user = set_imp_id_reg,			\
 	.reset = reset_imp_id_reg,			\
+	.val = mask,					\
 }
 
 /*
@@ -2620,9 +2655,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 
 	{ SYS_DESC(SYS_DBGVCR32_EL2), undef_access, reset_val, DBGVCR32_EL2, 0 },
 
-	IMPLEMENTATION_ID(MIDR_EL1),
+	IMPLEMENTATION_ID(MIDR_EL1, GENMASK_ULL(31, 0)),
 	{ SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
-	IMPLEMENTATION_ID(REVIDR_EL1),
+	IMPLEMENTATION_ID(REVIDR_EL1, -1),
 
 	/*
 	 * ID regs: all ID_SANITISED() entries here must have corresponding
@@ -2894,7 +2929,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	  .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
 	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
 	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
-	IMPLEMENTATION_ID(AIDR_EL1),
+	IMPLEMENTATION_ID(AIDR_EL1, -1),
 	{ SYS_DESC(SYS_CSSELR_EL1), access_csselr, reset_unknown, CSSELR_EL1 },
 	ID_FILTERED(CTR_EL0, ctr_el0,
 		    CTR_EL0_DIC_MASK |
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 45e6d8fca9b9..b6ae8ad8934b 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -929,6 +929,7 @@ struct kvm_enable_cap {
 #define KVM_CAP_PRE_FAULT_MEMORY 236
 #define KVM_CAP_X86_APIC_BUS_CYCLES_NS 237
 #define KVM_CAP_X86_GUEST_MODE 238
+#define KVM_CAP_ARM_WRITABLE_IMP_ID_REGS 239
 
 struct kvm_irq_routing_irqchip {
 	__u32 irqchip;
-- 
2.39.5

Re: [PATCH v4 4/5] KVM: arm64: Allow userspace to change the implementation ID registers

[Marc Zyngier]

On Tue, 25 Feb 2025 00:54:00 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> From: Sebastian Ott <sebott@redhat.com>
> 
> KVM's treatment of the ID registers that describe the implementation
> (MIDR, REVIDR, and AIDR) is interesting, to say the least. On the
> userspace-facing end of it, KVM presents the values of the boot CPU on
> all vCPUs and treats them as invariant. On the guest side of things KVM
> presents the hardware values of the local CPU, which can change during
> CPU migration in a big-little system.
> 
> While one may call this fragile, there is at least some degree of
> predictability around it. For example, if a VMM wanted to present
> big-little to a guest, it could affine vCPUs accordingly to the correct
> clusters.
> 
> All of this makes a giant mess out of adding support for making these
> implementation ID registers writable. Avoid breaking the rather subtle
> ABI around the old way of doing things by requiring opt-in from
> userspace to make the registers writable.
> 
> When the cap is enabled, allow userspace to set MIDR, REVIDR, and AIDR
> to any non-reserved value and present those values consistently across
> all vCPUs.
> 
> Signed-off-by: Sebastian Ott <sebott@redhat.com>
> [oliver: changelog, capability]
> Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
> ---
>  Documentation/virt/kvm/api.rst             | 18 +++++++++
>  arch/arm64/include/asm/kvm_host.h          |  2 +
>  arch/arm64/kvm/arm.c                       |  9 +++++
>  arch/arm64/kvm/hyp/include/hyp/sysreg-sr.h |  8 +++-
>  arch/arm64/kvm/sys_regs.c                  | 45 +++++++++++++++++++---
>  include/uapi/linux/kvm.h                   |  1 +
>  6 files changed, 77 insertions(+), 6 deletions(-)
>

[...]

> @@ -2620,9 +2655,9 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  
>  	{ SYS_DESC(SYS_DBGVCR32_EL2), undef_access, reset_val, DBGVCR32_EL2, 0 },
>  
> -	IMPLEMENTATION_ID(MIDR_EL1),
> +	IMPLEMENTATION_ID(MIDR_EL1, GENMASK_ULL(31, 0)),
>  	{ SYS_DESC(SYS_MPIDR_EL1), NULL, reset_mpidr, MPIDR_EL1 },
> -	IMPLEMENTATION_ID(REVIDR_EL1),
> +	IMPLEMENTATION_ID(REVIDR_EL1, -1),

nit: using GENMASK_ULL() would be much nicer.

>  
>  	/*
>  	 * ID regs: all ID_SANITISED() entries here must have corresponding
> @@ -2894,7 +2929,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	  .set_user = set_clidr, .val = ~CLIDR_EL1_RES0 },
>  	{ SYS_DESC(SYS_CCSIDR2_EL1), undef_access },
>  	{ SYS_DESC(SYS_SMIDR_EL1), undef_access },
> -	IMPLEMENTATION_ID(AIDR_EL1),
> +	IMPLEMENTATION_ID(AIDR_EL1, -1),

Same thing here.

Thanks,

	M.

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

[PATCH v4 5/5] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR

[Oliver Upton]

From: Sebastian Ott <sebott@redhat.com>

Assert that MIDR_EL1, REVIDR_EL1, AIDR_EL1 are writable from userspace,
that the changed values are visible to guests, and that they are
preserved across a vCPU reset.

Signed-off-by: Sebastian Ott <sebott@redhat.com>
Signed-off-by: Oliver Upton <oliver.upton@linux.dev>
---
 .../testing/selftests/kvm/arm64/set_id_regs.c | 37 +++++++++++++++----
 1 file changed, 30 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/kvm/arm64/set_id_regs.c b/tools/testing/selftests/kvm/arm64/set_id_regs.c
index 217541fe6536..1d65f4a09e6f 100644
--- a/tools/testing/selftests/kvm/arm64/set_id_regs.c
+++ b/tools/testing/selftests/kvm/arm64/set_id_regs.c
@@ -230,6 +230,9 @@ static void guest_code(void)
 	GUEST_REG_SYNC(SYS_ID_AA64MMFR2_EL1);
 	GUEST_REG_SYNC(SYS_ID_AA64ZFR0_EL1);
 	GUEST_REG_SYNC(SYS_CTR_EL0);
+	GUEST_REG_SYNC(SYS_MIDR_EL1);
+	GUEST_REG_SYNC(SYS_REVIDR_EL1);
+	GUEST_REG_SYNC(SYS_AIDR_EL1);
 
 	GUEST_DONE();
 }
@@ -609,18 +612,31 @@ static void test_ctr(struct kvm_vcpu *vcpu)
 	test_reg_vals[encoding_to_range_idx(SYS_CTR_EL0)] = ctr;
 }
 
-static void test_vcpu_ftr_id_regs(struct kvm_vcpu *vcpu)
+static void test_id_reg(struct kvm_vcpu *vcpu, u32 id)
 {
 	u64 val;
 
+	val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(id));
+	val++;
+	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(id), val);
+	test_reg_vals[encoding_to_range_idx(id)] = val;
+}
+
+static void test_vcpu_ftr_id_regs(struct kvm_vcpu *vcpu)
+{
 	test_clidr(vcpu);
 	test_ctr(vcpu);
 
-	val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1));
-	val++;
-	vcpu_set_reg(vcpu, KVM_ARM64_SYS_REG(SYS_MPIDR_EL1), val);
+	test_id_reg(vcpu, SYS_MPIDR_EL1);
+	ksft_test_result_pass("%s\n", __func__);
+}
+
+static void test_vcpu_non_ftr_id_regs(struct kvm_vcpu *vcpu)
+{
+	test_id_reg(vcpu, SYS_MIDR_EL1);
+	test_id_reg(vcpu, SYS_REVIDR_EL1);
+	test_id_reg(vcpu, SYS_AIDR_EL1);
 
-	test_reg_vals[encoding_to_range_idx(SYS_MPIDR_EL1)] = val;
 	ksft_test_result_pass("%s\n", __func__);
 }
 
@@ -647,6 +663,9 @@ static void test_reset_preserves_id_regs(struct kvm_vcpu *vcpu)
 	test_assert_id_reg_unchanged(vcpu, SYS_MPIDR_EL1);
 	test_assert_id_reg_unchanged(vcpu, SYS_CLIDR_EL1);
 	test_assert_id_reg_unchanged(vcpu, SYS_CTR_EL0);
+	test_assert_id_reg_unchanged(vcpu, SYS_MIDR_EL1);
+	test_assert_id_reg_unchanged(vcpu, SYS_REVIDR_EL1);
+	test_assert_id_reg_unchanged(vcpu, SYS_AIDR_EL1);
 
 	ksft_test_result_pass("%s\n", __func__);
 }
@@ -660,8 +679,11 @@ int main(void)
 	int test_cnt;
 
 	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_SUPPORTED_REG_MASK_RANGES));
+	TEST_REQUIRE(kvm_has_cap(KVM_CAP_ARM_WRITABLE_IMP_ID_REGS));
 
-	vm = vm_create_with_one_vcpu(&vcpu, guest_code);
+	vm = vm_create(1);
+	vm_enable_cap(vm, KVM_CAP_ARM_WRITABLE_IMP_ID_REGS, 0);
+	vcpu = vm_vcpu_add(vm, 0, guest_code);
 
 	/* Check for AARCH64 only system */
 	val = vcpu_get_reg(vcpu, KVM_ARM64_SYS_REG(SYS_ID_AA64PFR0_EL1));
@@ -675,13 +697,14 @@ int main(void)
 		   ARRAY_SIZE(ftr_id_aa64isar2_el1) + ARRAY_SIZE(ftr_id_aa64pfr0_el1) +
 		   ARRAY_SIZE(ftr_id_aa64pfr1_el1) + ARRAY_SIZE(ftr_id_aa64mmfr0_el1) +
 		   ARRAY_SIZE(ftr_id_aa64mmfr1_el1) + ARRAY_SIZE(ftr_id_aa64mmfr2_el1) +
-		   ARRAY_SIZE(ftr_id_aa64zfr0_el1) - ARRAY_SIZE(test_regs) + 2 +
+		   ARRAY_SIZE(ftr_id_aa64zfr0_el1) - ARRAY_SIZE(test_regs) + 3 +
 		   MPAM_IDREG_TEST;
 
 	ksft_set_plan(test_cnt);
 
 	test_vm_ftr_id_regs(vcpu, aarch64_only);
 	test_vcpu_ftr_id_regs(vcpu);
+	test_vcpu_non_ftr_id_regs(vcpu);
 	test_user_set_mpam_reg(vcpu);
 
 	test_guest_reg_read(vcpu);
-- 
2.39.5

Re: [PATCH v4 5/5] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR

[Mark Brown]

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

On Mon, Feb 24, 2025 at 04:54:01PM -0800, Oliver Upton wrote:

> Assert that MIDR_EL1, REVIDR_EL1, AIDR_EL1 are writable from userspace,
> that the changed values are visible to guests, and that they are
> preserved across a vCPU reset.

This newly added test fails with pKVM on every platform I've looked at:

# ok 92 ID_AA64PFR1_EL1.MPAM_frac not arbitrarily modifiable
# ==== Test Assertion Failure ====
#   arm64/set_id_regs.c:557: test_reg_vals[encoding_to_range_idx(uc.args[2])] == uc.args[3]
#   pid=3711 tid=3711 errno=4 - Interrupted system call
#      1	0x000000000040287f: test_guest_reg_read at set_id_regs.c:557
#      2	0x0000000000401df3: main at set_id_regs.c:710
#      3	0x0000ffff9b207543: ?? ??:0
#      4	0x0000ffff9b207617: ?? ??:0
#      5	0x000000000040206f: _start at ??:?
#   0x410fd035 != 0 (test_reg_vals[encoding_to_range_idx(uc.args[2])] != uc.args[3])
not ok 8 selftests: kvm: set_id_regs # exit=254

Full log:

   https://lava.sirena.org.uk/scheduler/job/1148551

# bad: [be5c7bbb3a64baf884481a1ba0c2f8fb2f93f7c3] Add linux-next specific files for 20250227
# good: [26ae3358dc4d3dfb1c449871840de41ef9ac3aa6] Merge branch 'for-linux-next-fixes' of https://gitlab.freedesktop.org/drm/misc/kernel.git
# good: [be1e3607f29a5a182eaa70e3058aef32fd0cc4f8] ASoC: tas2781: Clean up for some define
# good: [62142da241a08006f89b0620f7291e3a08c0a094] ASoC: rt712-sdca: Add FU05 playback switch control
# good: [1d2e01d53a8ebfffb49e8cc656f8c85239121b26] spi: spi-imx: convert timeouts to secs_to_jiffies()
# good: [55a1abd6e76ce91eb6049f32efec3a8506686748] MAINTAINERS: Add Vincenzo Frascino as Xilinx Sound Driver Maintainer
# good: [f00b3056843d14754ac1bab2106cf5599680f115] ASoC: dt-bindings: mediatek,mt8188-mt6359: Add DMIC backend to dai-link
# good: [3e7b375752b5e4de56e92dfb9c43309cd985b869] ASoC: dt-bindings: fsl,imx-asrc: Reference common DAI properties
# good: [8fd0e127d8da856e34391399df40b33af2b307e0] ASoC: amd: acp: acp70: Remove unnecessary if-check
# good: [0dffacbbf8d044456d50c893adb9499775c489f4] regulator: Add (devm_)of_regulator_get()
# good: [1877c3e7937fb2b9373ba263a4900448d50917b7] ASoC: imx-card: Add playback_only or capture_only support
# good: [a9409fcb979eaff401837b955b234ca1ee05fdbd] ASoC: Intel: avs: Support multi-channel PEAKVOL instantiation
# good: [10efa807929084a8a1c38655942a3bf83bce587a] ASoC: cros_ec_codec: Use str_enable_disable() helper in wov_enable_put()
# good: [97673ea38a77e42eaafcf5181c84f6c8d40b97e7] gpio: regmap: Group optional assignments together for better understanding
# good: [375790f18396b2ba706e031b150c58cd37b45a11] gpiolib: Extract gpiochip_choose_fwnode() for wider use
# good: [db305161880a024a43f4b1cbafa7a294793d7a9e] gpio: regmap: Allow ngpio to be read from the property
# good: [369552fd03f296261023872b8fc983d1fc55c8e9] Merge patch series "mpt3sas driver udpates"
# good: [b47834ee4485bbdcc6d36f086ff61c3efd8870d4] ASoC: SOF: amd: Add depends on CPU_SUP_AMD
# good: [f3bfa0f07976a7996b6dedba21d2e0d164f08ce8] spi: dt-bindings: Convert Freescale SPI bindings to YAML
# good: [64899904d6103500ad01be7b763298dc939285ae] ASoC: soc-core: Use str_yes_no() in snd_soc_close_delayed_work()
# good: [42ae6e2559e63c2d4096b698cd47aaeb974436df] firmware: cs_dsp: test_control_parse: null-terminate test strings
# good: [cb15abd47806b449e853caf43f41573c4c82fed3] spi: s3c64xx: extend description of compatible's fifo_depth
# good: [66d8e76e8e85a30fbf9809837e07e15a8c5ccb8b] regulator: pca9450: Remove duplicate code in probe
# good: [c8d08464bce947ee060e0174a3f4e87503269d0c] ASoC: dt-bindings: atmel-at91sam9g20ek: convert to json-schema
# good: [a261d77fec147b9974aacca8ae8f0693feede838] ASoC: SOF: amd: Drop host bridge ID from struct
# good: [c893ee3f95f16fcb98da934d61483d0b7d8ed568] x86/amd_node: Add a smn_read_register() helper
# good: [d1a09c610027e446ed30c21f61c2f2443bf92a3f] MAINTAINERS: adjust the file entry in SPI OFFLOAD
# good: [5d9fca12f54d3e25e02521aa8f3ec5d53759b334] ASoC: amd: ps: fix inconsistent indenting warning in check_and_handle_sdw_dma_irq()
# good: [d5d9e982a0886a1a99a1a8ef5320c57e257b4fb0] nvmem: core: fix bit offsets of more than one byte
# good: [eca73df072715efb5e880514bb8395e0a4e74a59] nvmem: core: update raw_len if the bit reading is required
# good: [4770132f37dbf327d25455d226e54fafbef6a5c4] nvmem: qfprom: switch to 4-byte aligned reads
# good: [11ccaa3121119eeff9ab9d537e0cf6be3b10698b] nvmem: core: verify cell's raw_len
# good: [73dad3ec96ae3f2da947ff0396c2910bd73f00a2] iio: dummy: Stop using iio_device_claim_direct_scoped()
# good: [8a1812d040c0059378fe8687d68a8b6a90493de1] iio: adc: ad7606: Stop using iio_device_claim_direct_scoped()
# good: [a927e72925c7a8aa2a1cf330c09ce13f4e5b1120] iio: amplifiers: hmc425a: use gpiod_multi_set_value_cansleep
# good: [b2108fc82a0acda34388bff3e3ee3544013b1623] drm: Move for_each_if() to util_macros.h for wider use
# good: [3419bdfd88e314bc5f80b02fa4651c81a0a85b57] dt-bindings: nvmem: qfprom: Add X1E80100 compatible
# good: [e08fe24c34d37d00e84009f2fb4c35f5978041e6] ASoC: SOF: Intel: Use str_enable_disable() helper
# good: [d64c4c3d1c578f98d70db1c5e2535b47adce9d07] ASoC: tas2764: Add reg defaults for TAS2764_INT_CLK_CFG
# good: [42da18e62652b58ba5ecd1524c146b202cda9bb7] ASoC: soc-pcm: cleanup dpcm_fe_dai_do_trigger()
# good: [7dff18535b93ea1ce6dbaf36b7ae670f04113d08] phy: PHY_LAN966X_SERDES should depend on SOC_LAN966 || MCHP_LAN966X_PCI
# good: [3df75289ddc28b46121d51d2812943b78676497b] soundwire: amd: add soundwire host wake interrupt enable/disable sequence
# good: [d2317767723b63d28e3b93da92760b7934935536] phy: samsung-ufs: support ExynosAutov920 ufs phy driver
# good: [aac2f8363f773ae1f65aab140e06e2084ac6b787] soundwire: slave: fix an OF node reference leak in soundwire slave device
# good: [994719ed6d81a6f4677875ab6730254c0bc484ea] ASoC: Intel: avs: Use str_on_off() in avs_dsp_core_power()
# good: [ae575d2145d1a2c8bb5d2835d7d54751f3b0bace] ASoC: tegra: Remove the isomgr_bw APIs export
# good: [e957c96455e8f4c630d5e374312cad0633ca7e17] spi: offload: fix use after free
# good: [f22ba3561daa792dd138ed543e0bf48efe0b999c] ASoC: SOF: imx-common: set sdev->pdata->hw_pdata after common is alloc'd
# good: [ad0fbcebb5f6e093d433a0873758a2778d747eb8] ASoC: adau1701: use gpiod_multi_set_value_cansleep
# good: [ff4d4158ef9143327a42f7be4298751cb0d1be69] spi: spi-offload-trigger-pwm: add extra headers
# good: [21aa330fec31bb530a4ef6c9555fb157d0711112] ASoC: fsl_micfil: Add decimation filter bypass mode support
# good: [c5528214c7c0a753c908a7b353309ba665985fb4] ASoC: codecs: wcd93xx-sdw: fix of_property_read_bool() warnings
# good: [330cbb40bb3664a18a19760bd6dc6003d6624041] dt-bindings: ASoC: rockchip: Add compatible for RK3588 SPDIF
# good: [5a19e1985d014fab9892348f6175a19143cec810] spi: axi-spi-engine: implement offload support
# good: [852c0b7204ded184924c41ab99b2ac7a70ad4dab] ASoC: Intel: soc-acpi-intel-ptl-match: add rt713_vb_l2_rt1320_l13
# good: [4bb5b6f13fd83b32c8a93fbd399e7558415d1ce0] ASoC: amd: amd_sdw: Add quirks for Dell SKU's
# good: [9da195880f167ab7c2d595388decf783c9920121] ASoC: SDCA: Add support for PDE Entity properties
# good: [6cf5df1040ba0694aea6a5edc6f31811a442ea36] ASoC: SOF: imx: add driver for the imx95 chip
# good: [4c7518062d638837cea915e0ffe30f846780639a] ASoC: SOF: ipc4: Add support for split firmware releases
# good: [cb161c333927142818d6bf22a4da2b023fb2b8c9] ASoC: tas2781: Switch to use %ptTsr
# good: [153dbf4adad0082d030c30d20541df2b1af52db6] regmap: irq: Use one way of setting all bits in the register
# good: [0e9a970d7b2cb98d741bc0e32ad8c8f30c009c63] ASoC: qcom: sdw: Add get and set channel maps support from codec to cpu dais
# good: [583348bd65ceaf4a5067a6267dd236929e1b4b37] ASoC: SOF: ipc4-topology: Improve the information in prepare_copier prints
# good: [0a7c85b516830c0bb088b0bdb2f2c50c76fc531a] regulator: ad5398: Fix incorrect power down bit mask
# good: [215705db51eb23052c73126d2efb6acbc2db0424] spi: Replace custom fsleep() implementation
# good: [6603c5133daadbb3277fbd93be0d0d5b8ec928e8] ASoC: dt-bindings: atmel,at91-ssc: Convert to YAML format
# good: [25fac20edd09b60651eabcc57c187b1277f43d08] spi: gpio: Support a single always-selected device
# good: [e27c125040b1e1f26d910b46daabbe55e67fdf3b] ASoC: codecs: wcd934x: use wcd934x binding header
# good: [652ffad172d089acb1a20e5fde1b66e687832b06] spi: fsi: Batch TX operations
# good: [6eab7034579917f207ca6d8e3f4e11e85e0ab7d5] ASoC: soc-core: Stop using of_property_read_bool() for non-boolean properties
# good: [856366dc924a9561dae39f252b45dfd6cc6895ce] ALSA: hda: Select avs-driver by default on MBL
# good: [7e17e80c3a7eb2734795f66ba946f933412d597f] Merge branch 'for-6.14/stack-order' into for-next
git bisect start 'be5c7bbb3a64baf884481a1ba0c2f8fb2f93f7c3' '26ae3358dc4d3dfb1c449871840de41ef9ac3aa6' 'be1e3607f29a5a182eaa70e3058aef32fd0cc4f8' '62142da241a08006f89b0620f7291e3a08c0a094' '1d2e01d53a8ebfffb49e8cc656f8c85239121b26' '55a1abd6e76ce91eb6049f32efec3a8506686748' 'f00b3056843d14754ac1bab2106cf5599680f115' '3e7b375752b5e4de56e92dfb9c43309cd985b869' '8fd0e127d8da856e34391399df40b33af2b307e0' '0dffacbbf8d044456d50c893adb9499775c489f4' '1877c3e7937fb2b9373ba263a4900448d50917b7' 'a9409fcb979eaff401837b955b234ca1ee05fdbd' '10efa807929084a8a1c38655942a3bf83bce587a' '97673ea38a77e42eaafcf5181c84f6c8d40b97e7' '375790f18396b2ba706e031b150c58cd37b45a11' 'db305161880a024a43f4b1cbafa7a294793d7a9e' '369552fd03f296261023872b8fc983d1fc55c8e9' 'b47834ee4485bbdcc6d36f086ff61c3efd8870d4' 'f3bfa0f07976a7996b6dedba21d2e0d164f08ce8' '64899904d6103500ad01be7b763298dc939285ae' '42ae6e2559e63c2d4096b698cd47aaeb974436df' 'cb15abd47806b449e853caf43f41573c4c82fed3' '66d8e76e8e85a30fbf9809837e07e15a8c5ccb8b' 'c8d08464bce947ee060e0174a3f4e87503269d0c' 'a261d77fec147b9974aacca8ae8f0693feede838' 'c893ee3f95f16fcb98da934d61483d0b7d8ed568' 'd1a09c610027e446ed30c21f61c2f2443bf92a3f' '5d9fca12f54d3e25e02521aa8f3ec5d53759b334' 'd5d9e982a0886a1a99a1a8ef5320c57e257b4fb0' 'eca73df072715efb5e880514bb8395e0a4e74a59' '4770132f37dbf327d25455d226e54fafbef6a5c4' '11ccaa3121119eeff9ab9d537e0cf6be3b10698b' '73dad3ec96ae3f2da947ff0396c2910bd73f00a2' '8a1812d040c0059378fe8687d68a8b6a90493de1' 'a927e72925c7a8aa2a1cf330c09ce13f4e5b1120' 'b2108fc82a0acda34388bff3e3ee3544013b1623' '3419bdfd88e314bc5f80b02fa4651c81a0a85b57' 'e08fe24c34d37d00e84009f2fb4c35f5978041e6' 'd64c4c3d1c578f98d70db1c5e2535b47adce9d07' '42da18e62652b58ba5ecd1524c146b202cda9bb7' '7dff18535b93ea1ce6dbaf36b7ae670f04113d08' '3df75289ddc28b46121d51d2812943b78676497b' 'd2317767723b63d28e3b93da92760b7934935536' 'aac2f8363f773ae1f65aab140e06e2084ac6b787' '994719ed6d81a6f4677875ab6730254c0bc484ea' 'ae575d2145d1a2c8bb5d2835d7d54751f3b0bace' 'e957c96455e8f4c630d5e374312cad0633ca7e17' 'f22ba3561daa792dd138ed543e0bf48efe0b999c' 'ad0fbcebb5f6e093d433a0873758a2778d747eb8' 'ff4d4158ef9143327a42f7be4298751cb0d1be69' '21aa330fec31bb530a4ef6c9555fb157d0711112' 'c5528214c7c0a753c908a7b353309ba665985fb4' '330cbb40bb3664a18a19760bd6dc6003d6624041' '5a19e1985d014fab9892348f6175a19143cec810' '852c0b7204ded184924c41ab99b2ac7a70ad4dab' '4bb5b6f13fd83b32c8a93fbd399e7558415d1ce0' '9da195880f167ab7c2d595388decf783c9920121' '6cf5df1040ba0694aea6a5edc6f31811a442ea36' '4c7518062d638837cea915e0ffe30f846780639a' 'cb161c333927142818d6bf22a4da2b023fb2b8c9' '153dbf4adad0082d030c30d20541df2b1af52db6' '0e9a970d7b2cb98d741bc0e32ad8c8f30c009c63' '583348bd65ceaf4a5067a6267dd236929e1b4b37' '0a7c85b516830c0bb088b0bdb2f2c50c76fc531a' '215705db51eb23052c73126d2efb6acbc2db0424' '6603c5133daadbb3277fbd93be0d0d5b8ec928e8' '25fac20edd09b60651eabcc57c187b1277f43d08' 'e27c125040b1e1f26d910b46daabbe55e67fdf3b' '652ffad172d089acb1a20e5fde1b66e687832b06' '6eab7034579917f207ca6d8e3f4e11e85e0ab7d5' '856366dc924a9561dae39f252b45dfd6cc6895ce' '7e17e80c3a7eb2734795f66ba946f933412d597f'
# bad: [be5c7bbb3a64baf884481a1ba0c2f8fb2f93f7c3] Add linux-next specific files for 20250227
git bisect bad be5c7bbb3a64baf884481a1ba0c2f8fb2f93f7c3
# good: [9ac8655d3c4771913214f5bf772bf20583d502c5] Merge branch 'master' of git://git.kernel.org/pub/scm/linux/kernel/git/bluetooth/bluetooth-next.git
git bisect good 9ac8655d3c4771913214f5bf772bf20583d502c5
# good: [b415bae06a9dea4518885389028e149bb2812266] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/dtor/input.git
git bisect good b415bae06a9dea4518885389028e149bb2812266
# bad: [3a8f85966dfecc8596fdb30e732eb68ff56392c5] Merge branch 'next' of git://git.kernel.org/pub/scm/linux/kernel/git/kvmarm/kvmarm.git
git bisect bad 3a8f85966dfecc8596fdb30e732eb68ff56392c5
# good: [574024b154d902bfc4c5d3c1eba95de6d5a5b236] Merge branch into tip/master: 'x86/platform'
git bisect good 574024b154d902bfc4c5d3c1eba95de6d5a5b236
# good: [cd204cf7aa640961221ceaa15cd90bfc6c270174] Merge branch 'for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/sre/linux-power-supply.git
git bisect good cd204cf7aa640961221ceaa15cd90bfc6c270174
# good: [ed3b9e01ab408ca6cc59ae34d65bedebd0772e5e] Merge branch 'edac-for-next' of git://git.kernel.org/pub/scm/linux/kernel/git/ras/ras.git
git bisect good ed3b9e01ab408ca6cc59ae34d65bedebd0772e5e
# good: [c52f04dbdef4bcdf518e8073eac9568b6e40e88e] Merge branches 'docs.2025.02.04a', 'lazypreempt.2025.02.24a', 'misc.2025.02.06a', 'srcu.2025.02.05a' and 'torture.2025.02.05a'
git bisect good c52f04dbdef4bcdf518e8073eac9568b6e40e88e
# bad: [11a7d6a1903714e58848ee227da228e90c76b3d8] Merge branch 'kvm-arm64/misc' into kvmarm/next
git bisect bad 11a7d6a1903714e58848ee227da228e90c76b3d8
# good: [8b0b98ebf34d6e6ad68a27109f78f2e153187737] KVM: arm64: Advertise FEAT_ECV when possible
git bisect good 8b0b98ebf34d6e6ad68a27109f78f2e153187737
# bad: [a88c7c22447932c9d5e45cd5b45224a8eb8efa40] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR
git bisect bad a88c7c22447932c9d5e45cd5b45224a8eb8efa40
# good: [b4043e7cb78b96dae07c62d3ac527f84010828cd] KVM: arm64: Maintain per-VM copy of implementation ID regs
git bisect good b4043e7cb78b96dae07c62d3ac527f84010828cd
# good: [3adaee78306148da5df5d3d655e9a90bf18e9513] KVM: arm64: Allow userspace to change the implementation ID registers
git bisect good 3adaee78306148da5df5d3d655e9a90bf18e9513
# first bad commit: [a88c7c22447932c9d5e45cd5b45224a8eb8efa40] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR

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

Re: [PATCH v4 5/5] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR

[Sebastian Ott]

On Thu, 27 Feb 2025, Mark Brown wrote:

> On Mon, Feb 24, 2025 at 04:54:01PM -0800, Oliver Upton wrote:
>
>> Assert that MIDR_EL1, REVIDR_EL1, AIDR_EL1 are writable from userspace,
>> that the changed values are visible to guests, and that they are
>> preserved across a vCPU reset.
>
> This newly added test fails with pKVM on every platform I've looked at:
>
> # ok 92 ID_AA64PFR1_EL1.MPAM_frac not arbitrarily modifiable
> # ==== Test Assertion Failure ====
> #   arm64/set_id_regs.c:557: test_reg_vals[encoding_to_range_idx(uc.args[2])] == uc.args[3]
> #   pid=3711 tid=3711 errno=4 - Interrupted system call
> #      1	0x000000000040287f: test_guest_reg_read at set_id_regs.c:557
> #      2	0x0000000000401df3: main at set_id_regs.c:710
> #      3	0x0000ffff9b207543: ?? ??:0
> #      4	0x0000ffff9b207617: ?? ??:0
> #      5	0x000000000040206f: _start at ??:?
> #   0x410fd035 != 0 (test_reg_vals[encoding_to_range_idx(uc.args[2])] != uc.args[3])
> not ok 8 selftests: kvm: set_id_regs # exit=254
>

Hm, I did test with kvm-arm.mode=nvhe but not with protected. When I do
now, this fails even earlier for me - with the ctr_el0 reg.

Re: [PATCH v4 5/5] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR

[Sebastian Ott]

On Fri, 28 Feb 2025, Sebastian Ott wrote:
> On Thu, 27 Feb 2025, Mark Brown wrote:
>
>>  On Mon, Feb 24, 2025 at 04:54:01PM -0800, Oliver Upton wrote:
>>
>>>  Assert that MIDR_EL1, REVIDR_EL1, AIDR_EL1 are writable from userspace,
>>>  that the changed values are visible to guests, and that they are
>>>  preserved across a vCPU reset.
>>
>>  This newly added test fails with pKVM on every platform I've looked at:
>> 
>> #  ok 92 ID_AA64PFR1_EL1.MPAM_frac not arbitrarily modifiable
>> #  ==== Test Assertion Failure ====
>> #    arm64/set_id_regs.c:557: 
>> #    test_reg_vals[encoding_to_range_idx(uc.args[2])] == uc.args[3]
>> #    pid=3711 tid=3711 errno=4 - Interrupted system call
>> #       1	0x000000000040287f: test_guest_reg_read at set_id_regs.c:557
>> #       2	0x0000000000401df3: main at set_id_regs.c:710
>> #       3	0x0000ffff9b207543: ?? ??:0
>> #       4	0x0000ffff9b207617: ?? ??:0
>> #       5	0x000000000040206f: _start at ??:?
>> #    0x410fd035 != 0 (test_reg_vals[encoding_to_range_idx(uc.args[2])] != 
>> #    uc.args[3])
>>  not ok 8 selftests: kvm: set_id_regs # exit=254
>> 
>
> Hm, I did test with kvm-arm.mode=nvhe but not with protected. When I do
> now, this fails even earlier for me - with the ctr_el0 reg.

For the CTR issue we're not setting the trap for pkvm. It works if I just
set HCR_TID2 - I still have to figure out how to do that only when needed
like we do it for the "normal" VHE case.

For MIDR it looks like we're not setting up VPIDR_EL2 for pkvm.

Sebastian

Re: [PATCH v4 0/5] KVM: arm64: Writable MIDR/REVIDR (and associated baggage)

[Marc Zyngier]

On Tue, 25 Feb 2025 00:53:56 +0000,
Oliver Upton <oliver.upton@linux.dev> wrote:
> 
> This is hopefully the last take on the writable MIDR/REVIDR/AIDR series.
> 
> I've restructured things in order to work around the existing
> shortcomings, have a stable-worthy backport for SMIDR_EL1, and avoid
> breaking ABI.
> 
> I personally hate using a capability for this, but given the subtlety of
> the issue (and our habit of breaking save/restore) it is probably best
> to put userspace in the driver's seat.
> 
> v3: https://lore.kernel.org/kvmarm/20250218163443.32836-1-sebott@redhat.com/
> 
> v3 -> v4:
>  - Trap SMIDR_EL1 accesses so they actually UNDEF
>  - Snapshot MIDR, REVIDR, and AIDR from the boot CPU
>  - Require buy-in from userspace to make ID registers writable

Here's what I added to the series in order to handle AIDR/REVIDR. Not
exactly pretty, but it does the job (tested on my Synquacker running a
hand-crafted guest).

Feel free to squash it into patch #1.

Thanks,

	M.

From: Marc Zyngier <maz@kernel.org>
Date: Tue, 25 Feb 2025 13:46:55 +0000
Subject: [PATCH] KVM: arm64: Map AArch32 AIDR/REVIDR to their AArch64
 counterpart

In order to handle AIDR/REVIDR traps from an AArch32 guest, add
some ugly handling to kvm_handle_cp15_32().

Signed-off-by: Marc Zyngier <maz@kernel.org>
---
 arch/arm64/kvm/sys_regs.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index c1727fe6c8cb0..1ceaec61796c9 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -4398,9 +4398,13 @@ int kvm_handle_cp15_32(struct kvm_vcpu *vcpu)
 	 * Certain AArch32 ID registers are handled by rerouting to the AArch64
 	 * system register table. Registers in the ID range where CRm=0 are
 	 * excluded from this scheme as they do not trivially map into AArch64
-	 * system register encodings.
+	 * system register encodings, except for AIDR/REVIDR.
 	 */
-	if (params.Op1 == 0 && params.CRn == 0 && params.CRm)
+	if (params.Op1 == 0 && params.CRn == 0 &&
+	    (params.CRm || params.Op2 == 6 /* REVIDR */))
+		return kvm_emulate_cp15_id_reg(vcpu, &params);
+	if (params.Op1 == 1 && params.CRn == 0 &&
+	    params.CRm == 0 && params.Op2 == 7 /* AIDR */)
 		return kvm_emulate_cp15_id_reg(vcpu, &params);
 
 	return kvm_handle_cp_32(vcpu, &params, cp15_regs, ARRAY_SIZE(cp15_regs));
-- 
2.39.2


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

Re: [PATCH v4 0/5] KVM: arm64: Writable MIDR/REVIDR (and associated baggage)

[Oliver Upton]

On Mon, 24 Feb 2025 16:53:56 -0800, Oliver Upton wrote:
> This is hopefully the last take on the writable MIDR/REVIDR/AIDR series.
> 
> I've restructured things in order to work around the existing
> shortcomings, have a stable-worthy backport for SMIDR_EL1, and avoid
> breaking ABI.
> 
> I personally hate using a capability for this, but given the subtlety of
> the issue (and our habit of breaking save/restore) it is probably best
> to put userspace in the driver's seat.
> 
> [...]

Applied to next, thanks!

[1/5] KVM: arm64: Set HCR_EL2.TID1 unconditionally
      https://git.kernel.org/kvmarm/kvmarm/c/4cd48565b0e5
[2/5] KVM: arm64: Maintain per-VM copy of implementation ID regs
      https://git.kernel.org/kvmarm/kvmarm/c/b4043e7cb78b
[3/5] KVM: arm64: Load VPIDR_EL2 with the VM's MIDR_EL1 value
      https://git.kernel.org/kvmarm/kvmarm/c/d0d81e03e629
[4/5] KVM: arm64: Allow userspace to change the implementation ID registers
      https://git.kernel.org/kvmarm/kvmarm/c/3adaee783061
[5/5] KVM: selftests: arm64: Test writes to MIDR,REVIDR,AIDR
      https://git.kernel.org/kvmarm/kvmarm/c/a88c7c224479

--
Best,
Oliver