Hello Andre,
I'm going to change "gic_raise_guest_irq()" function interface.
Could you please comment my understanding of vgic-v3-its.c code below? So that
I could fix it alongside the function interface change.
On 16.11.18 18:45, Andrii Anisov wrote:
diff --git a/xen/arch/arm/vgic-v3-its.c b/xen/arch/arm/vgic-v3-its.c
index 5b73c4e..193a28f 100644
--- a/xen/arch/arm/vgic-v3-its.c
+++ b/xen/arch/arm/vgic-v3-its.c
@@ -447,7 +447,7 @@ static void update_lpi_vgic_status(struct vcpu *v, struct
pending_irq *p)
{
if ( !list_empty(&p->inflight) &&
!test_bit(GIC_IRQ_GUEST_VISIBLE, &p->status) )
As I understand, the intention here is to reinsert an irq to `lr_pending` queue with
an updated priority, just in case the irq is not yet visible to guests. You try to
pass a `p->lpi_priority` to the `gic_raise_guest_irq` as a new priority. But, with
the current implementation, that parameter is clearly ignored by the function.
Moreover, it just could not be honored there, because `gic_raise_guest_irq` touches
only `lr_pending` queue, while both `lr_pending` and `inflight_irqs` are sorted by
`p->priority` and you can't change it while updating only one queue.
I guess here the irq should be removed from both queues first, then `p->priority`
updated to `p->lpi_priority`, then irq should be reinserted to both queues.
What do you think about that?
- gic_raise_guest_irq(v, p->irq, p->lpi_priority);
+ gic_raise_guest_irq(v, p);
}
else
The call below looks excessive to me. We can reach this branch in case
`p->inflight` is empty or it is not empty, but irq is visible to guest. In both
those cases irq can not be on lr_pending queue, so calling this function is just a
wasting cpu cycles.
gic_remove_from_lr_pending(v, p);
