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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

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.

Thanks, Roger.