diff mbox

powerpc: quick hack to get a functional eHEA with hardirq preemption

Message ID 20080915100406.342e027a@bull.net (mailing list archive)
State Not Applicable, archived
Headers show

Commit Message

Sebastien Dugue Sept. 15, 2008, 8:04 a.m. UTC
WARNING: HACK - HACK - HACK

  Under the RT kernel (with hardirq preemption) the eHEA driver hangs right
after booting. Fiddling with the hardirqs and softirqs priorities allows to
run a bit longer but as soon as the network gets under load, the hang
returns. After investigating, it appears that the driver is loosing interrupts.

  To make a long story short, looking at the code, it appears that the XICS
maps all its interrupts to level sensitive interrupts (I don't know if it's the
reality or if it's due to an incomplete implementation - no datasheets
available to check) and use the fasteoi processing flow.

  When entering the low level handler, level sensitive interrupts are masked,
then eio'd in interrupt context and then unmasked at the end of hardirq
processing.
That's fine as any interrupt comming in-between will still be processed since
the kernel replays those pending interrupts.

  However, it appears that the eHEA interrupts are behaving as edge sensitive
interrupts and are routed through the XICS which process those as level
sensitive using the fasteoi handler __OR__ the XICS loses interrupts when they
are masked.

  Therefore the masking done in the handler causes any interrupt happening while
in the handler to be lost.

  So this patch maps the interrupts being requested through
ibmebus_request_irq() as edge sensitive interrupts (this concerns both the eHEA
and the eHCA - only users of ibmebus_request_irq()) and changes the way edge
interrupts are processed by the fasteoi handler.

  It works for the eHEA, dunno for the eHCA.

  So, unless all the designers of the XICS & eHEA have been shot to keep it
a secret, could someone knowledgeable shed some light on this issue.

  Thanks,

  Sebastien.

Not-Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>

Comments

Jan-Bernd Themann Sept. 15, 2008, 12:17 p.m. UTC | #1
Hi,

we are a bit worried about putting this into the mainstream part of non 
real time linux.
There interrupts work perfectly fine, and it was a bit of a challenge to 
get there for all
cases / configurations / machines.

Could you try to enable these changes only for RT-Linux via a real-time 
kconfig switch?
This way we make sure we don't break the scheme for eHEA / eHCA. 

Regards,
Jan-Bernd & Christoph

Sebastien Dugue <sebastien.dugue@bull.net> wrote on 15.09.2008 10:04:06:

