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

Re: [Xen-devel] Why xen-pirq chip use startup_irq() for .irq_enable?

On Thu 27.Jul'17 at 12:06:10 -0400, Boris Ostrovsky wrote:
(Adjusting addressees: David is no longer maintaining Xen code, Juergen is)
Thanks Boris.

On 07/27/2017 09:04 AM, shuo.a.liu@xxxxxxxxx wrote:
Here is a device has xen-pirq-MSI interrupt. I found dom0 might lost
interrupt during driver irq_disable/irq_enable.
There is a pair of irq_disable/enable in driver. Here is the scenario,
 1. irq_disable(dev_irq) -> disable_dynirq -> mask_evtchn(dev_irq
 2. dev interrupt raised by HW and Xen mark its evtchn as *pending*
 3. irq_enable(dev_irq) -> startup_pirq -> eoi_pirq ->
    clear_evtchn(channel of dev_irq) -> clear *pending* status
 4. consume_one_event process the dev irq event without pending bit
    which result in interrupt lost once.
 5. No HW interrupt raising anymore.

The first question here is why using startup_irq for .irq_enable
rather than
enable_dynirq ? startup_irq will do eoi_pirq who clear the mask bit
and pending
bit of the channel while enable_dynirq just only unmask the channel.

Seems like enable_dynirq() would indeed be the right choice. What is a
bit strange is that scenario that you are describing looks pretty common
so we should have hit this problem before.
This point confused me also. It seems the code has been here for long time.
Anyway, if you think it is the right fix, i can send out a formal patch.

Second question is that what's the purpose of eoi_pirq in startup_irq?

When we are actually creating new pirq we want to make sure there are no
pending interrupts left over from previous use of the pirq.
If interrupt raise just before eoi_pirq in startup_irq, we might face
the same issue? Can we make sure pirq is clean when do binding?

Thx -


BTW, i can resolve my problem by below patch. Does it make sence?

drivers/xen/events/events_base.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/xen/events/events_base.c
index 4bf7a34..341c456 100644
--- a/drivers/xen/events/events_base.c
+++ b/drivers/xen/events/events_base.c
@@ -582,7 +582,7 @@ static void shutdown_pirq(struct irq_data *data)

static void enable_pirq(struct irq_data *data)
-    startup_pirq(data);
+    enable_dynirq(data);

static void disable_pirq(struct irq_data *data)

Xen-devel mailing list



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