Re: [PATCH v3 3/4] PCI/portdrv: Loose the condition check for disabling hotplug interrupts

[Feng Tang <feng.tang@linux.alibaba.com>]

Hi Lukas,

On Tue, Feb 25, 2025 at 05:09:11AM +0100, Lukas Wunner wrote:
> On Mon, Feb 24, 2025 at 11:44:59AM +0800, Feng Tang wrote:
> > Currently when 'pcie_ports_native' and host's 'native_pcie_hotplug' are
> > both false, kernel will not disable PCIe hotplug interrupts. But as
> > those could be affected by software setup like kernel cmdline parameter,
> > remove the depency over them.
> [...]
> > --- a/drivers/pci/pcie/portdrv.c
> > +++ b/drivers/pci/pcie/portdrv.c
> > @@ -263,9 +263,9 @@ static int get_port_device_capability(struct pci_dev *dev)
> >  
> >  	if (dev->is_hotplug_bridge &&
> >  	    (pci_pcie_type(dev) == PCI_EXP_TYPE_ROOT_PORT ||
> > -	     pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM) &&
> > -	    (pcie_ports_native || host->native_pcie_hotplug)) {
> > -		services |= PCIE_PORT_SERVICE_HP;
> > +	     pci_pcie_type(dev) == PCI_EXP_TYPE_DOWNSTREAM)) {
> > +		if  (pcie_ports_native || host->native_pcie_hotplug)
> > +			services |= PCIE_PORT_SERVICE_HP;
> >  
> >  		/*
> >  		 * Disable hot-plug interrupts in case they have been enabled
> 
> If the platform doesn't grant hotplug control to OSPM, OSPM isn't allowed
> to disable interrupts behind the platform's back.
> 
> So this change doesn't seem safe and we should focus on finding out
> why the platform isn't granting hotplug control in the pci=nomsi case.

Yes. As discussed in https://lore.kernel.org/lkml/Z6ycYOKUeOECrcgb@U-2FWC9VHC-2323.local/
the last state is:
"
I tried to remove OSC_PCI_MSI_SUPPORT from ACPI_PCIE_REQ_SUPPORT, but
after negotiate_os_control(), the 'PCIeHotplug' control is still
disabled in the control capability after ACPI query_osc, run_osc
routines (I haven't figured out why yet), thus the pciehp severvice
driver can't be loader.
"

After talking with some firmware developer, the root cause is the
parameter OS passed to OSC control shows it doesn't support MSI, then
firmware doesn't grant hotplug control. The hardware here is a ARM
server. As different OEM/vendor (X86/ARM) may have different
implementaions of firmware, other firmware may make different decision. 

Thanks,
Feng