From mboxrd@z Thu Jan 1 00:00:00 1970 From: Marc Zyngier Subject: Re: [PATCH v12 03/21] KVM: ARM64: Add offset defines for PMU registers Date: Mon, 22 Feb 2016 17:51:20 +0000 Message-ID: <56CB4A98.2020403@arm.com> References: <1456133877-9584-1-git-send-email-zhaoshenglong@huawei.com> <1456133877-9584-4-git-send-email-zhaoshenglong@huawei.com> Mime-Version: 1.0 Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Cc: linux-arm-kernel@lists.infradead.org, kvm@vger.kernel.org, will.deacon@arm.com, wei@redhat.com, cov@codeaurora.org, shannon.zhao@linaro.org, peter.huangpeng@huawei.com, hangaohuai@huawei.com To: Shannon Zhao , kvmarm@lists.cs.columbia.edu, christoffer.dall@linaro.org Return-path: Received: from foss.arm.com ([217.140.101.70]:59219 "EHLO foss.arm.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751937AbcBVRvX (ORCPT ); Mon, 22 Feb 2016 12:51:23 -0500 In-Reply-To: <1456133877-9584-4-git-send-email-zhaoshenglong@huawei.com> Sender: kvm-owner@vger.kernel.org List-ID: On 22/02/16 09:37, Shannon Zhao wrote: > From: Shannon Zhao > > We are about to trap and emulate accesses to each PMU register > individually. This adds the context offsets for the AArch64 PMU > registers. > > Signed-off-by: Shannon Zhao > Reviewed-by: Marc Zyngier > Reviewed-by: Andrew Jones > --- > arch/arm64/include/asm/kvm_host.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 6f0241f..6bab7fb 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -115,6 +115,21 @@ enum vcpu_sysreg { > MDSCR_EL1, /* Monitor Debug System Control Register */ > MDCCINT_EL1, /* Monitor Debug Comms Channel Interrupt Enable Reg */ > > + /* Performance Monitors Registers */ > + PMCR_EL0, /* Control Register */ > + PMOVSSET_EL0, /* Overflow Flag Status Set Register */ > + PMSELR_EL0, /* Event Counter Selection Register */ > + PMEVCNTR0_EL0, /* Event Counter Register (0-30) */ > + PMEVCNTR30_EL0 = PMEVCNTR0_EL0 + 30, > + PMCCNTR_EL0, /* Cycle Counter Register */ > + PMEVTYPER0_EL0, /* Event Type Register (0-30) */ > + PMEVTYPER30_EL0 = PMEVTYPER0_EL0 + 30, > + PMCCFILTR_EL0, /* Cycle Count Filter Register */ > + PMCNTENSET_EL0, /* Count Enable Set Register */ > + PMINTENSET_EL1, /* Interrupt Enable Set Register */ > + PMUSERENR_EL0, /* User Enable Register */ > + PMSWINC_EL0, /* Software Increment Register */ > + I've just noticed a rather fundamental issue with this: this makes it impossible to bisect the whole series. I was trying to pinpoint a performance regression with this series, and started bisecting. Unfortunately, declaring these registers in one go means that we end-up with uninitialized registers after this patch (and probably until PMUSERENR is dealt with). The consequence of that is something like this: Kernel panic - not syncing: Didn't reset vcpu_sys_reg(25) CPU: 1 PID: 1994 Comm: lkvm Tainted: G W 4.5.0-rc5+ #5563 Hardware name: Default string Default string/Default string, BIOS ROD0084E 09/03/2015 Call trace: [] dump_backtrace+0x0/0x1a8 [] show_stack+0x14/0x20 [] dump_stack+0x94/0xb8 [] panic+0x10c/0x250 [] kvm_reset_sys_regs+0xec/0xf0 [] kvm_reset_vcpu+0x58/0x80 [] kvm_arch_vcpu_ioctl+0x294/0x310 [] kvm_vcpu_ioctl+0xcc/0x698 [] do_vfs_ioctl+0xa4/0x750 [] SyS_ioctl+0x8c/0xa0 [] el0_svc_naked+0x24/0x28 The obvious fix would be to introduce each register with the patch that handles it. At least, we'll be able to bisect it... Thanks, M. -- Jazz is not dead. It just smells funny... From mboxrd@z Thu Jan 1 00:00:00 1970 From: marc.zyngier@arm.com (Marc Zyngier) Date: Mon, 22 Feb 2016 17:51:20 +0000 Subject: [PATCH v12 03/21] KVM: ARM64: Add offset defines for PMU registers In-Reply-To: <1456133877-9584-4-git-send-email-zhaoshenglong@huawei.com> References: <1456133877-9584-1-git-send-email-zhaoshenglong@huawei.com> <1456133877-9584-4-git-send-email-zhaoshenglong@huawei.com> Message-ID: <56CB4A98.2020403@arm.com> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On 22/02/16 09:37, Shannon Zhao wrote: > From: Shannon Zhao > > We are about to trap and emulate accesses to each PMU register > individually. This adds the context offsets for the AArch64 PMU > registers. > > Signed-off-by: Shannon Zhao > Reviewed-by: Marc Zyngier > Reviewed-by: Andrew Jones > --- > arch/arm64/include/asm/kvm_host.h | 15 +++++++++++++++ > 1 file changed, 15 insertions(+) > > diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h > index 6f0241f..6bab7fb 100644 > --- a/arch/arm64/include/asm/kvm_host.h > +++ b/arch/arm64/include/asm/kvm_host.h > @@ -115,6 +115,21 @@ enum vcpu_sysreg { > MDSCR_EL1, /* Monitor Debug System Control Register */ > MDCCINT_EL1, /* Monitor Debug Comms Channel Interrupt Enable Reg */ > > + /* Performance Monitors Registers */ > + PMCR_EL0, /* Control Register */ > + PMOVSSET_EL0, /* Overflow Flag Status Set Register */ > + PMSELR_EL0, /* Event Counter Selection Register */ > + PMEVCNTR0_EL0, /* Event Counter Register (0-30) */ > + PMEVCNTR30_EL0 = PMEVCNTR0_EL0 + 30, > + PMCCNTR_EL0, /* Cycle Counter Register */ > + PMEVTYPER0_EL0, /* Event Type Register (0-30) */ > + PMEVTYPER30_EL0 = PMEVTYPER0_EL0 + 30, > + PMCCFILTR_EL0, /* Cycle Count Filter Register */ > + PMCNTENSET_EL0, /* Count Enable Set Register */ > + PMINTENSET_EL1, /* Interrupt Enable Set Register */ > + PMUSERENR_EL0, /* User Enable Register */ > + PMSWINC_EL0, /* Software Increment Register */ > + I've just noticed a rather fundamental issue with this: this makes it impossible to bisect the whole series. I was trying to pinpoint a performance regression with this series, and started bisecting. Unfortunately, declaring these registers in one go means that we end-up with uninitialized registers after this patch (and probably until PMUSERENR is dealt with). The consequence of that is something like this: Kernel panic - not syncing: Didn't reset vcpu_sys_reg(25) CPU: 1 PID: 1994 Comm: lkvm Tainted: G W 4.5.0-rc5+ #5563 Hardware name: Default string Default string/Default string, BIOS ROD0084E 09/03/2015 Call trace: [] dump_backtrace+0x0/0x1a8 [] show_stack+0x14/0x20 [] dump_stack+0x94/0xb8 [] panic+0x10c/0x250 [] kvm_reset_sys_regs+0xec/0xf0 [] kvm_reset_vcpu+0x58/0x80 [] kvm_arch_vcpu_ioctl+0x294/0x310 [] kvm_vcpu_ioctl+0xcc/0x698 [] do_vfs_ioctl+0xa4/0x750 [] SyS_ioctl+0x8c/0xa0 [] el0_svc_naked+0x24/0x28 The obvious fix would be to introduce each register with the patch that handles it. At least, we'll be able to bisect it... Thanks, M. -- Jazz is not dead. It just smells funny...