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

[Shijie Huang]

Hi Leo,

在 2023/8/14 18:01, Leo Yan 写道:
> Hi Shijie,
>
> On Mon, Aug 14, 2023 at 05:29:54PM +0800, Shijie Huang wrote:
>
>
> [...]
>
>>> 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...
> You mean if set two guest events, the kvm_vcpu_pmu_resync_el0() will
> be invoked twice, and the second calling is not reasonable, right?

IMHO, even the first time is not reasonable. why call 
kvm_vcpu_pmu_resync_el0() when event rotation

does not happen?



> I can accept this since I personally think this should not introduce
> much performance penalty.
>
> I understand your preference to call kvm_vcpu_pmu_resync_el0() from
> perf core layer, but this is not a common issue for all PMU events and
> crossing arches.  Furthermore, even perf core rotates events, it's not

If we can find a better way to fix it in PMU code, I am okay too. :)

I tried to fix it in PMU code, but I am not satified with it.


> necessarily mean we must restore events for guest in the case there
> have no event is enabled for guest.

Not only for events in guest, but also for the events in the host too.

In the kvm_vcpu_pmu_restore_guest(), it also disable the EL0 for host 
events.


Thanks

Huang Shijie

>
> Thanks,
> Leo