Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <1542212438.23489.6.camel@pengutronix.de>
Subject: Re: [PATCH] PCI: dwc: Limit config space size for i.MX6
From:   Lucas Stach <l.stach@pengutronix.de>
To:     Stefan Agner <stefan@agner.ch>, jingoohan1@gmail.com,
        gustavo.pimentel@synopsys.com
Cc:     bhelgaas@google.com, linux-pci@vger.kernel.org,
        linux-kernel@vger.kernel.org
Date:   Wed, 14 Nov 2018 17:20:38 +0100
In-Reply-To: <40d0c27d612534033d146497029b1922@agner.ch>
References: <20181019111350.6170-1-stefan@agner.ch>
         <40d0c27d612534033d146497029b1922@agner.ch>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

Am Mittwoch, den 14.11.2018, 16:49 +0100 schrieb Stefan Agner:
> On 19.10.2018 13:13, Stefan Agner wrote:
> > Reading the full 4k config space through sysfs leads to an
> > external abort. Testing on a platform showed that the upper
> > limit is 512. Limit config space to 512.
> 
> Any comment on this patch?
> 
> Since other devices use similar quirks, I guess the fix can't be far
> off?
> 
> Maybe restricting to the PCI device ID used in i.MX 6 only is too
> restrictive, but I guess better restrictive for now?

I don't think we need a quirk here, but dw_pcie_rd_own_conf() should
really check that the config read isn't targeting registers that are
not implemented in the core.

According to the i.MX6 docs (PCIE_RC memory map) the last implemented
register is at 0x158.

Regards,
Lucas

