From: Shannon Zhao <shannon.zhao@linaro.org>
To: Marc Zyngier <marc.zyngier@arm.com>,
Shannon Zhao <zhaoshenglong@huawei.com>,
kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org
Cc: kvm@vger.kernel.org, will.deacon@arm.com,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v12 16/21] KVM: ARM64: Add PMU overflow interrupt routing
Date: Tue, 23 Feb 2016 22:52:16 +0800 [thread overview]
Message-ID: <56CC7220.3020807@linaro.org> (raw)
In-Reply-To: <56CC695E.4010805@arm.com>
On 2016/2/23 22:14, Marc Zyngier wrote:
> On 22/02/16 09:37, Shannon Zhao wrote:
>> From: Shannon Zhao <shannon.zhao@linaro.org>
>>
>> When calling perf_event_create_kernel_counter to create perf_event,
>> assign a overflow handler. Then when the perf event overflows, set the
>> corresponding bit of guest PMOVSSET register. If this counter is enabled
>> and its interrupt is enabled as well, kick the vcpu to sync the
>> interrupt.
>>
>> On VM entry, if there is counter overflowed, inject the interrupt with
>> the level set to 1. Otherwise, inject the interrupt with the level set
>> to 0.
>>
>> Signed-off-by: Shannon Zhao <shannon.zhao@linaro.org>
>> Reviewed-by: Marc Zyngier <marc.zyngier@arm.com>
>> Reviewed-by: Andrew Jones <drjones@redhat.com>
>
> As I mentioned yesterday, I was trying to pinpoint a performance drop, so I added PMU support to kvmtool (and made it an optional flag). This allowed be to find this:
>
>> ---
>> arch/arm/kvm/arm.c | 2 ++
>> include/kvm/arm_pmu.h | 2 ++
>> virt/kvm/arm/pmu.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
>> 3 files changed, 50 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
>> index dda1959..f54264c 100644
>> --- a/arch/arm/kvm/arm.c
>> +++ b/arch/arm/kvm/arm.c
>> @@ -28,6 +28,7 @@
>> #include <linux/sched.h>
>> #include <linux/kvm.h>
>> #include <trace/events/kvm.h>
>> +#include <kvm/arm_pmu.h>
>>
>> #define CREATE_TRACE_POINTS
>> #include "trace.h"
>> @@ -577,6 +578,7 @@ int kvm_arch_vcpu_ioctl_run(struct kvm_vcpu *vcpu, struct kvm_run *run)
>> * non-preemptible context.
>> */
>> preempt_disable();
>> + kvm_pmu_flush_hwstate(vcpu);
>> kvm_timer_flush_hwstate(vcpu);
>> kvm_vgic_flush_hwstate(vcpu);
>>
>> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
>> index 8bc92d1..cf68f9a 100644
>> --- a/include/kvm/arm_pmu.h
>> +++ b/include/kvm/arm_pmu.h
>> @@ -44,6 +44,7 @@ u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu);
>> void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val);
>> void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val);
>> void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val);
>> +void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu);
>> void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val);
>> void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val);
>> void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu, u64 data,
>> @@ -67,6 +68,7 @@ static inline u64 kvm_pmu_valid_counter_mask(struct kvm_vcpu *vcpu)
>> static inline void kvm_pmu_disable_counter(struct kvm_vcpu *vcpu, u64 val) {}
>> static inline void kvm_pmu_enable_counter(struct kvm_vcpu *vcpu, u64 val) {}
>> static inline void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val) {}
>> +static inline void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu) {}
>> static inline void kvm_pmu_software_increment(struct kvm_vcpu *vcpu, u64 val) {}
>> static inline void kvm_pmu_handle_pmcr(struct kvm_vcpu *vcpu, u64 val) {}
>> static inline void kvm_pmu_set_counter_event_type(struct kvm_vcpu *vcpu,
>> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
>> index cda869c..6ac52ce 100644
>> --- a/virt/kvm/arm/pmu.c
>> +++ b/virt/kvm/arm/pmu.c
>> @@ -21,6 +21,7 @@
>> #include <linux/perf_event.h>
>> #include <asm/kvm_emulate.h>
>> #include <kvm/arm_pmu.h>
>> +#include <kvm/arm_vgic.h>
>>
>> /**
>> * kvm_pmu_get_counter_value - get PMU counter value
>> @@ -181,6 +182,49 @@ void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val)
>> }
>>
>> /**
>> + * kvm_pmu_flush_hwstate - flush pmu state to cpu
>> + * @vcpu: The vcpu pointer
>> + *
>> + * Inject virtual PMU IRQ if IRQ is pending for this cpu.
>> + */
>> +void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu)
>> +{
>> + struct kvm_pmu *pmu = &vcpu->arch.pmu;
>> + u64 overflow;
>> +
>> + if (!kvm_arm_pmu_v3_ready(vcpu))
>> + return;
>> +
>> + overflow = kvm_pmu_overflow_status(vcpu);
>> + kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, pmu->irq_num, !!overflow);
>
> It turns out that this single line costs us about 400 cycles on each entry:
>
Oh, it's really a time-consuming function.
> maz@flakes:~/kvm-ws-tests$ make LKVM=~/kvmtool/lkvm LKVM_ARGS=--pmu PERF=perf_4.3 tests-gicv2
> GICv2:
> do_hvc.bin:5690.17
> do_sgi.bin:9395.05
> do_sysreg.bin:5912.6
> maz@flakes:~/kvm-ws-tests$ make LKVM=~/kvmtool/lkvm PERF=perf_4.3 tests-gicv2
> GICv2:
> do_hvc.bin:5285.02
> do_sgi.bin:9131.24
> do_sysreg.bin:5563.7
>
> Caching the irq state and only injecting it if it has changed (just like we do for the timer) brings performance back to its previous level:
>
Marc, thanks a lot for finding out this. I'll merge below changes into
this series and send out v13 tomorrow.
> diff --git a/include/kvm/arm_pmu.h b/include/kvm/arm_pmu.h
> index 176913f..b23e636 100644
> --- a/include/kvm/arm_pmu.h
> +++ b/include/kvm/arm_pmu.h
> @@ -35,6 +35,7 @@ struct kvm_pmu {
> int irq_num;
> struct kvm_pmc pmc[ARMV8_PMU_MAX_COUNTERS];
> bool ready;
> + bool irq_level;
> };
>
> #define kvm_arm_pmu_v3_ready(v) ((v)->arch.pmu.ready)
> diff --git a/virt/kvm/arm/pmu.c b/virt/kvm/arm/pmu.c
> index 904617e..7156f8b 100644
> --- a/virt/kvm/arm/pmu.c
> +++ b/virt/kvm/arm/pmu.c
> @@ -229,13 +229,17 @@ void kvm_pmu_overflow_set(struct kvm_vcpu *vcpu, u64 val)
> void kvm_pmu_flush_hwstate(struct kvm_vcpu *vcpu)
> {
> struct kvm_pmu *pmu = &vcpu->arch.pmu;
> - u64 overflow;
> + bool overflow;
>
> if (!kvm_arm_pmu_v3_ready(vcpu))
> return;
>
> - overflow = kvm_pmu_overflow_status(vcpu);
> - kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, pmu->irq_num, !!overflow);
> + overflow = !!kvm_pmu_overflow_status(vcpu);
> + if (pmu->irq_level != overflow) {
> + pmu->irq_level = overflow;
> + kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id,
> + pmu->irq_num, overflow);
> + }
> }
>
> static inline struct kvm_vcpu *kvm_pmc_to_vcpu(struct kvm_pmc *pmc)
>
>
> Thanks,
>
> M.
>
--
Shannon
next prev parent reply other threads:[~2016-02-23 14:52 UTC|newest]
Thread overview: 30+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-02-22 9:37 [PATCH v12 00/21] KVM: ARM64: Add guest PMU support Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 01/21] ARM64: Move PMU register related defines to asm/perf_event.h Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 02/21] KVM: ARM64: Define PMU data structure for each vcpu Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 03/21] KVM: ARM64: Add offset defines for PMU registers Shannon Zhao
2016-02-22 17:51 ` Marc Zyngier
2016-02-23 1:46 ` Shannon Zhao
2016-02-23 8:33 ` Marc Zyngier
2016-02-23 9:29 ` Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 04/21] KVM: ARM64: Add access handler for PMCR register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 05/21] KVM: ARM64: Add access handler for PMSELR register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 06/21] KVM: ARM64: Add access handler for PMCEID0 and PMCEID1 register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 07/21] KVM: ARM64: PMU: Add perf event map and introduce perf event creating function Shannon Zhao
2016-02-23 17:42 ` Marc Zyngier
2016-02-24 1:27 ` Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 08/21] KVM: ARM64: Add access handler for event type register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 09/21] KVM: ARM64: Add access handler for event counter register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 10/21] KVM: ARM64: Add access handler for PMCNTENSET and PMCNTENCLR register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 11/21] KVM: ARM64: Add access handler for PMINTENSET and PMINTENCLR register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 12/21] KVM: ARM64: Add access handler for PMOVSSET and PMOVSCLR register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 13/21] KVM: ARM64: Add access handler for PMSWINC register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 14/21] KVM: ARM64: Add helper to handle PMCR register bits Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 15/21] KVM: ARM64: Add access handler for PMUSERENR register Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 16/21] KVM: ARM64: Add PMU overflow interrupt routing Shannon Zhao
2016-02-23 14:14 ` Marc Zyngier
2016-02-23 14:52 ` Shannon Zhao [this message]
2016-02-22 9:37 ` [PATCH v12 17/21] KVM: ARM64: Reset PMU state when resetting vcpu Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 18/21] KVM: ARM64: Free perf event of PMU when destroying vcpu Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 19/21] KVM: ARM64: Add a new feature bit for PMUv3 Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 20/21] KVM: ARM: Introduce per-vcpu kvm device controls Shannon Zhao
2016-02-22 9:37 ` [PATCH v12 21/21] KVM: ARM64: Add a new vcpu device control group for PMUv3 Shannon Zhao
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=56CC7220.3020807@linaro.org \
--to=shannon.zhao@linaro.org \
--cc=christoffer.dall@linaro.org \
--cc=kvm@vger.kernel.org \
--cc=kvmarm@lists.cs.columbia.edu \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=marc.zyngier@arm.com \
--cc=will.deacon@arm.com \
--cc=zhaoshenglong@huawei.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: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).