Re: [Intel-wired-lan] [PATCH] pci: Use a bus-global mutex to protect VPD operations
On May 19, 2015, at 6:02 PM, Alexander Duyck alexander.h.du...@redhat.com wrote: My suspicion is that we have a number of bugs floating around out there like the Broadcom issue. Specifically, one of the ones I found was that the r8169 seems to have a similar issue as called out in the email thread at http://permalink.gmane.org/gmane.linux.network/232260. I'm wondering if we shouldn't add an initializer for the read/write functions that will go through and pull out the 3 or 4 headers from the VPD data needed to get the actual length. Then it would lock down the VPD and save some serious time on reads since most devices don't have 32K of VPD to read. That is interesting. I noticed that there are functions already present to find VPD tags. If the VPD were invalid, would this block its being read at all, or would it default to allow reading/writing anything? I don't know if there might be people using Linux to completely write the VPD area. Presumably your idea would prevent rewriting the VPD area to something larger. -- Mark Rustad, Networking Division, Intel Corporation signature.asc Description: Message signed with OpenPGP using GPGMail
Re: [Intel-wired-lan] [PATCH] pci: Use a bus-global mutex to protect VPD operations
On 05/20/2015 09:00 AM, Rustad, Mark D wrote: On May 19, 2015, at 6:02 PM, Alexander Duyck alexander.h.du...@redhat.com wrote: My suspicion is that we have a number of bugs floating around out there like the Broadcom issue. Specifically, one of the ones I found was that the r8169 seems to have a similar issue as called out in the email thread at http://permalink.gmane.org/gmane.linux.network/232260. I'm wondering if we shouldn't add an initializer for the read/write functions that will go through and pull out the 3 or 4 headers from the VPD data needed to get the actual length. Then it would lock down the VPD and save some serious time on reads since most devices don't have 32K of VPD to read. That is interesting. I noticed that there are functions already present to find VPD tags. If the VPD were invalid, would this block its being read at all, or would it default to allow reading/writing anything? I don't know if there might be people using Linux to completely write the VPD area. Presumably your idea would prevent rewriting the VPD area to something larger. What we probably would need to do is split the vpd read/write functions up a bit further as it turns out some vendors are using it as a means of reading/writing the EEPROM for the device. So we could have something like maybe a _raw version of the read/write and one that is intended for actually reading VPD. The VPD one could call something to initialize a set of offsets for the read-only descriptor, the read-write descriptor, and the end descriptor. If any read/write goes past the end descriptor you could then just return 0 for the read value or skip it for the write. By my math that means only having to read at most 6 locations in order to fill in all the descriptor info and then you could save significant time on VPD read for all drivers because would would cut the 32K read down to something like 256 bytes which is the more common VPD size. - Alex -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Intel-wired-lan] [PATCH] pci: Use a bus-global mutex to protect VPD operations
On 05/18/2015 05:00 PM, Mark D Rustad wrote: Some devices have a problem with concurrent VPD access to different functions of the same physical device, so move the protecting mutex from the pci_vpd structure to the pci_bus structure. There are a number of reports on support sites for a variety of devices from various vendors getting the vpd r/w failed message. This is likely to at least fix some of them. Thanks to Shannon Nelson for helping to come up with this approach. Signed-off-by: Mark Rustad mark.d.rus...@intel.com Acked-by: Shannon Nelson shannon.nel...@intel.com Acked-by: Jeff Kirsher jeffrey.t.kirs...@intel.com Instead of moving the mutex lock around you would be much better served by simply removing the duplicate VPD entries for a given device in a PCIe quirk. Then you can save yourself the extra pain and effort of having to deal with serialized VPD accesses for a multifunction device. The logic for the quirk should be fairly simple. 1. Scan for any other devices with VPD that share the same bus and device number. 2. If bdf is equal to us keep searching. 3. If bdf is less than our bdf we release our VPD area and set VPD pointer to NULL. - Alex -- To unsubscribe from this list: send the line unsubscribe netdev in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
