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

[Andrii Anisov <andrii.anisov@xxxxxxxxx>]

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@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel