From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [PATCH v2] arm/arm64: KVM: Properly account for guest CPU time Date: Tue, 2 Jun 2015 11:28:56 +0200 Message-ID: <20150602092856.GB7783@cbox> References: <1432838950-28774-1-git-send-email-christoffer.dall@linaro.org> <556C0E22.9090401@de.ibm.com> <20150601090817.GA18722@cbox> <556C240F.5070501@de.ibm.com> <20150601133558.GA20286@cbox> <556C601C.4000501@de.ibm.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: linux-arm-kernel@lists.infradead.org, Marc Zyngier , kvmarm@lists.cs.columbia.edu, kvm@vger.kernel.org, Paolo Bonzini To: Christian Borntraeger Return-path: Content-Disposition: inline In-Reply-To: <556C601C.4000501@de.ibm.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kvmarm-bounces@lists.cs.columbia.edu Sender: kvmarm-bounces@lists.cs.columbia.edu List-Id: kvm.vger.kernel.org On Mon, Jun 01, 2015 at 03:37:32PM +0200, Christian Borntraeger wrote: > Am 01.06.2015 um 15:35 schrieb Christoffer Dall: > > On Mon, Jun 01, 2015 at 11:21:19AM +0200, Christian Borntraeger wrote: > >> Am 01.06.2015 um 11:08 schrieb Christoffer Dall: > >> > >>>>> > >>>>> Second, looking at the ppc and mips code, they seem to also call > >>>>> kvm_guest_exit() before enabling interrupts, so I don't understand how > >>>>> guest CPU time accounting works on those architectures. > >>>> > >>>> Not an expert here, but I assume mips has the same logic as arm so if your > >>>> patch is right for arm its probably also for mips. > >>>> > >>>> powerpc looks similar to what s390 does (not using the tick, instead it uses > >>>> a hw-timer) so this should be fine. > >>>> > >>> I wonder if we can simply enable HAVE_VIRT_CPU_ACCOUNTING_GEN and get > >>> this for free which would avoid the need for this patch? > >> > >> Asssuming that HAVE_VIRT_CPU_ACCOUNTING_GEN behaves similar to > >> HAVE_VIRT_CPU_ACCOUNTING on s390/power in respect to not rely on ticks > >> - yes it might work out. Can you give it a try? > >> > > Adding HAVE_VIRT_CPU_ACCOUNTING_GEN to arch/arm64/Kconfig works, but has > > no effect unless you also enable CONFIG_NO_HZ_FULL, so that hardly feels > > like a fix since it would be a shame to force users to use this config > > option to report CPU usage correctly. > > > > I'm not entirely sure what the history and meaning behind these configs > > are, so maybe there is an entirely different rework needed here. It > > seems logical that you could simply sample the counter at entry/exit of > > the guest, but if there is nowhere to store this data without > > NO_HZ_FULL+VIRT_CPU_ACCOUNTING_GEN then I guess that would be why? > > Given Paolos response that irq_disable/enable is faster than save/restore > at least on x86 your v2 patch might actually be the right thing to do. > Thanks, I think so too, but we should enable HAVE_VIRT_CPU_ACCOUNTING_GEN for arm64 as well, assuming there are no mysterious side affects of doing so. -Christoffer From mboxrd@z Thu Jan 1 00:00:00 1970 From: christoffer.dall@linaro.org (Christoffer Dall) Date: Tue, 2 Jun 2015 11:28:56 +0200 Subject: [PATCH v2] arm/arm64: KVM: Properly account for guest CPU time In-Reply-To: <556C601C.4000501@de.ibm.com> References: <1432838950-28774-1-git-send-email-christoffer.dall@linaro.org> <556C0E22.9090401@de.ibm.com> <20150601090817.GA18722@cbox> <556C240F.5070501@de.ibm.com> <20150601133558.GA20286@cbox> <556C601C.4000501@de.ibm.com> Message-ID: <20150602092856.GB7783@cbox> To: linux-arm-kernel@lists.infradead.org List-Id: linux-arm-kernel.lists.infradead.org On Mon, Jun 01, 2015 at 03:37:32PM +0200, Christian Borntraeger wrote: > Am 01.06.2015 um 15:35 schrieb Christoffer Dall: > > On Mon, Jun 01, 2015 at 11:21:19AM +0200, Christian Borntraeger wrote: > >> Am 01.06.2015 um 11:08 schrieb Christoffer Dall: > >> > >>>>> > >>>>> Second, looking at the ppc and mips code, they seem to also call > >>>>> kvm_guest_exit() before enabling interrupts, so I don't understand how > >>>>> guest CPU time accounting works on those architectures. > >>>> > >>>> Not an expert here, but I assume mips has the same logic as arm so if your > >>>> patch is right for arm its probably also for mips. > >>>> > >>>> powerpc looks similar to what s390 does (not using the tick, instead it uses > >>>> a hw-timer) so this should be fine. > >>>> > >>> I wonder if we can simply enable HAVE_VIRT_CPU_ACCOUNTING_GEN and get > >>> this for free which would avoid the need for this patch? > >> > >> Asssuming that HAVE_VIRT_CPU_ACCOUNTING_GEN behaves similar to > >> HAVE_VIRT_CPU_ACCOUNTING on s390/power in respect to not rely on ticks > >> - yes it might work out. Can you give it a try? > >> > > Adding HAVE_VIRT_CPU_ACCOUNTING_GEN to arch/arm64/Kconfig works, but has > > no effect unless you also enable CONFIG_NO_HZ_FULL, so that hardly feels > > like a fix since it would be a shame to force users to use this config > > option to report CPU usage correctly. > > > > I'm not entirely sure what the history and meaning behind these configs > > are, so maybe there is an entirely different rework needed here. It > > seems logical that you could simply sample the counter at entry/exit of > > the guest, but if there is nowhere to store this data without > > NO_HZ_FULL+VIRT_CPU_ACCOUNTING_GEN then I guess that would be why? > > Given Paolos response that irq_disable/enable is faster than save/restore > at least on x86 your v2 patch might actually be the right thing to do. > Thanks, I think so too, but we should enable HAVE_VIRT_CPU_ACCOUNTING_GEN for arm64 as well, assuming there are no mysterious side affects of doing so. -Christoffer