On Mon, Nov 21, 2016 at 08:24:13PM +0100, Alexander Gordeev wrote:
> On Mon, Nov 21, 2016 at 03:27:13PM +0800, Peter Xu wrote:
> > On Wed, Nov 16, 2016 at 10:50:37AM +0100, Alexander Gordeev wrote:
> >
> > [...]
> >
> > > > struct pci_dev {
> > > > uint16_t bdf;
> > > > + phys_addr_t bar[PCI_BAR_NUM];
> > >
> > > This questions pop up time and again. If bars are 32 or 64 bit?
> > > What if bar is not 64 aligned? etc.
> >
> > Could you help elaborate what does "not 64 aligned" mean here? AFAIU
> > the bar can only be either 32bit or 64bit wide?
>
> Let's use your example below.
>
> > > I guess, it worth mentionig in comment that bar[] array here does
> > > not match PCI header binary layout.
> >
> > Not sure whether you mean this:
> >
> > void pci_scan_bars(struct pci_dev *dev)
> > {
> > int i, cur = 0;
> >
> > for (i = 0; i < PCI_BAR_NUM; i++) {
> > if (!pci_bar_is_valid(dev, i))
> > continue;
> > dev->bar[cur++] = pci_bar_get_addr(dev, i);
> > if (pci_bar_is64(dev, i))
> > i++;
> > }
> > }
> >
> > Assume that we have a device with:
> >
> > - bar 0: 32 bit addr A0
> > - bar 1: 64 bit addr A1
> > - bar 2: 32 bit addr A2
> >
> > The original patch will generate:
> >
> > dev->bar[0] == A0
> > dev->bar[1] == A1
> > dev->bar[2] == 0
> > dev->bar[3] == A2
> > dev->bar[4] == 0
> > dev->bar[5] == 0
> >
> > The new code (above) will generate:
> >
> > dev->bar[0] == A0
> > dev->bar[1] == A1
> > dev->bar[2] == A2
> > dev->bar[3] == 0
> > dev->bar[4] == 0
> > dev->bar[5] == 0
>
> So in all three cases (PCI header, 6 32-bit words and
> 6 64-bit physical addresses) the term 'bar' is somehow
> correct.
>
> Yet it is confusing, because in the first two cases it reflects
> device PCI header layout (6 32-bit words) while in the latter
> case it is a physical address.
>
> My suggestion is to rename pci_dev::bar[] to pci_dev::resource[]
> to uncouple what is stored in this array (addresses) from PCI
> header layout.
Looks like Linux is using resource rather than bar. Will take your
advice to avoid the confusion. Thanks.
-- peterx
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
