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