> 
> WARNING: HACK - HACK - HACK
> 
>   Under the RT kernel (with hardirq preemption) the eHEA driver hangs 
right
> after booting. Fiddling with the hardirqs and softirqs priorities allows 
to
> run a bit longer but as soon as the network gets under load, the hang
> returns. After investigating, it appears that the driver is loosing 
> interrupts.
> 
>   To make a long story short, looking at the code, it appears that the 
XICS
> maps all its interrupts to level sensitive interrupts (I don't know 
> if it's the
> reality or if it's due to an incomplete implementation - no datasheets
> available to check) and use the fasteoi processing flow.
> 
>   When entering the low level handler, level sensitive interrupts are 
masked,
> then eio'd in interrupt context and then unmasked at the end of hardirq
> processing.
> That's fine as any interrupt comming in-between will still be processed 
since
> the kernel replays those pending interrupts.
> 
>   However, it appears that the eHEA interrupts are behaving as edge 
sensitive
> interrupts and are routed through the XICS which process those as level
> sensitive using the fasteoi handler __OR__ the XICS loses interruptswhen 
they
> are masked.
> 
>   Therefore the masking done in the handler causes any interrupt 
> happening while
> in the handler to be lost.
> 
>   So this patch maps the interrupts being requested through
> ibmebus_request_irq() as edge sensitive interrupts (this concerns 
> both the eHEA
> and the eHCA - only users of ibmebus_request_irq()) and changes the way 
edge
> interrupts are processed by the fasteoi handler.
> 
>   It works for the eHEA, dunno for the eHCA.
> 
>   So, unless all the designers of the XICS & eHEA have been shot to keep 
it
> a secret, could someone knowledgeable shed some light on this issue.
> 
>   Thanks,
> 
>   Sebastien.
> 
> Not-Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
> ---
>  arch/powerpc/kernel/ibmebus.c |   11 ++++++++++-
>  kernel/irq/chip.c             |    5 +++--
>  kernel/irq/manage.c           |    9 ++++++---
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ibmebus.c 
b/arch/powerpc/kernel/ibmebus.c
> index 9971159..5200323 100644
> --- a/arch/powerpc/kernel/ibmebus.c
> +++ b/arch/powerpc/kernel/ibmebus.c
> @@ -41,6 +41,7 @@
>  #include <linux/kobject.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
> +#include <linux/irq.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <asm/ibmebus.h>
> @@ -213,11 +214,19 @@ int ibmebus_request_irq(u32 ist, irq_handler_t 
handler,
>           void *dev_id)
>  {
>     unsigned int irq = irq_create_mapping(NULL, ist);
> +   struct irq_desc *desc;
> +   int ret;
> 
>     if (irq == NO_IRQ)
>        return -EINVAL;
> 
> -   return request_irq(irq, handler, irq_flags, devname, dev_id);
> +   ret = request_irq(irq, handler, irq_flags, devname, dev_id);
> +
> +   desc = irq_desc + irq;
> +   desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
> +   desc->status |= IRQ_TYPE_EDGE_RISING;
> +
> +   return ret;
>  }
>  EXPORT_SYMBOL(ibmebus_request_irq);
> 
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b7b397a..6d366ca 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -430,7 +430,7 @@ handle_fasteoi_irq(unsigned int irq, struct 
> irq_desc *desc)
>     action = desc->action;
>     if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
>                     IRQ_DISABLED)))) {
> -      desc->status |= IRQ_PENDING;
> +      desc->status |= IRQ_PENDING | IRQ_MASKED;
>        if (desc->chip->mask)
>           desc->chip->mask(irq);
>        goto out;
> @@ -439,9 +439,10 @@ handle_fasteoi_irq(unsigned int irq, struct 
> irq_desc *desc)
>     desc->status |= IRQ_INPROGRESS;
>     /*
>      * In the threaded case we fall back to a mask+eoi sequence:
> +    * excepted for edge interrupts which are not masked.
>      */
>     if (redirect_hardirq(desc)) {
> -      if (desc->chip->mask)
> +      if (desc->chip->mask && !(desc->status & IRQ_TYPE_EDGE_BOTH))
>           desc->chip->mask(irq);
>        goto out;
>     }
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 3bffa20..3e39c71 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -788,9 +788,12 @@ static void do_hardirq(struct irq_desc *desc)
>        thread_simple_irq(desc);
>     else if (desc->handle_irq == handle_level_irq)
>        thread_level_irq(desc);
> -   else if (desc->handle_irq == handle_fasteoi_irq)
> -      thread_fasteoi_irq(desc);
> -   else if (desc->handle_irq == handle_edge_irq)
> +   else if (desc->handle_irq == handle_fasteoi_irq) {
> +      if (desc->status & IRQ_TYPE_EDGE_BOTH)
> +         thread_edge_irq(desc);
> +      else
> +         thread_fasteoi_irq(desc);
> +   } else if (desc->handle_irq == handle_edge_irq)
>        thread_edge_irq(desc);
>     else
>        thread_do_irq(desc);
> -- 
> 1.6.0.1.308.gede4c
>
Thomas Klein Sept. 15, 2008, 12:35 p.m. UTC | #2
Hi,

we are a bit worried about putting this into the mainstream part of non real
time linux. There interrupts work perfectly fine, and it was a bit of a
challenge to get there for all cases / configurations / machines.

Could you try to enable these changes only for RT-Linux via a real-time
kconfig switch? This way we make sure we don't break the scheme for
eHEA / eHCA.

Regards,
Jan-Bernd, Christoph


Sebastien Dugue wrote:
> WARNING: HACK - HACK - HACK
> 
>   Under the RT kernel (with hardirq preemption) the eHEA driver hangs right
> after booting. Fiddling with the hardirqs and softirqs priorities allows to
> run a bit longer but as soon as the network gets under load, the hang
> returns. After investigating, it appears that the driver is loosing interrupts.
> 
>   To make a long story short, looking at the code, it appears that the XICS
> maps all its interrupts to level sensitive interrupts (I don't know if it's the
> reality or if it's due to an incomplete implementation - no datasheets
> available to check) and use the fasteoi processing flow.
> 
>   When entering the low level handler, level sensitive interrupts are masked,
> then eio'd in interrupt context and then unmasked at the end of hardirq
> processing.
> That's fine as any interrupt comming in-between will still be processed since
> the kernel replays those pending interrupts.
> 
>   However, it appears that the eHEA interrupts are behaving as edge sensitive
> interrupts and are routed through the XICS which process those as level
> sensitive using the fasteoi handler __OR__ the XICS loses interrupts when they
> are masked.
> 
>   Therefore the masking done in the handler causes any interrupt happening while
> in the handler to be lost.
> 
>   So this patch maps the interrupts being requested through
> ibmebus_request_irq() as edge sensitive interrupts (this concerns both the eHEA
> and the eHCA - only users of ibmebus_request_irq()) and changes the way edge
> interrupts are processed by the fasteoi handler.
> 
>   It works for the eHEA, dunno for the eHCA.
> 
>   So, unless all the designers of the XICS & eHEA have been shot to keep it
> a secret, could someone knowledgeable shed some light on this issue.
> 
>   Thanks,
> 
>   Sebastien.
> 
> Not-Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
> ---
>  arch/powerpc/kernel/ibmebus.c |   11 ++++++++++-
>  kernel/irq/chip.c             |    5 +++--
>  kernel/irq/manage.c           |    9 ++++++---
>  3 files changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
> index 9971159..5200323 100644
> --- a/arch/powerpc/kernel/ibmebus.c
> +++ b/arch/powerpc/kernel/ibmebus.c
> @@ -41,6 +41,7 @@
>  #include <linux/kobject.h>
>  #include <linux/dma-mapping.h>
>  #include <linux/interrupt.h>
> +#include <linux/irq.h>
>  #include <linux/of.h>
>  #include <linux/of_platform.h>
>  #include <asm/ibmebus.h>
> @@ -213,11 +214,19 @@ int ibmebus_request_irq(u32 ist, irq_handler_t handler,
>  			void *dev_id)
>  {
>  	unsigned int irq = irq_create_mapping(NULL, ist);
> +	struct irq_desc *desc;
> +	int ret;
>  
>  	if (irq == NO_IRQ)
>  		return -EINVAL;
>  
> -	return request_irq(irq, handler, irq_flags, devname, dev_id);
> +	ret = request_irq(irq, handler, irq_flags, devname, dev_id);
> +
> +	desc = irq_desc + irq;
> +	desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
> +	desc->status |= IRQ_TYPE_EDGE_RISING;
> +
> +	return ret;
>  }
>  EXPORT_SYMBOL(ibmebus_request_irq);
>  
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b7b397a..6d366ca 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -430,7 +430,7 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
>  	action = desc->action;
>  	if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
>  						 IRQ_DISABLED)))) {
> -		desc->status |= IRQ_PENDING;
> +		desc->status |= IRQ_PENDING | IRQ_MASKED;
>  		if (desc->chip->mask)
>  			desc->chip->mask(irq);
>  		goto out;
> @@ -439,9 +439,10 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
>  	desc->status |= IRQ_INPROGRESS;
>  	/*
>  	 * In the threaded case we fall back to a mask+eoi sequence:
> +	 * excepted for edge interrupts which are not masked.
>  	 */
>  	if (redirect_hardirq(desc)) {
> -		if (desc->chip->mask)
> +		if (desc->chip->mask && !(desc->status & IRQ_TYPE_EDGE_BOTH))
>  			desc->chip->mask(irq);
>  		goto out;
>  	}
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 3bffa20..3e39c71 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -788,9 +788,12 @@ static void do_hardirq(struct irq_desc *desc)
>  		thread_simple_irq(desc);
>  	else if (desc->handle_irq == handle_level_irq)
>  		thread_level_irq(desc);
> -	else if (desc->handle_irq == handle_fasteoi_irq)
> -		thread_fasteoi_irq(desc);
> -	else if (desc->handle_irq == handle_edge_irq)
> +	else if (desc->handle_irq == handle_fasteoi_irq) {
> +		if (desc->status & IRQ_TYPE_EDGE_BOTH)
> +			thread_edge_irq(desc);
> +		else
> +			thread_fasteoi_irq(desc);
> +	} else if (desc->handle_irq == handle_edge_irq)
>  		thread_edge_irq(desc);
>  	else
>  		thread_do_irq(desc);
Sebastien Dugue Sept. 15, 2008, 1:13 p.m. UTC | #3
Hi Thomas, Jan-Bernd, Christoph,

On Mon, 15 Sep 2008 14:35:16 +0200 Thomas Klein <osstklei@de.ibm.com> wrote:

> Hi,
> 
> we are a bit worried about putting this into the mainstream part of non real
> time linux.

  Heck, I sure do not want this to be applied mainstream nor into any tree.
The sole purpose of this patch was to trigger some reaction from the people who
know the hardware and try to understand where the problem lies.

> There interrupts work perfectly fine, and it was a bit of a
> challenge to get there for all cases / configurations / machines.

  Agreed, but the fact that it fails with hardirq preemption leads me to
believe (without any more knowledge about the harware) that there might be
something amiss with this driver (or the code concerning the XICS)
nevertheless.

> 
> Could you try to enable these changes only for RT-Linux via a real-time
> kconfig switch?

  Nope, this is just a quick hack that allows me to have a functional eHEA under
the rt kernel. I want to understand what the problem is:

  - Is the eHEA really delivering level interrupts to the XICS?

  - Is the XICS loosing interrupts when they are masked?

  - ...?

> This way we make sure we don't break the scheme for
> eHEA / eHCA.

  Sure, I do not want to break anything, quite the opposite in fact ;-)


  Thanks,

  Sebastien.

