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: 9549227 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 4456460425 for ; Wed, 1 Feb 2017 10:07:52 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 228E1281D2 for ; Wed, 1 Feb 2017 10:07:52 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 1523C283E8; Wed, 1 Feb 2017 10:07:52 +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=-1.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, T_DKIM_INVALID autolearn=no version=3.3.1 Received: from bombadil.infradead.org (bombadil.infradead.org [65.50.211.133]) (using TLSv1.2 with cipher AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8AC18281D2 for ; Wed, 1 Feb 2017 10:07:51 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.87 #1 (Red Hat Linux)) id 1cYrpJ-0004pO-Rg; Wed, 01 Feb 2017 10:07:45 +0000 Received: from mail-lf0-x235.google.com ([2a00:1450:4010:c07::235]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cYrpG-0004nX-F2 for linux-arm-kernel@lists.infradead.org; Wed, 01 Feb 2017 10:07:44 +0000 Received: by mail-lf0-x235.google.com with SMTP id n124so225301696lfd.2 for ; Wed, 01 Feb 2017 02:07:21 -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=s3Jgscj51uNP08LiFGcVIQXBL4Sp0T/HWJJ6vCRTAya0HRBOSk3gDNedAGxQ0Hw45/ qKreriga5EjXDlcH+BUw5a5Lj6US+hPqLWrpi3GoNPmosfDsGLmN/J0yxsepm7URE34v kmGcLU/7GjIyxNrufFeULvODNVQz+ch1Ozv8G2zByZ124N7U0bSkdSmS/SDUOhBeCwIi NA2v0PnbAVTwaZQnnhIswWe4W4mPIrKNyFnPLCKdUT9kSE+uXuzvkWKOot8GYuEhbahV xmW7oG+GuRpBi0tJ8U360y/DfQNuCceqoB194TpG86QiTOlgAGzsmHNV8S3vuKkw2jbM yprA== X-Gm-Message-State: AIkVDXKakiHgqJGsIz5KUhiHMPi1f0L9kfwWxB1T+br55sA6rADX523+1jn+qLA6drp105Xs 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 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) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170201_020742_696961_353CC084 X-CRM114-Status: GOOD ( 25.18 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: KVM General , Radim =?utf-8?B?S3LEjW3DocWZ?= , Marc Zyngier , Catalin Marinas , Will Deacon , linux@armlinux.org.uk, lkml - Kernel Mailing List , Andre Przywara , Paolo Bonzini , kvmarm@lists.cs.columbia.edu, arm-mail-list Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.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)