[PATCH] x86/x86-64 PCI domain support

[PATCH] x86/x86-64 PCI domain support

[Jeff Garzik]

Now that the dust has settled and the prep work is upstream, adding PCI
domain support to x86 is a lot more straightforward.

Targetted for 2.6.24.



commit c7b14cf6aa39e09aef89a0cd08c553dba1ee7e98
Author: Jeff Garzik <jeff@garzik.org>
Date:   Sat Sep 1 07:36:25 2007 -0400

    PCI domain support for x86[-64]
    
    aka "PCI segment" support, in ACPI lingo.
    
    Signed-off-by: Jeff Garzik <jeff@garzik.org>

 arch/i386/pci/acpi.c     |   15 ++++++++++-----
 arch/i386/pci/common.c   |    6 ++++--
 arch/x86_64/Kconfig      |    4 ++++
 include/asm-i386/pci.h   |   14 ++++++++++++++
 include/asm-x86_64/pci.h |   14 ++++++++++++++
 5 files changed, 46 insertions(+), 7 deletions(-)

c7b14cf6aa39e09aef89a0cd08c553dba1ee7e98
diff --git a/arch/i386/pci/acpi.c b/arch/i386/pci/acpi.c
index bc8a44b..6beb789 100644
--- a/arch/i386/pci/acpi.c
+++ b/arch/i386/pci/acpi.c
@@ -11,6 +11,13 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int do
 	struct pci_sysdata *sd;
 	int pxm;
 
+#ifndef CONFIG_PCI_DOMAINS
+	if (domain != 0) {
+		printk(KERN_WARNING "PCI: Multiple domains not supported\n");
+		return NULL;
+	}
+#endif
+
 	/* Allocate per-root-bus (not per bus) arch-specific data.
 	 * TODO: leak; this memory is never freed.
 	 * It's arguable whether it's worth the trouble to care.
@@ -21,11 +28,9 @@ struct pci_bus * __devinit pci_acpi_scan_root(struct acpi_device *device, int do
 		return NULL;
 	}
 
-	if (domain != 0) {
-		printk(KERN_WARNING "PCI: Multiple domains not supported\n");
-		kfree(sd);
-		return NULL;
-	}
+#ifdef CONFIG_PCI_DOMAINS
+	sd->domain = domain;
+#endif
 
 	sd->node = -1;
 
diff --git a/arch/i386/pci/common.c b/arch/i386/pci/common.c
index ebc6f3c..6d7a274 100644
--- a/arch/i386/pci/common.c
+++ b/arch/i386/pci/common.c
@@ -29,12 +29,14 @@ struct pci_raw_ops *raw_pci_ops;
 
 static int pci_read(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 *value)
 {
-	return raw_pci_ops->read(0, bus->number, devfn, where, size, value);
+	return raw_pci_ops->read(pci_domain_nr(bus), bus->number,
+				 devfn, where, size, value);
 }
 
 static int pci_write(struct pci_bus *bus, unsigned int devfn, int where, int size, u32 value)
 {
-	return raw_pci_ops->write(0, bus->number, devfn, where, size, value);
+	return raw_pci_ops->write(pci_domain_nr(bus), bus->number,
+				  devfn, where, size, value);
 }
 
 struct pci_ops pci_root_ops = {
diff --git a/arch/x86_64/Kconfig b/arch/x86_64/Kconfig
index ffa0364..1cb16c6 100644
--- a/arch/x86_64/Kconfig
+++ b/arch/x86_64/Kconfig
@@ -732,6 +732,10 @@ config PCI_MMCONFIG
 	bool "Support mmconfig PCI config space access"
 	depends on PCI && ACPI
 
+config PCI_DOMAINS
+	bool "PCI domain support"
+	depends on PCI
+
 source "drivers/pci/pcie/Kconfig"
 
 source "drivers/pci/Kconfig"
diff --git a/include/asm-i386/pci.h b/include/asm-i386/pci.h
index 4fcacc7..37d3322 100644
--- a/include/asm-i386/pci.h
+++ b/include/asm-i386/pci.h
@@ -5,12 +5,26 @@
 #ifdef __KERNEL__
 
 struct pci_sysdata {
+	int		domain;		/* PCI domain */
 	int		node;		/* NUMA node */
 };
 
 /* scan a bus after allocating a pci_sysdata for it */
 extern struct pci_bus *pci_scan_bus_with_sysdata(int busno);
 
