Discussion:
pci_domain_nr vs. /sys/devices
Benjamin Herrenschmidt
2003-06-11 14:30:42 UTC
Permalink
The new pci_domain_nr() is good for adding the PCI domain number to
the /sys/devices/pciN/* names, but I think that's not the proper
representation. It should really be

/sys/devices/pci-domainN/pciN/*

So we can pave the way for when we'll stop play bus number tricks and
actually have overlapping PCI bus numbers between domains. (I don't plan
to do that immediately because that would break userland & /proc/bus/pci
backward compatiblity)

What do you think ?

Ben.
Matthew Wilcox
2003-06-11 14:48:01 UTC
Permalink
Post by Benjamin Herrenschmidt
The new pci_domain_nr() is good for adding the PCI domain number to
the /sys/devices/pciN/* names, but I think that's not the proper
representation. It should really be
/sys/devices/pci-domainN/pciN/*
So we can pave the way for when we'll stop play bus number tricks and
actually have overlapping PCI bus numbers between domains. (I don't plan
to do that immediately because that would break userland & /proc/bus/pci
backward compatiblity)
What do you think ?
I don't think sysfs works like that (please correct me if I've
misunderstood, mochel..)

Look in /sys/bus/pci/devices/ There you have all the PCI devices
lumped together in one place, and we obviously need the domain number
in the name. I don't know where the 0 on the end of /sys/devices/pci0/
comes from, but if we could, I wouldn't say no to:

/sys/devices/pciDDDD/DDDD:BB:SS.F
or
/sys/devices/pciDDDD:BB/DDDD:BB:SS.F
(Domain,Bus,Slot,Func)

I don't think the extra level of hierarchy in your suggestion is necessary
or particularly desirable.
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Benjamin Herrenschmidt
2003-06-11 15:06:20 UTC
Permalink
Post by Matthew Wilcox
I don't think sysfs works like that (please correct me if I've
misunderstood, mochel..)
Looks like nobody really understands it then ;)
Post by Matthew Wilcox
Look in /sys/bus/pci/devices/ There you have all the PCI devices
lumped together in one place, and we obviously need the domain number
in the name. I don't know where the 0 on the end of /sys/devices/pci0/
Nah, you are mixing up /sys/bus/* which is a flat list of busses in the
machine, with /sys/devices/* which is the hierarchical device tree.
Post by Matthew Wilcox
I don't think the extra level of hierarchy in your suggestion is necessary
or particularly desirable.
It's probably not, then it's a matter of properly renaming the pciN entries
in /sys/devices to be /sys/devices/pciDD:NN where DD is the domain number
and NN is the first bus on this domain, or just pciDD (though I like having
the bus number there as well)

Ben.
Matthew Wilcox
2003-06-11 15:12:38 UTC
Permalink
Post by Benjamin Herrenschmidt
Looks like nobody really understands it then ;)
Well, it keeps changing ;-)
Post by Benjamin Herrenschmidt
Post by Matthew Wilcox
Look in /sys/bus/pci/devices/ There you have all the PCI devices
lumped together in one place, and we obviously need the domain number
in the name. I don't know where the 0 on the end of /sys/devices/pci0/
Nah, you are mixing up /sys/bus/* which is a flat list of busses in the
machine, with /sys/devices/* which is the hierarchical device tree.
I'm not mixing them up, I'm just saying that the domain number has to be
part of the leafname.
Post by Benjamin Herrenschmidt
It's probably not, then it's a matter of properly renaming the pciN entries
in /sys/devices to be /sys/devices/pciDD:NN where DD is the domain number
and NN is the first bus on this domain, or just pciDD (though I like having
the bus number there as well)
Yep. Can you find where this happens, because it's not in pci-sysfs.c
where one might logically expect it to be ...
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Russell King
2003-06-11 15:40:40 UTC
Permalink
Post by Matthew Wilcox
Look in /sys/bus/pci/devices/ There you have all the PCI devices
lumped together in one place, and we obviously need the domain number
in the name. I don't know where the 0 on the end of /sys/devices/pci0/
See pci_alloc_primary_bus_parented() in drivers/pci/probe.c. The '0'
is the bus number passed in. So, the names include the pci bus numbers
of the root buses.
--
Russell King (***@arm.linux.org.uk) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
Benjamin Herrenschmidt
2003-06-11 16:00:52 UTC
Permalink
Post by Russell King
See pci_alloc_primary_bus_parented() in drivers/pci/probe.c. The '0'
is the bus number passed in. So, the names include the pci bus numbers
of the root buses.
This is the right solution imho, yes. Adding more indirection with
pci-domain isn't useful.

Now, we should also fix pci_setup_device to make this naming
generic to the entire kernel don't you think ? This won't
affect /proc/bus/pci as it doesn't use the slot_name field
in pci_dev, but at least it will make naming consistent.

(That also mean increasing slot_name size in pci.h)

Ben.
Anton Blanchard
2003-06-17 04:52:07 UTC
Permalink
Post by Benjamin Herrenschmidt
Now, we should also fix pci_setup_device to make this naming
generic to the entire kernel don't you think ? This won't
affect /proc/bus/pci as it doesn't use the slot_name field
in pci_dev, but at least it will make naming consistent.
(That also mean increasing slot_name size in pci.h)
Agreed. I did that in my patch since its important to be able to
uniquely identify a device:

PCI: Enabling device: (0000:21:01.0), cmd 143
PCI: Enabling bus mastering for device 0000:21:01.0
e100: selftest OK.
...
Patrick Mochel
2003-06-11 17:03:26 UTC
Permalink
Post by Matthew Wilcox
Post by Benjamin Herrenschmidt
/sys/devices/pci-domainN/pciN/*
So we can pave the way for when we'll stop play bus number tricks and
actually have overlapping PCI bus numbers between domains. (I don't plan
to do that immediately because that would break userland & /proc/bus/pci
backward compatiblity)
What do you think ?
I don't think sysfs works like that (please correct me if I've
misunderstood, mochel..)
We can do that, in two different ways. As PCI domains are discovered,
devices or kobjects can be registered that represent their existence.
Depending on what their parent is, they can be added anywhere into the
hierarchy.

It might be good to do this anyway, since it will give us the ability to
keep a list of all PCI domains in the system, assuming that's desired. :)
Post by Matthew Wilcox
Look in /sys/bus/pci/devices/ There you have all the PCI devices
lumped together in one place, and we obviously need the domain number
in the name. I don't know where the 0 on the end of /sys/devices/pci0/
/sys/devices/pciDDDD/DDDD:BB:SS.F
or
/sys/devices/pciDDDD:BB/DDDD:BB:SS.F
(Domain,Bus,Slot,Func)
We could do that (as well as what is described above). However, it's
important to note that this exposes a limitation of the driver model. We
have a flat name space in /sys/bus/<bus>/devices/, which is very handy,
and nearly free, since each bus maintains a list of all devices anyway.
However, since it's a flat listing, the names must be unique across all
instances of that bus.

Fine, but the names are currently taken directly from dev->bus_id, meaning
that the local directories for the devices must also be unique across the
entire bus namespace. This is certainly not true, and something that Linus
has complained about before. The names in the local directories only need
to be unique in the local namespace.

Unfortunately, the symlinks are created in the core, and the core doesn't
know about bus/device organization. A possilbe solution would be to
represent bus instances in the core, and have the symlink names created
from a concatenation of the bus name (e.g. DD:BB) and the local device ID
(SS.F). This would reduce pressure on bus_id size, and make things easier
to read..


-pat
Anton Blanchard
2003-06-17 04:49:48 UTC
Permalink
Post by Matthew Wilcox
Look in /sys/bus/pci/devices/ There you have all the PCI devices
lumped together in one place, and we obviously need the domain number
in the name. I don't know where the 0 on the end of /sys/devices/pci0/
/sys/devices/pciDDDD/DDDD:BB:SS.F
or
/sys/devices/pciDDDD:BB/DDDD:BB:SS.F
(Domain,Bus,Slot,Func)
I like it. I think we do need the bus number in the top level since we
could have multiple host bridges on the same domain. I put a quick patch
together, it lays things out as such:

/sys/devices/pci0002:00/0002:00:02.2

It also adds the domain to /proc/pci (are there userspace tools that
parse this directly?):

Domain 2, Bus 0, device 2, function 2:

And only creates /proc/bus/pci entries for the first domain. I was going
to extend it one level to encode the domain but I now think we should just
move that functionality into sysfs and be done with it. Willy, you had a
patch that exposed BARS etc in sysfs didnt you? X and lspci etc will need
updating to match, but they are currently broken.

I chose to add the domain into dev->slot_name since its needed for matching
kernel messages to drivers. Im wondering if we should make this conditional
on pci domain support since it does add some noise for those who couldnt
care less about domains.

Finally there was some shuffling required to make pci_bus_exists work
(passing in a pci_bus *, ->sysdata and ->number must be initialised
before calling it). There are some uses of pci_bus_exists in x86 that
will need updating.

Thoungts?

Anton

===== drivers/pci/probe.c 1.43 vs edited =====
--- 1.43/drivers/pci/probe.c Mon Jun 9 02:36:59 2003
+++ edited/drivers/pci/probe.c Tue Jun 17 10:29:08 2003
@@ -400,8 +400,8 @@
{
u32 class;

- sprintf(dev->slot_name, "%02x:%02x.%d", dev->bus->number,
- PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+ sprintf(dev->slot_name, "%04x:%02x:%02x.%d", pci_domain_nr(dev->bus),
+ dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
sprintf(dev->dev.name, "PCI device %04x:%04x",
dev->vendor, dev->device);

@@ -529,8 +529,7 @@
pci_name_device(dev);

/* now put in global tree */
- sprintf(dev->dev.bus_id, "%04x:%s", pci_domain_nr(bus),
- dev->slot_name);
+ strcpy(dev->dev.bus_id,dev->slot_name);
dev->dev.dma_mask = &dev->dma_mask;

