From patchwork Thu Aug 14 12:36:48 2014 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Marc Zyngier X-Patchwork-Id: 4724011 Return-Path: X-Original-To: patchwork-linux-arm@patchwork.kernel.org Delivered-To: patchwork-parsemail@patchwork1.web.kernel.org Received: from mail.kernel.org (mail.kernel.org [198.145.19.201]) by patchwork1.web.kernel.org (Postfix) with ESMTP id 865979F39D for ; Thu, 14 Aug 2014 12:39:58 +0000 (UTC) Received: from mail.kernel.org (localhost [127.0.0.1]) by mail.kernel.org (Postfix) with ESMTP id 1F2B7201EF for ; Thu, 14 Aug 2014 12:39:57 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.9]) (using TLSv1.2 with cipher DHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id B0862201E4 for ; Thu, 14 Aug 2014 12:39:55 +0000 (UTC) Received: from localhost ([127.0.0.1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XHuHM-0002gI-0I; Thu, 14 Aug 2014 12:37:16 +0000 Received: from fw-tnat.austin.arm.com ([217.140.110.23] helo=collaborate-mta1.arm.com) by bombadil.infradead.org with esmtp (Exim 4.80.1 #2 (Red Hat Linux)) id 1XHuHJ-0002dg-R7 for linux-arm-kernel@lists.infradead.org; Thu, 14 Aug 2014 12:37:14 +0000 Received: from approximate.cambridge.arm.com (approximate.cambridge.arm.com [10.1.209.43]) by collaborate-mta1.arm.com (Postfix) with ESMTPS id 97D8B13F61F; Thu, 14 Aug 2014 07:36:49 -0500 (CDT) From: Marc Zyngier To: Christoffer Dall Subject: Re: [PATCH 2/6] arm/arm64: KVM: Rename irq_active to irq_queued In-Reply-To: <87mwb7s1qz.fsf@approximate.cambridge.arm.com> (Marc Zyngier's message of "Thu, 14 Aug 2014 13:18:44 +0100") Organization: ARM Ltd References: <1405003196-12403-1-git-send-email-christoffer.dall@linaro.org> <1405003196-12403-3-git-send-email-christoffer.dall@linaro.org> <87mwb7s1qz.fsf@approximate.cambridge.arm.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) Date: Thu, 14 Aug 2014 13:36:48 +0100 Message-ID: <87iolvs0wv.fsf@approximate.cambridge.arm.com> MIME-Version: 1.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20140814_053713_969511_DE0C8565 X-CRM114-Status: GOOD ( 29.98 ) X-Spam-Score: -0.7 (/) Cc: "kvmarm@lists.cs.columbia.edu" , "linux-arm-kernel@lists.infradead.org" , "eric.auger@linaro.org" X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.18-1 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patchwork-linux-arm=patchwork.kernel.org@lists.infradead.org X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_NONE, RP_MATCHES_RCVD, UNPARSEABLE_RELAY autolearn=unavailable version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on mail.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Thu, Aug 14 2014 at 1:18:44 pm BST, Marc Zyngier wrote: > Hi Christoffer, > > On Thu, Jul 10 2014 at 3:39:52 pm BST, Christoffer Dall > wrote: >> We have a special bitmap on the distributor struct to keep track of when >> level-triggered interrupts are queued on the list registers. This was >> named irq_active, which is confusing, because the active state of an >> interrupt as per the GIC spec is a different thing, not specifically >> related to edge-triggered/level-triggered configurations but rather >> indicates an interrupt which has been ack'ed but not yet eoi'ed. >> >> Rename the bitmap and the corresponding accessor functions to irq_queued >> to clarify what this is actually used for. >> >> Signed-off-by: Christoffer Dall > > So to illustrate what I was going on about the first time you summitted > this patch, have a look at my below take on this. It is basically yours, > but just with the bitmap named "irq_can_sample", which is exactly what > this bitmap is about. > > What do you think? > > Thanks, > > M. > > From b6f864841878a5509e7d03581a224e270b25dd02 Mon Sep 17 00:00:00 2001 > From: Marc Zyngier > Date: Thu, 14 Aug 2014 13:14:34 +0100 > Subject: [PATCH] KVM: arm: vgic: rename irq_active to irq_can sample > > Signed-off-by: Marc Zyngier > --- > include/kvm/arm_vgic.h | 4 ++-- > virt/kvm/arm/vgic.c | 35 +++++++++++++++++++---------------- > 2 files changed, 21 insertions(+), 18 deletions(-) > > diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h > index 35b0c12..385d771 100644 > --- a/include/kvm/arm_vgic.h > +++ b/include/kvm/arm_vgic.h > @@ -143,8 +143,8 @@ struct vgic_dist { > /* Interrupt 'pin' level */ > struct vgic_bitmap irq_state; > > - /* Level-triggered interrupt in progress */ > - struct vgic_bitmap irq_active; > + /* Can we sample the pending bit to inject an interrupt? */ > + struct vgic_bitmap irq_can_sample; > > /* Interrupt priority. Not used yet. */ > struct vgic_bytemap irq_priority; > diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c > index 73eba79..1dedf03 100644 > --- a/virt/kvm/arm/vgic.c > +++ b/virt/kvm/arm/vgic.c > @@ -60,12 +60,12 @@ > * the 'line' again. This is achieved as such: > * > * - When a level interrupt is moved onto a vcpu, the corresponding > - * bit in irq_active is set. As long as this bit is set, the line > + * bit in irq_can_sample is cleared. As long as this bit is 0, the line > * will be ignored for further interrupts. The interrupt is injected > * into the vcpu with the GICH_LR_EOI bit set (generate a > * maintenance interrupt on EOI). > * - When the interrupt is EOIed, the maintenance interrupt fires, > - * and clears the corresponding bit in irq_active. This allow the > + * and sets the corresponding bit in irq_can_sample. This allow the > * interrupt line to be sampled again. > */ > > @@ -196,25 +196,25 @@ static int vgic_irq_is_enabled(struct kvm_vcpu *vcpu, int irq) > return vgic_bitmap_get_irq_val(&dist->irq_enabled, vcpu->vcpu_id, irq); > } > > -static int vgic_irq_is_active(struct kvm_vcpu *vcpu, int irq) > +static int vgic_irq_can_sample(struct kvm_vcpu *vcpu, int irq) > { > struct vgic_dist *dist = &vcpu->kvm->arch.vgic; > > - return vgic_bitmap_get_irq_val(&dist->irq_active, vcpu->vcpu_id, irq); > + return vgic_bitmap_get_irq_val(&dist->irq_can_sample, vcpu->vcpu_id, irq); > } > > -static void vgic_irq_set_active(struct kvm_vcpu *vcpu, int irq) > +static void vgic_irq_allow_sample(struct kvm_vcpu *vcpu, int irq) > { > struct vgic_dist *dist = &vcpu->kvm->arch.vgic; > > - vgic_bitmap_set_irq_val(&dist->irq_active, vcpu->vcpu_id, irq, 1); > + vgic_bitmap_set_irq_val(&dist->irq_can_sample, vcpu->vcpu_id, irq, 1); > } > > -static void vgic_irq_clear_active(struct kvm_vcpu *vcpu, int irq) > +static void vgic_irq_forbid_sample(struct kvm_vcpu *vcpu, int irq) > { > struct vgic_dist *dist = &vcpu->kvm->arch.vgic; > > - vgic_bitmap_set_irq_val(&dist->irq_active, vcpu->vcpu_id, irq, 0); > + vgic_bitmap_set_irq_val(&dist->irq_can_sample, vcpu->vcpu_id, irq, 0); > } > > static int vgic_dist_irq_is_pending(struct kvm_vcpu *vcpu, int irq) > @@ -1079,8 +1079,8 @@ static void vgic_retire_disabled_irqs(struct kvm_vcpu *vcpu) > > if (!vgic_irq_is_enabled(vcpu, vlr.irq)) { > vgic_retire_lr(lr, vlr.irq, vcpu); > - if (vgic_irq_is_active(vcpu, vlr.irq)) > - vgic_irq_clear_active(vcpu, vlr.irq); > + if (!vgic_irq_can_sample(vcpu, vlr.irq)) > + vgic_irq_allow_sample(vcpu, vlr.irq); > } > } > } > @@ -1170,7 +1170,7 @@ static bool vgic_queue_sgi(struct kvm_vcpu *vcpu, int irq) > > static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, int irq) > { > - if (vgic_irq_is_active(vcpu, irq)) > + if (!vgic_irq_can_sample(vcpu, irq)) > return true; /* level interrupt, already queued */ > > if (vgic_queue_irq(vcpu, 0, irq)) { > @@ -1178,7 +1178,7 @@ static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, int irq) > vgic_dist_irq_clear(vcpu, irq); > vgic_cpu_irq_clear(vcpu, irq); > } else { > - vgic_irq_set_active(vcpu, irq); > + vgic_irq_forbid_sample(vcpu, irq); > } > > return true; > @@ -1252,8 +1252,8 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu) > > if (status & INT_STATUS_EOI) { > /* > - * Some level interrupts have been EOIed. Clear their > - * active bit. > + * Some level interrupts have been EOIed. Allow them > + * to be resampled. > */ > u64 eisr = vgic_get_eisr(vcpu); > unsigned long *eisr_ptr = (unsigned long *)&eisr; > @@ -1262,7 +1262,7 @@ static bool vgic_process_maintenance(struct kvm_vcpu *vcpu) > for_each_set_bit(lr, eisr_ptr, vgic->nr_lr) { > struct vgic_lr vlr = vgic_get_lr(vcpu, lr); > > - vgic_irq_clear_active(vcpu, vlr.irq); > + vgic_irq_allow_sample(vcpu, vlr.irq); > WARN_ON(vlr.state & LR_STATE_MASK); > vlr.state = 0; > vgic_set_lr(vcpu, lr, vlr); > @@ -1429,7 +1429,7 @@ static bool vgic_update_irq_state(struct kvm *kvm, int cpuid, > goto out; > } > > - if (is_level && vgic_irq_is_active(vcpu, irq_num)) { > + if (is_level && !vgic_irq_can_sample(vcpu, irq_num)) { > /* > * Level interrupt in progress, will be picked up > * when EOId. > @@ -1506,6 +1506,9 @@ int kvm_vgic_vcpu_init(struct kvm_vcpu *vcpu) > if (i < VGIC_NR_PRIVATE_IRQS) > vgic_bitmap_set_irq_val(&dist->irq_cfg, > vcpu->vcpu_id, i, VGIC_CFG_EDGE); > + /* Let vcpu0 also allow sampling of SPIs */ > + if (i < VGIC_NR_PRIVATE_IRQS || vcpu->vcpu_id == 0) > + vgic_irq_allow_sample(vcpu, i); > > vgic_cpu->vgic_irq_lr_map[i] = LR_EMPTY; > } > -- > 2.0.0 The astute reader will have noticed that this change allows for further cleanups, such as this: M. diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c index 1dedf03..327588d 100644 --- a/virt/kvm/arm/vgic.c +++ b/virt/kvm/arm/vgic.c @@ -1395,15 +1395,12 @@ static bool vgic_update_irq_state(struct kvm *kvm, int cpuid, { struct vgic_dist *dist = &kvm->arch.vgic; struct kvm_vcpu *vcpu; - int is_edge, is_level; int enabled; bool ret = true; spin_lock(&dist->lock); vcpu = kvm_get_vcpu(kvm, cpuid); - is_edge = vgic_irq_is_edge(vcpu, irq_num); - is_level = !is_edge; if (!vgic_validate_injection(vcpu, irq_num, level)) { ret = false; @@ -1429,7 +1426,7 @@ static bool vgic_update_irq_state(struct kvm *kvm, int cpuid, goto out; } - if (is_level && !vgic_irq_can_sample(vcpu, irq_num)) { + if (!vgic_irq_can_sample(vcpu, irq_num)) { /* * Level interrupt in progress, will be picked up * when EOId.