+#ifdef CONFIG_PCI_DOMAINS
+static inline int pci_domain_nr(struct pci_bus *bus)
+{
+	struct pci_sysdata *sd = bus->sysdata;
+	return sd->domain;
+}
+
+static inline int pci_proc_domain(struct pci_bus *bus)
+{
+	return pci_domain_nr(bus);
+}
+#endif /* CONFIG_PCI_DOMAINS */
+
 #include <linux/mm.h>		/* for struct page */
 
 /* Can be used to override the logic in pci_scan_bus for skipping
diff --git a/include/asm-x86_64/pci.h b/include/asm-x86_64/pci.h
index 5da8cb0..6ce2731 100644
--- a/include/asm-x86_64/pci.h
+++ b/include/asm-x86_64/pci.h
@@ -6,12 +6,26 @@
 #ifdef __KERNEL__
 
 struct pci_sysdata {
+	int		domain;		/* PCI domain */
 	int		node;		/* NUMA node */
 	void*		iommu;		/* IOMMU private data */
 };
 
 extern struct pci_bus *pci_scan_bus_with_sysdata(int busno);
 
+#ifdef CONFIG_PCI_DOMAINS
+static inline int pci_domain_nr(struct pci_bus *bus)
+{
+	struct pci_sysdata *sd = bus->sysdata;
+	return sd->domain;
+}
+
+static inline int pci_proc_domain(struct pci_bus *bus)
+{
+	return pci_domain_nr(bus);
+}
+#endif /* CONFIG_PCI_DOMAINS */
+
 #ifdef CONFIG_CALGARY_IOMMU
 static inline void* pci_iommu(struct pci_bus *bus)
 {

Re: [PATCH] x86/x86-64 PCI domain support

[Andi Kleen]

>  
> +config PCI_DOMAINS
> +	bool "PCI domain support"
> +	depends on PCI

I don't think this should be a config option.

But there should be a pci=... option with Documentation to turn it off at runtime


> +static inline int pci_proc_domain(struct pci_bus *bus)
> +{
> +	return pci_domain_nr(bus);
> +}

The second function is redundant? 


-Andi

Re: [PATCH] x86/x86-64 PCI domain support

[Jeff Garzik]

Andi Kleen wrote:
>>  
>> +config PCI_DOMAINS
>> +	bool "PCI domain support"
>> +	depends on PCI
> 
> I don't think this should be a config option.

CONFIG_PCI_DOMAINS is referenced in arch-neutral code, so the symbol 
_must_ be defined.

It can be hidden, perhaps, if that makes our x86 maintainer happy :)


> But there should be a pci=... option with Documentation to turn it off at runtime

OK


>> +static inline int pci_proc_domain(struct pci_bus *bus)
>> +{
>> +	return pci_domain_nr(bus);
>> +}
> 
> The second function is redundant? 

No, it's a hook we must implement, when CONFIG_PCI_DOMAINS is enabled.

	Jeff

Re: [PATCH] x86/x86-64 PCI domain support

[Andi Kleen]

> > The second function is redundant? 
> 
> No, it's a hook we must implement, when CONFIG_PCI_DOMAINS is enabled.

Then the other function is redundant.

-Andi

Re: [PATCH] x86/x86-64 PCI domain support

[Jeff Garzik]

Andi Kleen wrote:
>>> The second function is redundant? 
>> No, it's a hook we must implement, when CONFIG_PCI_DOMAINS is enabled.
> 
> Then the other function is redundant.

