[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] gic:vgic: avoid excessive conversions


Could you please comment on the patch and below.

On 20.11.18 13:09, Andrii Anisov wrote:
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);
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);

Andrii Anisov.

Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.