Hi Oliver,
> This doesn't actually disallow userspace from configuring a vPMU, it
> only hides the KVM cap. kvm_host_pmu_init() will still insert the host
> PMU instance in the list of valid PMUs, and there doesn't appear to be
> any check against the static key anywhere on that path.
In v6.5-rc3, which I used as the base, or even in v6.5-rc4,
it appears kvm_reset_vcpu() checks against the static key.
So, the initial KVM_ARM_VCPU_INIT with vPMU configured will
fail on the systems. Or am I missing something ? (Or is that
going to be removed by other patches that are already queued?)
But, right, it still insert the host PMU instance in the list,
which is unnecessary.
> I actually prefer where we flip the static key, as PMU context switching
> depends on both KVM support as well as the PMU driver coming up successfully.
> Instead, you could hoist the check against the sanitised PMU version into
> kvm_host_pmu_init(), maybe something like:
Thank you, it looks better. I will fix this in v3.
Thank you,
Reiji
> diff --git a/arch/arm64/kvm/pmu-emul.c b/arch/arm64/kvm/pmu-emul.c
> index 560650972478..f6a0e558472f 100644
> --- a/arch/arm64/kvm/pmu-emul.c
> +++ b/arch/arm64/kvm/pmu-emul.c
> @@ -672,8 +672,11 @@ void kvm_host_pmu_init(struct arm_pmu *pmu)
> {
> struct arm_pmu_entry *entry;
>
> - if (pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_NI ||
> - pmu->pmuver == ID_AA64DFR0_EL1_PMUVer_IMP_DEF)
> + /*
> + * Check the sanitised PMU version for the system, as KVM does not
> + * support implementations where PMUv3 exists on a subset of CPUs.
> + */
> + if (!pmuv3_implemented(kvm_arm_pmu_get_pmuver_limit()))
> return;
>
> mutex_lock(&arm_pmus_lock);
>
> --
> Thanks,
> Oliver
