: Really disable pirq's

All,
I have a laptop here that has some interrupt trouble,
unrelated to Xen (I
believe it's a bug in the ACPI tables, but I digress). On the

kernel, if 999,900 out of 100,000 interrupts go un-acknowledged to a particular
interrupt line, the kernel disables that interrupt line. In the Xen kernel, the
same logic applies. However, when the Xen kernel goes to disable the physical
interrupt, it only masks out the event channel:

static void disable_pirq(unsigned int irq)

{
       int evtchn = evtchn_from_irq(irq);

       if (VALID_EVTCHN(evtchn))
               mask_evtchn(evtchn);
}

And at this point, *all* interrupts on the affected machine seem to stop, not
just this physical IRQ line. I believe the problem is that when we go to
disable a PIRQ, we actually need to get the HV to mask it out on the IOAPIC.
This patch does exactly that; on disable_pirq(), we actually just call out to
shutdown_pirq(), which ends up hypercalling and getting the HV to mask out the
interrupt. On the other side, I needed to modify enable_pirq() to call out to
startup_pirq(), so that it would actually allocate the event channel.

With this patch in place, the affected laptop no longer hangs up when the kernel
decides to disable that particular interrupt line.

Signed-off-by Chris Lalancette <[email protected]>

Not sure if the change is a bit over-kill, since enable_pirq is has void return type, while startup_pirq is "int" return type, with possibility to fail.

For example, in following situation, the startup_pirq may fail : 1) when startup_pirq again, fail to get free port, 2) if another domain try to bind the pirq with BIND_PIRQ_WILL_SHARE cleared (like to probing, will it happen?) between the shutdown_pirq/startup_pirq sequence.

Quoted 3 lines · Yunhong

Not sure if the change is a bit over-kill, since enable_pirq is has void return type, while startup_pirq is "int" return type, with possibility to fail.

Thanks for looking!

This is true, startup_pirq() *could* fail; but if you notice in the code, it doesn't actually have anything but a "return 0", so it doesn't report errors currently anyway.

Quoted 5 lines

For example, in following situation, the startup_pirq may fail : 1) when startup_pirq again, fail to get free port, 2) if another domain try to bind the pirq with BIND_PIRQ_WILL_SHARE cleared (like to probing, will it happen?) between the shutdown_pirq/startup_pirq sequence.

Yes, you are right, this can happen if another domain is probing. However, I'm
not sure that it is any different from when you are calling ->startup() for the
first time; you will just fail to get the event channel. Without introducing another event channel op (which seems like a LOT of overkill), I'm not aware of
another way of asking the HV to mask out that IRQ on the IOAPIC.

Chris Lalancette

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Quoted 1 line · Yunhong

Not sure if the change is a bit over-kill, since enable_pirq is has

Quoted 1 line

return type, while startup_pirq is "int" return type, with

Quoted 1 line

to fail.

Thanks for looking!

This is true, startup_pirq() *could* fail; but if you notice in the code, it
doesn't actually have anything but a "return 0", so it doesn't report

currently anyway.

Quoted 2 lines

For example, in following situation, the startup_pirq may fail : 1)

Quoted 1 line

startup_pirq again, fail to get free port, 2) if another domain try

Quoted 1 line

bind the pirq with BIND_PIRQ_WILL_SHARE cleared (like to probing,

Quoted 1 line

it happen?) between the shutdown_pirq/startup_pirq sequence.

Yes, you are right, this can happen if another domain is probing.

I'm not sure that it is any different from when you are calling

for the first time; you will just fail to get the event channel.

introducing another event channel op (which seems like a LOT of

I'm not aware of another way of asking the HV to mask out that IRQ on

IOAPIC.

Chris Lalancette

All,
I have a laptop here that has some interrupt trouble, unrelated to Xen (I
believe it's a bug in the ACPI tables, but I digress). On the bare-metal kernel, if 999,900 out of 100,000 interrupts go un-acknowledged to a particular
interrupt line, the kernel disables that interrupt line. In the Xen kernel, the
same logic applies. However, when the Xen kernel goes to disable the physical interrupt, it only masks out the event channel:

static void disable_pirq(unsigned int irq)

{
        int evtchn = evtchn_from_irq(irq);

        if (VALID_EVTCHN(evtchn))
                mask_evtchn(evtchn);
}

And at this point, *all* interrupts on the affected machine seem to stop, not just this physical IRQ line. I believe the problem is that when we go to disable a PIRQ, we actually need to get the HV to mask it out on the IOAPIC. This patch does exactly that; on disable_pirq(), we actually just call out to shutdown_pirq(), which ends up hypercalling and getting the HV to mask out the interrupt. On the other side, I needed to modify enable_pirq() to call out to startup_pirq(), so that it would actually allocate the event channel.

With this patch in place, the affected laptop no longer hangs up when the kernel
decides to disable that particular interrupt line.

Signed-off-by Chris Lalancette <[email protected]>

drivers/xen/core/evtchn.c renamed from drivers/xen/core/evtchn.c.orig +2 -10

@@ -629,20 +629,12 @@ static void shutdown_pirq(unsigned int i
 
 static void enable_pirq(unsigned int irq)
 {
- int evtchn = evtchn_from_irq(irq);
-
- if (VALID_EVTCHN(evtchn)) {
-  unmask_evtchn(evtchn);
-  pirq_unmask_notify(irq_to_pirq(irq));
- }
+ startup_pirq(irq);
 }
 
 static void disable_pirq(unsigned int irq)
 {
- int evtchn = evtchn_from_irq(irq);
-
- if (VALID_EVTCHN(evtchn))
-  mask_evtchn(evtchn);
+ shutdown_pirq(irq);
 }
 
 static void ack_pirq(unsigned int irq)

_______________________________________________
Xen-devel mailing list
[email protected]
http://lists.xensource.com/xen-devel

Chris,

Could you try the alternative attached patch? It's not been build or run tested, but hopefully you can see the gist of what's going on. Basically, in case the irq is edge-triggered, I want to have an 'edge in hand' before we actually shutdown the irq.

Chris,

Could you try the alternative attached patch? It's not been build or run tested, but hopefully you can see the gist of what's going on. Basically, in case the irq is edge-triggered, I want to have an 'edge in hand' before we actually shutdown the irq.

-- Keir