Subject: Re: [PATCH v1 09/26] KVM: arm64: Refactor idreg caching into dedicated structure

From: Oliver Upton <[email-masked]>
Date: Mon, 1 Jun 2026 15:28:15 -0700

On Fri, May 29, 2026 at 05:55:42PM +0200, Steffen Eiden wrote:
> +struct kvm_vm_id_regs {
> +	/*
> +	 * Emulated CPU ID registers per VM
> +	 * (Op0, Op1, CRn, CRm, Op2) of the ID registers to be saved in it
> +	 * is (3, 0, 0, crm, op2), where 1<=crm<8, 0<=op2<8.
> +	 *
> +	 * These emulated idregs are VM-wide, but accessed from the context of a vCPU.
> +	 * 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 KVM_ARM_ID_REG_NUM	(IDREG_IDX(sys_reg(3, 0, 0, 7, 7)) + 1)
> +	u64 normal[KVM_ARM_ID_REG_NUM];

nit: ftr_reg would be slightly more clear.

Thanks,
Oliver

> +	u64 midr_el1;
> +	u64 revidr_el1;
> +	u64 aidr_el1;
> +	u64 ctr_el0;
> +};
> +
> +static inline u64 *__vm_id_reg(struct kvm_vm_id_regs *id_regs, 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)];
> +		return &id_regs->normal[IDREG_IDX(reg)];
>  	case SYS_CTR_EL0:
> -		return &ka->ctr_el0;
> +		return &id_regs->ctr_el0;
>  	case SYS_MIDR_EL1:
> -		return &ka->midr_el1;
> +		return &id_regs->midr_el1;
>  	case SYS_REVIDR_EL1:
> -		return &ka->revidr_el1;
> +		return &id_regs->revidr_el1;
>  	case SYS_AIDR_EL1:
> -		return &ka->aidr_el1;
> +		return &id_regs->aidr_el1;
>  	default:
>  		WARN_ON_ONCE(1);
>  		return NULL;
> @@ -1419,7 +1423,7 @@ 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; })
> +	({ u64 __val = *__vm_id_reg(&(kvm)->arch.id_regs, reg); __val; })
>  
>  void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val);
>  
> diff --git a/arch/arm64/kvm/config.c b/arch/arm64/kvm/config.c
> index 014fe04daabf..58a439c3ab9c 100644
> --- a/arch/arm64/kvm/config.c
> +++ b/arch/arm64/kvm/config.c
> @@ -1398,7 +1398,7 @@ void __init check_feature_map(void)
>  
>  static bool idreg_feat_match(struct kvm *kvm, const struct reg_bits_to_feat_map *map)
>  {
> -	u64 regval = kvm->arch.id_regs[map->regidx];
> +	u64 regval = kvm->arch.id_regs.normal[map->regidx];
>  	u64 regfld = (regval >> map->shift) & GENMASK(map->width - 1, 0);
>  
>  	if (map->sign) {
> diff --git a/arch/arm64/kvm/hyp/nvhe/pkvm.c b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> index eb1c10120f9f..94620f142f42 100644
> --- a/arch/arm64/kvm/hyp/nvhe/pkvm.c
> +++ b/arch/arm64/kvm/hyp/nvhe/pkvm.c
> @@ -343,7 +343,7 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
>  	DECLARE_BITMAP(allowed_features, KVM_VCPU_MAX_FEATURES);
>  
>  	/* CTR_EL0 is always under host control, even for protected VMs. */
> -	hyp_vm->kvm.arch.ctr_el0 = host_kvm->arch.ctr_el0;
> +	hyp_vm->kvm.arch.id_regs.ctr_el0 = host_kvm->arch.id_regs.ctr_el0;
>  
>  	/* Preserve the vgic model so that GICv3 emulation works */
>  	hyp_vm->kvm.arch.vgic.vgic_model = host_kvm->arch.vgic.vgic_model;
> @@ -358,7 +358,7 @@ static void pkvm_init_features_from_host(struct pkvm_hyp_vm *hyp_vm, const struc
>  			    KVM_VCPU_MAX_FEATURES);
>  
>  		if (test_bit(KVM_ARCH_FLAG_WRITABLE_IMP_ID_REGS, &host_arch_flags))
> -			hyp_vm->kvm.arch.midr_el1 = host_kvm->arch.midr_el1;
> +			hyp_vm->kvm.arch.id_regs.midr_el1 = host_kvm->arch.id_regs.midr_el1;
>  
>  		return;
>  	}
> @@ -493,7 +493,8 @@ static int vm_copy_id_regs(struct pkvm_hyp_vcpu *hyp_vcpu)
>  	if (test_and_set_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags))
>  		return 0;
>  
> -	memcpy(kvm->arch.id_regs, host_kvm->arch.id_regs, sizeof(kvm->arch.id_regs));
> +	memcpy(kvm->arch.id_regs.normal, host_kvm->arch.id_regs.normal,
> +	       sizeof(kvm->arch.id_regs.normal));
>  
>  	return 0;
>  }
> diff --git a/arch/arm64/kvm/hyp/nvhe/sys_regs.c b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> index b5a0de84ce01..e8d773d38905 100644
> --- a/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> +++ b/arch/arm64/kvm/hyp/nvhe/sys_regs.c
> @@ -292,7 +292,7 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		return 0;
>  
>  	if (reg >= sys_reg(3, 0, 0, 1, 0) && reg <= sys_reg(3, 0, 0, 7, 7))
> -		return kvm->arch.id_regs[IDREG_IDX(reg)];
> +		return kvm->arch.id_regs.normal[IDREG_IDX(reg)];
>  
>  	return 0;
>  }
> @@ -543,7 +543,7 @@ void kvm_init_pvm_id_regs(struct kvm_vcpu *vcpu)
>  	 * for protected VMs.
>  	 */
>  	for (r = sys_reg(3, 0, 0, 4, 0); r <= sys_reg(3, 0, 0, 7, 7); r += sys_reg(0, 0, 0, 0, 1))
> -		ka->id_regs[IDREG_IDX(r)] = pvm_calc_id_reg(vcpu, r);
> +		ka->id_regs.normal[IDREG_IDX(r)] = pvm_calc_id_reg(vcpu, r);
>  
>  	set_bit(KVM_ARCH_FLAG_ID_REGS_INITIALIZED, &kvm->arch.flags);
>  }
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index b9aa892616ab..195ecdac7bd6 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -2477,7 +2477,7 @@ static int set_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  
>  void kvm_set_vm_id_reg(struct kvm *kvm, u32 reg, u64 val)
>  {
> -	u64 *p = __vm_id_reg(&kvm->arch, reg);
> +	u64 *p = __vm_id_reg(&kvm->arch.id_regs, reg);
>  
>  	lockdep_assert_held(&kvm->arch.config_lock);
>  
> -- 
> 2.53.0
>