Re: [RFC PATCH v3 09/29] KVM: arm64: Hide IMPLEMENTATION DEFINED PMU support for the guest

[Eric Auger <eauger@redhat.com>]

Hi Reiji,

On 12/4/21 2:04 AM, Reiji Watanabe wrote:
> Hi Eric,
> 
> On Thu, Dec 2, 2021 at 2:57 AM Eric Auger <eauger@redhat.com> wrote:
>>
>> Hi Reiji,
>>
>> On 11/30/21 6:32 AM, Reiji Watanabe wrote:
>>> Hi Eric,
>>>
>>> On Thu, Nov 25, 2021 at 12:30 PM Eric Auger <eauger@redhat.com> wrote:
>>>>
>>>> Hi Reiji,
>>>>
>>>> On 11/17/21 7:43 AM, Reiji Watanabe wrote:
>>>>> When ID_AA64DFR0_EL1.PMUVER or ID_DFR0_EL1.PERFMON is 0xf, which
>>>>> means IMPLEMENTATION DEFINED PMU supported, KVM unconditionally
>>>>> expose the value for the guest as it is.  Since KVM doesn't support
>>>>> IMPLEMENTATION DEFINED PMU for the guest, in that case KVM should
>>>>> exopse 0x0 (PMU is not implemented) instead.
>>>> s/exopse/expose
>>>>>
>>>>> Change cpuid_feature_cap_perfmon_field() to update the field value
>>>>> to 0x0 when it is 0xf.
>>>> is it wrong to expose the guest with a Perfmon value of 0xF? Then the
>>>> guest should not use it as a PMUv3?
>>>
>>>> is it wrong to expose the guest with a Perfmon value of 0xF? Then the
>>>> guest should not use it as a PMUv3?
>>>
>>> For the value 0xf in ID_AA64DFR0_EL1.PMUVER and ID_DFR0_EL1.PERFMON,
>>> Arm ARM says:
>>>   "IMPLEMENTATION DEFINED form of performance monitors supported,
>>>    PMUv3 not supported."
>>>
>>> Since the PMU that KVM supports for guests is PMUv3, 0xf shouldn't
>>> be exposed to guests (And this patch series doesn't allow userspace
>>> to set the fields to 0xf for guests).
>> What I don't get is why this isn't detected before (in kvm_reset_vcpu).
>> if the VCPU was initialized with KVM_ARM_VCPU_PMU_V3 can we honor this
>> init request if the host pmu is implementation defined?
> 
> KVM_ARM_VCPU_INIT with KVM_ARM_VCPU_PMU_V3 will fail in
> kvm_reset_vcpu() if the host PMU is implementation defined.

OK. This was not obvsious to me.

                if (kvm_vcpu_has_pmu(vcpu) && !kvm_arm_support_pmu_v3()) {
                        ret = -EINVAL;
                        goto out;
                }

kvm_perf_init
+	if (perf_num_counters() > 0)
+		static_branch_enable(&kvm_arm_pmu_available);

But I believe you ;-), sorry for the noise

Eric

> 
> The AA64DFR0 and DFR0 registers for a vCPU without KVM_ARM_VCPU_PMU_V3
> indicates IMPLEMENTATION DEFINED PMU support, which is not correct.


> 
> Thanks,
> Reiji
>