Re: pci_generic_config_write32() access size problem

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

 



+ 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.

Rob

> However, what's utterly wrong is that the generic PCI layer should give
> credence to this crap - it should _not_ provide a generic helper for
> performing only 32-bit accesses which is non-conformant to the PCI specs,
> at least without a big fat warning about it.
>
> The problem is not limited to what the PCI specs say - remember, vendors
> are free to add their own PCI configuration registers: you don't know
> where these registers are in the PCI config address space.  What's even
> worse is that you don't know what use vendors have put the neighbouring
> bytes of PCI config space to in the case of an 8-bit or 16-bit vendor
> register.
>
> For the standard PCI config registers, some addresses are well known.
> For example, the PCI status register is at fixed address 6, sharing the
> PCI command register at address 4.
>
> Others depend on the function, and whether there's capabilities present.
> For example, the PCIe device status register shares the same double-word
> as the PCI device control register, and the status register contains RW1C
> bits.
>
> The same is true for the PCIe AER capability.
>
> --
> FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
> according to speedtest.net.
--
To unsubscribe from this list: send the line "unsubscribe linux-pci" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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