This series applies on top of "ARM: Forwarding physical
interrupts to a guest VM" (http://lwn.net/Articles/603514/)
series.
It brings some extra functionalities that were requested to
be able to inject virtual level sensitive IRQs triggered from
VFIO/irqfd.
It adds:
- a specific handling of forwarded IRQ into the VGIC state machine.
- deactivation of physical IRQ and unforwarding on vgic destruction
- handling of forwarded IRQ injection before the vgic readiness:
this was needed because in a sample qemu/vfio use case, qemu
registers forwarded IRQ and set up VFIO signaling before the first
vcpu run and hence before vgic readiness. At that time some
physical IRQ may hit before the VGIC readiness. This is typically
observed with Calxeda xgmac on second QEMU run.
- rbtree lock addition.
Integrated pieces can be found at
ssh://git.linaro.org/people/eric.auger/linux.git
on branch irqfd_integ_v8
The first 2 patch files were previously part of [RFC v2 0/9]
KVM-VFIO IRQ forward control (https://lkml.org/lkml/2014/9/1/347).
Eric Auger (4):
KVM: arm: vgic: fix state machine for forwarded IRQ
KVM: arm: vgic: add forwarded irq rbtree lock
KVM: arm: vgic: cleanup forwarded IRQs on destroy
KVM: arm: vgic: handle irqfd forwarded IRQ injection before vgic
readiness
include/kvm/arm_vgic.h | 1 +
virt/kvm/arm/vgic.c | 128 +++++++++++++++++++++++++++++++++++++++++--------
2 files changed, 110 insertions(+), 19 deletions(-)
--
1.9.1
Add a lock related to the rb tree manipulation. The rb tree can be
searched in one thread (irqfd handler for instance) and map/unmap
may happen in another.
Signed-off-by: Eric Auger <[email protected]>
---
v2 -> v3:
re-arrange lock sequence in vgic_map_phys_irq
---
include/kvm/arm_vgic.h | 1 +
virt/kvm/arm/vgic.c | 56 ++++++++++++++++++++++++++++++++++++--------------
2 files changed, 42 insertions(+), 15 deletions(-)
diff --git a/include/kvm/arm_vgic.h b/include/kvm/arm_vgic.h
index ea31ac6..9b3290b 100644
--- a/include/kvm/arm_vgic.h
+++ b/include/kvm/arm_vgic.h
@@ -220,6 +220,7 @@ struct vgic_dist {
unsigned long *irq_pending_on_cpu;
struct rb_root irq_phys_map;
+ spinlock_t rb_tree_lock;
#endif
};
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 5484e3d..f592219 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -1750,9 +1750,22 @@ static struct rb_root *vgic_get_irq_phys_map(struct kvm_vcpu *vcpu,
int vgic_map_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
{
- struct rb_root *root = vgic_get_irq_phys_map(vcpu, virt_irq);
- struct rb_node **new = &root->rb_node, *parent = NULL;
+ struct rb_root *root;
+ struct rb_node **new, *parent = NULL;
struct irq_phys_map *new_map;
+ struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+
+ root = vgic_get_irq_phys_map(vcpu, virt_irq);
+ new = &root->rb_node;
+
+ new_map = kzalloc(sizeof(*new_map), GFP_KERNEL);
+ if (!new_map)
+ return -ENOMEM;
+
+ new_map->virt_irq = virt_irq;
+ new_map->phys_irq = phys_irq;
+
+ spin_lock(&dist->rb_tree_lock);
/* Boilerplate rb_tree code */
while (*new) {
@@ -1764,19 +1777,16 @@ int vgic_map_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
new = &(*new)->rb_left;
else if (this->virt_irq > virt_irq)
new = &(*new)->rb_right;
- else
+ else {
+ kfree(new_map);
+ spin_unlock(&dist->rb_tree_lock);
return -EEXIST;
+ }
}
- new_map = kzalloc(sizeof(*new_map), GFP_KERNEL);
- if (!new_map)
- return -ENOMEM;
-
- new_map->virt_irq = virt_irq;
- new_map->phys_irq = phys_irq;
-
rb_link_node(&new_map->node, parent, new);
rb_insert_color(&new_map->node, root);
+ spin_unlock(&dist->rb_tree_lock);
return 0;
}
@@ -1805,24 +1815,39 @@ static struct irq_phys_map *vgic_irq_map_search(struct kvm_vcpu *vcpu,
int vgic_get_phys_irq(struct kvm_vcpu *vcpu, int virt_irq)
{
- struct irq_phys_map *map = vgic_irq_map_search(vcpu, virt_irq);
+ struct irq_phys_map *map;
+ struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+ int ret;
+
+ spin_lock(&dist->rb_tree_lock);
+ map = vgic_irq_map_search(vcpu, virt_irq);
if (map)
- return map->phys_irq;
+ ret = map->phys_irq;
+ else
+ ret = -ENOENT;
+
+ spin_unlock(&dist->rb_tree_lock);
+ return ret;
- return -ENOENT;
}
int vgic_unmap_phys_irq(struct kvm_vcpu *vcpu, int virt_irq, int phys_irq)
{
- struct irq_phys_map *map = vgic_irq_map_search(vcpu, virt_irq);
+ struct irq_phys_map *map;
+ struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
+
+ spin_lock(&dist->rb_tree_lock);
+
+ map = vgic_irq_map_search(vcpu, virt_irq);
if (map && map->phys_irq == phys_irq) {
rb_erase(&map->node, vgic_get_irq_phys_map(vcpu, virt_irq));
kfree(map);
+ spin_unlock(&dist->rb_tree_lock);
return 0;
}
-
+ spin_unlock(&dist->rb_tree_lock);
return -ENOENT;
}
@@ -2078,6 +2103,7 @@ int kvm_vgic_create(struct kvm *kvm)
}
spin_lock_init(&kvm->arch.vgic.lock);
+ spin_lock_init(&kvm->arch.vgic.rb_tree_lock);
kvm->arch.vgic.in_kernel = true;
kvm->arch.vgic.vctrl_base = vgic->vctrl_base;
kvm->arch.vgic.vgic_dist_base = VGIC_ADDR_UNDEF;
--
1.9.1
This patch handles the case where irqfd attempts to inject
a forwarded IRQ before the vgic is ready to accept it. We
cannot simply return pretending nothing happened since the IRQ
will never be deactivated by the guest. Indeed, the corresponding
virtual IRQ cannot be injected into the guest and the host
must deactivate it.
Signed-off-by: Eric Auger <[email protected]>
---
virt/kvm/arm/vgic.c | 20 ++++++++++++++++++++
1 file changed, 20 insertions(+)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index 21419ac..27394b0 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -2672,6 +2672,13 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id,
u32 irq, int level, bool line_status)
{
unsigned int spi = irq + VGIC_NR_PRIVATE_IRQS;
+ struct kvm_vcpu *vcpu = kvm_get_vcpu(kvm, 0);
+ int phys_irq = vgic_get_phys_irq(vcpu, spi);
+ bool is_forwarded = (phys_irq >= 0);
+ unsigned long flags;
+ struct irq_desc *desc;
+ struct irq_data *d;
+ struct irq_chip *chip;
kvm_debug("irqfd sets vIRQ %d to %d\n", irq, level);
@@ -2679,6 +2686,19 @@ int kvm_set_irq(struct kvm *kvm, int irq_source_id,
if (spi > kvm->arch.vgic.nr_irqs)
return -EINVAL;
return kvm_vgic_inject_irq(kvm, 0, spi, level);
+ }
+
+ if (level && is_forwarded) {
+ /*
+ * the virtual IRQ never is injected into the guest
+ * it must be manually completed on host side.
+ */
+ desc = irq_to_desc(phys_irq);
+ raw_spin_lock_irqsave(&desc->lock, flags);
+ d = &desc->irq_data;
+ chip = desc->irq_data.chip;
+ chip->irq_eoi(d);
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
} else if (level && kvm_irq_has_notifier(kvm, 0, irq)) {
/*
* any IRQ injected before vgic readiness is
--
1.9.1
When the VGIC is destroyed it must take care of
- restoring the forwarded IRQs in non forwarded state,
- deactivating the IRQ in case the guest left without doing it
- cleaning nodes of the phys_map rbtree
Signed-off-by: Eric Auger <[email protected]>
---
virt/kvm/arm/vgic.c | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index f592219..21419ac 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -32,6 +32,7 @@
#include <asm/kvm_emulate.h>
#include <asm/kvm_arm.h>
#include <asm/kvm_mmu.h>
+#include <linux/spinlock.h>
/*
* How the whole thing works (courtesy of Christoffer Dall):
@@ -102,6 +103,8 @@ static struct vgic_lr vgic_get_lr(const struct kvm_vcpu *vcpu, int lr);
static void vgic_set_lr(struct kvm_vcpu *vcpu, int lr, struct vgic_lr lr_desc);
static void vgic_get_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
static void vgic_set_vmcr(struct kvm_vcpu *vcpu, struct vgic_vmcr *vmcr);
+static void vgic_clean_irq_phys_map(struct kvm_vcpu *vcpu,
+ struct rb_root *root);
static const struct vgic_ops *vgic_ops;
static const struct vgic_params *vgic;
@@ -1813,6 +1816,36 @@ static struct irq_phys_map *vgic_irq_map_search(struct kvm_vcpu *vcpu,
return NULL;
}
+static void vgic_clean_irq_phys_map(struct kvm_vcpu *vcpu,
+ struct rb_root *root)
+{
+ unsigned long flags;
+
+ while (1) {
+ struct rb_node *node = rb_first(root);
+ struct irq_phys_map *map;
+ struct irq_desc *desc;
+ struct irq_data *d;
+ struct irq_chip *chip;
+
+ if (!node)
+ break;
+
+ map = container_of(node, struct irq_phys_map, node);
+ desc = irq_to_desc(map->phys_irq);
+
+ raw_spin_lock_irqsave(&desc->lock, flags);
+ d = &desc->irq_data;
+ chip = desc->irq_data.chip;
+ irqd_clr_irq_forwarded(d);
+ chip->irq_eoi(d);
+ raw_spin_unlock_irqrestore(&desc->lock, flags);
+
+ rb_erase(node, root);
+ kfree(map);
+ }
+}
+
int vgic_get_phys_irq(struct kvm_vcpu *vcpu, int virt_irq)
{
struct irq_phys_map *map;
@@ -1855,6 +1888,7 @@ void kvm_vgic_vcpu_destroy(struct kvm_vcpu *vcpu)
{
struct vgic_cpu *vgic_cpu = &vcpu->arch.vgic_cpu;
+ vgic_clean_irq_phys_map(vcpu, &vgic_cpu->irq_phys_map);
kfree(vgic_cpu->pending_shared);
kfree(vgic_cpu->vgic_irq_lr_map);
vgic_cpu->pending_shared = NULL;
@@ -1920,6 +1954,8 @@ void kvm_vgic_destroy(struct kvm *kvm)
kvm_for_each_vcpu(i, vcpu, kvm)
kvm_vgic_vcpu_destroy(vcpu);
+ vgic_clean_irq_phys_map(vcpu, &dist->irq_phys_map);
+
vgic_free_bitmap(&dist->irq_enabled);
vgic_free_bitmap(&dist->irq_level);
vgic_free_bitmap(&dist->irq_pending);
--
1.9.1
Fix multiple injection of level sensitive forwarded IRQs.
With current code, the second injection fails since the state bitmaps
are not reset (process_maintenance is not called anymore).
New implementation follows those principles:
- A forwarded IRQ only can be sampled when it is pending
- when queueing the IRQ (programming the LR), the pending state is removed
as for edge sensitive IRQs
- an injection of a forwarded IRQ is considered always valid since
coming from the HW and level always is 1.
Signed-off-by: Eric Auger <[email protected]>
---
v2 -> v3:
- integration in new vgic_can_sample_irq
- remove the pending state when programming the LR
v1 -> v2:
- fix vgic state bypass in vgic_queue_hwirq
Conflicts:
virt/kvm/arm/vgic.c
---
virt/kvm/arm/vgic.c | 16 ++++++++++++----
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/virt/kvm/arm/vgic.c b/virt/kvm/arm/vgic.c
index ccb3801..5484e3d 100644
--- a/virt/kvm/arm/vgic.c
+++ b/virt/kvm/arm/vgic.c
@@ -360,7 +360,10 @@ static void vgic_cpu_irq_clear(struct kvm_vcpu *vcpu, int irq)
static bool vgic_can_sample_irq(struct kvm_vcpu *vcpu, int irq)
{
- return vgic_irq_is_edge(vcpu, irq) || !vgic_irq_is_queued(vcpu, irq);
+ bool is_forwarded = (vgic_get_phys_irq(vcpu, irq) >= 0);
+
+ return vgic_irq_is_edge(vcpu, irq) || !vgic_irq_is_queued(vcpu, irq) ||
+ (is_forwarded && vgic_dist_irq_is_pending(vcpu, irq));
}
static u32 mmio_data_read(struct kvm_exit_mmio *mmio, u32 mask)
@@ -1295,6 +1298,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
struct vgic_dist *dist = &vcpu->kvm->arch.vgic;
struct vgic_lr vlr;
int lr;
+ bool is_forwarded = (vgic_get_phys_irq(vcpu, irq) >= 0);
/* Sanitize the input... */
BUG_ON(sgi_source_id & ~7);
@@ -1330,7 +1334,7 @@ static bool vgic_queue_irq(struct kvm_vcpu *vcpu, u8 sgi_source_id, int irq)
vlr.irq = irq;
vlr.source = sgi_source_id;
vlr.state = LR_STATE_PENDING;
- if (!vgic_irq_is_edge(vcpu, irq))
+ if (!vgic_irq_is_edge(vcpu, irq) && !is_forwarded)
vlr.state |= LR_EOI_INT;
vgic_set_lr(vcpu, lr, vlr);
@@ -1371,11 +1375,12 @@ static bool vgic_queue_sgi(struct kvm_vcpu *vcpu, int irq)
static bool vgic_queue_hwirq(struct kvm_vcpu *vcpu, int irq)
{
+ bool is_forwarded = (vgic_get_phys_irq(vcpu, irq) >= 0);
if (!vgic_can_sample_irq(vcpu, irq))
return true; /* level interrupt, already queued */
if (vgic_queue_irq(vcpu, 0, irq)) {
- if (vgic_irq_is_edge(vcpu, irq)) {
+ if (vgic_irq_is_edge(vcpu, irq) || is_forwarded) {
vgic_dist_irq_clear_pending(vcpu, irq);
vgic_cpu_irq_clear(vcpu, irq);
} else {
@@ -1636,14 +1641,17 @@ static bool vgic_update_irq_pending(struct kvm *kvm, int cpuid,
int edge_triggered, level_triggered;
int enabled;
bool ret = true;
+ bool is_forwarded;
spin_lock(&dist->lock);
vcpu = kvm_get_vcpu(kvm, cpuid);
+ is_forwarded = (vgic_get_phys_irq(vcpu, irq_num) >= 0);
+
edge_triggered = vgic_irq_is_edge(vcpu, irq_num);
level_triggered = !edge_triggered;
- if (!vgic_validate_injection(vcpu, irq_num, level)) {
+ if (!vgic_validate_injection(vcpu, irq_num, level) && !is_forwarded) {
ret = false;
goto out;
}
--
1.9.1
On Sun, Nov 23, 2014 at 07:12:49PM +0100, Eric Auger wrote:
> This series applies on top of "ARM: Forwarding physical
> interrupts to a guest VM" (http://lwn.net/Articles/603514/)
> series.
Marc and Eric,
Does it make sense to review and look at these patches given the current
state of the forwarding patches, or should we wait until Marc respins
that series?
>
> It brings some extra functionalities that were requested to
> be able to inject virtual level sensitive IRQs triggered from
> VFIO/irqfd.
>
> It adds:
> - a specific handling of forwarded IRQ into the VGIC state machine.
> - deactivation of physical IRQ and unforwarding on vgic destruction
> - handling of forwarded IRQ injection before the vgic readiness:
> this was needed because in a sample qemu/vfio use case, qemu
> registers forwarded IRQ and set up VFIO signaling before the first
> vcpu run and hence before vgic readiness. At that time some
> physical IRQ may hit before the VGIC readiness. This is typically
> observed with Calxeda xgmac on second QEMU run.
this seems related to my note in the last patch? Same or different
problem?
-Christoffer
> - rbtree lock addition.
>
> Integrated pieces can be found at
> ssh://git.linaro.org/people/eric.auger/linux.git
> on branch irqfd_integ_v8
>
> The first 2 patch files were previously part of [RFC v2 0/9]
> KVM-VFIO IRQ forward control (https://lkml.org/lkml/2014/9/1/347).
>
>
> Eric Auger (4):
> KVM: arm: vgic: fix state machine for forwarded IRQ
> KVM: arm: vgic: add forwarded irq rbtree lock
> KVM: arm: vgic: cleanup forwarded IRQs on destroy
> KVM: arm: vgic: handle irqfd forwarded IRQ injection before vgic
> readiness
>
> include/kvm/arm_vgic.h | 1 +
> virt/kvm/arm/vgic.c | 128 +++++++++++++++++++++++++++++++++++++++++--------
> 2 files changed, 110 insertions(+), 19 deletions(-)
>
> --
> 1.9.1
>
On 24/11/14 10:50, Christoffer Dall wrote:
> On Sun, Nov 23, 2014 at 07:12:49PM +0100, Eric Auger wrote:
>> This series applies on top of "ARM: Forwarding physical
>> interrupts to a guest VM" (http://lwn.net/Articles/603514/)
>> series.
>
> Marc and Eric,
>
> Does it make sense to review and look at these patches given the current
> state of the forwarding patches, or should we wait until Marc respins
> that series?
I'm still in the process of respining all of this (we've had a few
iterations with tglx, but I got sidetracked with the ITS/v2m/MSI side of
things).
I'll get back onto this this week. Hopefully.
Thanks,
M.
--
Jazz is not dead. It just smells funny...
On 11/24/2014 11:54 AM, Marc Zyngier wrote:
> On 24/11/14 10:50, Christoffer Dall wrote:
>> On Sun, Nov 23, 2014 at 07:12:49PM +0100, Eric Auger wrote:
>>> This series applies on top of "ARM: Forwarding physical
>>> interrupts to a guest VM" (http://lwn.net/Articles/603514/)
>>> series.
>>
>> Marc and Eric,
>>
>> Does it make sense to review and look at these patches given the current
>> state of the forwarding patches, or should we wait until Marc respins
>> that series?
>
> I'm still in the process of respining all of this (we've had a few
> iterations with tglx, but I got sidetracked with the ITS/v2m/MSI side of
> things).
Hi Marc, Christoffer,
for your info, I integrated with kvm-arm64/irq-forward branch prior to
ITS integration, not with the RFC.
Best Regards
Eric
>
> I'll get back onto this this week. Hopefully.
>
> Thanks,
>
> M.
>