Re: [PATCH v3] PCI/portdrv: Allow probing even without child services

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Moving `pci_set_master()` after child device registration causes child drivers to probe with Bus Mastering disabled, risking permanently lost MSI/MSI-X interrupts.
--

commit 9b851bf8faf271ab0b2daad51bfd51f89a1e87d4
Author: Brian Norris <briannorris@chromium.org>

PCI/portdrv: Allow probing even without child services

This commit modifies the PCIe port driver to allow probe to succeed even
if no child services are available. This ensures the port can still be
configured for runtime PM and low power modes on systems that only have
certain port services available or lack legacy INTx interrupts.

Link: https://lore.kernel.org/all/nyada24tqwlkzdceyoxbzitzygvp4elvj5oajnqdwb33xkcdwk@76vnrx45fsfd/

> --- a/drivers/pci/pcie/portdrv.c
> +++ b/drivers/pci/pcie/portdrv.c
> @@ -343,7 +343,6 @@ static int pcie_port_device_register(struct pci_dev *dev)
>  	if (!capabilities)
>  		return 0;
>  
> -	pci_set_master(dev);
>  	/*
>  	 * Initialize service irqs. Don't use service devices that
>  	 * require interrupts if there is no way to generate them.

[ ... ]

>  	/* Allocate child services if any */
> -	status = -ENODEV;
> -	nr_service = 0;
>  	for (i = 0; i < PCIE_PORT_DEVICE_MAXSERVICES; i++) {
>  		int service = 1 << i;
>  		if (!(capabilities & service))
>  			continue;
> -		if (!pcie_device_init(dev, service, irqs[i]))
> -			nr_service++;
> +		pcie_device_init(dev, service, irqs[i]);
>  	}
> -	if (!nr_service)
> -		goto error_cleanup_irqs;
>  
> -	return 0;
> +	pci_set_master(dev);

Does moving pci_set_master() after the pcie_device_init() loop risk dropping
MSI/MSI-X interrupts during child driver probes?

The pcie_device_init() function calls device_register(), which synchronously
invokes the probe routines of child service drivers like pciehp or aer.
During probe, these child drivers configure the hardware and enable their
interrupts.

Because PCIe MSI and MSI-X interrupts are memory write transactions, they
require the Bus Master Enable (BME) bit to be set in the PCI Command Register.

If a hardware event occurs immediately after the child driver unmasks the
interrupt but before pci_set_master(dev) is called, will the hardware attempt
to send an MSI and drop the memory write because BME is 0?

Since PCIe interrupts are edge-triggered messages, could these dropped
interrupts result in permanently missed hotplug events or cause hardware error
handling regressions?

> -error_cleanup_irqs:
> -	pci_free_irq_vectors(dev);
> -error_disable:
> -	pci_disable_device(dev);
> -	return status;
> +	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260508133629.v3.1.I5fd5d83f518681b3949d8ab2f16ba8244fd3e774@changeid?part=1