> --
> Stefan
> 
> > 
> > > > Signed-off-by: Stefan Agner <stefan@agner.ch>
> > ---
> > I observed this on a Apalis iMX6 which uses the i.MX 6Quad without any
> > PCIe device connected. It is especially annoying since it breaks by
> > simply reading a file as root in sysfs (e.g. grepping through sysfs)...
> > 
> > I am not very familiar with PCIe, so there might be better/more generic
> > ways to fix this?
> > 
> > # cat /sys/devices/soc0/soc/1ffc000.pcie/pci0000\:00/0000\:00\:00.0/config
> > [  100.015997] pci_read_config, size 4096
> > [  100.021433] Unhandled fault: imprecise external abort (0x1406) at 0xb6ea7000
> > [  100.028985] pgd = 1a90ef4f
> > [  100.031780] [b6ea7000] *pgd=4a039831
> > [  100.035475] Internal error: : 1406 [#1] SMP ARM
> > [  100.040094] Modules linked in:
> > [  100.043230] CPU: 1 PID: 605 Comm: cat Not tainted 4.19.0-rc8+ #366
> > [  100.049626] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree)
> > [  100.056423] PC is at dw_pcie_read+0x50/0x84
> > [  100.060790] LR is at dw_pcie_rd_own_conf+0x44/0x48
> > [  100.065779] pc : [<c05bd600>]    lr : [<c05be248>]    psr: 60080093
> > [  100.072274] sp : c3f77d40  ip : c3f77d50  fp : c3f77d4c
> > [  100.077693] r10: 00000000  r9 : 00000004  r8 : c1708908
> > [  100.083122] r7 : c3f77dc8  r6 : 00000200  r5 : 00000004  r4 : 00000000
> > [  100.089895] r3 : 00000000  r2 : c3f77dc8  r1 : 00000000  r0 : f09b4200
> > [  100.096657] Flags: nZCv  IRQs off  FIQs on  Mode SVC_32  ISA ARM 
> > Segment none
> > [  100.104152] Control: 10c5387d  Table: 13fb404a  DAC: 00000051
> > [  100.110114] Process cat (pid: 605, stack limit = 0xcd2180c8)
> > [  100.115994] Stack: (0xc3f77d40 to 0xc3f78000)
> > [  100.120533] 7d40: c3f77d6c c3f77d50 c05be248 c05bd5bc eceede28
> > ecef7800 00000000 00000200
> > [  100.129025] 7d60: c3f77dbc c3f77d70 c05be2c4 c05be210 c05956dc
> > 00000200 c3f77e08 c1708908
> > [  100.137504] 7d80: c17665c4 c05956dc c3f77dbc 00000000 c0d27974
> > c05be24c c1708908 00000200
> > [  100.145980] 7da0: c3f77e08 c1708908 ec276000 ec276200 c3f77dec
> > c3f77dc0 c059570c c05be258
> > [  100.160586] 7dc0: c3f77dc8 00000000 ffffffff e5e423ab ec5c1800
> > ec277000 00001000 13d8a000
> > [  100.175434] 7de0: c3f77e3c c3f77df0 c05a4a5c c05956a8 c019b890
> > 000003ff 00000001 00000000
> > [  100.190328] 7e00: 00000000 00000000 00000000 e5e423ab ffffe000
> > c05a4944 00000000 00000000
> > [  100.205490] 7e20: 00000000 00000000 00000000 c38f5790 c3f77e6c
> > c3f77e40 c038022c c05a4950
> > [  100.220890] 7e40: 00000000 00000000 00001000 c3f77e58 c38f5780
> > 00001000 c3f77f58 b6ea8000
> > [  100.236386] 7e60: c3f77eac c3f77e70 c037fae0 c03801d8 00000000
> > 00000000 c3f77e9c ec276000
> > [  100.252114] 7e80: c0194a18 c1708908 c037fa28 c3fb83c0 c3f77f58
> > 00000000 b6ea8000 00020000
> > [  100.268039] 7ea0: c3f77f24 c3f77eb0 c02efcc4 c037fa34 00000022
> > 00000000 00000000 00000000
> > [  100.284170] 7ec0: 00000800 00000000 b6ea7000 c170eec8 00000817
> > c1708908 c0119574 b6ea7000
> > [  100.300390] 7ee0: c3f77fb0 00000000 c3f77fac c3f77ef8 c0119ac8
> > c0119580 c3f77f5c e5e423ab
> > [  100.316721] 7f00: 00020000 c3fb83c0 b6ea8000 c3f77f58 00000000
> > b6ea8000 c3f77f54 c3f77f28
> > [  100.333196] 7f20: c02efe74 c02efc94 c010e5c4 c010cc7c c3fb83c0
> > c3fb83c0 00000000 00000000
> > [  100.349704] 7f40: c1708908 b6ea8000 c3f77f94 c3f77f58 c02f0388
> > c02efdf4 00000000 00000000
> > [  100.366197] 7f60: c16eedd8 e5e423ab c1708d64 0000006c 7ff00000
> > 00000000 00000003 c01011e4
> > [  100.382693] 7f80: c3f76000 00000003 c3f77fa4 c3f77f98 c02f03fc
> > c02f0344 00000000 c3f77fa8
> > [  100.399206] 7fa0: c0101000 c02f03f8 0000006c 7ff00000 00000003
> > b6ea8000 00020000 00000000
> > [  100.415718] 7fc0: 0000006c 7ff00000 00000000 00000003 00000003
> > 00000000 00020000 00000000
> > [  100.432222] 7fe0: 00000003 bef56ab8 b6f53d57 b6ee06c6 80080030
> > 00000003 00000000 00000000
> > [  100.448703] Backtrace:
> > [  100.455228] [<c05bd5b0>] (dw_pcie_read) from [<c05be248>]
> > (dw_pcie_rd_own_conf+0x44/0x48)
> > [  100.471541] [<c05be204>] (dw_pcie_rd_own_conf) from [<c05be2c4>]
> > (dw_pcie_rd_conf+0x78/0x1c8)
> > [  100.488164]  r7:00000200 r6:00000000 r5:ecef7800 r4:eceede28
> > [  100.497930] [<c05be24c>] (dw_pcie_rd_conf) from [<c059570c>]
> > (pci_user_read_config_dword+0x70/0xec)
> > [  100.515006]  r10:ec276200 r9:ec276000 r8:c1708908 r7:c3f77e08
> > r6:00000200 r5:c1708908
> > [  100.530829]  r4:c05be24c
> > [  100.537271] [<c059569c>] (pci_user_read_config_dword) from
> > [<c05a4a5c>] (pci_read_config+0x118/0x2cc)
> > [  100.554299]  r7:13d8a000 r6:00001000 r5:ec277000 r4:ec5c1800
> > [  100.563911] [<c05a4944>] (pci_read_config) from [<c038022c>]
> > (sysfs_kf_bin_read+0x60/0xac)
> > [  100.579880]  r10:c38f5790 r9:00000000 r8:00000000 r7:00000000
> > r6:00000000 r5:00000000
> > [  100.595383]  r4:c05a4944
> > [  100.601650] [<c03801cc>] (sysfs_kf_bin_read) from [<c037fae0>]
> > (kernfs_fop_read+0xb8/0x200)
> > [  100.617481]  r7:b6ea8000 r6:c3f77f58 r5:00001000 r4:c38f5780
> > [  100.626926] [<c037fa28>] (kernfs_fop_read) from [<c02efcc4>]
> > (__vfs_read+0x3c/0x160)
> > [  100.642031]  r10:00020000 r9:b6ea8000 r8:00000000 r7:c3f77f58
> > r6:c3fb83c0 r5:c037fa28
> > [  100.657201]  r4:c1708908
> > [  100.663320] [<c02efc88>] (__vfs_read) from [<c02efe74>] (vfs_read+0x8c/0x114)
> > [  100.674165]  r9:b6ea8000 r8:00000000 r7:c3f77f58 r6:b6ea8000
> > r5:c3fb83c0 r4:00020000
> > [  100.689131] [<c02efde8>] (vfs_read) from [<c02f0388>] (ksys_read+0x50/0xb4)
> > [  100.699876]  r9:b6ea8000 r8:c1708908 r7:00000000 r6:00000000
> > r5:c3fb83c0 r4:c3fb83c0
> > [  100.714919] [<c02f0338>] (ksys_read) from [<c02f03fc>] (sys_read+0x10/0x14)
> > [  100.725650]  r10:00000003 r9:c3f76000 r8:c01011e4 r7:00000003
> > r6:00000000 r5:7ff00000
> > [  100.740784]  r4:0000006c
> > [  100.746897] [<c02f03ec>] (sys_read) from [<c0101000>]
> > (ret_fast_syscall+0x0/0x28)
> > [  100.761495] Exception stack(0xc3f77fa8 to 0xc3f77ff0)
> > [  100.770224] 7fa0:                   0000006c 7ff00000 00000003
> > b6ea8000 00020000 00000000
> > [  100.785624] 7fc0: 0000006c 7ff00000 00000000 00000003 00000003
> > 00000000 00020000 00000000
> > [  100.801074] 7fe0: 00000003 bef56ab8 b6f53d57 b6ee06c6
> > [  100.809800] Code: e5821000 e89da800 e5901000 ee073f9a (e5821000)
> > [  100.819571] ---[ end trace 1be350dbae42cdaf ]---
> > 
> > --
> > Stefan
> > 
> >  drivers/pci/quirks.c    | 10 ++++++++++
> >  include/linux/pci_ids.h |  1 +
> >  2 files changed, 11 insertions(+)
> > 
> > diff --git a/drivers/pci/quirks.c b/drivers/pci/quirks.c
> > index 6bc27b7fd452..24d8d1d614e5 100644
> > --- a/drivers/pci/quirks.c
> > +++ b/drivers/pci/quirks.c
> > @@ -471,6 +471,16 @@
> > > > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NETRONOME,	PCI_DEVICE_ID_NETRONOME_NFP600
> > 
> > > > > > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NETRONOME,	PCI_DEVICE_ID_NETRONOME_NFP5000,	quirk_nfp6000);
> > 
> > > > > > DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_NETRONOME,	PCI_DEVICE_ID_NETRONOME_NFP6000_VF,	quirk_nfp6000);
> >  
> > +/*
> > + * This PCIe controller causes external abort if config addresses above 0x200
> > + * are read.
> > + */
> > +static void quirk_dw_imx6(struct pci_dev *dev)
> > +{
> > > > +	dev->cfg_size = 0x200;
> > +}
> > > > > > +DECLARE_PCI_FIXUP_HEADER(PCI_VENDOR_ID_SYNOPSYS,	PCI_DEVICE_ID_SYNOPSYS_IMX6,	quirk_dw_imx6);
> > +
> >  /*  On IBM Crocodile ipr SAS adapters, expand BAR to system page size */
> >  static void quirk_extend_bar_to_page(struct pci_dev *dev)
> >  {
> > diff --git a/include/linux/pci_ids.h b/include/linux/pci_ids.h
> > index d157983b84cf..cfd43468f983 100644
> > --- a/include/linux/pci_ids.h
> > +++ b/include/linux/pci_ids.h
> > @@ -2354,6 +2354,7 @@
> > > >  #define PCI_DEVICE_ID_CENATEK_IDE	0x0001
> >  
> > > >  #define PCI_VENDOR_ID_SYNOPSYS		0x16c3
> > > > +#define PCI_DEVICE_ID_SYNOPSYS_IMX6	0xabcd
> >  
> > > >  #define PCI_VENDOR_ID_VITESSE		0x1725
> > > >  #define PCI_DEVICE_ID_VITESSE_VSC7174	0x7174