Re: [PATCH 2/2] vpci: use named rangeset for BARs

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

On 22.11.21 12:43, Jan Beulich wrote:
> On 22.11.2021 11:27, Roger Pau Monné wrote:
>> On Mon, Nov 22, 2021 at 11:28:25AM +0200, Oleksandr Andrushchenko wrote:
>>> --- a/xen/drivers/vpci/header.c
>>> +++ b/xen/drivers/vpci/header.c
>>> @@ -206,12 +206,16 @@ static void defer_map(struct domain *d, struct 
>>> pci_dev *pdev,
>>>   static int modify_bars(const struct pci_dev *pdev, uint16_t cmd, bool 
>>> rom_only)
>>>   {
>>>       struct vpci_header *header = &pdev->vpci->header;
>>> -    struct rangeset *mem = rangeset_new(NULL, NULL, 0);
>>> +    struct rangeset *mem;
>>> +    char str[32];
>>>       struct pci_dev *tmp, *dev = NULL;
>>>       const struct vpci_msix *msix = pdev->vpci->msix;
>>>       unsigned int i;
>>>       int rc;
>>>   
>>> +    snprintf(str, sizeof(str), "%pp", &pdev->sbdf);
>>> +    mem = rangeset_new(NULL, str, RANGESETF_no_print);
>> You are still not adding the rangeset to the domain list, as the first
>> parameter passed here in NULL instead of a domain struct.
>>
>> Given the current short living of the rangesets I'm not sure it makes
>> much sense to link them to the domain ATM, but I guess this is kind of
>> a preparatory change as other patches you have will have the
>> rangesets permanent as long as the device is assigned to a domain.
>>
>> Likely the above reasoning (or the appropriate one) should be added to
>> the commit message.
If I fold then there is no reason to add the comment, right?
> Or, as also suggested as an option, them getting accounted to the domain
> could be folded into the patch making them long-lived.
Ok, I can fold this patch and have:
@@ -95,10 +102,27 @@ int vpci_add_handlers(struct pci_dev *pdev)
      INIT_LIST_HEAD(&pdev->vpci->handlers);
      spin_lock_init(&pdev->vpci->lock);

+    header = &pdev->vpci->header;
+    for ( i = 0; i < ARRAY_SIZE(header->bars); i++ )
+    {
+        struct vpci_bar *bar = &header->bars[i];
+        char str[32];
+
+        snprintf(str, sizeof(str), "%pp:BAR%d", &pdev->sbdf, i);
+        bar->mem = rangeset_new(pdev->domain, str, RANGESETF_no_print);

>
> Jan
>
Thank you,
Oleksandr