On Wed, Apr 27, 2016 at 06:31:29PM +0100, Lorenzo Pieralisi wrote:
On Wed, Apr 27, 2016 at 11:44:53AM -0500, Bjorn Helgaas wrote:
On Wed, Apr 27, 2016 at 12:17:58PM +0100, Lorenzo Pieralisi wrote:
On Tue, Apr 26, 2016 at 09:26:49PM -0500, Bjorn Helgaas wrote:
On Fri, Apr 15, 2016 at 07:06:37PM +0200, Tomasz Nowicki wrote:
Today we call pci_bus_assign_domain_nr() from the PCI core (from pci_create_root_bus()). This is only implemented for PCI_DOMAINS_GENERIC, but even so, it fiddles around to figure out whether to get the domain from DT or to assign a new one.
That seems backwards to me. The host bridge drivers already know where the domain should come from (ACPI _SEG, DT, etc.) and in the long term, I think they should be responsible for looking up or assigning a domain number *before* they call pci_create_root_bus().
Yes, the question still is how pci_create_root_bus() can get that value (I am pretty certain this was heavily debated in the past, which does not mean we can't give it another try).
Right, we don't have a good mechanism for passing more info into pci_create_root_bus(). Maybe the caller could fill in a struct so we have a chance to extend it without having to change all the existing callers.
I wonder if there's a design pattern we can copy, e.g., would something like the scsi_host_alloc(), scsi_add_host(), scsi_scan_host() model work here?
+void pci_bus_assign_domain_nr(struct pci_bus *bus, struct device *parent) +{
- bus->domain_nr = acpi_disabled ? of_pci_bus_domain_nr(parent) :
acpi_pci_bus_domain_nr(parent);We have the pci_bus * here, so to_pci_host_bridge(bus->bridge) gives us the struct pci_host_bridge. I can't remember why we put domain_nr in the struct pci_bus instead of in the struct pci_host_bridge. It seems like pci_host_bridge is the more logical place for it, because every bus below the host bridge must have the same domain by definition.
Would it be feasible to either (a) move domain_nr to the pci_host_bridge, or (b) change acpi_pci_bus_domain_nr() so it uses the struct pci_bus * or the struct device * to find the struct acpi_pci_root where segment has already been stored by acpi_pci_root_add()?
(b) is what JC implemented even though it works differently for different hosts since it all depends on what's in bus->sysdata.
It can certainly be done in a generic way (that works on X86 and IA64 too), let's give it more thought.
Another wrinkle is the quirk added by 1f09b09b4de0 ("x86/PCI: Ignore _SEG on HP xw9300"). x86 doesn't use PCI_DOMAINS_GENERIC yet, so this patch wouldn't break it, but I hope x86 can use PCI_DOMAINS_GENERIC in the future, and then it will be a problem if we evaluate _SEG again.
Yes, I share your concern here and I thought about that, if that's the end goal let's find a solution that works across arches (or we temporarily use JC's code and we then generalize it).
I would ultimately like all arches to use PCI_DOMAINS_GENERIC, because I don't think there's anything intrisically arch-specific about where we store the domain number. The means of discovering or assigning a domain number might be arch-specific, but I think it would be cleanest if the host bridge driver handled that.
Bjorn