No, both functions are required by the interface.

	Jeff

Re: [PATCH] x86/x86-64 PCI domain support

[Muli Ben-Yehuda]

On Sat, Sep 01, 2007 at 10:32:23AM -0400, Jeff Garzik wrote:
> 
> Now that the dust has settled and the prep work is upstream, adding
> PCI edomain support to x86 is a lot more straightforward.
> 
> Targetted for 2.6.24.

Looks good to me.

Cheers,
Muli

Re: [PATCH] x86/x86-64 PCI domain support

[Randy Dunlap]

On Sat, 01 Sep 2007 18:40:27 -0400 Jeff Garzik wrote:

> Andi Kleen wrote:
> >>> The second function is redundant? 
> >> No, it's a hook we must implement, when CONFIG_PCI_DOMAINS is enabled.
> > 
> > Then the other function is redundant.
> 
> No, both functions are required by the interface.

by what interface?  and why, please?  (instead of just stating "required")


---
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***

Re: [PATCH] x86/x86-64 PCI domain support

[Jeff Garzik]

Randy Dunlap wrote:
> On Sat, 01 Sep 2007 18:40:27 -0400 Jeff Garzik wrote:
> 
>> Andi Kleen wrote:
>>>>> The second function is redundant? 
>>>> No, it's a hook we must implement, when CONFIG_PCI_DOMAINS is enabled.
>>> Then the other function is redundant.
>> No, both functions are required by the interface.
> 
> by what interface?  and why, please?  (instead of just stating "required")

grep for CONFIG_PCI_DOMAINS in arch code and include/linux/pci.h.

This is normal "arch" interface:  you enable a define, and a group of 
functions is assumed to be present.  Otherwise (!defined), a set of stub 
no-ops is activated for your arch.

I have implemented the [small] group of functions the code assumes to be 
present, when CONFIG_PCI_DOMAINS is enabled, like all the other arches 
that implement PCI domain support.

	Jeff

Re: [PATCH] x86/x86-64 PCI domain support

[Randy Dunlap]

Jeff Garzik wrote:
> Randy Dunlap wrote:
>> On Sat, 01 Sep 2007 18:40:27 -0400 Jeff Garzik wrote:
>>
>>> Andi Kleen wrote:
>>>>>> The second function is redundant? 
>>>>> No, it's a hook we must implement, when CONFIG_PCI_DOMAINS is enabled.
>>>> Then the other function is redundant.
>>> No, both functions are required by the interface.
>>
>> by what interface?  and why, please?  (instead of just stating 
>> "required")
> 
> grep for CONFIG_PCI_DOMAINS in arch code and include/linux/pci.h.

Thanks, I get it.

> This is normal "arch" interface:  you enable a define, and a group of 
> functions is assumed to be present.  Otherwise (!defined), a set of stub 
> no-ops is activated for your arch.
> 
> I have implemented the [small] group of functions the code assumes to be 
> present, when CONFIG_PCI_DOMAINS is enabled, like all the other arches 
> that implement PCI domain support.


+#ifdef CONFIG_PCI_DOMAINS
+static inline int pci_domain_nr(struct pci_bus *bus)
+{
+	struct pci_sysdata *sd = bus->sysdata;
+	return sd->domain;
+}
+
+static inline int pci_proc_domain(struct pci_bus *bus)
+{
+	return pci_domain_nr(bus);
+}
+#endif /* CONFIG_PCI_DOMAINS */

So if CONFIG_PCI_DOMAINS=y, proc_proc_domain() decides to print the domain nr
in /proc iff the domain nr != 0 ?  so that the /proc file format is
different depending on the domain nr (0 vs. !0) ?

I suppose that is similar to what arch/powerpc/kernel/pci_64.c does
with hose->buid.  I think that I had rather see the domain nr printed
whenever CONFIG_PCI_DOMAINS=y... oh well.


-- 
~Randy
*** Remember to use Documentation/SubmitChecklist when testing your code ***