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

[Marc Zyngier <maz@kernel.org>]

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.