| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <20210604064828.1497-1-lushenming@huawei.com>
Date: Fri, 4 Jun 2021 14:48:28 +0800
From: Shenming Lu <lushenming@...wei.com>
To: Marc Zyngier <maz@...nel.org>, Will Deacon <will@...nel.org>,
<linux-arm-kernel@...ts.infradead.org>,
<kvmarm@...ts.cs.columbia.edu>, <linux-kernel@...r.kernel.org>
CC: <wanghaibin.wang@...wei.com>, <yuzenghui@...wei.com>,
<lushenming@...wei.com>
Subject: [PATCH] KVM: arm64: vgic: Communicate a change of the IRQ state via vgic_queue_irq_unlock
Hi Marc,
Some time ago, you suggested that we should communicate a change
of the IRQ state via vgic_queue_irq_unlock [1], which needs to
include dropping the IRQ from the VCPU's ap_list if the IRQ is
not pending or enabled but on the ap_list. And I additionally
add a case where the IRQ has to be migrated to another ap_list.
(maybe you forget this...)
Does this patch match your thought at the time?
[1] https://lore.kernel.org/patchwork/patch/1371884/
Signed-off-by: Shenming Lu <lushenming@...wei.com>
---
arch/arm64/kvm/vgic/vgic.c | 116 ++++++++++++++++++++++++-------------
1 file changed, 75 insertions(+), 41 deletions(-)
diff --git a/arch/arm64/kvm/vgic/vgic.c b/arch/arm64/kvm/vgic/vgic.c
index 15b666200f0b..9b88d49aa439 100644
--- a/arch/arm64/kvm/vgic/vgic.c
+++ b/arch/arm64/kvm/vgic/vgic.c
@@ -326,8 +326,9 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level, void *owne
/*
* Check whether an IRQ needs to (and can) be queued to a VCPU's ap list.
- * Do the queuing if necessary, taking the right locks in the right order.
- * Returns true when the IRQ was queued, false otherwise.
+ * Do the queuing, dropping or migrating if necessary, taking the right
+ * locks in the right order. Returns true when the IRQ was queued, false
+ * otherwise.
*
* Needs to be entered with the IRQ lock already held, but will return
* with all locks dropped.
@@ -335,49 +336,38 @@ static bool vgic_validate_injection(struct vgic_irq *irq, bool level, void *owne
bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
unsigned long flags)
{
+ struct kvm_vcpu *target_vcpu;
struct kvm_vcpu *vcpu;
+ bool ret = false;
lockdep_assert_held(&irq->irq_lock);
retry:
- vcpu = vgic_target_oracle(irq);
- if (irq->vcpu || !vcpu) {
+ target_vcpu = vgic_target_oracle(irq);
+ vcpu = irq->vcpu;
+ if (target_vcpu == vcpu) {
/*
- * If this IRQ is already on a VCPU's ap_list, then it
- * cannot be moved or modified and there is no more work for
+ * If this IRQ's state is consistent with whether on
+ * the right ap_lsit or not, there is no more work for
* us to do.
- *
- * Otherwise, if the irq is not pending and enabled, it does
- * not need to be inserted into an ap_list and there is also
- * no more work for us to do.
*/
raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
-
- /*
- * We have to kick the VCPU here, because we could be
- * queueing an edge-triggered interrupt for which we
- * get no EOI maintenance interrupt. In that case,
- * while the IRQ is already on the VCPU's AP list, the
- * VCPU could have EOI'ed the original interrupt and
- * won't see this one until it exits for some other
- * reason.
- */
- if (vcpu) {
- kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
- kvm_vcpu_kick(vcpu);
- }
- return false;
+ goto out;
}
/*
* We must unlock the irq lock to take the ap_list_lock where
- * we are going to insert this new pending interrupt.
+ * we are going to insert/drop this IRQ.
*/
raw_spin_unlock_irqrestore(&irq->irq_lock, flags);
/* someone can do stuff here, which we re-check below */
- raw_spin_lock_irqsave(&vcpu->arch.vgic_cpu.ap_list_lock, flags);
+ if (target_vcpu)
+ raw_spin_lock_irqsave(&target_vcpu->arch.vgic_cpu.ap_list_lock,
+ flags);
+ if (vcpu)
+ raw_spin_lock_irqsave(&vcpu->arch.vgic_cpu.ap_list_lock, flags);
raw_spin_lock(&irq->irq_lock);
/*
@@ -392,30 +382,74 @@ bool vgic_queue_irq_unlock(struct kvm *kvm, struct vgic_irq *irq,
* In both cases, drop the locks and retry.
*/
- if (unlikely(irq->vcpu || vcpu != vgic_target_oracle(irq))) {
+ if (unlikely(target_vcpu != vgic_target_oracle(irq) ||
+ vcpu != irq->vcpu)) {
raw_spin_unlock(&irq->irq_lock);
- raw_spin_unlock_irqrestore(&vcpu->arch.vgic_cpu.ap_list_lock,
- flags);
+ if (target_vcpu)
+ raw_spin_unlock_irqrestore(&target_vcpu->arch.vgic_cpu.ap_list_lock,
+ flags);
+ if (vcpu)
+ raw_spin_unlock_irqrestore(&vcpu->arch.vgic_cpu.ap_list_lock,
+ flags);
raw_spin_lock_irqsave(&irq->irq_lock, flags);
goto retry;
}
- /*
- * Grab a reference to the irq to reflect the fact that it is
- * now in the ap_list.
- */
- vgic_get_irq_kref(irq);
- list_add_tail(&irq->ap_list, &vcpu->arch.vgic_cpu.ap_list_head);
- irq->vcpu = vcpu;
+ if (!vcpu && target_vcpu) {
+ /*
+ * Insert this new pending interrupt.
+ * Grab a reference to the irq to reflect the fact that
+ * it is now in the ap_list.
+ */
+ vgic_get_irq_kref(irq);
+ list_add_tail(&irq->ap_list,
+ &target_vcpu->arch.vgic_cpu.ap_list_head);
+ irq->vcpu = target_vcpu;
+ ret = true;
+ } else if (vcpu && !target_vcpu) {
+ /*
+ * This IRQ is not pending or enabled but on the ap_list,
+ * drop it from the ap_list.
+ */
+ list_del(&irq->ap_list);
+ irq->vcpu = NULL;
+ raw_spin_unlock(&irq->irq_lock);
+ vgic_put_irq(vcpu->kvm, irq);
+ raw_spin_unlock_irqrestore(&vcpu->arch.vgic_cpu.ap_list_lock,
+ flags);
+ goto out;
+ } else {
+ /* This IRQ looks like it has to be migrated. */
+ list_del(&irq->ap_list);
+ list_add_tail(&irq->ap_list,
+ &target_vcpu->arch.vgic_cpu.ap_list_head);
+ irq->vcpu = target_vcpu;
+ }
raw_spin_unlock(&irq->irq_lock);
- raw_spin_unlock_irqrestore(&vcpu->arch.vgic_cpu.ap_list_lock, flags);
+ if (target_vcpu)
+ raw_spin_unlock_irqrestore(&target_vcpu->arch.vgic_cpu.ap_list_lock,
+ flags);
+ if (vcpu)
+ raw_spin_unlock_irqrestore(&vcpu->arch.vgic_cpu.ap_list_lock, flags);
- kvm_make_request(KVM_REQ_IRQ_PENDING, vcpu);
- kvm_vcpu_kick(vcpu);
+out:
+ /*
+ * Even for the already queuing rightly case we have
+ * to kick the VCPU, because we could be queueing an
+ * edge-triggered interrupt for which we get no EOI
+ * maintenance interrupt. In that case, while the IRQ
+ * is already on the VCPU's AP list, the VCPU could
+ * have EOI'ed the original interrupt and won't see
+ * this one until it exits for some other reason.
+ */
+ if (target_vcpu) {
+ kvm_make_request(KVM_REQ_IRQ_PENDING, target_vcpu);
+ kvm_vcpu_kick(target_vcpu);
+ }
- return true;
+ return ret;
}
/**
--
2.19.1
Powered by blists - more mailing lists