VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Pavel Pisa]

Hello all,

because there is no maintainer specified for VM86 subsystem, I bother you all.

There is is problem in VM86 emulation code. It returns IRQ_NONE for each
handler invocation. This results in missdetection of interrupt as uncared
and it is disabled after 100000 invocations. Next patch solves this problem.
It still returns IRQ_NONE, if the interrupt servicing by userspace
is lacking/broken or if interrupt is stuck and could freeze kernel.

The original code could not work with level invocated interrupts at all,
it would lead to the contant system freeze if IRQ_HANDLED is returned.
IRQ is didabled now in the handler and reenabled in get_and_reset_irq()
by VM86_GET_AND_RESET_IRQ IOCTL.

The FIX has been tested on 2.6.4 kernel, but I have not noticed any
change solving problem of VM86 IRQ in 2.6 BitKeeper repository
till yesterday.

We are using this VM86 feature for education purposes. Students
can write and debug their code controlling some IRC with IRQ
equipped DC motor from userspace first.
Than code could be moved to RT-Linux threads or Linux driver module.
This will be part of next run of Computer for Control curse
at our department
   http://dce.felk.cvut.cz/por/

I have used this setup even for testing and development of real drivers.
I have macros and simple adaptation code, that low level parts of
real driver code can be run in userspace, attaching interrupts through
VM86, IO/MEM through ioperm and mmap. If somebody is interrested
in this approach, I can send my sources.

Best wishes

                Pavel Pisa
        e-mail: pisa@cmp.felk.cvut.cz
        www:    http://cmp.felk.cvut.cz/~pisa
        work:   http://www.pikron.com


Name: VM86 interrupt emulation FIX for 2.6.x
Status: Trivial
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>

Fixes faulty IRQ_NONE value returning by VM86 irq_handler().
The IRQ source is blocked as well until userspace confirms processing.
This should enable to use VM86 code even for level triggered
interrupt sources.

--- linux-2.6.4/arch/i386/kernel/vm86.c.orig 2004-03-11 03:55:22.000000000 
+0100
+++ linux-2.6.4/arch/i386/kernel/vm86.c 2004-12-08 11:25:08.000000000 +0100
@@ -718,6 +718,10 @@ static irqreturn_t irq_handler(int intno
  if (vm86_irqs[intno].sig)
   send_sig(vm86_irqs[intno].sig, vm86_irqs[intno].tsk, 1);
  /* else user will poll for IRQs */
+ spin_unlock_irqrestore(&irqbits_lock, flags); 
+ /* Next line would be required to handle correctly level activated 
interrupts */
+ disable_irq(intno);
+ return IRQ_HANDLED;
 out:
  spin_unlock_irqrestore(&irqbits_lock, flags); 
  return IRQ_NONE;

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Linus Torvalds]

On Thu, 9 Dec 2004, Pavel Pisa wrote:
> 
> because there is no maintainer specified for VM86 subsystem, I bother you all.
> 
> There is is problem in VM86 emulation code. It returns IRQ_NONE for each
> handler invocation.

Yes. I realized how broken it was when I did the conversion, but I didn't 
have anybody that I knew of that actually _used_ it, so I didn't bother 
trying to fix it.

It's been broken for a long time, even before the IRQ_NONE/IRQ_HANDLED
fixes (the disable/enable counting was out of whack).

Your fix looks fine. You must be one of a _very_ small group of people 
using this. Thanks,

			Linus

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Alan Cox]

On Iau, 2004-12-09 at 13:59, Pavel Pisa wrote:
>   if (vm86_irqs[intno].sig)
>    send_sig(vm86_irqs[intno].sig, vm86_irqs[intno].tsk, 1);
>   /* else user will poll for IRQs */
> + spin_unlock_irqrestore(&irqbits_lock, flags); 
> + /* Next line would be required to handle correctly level activated 
> interrupts */
> + disable_irq(intno);
> + return IRQ_HANDLED;

You can't disable_irq and return to user space - the IRQ may be shared
by a device needed to make user space progress. 

The rest looks like a real bug.

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Linus Torvalds]

On Fri, 10 Dec 2004, Alan Cox wrote:
> 
> You can't disable_irq and return to user space - the IRQ may be shared
> by a device needed to make user space progress. 

The vm86 interrupt does not allow sharing. And it really _has_ to be 
disabled until user mode has cleared the irq source, or you'll have a very 
dead machine.

		Linus

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Alan Cox]

