Re: [PATCH v21 1/4] PCI: Allow per function PCI slots to fix slot reset on s390

[Bjorn Helgaas <helgaas@kernel.org>]

On Thu, Jul 16, 2026 at 11:15:33AM -0700, Farhan Ali wrote:
> 
> On 7/15/2026 4:34 PM, Bjorn Helgaas wrote:
> > > On s390 systems, which use a machine level hypervisor, PCI devices are
> > > always accessed through a form of PCI pass-through which fundamentally
> > > operates on a per PCI function granularity. This is also reflected in the
> > > s390 PCI hotplug driver which creates hotplug slots for individual PCI
> > > functions. Its reset_slot() function, which is a wrapper for
> > > zpci_hot_reset_device(), thus also resets individual functions.
> > > 
> > > Currently, the kernel's PCI_SLOT() macro assigns the same pci_slot object
> > > to multifunction devices.
> > PCI_SLOT() doesn't assign pci_slot objects; I guess they're assigned
> > by some code that*uses* PCI_SLOT().  Since this says "currently," I
> > assume you're changing that code, so we should mention where it is to
> > help readers out.
> 
> Thanks for your response! I can re-word the commit message, how about
> something like this:
> 
> Currently, the pci_create_slot() assigns the same pci_slot object to
> multifunction devices.
> 
> > I see some Sashiko comments; those also need to be addressed or
> > explained away.
> 
> Regarding Sashiko's comments for this patch, it mentions 2 issues:
> 
> New issues: - [High] Unconditional enablement of `per_func_slot` on S390
> breaks standard PCI hotplug (e.g., pciehp, shpchp) slot matching and resets.
> 
> I believe this is not applicable as on s390 we don't support any other PCI
> hotplug drivers given the unique nature of zPCI architecture.

Makes sense.

> Pre-existing issues:
> - [High] Lockless access to `dev->slot` in `pci_dev_reset_slot_function` can
> lead to Use-After-Free if a hotplug driver is concurrently unbound.
> 
> Sashiko identified this as a pre-existing issue, so I don't think should be
> addressed with this patch.

Right, we don't need to fix pre-existing issues in this series, but I
meant there were Sashiko comments on other patches in this series that
look like they *should* be addressed, e.g.,

  [PATCH v21 2/4] PCI: Avoid saving config space state if inaccessible

    - [High] pci_dev_save_and_disable() skips disabling the device if
      config space is momentarily inaccessible, potentially leaving
      DMA and interrupts enabled.
    - [Low] String literal passed to a non-const `char *` pointer in
      `pci_dev_config_accessible()`.

    https://lore.kernel.org/all/20260630170754.093021F00A3A@smtp.kernel.org

  [PATCH v21 3/4] PCI: Fail FLR when config space is inaccessible

    - [High] Un-ratelimited pci_warn() in pci_dev_config_accessible()
      allows an attacker to flood the host kernel log.
    - [High] Early bailout in pcie_flr() when config space is
      inaccessible skips the Function Level Reset and the subsequent
      wait, allowing device assignment to continue without reset,
      leaking state.

    https://lore.kernel.org/all/20260630171310.F25D41F000E9@smtp.kernel.org

  [PATCH v21 4/4] PCI/MSI: Enable memory decoding before restoring MSI-X messages

    - [Medium] Blindly restoring a stale PCI_COMMAND value introduces
      a TOCTOU race window that can erase concurrent modifications to
      the register.

    https://lore.kernel.org/all/20260630171255.20BD91F000E9@smtp.kernel.org

Annoyingly, on v22, Sashiko complained about *different* things and
didn't complain about some of these even though the v22 patches are
identical to v21.  Sigh.