Re: pci_generic_config_write32() access size problem

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Sep 30, 2015 at 04:53:00PM -0500, Rob Herring wrote:
> On Wed, Sep 30, 2015 at 4:47 PM, Duc Dang <dhdang@xxxxxxx> wrote:
> > On Wed, Sep 30, 2015 at 2:30 PM, Rob Herring <robh@xxxxxxxxxx> wrote:
> >> On Wed, Sep 30, 2015 at 4:17 PM, Duc Dang <dhdang@xxxxxxx> wrote:
> >>> On Tue, Sep 29, 2015 at 9:47 AM, Rob Herring <robh@xxxxxxxxxx> wrote:
> >>>> + several affected driver maintainers
> >>>>
> >>>> On Mon, Sep 28, 2015 at 6:05 PM, Russell King - ARM Linux
> >>>> <linux@xxxxxxxxxxxxxxxx> wrote:
> >>>>> On Mon, Sep 28, 2015 at 05:42:20PM -0500, Rob Herring wrote:
> >>>>>> On Mon, Sep 28, 2015 at 5:08 PM, Bjorn Helgaas <helgaas@xxxxxxxxxx> wrote:
> >>>>>> > Hi Rob,
> >>>>>> >
> >>>>>> > Russell pointed out a problem with 1f94a94f67e1 ("PCI: Add generic config
> >>>>>> > accessors").  pci_generic_config_write32() does a read/modify/write if the
> >>>>>> > size is less than 32 bits, so I think we have problem if this is used with
> >>>>>> > 8- or 16-bit registers that contain RW1C bits.  Any thoughts on how we can
> >>>>>> > fix this?
> >>>>>>
> >>>>>> My series didn't change access sizes unless I made a made a mistake
> >>>>>> somewhere, so the problem should have existed before.
> >>>>>>
> >>>>>> Is it known addresses we need to deal with? We could do special case
> >>>>>> handling of certain addresses to mask out RW1C bits. This could be in
> >>>>>> the generic functions or in wrappers around the generic functions.
> >>>>>> There's already some similar examples of special address handling
> >>>>>> IIRC.
> >>>>>
> >>>>> I think this originally came into being because Intel decided that their
> >>>>> IOP platforms wouldn't support anything except 32-bit accesses, and
> >>>>> cargo cult programming took over.  I complained about it on the Intel IOP
> >>>>> platforms, but obviously if the hardware doesn't support anything else
> >>>>> then your stuck with it.
> >>>>
> >>>> What about SA1100 nanoengine?
> >>>>
> >>>> Others are Tegra, XGene, iproc, and maybe rcar. These are all
> >>>> relatively active platforms, so hopefully some testing can be done.
> >>>
> >>> On X-Gene, so far I don't see anything strange after switching to use
> >>> pci_generic_config_write32 with all the cards that I can test.
> >>
> >> You mean switching to use pci_generic_config_write, right?
> > Hi Rob,
> >
> > I meant even using pci_generic_config_write32 , all the cards that I
> > have right now are still running fine.
> 
> No doubt. I would hope your driver was already tested to some level.
> 
> The question is can you switch to pci_generic_config_write? Is there
> some h/w bug like hanging the bus or garbage data that prevents 8 and
> 16 bit accesses from working?

It seems like at least on Tegra there is a hardware bug that prevents 8
and 16 bit accesses from working. I've tried next-20151001 on Jetson TK1
(I have root on NFS, and the network card is on PCIe) and I see this:

	[    1.535877] tegra-pcie 1003000.pcie-controller: probing port 0, using 2 lanes
	[    1.545099] tegra-pcie 1003000.pcie-controller: Slot present pin change, signature: 00000008
	[    1.985057] tegra-pcie 1003000.pcie-controller: link 0 down, retrying
	[    2.420307] tegra-pcie 1003000.pcie-controller: link 0 down, retrying
	[    2.856198] tegra-pcie 1003000.pcie-controller: link 0 down, retrying
	[    2.864783] tegra-pcie 1003000.pcie-controller: link 0 down, ignoring
	[    2.871296] tegra-pcie 1003000.pcie-controller: probing port 1, using 1 lanes
	[    2.880794] tegra-pcie 1003000.pcie-controller: Slot present pin change, signature: 00000000
	[    2.898441] tegra-pcie 1003000.pcie-controller: PCI host bridge to bus 0000:00
	[    2.905718] pci_bus 0000:00: root bus resource [mem 0x13000000-0x1fffffff]
	[    2.912587] pci_bus 0000:00: root bus resource [mem 0x20000000-0x3fffffff pref]
	[    2.919905] pci_bus 0000:00: root bus resource [bus 00-ff]
	[    2.925386] pci_bus 0000:00: root bus resource [io  0x1000-0xffff]
	[    2.931625] pci 0000:00:02.0: [10de:0e13] type 01 class 0x060400
	[    2.937733] pci 0000:00:02.0: PME# supported from D0 D1 D2 D3hot D3cold
	[    2.945062] PCI: bus0: Fast back to back transfers disabled
	[    2.950649] pci 0000:00:02.0: bridge configuration invalid ([bus 00-00]), reconfiguring
	[    2.959096] pci 0000:01:00.0: [10ec:8168] type 00 class 0x020000
	[    2.965148] pci 0000:01:00.0: reg 0x10: [io  0x0000-0x00ff]
	[    2.970762] pci 0000:01:00.0: reg 0x18: [mem 0x00000000-0x00000fff 64bit]
	[    2.977575] pci 0000:01:00.0: reg 0x20: [mem 0x00000000-0x00003fff 64bit pref]
	[    2.984879] pci 0000:01:00.0: supports D1 D2
	[    2.989162] pci 0000:01:00.0: PME# supported from D0 D1 D2 D3hot D3cold
	[    3.015680] PCI: bus1: Fast back to back transfers disabled
	[    3.021261] pci_bus 0000:01: busn_res: [bus 01-ff] end is updated to 01
	[    3.028012] pci 0000:00:02.0: BAR 8: assigned [mem 0x13000000-0x130fffff]
	[    3.034796] pci 0000:00:02.0: BAR 9: assigned [mem 0x20000000-0x200fffff 64bit pref]
	[    3.042574] pci 0000:00:02.0: BAR 7: assigned [io  0x1000-0x1fff]
	[    3.048715] pci 0000:01:00.0: BAR 4: assigned [mem 0x20000000-0x20003fff 64bit pref]
	[    3.056491] pci 0000:01:00.0: BAR 2: assigned [mem 0x13000000-0x13000fff 64bit]
	[    3.063807] pci 0000:01:00.0: BAR 0: assigned [io  0x1000-0x10ff]
	[    3.070021] pci 0000:00:02.0: PCI bridge to [bus 01]
	[    3.074984] pci 0000:00:02.0:   bridge window [io  0x1000-0x1fff]
	[    3.081093] pci 0000:00:02.0:   bridge window [mem 0x13000000-0x130fffff]
	[    3.087890] pci 0000:00:02.0:   bridge window [mem 0x20000000-0x200fffff 64bit pref]
	[    3.095668] pci 0000:00:02.0: nv_msi_ht_cap_quirk didn't locate host bridge
	[    3.102813] pcieport 0000:00:02.0: enabling device (0140 -> 0143)
	[    3.109455] pcieport 0000:00:02.0: Signaling PME through PCIe PME interrupt
	[    3.116428] pci 0000:01:00.0: Signaling PME through PCIe PME interrupt
	[    3.122950] pcie_pme 0000:00:02.0:pcie01: service driver pcie_pme loaded
	[    3.130024] r8169 Gigabit Ethernet driver 2.3LK-NAPI loaded
	[    3.135670] r8169 0000:01:00.0: enabling device (0140 -> 0143)
	[    3.158326] r8169 0000:01:00.0 eth0: RTL8168g/8111g at 0xf08f6000, 00:04:4b:1e:07:bb, XID 0c000800 IRQ 390
	[    3.168015] r8169 0000:01:00.0 eth0: jumbo features [frames: 9200 bytes, tx checksumming: ko]

The network card is detected and boot continues normally. When I change
the pci_ops to use pci_generic_config_{read,write} the system no longer
boots because the network card never shows up. The kernel log has this:

	[    1.546563] tegra-pcie 1003000.pcie-controller: probing port 0, using 2 lanes
	[    1.556137] tegra-pcie 1003000.pcie-controller: Slot present pin change, signature: 00000008
	[    1.996905] tegra-pcie 1003000.pcie-controller: link 0 down, retrying
	[    2.432731] tegra-pcie 1003000.pcie-controller: link 0 down, retrying
	[    2.868648] tegra-pcie 1003000.pcie-controller: link 0 down, retrying
	[    2.877241] tegra-pcie 1003000.pcie-controller: link 0 down, ignoring
	[    2.883707] tegra-pcie 1003000.pcie-controller: probing port 1, using 1 lanes
	[    2.893239] tegra-pcie 1003000.pcie-controller: Slot present pin change, signature: 00000000
	[    2.908770] tegra-pcie 1003000.pcie-controller: PCI host bridge to bus 0000:00
	[    2.916039] pci_bus 0000:00: root bus resource [mem 0x13000000-0x1fffffff]
	[    2.922906] pci_bus 0000:00: root bus resource [mem 0x20000000-0x3fffffff pref]
	[    2.930225] pci_bus 0000:00: root bus resource [bus 00-ff]
	[    2.935721] pci_bus 0000:00: root bus resource [io  0x1000-0xffff]
	[    2.941935] pci 0000:00:02.0: [10de:0e13] type 00 class 0x060400
	[    2.947958] pci 0000:00:02.0: ignoring class 0x060400 (doesn't match header type 00)
	[    2.956292] PCI: bus0: Fast back to back transfers disabled

So it seems like it's failing to properly read the header type. That's
retrieved from byte 0xe as an 8 bit access. That said, even with 32-bit
accesses I haven't seen any problems, but that might just be because
none of the devices I've tested with have R1WC bits in problematic
places.

Thierry

Attachment: signature.asc
Description: PGP signature


[Index of Archives]     [DMA Engine]     [Linux Coverity]     [Linux USB]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Greybus]

  Powered by Linux