From patchwork Wed Feb 1 10:07:16 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 9549231 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id BAF0F60425 for ; Wed, 1 Feb 2017 10:08:11 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id A14C9281D2 for ; Wed, 1 Feb 2017 10:08:11 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 9512528401; Wed, 1 Feb 2017 10:08:11 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, RCVD_IN_DNSWL_HI autolearn=unavailable version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 42C85281D2 for ; Wed, 1 Feb 2017 10:07:57 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751473AbdBAKHY (ORCPT ); Wed, 1 Feb 2017 05:07:24 -0500 Received: from mail-lf0-f45.google.com ([209.85.215.45]:33995 "EHLO mail-lf0-f45.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751352AbdBAKHX (ORCPT ); Wed, 1 Feb 2017 05:07:23 -0500 Received: by mail-lf0-f45.google.com with SMTP id v186so225810533lfa.1 for ; Wed, 01 Feb 2017 02:07:22 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=of43khuN1wo7GS3qDMHz3EFRJzK641OYbnmXtQyBCtw=; b=KG9pz9g/leaMe7ZZbjB3WopYSRC3nGadp/ebSmNdgYTklUpFVNdTUOcRIvBm+LevFY iykIJ/rD8FDVwNV4XAUWAGM+35vkreV9T03ng/lNgHIi232XD76xZOcdMWMK/x5ZW3Ut L9YeKIrW3bG4F2sUxoWsxQNiGlZswfGLuWvl0= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=of43khuN1wo7GS3qDMHz3EFRJzK641OYbnmXtQyBCtw=; b=p+8noddr+SxP3kllWcnNnfKHaoTOWuMtY6WvhV+XYenU2bN2Q4MAy+viU3JsRY734c r+Nn7H1sQ+AG1iSe9SPGV8UWgqL1YQB8StZIaMUvKQ0R8LKxqAIA7mqcq62IgYAYPzsR e3IG6EQARvz6M3BgbPpCbOO2MXSH7slZYMhE77LfhFF3ROVEwGXDAxIDi0SuJCivWI0o zzdQM9fIlw+W75vq2f0YKy4+gL9WoFFejpEt35+FfnkEJeWhWYLvjKiTdPGIkZ97iEip zO3T8beAlh/GVTwzyCbDW//SiFa6/gJuO7+3z/cpOTR73tG8/sngTM0x5HKX50DUDxNG b7Dw== X-Gm-Message-State: AIkVDXIhHquL1OpaGBuTQyCyPcK9CUyKc5TQUFy7w0MdghhVS8mjHTGxkFcaThDZk00htRvi X-Received: by 10.46.76.9 with SMTP id z9mr841342lja.1.1485943639946; Wed, 01 Feb 2017 02:07:19 -0800 (PST) Received: from localhost (94.191.187.220.mobile.3.dk. [94.191.187.220]) by smtp.gmail.com with ESMTPSA id f188sm1925198lfe.12.2017.02.01.02.07.18 (version=TLS1_2 cipher=AES128-SHA bits=128/128); Wed, 01 Feb 2017 02:07:19 -0800 (PST) Date: Wed, 1 Feb 2017 11:07:16 +0100 From: Christoffer Dall To: Jintack Lim Cc: Marc Zyngier , Paolo Bonzini , Radim =?utf-8?B?S3LEjW3DocWZ?= , linux@armlinux.org.uk, Catalin Marinas , Will Deacon , Andre Przywara , KVM General , arm-mail-list , kvmarm@lists.cs.columbia.edu, lkml - Kernel Mailing List Subject: Re: [RFC v2 06/10] KVM: arm/arm64: Update the physical timer interrupt level Message-ID: <20170201100716.GC6226@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> <20170201080440.GB6226@cbox> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.21 (2010-09-15) Sender: kvm-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: kvm@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, Feb 01, 2017 at 03:40:10AM -0500, Jintack Lim wrote: > On Wed, Feb 1, 2017 at 3:04 AM, Christoffer Dall > wrote: > > 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? > > > > Sounds good to me :) Thanks! > So just as you thought you were getting off easy... The reason we now have kvm_timer_update_irq and kvm_timer_update_mapped_irq is that we have the following two vgic functions: kvm_vgic_inject_irq kvm_vgic_inject_mapped_irq But the only difference between the two is what they pass as the mapped_irq argument to vgic_update_irq_pending. What about if we just had this as a precursor patch: That would make this patch simpler. If so, I can send out the above patch with a proper description. Thanks, -Christoffer diff --git a/virt/kvm/arm/arch_timer.c b/virt/kvm/arm/arch_timer.c index 6a084cd..91ecf48 100644 --- a/virt/kvm/arm/arch_timer.c +++ b/virt/kvm/arm/arch_timer.c @@ -175,7 +175,8 @@ static void kvm_timer_update_irq(struct kvm_vcpu *vcpu, bool new_level) timer->irq.level = new_level; trace_kvm_timer_update_irq(vcpu->vcpu_id, timer->irq.irq, timer->irq.level); - ret = kvm_vgic_inject_mapped_irq(vcpu->kvm, vcpu->vcpu_id, + + ret = kvm_vgic_inject_irq(vcpu->kvm, vcpu->vcpu_id, timer->irq.irq, timer->irq.level); WARN_ON(ret); diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c index dea12df..4c87fd0 100644 --- a/virt/kvm/arm/vgic/vgic.c +++ b/virt/kvm/arm/vgic/vgic.c @@ -336,8 +336,7 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq) } static int vgic_update_irq_pending(struct kvm *kvm, int cpuid, - unsigned int intid, bool level, - bool mapped_irq) + unsigned int intid, bool level) { struct kvm_vcpu *vcpu; struct vgic_irq *irq; @@ -357,11 +356,6 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid, if (!irq) return -EINVAL; - if (irq->hw != mapped_irq) { - vgic_put_irq(kvm, irq); - return -EINVAL; - } - spin_lock(&irq->irq_lock); if (!vgic_validate_injection(irq, level)) { @@ -399,13 +393,7 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid, int kvm_vgic_inject_irq(struct kvm *kvm, int cpuid, unsigned int intid, bool level) { - return vgic_update_irq_pending(kvm, cpuid, intid, level, false); -} - -int kvm_vgic_inject_mapped_irq(struct kvm *kvm, int cpuid, unsigned int intid, - bool level) -{ - return vgic_update_irq_pending(kvm, cpuid, intid, level, true); + return vgic_update_irq_pending(kvm, cpuid, intid, level); } int kvm_vgic_map_phys_irq(struct kvm_vcpu *vcpu, u32 virt_irq, u32 phys_irq)