> 
> Regards,
> Jan-Bernd, Christoph
> 
> 
> Sebastien Dugue wrote:
> > WARNING: HACK - HACK - HACK
> > 
> >   Under the RT kernel (with hardirq preemption) the eHEA driver hangs right
> > after booting. Fiddling with the hardirqs and softirqs priorities allows to
> > run a bit longer but as soon as the network gets under load, the hang
> > returns. After investigating, it appears that the driver is loosing interrupts.
> > 
> >   To make a long story short, looking at the code, it appears that the XICS
> > maps all its interrupts to level sensitive interrupts (I don't know if it's the
> > reality or if it's due to an incomplete implementation - no datasheets
> > available to check) and use the fasteoi processing flow.
> > 
> >   When entering the low level handler, level sensitive interrupts are masked,
> > then eio'd in interrupt context and then unmasked at the end of hardirq
> > processing.
> > That's fine as any interrupt comming in-between will still be processed since
> > the kernel replays those pending interrupts.
> > 
> >   However, it appears that the eHEA interrupts are behaving as edge sensitive
> > interrupts and are routed through the XICS which process those as level
> > sensitive using the fasteoi handler __OR__ the XICS loses interrupts when they
> > are masked.
> > 
> >   Therefore the masking done in the handler causes any interrupt happening while
> > in the handler to be lost.
> > 
> >   So this patch maps the interrupts being requested through
> > ibmebus_request_irq() as edge sensitive interrupts (this concerns both the eHEA
> > and the eHCA - only users of ibmebus_request_irq()) and changes the way edge
> > interrupts are processed by the fasteoi handler.
> > 
> >   It works for the eHEA, dunno for the eHCA.
> > 
> >   So, unless all the designers of the XICS & eHEA have been shot to keep it
> > a secret, could someone knowledgeable shed some light on this issue.
> > 
> >   Thanks,
> > 
> >   Sebastien.
> > 
> > Not-Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
> > ---
> >  arch/powerpc/kernel/ibmebus.c |   11 ++++++++++-
> >  kernel/irq/chip.c             |    5 +++--
> >  kernel/irq/manage.c           |    9 ++++++---
> >  3 files changed, 19 insertions(+), 6 deletions(-)
> > 
> > diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
> > index 9971159..5200323 100644
> > --- a/arch/powerpc/kernel/ibmebus.c
> > +++ b/arch/powerpc/kernel/ibmebus.c
> > @@ -41,6 +41,7 @@
> >  #include <linux/kobject.h>
> >  #include <linux/dma-mapping.h>
> >  #include <linux/interrupt.h>
> > +#include <linux/irq.h>
> >  #include <linux/of.h>
> >  #include <linux/of_platform.h>
> >  #include <asm/ibmebus.h>
> > @@ -213,11 +214,19 @@ int ibmebus_request_irq(u32 ist, irq_handler_t handler,
> >  			void *dev_id)
> >  {
> >  	unsigned int irq = irq_create_mapping(NULL, ist);
> > +	struct irq_desc *desc;
> > +	int ret;
> >  
> >  	if (irq == NO_IRQ)
> >  		return -EINVAL;
> >  
> > -	return request_irq(irq, handler, irq_flags, devname, dev_id);
> > +	ret = request_irq(irq, handler, irq_flags, devname, dev_id);
> > +
> > +	desc = irq_desc + irq;
> > +	desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
> > +	desc->status |= IRQ_TYPE_EDGE_RISING;
> > +
> > +	return ret;
> >  }
> >  EXPORT_SYMBOL(ibmebus_request_irq);
> >  
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index b7b397a..6d366ca 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -430,7 +430,7 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
> >  	action = desc->action;
> >  	if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
> >  						 IRQ_DISABLED)))) {
> > -		desc->status |= IRQ_PENDING;
> > +		desc->status |= IRQ_PENDING | IRQ_MASKED;
> >  		if (desc->chip->mask)
> >  			desc->chip->mask(irq);
> >  		goto out;
> > @@ -439,9 +439,10 @@ handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
> >  	desc->status |= IRQ_INPROGRESS;
> >  	/*
> >  	 * In the threaded case we fall back to a mask+eoi sequence:
> > +	 * excepted for edge interrupts which are not masked.
> >  	 */
> >  	if (redirect_hardirq(desc)) {
> > -		if (desc->chip->mask)
> > +		if (desc->chip->mask && !(desc->status & IRQ_TYPE_EDGE_BOTH))
> >  			desc->chip->mask(irq);
> >  		goto out;
> >  	}
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 3bffa20..3e39c71 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -788,9 +788,12 @@ static void do_hardirq(struct irq_desc *desc)
> >  		thread_simple_irq(desc);
> >  	else if (desc->handle_irq == handle_level_irq)
> >  		thread_level_irq(desc);
> > -	else if (desc->handle_irq == handle_fasteoi_irq)
> > -		thread_fasteoi_irq(desc);
> > -	else if (desc->handle_irq == handle_edge_irq)
> > +	else if (desc->handle_irq == handle_fasteoi_irq) {
> > +		if (desc->status & IRQ_TYPE_EDGE_BOTH)
> > +			thread_edge_irq(desc);
> > +		else
> > +			thread_fasteoi_irq(desc);
> > +	} else if (desc->handle_irq == handle_edge_irq)
> >  		thread_edge_irq(desc);
> >  	else
> >  		thread_do_irq(desc);
> 
>
Anton Vorontsov Sept. 16, 2008, 11:59 a.m. UTC | #4
On Mon, Sep 15, 2008 at 03:13:32PM +0200, Sebastien Dugue wrote:
[...]
> > we are a bit worried about putting this into the mainstream part of non real
> > time linux.
> 
>   Heck, I sure do not want this to be applied mainstream nor into any tree.
> The sole purpose of this patch was to trigger some reaction from the people who
> know the hardware and try to understand where the problem lies.
> 
> > There interrupts work perfectly fine, and it was a bit of a
> > challenge to get there for all cases / configurations / machines.
> 
>   Agreed, but the fact that it fails with hardirq preemption leads me to
> believe (without any more knowledge about the harware) that there might be
> something amiss with this driver (or the code concerning the XICS)
> nevertheless.
> 
> > 
> > Could you try to enable these changes only for RT-Linux via a real-time
> > kconfig switch?
> 
>   Nope, this is just a quick hack that allows me to have a functional eHEA under
> the rt kernel. I want to understand what the problem is:
> 
>   - Is the eHEA really delivering level interrupts to the XICS?
> 
>   - Is the XICS loosing interrupts when they are masked?

There is a known bug in the -rt kernels, the bug causes handlers
to lose edge interrupts.

See this patch:

http://lkml.org/lkml/2008/6/30/372

>   - ...?
Sebastien Dugue Sept. 16, 2008, 12:22 p.m. UTC | #5
Hi Anton,

On Tue, 16 Sep 2008 15:59:47 +0400 Anton Vorontsov <avorontsov@ru.mvista.com> wrote:

> On Mon, Sep 15, 2008 at 03:13:32PM +0200, Sebastien Dugue wrote:
> [...]
> > > we are a bit worried about putting this into the mainstream part of non real
> > > time linux.
> > 
> >   Heck, I sure do not want this to be applied mainstream nor into any tree.
> > The sole purpose of this patch was to trigger some reaction from the people who
> > know the hardware and try to understand where the problem lies.
> > 
> > > There interrupts work perfectly fine, and it was a bit of a
> > > challenge to get there for all cases / configurations / machines.
> > 
> >   Agreed, but the fact that it fails with hardirq preemption leads me to
> > believe (without any more knowledge about the harware) that there might be
> > something amiss with this driver (or the code concerning the XICS)
> > nevertheless.
> > 
> > > 
> > > Could you try to enable these changes only for RT-Linux via a real-time
> > > kconfig switch?
> > 
> >   Nope, this is just a quick hack that allows me to have a functional eHEA under
> > the rt kernel. I want to understand what the problem is:
> > 
> >   - Is the eHEA really delivering level interrupts to the XICS?
> > 
> >   - Is the XICS loosing interrupts when they are masked?
> 
> There is a known bug in the -rt kernels, the bug causes handlers
> to lose edge interrupts.
> 
> See this patch:
> 
> http://lkml.org/lkml/2008/6/30/372

  Yes, I've been following that thread back then and my hack is based on your
patch. So yes, it seems to be the same problem and it lies in the way -rt handles
the fasteoi flow.

  But looking at the comments from the XICS code, it seems that its wired for
level only interrupts. Therefore without any more specs, it's still not clear to
me that there's not a bug with the way the xics handles eHEA interrupts.

  Are the eHEA interrupts really level interrupts? If so why do they get lost
when masked?

  I just found that not masking that particular interrupt in the fasteoi path
makes the thing work!

  Thanks,

  Sebastien.

> 
> >   - ...?
> 
> -- 
> Anton Vorontsov
> email: cbouatmailru@gmail.com
> irc://irc.freenode.net/bd2
> --
> To unsubscribe from this list: send the line "unsubscribe linux-rt-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
Christoph Raisch Sept. 18, 2008, 7:53 a.m. UTC | #6
Sebastien Dugue <sebastien.dugue@bull.net> wrote on 15.09.2008 10:04:06:
> [PATCH HACK] powerpc: quick hack to get a functional eHEA with
> hardirq preemption
>
> Sebastien Dugue
>
> to:
>
> 15.09.2008 10:07
>
> Cc:
>
> linux-ppc, linux-kernel, Linux-rt, netdev, Jan-Bernd Themann, Thomas
> Q Klein, Christoph Raisch, jean-pierre.dion, gilles.carry, tinytim
>
>
> WARNING: HACK - HACK - HACK
> Not-Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
> ---
> --- a/arch/powerpc/kernel/ibmebus.c
> +++ b/arch/powerpc/kernel/ibmebus.c
> @@ -41,6 +41,7 @@
> -   return request_irq(irq, handler, irq_flags, devname, dev_id);
> +   ret = request_irq(irq, handler, irq_flags, devname, dev_id);
> +
> +   desc = irq_desc + irq;
> +   desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
> +   desc->status |= IRQ_TYPE_EDGE_RISING;
> +
> +   return ret;
This looks a bit like a set_irq_type call.
Don't know if this is fully implemented for xics though...
>  }
>  EXPORT_SYMBOL(ibmebus_request_irq);
>
> diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> index b7b397a..6d366ca 100644
> --- a/kernel/irq/chip.c
> +++ b/kernel/irq/chip.c
> @@ -430,7 +430,7 @@ handle_fasteoi_irq(unsigned int irq, struct
> irq_desc *desc)
>     action = desc->action;
>     if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
>                     IRQ_DISABLED)))) {
> -      desc->status |= IRQ_PENDING;
> +      desc->status |= IRQ_PENDING | IRQ_MASKED;
>        if (desc->chip->mask)
>           desc->chip->mask(irq);
>        goto out;
> @@ -439,9 +439,10 @@ handle_fasteoi_irq(unsigned int irq, struct
> irq_desc *desc)
>     desc->status |= IRQ_INPROGRESS;
>     /*
>      * In the threaded case we fall back to a mask+eoi sequence:
> +    * excepted for edge interrupts which are not masked.
>      */
>     if (redirect_hardirq(desc)) {
> -      if (desc->chip->mask)
> +      if (desc->chip->mask && !(desc->status & IRQ_TYPE_EDGE_BOTH))
>           desc->chip->mask(irq);
>        goto out;
>     }
> diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> index 3bffa20..3e39c71 100644
> --- a/kernel/irq/manage.c
> +++ b/kernel/irq/manage.c
> @@ -788,9 +788,12 @@ static void do_hardirq(struct irq_desc *desc)
>        thread_simple_irq(desc);
>     else if (desc->handle_irq == handle_level_irq)
>        thread_level_irq(desc);
> -   else if (desc->handle_irq == handle_fasteoi_irq)
> -      thread_fasteoi_irq(desc);
> -   else if (desc->handle_irq == handle_edge_irq)
> +   else if (desc->handle_irq == handle_fasteoi_irq) {
> +      if (desc->status & IRQ_TYPE_EDGE_BOTH)
> +         thread_edge_irq(desc);
> +      else
> +         thread_fasteoi_irq(desc);
> +   } else if (desc->handle_irq == handle_edge_irq)
>        thread_edge_irq(desc);
>     else
>        thread_do_irq(desc);
> --
> 1.6.0.1.308.gede4c
>
According to the specs at some point in the system the HEA IRQs have a edge
characteristic.
But since PCI-E edge and level can both be forwarded through a message
interface
(HEA is not PCI-E, it's only connected to the same internal bus, where the
PHB resides)

Anybody from the xics experts want to comment on this?



Gruss / Regards
Christoph R.  & Jan-Bernd
Sebastien Dugue Sept. 18, 2008, 9:27 a.m. UTC | #7
On Thu, 18 Sep 2008 09:53:54 +0200 Christoph Raisch <RAISCH@de.ibm.com> wrote:

> 
> Sebastien Dugue <sebastien.dugue@bull.net> wrote on 15.09.2008 10:04:06:
> > [PATCH HACK] powerpc: quick hack to get a functional eHEA with
> > hardirq preemption
> >
> > Sebastien Dugue
> >
> > to:
> >
> > 15.09.2008 10:07
> >
> > Cc:
> >
> > linux-ppc, linux-kernel, Linux-rt, netdev, Jan-Bernd Themann, Thomas
> > Q Klein, Christoph Raisch, jean-pierre.dion, gilles.carry, tinytim
> >
> >
> > WARNING: HACK - HACK - HACK
> > Not-Signed-off-by: Sebastien Dugue <sebastien.dugue@bull.net>
> > ---
> > --- a/arch/powerpc/kernel/ibmebus.c
> > +++ b/arch/powerpc/kernel/ibmebus.c
> > @@ -41,6 +41,7 @@
> > -   return request_irq(irq, handler, irq_flags, devname, dev_id);
> > +   ret = request_irq(irq, handler, irq_flags, devname, dev_id);
> > +
> > +   desc = irq_desc + irq;
> > +   desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
> > +   desc->status |= IRQ_TYPE_EDGE_RISING;
> > +
> > +   return ret;
> This looks a bit like a set_irq_type call.

  Yep.

> Don't know if this is fully implemented for xics though...

  No, the xics does not implement a set_type() method.

> >  }
> >  EXPORT_SYMBOL(ibmebus_request_irq);
> >
> > diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
> > index b7b397a..6d366ca 100644
> > --- a/kernel/irq/chip.c
> > +++ b/kernel/irq/chip.c
> > @@ -430,7 +430,7 @@ handle_fasteoi_irq(unsigned int irq, struct
> > irq_desc *desc)
> >     action = desc->action;
> >     if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
> >                     IRQ_DISABLED)))) {
> > -      desc->status |= IRQ_PENDING;
> > +      desc->status |= IRQ_PENDING | IRQ_MASKED;
> >        if (desc->chip->mask)
> >           desc->chip->mask(irq);
> >        goto out;
> > @@ -439,9 +439,10 @@ handle_fasteoi_irq(unsigned int irq, struct
> > irq_desc *desc)
> >     desc->status |= IRQ_INPROGRESS;
> >     /*
> >      * In the threaded case we fall back to a mask+eoi sequence:
> > +    * excepted for edge interrupts which are not masked.
> >      */
> >     if (redirect_hardirq(desc)) {
> > -      if (desc->chip->mask)
> > +      if (desc->chip->mask && !(desc->status & IRQ_TYPE_EDGE_BOTH))
> >           desc->chip->mask(irq);
> >        goto out;
> >     }
> > diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
> > index 3bffa20..3e39c71 100644
> > --- a/kernel/irq/manage.c
> > +++ b/kernel/irq/manage.c
> > @@ -788,9 +788,12 @@ static void do_hardirq(struct irq_desc *desc)
> >        thread_simple_irq(desc);
> >     else if (desc->handle_irq == handle_level_irq)
> >        thread_level_irq(desc);
> > -   else if (desc->handle_irq == handle_fasteoi_irq)
> > -      thread_fasteoi_irq(desc);
> > -   else if (desc->handle_irq == handle_edge_irq)
> > +   else if (desc->handle_irq == handle_fasteoi_irq) {
> > +      if (desc->status & IRQ_TYPE_EDGE_BOTH)
> > +         thread_edge_irq(desc);
> > +      else
> > +         thread_fasteoi_irq(desc);
> > +   } else if (desc->handle_irq == handle_edge_irq)
> >        thread_edge_irq(desc);
> >     else
> >        thread_do_irq(desc);
> > --
> > 1.6.0.1.308.gede4c
> >
> According to the specs at some point in the system the HEA IRQs have a edge
> characteristic.
> But since PCI-E edge and level can both be forwarded through a message
> interface
> (HEA is not PCI-E, it's only connected to the same internal bus, where the
> PHB resides)

  Good to know, the problem here seems to be that the xics is using the
fasteoi flow handler, which under unconditionally masks the irq. Will have to
dig in the genirq stuff a bit more to understand what the differences are
between -rt and mainline.

> 
> Anybody from the xics experts want to comment on this?

  It would be really interresting to know if the eHCA exhibits the same
problem under -rt as it's the only other user of the ibmebus.
Unfortunately I don't have the hardware to test.

  Thanks,

  Sebastien.


> 
> 
> 
> Gruss / Regards
> Christoph R.  & Jan-Bernd
> 
>
Christoph Raisch Sept. 18, 2008, 10:42 a.m. UTC | #8
Sebastien Dugue <sebastien.dugue@bull.net> wrote on 18.09.2008 11:27:13:

>
>   It would be really interresting to know if the eHCA exhibits the same
> problem under -rt as it's the only other user of the ibmebus.
> Unfortunately I don't have the hardware to test.
>

eHCA is very close from the interrupt generation and handling perspective,
so yes, could be an issue there as well.


Gruss / Regards
Christoph Raisch
Sebastien Dugue Sept. 18, 2008, 12:31 p.m. UTC | #9
On Thu, 18 Sep 2008 12:42:05 +0200 Christoph Raisch <RAISCH@de.ibm.com> wrote:

> 
> Sebastien Dugue <sebastien.dugue@bull.net> wrote on 18.09.2008 11:27:13:
> 
> >
> >   It would be really interresting to know if the eHCA exhibits the same
> > problem under -rt as it's the only other user of the ibmebus.
> > Unfortunately I don't have the hardware to test.
> >
> 
> eHCA is very close from the interrupt generation and handling perspective,
> so yes, could be an issue there as well.

  That's what I was speculating.

  Thanks,

  Sebastien.

> 
> 
> Gruss / Regards
> Christoph Raisch
> 
> 
>
diff mbox

Patch

diff --git a/arch/powerpc/kernel/ibmebus.c b/arch/powerpc/kernel/ibmebus.c
index 9971159..5200323 100644
--- a/arch/powerpc/kernel/ibmebus.c
+++ b/arch/powerpc/kernel/ibmebus.c
@@ -41,6 +41,7 @@ 
 #include <linux/kobject.h>
 #include <linux/dma-mapping.h>
 #include <linux/interrupt.h>
+#include <linux/irq.h>
 #include <linux/of.h>
 #include <linux/of_platform.h>
 #include <asm/ibmebus.h>
@@ -213,11 +214,19 @@  int ibmebus_request_irq(u32 ist, irq_handler_t handler,
 			void *dev_id)
 {
 	unsigned int irq = irq_create_mapping(NULL, ist);
+	struct irq_desc *desc;
+	int ret;
 
 	if (irq == NO_IRQ)
 		return -EINVAL;
 
-	return request_irq(irq, handler, irq_flags, devname, dev_id);
+	ret = request_irq(irq, handler, irq_flags, devname, dev_id);
+
+	desc = irq_desc + irq;
+	desc->status &= ~(IRQ_TYPE_SENSE_MASK | IRQ_LEVEL);
+	desc->status |= IRQ_TYPE_EDGE_RISING;
+
+	return ret;
 }
 EXPORT_SYMBOL(ibmebus_request_irq);
 
diff --git a/kernel/irq/chip.c b/kernel/irq/chip.c
index b7b397a..6d366ca 100644
--- a/kernel/irq/chip.c
+++ b/kernel/irq/chip.c
@@ -430,7 +430,7 @@  handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
 	action = desc->action;
 	if (unlikely(!action || (desc->status & (IRQ_INPROGRESS |
 						 IRQ_DISABLED)))) {
-		desc->status |= IRQ_PENDING;
+		desc->status |= IRQ_PENDING | IRQ_MASKED;
 		if (desc->chip->mask)
 			desc->chip->mask(irq);
 		goto out;
@@ -439,9 +439,10 @@  handle_fasteoi_irq(unsigned int irq, struct irq_desc *desc)
 	desc->status |= IRQ_INPROGRESS;
 	/*
 	 * In the threaded case we fall back to a mask+eoi sequence:
+	 * excepted for edge interrupts which are not masked.
 	 */
 	if (redirect_hardirq(desc)) {
-		if (desc->chip->mask)
+		if (desc->chip->mask && !(desc->status & IRQ_TYPE_EDGE_BOTH))
 			desc->chip->mask(irq);
 		goto out;
 	}
diff --git a/kernel/irq/manage.c b/kernel/irq/manage.c
index 3bffa20..3e39c71 100644
--- a/kernel/irq/manage.c
+++ b/kernel/irq/manage.c
@@ -788,9 +788,12 @@  static void do_hardirq(struct irq_desc *desc)
 		thread_simple_irq(desc);
 	else if (desc->handle_irq == handle_level_irq)
 		thread_level_irq(desc);
-	else if (desc->handle_irq == handle_fasteoi_irq)
-		thread_fasteoi_irq(desc);
-	else if (desc->handle_irq == handle_edge_irq)
+	else if (desc->handle_irq == handle_fasteoi_irq) {
+		if (desc->status & IRQ_TYPE_EDGE_BOTH)
+			thread_edge_irq(desc);
+		else
+			thread_fasteoi_irq(desc);
+	} else if (desc->handle_irq == handle_edge_irq)
 		thread_edge_irq(desc);
 	else
 		thread_do_irq(desc);