Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Mon, Jun 18, 2012 at 2:39 PM, Benjamin Herrenschmidt b...@kernel.crashing.org wrote: On Mon, 2012-06-18 at 09:23 -0600, Bjorn Helgaas wrote: Ooh, sorry about that. Who's going to push this fix? I don't see it in Linus' tree yet. If you want me to, please forward me the original patch. I want to double check something. We are still printing something wrong in the kernel log, so it looks like we have some incorrect carry in some arithmetic happening somewhere. Probably not related to whatever you're seeing, but just FYI, I think the core has a sign-extension bug related to P2P bridge I/O windows: http://marc.info/?l=linux-pcim=134023801524895w=2 ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
(Bjorn, added you back because I ended up digging a shitload
of issues including some generic ... see below)
On Tue, 2012-06-05 at 23:50 -0400, Ben Collins wrote:
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
So I came to the conclusion that this isn't the right fix... what we
have is an interesting combinations of issues here and I'll have to
dig with Bjorn to sort them all out.
In the meantime, let me know if the quick fix below works for you
as a workaround:
@@ -734,7 +740,7 @@ void __devinit pci_process_bridge_OF_ranges(struct pci_contr
hose-io_base_virt = ioremap(cpu_addr, size);
/* Expect trouble if pci_addr is not 0 */
- if (primary)
+ if (primary || !isa_io_base)
isa_io_base =
(unsigned long)hose-io_base_virt;
(Note: The root of the qemu issue seems to be that qemu doesn't accept
an IO BAR with a value of 0 as a valid IO BAR... which is arguably a
qemu bug. However, we shouldn't have assigned that if it wasn't for
a bunch of other nasties)
Now, we have a combination of problems here Bjorn. A bunch of them are
powerpc specific and I'll fix them, but I'm hitting a couple of nasties
in the generic resource allocation code:
* I'm not too happy with our pcibios_io_space_offset() sign extension,
but that's no biggie, I will fix that
* It triggers a bunch of other problems where quite a bit of code
in pci-common.c seems to be assuming 32-bit arithmetic when messing
around with the port remapping, so I have fixes for all of that
* Additionally, we end up with a crazy offset for IO because our
isa_io_base is only set if we mark a host bridge as primary and it
looks like the FSL code doesn't mark any. So our isa_io_base is 0
instead of the virt base of the first bridge, and thus our resource
offsets are equal to the virt base of the IO space of the bridge :-)
This isn't wrong per-se, ie, it -should- still work, but it then
trigger interesting problems. The patch above corrects that by making
isa_io_base default to the first bridge IO base if there's no primary.
* PCIBIOS_IO_MIN (and MEM_MIN) are bogus... On most architectures
they represent a bus address, but the generic code uses them as a
resource address. So they don't work and we end up handing out
an IO resource equal to 0 (bus) instead of 0x1000 (bus) because the
offset'ed resource address isn't smaller than the min. Now I do have a
patch to fix that in the generic code, but I suspect that will break
a couple of archs:
arch/mn10300/include/asm/pci.h:#define PCIBIOS_MIN_IO 0xBE04
arch/alpha/include/asm/pci.h:#define PCIBIOS_MIN_IO
alpha_mv.min_io_address
and maybe some mips stuff, I'm not sure. Do those archs do any
offsetting ? If they do we might want to change those defines
along with my patch:
diff --git a/drivers/pci/setup-res.c b/drivers/pci/setup-res.c
index eea85da..ec429f3 100644
--- a/drivers/pci/setup-res.c
+++ b/drivers/pci/setup-res.c
@@ -131,10 +131,20 @@ static int __pci_assign_resource(struct pci_bus *bus,
struct pci_dev *dev,
int resno, resource_size_t size, resource_size_t align)
{
struct resource *res = dev-resource + resno;
+ struct resource min_res;
+ struct pci_bus_region min_reg;
resource_size_t min;
int ret;
- min = (res-flags IORESOURCE_IO) ? PCIBIOS_MIN_IO : PCIBIOS_MIN_MEM;
+ /*
+* We convert PCIBIOS_MIN_IO/MEM from bus addresses to
+* resources before passing them to pci_bus_alloc_resource
+*/
+ min_res.flags = res-flags;
+ min_reg.start = (res-flags IORESOURCE_IO) ? PCIBIOS_MIN_IO :
PCIBIOS_MIN_MEM;
+ min_reg.end = min_reg.start;
+ pcibios_bus_to_resource(dev, min_res, min_reg);
+ min = min_res.start;
/* First, try exact prefetching match.. */
ret = pci_bus_alloc_resource(bus, res, size, align, min,
* Additionally, there's another problem inside pci_bus_alloc_resource()
itself:
/* Ok, try it out.. */
ret = allocate_resource(r, res, size,
r-start ? : min,
max, align,
alignf, alignf_data);
See the r-start ? : min ?
It only applies the min if r-start is 0 now we have a hack in
arch/powerpc
to clear out resources we think are unassigned. Unfortunately that hack got
broken
when moving the offett'ing to generic code because it now tests the resource
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Tue, Jun 5, 2012 at 11:15 PM, Benjamin Herrenschmidt
b...@kernel.crashing.org wrote:
On Tue, 2012-06-05 at 23:50 -0400, Ben Collins wrote:
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
See Bjorn, we both did end up getting it wrong :-)
Ooh, sorry about that. Who's going to push this fix? I don't see it
in Linus' tree yet. If you want me to, please forward me the original
patch.
Signed-off-by: Ben Collins bcoll...@ubuntu.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org
arch/powerpc/kernel/pci-common.c | 8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 8e78e93..be9ced7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int
mask)
return pci_enable_resources(dev, mask);
}
+/* Before assuming too much here, take care to realize that we need sign
+ * extension from 32-bit pointers to 64-bit resource addresses to work.
+ */
resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
{
- return (unsigned long) hose-io_base_virt - _IO_BASE;
+ long vbase = (long)hose-io_base_virt;
+ long io_base = _IO_BASE;
+
+ return (resource_size_t)(vbase - io_base);
}
static void __devinit pcibios_setup_phb_resources(struct pci_controller
*hose, struct list_head *resources)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Mon, 2012-06-18 at 09:23 -0600, Bjorn Helgaas wrote: Ooh, sorry about that. Who's going to push this fix? I don't see it in Linus' tree yet. If you want me to, please forward me the original patch. I want to double check something. We are still printing something wrong in the kernel log, so it looks like we have some incorrect carry in some arithmetic happening somewhere. Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Jun 18, 2012, at 3:39 PM, Benjamin Herrenschmidt wrote: On Mon, 2012-06-18 at 09:23 -0600, Bjorn Helgaas wrote: Ooh, sorry about that. Who's going to push this fix? I don't see it in Linus' tree yet. If you want me to, please forward me the original patch. I want to double check something. We are still printing something wrong in the kernel log, so it looks like we have some incorrect carry in some arithmetic happening somewhere. Can we at least get this patch in to get things working again? It's still a regression either way. Cheers, Ben. -- Ben Collins Servergy, Inc. (757) 243-7557 CONFIDENTIALITY NOTICE: This communication contains privileged and/or confidential information; and should be maintained with the strictest confidence. It is intended solely for the use of the person or entity in which it is addressed. If you are not the intended recipient, you are STRICTLY PROHIBITED from disclosing, copying, distributing or using any of this information. If you received this communication in error, please contact the sender immediately and destroy the material in its entirety, whether electronic or hard copy. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Mon, 2012-06-18 at 16:04 -0500, Ben Collins wrote: On Jun 18, 2012, at 3:39 PM, Benjamin Herrenschmidt wrote: On Mon, 2012-06-18 at 09:23 -0600, Bjorn Helgaas wrote: Ooh, sorry about that. Who's going to push this fix? I don't see it in Linus' tree yet. If you want me to, please forward me the original patch. I want to double check something. We are still printing something wrong in the kernel log, so it looks like we have some incorrect carry in some arithmetic happening somewhere. Can we at least get this patch in to get things working again? It's still a regression either way. Sure, I'm only just back from my sick leave, I haven't had a chance to go through the pending patches yet. Cheers, Ben. Cheers, Ben. -- Ben Collins Servergy, Inc. (757) 243-7557 CONFIDENTIALITY NOTICE: This communication contains privileged and/or confidential information; and should be maintained with the strictest confidence. It is intended solely for the use of the person or entity in which it is addressed. If you are not the intended recipient, you are STRICTLY PROHIBITED from disclosing, copying, distributing or using any of this information. If you received this communication in error, please contact the sender immediately and destroy the material in its entirety, whether electronic or hard copy. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Jun 7, 2012, at 5:32 PM, Benjamin Herrenschmidt wrote: On Thu, 2012-06-07 at 11:38 -0400, Ben Collins wrote: Note that oddly, the second range of bus addresses looks -more- correct than the first one... Except that the first one is exactly the same bus address as my bare metal system: pci_bus :00: root bus resource [io 0xffbeb000-0xffbfafff] (bus address [0x1-0x1]) I only have one PCIe RAID card on the bare metal system. Not surprising I never noticed the problem on it directly. Can you show me the device-tree node for that PCI host bridge ? It's a p4080ds, so it's in arch/powerpc/boot/ And that means that this bug affects a real hardware platform, so I think it makes it more valid to include it. The only reason it didn't affect me directly is because my only PCIe card doesn't have io, just mem BARs. -- Bluecherry: http://www.bluecherrydvr.com/ SwissDisk : http://www.swissdisk.com/ Ubuntu: http://www.ubuntu.com/ My Blog : http://ben-collins.blogspot.com/ signature.asc Description: Message signed with OpenPGP using GPGMail ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Fri, 2012-06-08 at 14:38 -0400, Ben Collins wrote: pci_bus :00: root bus resource [io 0xffbeb000-0xffbfafff] (bus address [0x1-0x1]) I only have one PCIe RAID card on the bare metal system. Not surprising I never noticed the problem on it directly. Can you show me the device-tree node for that PCI host bridge ? It's a p4080ds, so it's in arch/powerpc/boot/ And that means that this bug affects a real hardware platform, so I think it makes it more valid to include it. The only reason it didn't affect me directly is because my only PCIe card doesn't have io, just mem BARs. Something doesn't make sense. The bus address printed above are clearly not right, the .dts contains for the ranges entries for IO space of all 3 bridges: 0x0100 0 0x 0xf 0xf800 0x0 0x0001 0x0100 0 0x 0xf 0xf801 0x0 0x0001 0x0100 0 0x 0xf 0xf802 0x0 0x0001 So something is wrong with the printing of the bus address, there's a stale top bit (overflow from the 64-bit substraction somewhere ?) Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Wed, 2012-06-06 at 19:35 -0400, Ben Collins wrote: pci_bus :00: root bus resource [io 0xffbed000-0xffbfcfff] (bus address [0x1-0x1]) Without the fix that I sent, it ends up looking like: pci_bus :00: root bus resource [io 0xffbed000-0xffbfcfff] (bus address [0x-0x]) And that's when some devices fail to be assigned valid bar 0's and the kernel complains because of it. Note that oddly, the second range of bus addresses looks -more- correct than the first one... Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Jun 7, 2012, at 5:30 AM, Benjamin Herrenschmidt wrote: On Wed, 2012-06-06 at 19:35 -0400, Ben Collins wrote: pci_bus :00: root bus resource [io 0xffbed000-0xffbfcfff] (bus address [0x1-0x1]) Without the fix that I sent, it ends up looking like: pci_bus :00: root bus resource [io 0xffbed000-0xffbfcfff] (bus address [0x-0x]) And that's when some devices fail to be assigned valid bar 0's and the kernel complains because of it. Note that oddly, the second range of bus addresses looks -more- correct than the first one... Except that the first one is exactly the same bus address as my bare metal system: pci_bus :00: root bus resource [io 0xffbeb000-0xffbfafff] (bus address [0x1-0x1]) I only have one PCIe RAID card on the bare metal system. Not surprising I never noticed the problem on it directly. Cheers, Ben. -- Ben Collins Servergy, Inc. (757) 243-7557 CONFIDENTIALITY NOTICE: This communication contains privileged and/or confidential information; and should be maintained with the strictest confidence. It is intended solely for the use of the person or entity in which it is addressed. If you are not the intended recipient, you are STRICTLY PROHIBITED from disclosing, copying, distributing or using any of this information. If you received this communication in error, please contact the sender immediately and destroy the material in its entirety, whether electronic or hard copy. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Thu, 2012-06-07 at 11:38 -0400, Ben Collins wrote: Note that oddly, the second range of bus addresses looks -more- correct than the first one... Except that the first one is exactly the same bus address as my bare metal system: pci_bus :00: root bus resource [io 0xffbeb000-0xffbfafff] (bus address [0x1-0x1]) I only have one PCIe RAID card on the bare metal system. Not surprising I never noticed the problem on it directly. Can you show me the device-tree node for that PCI host bridge ? Cheers, Ben. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On 06/05/2012 10:50 PM, Ben Collins wrote:
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
Where do you see addresses over 32 bits in QEMU's e500-pci, at least
with current mainline QEMU and the mpc8544ds model?
I/O space should be at 0xe100.
I'm also not sure what this has to do with the virtual address returned
by ioremap().
Signed-off-by: Ben Collins bcoll...@ubuntu.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
---
arch/powerpc/kernel/pci-common.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 8e78e93..be9ced7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int
mask)
return pci_enable_resources(dev, mask);
}
+/* Before assuming too much here, take care to realize that we need sign
+ * extension from 32-bit pointers to 64-bit resource addresses to work.
+ */
resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
{
- return (unsigned long) hose-io_base_virt - _IO_BASE;
+ long vbase = (long)hose-io_base_virt;
+ long io_base = _IO_BASE;
+
+ return (resource_size_t)(vbase - io_base);
Why do we want sign extension here?
If we do want it, there are a lot of other places in this file where the
same calculation is done.
-Scott
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Wed, 2012-06-06 at 16:15 -0500, Scott Wood wrote:
On 06/05/2012 10:50 PM, Ben Collins wrote:
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
Where do you see addresses over 32 bits in QEMU's e500-pci, at least
with current mainline QEMU and the mpc8544ds model?
I/O space should be at 0xe100.
I'm also not sure what this has to do with the virtual address returned
by ioremap().
This is due to how we calculate IO offsets on ppc32, see below
Signed-off-by: Ben Collins bcoll...@ubuntu.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
---
arch/powerpc/kernel/pci-common.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 8e78e93..be9ced7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int
mask)
return pci_enable_resources(dev, mask);
}
+/* Before assuming too much here, take care to realize that we need sign
+ * extension from 32-bit pointers to 64-bit resource addresses to work.
+ */
resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
{
- return (unsigned long) hose-io_base_virt - _IO_BASE;
+ long vbase = (long)hose-io_base_virt;
+ long io_base = _IO_BASE;
+
+ return (resource_size_t)(vbase - io_base);
Why do we want sign extension here?
If we do want it, there are a lot of other places in this file where the
same calculation is done.
We should probably as much as possible factor it, but basically what
happens is that to access IO space, we turn:
oub(port)
into
out_8(_IO_BASE + port)
With _IO_BASE being a global.
Now what happens when you have several PHBs ? Well, we make _IO_BASE be
the result of ioremap'ing the IO space window of the first one, minus
the bus address corresponding to the beginning of that window. Then for
each device, we offset devices with the offset calculated above.
Now that means that we can end up with funky arithmetic in a couple of
cases:
- If the bus address of the IO space is larger than the virtual address
returned by ioremap (it's a bit silly to use large IO addresses but it's
technically possible, normally IO windows start at 0 bus-side though).
In fact I wouldn't be surprised if we have various other bugs if IO
windows don't start at 0 (you may want to double check your dts setup
here).
- If the ioremap'ed address of the IO space of another domain is lower
than the ioremap'ed address of the first domain, in which case the
calculation:
host-io_base_virt - _IO_BASE
results in a negative offset.
Thus we need to make sure that this offset is fully sign extended so
that things work properly when applied to a resource_size_t which can be
64-bit.
On ppc64 we do things differently, we have a single linear region that
has all IO spaces and _IO_BASE is the beginning of it so offsets are
never negative, we can do that because we don't care wasting address
space there.
Cheers,
Ben.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Jun 6, 2012, at 5:15 PM, Scott Wood wrote:
On 06/05/2012 10:50 PM, Ben Collins wrote:
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
Where do you see addresses over 32 bits in QEMU's e500-pci, at least
with current mainline QEMU and the mpc8544ds model?
I/O space should be at 0xe100.
The problem is this:
pci_bus :00: root bus resource [io 0xffbed000-0xffbfcfff] (bus address
[0x1-0x1])
Without the fix that I sent, it ends up looking like:
pci_bus :00: root bus resource [io 0xffbed000-0xffbfcfff] (bus address
[0x-0x])
And that's when some devices fail to be assigned valid bar 0's and the kernel
complains because of it.
I'm also not sure what this has to do with the virtual address returned
by ioremap().
Signed-off-by: Ben Collins bcoll...@ubuntu.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
---
arch/powerpc/kernel/pci-common.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 8e78e93..be9ced7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int
mask)
return pci_enable_resources(dev, mask);
}
+/* Before assuming too much here, take care to realize that we need sign
+ * extension from 32-bit pointers to 64-bit resource addresses to work.
+ */
resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
{
-return (unsigned long) hose-io_base_virt - _IO_BASE;
+long vbase = (long)hose-io_base_virt;
+long io_base = _IO_BASE;
+
+return (resource_size_t)(vbase - io_base);
Why do we want sign extension here?
If we do want it, there are a lot of other places in this file where the
same calculation is done.
-Scott
--
Ben Collins
Servergy, Inc.
(757) 243-7557
CONFIDENTIALITY NOTICE: This communication contains privileged and/or
confidential information; and should be maintained with the strictest
confidence. It is intended solely for the use of the person or entity in which
it is addressed. If you are not the intended recipient, you are STRICTLY
PROHIBITED from disclosing, copying, distributing or using any of this
information. If you received this communication in error, please contact the
sender immediately and destroy the material in its entirety, whether electronic
or hard copy.
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On 06/06/2012 05:21 PM, Benjamin Herrenschmidt wrote: Now that means that we can end up with funky arithmetic in a couple of cases: - If the bus address of the IO space is larger than the virtual address returned by ioremap (it's a bit silly to use large IO addresses but it's technically possible, normally IO windows start at 0 bus-side though). In fact I wouldn't be surprised if we have various other bugs if IO windows don't start at 0 (you may want to double check your dts setup here). The dts does show the I/O beginning at bus address zero: ranges = 0x200 0x0 0xc000 0xc000 0x0 0x2000 0x100 0x0 0x0 0xe100 0x0 0x1; - If the ioremap'ed address of the IO space of another domain is lower than the ioremap'ed address of the first domain, in which case the calculation: host-io_base_virt - _IO_BASE results in a negative offset. There should have been only one PCI domain in the QEMU case. -Scott ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] PPC: PCI: Fix pcibios_io_space_offset() so it works for 32-bit ptr/64-bit rsrcs
On Tue, 2012-06-05 at 23:50 -0400, Ben Collins wrote:
The commit introducing pcibios_io_space_offset() was ignoring 32-bit to
64-bit sign extention, which is the case on ppc32 with 64-bit resource
addresses. This only seems to have shown up while running under QEMU for
e500mc target. It may or may be suboptimal that QEMU has an IO base
address 32-bits for the e500-pci implementation, but 1) it's still a
regression and 2) it's more correct to handle things this way.
See Bjorn, we both did end up getting it wrong :-)
Signed-off-by: Ben Collins bcoll...@ubuntu.com
Cc: Benjamin Herrenschmidt b...@kernel.crashing.org
Acked-by: Benjamin Herrenschmidt b...@kernel.crashing.org
arch/powerpc/kernel/pci-common.c |8 +++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/arch/powerpc/kernel/pci-common.c
b/arch/powerpc/kernel/pci-common.c
index 8e78e93..be9ced7 100644
--- a/arch/powerpc/kernel/pci-common.c
+++ b/arch/powerpc/kernel/pci-common.c
@@ -1477,9 +1477,15 @@ int pcibios_enable_device(struct pci_dev *dev, int
mask)
return pci_enable_resources(dev, mask);
}
+/* Before assuming too much here, take care to realize that we need sign
+ * extension from 32-bit pointers to 64-bit resource addresses to work.
+ */
resource_size_t pcibios_io_space_offset(struct pci_controller *hose)
{
- return (unsigned long) hose-io_base_virt - _IO_BASE;
+ long vbase = (long)hose-io_base_virt;
+ long io_base = _IO_BASE;
+
+ return (resource_size_t)(vbase - io_base);
}
static void __devinit pcibios_setup_phb_resources(struct pci_controller
*hose, struct list_head *resources)
___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev
