[PATCH] deal with lack of acpi prt entries gracefully

[PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

Instead of going into an infinite loop because the list isn't setup yet,
just return NULL if there are no prt entries.

Jesse


diff -Nru a/drivers/acpi/pci_irq.c b/drivers/acpi/pci_irq.c
--- a/drivers/acpi/pci_irq.c	Tue Sep  9 10:24:14 2003
+++ b/drivers/acpi/pci_irq.c	Tue Sep  9 10:24:14 2003
@@ -71,6 +71,9 @@
 
 	ACPI_FUNCTION_TRACE("acpi_pci_irq_find_prt_entry");
 
+	if (!acpi_prt.count)
+		return_PTR(NULL);
+
 	/*
 	 * Parse through all PRT entries looking for a match on the specified
 	 * PCI device's segment, bus, device, and pin (don't care about func).

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Andrew de Quincey]

On Tuesday 09 September 2003 21:13, Jesse Barnes wrote:
> Instead of going into an infinite loop because the list isn't setup yet,
> just return NULL if there are no prt entries.

Ah, this is a patch against the vanilla kernel.. This is unfortunately 
incompatible with my recent ACPI patches.

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

On Tue, Sep 09, 2003 at 09:43:58PM +0100, Andrew de Quincey wrote:
> On Tuesday 09 September 2003 21:13, Jesse Barnes wrote:
> > Instead of going into an infinite loop because the list isn't setup yet,
> > just return NULL if there are no prt entries.
> 
> Ah, this is a patch against the vanilla kernel.. This is unfortunately 
> incompatible with my recent ACPI patches.

Maybe you could include it in your patch then?  I noticed that your
patch changes some of the IRQ routing code...

Thanks,
Jesse

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Andrew de Quincey]

On Tuesday 09 September 2003 22:17, Jesse Barnes wrote:
> On Tue, Sep 09, 2003 at 09:43:58PM +0100, Andrew de Quincey wrote:
> > On Tuesday 09 September 2003 21:13, Jesse Barnes wrote:
> > > Instead of going into an infinite loop because the list isn't setup
> > > yet, just return NULL if there are no prt entries.
> >
> > Ah, this is a patch against the vanilla kernel.. This is unfortunately
> > incompatible with my recent ACPI patches.
>
> Maybe you could include it in your patch then?  I noticed that your
> patch changes some of the IRQ routing code...

I'm not sure it is still needed or not. The patch makes a lot of changes as to 
how the acpi_prt list is generated. What triggers the problem exactly, so I 
can test? 

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

On Tue, Sep 09, 2003 at 10:38:26PM +0100, Andrew de Quincey wrote:
> On Tuesday 09 September 2003 22:17, Jesse Barnes wrote:
> > On Tue, Sep 09, 2003 at 09:43:58PM +0100, Andrew de Quincey wrote:
> > > On Tuesday 09 September 2003 21:13, Jesse Barnes wrote:
> > > > Instead of going into an infinite loop because the list isn't setup
> > > > yet, just return NULL if there are no prt entries.
> > >
> > > Ah, this is a patch against the vanilla kernel.. This is unfortunately
> > > incompatible with my recent ACPI patches.
> >
> > Maybe you could include it in your patch then?  I noticed that your
> > patch changes some of the IRQ routing code...
> 
> I'm not sure it is still needed or not. The patch makes a lot of changes as to 
> how the acpi_prt list is generated. What triggers the problem exactly, so I 
> can test? 

An SGI Altix system that only supports part of the ACPI spec :).  Our
PROM currently doesn't generate any MADT entries other than CPUs (I'm
not even sure how we could add them since we use our own interrupt
controller, not an IOAPIC or IOSAPIC) and lacks an ACPI namespace, so
we're missing all sorts of stuff.

Jesse

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Andrew de Quincey]

On Tuesday 09 Sep 2003 11:01 pm, Jesse Barnes wrote:
> On Tue, Sep 09, 2003 at 10:38:26PM +0100, Andrew de Quincey wrote:
> > On Tuesday 09 September 2003 22:17, Jesse Barnes wrote:
> > > On Tue, Sep 09, 2003 at 09:43:58PM +0100, Andrew de Quincey wrote:
> > > > On Tuesday 09 September 2003 21:13, Jesse Barnes wrote:
> > > > > Instead of going into an infinite loop because the list isn't setup
> > > > > yet, just return NULL if there are no prt entries.
> > > >
> > > > Ah, this is a patch against the vanilla kernel.. This is
> > > > unfortunately incompatible with my recent ACPI patches.
> > >
> > > Maybe you could include it in your patch then?  I noticed that your
> > > patch changes some of the IRQ routing code...
> >
> > I'm not sure it is still needed or not. The patch makes a lot of changes
> > as to how the acpi_prt list is generated. What triggers the problem
> > exactly, so I can test?
>
> An SGI Altix system that only supports part of the ACPI spec :).  Our
> PROM currently doesn't generate any MADT entries other than CPUs (I'm
> not even sure how we could add them since we use our own interrupt
> controller, not an IOAPIC or IOSAPIC) and lacks an ACPI namespace, so
> we're missing all sorts of stuff.

So, exactly as your patch did, you just want it to drop back if there were no 
PCI routing entries found by ACPI... sounds sensible enough.

Can you confirm I have this right?

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

On Wed, Sep 10, 2003 at 10:30:29PM +0100, Andrew de Quincey wrote:
> So, exactly as your patch did, you just want it to drop back if there were no 
> PCI routing entries found by ACPI... sounds sensible enough.
> 
> Can you confirm I have this right?

Yep, that's it.  The code should do that, but we get there before the
list has been initialized, so we just hang.

Thanks,
Jesse

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Andrew de Quincey]

On Wednesday 10 Sep 2003 10:38 pm, Jesse Barnes wrote:
> On Wed, Sep 10, 2003 at 10:30:29PM +0100, Andrew de Quincey wrote:
> > So, exactly as your patch did, you just want it to drop back if there
> > were no PCI routing entries found by ACPI... sounds sensible enough.
> >
> > Can you confirm I have this right?
>
> Yep, that's it.  The code should do that, but we get there before the
> list has been initialized, so we just hang.

I'm not sure if this is automatically fixed or not yet.

With the new patch:

1) If ACPI fails to parse a table, it disables ACPI, and so disables any 
attempt to use ACPI for PRT routing.

2) If ACPI is enabled, and enters the function you patched, code further in 
checks if the routing tables have any entries. If not, it rejects the 
attempt.

>From your patch, I get the impression (1) is what you were patching for.. am I 
right? In that case, there shouldn't be a problem.

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

On Thu, Sep 11, 2003 at 09:40:08PM +0100, Andrew de Quincey wrote:
> On Wednesday 10 Sep 2003 10:38 pm, Jesse Barnes wrote:
> > On Wed, Sep 10, 2003 at 10:30:29PM +0100, Andrew de Quincey wrote:
> > > So, exactly as your patch did, you just want it to drop back if there
> > > were no PCI routing entries found by ACPI... sounds sensible enough.
> > >
> > > Can you confirm I have this right?
> >
> > Yep, that's it.  The code should do that, but we get there before the
> > list has been initialized, so we just hang.
> 
> I'm not sure if this is automatically fixed or not yet.
> 
> With the new patch:
> 
> 1) If ACPI fails to parse a table, it disables ACPI, and so disables any 
> attempt to use ACPI for PRT routing.

That might work, though I'll be using the ACPI namespace to drive PCI
discovery soon (hacking the PROM now).  Maybe I should add some MADT and
_PRT entries while I'm at it?  The problem is that we don't support
IOAPIC or IOSAPIC interrupt models/hw registers.

> 2) If ACPI is enabled, and enters the function you patched, code further in 
> checks if the routing tables have any entries. If not, it rejects the 
> attempt.

That would work I guess.

> From your patch, I get the impression (1) is what you were patching for.. am I 
> right? In that case, there shouldn't be a problem.

We also use ACPI tables SLIT and SRAT for memory/proximity detection,
and fill in the proper FADT fields.

Thanks,
Jesse

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Andrew de Quincey]

On Thursday 11 Sep 2003 9:53 pm, Jesse Barnes wrote:
> On Thu, Sep 11, 2003 at 09:40:08PM +0100, Andrew de Quincey wrote:
> > On Wednesday 10 Sep 2003 10:38 pm, Jesse Barnes wrote:
> > > On Wed, Sep 10, 2003 at 10:30:29PM +0100, Andrew de Quincey wrote:
> > > > So, exactly as your patch did, you just want it to drop back if there
> > > > were no PCI routing entries found by ACPI... sounds sensible enough.
> > > >
> > > > Can you confirm I have this right?
> > >
> > > Yep, that's it.  The code should do that, but we get there before the
> > > list has been initialized, so we just hang.
> >
> > I'm not sure if this is automatically fixed or not yet.
> >
> > With the new patch:
> >
> > 1) If ACPI fails to parse a table, it disables ACPI, and so disables any
> > attempt to use ACPI for PRT routing.
>
> That might work, though I'll be using the ACPI namespace to drive PCI
> discovery soon (hacking the PROM now).  Maybe I should add some MADT and
> _PRT entries while I'm at it?  The problem is that we don't support
> IOAPIC or IOSAPIC interrupt models/hw registers.

Which base architecture do you use? x86 and x86_64 ACPI now both support PIC 
based interrupt models.. as thats the only other option AFAIK (It tries 
IOAPIC first, then if that fails, it drops back to trying PIC mode).

> > 2) If ACPI is enabled, and enters the function you patched, code further
> > in checks if the routing tables have any entries. If not, it rejects the
> > attempt.
>
> That would work I guess.

Cool, well if it doesn't work, at least we know exactly what to fix.

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

On Thu, Sep 11, 2003 at 10:13:13PM +0100, Andrew de Quincey wrote:
> > That might work, though I'll be using the ACPI namespace to drive PCI
> > discovery soon (hacking the PROM now).  Maybe I should add some MADT and
> > _PRT entries while I'm at it?  The problem is that we don't support
> > IOAPIC or IOSAPIC interrupt models/hw registers.
> 
> Which base architecture do you use? x86 and x86_64 ACPI now both support PIC 
> based interrupt models.. as thats the only other option AFAIK (It tries 
> IOAPIC first, then if that fails, it drops back to trying PIC mode).

None of the above.  We have our own NUMAlink based interrupt protocol
model.

> > > 2) If ACPI is enabled, and enters the function you patched, code further
> > > in checks if the routing tables have any entries. If not, it rejects the
> > > attempt.
> >
> > That would work I guess.
> 
> Cool, well if it doesn't work, at least we know exactly what to fix.

Yeah, I found the problem pretty quickly last time, so I'm ok with
retesting once your patch goes in.

Thanks,
Jesse

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Andrew de Quincey]

On Thursday 11 Sep 2003 10:20 pm, Jesse Barnes wrote:
> On Thu, Sep 11, 2003 at 10:13:13PM +0100, Andrew de Quincey wrote:
> > > That might work, though I'll be using the ACPI namespace to drive PCI
> > > discovery soon (hacking the PROM now).  Maybe I should add some MADT
> > > and _PRT entries while I'm at it?  The problem is that we don't support
> > > IOAPIC or IOSAPIC interrupt models/hw registers.
> >
> > Which base architecture do you use? x86 and x86_64 ACPI now both support
> > PIC based interrupt models.. as thats the only other option AFAIK (It
> > tries IOAPIC first, then if that fails, it drops back to trying PIC
> > mode).
>
> None of the above.  We have our own NUMAlink based interrupt protocol
> model.

Oooer! Hmm, the existing code would probably NOT like having _PRT entries for 
a model it doesn't know about.... you could add support for it fairly easily 
though I suppose...

Re: [PATCH] deal with lack of acpi prt entries gracefully

[Jesse Barnes]

On Thu, Sep 11, 2003 at 11:00:30PM +0100, Andrew de Quincey wrote:
> > None of the above.  We have our own NUMAlink based interrupt protocol
> > model.
> 
> Oooer! Hmm, the existing code would probably NOT like having _PRT entries for 
> a model it doesn't know about.... you could add support for it fairly easily 
> though I suppose...

Yeah, that's what Andy suggested too.  I guess I have to use one of the
reserved fields and try to get the ACPI spec updated.

Thanks,
Jesse

RE: [PATCH] deal with lack of acpi prt entries gracefully

[Brown, Len]

> -----Original Message-----
> From: Jesse Barnes [mailto:jbarnes@sgi.com] 
> Sent: Thursday, September 11, 2003 6:05 PM
> To: Andrew de Quincey
> Cc: Grover, Andrew; linux-kernel@vger.kernel.org
> Subject: Re: [PATCH] deal with lack of acpi prt entries gracefully
> 
> 
> On Thu, Sep 11, 2003 at 11:00:30PM +0100, Andrew de Quincey wrote:
> > > None of the above.  We have our own NUMAlink based 
> interrupt protocol
> > > model.
> > 
> > Oooer! Hmm, the existing code would probably NOT like 
> having _PRT entries for 
> > a model it doesn't know about.... you could add support for 
> it fairly easily 
> > though I suppose...
> 
> Yeah, that's what Andy suggested too.  I guess I have to use 
> one of the
> reserved fields and try to get the ACPI spec updated.
> 
> Thanks,
> Jesse

Even if this exotic box shouldn't be running this flavor of the code,
the inifinite loop part struck me as less than bomb proof;-)
So I pulled the return on count==0 check into the ACPI patch.

Thanks,
-Len