Thread (92 messages) 92 messages, 9 authors, 2014-07-25

[PATCH v8 4/9] pci: OF: Fix the conversion of IO ranges into IO resources.

From: arnd@arndb.de (Arnd Bergmann)
Date: 2014-07-16 14:48:22
Also in: linux-devicetree, linux-pci, lkml

On Wednesday 16 July 2014 09:35:37 Rob Herring wrote:
On Wed, Jul 9, 2014 at 3:31 AM, Arnd Bergmann [off-list ref] wrote:
quoted
On Tuesday 08 July 2014, Liviu Dudau wrote:
quoted
On Mon, Jul 07, 2014 at 10:22:00PM +0100, Arnd Bergmann wrote:
quoted
I looked at the other drivers briefly, and I think you indeed fix the Tegra
driver with this but break the integrator driver as mentioned above.
The other callers of of_pci_range_to_resource() are apparently not
impacted as they recalculate the values they get.
I would argue that integrator version is having broken assumptions. If it would
try to allocate that IO range or request the resource as returned currently by
of_pci_range_to_resource (without my patch) it would fail. I know because I did
the same thing in my host bridge driver and it failed miserably. That's why I
tried to patch it.
The integrator code was just introduced and the reason for how it does things
is the way that of_pci_range_to_resource() works today. We tried to cope with
it and not change the existing behavior in order to not break any other drivers.

It's certainly not fair to call the integrator version broken, it just works
around the common code having a quirky interface. We should probably have
done of_pci_range_to_resource better than it is today (I would have argued
for it to return an IORESOURCE_MEM with the CPU address), but it took long
enough to get that merged and I was sick of arguing about it.
quoted
If the IO space is memory mapped, then we use the port number, the io_offset
and the PCI_IOBASE to get to the virtual address that, when accessed, will
generate the correct addresses on the bus, based on what the host bridge has
been configured.

This is the current level of my understanding of PCI IO.
What is io_offset supposed to be and be based on?
(you probably know most of this, but I'll explain it the long way
to avoid ambiguity).

io_offset is a concept used internally to translate bus-specific I/O port
numbers into Linux-global ports.

A simple example would be having two PCI host bridges each with a
(hardware) port range from 0 to 0xffff. These numbers are programmed
into "BARs" in PCI device config space and they are used on the physical
address lines in PCI or in the packet header on PCIe.

In Linux, we have a single logical port range that is seen by device
drivers, in the example the first host bridge would use ports 0-0xfffff
and the second one would use ports 0x10000-0x1ffff.

The PCI core uses the io_offset to translate between the two address
spaces when it does the resource allocation during bus probe, so a device
that gets Linux I/O port 0x10100 has its BAR programmed with 0x100 and
the struct resource filled as 0x10000.

When a PCI host bridge driver registers its root bus with the PCI core,
it passes the io_offset using the last argument to pci_add_resource_offset()
along with the Linux I/O port resource, so in the example the first
io_offset is zero, while the second one is 0x10000.

Note that there is no requirement for the I/O port range on the bus to
start at zero, and you can even have negative io_offset values to
deal with that, but this is the exception.
quoted
quoted
Now, I believe Rob has switched entirely to using my series in some test that
he has run and he hasn't encountered any issues, as long as one remembers in
the host bridge driver to add the io_base offset to the .start resource. If
not then I need to patch pci_v3.c.
The crazy part of all these discussions is that basically nobody ever uses
I/O port access, so it's very hard to test and we don't even notice when
we get it wrong, but we end up spending most of the time for PCI host controller
reviews trying to get these right.
FWIW, I test i/o accesses with Versatile QEMU. The LSI53xxxx device in
the model has a kconfig option to use i/o accesses. However, I have
seen in the past this is an area where 2 wrongs can make a right.
Can you point me to a git tree with your kernel and dts?

	Arnd
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help