From: Andrew Murray <andrew.murray@arm.com> To: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: marc.zyngier@arm.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v9 4/5] KVM: arm/arm64: remove pmc->bitmask Date: Mon, 17 Jun 2019 19:06:58 +0100 [thread overview] Message-ID: <20190617180658.GN20984@e119886-lin.cambridge.arm.com> (raw) In-Reply-To: <f3ea8075-b395-9fa4-afba-771011f62b59@arm.com> On Mon, Jun 17, 2019 at 05:33:40PM +0100, Suzuki K Poulose wrote: > > > On 17/06/2019 16:43, Andrew Murray wrote: > > On Thu, Jun 13, 2019 at 05:50:43PM +0100, Suzuki K Poulose wrote: > > > > > > > > > On 13/06/2019 10:39, Andrew Murray wrote: > > > > On Thu, Jun 13, 2019 at 08:30:51AM +0100, Julien Thierry wrote: > > > > > > > index ae1e886d4a1a..88ce24ae0b45 100644 > > > > > > --- a/virt/kvm/arm/pmu.c > > > > > > +++ b/virt/kvm/arm/pmu.c > > > > > > @@ -47,7 +47,10 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) > > > > > > counter += perf_event_read_value(pmc->perf_event, &enabled, > > > > > > &running); > > > > > > - return counter & pmc->bitmask; > > > > > > + if (select_idx != ARMV8_PMU_CYCLE_IDX) > > > > > > + counter = lower_32_bits(counter); > > > > > > > > > > Shouldn't this depend on PMCR.LC as well? If PMCR.LC is clear we only > > > > > want the lower 32bits of the cycle counter. > > > > > > > > Yes that's correct. The hunk should look like this: > > > > > > > > - return counter & pmc->bitmask; > > > > + if (!(select_idx == ARMV8_PMU_CYCLE_IDX && > > > > + __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC)) > > > > + counter = lower_32_bits(counter); > > > > + > > > > + return counter; > > > > > > May be you could add a macro : > > > > > > #define vcpu_pmu_counter_is_64bit(vcpu, idx) ? > > > > Yes I think a helper would be useful - though I'd prefer the name > > 'kvm_pmu_idx_is_long_cycle_counter'. This seems a little clearer as > > you could otherwise argue that a chained counter is also 64 bits. > > When you get to add 64bit PMU counter (v8.5), this would be handy. So > having it de-coupled from the cycle counter may be a good idea. Anyways, > I leave that to you. Yes that thought crossed my mind. I'll take your suggestion afterall. Thanks, Andrew Murray > > Cheers > Suzuki _______________________________________________ kvmarm mailing list kvmarm@lists.cs.columbia.edu https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
WARNING: multiple messages have this Message-ID (diff)
From: Andrew Murray <andrew.murray@arm.com> To: Suzuki K Poulose <suzuki.poulose@arm.com> Cc: julien.thierry@arm.com, marc.zyngier@arm.com, christoffer.dall@arm.com, james.morse@arm.com, kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: Re: [PATCH v9 4/5] KVM: arm/arm64: remove pmc->bitmask Date: Mon, 17 Jun 2019 19:06:58 +0100 [thread overview] Message-ID: <20190617180658.GN20984@e119886-lin.cambridge.arm.com> (raw) In-Reply-To: <f3ea8075-b395-9fa4-afba-771011f62b59@arm.com> On Mon, Jun 17, 2019 at 05:33:40PM +0100, Suzuki K Poulose wrote: > > > On 17/06/2019 16:43, Andrew Murray wrote: > > On Thu, Jun 13, 2019 at 05:50:43PM +0100, Suzuki K Poulose wrote: > > > > > > > > > On 13/06/2019 10:39, Andrew Murray wrote: > > > > On Thu, Jun 13, 2019 at 08:30:51AM +0100, Julien Thierry wrote: > > > > > > > index ae1e886d4a1a..88ce24ae0b45 100644 > > > > > > --- a/virt/kvm/arm/pmu.c > > > > > > +++ b/virt/kvm/arm/pmu.c > > > > > > @@ -47,7 +47,10 @@ u64 kvm_pmu_get_counter_value(struct kvm_vcpu *vcpu, u64 select_idx) > > > > > > counter += perf_event_read_value(pmc->perf_event, &enabled, > > > > > > &running); > > > > > > - return counter & pmc->bitmask; > > > > > > + if (select_idx != ARMV8_PMU_CYCLE_IDX) > > > > > > + counter = lower_32_bits(counter); > > > > > > > > > > Shouldn't this depend on PMCR.LC as well? If PMCR.LC is clear we only > > > > > want the lower 32bits of the cycle counter. > > > > > > > > Yes that's correct. The hunk should look like this: > > > > > > > > - return counter & pmc->bitmask; > > > > + if (!(select_idx == ARMV8_PMU_CYCLE_IDX && > > > > + __vcpu_sys_reg(vcpu, PMCR_EL0) & ARMV8_PMU_PMCR_LC)) > > > > + counter = lower_32_bits(counter); > > > > + > > > > + return counter; > > > > > > May be you could add a macro : > > > > > > #define vcpu_pmu_counter_is_64bit(vcpu, idx) ? > > > > Yes I think a helper would be useful - though I'd prefer the name > > 'kvm_pmu_idx_is_long_cycle_counter'. This seems a little clearer as > > you could otherwise argue that a chained counter is also 64 bits. > > When you get to add 64bit PMU counter (v8.5), this would be handy. So > having it de-coupled from the cycle counter may be a good idea. Anyways, > I leave that to you. Yes that thought crossed my mind. I'll take your suggestion afterall. Thanks, Andrew Murray > > Cheers > Suzuki _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
next prev parent reply other threads:[~2019-06-17 18:07 UTC|newest] Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-06-12 19:04 [PATCH v9 0/5] KVM: arm/arm64: add support for chained counters Andrew Murray 2019-06-12 19:04 ` Andrew Murray 2019-06-12 19:04 ` [PATCH v9 1/5] KVM: arm/arm64: rename kvm_pmu_{enable/disable}_counter functions Andrew Murray 2019-06-12 19:04 ` Andrew Murray 2019-06-12 19:04 ` [PATCH v9 2/5] KVM: arm/arm64: extract duplicated code to own function Andrew Murray 2019-06-12 19:04 ` Andrew Murray 2019-06-12 19:04 ` [PATCH v9 3/5] KVM: arm/arm64: re-create event when setting counter value Andrew Murray 2019-06-12 19:04 ` Andrew Murray 2019-06-12 19:04 ` [PATCH v9 4/5] KVM: arm/arm64: remove pmc->bitmask Andrew Murray 2019-06-12 19:04 ` Andrew Murray 2019-06-13 7:30 ` Julien Thierry 2019-06-13 7:30 ` Julien Thierry 2019-06-13 9:39 ` Andrew Murray 2019-06-13 9:39 ` Andrew Murray 2019-06-13 16:50 ` Suzuki K Poulose 2019-06-13 16:50 ` Suzuki K Poulose 2019-06-17 15:43 ` Andrew Murray 2019-06-17 15:43 ` Andrew Murray 2019-06-17 16:33 ` Suzuki K Poulose 2019-06-17 16:33 ` Suzuki K Poulose 2019-06-17 18:06 ` Andrew Murray [this message] 2019-06-17 18:06 ` Andrew Murray 2019-06-12 19:04 ` [PATCH v9 5/5] KVM: arm/arm64: support chained PMU counters Andrew Murray 2019-06-12 19:04 ` Andrew Murray 2019-06-14 12:08 ` Suzuki K Poulose 2019-06-14 12:08 ` Suzuki K Poulose
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190617180658.GN20984@e119886-lin.cambridge.arm.com \ --to=andrew.murray@arm.com \ --cc=kvmarm@lists.cs.columbia.edu \ --cc=linux-arm-kernel@lists.infradead.org \ --cc=marc.zyngier@arm.com \ --cc=suzuki.poulose@arm.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.