Date: Wed, 12 Jan 2005 15:06:50 -0500 From: John Baldwin <jhb@FreeBSD.org> To: freebsd-current@FreeBSD.org Cc: Nate Lawson <nate@root.org> Subject: Re: Intel SHG2 and ACPI problems. Message-ID: <200501121506.50867.jhb@FreeBSD.org> In-Reply-To: <200501121442.02702.jhb@FreeBSD.org> References: <20050111202452.GK795@darkness.comp.waw.pl> <20050111224007.GL795@darkness.comp.waw.pl> <200501121442.02702.jhb@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wednesday 12 January 2005 02:42 pm, John Baldwin wrote: > On Tuesday 11 January 2005 05:40 pm, Pawel Jakub Dawidek wrote: > > On Tue, Jan 11, 2005 at 02:16:06PM -0800, Nate Lawson wrote: > > +> Pawel Jakub Dawidek wrote: > > +> >I had problems with ACPI on Intel SHG2 motherboard. > > +> >I made a patch with works for me just fine. Could you, Nate, verify > > it +> >and commit if it is ok. > > +> >If you need some more info, just ask. > > +> > > > +> > http://people.freebsd.org/~pjd/patches/acpi_pci_link.c.patch > > +> > > +> John mentioned that it appears the root problem is that _CRS is > > failing +> for you. Can you send a dmesg from a broken boot (without > > your patch)? > > > > Here you go: > > > > http://people.freebsd.org/~pjd/misc/boot-v1.txt > > Ok, this is a rather large patch as allowing for a b0rked _CRS required a > good bit of work. I've only compile tested it and haven't run tested it so > far, so beware. Note that it does include fixes for some bugs related to > ExtIRQ routing (I wrote the irq to the wrong resource structure :( ) and to > parsing the buffer we handed to _SRS (end pointer was wrong so I probably > only ever parsed the first resource, which is the common case, so this > probably didn't affect anyone). Gee, patch would help: --- //depot/vendor/freebsd/src/sys/dev/acpica/acpi_pci_link.c 2004/12/27 05:45:32 +++ //depot/user/jhb/acpipci/dev/acpica/acpi_pci_link.c 2005/01/12 19:31:01 @@ -77,6 +77,7 @@ * DMA Channel 3 * * The XXX is because I'm not sure if this is a valid assumption to make. + * Further reading of the spec is advised before this hits CVS. */ /* States during DPF processing. */ @@ -88,7 +89,9 @@ struct acpi_pci_link_softc { int pl_num_links; + int pl_crs_bad; struct link *pl_links; + device_t pl_dev; }; struct link { @@ -302,7 +305,13 @@ KASSERT(req->link_index < req->sc->pl_num_links, ("%s: array boundary violation", __func__)); link = &req->sc->pl_links[req->link_index]; + if (link->l_res_index == -1) { + KASSERT(req->sc->pl_crs_bad, + ("res_index should be set")); + link->l_res_index = req->res_index; + } req->link_index++; + req->res_index++; /* * Stash a copy of the resource for later use when doing @@ -334,6 +343,14 @@ link->l_isa_irq = FALSE; } break; + default: + if (req->in_dpf == DPF_IGNORE) + break; + if (req->sc->pl_crs_bad) + device_printf(req->sc->pl_dev, + "Warning: possible resource %d will be lost during _SRS\n", + req->res_index); + req->res_index++; } return (AE_OK); } @@ -396,21 +413,35 @@ int i; sc = device_get_softc(dev); + sc->pl_dev = dev; ACPI_SERIAL_BEGIN(pci_link); /* * Count the number of current resources so we know how big of - * a link array to allocate. + * a link array to allocate. On some systems, _CRS is broken, + * so for those systems try to derive the count from _PRS instead. */ creq.in_dpf = DPF_OUTSIDE; creq.count = 0; status = AcpiWalkResources(acpi_get_handle(dev), "_CRS", acpi_count_irq_resources, &creq); - if (ACPI_FAILURE(status)) - return (ENXIO); + sc->pl_crs_bad = ACPI_FAILURE(status); + if (sc->pl_crs_bad) { + creq.in_dpf = DPF_OUTSIDE; + creq.count = 0; + status = AcpiWalkResources(acpi_get_handle(dev), "_PRS", + acpi_count_irq_resources, &creq); + if (ACPI_FAILURE(status)) { + device_printf(dev, + "Unable to parse _CRS or _PRS: %s\n", + AcpiFormatException(status)); + ACPI_SERIAL_END(pci_link); + return (ENXIO); + } + } + sc->pl_num_links = creq.count; if (creq.count == 0) return (0); - sc->pl_num_links = creq.count; sc->pl_links = malloc(sizeof(struct link) * sc->pl_num_links, M_PCI_LINK, M_WAITOK | M_ZERO); @@ -420,22 +451,41 @@ sc->pl_links[i].l_bios_irq = PCI_INVALID_IRQ; sc->pl_links[i].l_sc = sc; sc->pl_links[i].l_isa_irq = FALSE; + sc->pl_links[i].l_res_index = -1; + } + + /* Try to read the current settings from _CRS if it is valid. */ + if (!sc->pl_crs_bad) { + rreq.in_dpf = DPF_OUTSIDE; + rreq.link_index = 0; + rreq.res_index = 0; + rreq.sc = sc; + status = AcpiWalkResources(acpi_get_handle(dev), "_CRS", + link_add_crs, &rreq); + if (ACPI_FAILURE(status)) { + device_printf(dev, "Unable to parse _CRS: %s\n", + AcpiFormatException(status)); + goto fail; + } } + + /* + * Try to read the possible settings from _PRS. Note that if the + * _CRS is toast, we depend on having a working _PRS. However, if + * _CRS works, then it is ok for _PRS to be missing. + */ rreq.in_dpf = DPF_OUTSIDE; rreq.link_index = 0; rreq.res_index = 0; rreq.sc = sc; - status = AcpiWalkResources(acpi_get_handle(dev), "_CRS", - link_add_crs, &rreq); - if (ACPI_FAILURE(status)) - goto fail; - rreq.in_dpf = DPF_OUTSIDE; - rreq.link_index = 0; - rreq.res_index = 0; status = AcpiWalkResources(acpi_get_handle(dev), "_PRS", link_add_prs, &rreq); - if (ACPI_FAILURE(status) && status != AE_NOT_FOUND) + if (ACPI_FAILURE(status) && + (status != AE_NOT_FOUND || sc->pl_crs_bad)) { + device_printf(dev, "Unable to parse _PRS: %s\n", + AcpiFormatException(status)); goto fail; + } if (bootverbose) { device_printf(dev, "Links after initial probe:\n"); acpi_pci_link_dump(sc); @@ -589,33 +639,31 @@ } static ACPI_STATUS -acpi_pci_link_route_irqs(device_t dev) +acpi_pci_link_srs_from_crs(struct acpi_pci_link_softc *sc, ACPI_BUFFER *srsbuf) { - struct acpi_pci_link_softc *sc; ACPI_RESOURCE *resource, *end, newres, *resptr; - ACPI_BUFFER crsbuf, srsbuf; + ACPI_BUFFER crsbuf; ACPI_STATUS status; struct link *link; int i, in_dpf; /* Fetch the _CRS. */ ACPI_SERIAL_ASSERT(pci_link); - sc = device_get_softc(dev); crsbuf.Pointer = NULL; crsbuf.Length = ACPI_ALLOCATE_BUFFER; - status = AcpiGetCurrentResources(acpi_get_handle(dev), &crsbuf); + status = AcpiGetCurrentResources(acpi_get_handle(sc->pl_dev), &crsbuf); if (ACPI_SUCCESS(status) && crsbuf.Pointer == NULL) status = AE_NO_MEMORY; if (ACPI_FAILURE(status)) { if (bootverbose) - device_printf(dev, + device_printf(sc->pl_dev, "Unable to fetch current resources: %s\n", AcpiFormatException(status)); return (status); } /* Fill in IRQ resources via link structures. */ - srsbuf.Pointer = NULL; + srsbuf->Pointer = NULL; link = sc->pl_links; i = 0; in_dpf = DPF_OUTSIDE; @@ -668,10 +716,10 @@ resptr = &newres; resptr->Data.ExtendedIrq.NumberOfInterrupts = 1; if (PCI_INTERRUPT_VALID(link->l_irq)) - resource->Data.ExtendedIrq.Interrupts[0] = + resptr->Data.ExtendedIrq.Interrupts[0] = link->l_irq; else - resource->Data.ExtendedIrq.Interrupts[0] = 0; + resptr->Data.ExtendedIrq.Interrupts[0] = 0; link++; i++; break; @@ -679,13 +727,13 @@ resptr = resource; } if (resptr != NULL) { - status = acpi_AppendBufferResource(&srsbuf, resptr); + status = acpi_AppendBufferResource(srsbuf, resptr); if (ACPI_FAILURE(status)) { - device_printf(dev, - "Unable to build reousrces: %s\n", + device_printf(sc->pl_dev, + "Unable to build resources: %s\n", AcpiFormatException(status)); - if (srsbuf.Pointer != NULL) - AcpiOsFree(srsbuf.Pointer); + if (srsbuf->Pointer != NULL) + AcpiOsFree(srsbuf->Pointer); AcpiOsFree(crsbuf.Pointer); return (status); } @@ -696,17 +744,88 @@ if (resource >= end) break; } + AcpiOsFree(crsbuf.Pointer); + return (AE_OK); +} + +static ACPI_STATUS +acpi_pci_link_srs_from_links(struct acpi_pci_link_softc *sc, + ACPI_BUFFER *srsbuf) +{ + ACPI_RESOURCE newres; + ACPI_STATUS status; + struct link *link; + int i; + + /* Start off with an empty buffer. */ + srsbuf->Pointer = NULL; + link = sc->pl_links; + for (i = 0; i < sc->pl_num_links; i++) { + + /* Add a new IRQ resource from each link. */ + link = &sc->pl_links[i]; + newres = link->l_prs_template; + if (newres.Id == ACPI_RSTYPE_IRQ) { + + /* Build an IRQ resource. */ + newres.Data.Irq.NumberOfInterrupts = 1; + if (PCI_INTERRUPT_VALID(link->l_irq)) { + KASSERT(link->l_irq < NUM_ISA_INTERRUPTS, + ("%s: can't put non-ISA IRQ %d in legacy IRQ resource type", + __func__, link->l_irq)); + newres.Data.Irq.Interrupts[0] = link->l_irq; + } else + newres.Data.Irq.Interrupts[0] = 0; + } else { + + /* Build an ExtIRQ resuorce. */ + newres.Data.ExtendedIrq.NumberOfInterrupts = 1; + if (PCI_INTERRUPT_VALID(link->l_irq)) + newres.Data.ExtendedIrq.Interrupts[0] = + link->l_irq; + else + newres.Data.ExtendedIrq.Interrupts[0] = 0; + } + + /* Add the new resource to the end of the _SRS buffer. */ + status = acpi_AppendBufferResource(srsbuf, &newres); + if (ACPI_FAILURE(status)) { + device_printf(sc->pl_dev, + "Unable to build resources: %s\n", + AcpiFormatException(status)); + if (srsbuf->Pointer != NULL) + AcpiOsFree(srsbuf->Pointer); + return (status); + } + } + return (AE_OK); +} + +static ACPI_STATUS +acpi_pci_link_route_irqs(device_t dev) +{ + struct acpi_pci_link_softc *sc; + ACPI_RESOURCE *resource, *end; + ACPI_BUFFER srsbuf; + ACPI_STATUS status; + struct link *link; + int i; + ACPI_SERIAL_ASSERT(pci_link); + sc = device_get_softc(dev); + if (sc->pl_crs_bad) + status = acpi_pci_link_srs_from_links(sc, &srsbuf); + else + status = acpi_pci_link_srs_from_crs(sc, &srsbuf); + /* Write out new resources via _SRS. */ status = AcpiSetCurrentResources(acpi_get_handle(dev), &srsbuf); if (ACPI_FAILURE(status)) { device_printf(dev, "Unable to route IRQs: %s\n", AcpiFormatException(status)); - AcpiOsFree(crsbuf.Pointer); AcpiOsFree(srsbuf.Pointer); return (status); } - AcpiOsFree(crsbuf.Pointer); /* * Perform acpi_config_intr() on each IRQ resource if it was just @@ -715,6 +834,7 @@ link = sc->pl_links; i = 0; resource = (ACPI_RESOURCE *)srsbuf.Pointer; + end = (ACPI_RESOURCE *)((char *)srsbuf.Pointer + srsbuf.Length); for (;;) { if (resource->Id == ACPI_RSTYPE_END_TAG) break; --- //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/11 21:44:38 @@ -98,8 +98,7 @@ /* Lookup the associated handle and device. */ pcib = (device_t)arg; - if (ACPI_FAILURE(AcpiGetHandle(acpi_get_handle(pcib), entry->Source, - &handle))) + if (ACPI_FAILURE(AcpiGetHandle(ACPI_ROOT_OBJECT, entry->Source, &handle))) return; child = acpi_get_device(handle); if (child == NULL) --- //depot/vendor/freebsd/src/sys/dev/acpica/acpi_resource.c 2004/12/27 05:40:30 +++ //depot/user/jhb/acpipci/dev/acpica/acpi_resource.c 2004/12/31 18:57:45 @@ -439,7 +439,11 @@ "unimplemented Address64 resource\n")); break; case ACPI_RSTYPE_EXT_IRQ: - /* XXX special handling? */ + if (res->Data.ExtendedIrq.ProducerConsumer != ACPI_CONSUMER) { + ACPI_DEBUG_PRINT((ACPI_DB_RESOURCES, + "ignored ExtIRQ producer\n")); + break; + } set->set_irq(dev, context,res->Data.ExtendedIrq.Interrupts, res->Data.ExtendedIrq.NumberOfInterrupts, res->Data.ExtendedIrq.EdgeLevel, --- //depot/vendor/freebsd/src/sys/i386/i386/mptable.c 2005/01/07 18:45:35 +++ //depot/user/jhb/acpipci/i386/i386/mptable.c 2005/01/12 18:06:08 @@ -635,23 +635,38 @@ mptable_parse_io_int(int_entry_ptr intr) { void *ioapic; - u_int pin; + u_int pin, apic_id; + apic_id = intr->dst_apic_id; if (intr->dst_apic_id == 0xff) { - printf("MPTable: Ignoring global interrupt entry for pin %d\n", - intr->dst_apic_int); - return; + /* + * An APIC ID of 0xff means that the interrupt is connected + * to the specified pin on all I/O APICs in the system. If + * there is only one I/O APIC, then use that APIC to route + * the interrupts. If there is more than one I/O APIC, then + * punt. + */ + if (mptable_nioapics == 1) { + apic_id = 0; + while (ioapics[apic_id] == NULL) + apic_id++; + } else { + printf( + "MPTable: Ignoring global interrupt entry for pin %d\n", + intr->dst_apic_int); + return; + } } - if (intr->dst_apic_id >= NAPICID) { + if (apic_id >= NAPICID) { printf("MPTable: Ignoring interrupt entry for ioapic%d\n", intr->dst_apic_id); return; } - ioapic = ioapics[intr->dst_apic_id]; + ioapic = ioapics[apic_id]; if (ioapic == NULL) { printf( "MPTable: Ignoring interrupt entry for missing ioapic%d\n", - intr->dst_apic_id); + apic_id); return; } pin = intr->dst_apic_int; --- //depot/vendor/freebsd/src/sys/i386/pci/pci_pir.c 2005/01/06 22:21:32 +++ //depot/user/jhb/acpipci/i386/pci/pci_pir.c 2005/01/07 20:04:48 @@ -324,22 +324,50 @@ pin = intpin - entry->pe_intpin; pci_link = pci_pir_find_link(intpin->link); irq = pci_pir_search_irq(entry->pe_bus, entry->pe_device, pin); - if (irq == PCI_INVALID_IRQ) + if (irq == PCI_INVALID_IRQ || irq == pci_link->pl_irq) return; - if (pci_pir_valid_irq(pci_link, irq)) { - if (pci_link->pl_irq == PCI_INVALID_IRQ) { - pci_link->pl_irq = irq; - pci_link->pl_routed = 1; - } else if (pci_link->pl_irq != irq) + + /* + * If we don't have an IRQ for this link yet, then we trust the + * BIOS, even if it seems invalid from the $PIR entries. + */ + if (pci_link->pl_irq == PCI_INVALID_IRQ) { + if (!pci_pir_valid_irq(pci_link, irq)) printf( - "$PIR: BIOS IRQ %d for %d.%d.INT%c does not match link %#x irq %d\n", + "$PIR: Using invalid BIOS IRQ %d from %d.%d.INT%c is for link %#x\n", irq, entry->pe_bus, entry->pe_device, pin + 'A', - pci_link->pl_id, pci_link->pl_irq); - } else + pci_link->pl_id); + pci_link->pl_irq = irq; + pci_link->pl_routed = 1; + return; + } + + /* + * We have an IRQ and it doesn't match the current IRQ for this + * link. If the new IRQ is invalid, then warn about it and ignore + * it. If the old IRQ is invalid and the new IRQ is valid, then + * prefer the new IRQ instead. If both IRQs are valid, then just + * use the first one. Note that if we ever get into this situation + * we are having to guess which setting the BIOS actually routed. + * Perhaps we should just give up instead. + */ + if (!pci_pir_valid_irq(pci_link, irq)) { printf( "$PIR: BIOS IRQ %d for %d.%d.INT%c is not valid for link %#x\n", irq, entry->pe_bus, entry->pe_device, pin + 'A', pci_link->pl_id); + } else if (!pci_pir_valid_irq(pci_link, pci_link->pl_irq)) { + printf( +"$PIR: Preferring valid BIOS IRQ %d from %d.%d.INT%c for link %#x to IRQ %d\n", + irq, entry->pe_bus, entry->pe_device, pin + 'A', + pci_link->pl_id, pci_link->pl_irq); + pci_link->pl_irq = irq; + pci_link->pl_routed = 1; + } else + printf( + "$PIR: BIOS IRQ %d for %d.%d.INT%c does not match link %#x irq %d\n", + irq, entry->pe_bus, entry->pe_device, pin + 'A', + pci_link->pl_id, pci_link->pl_irq); } /* @@ -386,9 +414,9 @@ } /* - * Allow the user to override the IRQ for a given link device as - * long as the override is valid or is 255 or 0 to clear a preset - * IRQ. + * Allow the user to override the IRQ for a given link device. We + * allow invalid IRQs to be specified but warn about them. An IRQ + * of 255 or 0 clears any preset IRQ. */ i = 0; TAILQ_FOREACH(pci_link, &pci_links, pl_links) { @@ -398,12 +426,14 @@ continue; if (irq == 0) irq = PCI_INVALID_IRQ; - if (irq == PCI_INVALID_IRQ || - pci_pir_valid_irq(pci_link, irq)) { - pci_link->pl_routed = 0; - pci_link->pl_irq = irq; - i = 1; - } + if (irq != PCI_INVALID_IRQ && + !pci_pir_valid_irq(pci_link, irq) && bootverbose) + printf( + "$PIR: Warning, IRQ %d for link %#x is not listed as valid\n", + irq, pci_link->pl_id); + pci_link->pl_routed = 0; + pci_link->pl_irq = irq; + i = 1; } if (bootverbose && i) { printf("$PIR: Links after tunable overrides:\n"); -- John Baldwin <jhb@FreeBSD.org> <>< http://www.FreeBSD.org/~jhb/ "Power Users Use the Power to Serve" = http://www.FreeBSD.org
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200501121506.50867.jhb>