linux-kernel - Re: [PATCH] PCI: Add sysfs attribute for disabling PCIe link to downstream component

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: <20190821072833.GM19908@lahna.fi.intel.com>
Date:   Wed, 21 Aug 2019 10:28:33 +0300
From:   Mika Westerberg <mika.westerberg@...ux.intel.com>
To:     Bjorn Helgaas <helgaas@...nel.org>
Cc:     Logan Gunthorpe <logang@...tatee.com>, linux-pci@...r.kernel.org,
        Moritz Fischer <mdf@...nel.org>, Wu Hao <hao.wu@...el.com>,
        linux-fpga@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH] PCI: Add sysfs attribute for disabling PCIe link to
 downstream component

On Tue, Aug 20, 2019 at 09:17:17AM -0500, Bjorn Helgaas wrote:
> On Tue, Aug 20, 2019 at 12:58:20PM +0300, Mika Westerberg wrote:
> > On Mon, Aug 19, 2019 at 06:52:45PM -0500, Bjorn Helgaas wrote:
> > > > Right, it looks like we need some sort of flag there anyway.
> > > 
> > > Does this mean you're looking at getting rid of "has_secondary_link",
> > > you think it's impossible, or you think it's not worth trying?
> > 
> > I was of thinking that we need some flag anyway for the downstream port
> > (such as has_secondary_link) that tells us the which side of the port
> > the link is.
> > 
> > > I'm pretty sure we could get rid of it by looking upstream, but I
> > > haven't actually tried it.
> > 
> > So if we are downstream port, look at the parent and if it is also
> > downstream port (or root port) we change the type to upstream port
> > accordingly? That might work.
> 
> If we see a type of PCI_EXP_TYPE_ROOT_PORT or
> PCI_EXP_TYPE_PCIE_BRIDGE, I think we have to assume that's accurate
> (which we already do today -- for those types, we assume the device
> has a secondary link).
> 
> For a device that claims to be PCI_EXP_TYPE_DOWNSTREAM, if a parent
> device exists and is a Downstream Port (Root Port, Switch Downstream
> Port, and I suppose a PCI-to-PCIe bridge (this is basically
> pcie_downstream_port()), this device must actually be acting as a
> PCI_EXP_TYPE_UPSTREAM device.
> 
> If a device claiming to be PCI_EXP_TYPE_UPSTREAM has a parent that is
> PCI_EXP_TYPE_UPSTREAM, this device must actually be a
> PCI_EXP_TYPE_DOWNSTREAM port.
> 
> For PCI_EXP_TYPE_DOWNSTREAM and PCI_EXP_TYPE_UPSTREAM devices that
> don't have parents, we just have to assume they advertise the correct
> type (as we do today).  There are sparc and virtualization configs
> like this.

OK, thanks for the details. I'll try to make patch based on the above.

> > Another option may be to just add a quirk for these ports.
> 
> I don't really like the quirk approach because then we have to rely on
> user reports of something being broken.
> 
> > Only concern for both is that we have functions that rely on the type
> > such as pcie_capability_read_word() so if we change the type do we end
> > up breaking something? I did not check too closely, though.
> 
> I don't think we'll break anything that's not already broken because
> the type will reflect exactly what has_secondary_link now tells us.
> In fact, we might *fix* some things, e.g., pcie_capability_read_word()
> should work better if we fix the type that pcie_downstream_port()
> checks.

Fair enough :)