pci rework not quite ready yet?

[Jesse Barnes]

On Wednesday, August 29, 2007 9:10 am Keith Packard wrote:
> On Wed, 2007-08-29 at 08:11 -0700, Jesse Barnes wrote:
> > I'd be much happier if we just got away from using bus
> > addresses entirely.  There's really no way we can use them reliably
> > short of growing sophisticated bus drivers in the X server...
>
> I disagree -- all bus addresses for a single card live in a single
> address space, and the driver must use many of them while programming
> various operations. In particular, if you want to split a single BAR
> into multiple 'regions' (as we do on Intel cards when mapping the
> GTT), then you must talk about bus addresses instead of trying to
> hide entirely behind a BAR abstraction.
>
> It's also far easier to store just the bus address in the driver than
> to track both BAR index and offset. Given that you must often program
> the card with bus addresses, it doesn't make sense to make the driver
> constantly refer back to the pci BAR tables unless you think those
> are going to change.

Yeah, I guess my main concern is how buggy some of the core X usage of 
bus addresses has been.  As long as we can avoid that by making clear 
the difference between bus, CPU physical, GTT virtual, and process 
virtual I'll be happy.

> > Also, adding this new interface seems like a lot of churn for just
> > dealing with cacheability attributes.  Maybe it would be best to
> > have a separate, madvise-like call to set the attributes instead?
>
> libpciaccess is a brand new API, so changing it now is far easier
> than at any future point. In particular, as every mapping must
> indicate what kind of access it needs, we should enforce this by
> including it in the mapping call.
>
> Right now, the library API isn't sufficient for the X server's needs;
> let's just make it right.

Sure, but still, having a separate call for this might make more sense.

FYI, subrange mappings don't buy us anything on a security basis ATM, 
since the sysfs resourceX files used by pciaccess are managed as whole 
files, not as collections of pages.  We can't do fine grained access 
control without using device specific ioctls or something.  So having a 
subrange API will only help with attribute control, not security, so it 
might be best to do it separately.

> > Ick, yeah, getting rid of PCITAG entirely would be nice.  But any
> > cleanups (I think there are many more we could pursue) should be
> > put off until things are working well.
>
> Just thinking about preserving our existing API to make driver
> porting easier, but as we cannot retain it entirely, perhaps it's
> better to just bail and remove the old code.

I hope we can get to that point eventually.  X has too many slightly 
overlapping and obsoleted APIs as it is... :)

Jesse