[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v2 1/2] xen-pciback: redo VF placement in the virtual topology


  • To: Jan Beulich <jbeulich@xxxxxxxx>
  • From: Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
  • Date: Thu, 20 May 2021 12:31:46 -0400
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=oracle.com; dmarc=pass action=none header.from=oracle.com; dkim=pass header.d=oracle.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=TdWsbshtvy8CTiEBqnJ9thX/c6XJnD8yNkyF2CmpKsg=; b=kw/J4vZFNiLMMzYHIMV+Jl+C0FJ4gddxtKCFAVUQ7vKcpvMhnoh04aJsk3NnpUnlbv1Vbm2MzwVZT/MQtuX43wmTZ0hzu4fiU5fT2No0a9KYQLlyjI1SruFLH9n+IUQ9dHjxB24uIUO1sGwYojqzx8/g12VubSp2NEYPDvaH0mYstZU9yE0id8IaW8oOKABkX7gDYyFKEmVcDbRlix8jcSBGWi0O+wgKt3FxxJcUincKjD96KCEsq5cr2M8rZPnJ4aNc9+IT3tAxNvHpKEmTEKGszkVNOqHVIJWd8mVXpJLuAg00mfSbI7LO+P0kQztS17iVA2CygdnU53GH4MgUuA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Q3zGizRzFlHexRXcAqMULNgcek0EoWzpOnSIbdWYkp0dBzkOgn8xYb0JARl/uXB+VPnldqdzwRYjimCCZi/kOyrqeloWk2fJM94U2AKEQ1JlysFNH+9VQeHItSA0GN5PTeqlvG0OZdG3hSFXTwLw+b0cGkEehrqyuxDGQWpzzvxpo37mQCkRPgY9ZL61fSOVGT1Ci805PIncTXoaALmbw4f9m0mYgA6ikyTJfZGbn7WTL57w0WPwgG+E98mIlFmGQ2q4GOO/3gwYQaR5SogI0noW6MFTN8GnvX5XVdNn6oaYqLQjcZij0P6T5EJGx4cEmlg87WmOpyWfADWIDusb3A==
  • Authentication-results: lists.xenproject.org; dkim=none (message not signed) header.d=none;lists.xenproject.org; dmarc=none action=none header.from=oracle.com;
  • Cc: Juergen Gross <jgross@xxxxxxxx>, Konrad Wilk <konrad.wilk@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 20 May 2021 16:32:02 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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.ostrovsky@xxxxxxxxxx>




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.