return dev;
@@ -632,56 +636,60 @@
return max;
}

-int __devinit pci_bus_exists(const struct list_head *list, int nr)
+int __devinit pci_bus_exists(const struct list_head *list, struct pci_bus *bus)
{
- const struct pci_bus *b;
+ struct pci_bus *b;

list_for_each_entry(b, list, node) {
- if (b->number == nr || pci_bus_exists(&b->children, nr))
+ if (((b->number == bus->number) &&
+ pci_domain_nr(b) == pci_domain_nr(bus)) ||
+ pci_bus_exists(&b->children, bus))
return 1;
}
return 0;
}

-static struct pci_bus * __devinit pci_alloc_primary_bus_parented(struct device *parent, int bus)
+static struct pci_bus * __devinit pci_alloc_primary_bus_parented(struct device *parent, int bus, void *sysdata)
{
struct pci_bus *b;

- if (pci_bus_exists(&pci_root_buses, bus)) {
+ b = pci_alloc_bus();
+ if (!b)
+ return NULL;
+
+ b->sysdata = sysdata;
+ b->number = b->secondary = bus;
+ b->resource[0] = &ioport_resource;
+ b->resource[1] = &iomem_resource;
+
+ if (pci_bus_exists(&pci_root_buses, b)) {
/* If we already got to this bus through a different bridge, ignore it */
DBG("PCI: Bus %02x already known\n", bus);
+ kfree(b);
return NULL;
}

- b = pci_alloc_bus();
- if (!b)
- return NULL;
-
b->dev = kmalloc(sizeof(*(b->dev)),GFP_KERNEL);
if (!b->dev){
kfree(b);
return NULL;
}
-
+
list_add_tail(&b->node, &pci_root_buses);

memset(b->dev,0,sizeof(*(b->dev)));
- sprintf(b->dev->bus_id,"pci%d",bus);
+ sprintf(b->dev->bus_id, "pci%04x:%02x", pci_domain_nr(b), bus);
strcpy(b->dev->name,"Host/PCI Bridge");
b->dev->parent = parent;
device_register(b->dev);

- b->number = b->secondary = bus;
- b->resource[0] = &ioport_resource;
- b->resource[1] = &iomem_resource;
return b;
}

struct pci_bus * __devinit pci_scan_bus_parented(struct device *parent, int bus, struct pci_ops *ops, void *sysdata)
{
- struct pci_bus *b = pci_alloc_primary_bus_parented(parent, bus);
+ struct pci_bus *b = pci_alloc_primary_bus_parented(parent, bus, sysdata);
if (b) {
- b->sysdata = sysdata;
b->ops = ops;
b->subordinate = pci_scan_child_bus(b);
pci_bus_add_devices(b);
===== drivers/pci/proc.c 1.29 vs edited =====
--- 1.29/drivers/pci/proc.c Wed Jun 11 02:33:14 2003
+++ edited/drivers/pci/proc.c Tue Jun 17 09:32:20 2003
@@ -382,6 +382,10 @@
if (!proc_initialized)
return -EACCES;

+ /* Backwards compatibility for domain 0 only */
+ if (pci_domain_nr(dev->bus) != 0)
+ return 0;
+
if (!(de = bus->procdir)) {
sprintf(name, "%02x", bus->number);
de = bus->procdir = proc_mkdir(name, proc_bus_pci_dir);
@@ -470,8 +474,9 @@
pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
- seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
- dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+ seq_printf(m, " Domain %2d, Bus %2d, device %3d, function %2d:\n",
+ pci_domain_nr(dev->bus), dev->bus->number, PCI_SLOT(dev->devfn),
+ PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
if (class)
seq_printf(m, " %s", class);
===== include/linux/pci.h 1.90 vs edited =====
--- 1.90/include/linux/pci.h Wed Jun 11 16:49:42 2003
+++ edited/include/linux/pci.h Tue Jun 17 10:27:32 2003
@@ -414,7 +414,7 @@
struct resource dma_resource[DEVICE_COUNT_DMA];
struct resource irq_resource[DEVICE_COUNT_IRQ];

- char slot_name[8]; /* slot name */
+ char slot_name[13]; /* slot name */

/* These fields are used by common fixups */
unsigned int transparent:1; /* Transparent PCI bridge */
@@ -536,7 +536,7 @@

/* Generic PCI functions used internally */

-int pci_bus_exists(const struct list_head *list, int nr);
+int pci_bus_exists(const struct list_head *list, struct pci_bus *bus);
struct pci_bus *pci_scan_bus_parented(struct device *parent, int bus, struct pci_ops *ops, void *sysdata);
static inline struct pci_bus *pci_scan_bus(int bus, struct pci_ops *ops, void *sysdata)
{
Ivan Kokshaysky
2003-06-17 09:41:56 UTC
Permalink
Post by Anton Blanchard
And only creates /proc/bus/pci entries for the first domain.
Err, this definitely breaks X on alpha. On small and mid-range
machines we always have pci_domain_nr(bus) == bus->number.
Practically, it's only Marvel where we could overflow an 8-bit
bus number.
Post by Anton Blanchard
Finally there was some shuffling required to make pci_bus_exists work
(passing in a pci_bus *, ->sysdata and ->number must be initialised
before calling it). There are some uses of pci_bus_exists in x86 that
will need updating.
Thoungts?
+ /* Backwards compatibility for domain 0 only */
+ if (pci_domain_nr(dev->bus) != 0)
+ return 0;
How about this instead?

/* Backwards compatibility for first N PCI domains. */
if (pci_domain_nr(dev->bus) > PCI_PROC_MAX_DOMAIN)
return 0;

PCI_PROC_MAX_DOMAIN could be defined in asm/pci.h (255 on alpha), default 0.

Ivan.
Anton Blanchard
2003-06-17 12:49:50 UTC
Permalink
Hi,
Post by Ivan Kokshaysky
Err, this definitely breaks X on alpha. On small and mid-range
machines we always have pci_domain_nr(bus) == bus->number.
Practically, it's only Marvel where we could overflow an 8-bit
bus number.
OK.
Post by Ivan Kokshaysky
How about this instead?
/* Backwards compatibility for first N PCI domains. */
if (pci_domain_nr(dev->bus) > PCI_PROC_MAX_DOMAIN)
return 0;
PCI_PROC_MAX_DOMAIN could be defined in asm/pci.h (255 on alpha), default 0.
A runtime test would be useful, at least for ppc64. That would allow our
older machines to work (multiple host bridges without overlapping
buses). What if we had pci_proc_max_domain and arch code could change its
value during pcibios_init?

Anton
Ivan Kokshaysky
2003-06-17 13:11:00 UTC
Permalink
Post by Anton Blanchard
A runtime test would be useful, at least for ppc64. That would allow our
older machines to work (multiple host bridges without overlapping
buses). What if we had pci_proc_max_domain and arch code could change its
value during pcibios_init?
Maybe. Or pci_proc_max_domain(), which can be a macro or a real function,
depending on arch.

Ivan.
Matthew Wilcox
2003-06-17 19:42:27 UTC
Permalink
Post by Ivan Kokshaysky
Post by Anton Blanchard
A runtime test would be useful, at least for ppc64. That would allow our
older machines to work (multiple host bridges without overlapping
buses). What if we had pci_proc_max_domain and arch code could change its
value during pcibios_init?
Maybe. Or pci_proc_max_domain(), which can be a macro or a real function,
depending on arch.
I don't think this is going to work out well; it's too complicated.

How about this (PPC & Sparc64 will have to decide what they want to do
for this case):

Index: drivers/pci/proc.c
===================================================================
RCS file: /var/cvs/linux-2.5/drivers/pci/proc.c,v
retrieving revision 1.9
diff -u -p -r1.9 proc.c
--- drivers/pci/proc.c 14 Jun 2003 22:15:29 -0000 1.9
+++ drivers/pci/proc.c 17 Jun 2003 19:36:50 -0000
@@ -383,7 +383,8 @@ int pci_proc_attach_device(struct pci_de
return -EACCES;

if (!(de = bus->procdir)) {
- sprintf(name, "%02x", bus->number);
+ if (!pci_name_bus(name, bus))
+ return -EEXIST;
de = bus->procdir = proc_mkdir(name, proc_bus_pci_dir);
if (!de)
return -ENOMEM;
Index: include/asm-alpha/pci.h
===================================================================
RCS file: /var/cvs/linux-2.5/include/asm-alpha/pci.h,v
retrieving revision 1.10
diff -u -p -r1.10 pci.h
--- include/asm-alpha/pci.h 14 Jun 2003 22:15:52 -0000 1.10
+++ include/asm-alpha/pci.h 17 Jun 2003 19:37:28 -0000
@@ -194,6 +194,13 @@ pcibios_resource_to_bus(struct pci_dev *

#define pci_domain_nr(bus) ((struct pci_controller *)(bus)->sysdata)->index

+/* We never have overlapping bus numbers on Alpha */
+static inline int pci_name_bus(char *name, struct pci_bus *bus)
+{
+ sprintf(name, "%02x", bus->number);
+ return 0;
+}
+
#endif /* __KERNEL__ */

/* Values for the `which' argument to sys_pciconfig_iobase. */
Index: include/asm-ia64/pci.h
===================================================================
RCS file: /var/cvs/linux-2.5/include/asm-ia64/pci.h,v
retrieving revision 1.8
diff -u -p -r1.8 pci.h
--- include/asm-ia64/pci.h 14 Jun 2003 22:15:56 -0000 1.8
+++ include/asm-ia64/pci.h 17 Jun 2003 19:37:45 -0000
@@ -93,6 +93,16 @@ struct pci_controller {

#define PCI_CONTROLLER(busdev) ((struct pci_controller *) busdev->sysdata)
#define pci_domain_nr(busdev) (PCI_CONTROLLER(busdev)->segment)
+
+static inline int pci_name_bus(char *name, struct pci_bus *bus)
+{
+ if (pci_domain_nr(bus) == 0) {
+ sprintf(name, "%02x", bus->number);
+ } else {
+ sprintf(name, "%04x:%02x", pci_domain_nr(bus), bus->number);
+ }
+ return 0;
+}

/* generic pci stuff */
#include <asm-generic/pci.h>
Index: include/linux/pci.h
===================================================================
RCS file: /var/cvs/linux-2.5/include/linux/pci.h,v
retrieving revision 1.17
diff -u -p -r1.17 pci.h
--- include/linux/pci.h 14 Jun 2003 22:16:01 -0000 1.17
+++ include/linux/pci.h 17 Jun 2003 19:37:56 -0000
@@ -808,6 +808,11 @@ extern int pci_pci_problems;

#ifndef CONFIG_PCI_DOMAINS
static inline int pci_domain_nr(struct pci_bus *bus) { return 0; }
+static inline int pci_name_bus(char *name, struct pci_bus *bus)
+{
+ sprintf(name, "%02x", bus->number);
+ return 0;
+}
#endif

#endif /* __KERNEL__ */
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Ivan Kokshaysky
2003-06-17 21:30:58 UTC
Permalink
Post by Matthew Wilcox
How about this (PPC & Sparc64 will have to decide what they want to do
I'm fine with it.
Post by Matthew Wilcox
+/* We never have overlapping bus numbers on Alpha */
"Never" is not quite correct - "in general we don't have"
would be better. Full-sized Marvel can have up to 512 root buses.

Ivan.
Matthew Wilcox
2003-06-18 13:02:34 UTC
Permalink
Post by Ivan Kokshaysky
Post by Matthew Wilcox
How about this (PPC & Sparc64 will have to decide what they want to do
I'm fine with it.
Post by Matthew Wilcox
+/* We never have overlapping bus numbers on Alpha */
"Never" is not quite correct - "in general we don't have"
would be better. Full-sized Marvel can have up to 512 root buses.
So what do you want to do about that case? If it's going to turn out to
be the same as other architectures, maybe we can do it differently...
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Ivan Kokshaysky
2003-06-18 13:24:09 UTC
Permalink
Post by Matthew Wilcox
Post by Ivan Kokshaysky
"Never" is not quite correct - "in general we don't have"
would be better. Full-sized Marvel can have up to 512 root buses.
So what do you want to do about that case?
Probably something like this:

static inline int pci_name_bus(char *name, struct pci_bus *bus)
{
if (pci_domain_nr(bus) < 256) {
sprintf(name, "%02x", bus->number);
} else {
sprintf(name, "%04x:%02x", pci_domain_nr(bus), bus->number);
}
return 0;
}

Ivan.

Matthew Wilcox
2003-06-17 13:55:48 UTC
Permalink
Post by Anton Blanchard
I like it. I think we do need the bus number in the top level since we
could have multiple host bridges on the same domain. I put a quick patch
/sys/devices/pci0002:00/0002:00:02.2
Yep, I have a patch to do the same thing. Sorry, didn't realise you
were working on this too; I should've cc'd you.
Post by Anton Blanchard
It also adds the domain to /proc/pci (are there userspace tools that
Probably ;-(
Post by Anton Blanchard
And only creates /proc/bus/pci entries for the first domain. I was going
to extend it one level to encode the domain but I now think we should just
move that functionality into sysfs and be done with it. Willy, you had a
patch that exposed BARS etc in sysfs didnt you? X and lspci etc will need
updating to match, but they are currently broken.
Yes. Some people felt my patch didn't go far enough (they wanted
_everything_ as its own little file, and damn the dentry/inode
consumption!), but it's resurrectable.

My personal feeling is that we should leave the resources file alone
(except for fixing its formatting; patch already with greg), expose the
config file and expose the contents of the resources.
Post by Anton Blanchard
I chose to add the domain into dev->slot_name since its needed for matching
kernel messages to drivers. Im wondering if we should make this conditional
on pci domain support since it does add some noise for those who couldnt
care less about domains.
I think we probably shouldn't since that requires additional testing &
fixing of stuff for those of us with multiple-domain boxes.
Post by Anton Blanchard
Finally there was some shuffling required to make pci_bus_exists work
(passing in a pci_bus *, ->sysdata and ->number must be initialised
before calling it). There are some uses of pci_bus_exists in x86 that
will need updating.
I actually eliminated pci_bus_exists() completely in my tree.
pci_find_bus() does the job just as well.
Post by Anton Blanchard
Thoungts?
We're definitely moving in the same direction ;-)

Here's one place we differ...
Post by Anton Blanchard
===== drivers/pci/proc.c 1.29 vs edited =====
--- 1.29/drivers/pci/proc.c Wed Jun 11 02:33:14 2003
+++ edited/drivers/pci/proc.c Tue Jun 17 09:32:20 2003
@@ -382,6 +382,10 @@
if (!proc_initialized)
return -EACCES;
+ /* Backwards compatibility for domain 0 only */
+ if (pci_domain_nr(dev->bus) != 0)
+ return 0;
+
if (!(de = bus->procdir)) {
sprintf(name, "%02x", bus->number);
de = bus->procdir = proc_mkdir(name, proc_bus_pci_dir);
I create them anyway, but put the domain on the front. I bet that'll
break Alpha too (I've read further in the thread but need to reply from
here ..)
Post by Anton Blanchard
@@ -470,8 +474,9 @@
pci_read_config_byte (dev, PCI_LATENCY_TIMER, &latency);
pci_read_config_byte (dev, PCI_MIN_GNT, &min_gnt);
pci_read_config_byte (dev, PCI_MAX_LAT, &max_lat);
- seq_printf(m, " Bus %2d, device %3d, function %2d:\n",
- dev->bus->number, PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
+ seq_printf(m, " Domain %2d, Bus %2d, device %3d, function %2d:\n",
+ pci_domain_nr(dev->bus), dev->bus->number, PCI_SLOT(dev->devfn),
+ PCI_FUNC(dev->devfn));
class = pci_class_name(class_rev >> 16);
if (class)
seq_printf(m, " %s", class);
I'd prefer not to touch it.
Post by Anton Blanchard
===== include/linux/pci.h 1.90 vs edited =====
--- 1.90/include/linux/pci.h Wed Jun 11 16:49:42 2003
+++ edited/include/linux/pci.h Tue Jun 17 10:27:32 2003
@@ -414,7 +414,7 @@
struct resource dma_resource[DEVICE_COUNT_DMA];
struct resource irq_resource[DEVICE_COUNT_IRQ];
- char slot_name[8]; /* slot name */
+ char slot_name[13]; /* slot name */
/* These fields are used by common fixups */
unsigned int transparent:1; /* Transparent PCI bridge */
Let's make slot_name a pointer to the struct device busid.
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Matthew Wilcox
2003-06-17 16:25:46 UTC
Permalink
Post by Anton Blanchard
I chose to add the domain into dev->slot_name since its needed for matching
kernel messages to drivers. Im wondering if we should make this conditional
on pci domain support since it does add some noise for those who couldnt
care less about domains.
It's also exposed to userspace in some ways I don't think I like.
Here's some of the fun ones:

./sound/oss/emu10k1/main.c: sprintf(s, "driver/emu10k1/%s", card->pci_dev->slot_name);
./drivers/scsi/scsi_ioctl.c: * pci_dev::slot_name (8 characters) for the PCI device (if any).
(oops, that one already changed to use the device->bus_id, so that broke ...)

And some potential buffer overruns:
./drivers/input/gameport/cs461x.c: sprintf(phys, "pci%s/gameport0", pdev->slot_name);
./drivers/net/3c59x.c: strcpy(info.bus_info, VORTEX_PCI(vp)->slot_name);
--
"It's not Hollywood. War is real, war is primarily not about defeat or
victory, it is about death. I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
Olaf Hering
2003-06-17 18:39:29 UTC
Permalink
Post by Matthew Wilcox
./drivers/net/3c59x.c: strcpy(info.bus_info, VORTEX_PCI(vp)->slot_name);
that one is for ethtool ioctl.
Is 'ethtool -i' obsolete in 2.6?
It seems sysfs has every info already.

***@smirnow:~/linux-2.5.72> l /sys/class/net/eth0/
total 0
drwxr-xr-x 3 root root 0 Jun 17 13:14 ./
drwxr-xr-x 4 root root 0 Jun 17 13:14 ../
-r--r--r-- 1 root root 4096 Jun 17 13:14 addr_len
-r--r--r-- 1 root root 4096 Jun 17 13:14 address
-r--r--r-- 1 root root 4096 Jun 17 13:14 broadcast
lrwxrwxrwx 1 root root 34 Jun 17 13:14 device -> ../../../devices/pci0/0000:00:0a.0/
lrwxrwxrwx 1 root root 30 Jun 17 13:14 driver -> ../../../bus/pci/drivers/3c59x/
-r--r--r-- 1 root root 4096 Jun 17 13:14 features
-rw-r--r-- 1 root root 4096 Jun 17 13:14 flags
-r--r--r-- 1 root root 4096 Jun 17 13:14 ifindex
-r--r--r-- 1 root root 4096 Jun 17 13:14 iflink
-rw-r--r-- 1 root root 4096 Jun 17 13:14 mtu
drwxr-xr-x 2 root root 0 Jun 17 13:14 statistics/
-rw-r--r-- 1 root root 4096 Jun 17 13:14 tx_queue_len
-r--r--r-- 1 root root 4096 Jun 17 13:14 type

driver, businfo.
drivers/<foo> does not provide a version string.
Should there be an entry for that?
--
USB is for mice, FireWire is for men!
Russell King
2003-06-11 15:42:20 UTC
Permalink
Post by Benjamin Herrenschmidt
The new pci_domain_nr() is good for adding the PCI domain number to
the /sys/devices/pciN/* names, but I think that's not the proper
representation. It should really be
/sys/devices/pci-domainN/pciN/*
So, "pci-domainN" would be a device itself, separate from the "pciN"
device. What physical hardware do these represent, or are they just
eye candy?
--
Russell King (***@arm.linux.org.uk) The developer of ARM Linux
http://www.arm.linux.org.uk/personal/aboutme.html
Anton Blanchard
2003-06-12 00:37:16 UTC
Permalink
Post by Benjamin Herrenschmidt
So we can pave the way for when we'll stop play bus number tricks and
actually have overlapping PCI bus numbers between domains. (I don't plan
to do that immediately because that would break userland & /proc/bus/pci
backward compatiblity)
As davem suggested, /proc/bus/pci should present domain 0 in the old
format even with pci domains enabled. If your graphics card is on domain
0 then X continues to work :)

Anton
Benjamin Herrenschmidt
2003-06-12 13:27:46 UTC
Permalink
Post by Anton Blanchard
Post by Benjamin Herrenschmidt
So we can pave the way for when we'll stop play bus number tricks and
actually have overlapping PCI bus numbers between domains. (I don't plan
to do that immediately because that would break userland & /proc/bus/pci
backward compatiblity)
As davem suggested, /proc/bus/pci should present domain 0 in the old
format even with pci domains enabled. If your graphics card is on domain
0 then X continues to work :)
Hrm... On most pmacs, it is, since domain 0 is the AGP port. Though
people with an additional PCI video card will not be happy. But X will
be fixed, so....

Ben.
Loading...