From mboxrd@z Thu Jan 1 00:00:00 1970 From: Christoffer Dall Subject: Re: [RFC v2 06/10] KVM: arm/arm64: Update the physical timer interrupt level Date: Wed, 1 Feb 2017 09:04:40 +0100 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-Transfer-Encoding: 7bit Cc: linux-kernel@vger.kernel.org, kvm@vger.kernel.org, catalin.marinas@arm.com, will.deacon@arm.com, linux@armlinux.org.uk, kvmarm@lists.cs.columbia.edu, andre.przywara@arm.com, pbonzini@redhat.com, linux-arm-kernel@lists.infradead.org To: Marc Zyngier Return-path: Content-Disposition: inline In-Reply-To: <86sho199jx.fsf@arm.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 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