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.
> 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
