Re: [PATCH v3 03/11] vpci/header: Move register assignments from init_bars

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

On Fri, Oct 15, 2021 at 08:04:56AM +0200, Jan Beulich wrote:
> On 13.10.2021 15:51, Roger Pau Monné wrote:
> > On Thu, Sep 30, 2021 at 10:52:15AM +0300, Oleksandr Andrushchenko wrote:
> >> --- a/xen/drivers/vpci/header.c
> >> +++ b/xen/drivers/vpci/header.c
> >> @@ -445,6 +445,55 @@ static void rom_write(const struct pci_dev *pdev, 
> >> unsigned int reg,
> >>          rom->addr = val & PCI_ROM_ADDRESS_MASK;
> >>  }
> >>  
> >> +static int add_bar_handlers(const struct pci_dev *pdev)
> > 
> > Making this const is again misleading IMO, as you end up modifying
> > fields inside the pdev, you get away with it because vpci data is
> > stored in a pointer.
> 
> I think it was me who asked for const to be added in places like this
> one. vpci data hanging off of struct pci_dev is an implementation
> artifact imo, not an unavoidable connection. In principle the vpci
> data corresponding to a physical device could also be looked up using
> e.g. SBDF.

I was considering vPCI part an intrinsic part of the pci_dev, but I
can see you thinking otherwise. We similarly have other pieces of data
hanging off pci_dev, so I think it's hard to tell which ones as fine
to have as part of the struct vs as pointer references.

> Here the intention really is to leave the physical device unchanged;
> that's what the const documents (apart from enforcing).

Ack. I wouldn't have asked for those myself, but as said above I can
see your point.

Regards, Roger.