Question about PCI I/O space in ARM64

DISCUSSION Kefeng Wang 4 participants 6 messages Mar 23, 2016 – Mar 25, 2016 PCI

Share & tools

Kefeng Wang

author

2016-03-23 03:12 UTC #0 lore raw

Hi all?

If no pci, the PCI I/O space(16M) is mapped into an irrelevant mem space(right ?), not a right IO space, that is, no one call pci_remap_iospace() to remap the memory mapped I/O space, once driver like f71805f loaded, write value to IO space(see f71805f_init->f71805f_find->superio_enter->outb), we met following oops,
------------------------

Unable to handle kernel paging request at virtual address ffffffbffee0002e
pgd = ffffffc1d68d4000
[ffffffbffee0002e] *pgd=0000000000000000, *pud=0000000000000000
Internal error: Oops: 94000046 [#1] PREEMPT SMP
Modules linked in: f71805f(+) hwmon
CPU: 3 PID: 1659 Comm: insmod Not tainted 4.5.0+ #88
Hardware name: linux,dummy-virt (DT)
task: ffffffc1f6665400 ti: ffffffc1d6418000 task.ti: ffffffc1d6418000
PC is at f71805f_find+0x6c/0x358 [f71805f]
------------------------

I am not clear about PCI I/O, but if this is indeed a bug, how to solve this issue, any advice will be appreciated.

Thanks
Kefeng

Catalin Marinas

2016-03-23 10:18 UTC #1 lore raw

Quoted 1 line · Kefeng Wang

If no pci, the PCI I/O space(16M) is mapped into an irrelevant mem space(right ?),

No. It is not mapped at all.

Quoted 5 lines

not a right IO space,
that is, no one call pci_remap_iospace() to remap the memory mapped I/O space, once driver like f71805f loaded, write value to IO space(see f71805f_init->f71805f_find->superio_enter->outb), we met following oops,
------------------------

Quoted 10 lines

Unable to handle kernel paging request at virtual address ffffffbffee0002e
pgd = ffffffc1d68d4000
[ffffffbffee0002e] *pgd=0000000000000000, *pud=0000000000000000
Internal error: Oops: 94000046 [#1] PREEMPT SMP
Modules linked in: f71805f(+) hwmon
CPU: 3 PID: 1659 Comm: insmod Not tainted 4.5.0+ #88
Hardware name: linux,dummy-virt (DT)
task: ffffffc1f6665400 ti: ffffffc1d6418000 task.ti: ffffffc1d6418000
PC is at f71805f_find+0x6c/0x358 [f71805f]
------------------------

That's caused by not having a mapped PCI I/O space.

Quoted 2 lines

I am not clear about PCI I/O, but if this is indeed a bug, how to solve this issue, any advice will be appreciated.

You need a PCI host controller driver (e.g. drivers/pci/host/pci-host-generic.c) and corresponding bindings in DT or ACPI.

Signature · 1 line

Catalin

Arnd Bergmann

2016-03-23 12:26 UTC #2 lore raw

Quoted 21 lines · On Wednesday 23 March 2016 10:18:40 Catalin Marinas wrote:

Quoted 1 line · Kefeng Wang
If no pci, the PCI I/O space(16M) is mapped into an irrelevant mem space(right ?),
No. It is not mapped at all.
Quoted 5 lines
not a right IO space,
that is, no one call pci_remap_iospace() to remap the memory mapped I/O space, once driver like f71805f loaded, write value to IO space(see f71805f_init->f71805f_find->superio_enter->outb), we met following oops,
------------------------
Quoted 10 lines
Unable to handle kernel paging request at virtual address ffffffbffee0002e
pgd = ffffffc1d68d4000
[ffffffbffee0002e] *pgd=0000000000000000, *pud=0000000000000000
Internal error: Oops: 94000046 [#1] PREEMPT SMP
Modules linked in: f71805f(+) hwmon
CPU: 3 PID: 1659 Comm: insmod Not tainted 4.5.0+ #88
Hardware name: linux,dummy-virt (DT)
task: ffffffc1f6665400 ti: ffffffc1d6418000 task.ti: ffffffc1d6418000
PC is at f71805f_find+0x6c/0x358 [f71805f]
------------------------

Quoted 8 lines

That's caused by not having a mapped PCI I/O space.
Quoted 2 lines
I am not clear about PCI I/O, but if this is indeed a bug, how to solve this issue, any advice will be appreciated.
You need a PCI host controller driver (e.g. drivers/pci/host/pci-host-generic.c) and corresponding bindings in DT or ACPI.

I think getting an Oops is not the best behavior though, it would be nice if that could be improved in some way.

Ideally, each driver that accesses PCI I/O space would call request_region() before doing so, and it would be good if that call could be made to return an error when asked about an address that has not been mapped.

I see that ioport_resource gets initialized to the {0, IO_SPACE_LIMIT} range. If we could change it so that pci_remap_iospace() hooks up to ioport_resource and extends it whenever something gets mapped there up to IO_SPACE_LIMIT, we can change the default range to {0,0}, which would fail for any request_region call before the first pci_remap_iospace.

This won't help for the specific f71805f driver example, because that does not call request_region(), but we can treat that as a driver bug and fix it.

Arnd

Kefeng Wang

author

2016-03-24 03:14 UTC #3 lore raw

Quoted 22 lines · Arnd Bergmann

Quoted 21 lines · On Wednesday 23 March 2016 10:18:40 Catalin Marinas wrote:
Quoted 1 line · Kefeng Wang
If no pci, the PCI I/O space(16M) is mapped into an irrelevant mem space(right ?),
No. It is not mapped at all.
Quoted 15 lines
not a right IO space,
that is, no one call pci_remap_iospace() to remap the memory mapped I/O space, once driver like f71805f loaded, write value to IO space(see f71805f_init->f71805f_find->superio_enter->outb), we met following oops,
------------------------
Unable to handle kernel paging request at virtual address ffffffbffee0002e
pgd = ffffffc1d68d4000
[ffffffbffee0002e] *pgd=0000000000000000, *pud=0000000000000000
Internal error: Oops: 94000046 [#1] PREEMPT SMP
Modules linked in: f71805f(+) hwmon
CPU: 3 PID: 1659 Comm: insmod Not tainted 4.5.0+ #88
Hardware name: linux,dummy-virt (DT)
task: ffffffc1f6665400 ti: ffffffc1d6418000 task.ti: ffffffc1d6418000
PC is at f71805f_find+0x6c/0x358 [f71805f]
------------------------

Quoted 1 line

Quoted 1 line
That's caused by not having a mapped PCI I/O space.

Clear.

Quoted 7 lines

Quoted 7 lines
Quoted 3 lines
I am not clear about PCI I/O, but if this is indeed a bug, how to solve this issue, any advice will be appreciated.
You need a PCI host controller driver (e.g. drivers/pci/host/pci-host-generic.c) and corresponding bindings in DT or ACPI.

In our inner test, there are some board without pcie host driver(even without pci host controller).

Quoted 15 lines

I think getting an Oops is not the best behavior though, it would be nice if that could be improved in some way.
Ideally, each driver that accesses PCI I/O space would call request_region() before doing so, and it would be good if that call could be made to return an error when asked about an address that has not been mapped.
I see that ioport_resource gets initialized to the {0, IO_SPACE_LIMIT} range. If we could change it so that pci_remap_iospace() hooks up to ioport_resource and extends it whenever something gets mapped there up to IO_SPACE_LIMIT, we can change the default range to {0,0}, which would fail for any request_region call before the first pci_remap_iospace.

Quoted 3 lines

This won't help for the specific f71805f driver example, because that does not call request_region(), but we can treat that as a driver bug and fix it.

Yes, we met same error with several modules(at least 8+), so it's better to find a good way to avoid it, change modules one by one maybe not a good choice.

Define some arch in/out func instead of generic ops? when in/out vals, check whether or not the pci_iobase is mapped.

Thanks catalin and arnd for your reply.

Quoted 5 lines

Arnd
.

Lorenzo Pieralisi

2016-03-24 17:54 UTC #4 lore raw

[+ Jean]

On Thu, Mar 24, 2016 at 11:14:50AM +0800, Kefeng Wang wrote:

[...]

Quoted 6 lines

Quoted 3 lines
Quoted 3 lines
You need a PCI host controller driver (e.g. drivers/pci/host/pci-host-generic.c) and corresponding bindings in DT or ACPI.
In our inner test, there are some board without pcie host driver(even without pci host controller).

So I guess those drivers are for devices that are attached to an LPC controller that is not part of a PCI host controller, right ?

BTW, what happened to this (not that I particularly like this patchset) ?

https://lkml.org/lkml/2015/12/29/154

Quoted 14 lines

Quoted 14 lines
I think getting an Oops is not the best behavior though, it would be nice if that could be improved in some way.
Ideally, each driver that accesses PCI I/O space would call request_region() before doing so, and it would be good if that call could be made to return an error when asked about an address that has not been mapped.
I see that ioport_resource gets initialized to the {0, IO_SPACE_LIMIT} range. If we could change it so that pci_remap_iospace() hooks up to ioport_resource and extends it whenever something gets mapped there up to IO_SPACE_LIMIT, we can change the default range to {0,0}, which would fail for any request_region call before the first pci_remap_iospace.

Quoted 7 lines

Quoted 3 lines
This won't help for the specific f71805f driver example, because that does not call request_region(), but we can treat that as a driver bug and fix it.
Yes, we met same error with several modules(at least 8+), so it's better to find a good way to avoid it, change modules one by one maybe not a good choice.

But that's the right thing to do :)

The problem with those drivers is that IIUC they use ISA IO port space to detect if the devices they manage are actually present, it does not look that correct to me, at least not on !X86 (IA64 ?) systems.

Those drivers can't be built on PPC for a reason (?), I found these threads which might be telling:

http://www.spinics.net/lists/lm-sensors/msg43868.html
http://comments.gmane.org/gmane.linux.drivers.sensors/17290

I agree with Arnd on this and we need to patch the eg f71805f driver too (or we just do not build on ARM) to make it request the IO port region it needs to actually probe the device, it is not correct to assume IO space is available and mapped, I think that's a driver bug rather than anything else, Jean can certainly shed some light here.

Quoted 2 lines

Define some arch in/out func instead of generic ops? when in/out vals, check whether or not the pci_iobase is mapped.

I do not think the problem should be solved in the IO accessors implementation, see above.

Lorenzo

Kefeng Wang

author

2016-03-25 06:49 UTC #5 lore raw

Quoted 10 lines · Lorenzo Pieralisi

[+ Jean]
On Thu, Mar 24, 2016 at 11:14:50AM +0800, Kefeng Wang wrote:
[...]
Quoted 4 lines
Quoted 3 lines
Quoted 3 lines
You need a PCI host controller driver (e.g. drivers/pci/host/pci-host-generic.c) and corresponding bindings in DT or ACPI.

Quoted 7 lines

Quoted 2 lines
In our inner test, there are some board without pcie host driver(even without pci host controller).
So I guess those drivers are for devices that are attached to an LPC controller that is not part of a PCI host controller, right ?
BTW, what happened to this (not that I particularly like this patchset) ?

They are working on it, and will upstream again.

Quoted 3 lines

https://lkml.org/lkml/2015/12/29/154

[...]

Quoted 6 lines

I agree with Arnd on this and we need to patch the eg f71805f driver too (or we just do not build on ARM) to make it request the IO port region it needs to actually probe the device, it is not correct to assume IO space is available and mapped, I think that's a driver bug rather than anything else, Jean can certainly shed some light here.

Ok, so Jean, would you take a look this? Thanks.

Kefeng

Quoted 11 lines

Quoted 3 lines
Define some arch in/out func instead of generic ops? when in/out vals, check whether or not the pci_iobase is mapped.
I do not think the problem should be solved in the IO accessors implementation, see above.
Lorenzo
.