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: 35404 Return-Path: X-Original-To: linaro@patches.linaro.org Delivered-To: linaro@patches.linaro.org Received: from mail-ig0-f199.google.com (mail-ig0-f199.google.com [209.85.213.199]) by ip-10-151-82-157.ec2.internal (Postfix) with ESMTPS id 37810202E4 for ; Thu, 14 Aug 2014 12:39:09 +0000 (UTC) Received: by mail-ig0-f199.google.com with SMTP id l13sf10566133iga.2 for ; Thu, 14 Aug 2014 05:39:08 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:delivered-to:from:to:subject:in-reply-to :organization:references:user-agent:date:message-id:mime-version:cc :precedence:list-id:list-unsubscribe:list-archive:list-post :list-help:list-subscribe:sender:errors-to:x-original-sender :x-original-authentication-results:mailing-list:content-type :content-transfer-encoding; bh=lJHiQY19nOr1Y/P47bZ0FUqUB8QPcTwQ1UM2f45X9gs=; b=SXe0dvZNoVwJb2J7wEvMYlvL/M62TuLyYNGHi6HOFIszUFSrhhEkotvPyC4zWTf+Zy wIj97TCNB4rxU8jRhvdVmb8m6ngOvgfvVkSkdxHiuJY0ZP9jUGiOF0sfX8BgUIPY2VCk ErUSShPX4x0h5FX0HsH9e8ZoR5PSxXFJZ6wMW4KqvhQ2+ttMhm7UELxbIfn1xQi4+9x/ MjT58LHPWRhxUSdhp3ynqkL+tDwMYSZZMJvvvxshFA9NxslkRgyRrZFfCpbRwgCTa6Rg WPAgrURaJN3ZhW8Q9PBi/WxLL11p8n/I8InEEI27IT8bm/czTG97mXtjcxGlPimw/UcF PulA== X-Gm-Message-State: ALoCoQntvBHNBG+Xs8+BZLzuol0xM30vxk1ya0zmHTxspJLfU8+oWqgVs6aetA5BBR38qjZTC0SF X-Received: by 10.182.45.197 with SMTP id p5mr5730316obm.2.1408019948746; Thu, 14 Aug 2014 05:39:08 -0700 (PDT) X-BeenThere: patchwork-forward@linaro.org Received: by 10.140.36.148 with SMTP id p20ls992399qgp.50.gmail; Thu, 14 Aug 2014 05:39:08 -0700 (PDT) X-Received: by 10.52.170.69 with SMTP id ak5mr807881vdc.63.1408019948609; Thu, 14 Aug 2014 05:39:08 -0700 (PDT) Received: from mail-vc0-f171.google.com (mail-vc0-f171.google.com [209.85.220.171]) by mx.google.com with ESMTPS id cv9si896608veb.2.2014.08.14.05.39.08 for (version=TLSv1 cipher=ECDHE-RSA-RC4-SHA bits=128/128); Thu, 14 Aug 2014 05:39:08 -0700 (PDT) Received-SPF: pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.171 as permitted sender) client-ip=209.85.220.171; Received: by mail-vc0-f171.google.com with SMTP id hq11so1284898vcb.16 for ; Thu, 14 Aug 2014 05:39:08 -0700 (PDT) X-Received: by 10.220.190.134 with SMTP id di6mr1423179vcb.43.1408019948514; Thu, 14 Aug 2014 05:39:08 -0700 (PDT) X-Forwarded-To: patchwork-forward@linaro.org X-Forwarded-For: patch@linaro.org patchwork-forward@linaro.org Delivered-To: patch@linaro.org Received: by 10.221.37.5 with SMTP id tc5csp30479vcb; Thu, 14 Aug 2014 05:39:07 -0700 (PDT) X-Received: by 10.66.192.73 with SMTP id he9mr4062393pac.88.1408019947002; Thu, 14 Aug 2014 05:39:07 -0700 (PDT) Received: from bombadil.infradead.org (bombadil.infradead.org. [2001:1868:205::9]) by mx.google.com with ESMTPS id w10si4124905pas.32.2014.08.14.05.39.06 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 14 Aug 2014 05:39:06 -0700 (PDT) Received-SPF: none (google.com: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org does not designate permitted sender hosts) client-ip=2001:1868:205::9; 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 (/) X-Spam-Report: SpamAssassin version 3.4.0 on bombadil.infradead.org summary: Content analysis details: (-0.7 points) pts rule name description ---- ---------------------- -------------------------------------------------- -0.7 RP_MATCHES_RCVD Envelope sender domain matches handover relay domain -0.0 SPF_PASS SPF: sender matches SPF record 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+patch=linaro.org@lists.infradead.org X-Removed-Original-Auth: Dkim didn't pass. X-Original-Sender: marc.zyngier@arm.com X-Original-Authentication-Results: mx.google.com; spf=pass (google.com: domain of patch+caf_=patchwork-forward=linaro.org@linaro.org designates 209.85.220.171 as permitted sender) smtp.mail=patch+caf_=patchwork-forward=linaro.org@linaro.org Mailing-list: list patchwork-forward@linaro.org; contact patchwork-forward+owners@linaro.org X-Google-Group-Id: 836684582541 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.