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);
--
Sincerely,
Andrii Anisov.
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
