page fault panic in device_get_softc/acpi_pcib_route_interrupt

[John Baldwin]

On Wednesday 05 January 2005 06:02 pm, Nate Lawson wrote:
> John Baldwin wrote:
> > On Sunday 02 January 2005 07:35 pm, Nate Lawson wrote:
> >>We already associate handles and devices in
> >>sys/dev/acpica/acpi.c:acpi_probe_child() before probing anything.  See
> >>the AcpiAttachData() step.  I don't think that's the problem.
> >
> > I do because he passes a null device_t pointer in as an argument to a
> > function.  The calling code is:
> >
> >     /*
> >      * We have to find the source device (PCI interrupt link device).
> >      */
> >     if (ACPI_FAILURE(AcpiGetHandle(ACPI_ROOT_OBJECT, prt->Source,
> > &lnkdev))) { device_printf(pcib, "couldn't find PCI interrupt link device
> > %s\n", prt->Source);
> >     interrupt = acpi_pci_link_route_interrupt(acpi_get_device(lnkdev),
> > 	prt->SourceIndex);
> >
> > And Pawel's trace shows that the first argument to
> > acpi_pci_link_route_interrupt() is NULL.
>
> What's the value of prt->Source?  If it's not a valid reference to a
> link device (i.e. \_SB.PCIx.LNKx), then trying to get a device_t from it
> would yield NULL.  For instance, if it points to \_SB, you'll get a
> valid handle from AcpiGetHandle but that handle obviously has no
> associated device_t.
>
> Additionally, I see you're using the root handle ACPI_ROOT_OBJECT as the
> base for lookup.  If the reference is relative (doesn't start with \),
> this won't work.  You should be using the handle of the parent of _PRT
> (the PCI bus handle) as the root of the lookup.  Commonly, this will be
> something like a \_SB.PCI0 string.
>
> This would fix this scenario:
> \_SB.PCI0
>      _PRT (Source = LNKA)
>      LNKA
>      LNKB

Ok, that might be it.  I'll work up a patch to use the relative roots instead.  
In fact, the patch is very simple.  It already used relative lookups when 
force-attaching the link devices during attach.  Pawel, the change is this:

--- //depot/vendor/freebsd/src/sys/dev/acpica/acpi_pcib.c	2004/12/27 05:40:30
+++ //depot/user/jhb/acpipci/dev/acpica/acpi_pcib.c	2005/01/06 18:40:54
@@ -249,7 +249,8 @@
     /*
      * We have to find the source device (PCI interrupt link device).
      */
-    if (ACPI_FAILURE(AcpiGetHandle(ACPI_ROOT_OBJECT, prt->Source, &lnkdev))) 
{
+    if (ACPI_FAILURE(AcpiGetHandle(acpi_get_handle(pcib), prt->Source,
+	&lnkdev))) {
 	device_printf(pcib, "couldn't find PCI interrupt link device %s\n",
 	    prt->Source);
 	goto out;

> Also, I'm not sure if you picked up the size issue with the _PRT struct
> supplied by ACPI-CA.  Since Source is a variable-length string, if you
> copy the struct you get from AcpiGetRoutingTable (or whatever), you only
> get the first 4 bytes, non-null terminated, of the string.
>
> typedef struct acpi_pci_routing_table
> {
>      UINT32                      Length;
>      UINT32                      Pin;
>      ACPI_INTEGER                Address;
>      UINT32                      SourceIndex;
>      char                        Source[4];
> } ACPI_PCI_ROUTING_TABLE;
>
> Note that Source above is not 4 bytes, it's variable-length.  That's why
> I copied it to a different field in the old acpi_pci_link PRT struct.

I don't ever store the Source anywhere, I just use it to lookup handles, so I 
don't have to worry about the size change.

-- 
John Baldwin <jhb at FreeBSD.org>  <><  http://www.FreeBSD.org/~jhb/
"Power Users Use the Power to Serve"  =  http://www.FreeBSD.org