Re: [PATCH v8 5/6] KVM: arm/arm64: remove pmc->bitmask

[Suzuki K Poulose <suzuki.poulose@arm.com>]

On 22/05/2019 17:26, Andrew Murray wrote:
> On Wed, May 22, 2019 at 05:07:31PM +0100, Marc Zyngier wrote:
>> On 22/05/2019 16:30, Andrew Murray wrote:
>>> We currently use pmc->bitmask to determine the width of the pmc - however
>>> it's superfluous as the pmc index already describes if the pmc is a cycle
>>> counter or event counter. The architecture clearly describes the widths of
>>> these counters.
>>>
>>> Let's remove the bitmask to simplify the code.
>>>
>>> Signed-off-by: Andrew Murray <andrew.murray@arm.com>
>>> ---
>>>   include/kvm/arm_pmu.h |  1 -
>>>   virt/kvm/arm/pmu.c    | 15 +++++----------
>>>   2 files changed, 5 insertions(+), 11 deletions(-)
>>>
>>> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
>>> index b73f31baca52..2f0e28dc5a9e 100644
>>> --- a/include/kvm/arm_pmu.h
>>> +++ b/include/kvm/arm_pmu.h
>>> @@ -28,7 +28,6 @@
>>>   struct kvm_pmc {
>>>   	u8 idx;	/* index into the pmu->pmc array */
>>>   	struct perf_event *perf_event;
>>> -	u64 bitmask;
>>>   };
>>>   


>>> -
>>> -	if (val & ARMV8_PMU_PMCR_LC) {
>>> -		pmc = &pmu->pmc[ARMV8_PMU_CYCLE_IDX];
>>> -		pmc->bitmask = 0xffffffffffffffffUL;
>>> -	}
>>>   }

...

>>>   
>>>   static bool kvm_pmu_counter_is_enabled(struct kvm_vcpu *vcpu, u64 select_idx)
>>> @@ -420,7 +415,7 @@ static void kvm_pmu_create_perf_event(struct kvm_vcpu *vcpu, u64 select_idx)
>>>   
>>>   	counter = kvm_pmu_get_counter_value(vcpu, select_idx);
>>>   	/* The initial sample period (overflow count) of an event. */
>>> -	attr.sample_period = (-counter) & pmc->bitmask;
>>> +	attr.sample_period = (-counter) & GENMASK(31, 0);
>>
>> Isn't this the one case where the bitmask actually matters? If we're
>> dealing with the cycle counter, it shouldn't be truncated, right?
> 
> Ah yes, that should be conditional on idx as well.

The mask for Cycle counter also depends on the PMCR.LC field set by the
guest, isn't it ? So unless we correlate that with the idx, we could be
passing in wrong results ?

Suzuki
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm