From patchwork Tue Jan 24 11:01:17 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Christoffer Dall X-Patchwork-Id: 92334 Delivered-To: patch@linaro.org Received: by 10.140.20.99 with SMTP id 90csp1655349qgi; Tue, 24 Jan 2017 03:03:21 -0800 (PST) X-Received: by 10.237.36.24 with SMTP id r24mr27580581qtc.229.1485255801531; Tue, 24 Jan 2017 03:03:21 -0800 (PST) Return-Path: Received: from bombadil.infradead.org (bombadil.infradead.org. [65.50.211.133]) by mx.google.com with ESMTPS id e63si12869085qtd.131.2017.01.24.03.03.21 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 24 Jan 2017 03:03:21 -0800 (PST) Received-SPF: pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 65.50.211.133 as permitted sender) client-ip=65.50.211.133; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@linaro.org; spf=pass (google.com: best guess record for domain of linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org designates 65.50.211.133 as permitted sender) smtp.mailfrom=linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org; dmarc=fail (p=NONE sp=NONE dis=NONE) header.from=linaro.org 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 1cVysg-00013n-Uc; Tue, 24 Jan 2017 11:03:18 +0000 Received: from mail-lf0-f44.google.com ([209.85.215.44]) by bombadil.infradead.org with esmtps (Exim 4.87 #1 (Red Hat Linux)) id 1cVysb-0000o9-Hg for linux-arm-kernel@lists.infradead.org; Tue, 24 Jan 2017 11:03:17 +0000 Received: by mail-lf0-f44.google.com with SMTP id x1so22001032lff.0 for ; Tue, 24 Jan 2017 03:02:53 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=from:to:cc:subject:date:message-id; bh=qvW4Hz2gC1eG0IeyA2KD2F3e+bcteLATXEQD+wi53Rs=; b=EgM3+wKnuKCiJLRCl+XUIarNac3cCD5dQ3GY2MU24kYjEyFXTdxqo857kWENf7SFgC k3LCSffIKSdy1cwuCULy4nEeEjBIAdo/gwZlMsLdgHyAx8Z1wF9967K14xDyXyeZNSt0 MIf0H6A5zGFpS19SQ3689BsR7jp61p1tdNHD4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=qvW4Hz2gC1eG0IeyA2KD2F3e+bcteLATXEQD+wi53Rs=; b=Axb0LyQb2JtjG8EfR2LmaZy6QvVGmxO97Zl8VwXwZmopBpFJtGBm41Q/YREQp7a9dA 38CvCwyBx/vmfln9kRMFFMeFCdYienO/L7GrCSpUlyZ43OmckZNm9nhFsh8OED+CP97o tVE6Oqflx0fJQulrKgQZjmAp7Tx/zjhNlK7mXouyKWCm0L2Iv0odHzu3H1pUEDVIPx6D XZHaahrUpJxZ68rmdqQwq2hkN4wYtwRXlHzWB5drRCrP+f2wvFwiQLeTfkFVqiogb3qW DRTACM5aHSZNcDWmnuccbXXrmRK2UPVX9mrOXQVuwNdy93tNlb1lsZr6TD0ttZu56WR1 Fgqw== X-Gm-Message-State: AIkVDXJ785QZBWiZ7t8Mt1XeGofKKAEZSrSkCtnzofgREWjK6D6RNBsQgjuwSwRaM8sjwrsZ X-Received: by 10.25.133.65 with SMTP id h62mr2362920lfd.135.1485255711234; Tue, 24 Jan 2017 03:01:51 -0800 (PST) Received: from localhost.localdomain (x1-6-50-6a-03-de-ec-c2.cpe.webspeed.dk. [2.108.209.202]) by smtp.gmail.com with ESMTPSA id 96sm7276697lfp.18.2017.01.24.03.01.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Tue, 24 Jan 2017 03:01:50 -0800 (PST) From: Christoffer Dall To: kvmarm@lists.cs.columbia.edu, linux-arm-kernel@lists.infradead.org Subject: [PATCH v2] KVM: arm/arm64: Remove struct vgic_irq pending field Date: Tue, 24 Jan 2017 12:01:17 +0100 Message-Id: <20170124110117.13390-1-christoffer.dall@linaro.org> X-Mailer: git-send-email 2.9.0 X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20170124_030314_007359_1556C3E0 X-CRM114-Status: GOOD ( 23.49 ) X-Spam-Score: -1.5 (-) X-Spam-Report: SpamAssassin version 3.4.1 on bombadil.infradead.org summary: Content analysis details: (-1.5 points) pts rule name description ---- ---------------------- -------------------------------------------------- 0.5 RCVD_IN_SORBS_SPAM RBL: SORBS: sender is a spam source [209.85.215.44 listed in dnsbl.sorbs.net] -0.0 RCVD_IN_DNSWL_NONE RBL: Sender listed at http://www.dnswl.org/, no trust [209.85.215.44 listed in list.dnswl.org] -0.0 RCVD_IN_MSPIKE_H3 RBL: Good reputation (+3) [209.85.215.44 listed in wl.mailspike.net] -0.0 SPF_PASS SPF: sender matches SPF record -1.9 BAYES_00 BODY: Bayes spam probability is 0 to 1% [score: 0.0000] 0.1 DKIM_SIGNED Message has a DKIM or DK signature, not necessarily valid -0.1 DKIM_VALID_AU Message has a valid DKIM or DK signature from author's domain -0.1 DKIM_VALID Message has at least one valid DKIM or DK signature -0.0 RCVD_IN_MSPIKE_WL Mailspike good senders 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: Peter Maydell , vijay.kilari@gmail.com, Marc Zyngier , Andre Przywara , Eric Auger , Christoffer Dall MIME-Version: 1.0 Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+patch=linaro.org@lists.infradead.org One of the goals behind the VGIC redesign was to get rid of cached or intermediate state in the data structures, but we decided to allow ourselves to precompute the pending value of an IRQ based on the line level and pending latch state. However, this has now become difficult to base proper GICv3 save/restore on, because there is a potential to modify the pending state without knowing if an interrupt is edge or level configured. See the following post and related message for more background: https://lists.cs.columbia.edu/pipermail/kvmarm/2017-January/023195.html This commit gets rid of the precomputed pending field in favor of a function that calculates the value when needed, irq_is_pending(). The soft_pending field is renamed to pending_latch to represent that this latch is the equivalent hardware latch which gets manipulated by the input signal for edge-triggered interrupts and when writing to the SPENDR/CPENDR registers. After this commit save/restore code should be able to simply restore the pending_latch state, line_level state, and config state in any order and get the desired result. Reviewed-by: Andre Przywara Reviewed-by: Marc Zyngier Tested-by: Andre Przywara Signed-off-by: Christoffer Dall --- I kept the reviewed and tested by tags despite having modified this to no longer include the wrapper function, as I found the change trivial enough to trust it. I also tested with UEFI, just to be on the safe side. include/kvm/arm_vgic.h | 5 +++-- virt/kvm/arm/vgic/vgic-its.c | 6 +++--- virt/kvm/arm/vgic/vgic-mmio-v2.c | 6 +++--- virt/kvm/arm/vgic/vgic-mmio-v3.c | 2 +- virt/kvm/arm/vgic/vgic-mmio.c | 19 +++++-------------- virt/kvm/arm/vgic/vgic-v2.c | 12 +++++------- virt/kvm/arm/vgic/vgic-v3.c | 12 +++++------- virt/kvm/arm/vgic/vgic.c | 16 +++++++--------- virt/kvm/arm/vgic/vgic.h | 8 ++++++++ 9 files changed, 40 insertions(+), 46 deletions(-) -- 2.9.0 _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h index 002f092..da2ce08 100644 --- a/include/kvm/arm_vgic.h +++ b/include/kvm/arm_vgic.h @@ -101,9 +101,10 @@ struct vgic_irq { */ u32 intid; /* Guest visible INTID */ - bool pending; bool line_level; /* Level only */ - bool soft_pending; /* Level only */ + bool pending_latch; /* The pending latch state used to calculate + * the pending state for both level + * and edge triggered IRQs. */ bool active; /* not used for LPIs */ bool enabled; bool hw; /* Tied to HW IRQ */ diff --git a/virt/kvm/arm/vgic/vgic-its.c b/virt/kvm/arm/vgic/vgic-its.c index 8c2b3cd..571b64a 100644 --- a/virt/kvm/arm/vgic/vgic-its.c +++ b/virt/kvm/arm/vgic/vgic-its.c @@ -350,7 +350,7 @@ static int its_sync_lpi_pending_table(struct kvm_vcpu *vcpu) irq = vgic_get_irq(vcpu->kvm, NULL, intids[i]); spin_lock(&irq->irq_lock); - irq->pending = pendmask & (1U << bit_nr); + irq->pending_latch = pendmask & (1U << bit_nr); vgic_queue_irq_unlock(vcpu->kvm, irq); vgic_put_irq(vcpu->kvm, irq); } @@ -465,7 +465,7 @@ static int vgic_its_trigger_msi(struct kvm *kvm, struct vgic_its *its, return -EBUSY; spin_lock(&itte->irq->irq_lock); - itte->irq->pending = true; + itte->irq->pending_latch = true; vgic_queue_irq_unlock(kvm, itte->irq); return 0; @@ -913,7 +913,7 @@ static int vgic_its_cmd_handle_clear(struct kvm *kvm, struct vgic_its *its, if (!itte) return E_ITS_CLEAR_UNMAPPED_INTERRUPT; - itte->irq->pending = false; + itte->irq->pending_latch = false; return 0; } diff --git a/virt/kvm/arm/vgic/vgic-mmio-v2.c b/virt/kvm/arm/vgic/vgic-mmio-v2.c index 78e34bc..07e67f1 100644 --- a/virt/kvm/arm/vgic/vgic-mmio-v2.c +++ b/virt/kvm/arm/vgic/vgic-mmio-v2.c @@ -98,7 +98,7 @@ static void vgic_mmio_write_sgir(struct kvm_vcpu *source_vcpu, irq = vgic_get_irq(source_vcpu->kvm, vcpu, intid); spin_lock(&irq->irq_lock); - irq->pending = true; + irq->pending_latch = true; irq->source |= 1U << source_vcpu->vcpu_id; vgic_queue_irq_unlock(source_vcpu->kvm, irq); @@ -182,7 +182,7 @@ static void vgic_mmio_write_sgipendc(struct kvm_vcpu *vcpu, irq->source &= ~((val >> (i * 8)) & 0xff); if (!irq->source) - irq->pending = false; + irq->pending_latch = false; spin_unlock(&irq->irq_lock); vgic_put_irq(vcpu->kvm, irq); @@ -204,7 +204,7 @@ static void vgic_mmio_write_sgipends(struct kvm_vcpu *vcpu, irq->source |= (val >> (i * 8)) & 0xff; if (irq->source) { - irq->pending = true; + irq->pending_latch = true; vgic_queue_irq_unlock(vcpu->kvm, irq); } else { spin_unlock(&irq->irq_lock); diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c b/virt/kvm/arm/vgic/vgic-mmio-v3.c index 50f42f0..2aca52a 100644 --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c @@ -646,7 +646,7 @@ void vgic_v3_dispatch_sgi(struct kvm_vcpu *vcpu, u64 reg) irq = vgic_get_irq(vcpu->kvm, c_vcpu, sgi); spin_lock(&irq->irq_lock); - irq->pending = true; + irq->pending_latch = true; vgic_queue_irq_unlock(vcpu->kvm, irq); vgic_put_irq(vcpu->kvm, irq); diff --git a/virt/kvm/arm/vgic/vgic-mmio.c b/virt/kvm/arm/vgic/vgic-mmio.c index ebe1b9f..2670d39 100644 --- a/virt/kvm/arm/vgic/vgic-mmio.c +++ b/virt/kvm/arm/vgic/vgic-mmio.c @@ -111,7 +111,7 @@ unsigned long vgic_mmio_read_pending(struct kvm_vcpu *vcpu, for (i = 0; i < len * 8; i++) { struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); - if (irq->pending) + if (irq_is_pending(irq)) value |= (1U << i); vgic_put_irq(vcpu->kvm, irq); @@ -131,9 +131,7 @@ void vgic_mmio_write_spending(struct kvm_vcpu *vcpu, struct vgic_irq *irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); spin_lock(&irq->irq_lock); - irq->pending = true; - if (irq->config == VGIC_CONFIG_LEVEL) - irq->soft_pending = true; + irq->pending_latch = true; vgic_queue_irq_unlock(vcpu->kvm, irq); vgic_put_irq(vcpu->kvm, irq); @@ -152,12 +150,7 @@ void vgic_mmio_write_cpending(struct kvm_vcpu *vcpu, spin_lock(&irq->irq_lock); - if (irq->config == VGIC_CONFIG_LEVEL) { - irq->soft_pending = false; - irq->pending = irq->line_level; - } else { - irq->pending = false; - } + irq->pending_latch = false; spin_unlock(&irq->irq_lock); vgic_put_irq(vcpu->kvm, irq); @@ -359,12 +352,10 @@ void vgic_mmio_write_config(struct kvm_vcpu *vcpu, irq = vgic_get_irq(vcpu->kvm, vcpu, intid + i); spin_lock(&irq->irq_lock); - if (test_bit(i * 2 + 1, &val)) { + if (test_bit(i * 2 + 1, &val)) irq->config = VGIC_CONFIG_EDGE; - } else { + else irq->config = VGIC_CONFIG_LEVEL; - irq->pending = irq->line_level | irq->soft_pending; - } spin_unlock(&irq->irq_lock); vgic_put_irq(vcpu->kvm, irq); diff --git a/virt/kvm/arm/vgic/vgic-v2.c b/virt/kvm/arm/vgic/vgic-v2.c index 834137e..b834ecd 100644 --- a/virt/kvm/arm/vgic/vgic-v2.c +++ b/virt/kvm/arm/vgic/vgic-v2.c @@ -104,7 +104,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu) /* Edge is the only case where we preserve the pending bit */ if (irq->config == VGIC_CONFIG_EDGE && (val & GICH_LR_PENDING_BIT)) { - irq->pending = true; + irq->pending_latch = true; if (vgic_irq_is_sgi(intid)) { u32 cpuid = val & GICH_LR_PHYSID_CPUID; @@ -120,9 +120,7 @@ void vgic_v2_fold_lr_state(struct kvm_vcpu *vcpu) */ if (irq->config == VGIC_CONFIG_LEVEL) { if (!(val & GICH_LR_PENDING_BIT)) - irq->soft_pending = false; - - irq->pending = irq->line_level || irq->soft_pending; + irq->pending_latch = false; } spin_unlock(&irq->irq_lock); @@ -145,11 +143,11 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) { u32 val = irq->intid; - if (irq->pending) { + if (irq_is_pending(irq)) { val |= GICH_LR_PENDING_BIT; if (irq->config == VGIC_CONFIG_EDGE) - irq->pending = false; + irq->pending_latch = false; if (vgic_irq_is_sgi(irq->intid)) { u32 src = ffs(irq->source); @@ -158,7 +156,7 @@ void vgic_v2_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) val |= (src - 1) << GICH_LR_PHYSID_CPUID_SHIFT; irq->source &= ~(1 << (src - 1)); if (irq->source) - irq->pending = true; + irq->pending_latch = true; } } diff --git a/virt/kvm/arm/vgic/vgic-v3.c b/virt/kvm/arm/vgic/vgic-v3.c index e6b03fd..679ba93 100644 --- a/virt/kvm/arm/vgic/vgic-v3.c +++ b/virt/kvm/arm/vgic/vgic-v3.c @@ -94,7 +94,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu) /* Edge is the only case where we preserve the pending bit */ if (irq->config == VGIC_CONFIG_EDGE && (val & ICH_LR_PENDING_BIT)) { - irq->pending = true; + irq->pending_latch = true; if (vgic_irq_is_sgi(intid) && model == KVM_DEV_TYPE_ARM_VGIC_V2) { @@ -111,9 +111,7 @@ void vgic_v3_fold_lr_state(struct kvm_vcpu *vcpu) */ if (irq->config == VGIC_CONFIG_LEVEL) { if (!(val & ICH_LR_PENDING_BIT)) - irq->soft_pending = false; - - irq->pending = irq->line_level || irq->soft_pending; + irq->pending_latch = false; } spin_unlock(&irq->irq_lock); @@ -127,11 +125,11 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) u32 model = vcpu->kvm->arch.vgic.vgic_model; u64 val = irq->intid; - if (irq->pending) { + if (irq_is_pending(irq)) { val |= ICH_LR_PENDING_BIT; if (irq->config == VGIC_CONFIG_EDGE) - irq->pending = false; + irq->pending_latch = false; if (vgic_irq_is_sgi(irq->intid) && model == KVM_DEV_TYPE_ARM_VGIC_V2) { @@ -141,7 +139,7 @@ void vgic_v3_populate_lr(struct kvm_vcpu *vcpu, struct vgic_irq *irq, int lr) val |= (src - 1) << GICH_LR_PHYSID_CPUID_SHIFT; irq->source &= ~(1 << (src - 1)); if (irq->source) - irq->pending = true; + irq->pending_latch = true; } } diff --git a/virt/kvm/arm/vgic/vgic.c b/virt/kvm/arm/vgic/vgic.c index 6440b56..dea12df 100644 --- a/virt/kvm/arm/vgic/vgic.c +++ b/virt/kvm/arm/vgic/vgic.c @@ -160,7 +160,7 @@ static struct kvm_vcpu *vgic_target_oracle(struct vgic_irq *irq) * If the distributor is disabled, pending interrupts shouldn't be * forwarded. */ - if (irq->enabled && irq->pending) { + if (irq->enabled && irq_is_pending(irq)) { if (unlikely(irq->target_vcpu && !irq->target_vcpu->kvm->arch.vgic.enabled)) return NULL; @@ -204,8 +204,8 @@ static int vgic_irq_cmp(void *priv, struct list_head *a, struct list_head *b) goto out; } - penda = irqa->enabled && irqa->pending; - pendb = irqb->enabled && irqb->pending; + penda = irqa->enabled && irq_is_pending(irqa); + pendb = irqb->enabled && irq_is_pending(irqb); if (!penda || !pendb) { ret = (int)pendb - (int)penda; @@ -371,12 +371,10 @@ static int vgic_update_irq_pending(struct kvm *kvm, int cpuid, return 0; } - if (irq->config == VGIC_CONFIG_LEVEL) { + if (irq->config == VGIC_CONFIG_LEVEL) irq->line_level = level; - irq->pending = level || irq->soft_pending; - } else { - irq->pending = true; - } + else + irq->pending_latch = true; vgic_queue_irq_unlock(kvm, irq); vgic_put_irq(kvm, irq); @@ -689,7 +687,7 @@ int kvm_vgic_vcpu_pending_irq(struct kvm_vcpu *vcpu) list_for_each_entry(irq, &vgic_cpu->ap_list_head, ap_list) { spin_lock(&irq->irq_lock); - pending = irq->pending && irq->enabled; + pending = irq_is_pending(irq) && irq->enabled; spin_unlock(&irq->irq_lock); if (pending) diff --git a/virt/kvm/arm/vgic/vgic.h b/virt/kvm/arm/vgic/vgic.h index 859f65c..b2cf34b 100644 --- a/virt/kvm/arm/vgic/vgic.h +++ b/virt/kvm/arm/vgic/vgic.h @@ -30,6 +30,14 @@ #define vgic_irq_is_sgi(intid) ((intid) < VGIC_NR_SGIS) +static inline bool irq_is_pending(struct vgic_irq *irq) +{ + if (irq->config == VGIC_CONFIG_EDGE) + return irq->pending_latch; + else + return irq->pending_latch || irq->line_level; +} + struct vgic_vmcr { u32 ctlr; u32 abpr;