Hi,
On Fri, Sep 14, 2012 at 10:31:05AM +0200, Christian Ehrhardt wrote:
> On Thu, Sep 13, 2012 at 11:00:12PM +0200, Mark Kettenis wrote:
> > > However, there might be a different solution:
> > > We could track attached busses (not just host bridge busses but all
> > > busses)
> > > in the pci code (or via MD code and hooks but I think that would be a
> > > generic thing) and do nothing in pciattach if a bus is discovered for
> > > the second time.
> > >
> > > With this MD code could use whatever methods it wants to detect otherwise
> > > undiscovered host bridges. Possible methods might be magic numbers, hints
> > > from ACPI or even a linear scan of all possible bus numbers.
> > >
> > > Does this sound like a possible solution?
> >
> > It actually got me thinking about proper bus number resource
> > accounting again. Something that's needed for properly hot-plugging
> > pci busses. Below is a diff that does this.
> >
> > With that code, your acpi code can just try to grab the bus number
> > using extent_alloc_region() and only attach a bus if that succeeds.
>
> Nice idea!
>
> However, I have some doubts about the implementation. In particular
> you don't pass sc_busex down through bridge devices. This is ok as long
> as all busses that we _discover_ downstream of that bridge are in fact
> _physically_ dowstream of that bridge, too. In this case the bus range
> of the bridge (PPB_BUSINFO_SECONDARY..PPB_BUSINFO_SUBORDINATE) covers
> all busses.
>
> However, host bridges that are handled e.g. in arch/i386/pci/pchb.c
> can appear downstream of another bridge but use bus numbers that are
> outside of the upstream bridge's bus number range.
>
> The easiest solution would probably be, to make the pcibus_ex an MI
> thing, i.e. define and allocate it globally in dev/pci/pci.c.
Below is the suggestion for an updated patch that follows this strategy.
I'm not entirely confident about the i386/pchb.c part of the patch, though.
People that own such hardware would have to verify that all their hardware
is discovered properly.
This code tries to catch theoretical cases where we have two ACPI
discovered host bridges and the one listed _first_ in ACPI is discoverable
from the second one via PCI mechanisms (but not vice versa). In this case
the host bridge code and not the ACPI code must detect that the second
host bridge has already been attached. However, this assumes that host
bridges never use a bus numbers that are within the bus range of their
upstream bridge.
regards Christian
Index: arch/i386/i386/mainbus.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/i386/mainbus.c,v
retrieving revision 1.48
diff -u -r1.48 mainbus.c
--- arch/i386/i386/mainbus.c 3 Nov 2010 10:15:23 -0000 1.48
+++ arch/i386/i386/mainbus.c 14 Sep 2012 09:48:34 -0000
@@ -244,6 +244,9 @@
mba.mba_pba.pba_domain = pci_ndomains++;
mba.mba_pba.pba_bus = 0;
config_found(self, &mba.mba_pba, mainbus_print);
+#if NACPI > 0
+ acpi_pciroots_attach(self, &mba.mba_pba, mainbus_print);
+#endif
}
#endif
Index: arch/i386/pci/pchb.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/pci/pchb.c,v
retrieving revision 1.85
diff -u -r1.85 pchb.c
--- arch/i386/pci/pchb.c 31 Aug 2010 17:13:46 -0000 1.85
+++ arch/i386/pci/pchb.c 14 Sep 2012 09:48:34 -0000
@@ -403,6 +421,8 @@
if (doattach == 0)
return;
+ if (extent_alloc_region(pcibus_ex, pbnum, 1, EX_NOWAIT) != 0)
+ return;
bzero(&pba, sizeof(pba));
pba.pba_busname = "pci";
@@ -505,6 +525,8 @@
pba.pba_domain = pa->pa_domain;
pba.pba_bus = AMD64HT_LDT_SEC_BUS_NUM(bus);
pba.pba_pc = pa->pa_pc;
- config_found(self, &pba, pchb_print);
+ if (extent_alloc_region(pcibus_ex, pba.pba_bus, 1, EX_NOWAIT)
+ == 0)
+ config_found(self, &pba, pchb_print);
}
}
Index: arch/i386/pci/pci_machdep.c
===================================================================
RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.c,v
retrieving revision 1.68
diff -u -r1.68 pci_machdep.c
--- arch/i386/pci/pci_machdep.c 4 Dec 2011 20:08:09 -0000 1.68
+++ arch/i386/pci/pci_machdep.c 14 Sep 2012 09:48:34 -0000
@@ -921,6 +921,11 @@
extent_alloc_region(pcimem_ex, IOM_BEGIN, IOM_SIZE,
EX_CONFLICTOK | EX_NOWAIT);
}
+
+ if (pcibus_ex == NULL) {
+ pcibus_ex = extent_create("pcibus", 0, 0xff, M_DEVBUF,
+ NULL, 0, EX_NOWAIT);
+ }
}
#include "acpi.h"
Index: arch/i386/pci/pci_machdep.h
===================================================================
RCS file: /cvs/src/sys/arch/i386/pci/pci_machdep.h,v
retrieving revision 1.23
diff -u -r1.23 pci_machdep.h
--- arch/i386/pci/pci_machdep.h 10 Oct 2011 19:42:36 -0000 1.23
+++ arch/i386/pci/pci_machdep.h 14 Sep 2012 09:48:34 -0000
@@ -82,6 +82,7 @@
extern struct extent *pciio_ex;
extern struct extent *pcimem_ex;
+extern struct extent *pcibus_ex;
void pci_init_extents(void);
/*
Index: dev/acpi/acpi.c
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpi.c,v
retrieving revision 1.238
diff -u -r1.238 acpi.c
--- dev/acpi/acpi.c 13 Jul 2012 11:51:41 -0000 1.238
+++ dev/acpi/acpi.c 14 Sep 2012 09:48:36 -0000
@@ -392,6 +392,8 @@
TAILQ_HEAD(, acpi_pci) acpi_pcidevs =
TAILQ_HEAD_INITIALIZER(acpi_pcidevs);
+TAILQ_HEAD(, acpi_pci) acpi_pcirootdevs =
+ TAILQ_HEAD_INITIALIZER(acpi_pcirootdevs);
int acpi_getpci(struct aml_node *node, void *arg);
int acpi_getminbus(union acpi_resource *crs, void *arg);
@@ -480,6 +482,7 @@
node->pci = pci;
dnprintf(10, "found PCI root: %s %d\n",
aml_nodename(node), pci->bus);
+ TAILQ_INSERT_TAIL(&acpi_pcirootdevs, pci, next);
}
aml_freevalue(&res);
return 0;
@@ -548,6 +551,24 @@
dev->dv_xname, aml_nodename(pdev->node));
pdev->device = dev;
}
+ }
+}
+
+void
+acpi_pciroots_attach(struct device *dev, void *aux, cfprint_t pr)
+{
+ struct acpi_pci *pdev;
+ struct pcibus_attach_args *pba = aux;
+
+ if (pcibus_ex == NULL) {
+ printf("cannot attach ACPI discovered busses\n");
+ return;
+ }
+ TAILQ_FOREACH(pdev, &acpi_pcirootdevs, next) {
+ if (extent_alloc_region(pcibus_ex, pdev->bus, 1, EX_NOWAIT))
+ continue;
+ pba->pba_bus = pdev->bus;
+ config_found(dev, pba, pr);
}
}
Index: dev/acpi/acpivar.h
===================================================================
RCS file: /cvs/src/sys/dev/acpi/acpivar.h,v
retrieving revision 1.71
diff -u -r1.71 acpivar.h
--- dev/acpi/acpivar.h 15 Apr 2011 17:34:51 -0000 1.71
+++ dev/acpi/acpivar.h 14 Sep 2012 09:48:36 -0000
@@ -322,6 +322,8 @@
void acpi_powerdown_task(void *, int);
void acpi_sleep_task(void *, int);
+void acpi_pciroots_attach(struct device *, void *, cfprint_t);
+
#endif
#endif /* !_ACPI_WAKECODE */
Index: dev/pci/pci.c
===================================================================
RCS file: /cvs/src/sys/dev/pci/pci.c,v
retrieving revision 1.94
diff -u -r1.94 pci.c
--- dev/pci/pci.c 10 Oct 2011 19:42:37 -0000 1.94
+++ dev/pci/pci.c 14 Sep 2012 09:48:36 -0000
@@ -86,6 +86,8 @@
struct proc *pci_vga_proc;
struct pci_softc *pci_vga_pci;
+struct extent *pcibus_ex;
+
pcitag_t pci_vga_tag;
int pci_vga_count;
@@ -182,7 +184,13 @@
sc->sc_maxndevs = pci_bus_maxdevs(pba->pba_pc, pba->pba_bus);
sc->sc_intrswiz = pba->pba_intrswiz;
sc->sc_intrtag = pba->pba_intrtag;
+
+ /* Reserve our own bus number. */
+ if (pcibus_ex)
+ extent_alloc_region(pcibus_ex, sc->sc_bus, 1, EX_NOWAIT);
+
pci_enumerate_bus(sc, pci_reserve_resources, NULL);
+
pci_enumerate_bus(sc, pci_count_vga, NULL);
if (pci_enumerate_bus(sc, pci_primary_vga, NULL))
pci_vga_pci = sc;
@@ -746,12 +754,16 @@
{
pci_chipset_tag_t pc = pa->pa_pc;
pcitag_t tag = pa->pa_tag;
- pcireg_t bhlc, blr, type;
+ pcireg_t bhlc, blr, type, bir;
bus_addr_t base, limit;
bus_size_t size;
int reg, reg_start, reg_end;
+ int bus, dev, func;
+ int sec, sub;
int flags;
+ pci_decompose_tag(pc, tag, &bus, &dev, &func);
+
bhlc = pci_conf_read(pc, tag, PCI_BHLC_REG);
switch (PCI_HDRTYPE_TYPE(bhlc)) {
case 0:
@@ -792,8 +804,8 @@
#endif
if (pa->pa_memex && extent_alloc_region(pa->pa_memex,
base, size, EX_NOWAIT)) {
- printf("mem address conflict 0x%x/0x%x\n",
- base, size);
+ printf("%d:%d:%d: mem address conflict
0x%x/0x%x\n",
+ bus, dev, func, base, size);
pci_conf_write(pc, tag, reg, 0);
if (type & PCI_MAPREG_MEM_TYPE_64BIT)
pci_conf_write(pc, tag, reg + 4, 0);
@@ -802,8 +814,8 @@
case PCI_MAPREG_TYPE_IO:
if (pa->pa_ioex && extent_alloc_region(pa->pa_ioex,
base, size, EX_NOWAIT)) {
- printf("io address conflict 0x%x/0x%x\n",
- base, size);
+ printf("%d:%d:%d: io address conflict
0x%x/0x%x\n",
+ bus, dev, func, base, size);
pci_conf_write(pc, tag, reg, 0);
}
break;
@@ -829,8 +841,8 @@
size = 0;
if (pa->pa_ioex && base > 0 && size > 0) {
if (extent_alloc_region(pa->pa_ioex, base, size, EX_NOWAIT)) {
- printf("bridge io address conflict 0x%x/0x%x\n",
- base, size);
+ printf("%d:%d:%d: bridge io address conflict
0x%x/0x%x\n",
+ bus, dev, func, base, size);
blr &= 0xffff0000;
blr |= 0x000000f0;
pci_conf_write(pc, tag, PPB_REG_IOSTATUS, blr);
@@ -847,8 +859,8 @@
size = 0;
if (pa->pa_memex && base > 0 && size > 0) {
if (extent_alloc_region(pa->pa_memex, base, size, EX_NOWAIT)) {
- printf("bridge mem address conflict 0x%x/0x%x\n",
- base, size);
+ printf("%d:%d:%d: bridge mem address conflict
0x%x/0x%x\n",
+ bus, dev, func, base, size);
pci_conf_write(pc, tag, PPB_REG_MEM, 0x0000fff0);
}
}
@@ -863,17 +875,24 @@
size = 0;
if (pa->pa_pmemex && base > 0 && size > 0) {
if (extent_alloc_region(pa->pa_pmemex, base, size, EX_NOWAIT)) {
- printf("bridge mem address conflict 0x%x/0x%x\n",
- base, size);
+ printf("%d:%d:%d: bridge mem address conflict
0x%x/0x%x\n",
+ bus, dev, func, base, size);
pci_conf_write(pc, tag, PPB_REG_PREFMEM, 0x0000fff0);
}
} else if (pa->pa_memex && base > 0 && size > 0) {
if (extent_alloc_region(pa->pa_memex, base, size, EX_NOWAIT)) {
- printf("bridge mem address conflict 0x%x/0x%x\n",
- base, size);
+ printf("%d:%d:%d: bridge mem address conflict
0x%x/0x%x\n",
+ bus, dev, func, base, size);
pci_conf_write(pc, tag, PPB_REG_PREFMEM, 0x0000fff0);
}
}
+
+ /* Figure out the bus range handled by the bridge. */
+ bir = pci_conf_read(pc, tag, PPB_REG_BUSINFO);
+ sec = PPB_BUSINFO_SECONDARY(bir);
+ sub = PPB_BUSINFO_SUBORDINATE(bir);
+ if (pcibus_ex && sub >= sec)
+ extent_alloc_region(pcibus_ex, sec, sub - sec + 1, EX_NOWAIT);
return (0);
}
Index: dev/pci/pcivar.h
===================================================================
RCS file: /cvs/src/sys/dev/pci/pcivar.h,v
retrieving revision 1.66
diff -u -r1.66 pcivar.h
--- dev/pci/pcivar.h 30 May 2011 19:09:46 -0000 1.66
+++ dev/pci/pcivar.h 14 Sep 2012 09:48:36 -0000
@@ -197,6 +197,7 @@
extern int pci_ndomains;
extern int pci_dopm;
+extern struct extent *pcibus_ex;
/*
* Locators devices that attach to 'pcibus', as specified to config.
