On Thu, Oct 01, 2015 at 09:46:59AM +0200, Thierry Reding wrote: > 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. As I've explained, the registers which are most likely to be affected in PCIe are: - PCI status register - PCIe device status register - PCIe error status(es) and possibly others. Some of these registers are accessed by the PCIe AER driver. Here's the problem: how do you know whether the PCIe on your board is operating correctly if your PCI host driver goes around writing '1's to the RW1C bits in these status registers, thereby clearing the error status(es)? It may appear to be working correctly, but generating a number of correctable errors. Also, as I've pointed out, how do you know whether 8-bit or 16-bit registers neighbouring the register you're accessing are safe to be written, especially those in the space above 0x40, where vendors are allowed to place their own vendor specific registers - this is the bigger problem. Yes, you can get away with saying "the cards I have in front of me work today" but as long as the bus doesn't support 8-bit or 16-bit accesses, you're at the mercy of these issues, because what you have is an implementation which doesn't conform to the PCI specifications. The point that I initially raised was that (IMHO) generic PCI code should not be making it easy to make "mistakes" like always accessing config space via 32-bit accesses, at least not without taking some counter-measures against it - even at a basic level of detecting a write to the PCI command register and zeroing the upper 16-bits of the 32-bit quantity it's about to write back. Maybe also looking up the PCI device, and then its capabilities, and doing a similar thing for the PCIe status registers containing RW1C bits. Right now, it's all too easy to just plug the 32-bit accessors in without thinking, and without even checking or testing whether the PCI conformant accessors should be used. It seems that the Marvell platforms have suffered from something like this: the later hardware definitely can (it's specified to) but it's not mentioned whether or not they're supported in older hardware - but practical tests have shown that the 8-bit and 16-bit accesses are supported there too. However, the driver opted to just use the 32-bit accessors anyway. I'd hope that having the code more complex behind the 32-bit accessors to at least try a little harder to avoid clearing these RW1C bits will be enough to make people think twice about using them - especially if they have to walk the list of PCI devices on the bus to try and find the appropriate PCI device struct corresponding to the requested devfn. Those who are dealing with non-conformant hardware then get a slight performance impact when accessing PCI configuration space, which I think is entirely justified given that it _is_ non-conformant hardware. -- 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