On Gwe, 2004-12-10 at 22:55, Linus Torvalds wrote:
> On Fri, 10 Dec 2004, Alan Cox wrote:
> The vm86 interrupt does not allow sharing. And it really _has_ to be 
> disabled until user mode has cleared the irq source, or you'll have a very 
> dead machine.

Until the 10,000th event it actually seems to work rather happily
without
that change. The interrupt has already occurred at this point and it was
edge triggered so the interrupt will be cleared down by the kernel on
the irq handler return path. Nothing expects that interrupt to get
re-enabled, or deals with refcounting in his patch, or with races. It
doesn't need disable_irq except for level triggered and vm86 has never
handled that. In fact right now it can't because multiple signals are
merged and you never know how many IRQ events occurred.

That limit works because the old vm86 irq hack only works with the edge
triggered old style PC ISA interrupts.

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Alan Cox]

On Sad, 2004-12-11 at 01:23, Linus Torvalds wrote:
> > Until the 10,000th event it actually seems to work rather happily
> > without that change.
> 
> I suspect you never tried the level-triggered case.

Level triggered has never been supported. Putting a single disable_irq
in doesn't change the fact it doesn't work because the IRQ is never
re-enabled.

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Linus Torvalds]

On Fri, 10 Dec 2004, Alan Cox wrote:
>
> On Gwe, 2004-12-10 at 22:55, Linus Torvalds wrote:
> > On Fri, 10 Dec 2004, Alan Cox wrote:
> > The vm86 interrupt does not allow sharing. And it really _has_ to be 
> > disabled until user mode has cleared the irq source, or you'll have a very 
> > dead machine.
> 
> Until the 10,000th event it actually seems to work rather happily
> without that change.

I suspect you never tried the level-triggered case.

In the level-triggered case, the 10,000 interrupts happen faster than 
user-mode can even say "Whaaa?".

		Linus

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Pavel Pisa]

On Friday 10 December 2004 23:55, Linus Torvalds wrote:
> 
> On Fri, 10 Dec 2004, Alan Cox wrote:
> > 
> > You can't disable_irq and return to user space - the IRQ may be shared
> > by a device needed to make user space progress. 
> 
> The vm86 interrupt does not allow sharing. And it really _has_ to be 
> disabled until user mode has cleared the irq source, or you'll have a very 
> dead machine.

I have thought about all these problems before change proposal.
There is no other way to do interrupt propagation to user-space
for user-space only serviced level activated interrupt.
I would like very much ability to propagate some interrupts
serviced by real driver into userspace as well.
This is well doable, if real driver clears source and VM86
shared handler would return IRQ_NONE and not disable IRQ in this case.
There is no clean way, how to solve level activated IRQ sharing between
two devices, one serviced by real driver and the second by userspace.
Probably IRQ disable would partially work, if real driver would
not be at core path (swapping) device or device blocking userspace
"driver" thread.

There are some more IRQ related issues ad questions.

1) Actual VM86 IRQ solution works well for us, but I have
   found even before patch sending one scenario asking for check.
   IRQ appears in HW, disable_irq() is called by the handler.
   Userspace task is deleted before get_and_reset_irq()
   reenables IRQ. free_vm86_irq() is called.
   Now some real kernel module asks for same IRQ
   by request_irq() call. It succeeds, because IRQ is no longer
   reserved for VM86. But what does happen with IRQ disable counter
   in such case? 
   ... OK, I have recheck that now, "if (!shared) {" in  setup_irq()
   should correctly solve that case. It enables IRQ again and cleans counter.

2) I think, that the VM86 IRQ handling is x86 specific only by its name.
   It is shame, that it is allowed only for interrupts 3 to 15
   and implemented only for x86 architecture. This is feature, which
   in combination with some libraries, could be used for drivers
   debugging in userspace even for ARM or other targets.
   I have used this feature years ago for debugging of uLan driver
   and pre-LinCAN incarnations of my CAN experiments.
   Are there more people who think that this could be of some value?

