Re: [PATCH v5 19/24] KVM: arm64: Implement lazy PMU context swaps

[Oliver Upton <oupton@kernel.org>]

On Tue, Dec 09, 2025 at 08:51:16PM +0000, Colton Lewis wrote:
> +enum vcpu_pmu_register_access {
> +	VCPU_PMU_ACCESS_UNSET,
> +	VCPU_PMU_ACCESS_VIRTUAL,
> +	VCPU_PMU_ACCESS_PHYSICAL,
> +};

This is confusing. Even when the guest is accessing registers directly
on the CPU I'd still call that "hardware assisted virtualization" and
not "physical".

> +#endif /* _ASM_ARM64_KVM_TYPES_H */
> diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
> index 0ab89c91e19cb..c2cf6b308ec60 100644
> --- a/arch/arm64/kvm/debug.c
> +++ b/arch/arm64/kvm/debug.c
> @@ -34,7 +34,7 @@ static int cpu_has_spe(u64 dfr0)
>   *  - Self-hosted Trace Filter controls (MDCR_EL2_TTRF)
>   *  - Self-hosted Trace (MDCR_EL2_TTRF/MDCR_EL2_E2TB)
>   */
> -static void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
> +void kvm_arm_setup_mdcr_el2(struct kvm_vcpu *vcpu)
>  {
>  	int hpmn = kvm_pmu_hpmn(vcpu);
>  
> diff --git a/arch/arm64/kvm/hyp/include/hyp/switch.h b/arch/arm64/kvm/hyp/include/hyp/switch.h
> index bde79ec1a1836..ea288a712bb5d 100644
> --- a/arch/arm64/kvm/hyp/include/hyp/switch.h
> +++ b/arch/arm64/kvm/hyp/include/hyp/switch.h
> @@ -963,6 +963,8 @@ static bool kvm_hyp_handle_pmu_regs(struct kvm_vcpu *vcpu)
>  	if (ret)
>  		__kvm_skip_instr(vcpu);
>  
> +	kvm_pmu_set_physical_access(vcpu);
> +
>  	return ret;
>  }
>  
> diff --git a/arch/arm64/kvm/pmu-direct.c b/arch/arm64/kvm/pmu-direct.c
> index 8d0d6d1a0d851..c5767e2ebc651 100644
> --- a/arch/arm64/kvm/pmu-direct.c
> +++ b/arch/arm64/kvm/pmu-direct.c
> @@ -73,6 +73,7 @@ bool kvm_vcpu_pmu_use_fgt(struct kvm_vcpu *vcpu)
>  	u8 hpmn = vcpu->kvm->arch.nr_pmu_counters;
>  
>  	return kvm_vcpu_pmu_is_partitioned(vcpu) &&
> +		vcpu->arch.pmu.access == VCPU_PMU_ACCESS_PHYSICAL &&
>  		cpus_have_final_cap(ARM64_HAS_FGT) &&
>  		(hpmn != 0 || cpus_have_final_cap(ARM64_HAS_HPMN0));
>  }
> @@ -92,6 +93,26 @@ u64 kvm_pmu_fgt2_bits(void)
>  		| HDFGRTR2_EL2_nPMICNTR_EL0;
>  }
>  
> +/**
> + * kvm_pmu_set_physical_access()
> + * @vcpu: Pointer to vcpu struct
> + *
> + * Reconfigure the guest for physical access of PMU hardware if
> + * allowed. This means reconfiguring mdcr_el2 and loading the vCPU
> + * state onto hardware.
> + *
> + */
> +
> +void kvm_pmu_set_physical_access(struct kvm_vcpu *vcpu)
> +{
> +	if (kvm_vcpu_pmu_is_partitioned(vcpu)
> +	    && vcpu->arch.pmu.access == VCPU_PMU_ACCESS_VIRTUAL) {
> +		vcpu->arch.pmu.access = VCPU_PMU_ACCESS_PHYSICAL;
> +		kvm_arm_setup_mdcr_el2(vcpu);
> +		kvm_pmu_load(vcpu);
> +	}

It isn't immediately obvious how this guards against preemption.

Also, the general approach for these context-loading situations is to do
a full load/put on the vCPU rather than a directed load.

> +static void kvm_pmu_register_init(struct kvm_vcpu *vcpu)
> +{
> +	if (vcpu->arch.pmu.access == VCPU_PMU_ACCESS_UNSET)
> +		vcpu->arch.pmu.access = VCPU_PMU_ACCESS_VIRTUAL;

This is confusing. The zero value of the enum should be consistent with
the "unloaded" state.

> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index f2ae761625a66..d73218706b834 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1197,6 +1197,8 @@ static bool access_pmu_evtyper(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  		p->regval = __vcpu_sys_reg(vcpu, reg);
>  	}
>  
> +	kvm_pmu_set_physical_access(vcpu);
> +
>  	return true;
>  }
>  
> @@ -1302,6 +1304,8 @@ static bool access_pmovs(struct kvm_vcpu *vcpu, struct sys_reg_params *p,
>  		p->regval = __vcpu_sys_reg(vcpu, PMOVSSET_EL0);
>  	}
>  
> +	kvm_pmu_set_physical_access(vcpu);
> +
>  	return true;
>  }

Aren't there a ton of other registers the guest may access before
these two? Having generic PMU register accessors would allow you to
manage residence of PMU registers from a single spot.

Thanks,
Oliver