Re: [PATCH v6 3/5] PCI: Move jailhouse's isolated function handling to pci_scan_slot()

[Pierre Morel <pmorel@linux.ibm.com>]

On 6/28/22 16:30, Niklas Schnelle wrote:
> The special case of the jailhouse hypervisor passing through individual
> PCI functions handles scanning for PCI functions even if function 0 does
> not exist. Currently this is done with an extra loop duplicating the one
> in pci_scan_slot(). By incorporating the check for jailhouse_paravirt()
> into pci_scan_slot() we can instead do this as part of the normal
> slot scan. Note that with the assignment of dev->multifunction gated by
> fn > 0 we set dev->multifunction unconditionally for all functions if
> function 0 is missing just as in the existing jailhouse loop.
> 
> The only functional change is that we now call
> pcie_aspm_init_link_state() for these functions but this already
> happened if function 0 was passed through and should not be a problem.
> 
> Cc: Jan Kiszka <jan.kiszka@siemens.com>
> Link: https://lore.kernel.org/linux-pci/20220408224514.GA353445@bhelgaas/
> Suggested-by: Bjorn Helgaas <bhelgaas@google.com>
> Signed-off-by: Niklas Schnelle <schnelle@linux.ibm.com>
> ---
>   drivers/pci/probe.c | 30 ++++++++++--------------------
>   1 file changed, 10 insertions(+), 20 deletions(-)
> 
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index 2c737dce757e..a18e07e6a7df 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2662,8 +2662,13 @@ int pci_scan_slot(struct pci_bus *bus, int devfn)
>   			if (fn > 0)
>   				dev->multifunction = 1;
>   		} else if (fn == 0) {
> -			/* function 0 is required */
> -			break;
> +			/*
> +			 * function 0 is required unless we are running on
> +			 * a hypervisor which passes through individual PCI
> +			 * functions.
> +			 */
> +			if (!jailhouse_paravirt())
> +				break;
>   		}
>   		fn = next_fn(bus, dev, fn);
>   	} while (fn >= 0);
> @@ -2862,29 +2867,14 @@ static unsigned int pci_scan_child_bus_extend(struct pci_bus *bus,
>   {
>   	unsigned int used_buses, normal_bridges = 0, hotplug_bridges = 0;
>   	unsigned int start = bus->busn_res.start;
> -	unsigned int devfn, fn, cmax, max = start;
> +	unsigned int devfn, cmax, max = start;
>   	struct pci_dev *dev;
> -	int nr_devs;
>   
>   	dev_dbg(&bus->dev, "scanning bus\n");
>   
>   	/* Go find them, Rover! */
> -	for (devfn = 0; devfn < 256; devfn += 8) {
> -		nr_devs = pci_scan_slot(bus, devfn);
> -
> -		/*
> -		 * The Jailhouse hypervisor may pass individual functions of a
> -		 * multi-function device to a guest without passing function 0.
> -		 * Look for them as well.
> -		 */
> -		if (jailhouse_paravirt() && nr_devs == 0) {
> -			for (fn = 1; fn < 8; fn++) {
> -				dev = pci_scan_single_device(bus, devfn + fn);
> -				if (dev)
> -					dev->multifunction = 1;
> -			}
> -		}
> -	}
> +	for (devfn = 0; devfn < 256; devfn += 8)
> +		pci_scan_slot(bus, devfn);
>   
>   	/* Reserve buses for SR-IOV capability */
>   	used_buses = pci_iov_bus_range(bus);
> 


Reviewed-by: Pierre Morel <pmorel@linux.ibm.com>

-- 
Pierre Morel
IBM Lab Boeblingen