On 5/20/21 10:46 AM, Jan Beulich wrote: > On 20.05.2021 16:44, Jan Beulich wrote: >> On 20.05.2021 16:38, Boris Ostrovsky wrote: >>> On 5/20/21 3:43 AM, Jan Beulich wrote: >>>> On 20.05.2021 02:36, Boris Ostrovsky wrote: >>>>> On 5/18/21 12:13 PM, Jan Beulich wrote: >>>>>> >>>>>> @@ -95,22 +95,25 @@ static int __xen_pcibk_add_pci_dev(struc >>>>>> >>>>>> /* >>>>>> * Keep multi-function devices together on the virtual PCI bus, >>>>>> except >>>>>> - * virtual functions. >>>>>> + * that we want to keep virtual functions at func 0 on their >>>>>> own. They >>>>>> + * aren't multi-function devices and hence their presence at >>>>>> func 0 >>>>>> + * may cause guests to not scan the other functions. >>>>> So your reading of the original commit is that whatever the issue it was, >>>>> only function zero was causing the problem? In other words, you are not >>>>> concerned that pci_scan_slot() may now look at function 1 and skip all >>>>> higher-numbered function (assuming the problem is still there)? >>>> I'm not sure I understand the question: Whether to look at higher numbered >>>> slots is a function of slot 0's multi-function bit alone, aiui. IOW if >>>> slot 1 is being looked at in the first place, slots 2-7 should also be >>>> looked at. >>> >>> Wasn't the original patch describing a problem strictly as one for >>> single-function devices, so the multi-function bit is not set? I.e. if all >>> VFs (which are single-function devices) are placed in the same slot then >>> pci_scan_slot() would only look at function 0 and ignore anything >>> higher-numbered. >>> >>> >>> My question is whether it would "only look at function 0 and ignore >>> anything higher-numbered" or "only look at the lowest-numbered function and >>> ignore anything higher-numbered". >> The common scanning logic is to look at slot 0 first. If that's populated, >> other slots get looked at only if slot 0 has the multi-function bit set. >> If slot 0 is not populated, nothing is known about the other slots, and >> hence they need to be scanned. > In particular Linux'es next_fn() ends with > > /* dev may be NULL for non-contiguous multifunction devices */ > if (!dev || dev->multifunction) > return (fn + 1) % 8; > > return 0;
Ah yes. Reviewed-by: Boris Ostrovsky <boris.ostrov...@oracle.com>
