From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751626AbdBAIEr (ORCPT ); Wed, 1 Feb 2017 03:04:47 -0500 Received: from mail-lf0-f50.google.com ([209.85.215.50]:33994 "EHLO mail-lf0-f50.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751388AbdBAIEo (ORCPT ); Wed, 1 Feb 2017 03:04:44 -0500 Date: Wed, 1 Feb 2017 09:04:40 +0100 From: Christoffer Dall To: Marc Zyngier Cc: Jintack Lim , pbonzini@redhat.com, rkrcmar@redhat.com, linux@armlinux.org.uk, catalin.marinas@arm.com, will.deacon@arm.com, andre.przywara@arm.com, kvm@vger.kernel.org, linux-arm-kernel@lists.infradead.org, kvmarm@lists.cs.columbia.edu, linux-kernel@vger.kernel.org Subject: Re: [RFC v2 06/10] KVM: arm/arm64: Update the physical timer interrupt level Message-ID: <20170201080440.GB6226@cbox> References: <1485479100-4966-1-git-send-email-jintack@cs.columbia.edu> <1485479100-4966-7-git-send-email-jintack@cs.columbia.edu> <86sho199jx.fsf@arm.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <86sho199jx.fsf@arm.com> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Sun, Jan 29, 2017 at 03:21:06PM +0000, Marc Zyngier wrote: > On Fri, Jan 27 2017 at 01:04:56 AM, Jintack Lim wrote: > > Now that we maintain the EL1 physical timer register states of VMs, > > update the physical timer interrupt level along with the virtual one. > > > > Note that the emulated EL1 physical timer is not mapped to any hardware > > timer, so we call a proper vgic function. > > > > Signed-off-by: Jintack Lim > > --- > > virt/kvm/arm/arch_timer.c | 20 ++++++++++++++++++++ > > 1 file changed, 20 insertions(+) > > > > diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c > > index 0f6e935..3b6bd50 100644 > > --- a/virt/kvm/arm/arch_timer.c > > +++ b/virt/kvm/arm/arch_timer.c > > @@ -180,6 +180,21 @@ static void kvm_timer_update_mapped_irq(struct kvm_vcpu *vcpu, bool new_level, > > WARN_ON(ret); > > } > > > > +static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level, > > + struct arch_timer_context *timer) > > +{ > > + int ret; > > + > > + BUG_ON(!vgic_initialized(vcpu->kvm)); > > Although I've added my fair share of BUG_ON() in the code base, I've > since reconsidered my position. If we get in a situation where the vgic > is not initialized, maybe it would be better to just WARN_ON and return > early rather than killing the whole box. Thoughts? > Could we help this series along by saying that since this BUG_ON already exists in the kvm_timer_update_mapped_irq function, then it just preserves functionality and it's up to someone else (me) to remove the BUG_ON from both functions later in life? Thanks, -Christoffer