[PATCH] KVM: arm64: pmu: Resync EL0 state on counter rotation

[Shijie Huang]

Hi Leo,

在 2023/8/14 16:47, Leo Yan 写道:
> Hi Shijie,
>
> On Mon, Aug 14, 2023 at 04:12:23PM +0800, Shijie Huang wrote:
>
> [...]
>
>>>> Their investigation indicates that upon counter rotation (which
>>>> takes place on the back of a timer interrupt), we fail to
>>>> re-apply the guest EL0 enabling, leading to the counting of host
>>>> events instead of guest events.
>>> Seems to me, it's not clear for why the counter rotation will cause
>>> the issue.
>>>
>>> In the example shared by Shijie in [1], the cycle counter is enabled for
>>> both host and guest, and cycle counter is a dedicated event which does
>>> not share counter with other events.  Even there have counter rotation,
>>> it should not impact the cycle counter.
>> Just take a simple case:
>>
>>     perf stat -e cycles:G,cycles:H, e2,e3,e4,e5,e6,e7 ....
>>
>>
>> Assume we have 8 events, but PMU only privides 7 counters(cycle + 6 normal)
> Thanks for the detailed info, now I understand it.
>
> Seems to me, based on Marc's patch, we need to apply below change.  In
> below code, we don't need to change the perf core code and we can
> resolve it as a common issue for Arm PMU drivers.
>
>
> diff --git a/arch/arm64/kvm/pmu.c b/arch/arm64/kvm/pmu.c
> index 121f1a14c829..8f9673cdadec 100644
> --- a/arch/arm64/kvm/pmu.c
> +++ b/arch/arm64/kvm/pmu.c
> @@ -38,14 +38,20 @@ struct kvm_pmu_events *kvm_get_pmu_events(void)
>   void kvm_set_pmu_events(u32 set, struct perf_event_attr *attr)
>   {
>   	struct kvm_pmu_events *pmu = kvm_get_pmu_events();
> +	int resync;
>   
>   	if (!kvm_arm_support_pmu_v3() || !pmu || !kvm_pmu_switch_needed(attr))
>   		return;
>   
> +	resync = pmu->events_guest != set;

If we set two events in guest, the resync will set

For example:

            perf stat -e cycles:Gu, cycles:Gk


If so, this is not reasonble...


Thanks

Huang Shijie

> +
>   	if (!attr->exclude_host)
>   		pmu->events_host |= set;
>   	if (!attr->exclude_guest)
>   		pmu->events_guest |= set;
> +
> +	if (resync)
> +		kvm_vcpu_pmu_resync_el0();
>   }
>   
>   /*
> @@ -60,6 +66,8 @@ void kvm_clr_pmu_events(u32 clr)
>   
>   	pmu->events_host &= ~clr;
>   	pmu->events_guest &= ~clr;
> +
> +	kvm_vcpu_pmu_resync_el0();
>   }
>