3) What is your opinion about sharing edge triggered interrupts
   on x86 architecture?
   It was unserviceable for all kernels before irqreturn_t introduction.
   This seems to be broken up to 2.6.9 still.
   Let there are two shared edge triggered sources A and B.
   Device B asks for IRQ, ISR for A is called and returns,
   request from device A arrives, but there is no edge, line is
   held by B. B request proceeds, but no edge appears, line is held by A.
   Handlers are not called any more => stuck IRQ condition.
   It is necessary cycle calls to all handlers until one full round
   of IRQ_NONE is received. Have I overlooked something, which solves
   this situation in the current kernel? Is there willing to do something
   with that. I can prepare and test some solution if there is interrest.
   I know, that ISA is out of scope for desktop PCs now, but low count of
   available IRQs is common problem on embedded PC-104 targets.
   They would benefit from this enhancement.
   
Best wishes

                Pavel Pisa

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Pavel Pisa]

On Saturday 11 December 2004 01:57, you wrote:
> On Sad, 2004-12-11 at 01:23, Linus Torvalds wrote:
> > > Until the 10,000th event it actually seems to work rather happily
> > > without that change.
> > 
> > I suspect you never tried the level-triggered case.
> 
> Level triggered has never been supported. Putting a single disable_irq
> in doesn't change the fact it doesn't work because the IRQ is never
> re-enabled.

The real very first reason for disable_irq introduction has been
infinite list of unbalanced enable_irq calls reported in the syslog,
caused by enable_irq(irqnumber) call from get_and_reset_irq().
It has been there for ages.
Not nice behavior. You see 100000 of such messages and then
processing stops on IRQ_NONE catch. I am voting for benefit of
our _very_ small VM86 IRQ emulation users community, that actual
state is many times better, than previous state. It is usable now.
I believe, that even level triggered interrupts should work
now without problems. We will give this feature real load testing.
We use it for educational DC motor control. 10kHz of IRQs.
We know, that even 2.6.x with preempt cannot handle that
without IRQ misses. But it is nice test to see on a osciloscope,
how userspace task reacts on IRQ events. And I think,
that 2.6.x kernel can evolve to one millisecond guaranteed response
time one day. It would be great. We have experience with real
drivers and writting code for RT-Linux as well.
I do not believe, that 2.6 kernels could get under 300 us
in reasonable future, so somethink like RT-Linux or dedicated
MCU would be still required for more demanding tasks.
But one millisecond could be enough for many complex control tasks.

Thanks for care

                Pavel

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Linus Torvalds]

On Sat, 11 Dec 2004, Alan Cox wrote:
>
> On Sad, 2004-12-11 at 01:23, Linus Torvalds wrote:
> > > Until the 10,000th event it actually seems to work rather happily
> > > without that change.
> > 
> > I suspect you never tried the level-triggered case.
> 
> Level triggered has never been supported.

So? You want it to lock up the machine?

Alan, what _are_ you arguing about? That "disable_irq()" is absolutely 
rquired, because:
 - not having it locks up the machine if the irq happens to be level.
 - not having it means that the "enable_irq()" that happens when the irq 
   is reported to user space is unbalanced.

> Putting a single disable_irq in doesn't change the fact it doesn't work
> because the IRQ is never re-enabled.

Did you actually test the code? Did you ever _look_ at it?

		Linus

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Alan Cox]

On Sad, 2004-12-11 at 04:21, Linus Torvalds wrote:
> Alan, what _are_ you arguing about? That "disable_irq()" is absolutely 
> rquired, because:
>  - not having it locks up the machine if the irq happens to be level.
>  - not having it means that the "enable_irq()" that happens when the irq 
>    is reported to user space is unbalanced.

That part I missed. 

> > Putting a single disable_irq in doesn't change the fact it doesn't work
> > because the IRQ is never re-enabled.
> 
> Did you actually test the code? Did you ever _look_ at it?

Yes I tested it. It worked in my test code, unfortunately the enable_irq
part of it didn't show up because the other patches in that test set
included ones for dynamically detecting interrupt routing errors and
they hid it.

Alan

ps: Pavel - the X folks played with several ideas for handling
interrupts
from user space that could be shared, forwarded to user space and
handled and it always came back to either a small kernel module or an
interpretable set of
descriptions of how to test for and mask the IRQ, and in some cases to
save
several values. Something like

struct descriptor {
	u8  type:2; /* 0 PCI cfg, 1 mem, 2 I/O */
	u8  width:2;
	u32 offset;
	u32 mask;
	u32 value;
}

struct irq_descriptor {
	char name[16];			/* For request irq */
	struct descriptor test;
	struct descriptor mask;
	struct descriptor save[4];
};

although nobody ever implemented it. This would also have been outside
of just vm86 as the main user would be X.

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Linus Torvalds]

