On Mon, Feb 13, 2017 at 10:44:21AM +0100, Alexander Gordeev wrote:
> On Wed, Feb 08, 2017 at 01:49:20PM +0100, Alexander Gordeev wrote:
> > On Sat, Jan 28, 2017 at 12:10:41PM +0100, Andrew Jones wrote:
> > > How about just this instead (untested)
> > >
> > > bool pci_bar_is_valid(pcidevaddr_t dev, int bar_num)
> > > {
> > > assert(bar_num >= 0 && bar_num < PCI_BAR_NUM);
> > > if (bar_num > 0 && pci_bar_is64(dev, bar_num - 1))
> > > return pci_bar_is_valid(dev, bar_num - 1);
> > > return pci_bar_size(dev, bar_num);
> > > }
> >
> > This one can not distinguish between PCI_BASE_ADDRESS_* flags
> > and address bits of high part of a 64 bit address.
> >
> > However, it seems a newly introduced pci_dev::resource[] can
> > be used for that.
>
> Hi Andrew,
>
> Although using pci_dev::resource[] is possible I do not like a
> result for various reasons. I.e. I anticipate these:
>
> - pci_bar_get/set_addr() logic will get quite cryptic;
> - mismatch between pci_dev::resource[] 64 bit size and
> 32 bit BAR size. While it is okay right now it will
> be confusing once pci_dev::resource[] are reused as
> BAR validity flags;
> - architecture specific pci_probe() reworked to seed
> pci_dev::resource[] (and being invoked on all archs);
>
> Overall, the current compact implementetion is going to turn
> into a framework with not so obvious logic.
>
> I suggest just polish v1 or drop the idea of checking BARs
> altogether.
We definitely need to improve pci_bar_is_valid(), so let's see
how a v2 of this series looks.
Thanks,
drew
>
> Thoughts?
>
> > > Thanks,
> > > drew
