2008-07-11 21:13:56

by John Keller

[permalink] [raw]
Subject: x86: PCI multi domain/segment config access

On x86 PCI Config space access to domains/segments > 0 appears to be broken.
The noted patch related to PCI ext config space has all config accesses
to offsets < 256 using conf1 if available, which is most likely the case.

This would force usage of conf1 for non-extended config access (< 256) on
PCI domains > 0.

Am I missing something, or how would a conf1 access reference a domain
other than zero?

John
------------

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=b6ce068a1285a24185b01be8a49021827516b3e1




int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
int reg, int len, u32 *val)
{
if (reg < 256 && raw_pci_ops)
return raw_pci_ops->read(domain, bus, devfn, reg, len, val);
if (raw_pci_ext_ops)
return raw_pci_ext_ops->read(domain, bus, devfn, reg, len, val);
return -EINVAL;
}

int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
int reg, int len, u32 val)
{
if (reg < 256 && raw_pci_ops)
return raw_pci_ops->write(domain, bus, devfn, reg, len, val);
if (raw_pci_ext_ops)
return raw_pci_ext_ops->write(domain, bus, devfn, reg, len, val);
return -EINVAL;
}



2008-07-11 21:21:27

by Matthew Wilcox

[permalink] [raw]
Subject: Re: x86: PCI multi domain/segment config access

On Fri, Jul 11, 2008 at 04:13:47PM -0500, John Keller wrote:
> On x86 PCI Config space access to domains/segments > 0 appears to be broken.
> The noted patch related to PCI ext config space has all config accesses
> to offsets < 256 using conf1 if available, which is most likely the case.
>
> This would force usage of conf1 for non-extended config access (< 256) on
> PCI domains > 0.
>
> Am I missing something, or how would a conf1 access reference a domain
> other than zero?

You're correct. Sorry about that, I wasn't thinking about domains at
the time I wrote the patch. Try this:

[X86] Fix PCI config space for domains > 0

John Keller reports that PCI config space access is broken on machines
with more than one domain. conf1 accesses only work for domain 0.

Signed-off-by: Matthew Wilcox <[email protected]>

diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
index 6e64aaf..fe33533 100644
--- a/arch/x86/pci/common.c
+++ b/arch/x86/pci/common.c
@@ -31,7 +31,7 @@ struct pci_raw_ops *raw_pci_ext_ops;
int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
int reg, int len, u32 *val)
{
- if (reg < 256 && raw_pci_ops)
+ if (domain == 0 && reg < 256 && raw_pci_ops)
return raw_pci_ops->read(domain, bus, devfn, reg, len, val);
if (raw_pci_ext_ops)
return raw_pci_ext_ops->read(domain, bus, devfn, reg, len, val);
@@ -41,7 +41,7 @@ int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
int reg, int len, u32 val)
{
- if (reg < 256 && raw_pci_ops)
+ if (domain == 0 && reg < 256 && raw_pci_ops)
return raw_pci_ops->write(domain, bus, devfn, reg, len, val);
if (raw_pci_ext_ops)
return raw_pci_ext_ops->write(domain, bus, devfn, reg, len, val);

--
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours. We can't possibly take such
a retrograde step."

2008-07-11 21:44:51

by John Keller

[permalink] [raw]
Subject: Re: x86: PCI multi domain/segment config access

>
> On Fri, Jul 11, 2008 at 04:13:47PM -0500, John Keller wrote:
> > On x86 PCI Config space access to domains/segments > 0 appears to be broken.
> > The noted patch related to PCI ext config space has all config accesses
> > to offsets < 256 using conf1 if available, which is most likely the case.
> >
> > This would force usage of conf1 for non-extended config access (< 256) on
> > PCI domains > 0.
> >
> > Am I missing something, or how would a conf1 access reference a domain
> > other than zero?
>
> You're correct. Sorry about that, I wasn't thinking about domains at
> the time I wrote the patch. Try this:

Yup, just what we were thinking.
Works fine.

Acked-by: John Keller <[email protected]>



>
> [X86] Fix PCI config space for domains > 0
>
> John Keller reports that PCI config space access is broken on machines
> with more than one domain. conf1 accesses only work for domain 0.
>
> Signed-off-by: Matthew Wilcox <[email protected]>
>
> diff --git a/arch/x86/pci/common.c b/arch/x86/pci/common.c
> index 6e64aaf..fe33533 100644
> --- a/arch/x86/pci/common.c
> +++ b/arch/x86/pci/common.c
> @@ -31,7 +31,7 @@ struct pci_raw_ops *raw_pci_ext_ops;
> int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
> int reg, int len, u32 *val)
> {
> - if (reg < 256 && raw_pci_ops)
> + if (domain == 0 && reg < 256 && raw_pci_ops)
> return raw_pci_ops->read(domain, bus, devfn, reg, len, val);
> if (raw_pci_ext_ops)
> return raw_pci_ext_ops->read(domain, bus, devfn, reg, len, val);
> @@ -41,7 +41,7 @@ int raw_pci_read(unsigned int domain, unsigned int bus, unsigned int devfn,
> int raw_pci_write(unsigned int domain, unsigned int bus, unsigned int devfn,
> int reg, int len, u32 val)
> {
> - if (reg < 256 && raw_pci_ops)
> + if (domain == 0 && reg < 256 && raw_pci_ops)
> return raw_pci_ops->write(domain, bus, devfn, reg, len, val);
> if (raw_pci_ext_ops)
> return raw_pci_ext_ops->write(domain, bus, devfn, reg, len, val);
>
> --
> Intel are signing my paycheques ... these opinions are still mine
> "Bill, look, we understand that you're interested in selling us this
> operating system, but compare it to ours. We can't possibly take such
> a retrograde step."
>

2008-07-14 21:28:28

by Jesse Barnes

[permalink] [raw]
Subject: Re: x86: PCI multi domain/segment config access

On Friday, July 11, 2008 2:21 pm Matthew Wilcox wrote:
> On Fri, Jul 11, 2008 at 04:13:47PM -0500, John Keller wrote:
> > On x86 PCI Config space access to domains/segments > 0 appears to be
> > broken. The noted patch related to PCI ext config space has all config
> > accesses to offsets < 256 using conf1 if available, which is most likely
> > the case.
> >
> > This would force usage of conf1 for non-extended config access (< 256) on
> > PCI domains > 0.
> >
> > Am I missing something, or how would a conf1 access reference a domain
> > other than zero?
>
> You're correct. Sorry about that, I wasn't thinking about domains at
> the time I wrote the patch. Try this:
>
> [X86] Fix PCI config space for domains > 0
>
> John Keller reports that PCI config space access is broken on machines
> with more than one domain. conf1 accesses only work for domain 0.
>
> Signed-off-by: Matthew Wilcox <[email protected]>

Oops, yeah, good catch. Thanks for fixing this up, Matthew. Just applied it
to my linux-next branch.

Thanks,
Jesse