Next steps for pci-rework branches

[Jesse Barnes]

On Sunday, August 13, 2006 7:55 am, Mark Kettenis wrote:
> > From: Erwin Rol <mailinglists at erwinrol.com>
> > Date: Sun, 13 Aug 2006 15:42:12 +0200
> >
> > On Sat, 2006-08-12 at 23:43 +0200, Mark Kettenis wrote:
> > > > 4. Replace any calls to the old PCI accessor routines (e.g.,
> > > > pciReadLong, xf86MapPciMem, etc.) with calls to the new
> > > > accessor routines.  The APIs are similar, but there are some
> > > > differences.  The biggest difference, which bit me during the
> > > > Savage driver conversion, is that the new routines only allow
> > > > an entire BAR to be mapped, whereas the old routines allowed
> > > > subranges of a BAR to be mapped.  I suggest converting subrange
> > > > mappings to full BAR mappings *before* converting to the new
> > > > interfaces.  This is the approach that I took with the Savage
> > > > driver.
> > >
> > > Sorry to bring this up again, but this means that the
> > > libpciaccess API is *broken*.  There needs to be an interface for
> > > subrange mappings. Since:
> > >
> > > 1. Mappings can take up valueable resource in the kernel.
> >
> > Doesn't mapping several small regions instead of one big region
> > take up more resources ?
>
> Depends on how small the regions are.  If they're smaller than the
> page size and the kernel makes separate mappings for regions of the
> same physical page, then yes, it'll cost ya.

Well, one generally can't get smaller than page size protection anyway 
(for MMIO anyway), so...

> There also might be a 
> per-region resource tracking overhead, but in general that'll be
> pretty small.  The valueable resources I was thinking about are
> things like page table entries, and virtual address space; whose
> usage increases linearly with the size of of the mapped region.

I don't think that's much of an issue in most cases either.  For MMIO 
regions containing registers we're typically 4k-128k of memory, so the 
page table overhead will be fairly low.  On some devices, the 
protection factor is still important though, in case user-safe 
registers are sharing a PCI resource with stuff that users shouldn't 
see.  Often they're on the same page though, so you can't really 
provide much protection aside from refusing users the ability to map 
them entirely.  The question is, how many devices does X.Org support 
where this fine grained mapping protection is important?

> I wasn't really thinking of subdividing the mapping, but rather of
> not mapping any resources that aren't needed.  Think of framebuffer
> memory.  If I have a graphics card with 128MB of framebuffer memory,
> and the highest resolution I'm ever going to run at is 1024x768x32bpp
> mapping the complete 128MB is a waste of resources.

Again, it's just a few page table entries (which are small) so the waste 
is minimal.  Protection is the more important thing here (as you noted 
in your earlier mail).

> > > 2. Mapping the whole BAR might expose "dangerous" registers that
> > > could be accidentally (or on purpose by a malicious attacker)
> > > accessed. It's much better to not map these registers in the
> > > first place.
> >
> > If the library can map them, anything else can map them too. So if
> > your x-server/driver gets hacked it doesn't help you at all when
> > only "safe" registers are mapped.
>
> Sorry, no that's not true.  Mapping requires executing code.  This is
> generally a lot more difficult than just writing to an arbitrary
> memory area, especially on systems where memory is either executable
> or writable, but never both.  And the X server could revoke the
> privilige to map any further resources once it has allocated all the
> resources it needs.

On Linux at least, PCI resources are mmapable via sysfs.  Applications 
not needing to map the whole region should be able to do that (though I 
admit I haven't tested that much), but the permissions are such that if 
an application can map any of the region it can map all of it.  Doing 
anything more would be possible via per-device protection maps in the 
kernel, but probably wouldn't benefit that many devices.

That said, providing a way to do sub-region mappings might be worthwhile 
if we have enough devices that really need that sort of protection.

Jesse