On Sat, 11 Dec 2004, Alan Cox wrote:
> 
> ps: Pavel - the X folks played with several ideas for handling
> interrupts from user space that could be shared, forwarded to user space
> and handled and it always came back to either a small kernel module or
> an interpretable set of descriptions of how to test for and mask the
> IRQ, and in some cases to save several values.

The interpreter idea is somewhat interesting, especially if the "language"
can be actually "compiled" into some threaded format or similar. I suspect 
that a number of special devices that you don't want to maintain a 
real kernel module for could be handled that way.

However, I also suspect that such a thing would eventually explode with 
special cases and support for new features people want, to the point 
where it gets quite complex, and a kernel module might be easier after all ;)

		Linus

Re: VM86 interrupt emulation breakage and FIXes for 2.6.x kernel series

[Alan Cox]

On Sad, 2004-12-11 at 18:45, Linus Torvalds wrote:
> The interpreter idea is somewhat interesting, especially if the "language"
> can be actually "compiled" into some threaded format or similar. I suspect 
> that a number of special devices that you don't want to maintain a 
> real kernel module for could be handled that way.

The example I gave is sufficient for vblank on every video card that was
looked at (which is the main X interest for the Xsync extensions). You
also only have to turn the descriptor from "type, size" into function
pointer to get good performance .. at least I think that will be the
case

	if((op->func(op->offset) & op->mask) != op->value)
		return IRQ_NONE;

> However, I also suspect that such a thing would eventually explode with 
> special cases and support for new features people want, to the point 
> where it gets quite complex, and a kernel module might be easier after all ;)

For some devices most definitely. The simple descriptors unfortunately
will degenerate into ACPI otherwise.

Alan

[PATCH] Linux 2.6.x VM86 interrupt emulation fixes - the second round

[Pavel Pisa]

Patch solves VM86 interrupt emulation deadlock on SMP systems.
The VM86 interrupt emulation has been heavily tested and works
well on UP systems after last update, but it seems to deadlock
when we have used it on SMP/HT boxes now.
It seems, that disable_irq() cannot be called from interrupts,
because it waits until disabled interrupt handler finishes
(/kernel/irq/manage.c:synchronize_irq():while(IRQ_INPROGRESS);).
This blocks one CPU after another. Solved by use disable_irq_nosync.
There is the second problem. If IRQ source is fast, it is possible,
that interrupt is sometimes processed and re-enabled by the second
CPU, before it is disabled by the first one, but negative IRQ disable
depths are not allowed. The spinlocking and disabling IRQs over
call to disable_irq_nosync/enable_irq is the only solution found
reliable till now.

Signed-off-by: Michal Sojka <sojkam1@control.felk.cvut.cz>
Signed-off-by: Pavel Pisa <pisa@cmp.felk.cvut.cz>

Index: linux-2.6.11.5/arch/i386/kernel/vm86.c
===================================================================
--- linux-2.6.11.5.orig/arch/i386/kernel/vm86.c
+++ linux-2.6.11.5/arch/i386/kernel/vm86.c
@@ -732,12 +732,12 @@ static irqreturn_t irq_handler(int intno
 	irqbits |= irq_bit;
 	if (vm86_irqs[intno].sig)
 		send_sig(vm86_irqs[intno].sig, vm86_irqs[intno].tsk, 1);
-	spin_unlock_irqrestore(&irqbits_lock, flags);
 	/*
 	 * IRQ will be re-enabled when user asks for the irq (whether
 	 * polling or as a result of the signal)
 	 */
-	disable_irq(intno);
+	disable_irq_nosync(intno);
+	spin_unlock_irqrestore(&irqbits_lock, flags);
 	return IRQ_HANDLED;
 
 out:
@@ -769,17 +769,20 @@ static inline int get_and_reset_irq(int 
 {
 	int bit;
 	unsigned long flags;
+	int ret = 0;
 	
 	if (invalid_vm86_irq(irqnumber)) return 0;
 	if (vm86_irqs[irqnumber].tsk != current) return 0;
 	spin_lock_irqsave(&irqbits_lock, flags);	
 	bit = irqbits & (1 << irqnumber);
 	irqbits &= ~bit;
+	if (bit) {
+		enable_irq(irqnumber);
+		ret = 1;
+        }
+
 	spin_unlock_irqrestore(&irqbits_lock, flags);	
-	if (!bit)
-		return 0;
-	enable_irq(irqnumber);
-	return 1;
+	return ret;
 }