[v1,0/2] PCI: hotplug: Add checks to avoid doing hotplug on PCIe Upstream Ports

Message

Rafael J. Wysocki Nov. 21, 2022, 6:13 p.m. UTC

Hi All,

PCIe Upstream Ports are not hotplug-capable by definition, but it turns out
that in some cases, if the system is configured in a particularly interesting
way, the kernel may be made attempt to operate an Upstream Port as a hotplug
one which causes functional issues to appear.

The following 2 patches amend the code to prevent this behavior from occurring.

Thanks!

Comments

Lukas Wunner Nov. 22, 2022, 8:04 a.m. UTC | #1

On Mon, Nov 21, 2022 at 07:13:15PM +0100, Rafael J. Wysocki wrote:
> PCIe Upstream Ports are not hotplug-capable by definition, but it turns out
> that in some cases, if the system is configured in a particularly interesting
> way, the kernel may be made attempt to operate an Upstream Port as a hotplug
> one which causes functional issues to appear.
> 
> The following 2 patches amend the code to prevent this behavior from occurring.

Both patches LGTM.

The spec reference for this change is PCIe r6.0.1 sec 7.5.3.2:

The Slot Implemented bit in the PCI Express Capabilities register
is only valid for Downstream Ports and undefined on Upstream Ports.

The Slot Capabilities / Control / Status registers are only operable
if the Slot Implemented bit is valid and set.  PCIe hotplug depends
on those registers.

(pcie_capability_reg_implemented() in drivers/pci/access.c returns false
for the Slot Capabilities / Control / Status registers unless the port
is a Root or Downstream Port.  Reads of those registers thus always
return 0.)

(Root Ports are Downstream Ports per the definition of "Downstream"
on page 94 of the PCIe r6.0.1 Base Spec.)

Thanks,

Lukas

Bjorn Helgaas Nov. 22, 2022, 6:06 p.m. UTC | #2

On Mon, Nov 21, 2022 at 07:13:15PM +0100, Rafael J. Wysocki wrote:
> Hi All,
> 
> PCIe Upstream Ports are not hotplug-capable by definition, but it turns out
> that in some cases, if the system is configured in a particularly interesting
> way, the kernel may be made attempt to operate an Upstream Port as a hotplug
> one which causes functional issues to appear.
> 
> The following 2 patches amend the code to prevent this behavior from occurring.

Thanks, applied to pci/hotplug for v6.2.  Lukas, I didn't presume to
convert your LGTM to Reviewed-by, but would be happy add it.

Bjorn

Lukas Wunner Nov. 22, 2022, 6:34 p.m. UTC | #3

On Tue, Nov 22, 2022 at 12:06:03PM -0600, Bjorn Helgaas wrote:
> On Mon, Nov 21, 2022 at 07:13:15PM +0100, Rafael J. Wysocki wrote:
> > PCIe Upstream Ports are not hotplug-capable by definition, but it turns out
> > that in some cases, if the system is configured in a particularly interesting
> > way, the kernel may be made attempt to operate an Upstream Port as a hotplug
> > one which causes functional issues to appear.
> > 
> > The following 2 patches amend the code to prevent this behavior from occurring.
> 
> Thanks, applied to pci/hotplug for v6.2.  Lukas, I didn't presume to
> convert your LGTM to Reviewed-by, but would be happy add it.

I figured that having both a Suggested-by and a Reviewed-by might
look odd, hence went with the more neutral LGTM.  But I see it was
ambiguous.  Either way is fine for me. :)

Thanks,

Lukas