Re: [PATCH v3 2/2] PCI: imx6: limit DBI register length

[Trent Piepho]

On Tue, 2018-11-20 at 18:19 +0000, Leonard Crestez wrote:
> On Tue, 2018-11-20 at 17:56 +0100, Stefan Agner wrote:
> > Define the length of the DBI registers. This makes sure that
> > the kernel does not access registers beyond that point, avoiding
> > the following abort on a i.MX 6Quad:
> >   # cat
> > /sys/devices/soc0/soc/1ffc000.pcie/pci0000\:00/0000\:00\:00.0/config
> >   [  100.021433] Unhandled fault: imprecise external abort (0x1406)
> > at 0xb6ea7000
> >   ...
> >   [  100.056423] PC is at dw_pcie_read+0x50/0x84
> >   [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
> 
> I don't know exactly where this limitation comes from, I can indeed
> reproduce a stack dump when dumping pci config from /sys/
> 
> Unfortunately this seems to block access to registers used for
> functionality like interrupts. For example dw_handle_msi_irq does:
> 
> 	dw_pcie_rd_own_conf(pp, PCIE_MSI_INTR0_STATUS +
> 			    (i * MSI_REG_CTRL_BLOCK_SIZE),
> 			    4, &val);
> 
> where PCI_MSI_INTR0_STATUS is 0x830. There are more accesses like this.
> 
> Testing on 6dl-sabreauto (dts change required) with an ath9k pcie card
> with your series I sometimes get "irq 295: nobody cared" on boot. Maybe
> I'm missing something?

On IMX7d, there are significant blocks of 00s in the config space, and
all 0xff at 0xb50 on up.

I.e., significant portions are empty, in the middle of the config
space, not just at the end.

But they can be read without problem.

Perhaps imx6q aborts on a read of an unimplemented address instead of
returning zeros like imx7d.  In that case it really needs something
more complex to prevent abort than just a length.

It also seems to me that this doesn't need to be in the internal pci
config access functions.  The driver shouldn't be reading registers
that don't exist anyway.  It's really about trying to fix sysfs access
to registers that don't exist.  So maybe it should be done there.