netdev - Re: [Intel-wired-lan] [PATCH] pci: Use a bus-global mutex to protect VPD operations

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20150519160158.00002cd6@unknown>
Date:	Tue, 19 May 2015 16:01:58 -0700
From:	Jesse Brandeburg <jesse.brandeburg@...el.com>
To:	Alexander Duyck <alexander.h.duyck@...hat.com>
Cc:	Mark D Rustad <mark.d.rustad@...el.com>, <bhelgaas@...gle.com>,
	<linux-pci@...r.kernel.org>, <intel-wired-lan@...ts.osuosl.org>,
	<netdev@...r.kernel.org>, jesse.brandeburg@...el.com
Subject: Re: [Intel-wired-lan] [PATCH] pci: Use a bus-global mutex to
 protect VPD operations

On Tue, 19 May 2015 10:55:03 -0700
Alexander Duyck <alexander.h.duyck@...hat.com> wrote:

> 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.rustad@...el.com>
> > Acked-by: Shannon Nelson <shannon.nelson@...el.com>
> > Acked-by: Jeff Kirsher <jeffrey.t.kirsher@...el.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.

But Alex if you do this you're violating the principle of least
surprise, not to mention changing a user-space interface which should
not be done.

Mark's solution is pretty graceful and solves the issue at heart, which
is that
1) several Intel chips have this issue
2) it appears that several other vendor's chips have this issue (or
similar) as well, but even if they don't Mark's fix will not change
their general operation, only make a small serializing effect when
multiple simultaneous reads are made.

This is a reasonably small fix, with a small kernel footprint, which
does not require changing user expectations or violating user-space
semantics that are already established, so I support it as is.
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html