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

Re: [Xen-devel] [PATCH v2] gic: drop interrupts enabling on interrupts processing

(+ Andre)


Title: Interrupts are still unmasked when executing action for interrupt routed to Xen. So you need to be more specific. How about
"xen/arm: gic: Defer the decision to unmask interrupts to do_{LPI, IRQ}()"?

On 5/27/19 10:29 AM, Andrii Anisov wrote:
From: Andrii Anisov <andrii_anisov@xxxxxxxx>

This reduces the number of context switches in case we have coming guest
interrupts from different sources at a high rate. What is likely for


multimedia use-cases.
Having irqs unlocked here makes us go through trap path again in case we

what do you mean by "unlocked"?

have a new guest interrupt arrived (even with the same priority, after
`desc->handler->end(desc)` in `do_IRQ()`), what is just a processor
cycles wasting.
after `desc->....`. This is just a waste a processor cycle as we will catch them all in the function gic_interrupt() loop.

 We will catch them all in the `gic_interrupt() function
loop anyway. And the guest irqs arrival prioritization is meaningless
here, it is only effective at guest's level.

I am not sure why you speak about guest prioritization here. The main issue would be an interrupt to Xen (i.e timer) that would get delayed because of longer period without interrupt enabled. I would also not rule out the possibility to prioritize guest interrupt at hardware level.

I know we have been discussing on the problem in the past, but a summary in the commit message is quite important to not miss out all the problems.

The real problem here is for interrupt routed to guest the interrupt will be kept unmasked when calling desc->handler->end(desc). This will result to receive the next interrupt as soon as desc->handler->end(desc) is called.

In the case of interrupt routed to Xen, interrupts will be kept enabled while executing the action but then disabled before calling desc->handler->end(desc).

It would be fine to keep the interrupts masked for interrupts routed to the guest because vgic_inject_irq(...) will be masking the interrupt in most of the cases.

The code below looks good to me. I am happy to help rewording the commit message if necessary.

While looking at the code, I noticed that in the new vgic vgic_get_irq() looks unsafe to be called with interrupt unmasked. This is because one of the callee (vgic_get_lpi()) takes a spinlock and not a spinlock_irq. Andre, what do you think?

Signed-off-by: Andrii Anisov <andrii_anisov@xxxxxxxx>


     in v2: Drop irq enabling for lpi processing as well.

  xen/arch/arm/gic.c | 4 ----
  1 file changed, 4 deletions(-)

diff --git a/xen/arch/arm/gic.c b/xen/arch/arm/gic.c
index 6cc7dec..113655a 100644
--- a/xen/arch/arm/gic.c
+++ b/xen/arch/arm/gic.c
@@ -386,17 +386,13 @@ void gic_interrupt(struct cpu_user_regs *regs, int is_fiq)
if ( likely(irq >= 16 && irq < 1020) )
-            local_irq_enable();
              do_IRQ(regs, irq, is_fiq);
-            local_irq_disable();
          else if ( is_lpi(irq) )
-            local_irq_enable();
-            local_irq_disable();
          else if ( unlikely(irq < 16) )


Julien Grall

Xen-devel mailing list



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