Hi Akihiko,
On Wed, Feb 25, 2026 at 01:31:15PM +0900, Akihiko Odaki wrote:
> @@ -629,6 +629,8 @@ void kvm_arch_vcpu_load(struct kvm_vcpu *vcpu, int cpu)
> kvm_vcpu_load_vhe(vcpu);
> kvm_arch_vcpu_load_fp(vcpu);
> kvm_vcpu_pmu_restore_guest(vcpu);
> + if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
> &vcpu->kvm->arch.flags))
> + kvm_make_request(KVM_REQ_CREATE_PMU, vcpu);
We only need to set the request if the vCPU has migrated to a different
PMU implementation, no?
> if (kvm_arm_is_pvtime_enabled(&vcpu->arch))
> kvm_make_request(KVM_REQ_RECORD_STEAL, vcpu);
>
> @@ -1056,6 +1058,9 @@ static int check_vcpu_requests(struct kvm_vcpu *vcpu)
> if (kvm_check_request(KVM_REQ_RELOAD_PMU, vcpu))
> kvm_vcpu_reload_pmu(vcpu);
>
> + if (kvm_check_request(KVM_REQ_CREATE_PMU, vcpu))
> + kvm_vcpu_create_pmu(vcpu);
> +
My strong preference would be to squash the migration handling into
kvm_vcpu_reload_pmu(). It is already reprogramming PMU events in
response to other things.
> if (kvm_check_request(KVM_REQ_RESYNC_PMU_EL0, vcpu))
> kvm_vcpu_pmu_restore_guest(vcpu);
>
> @@ -1516,7 +1521,8 @@ static int kvm_setup_vcpu(struct kvm_vcpu *vcpu)
> * When the vCPU has a PMU, but no PMU is set for the guest
> * yet, set the default one.
> */
> - if (kvm_vcpu_has_pmu(vcpu) && !kvm->arch.arm_pmu)
> + if (kvm_vcpu_has_pmu(vcpu) && !kvm->arch.arm_pmu &&
> + !test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
> &kvm->arch.flags))
> ret = kvm_arm_set_default_pmu(kvm);
I'd rather just initialize it to a default than have to deal with the
field being sometimes null.
> -static bool kvm_pmu_counter_is_enabled(struct kvm_pmc *pmc)
> +static u64 kvm_pmu_enabled_counter_mask(struct kvm_vcpu *vcpu)
> {
> - struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
> - unsigned int mdcr = __vcpu_sys_reg(vcpu, MDCR_EL2);
> + u64 mask = 0;
>
> - if (!(__vcpu_sys_reg(vcpu, PMCNTENSET_EL0) & BIT(pmc->idx)))
> - return false;
> + if (__vcpu_sys_reg(vcpu, MDCR_EL2) & MDCR_EL2_HPME)
> + mask |= kvm_pmu_hyp_counter_mask(vcpu);
>
> - if (kvm_pmu_counter_is_hyp(vcpu, pmc->idx))
> - return mdcr & MDCR_EL2_HPME;
> + if (kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E)
> + mask |= ~kvm_pmu_hyp_counter_mask(vcpu);
>
> - return kvm_vcpu_read_pmcr(vcpu) & ARMV8_PMU_PMCR_E;
> + return __vcpu_sys_reg(vcpu, PMCNTENSET_EL0) & mask;
> +}
> +
> +static bool kvm_pmu_counter_is_enabled(struct kvm_pmc *pmc)
> +{
> + struct kvm_vcpu *vcpu = kvm_pmc_to_vcpu(pmc);
> +
> + return kvm_pmu_enabled_counter_mask(vcpu) & BIT(pmc->idx);
> }
You're churning a good bit of code, this needs to happen in a separate
patch (if at all).
> @@ -689,6 +710,14 @@ static void kvm_pmu_create_perf_event(struct kvm_pmc
> *pmc)
> int eventsel;
> u64 evtreg;
>
> + if (!arm_pmu) {
> + arm_pmu = kvm_pmu_probe_armpmu(vcpu->cpu);
kvm_pmu_probe_armpmu() takes a global mutex, I'm not sure that's what we
want.
What prevents us from opening a PERF_TYPE_RAW event and allowing perf to
work out the right PMU for this CPU?
> + if (!arm_pmu) {
> + vcpu_set_on_unsupported_cpu(vcpu);
At this point it seems pretty late to flag the CPU as unsupported. Maybe
instead we can compute the union cpumask for all the PMU implemetations
the VM may schedule on.
> @@ -1249,6 +1299,10 @@ int kvm_arm_pmu_v3_get_attr(struct kvm_vcpu *vcpu,
> struct kvm_device_attr *attr)
> irq = vcpu->arch.pmu.irq_num;
> return put_user(irq, uaddr);
> }
> + case KVM_ARM_VCPU_PMU_V3_FIXED_COUNTERS_ONLY:
> + lockdep_assert_held(&vcpu->kvm->arch.config_lock);
> + if (test_bit(KVM_ARCH_FLAG_PMU_V3_FIXED_COUNTERS_ONLY,
> &vcpu->kvm->arch.flags))
> + return 0;
We don't need a getter for this, userspace should remember how it
provisioned the VM.
Thanks,
Oliver
