Re: [PATCH v5 07/14] vpci/header: handle p2m range sets per BAR

[Oleksandr Andrushchenko <Oleksandr_Andrushchenko@xxxxxxxx>]

Hi, Roger!

On 02.02.22 11:56, Roger Pau Monné wrote:
> On Wed, Feb 02, 2022 at 06:44:41AM +0000, Oleksandr Andrushchenko wrote:
>> Hi, Roger!
>>
>> On 12.01.22 17:15, Roger Pau Monné wrote:
>>> On Thu, Nov 25, 2021 at 01:02:44PM +0200, Oleksandr Andrushchenko wrote:
>>>> @@ -108,11 +115,32 @@ int vpci_add_handlers(struct pci_dev *pdev)
>>>>        pdev->vpci = vpci;
>>>>        INIT_LIST_HEAD(&pdev->vpci->handlers);
>>>>    
>>>> +    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);
>>>> +        if ( !bar->mem )
>>>> +        {
>>>> +            rc = -ENOMEM;
>>>> +            goto fail;
>>>> +        }
>>>> +    }
>>> You just need the ranges for the VPCI_BAR_MEM32, VPCI_BAR_MEM64_LO and
>>> VPCI_BAR_ROM BAR types (see the MAPPABLE_BAR macro). Would it be
>>> possible to only allocate the rangeset for those BAR types?
>> I guess so
>>> Also this should be done in init_bars rather than here, as you would
>>> know the BAR types.
>> So, if we allocate these in init_bars so where are they destroyed then?
>> I think this should be vpci_remove_device and from this POV it would
>> be good to keep alloc/free code close to each other, e.g.
>> vpci_add_handlers/vpci_remove_device in the same file
> The alloc/free is asymmetric already, as vpci->{msix,msi} gets
> allocated in init_msi{x} but freed at vpci_remove_device.
Makes sense, I will implement as you suggest
>
> Thanks, Roger.
Thank you,
Oleksandr