Bus space routines

Wed Jun 19 17:58:45 UTC 2013

On Tuesday, June 18, 2013 6:45:01 pm Marius Strobl wrote:
> On Tue, Jun 18, 2013 at 04:19:16PM -0600, Warner Losh wrote:
> > 
> > On Jun 18, 2013, at 4:03 PM, Jung-uk Kim wrote:
> > 
> > > -----BEGIN PGP SIGNED MESSAGE-----
> > > Hash: SHA1
> > > 
> > > On 2013-06-18 17:49:57 -0400, Marius Strobl wrote:
> > >> On Tue, Jun 18, 2013 at 09:24:19PM +0000, Jung-uk Kim wrote:
> > >>> On 2013-06-18 17:14:38 -0400, Jung-uk Kim wrote:
> > >>>> On 2013-06-18 17:06:48 -0400, Jung-uk Kim wrote: 2013?  6? 18?
> > >>>> 17:06, Jung-uk Kim ? ?:> On 2013-06-18 16:59:43 -0400, Marius
> > >>>> Strobl wrote:
> > >>>>>> On Tue, Jun 18, 2013 at 02:17:53PM -0400, Jung-uk Kim
> > >>>>>> wrote:
> > >>>>>>> -----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1
> > >>>>>>> 
> > >>>>>>> On 2013-06-18 08:40:38 -0400, Marius Strobl wrote:
> > >>>>>>>> On Tue, Jun 18, 2013 at 01:30:34PM +0200, Niclas
> > >>>>>>>> Zeising wrote:
> > >>>>>>>>> On 2013-06-18 13:13, Marius Strobl wrote:
> > >>>>>>>>>> On Tue, Jun 18, 2013 at 12:20:14PM +0200, Niclas
> > >>>>>>>>>> Zeising wrote:
> > >>>>>>>>>>> This has been discussed before [1], but there
> > >>>>>>>>>>> seem to still be a lack of consensus, so I'll ask
> > >>>>>>>>>>> again.
> > >>>>>>>>>>> 
> > >>>>>>>>>>> Should in*/out* macros or bus_space* functions be
> > >>>>>>>>>>> used in userland code? The background is that the
> > >>>>>>>>>>> port devel/libpciaccess uses these routines on
> > >>>>>>>>>>> FreeBSD.  In a first incarnation it used the
> > >>>>>>>>>>> bus_space* routines, see this patch:
> > >>>>>>>>>>> 
> > >>>>>>>>>>> 
http://trillian.chruetertee.ch/ports/browser/trunk/devel/libpciaccess/files/patch-
src-freebsd_pci.c?rev=591
> > >>>>>>>>>>> 
> > >>>>>>>>>>> 
> > >>>>>>>>>>> 
> > >>>>>>> 
> > >>>>>>>>>>> 
> > >>>> 
> > >>>>>>>>>>> 
> > >>>> 
> > >>>>>>>>>>> 
> > > This was later changed to use the in*/out* macros directly, with the
> > >>>>>>>>>>> motivation that the bus_space* functions is a KPI
> > >>>>>>>>>>> that shouldn't be used in userland.  See
> > >>>>>>>>>>> following for an updated patch:
> > >>>>>>>>>>> 
> > >>>>>>>>>>> 
http://trillian.chruetertee.ch/ports/browser/trunk/devel/libpciaccess/files/patch-
src-freebsd_pci.c?rev=815
> > >>>>>>>>>>> 
> > >>>>>>>>>>> 
> > >>>>>>>>>>> 
> > >>>>>>> 
> > >>>>>>>>>>> 
> > >>>> 
> > >>>>>>>>>>> 
> > >>>> 
> > >>>>>>>>>>> 
> > > The problem is that the in*/out* macros differ between FreeBSD and
> > >>>>>>>>>>> Debian/kFreeBSD, and Debian/kFreeBSD want to
> > >>>>>>>>>>> switch back to use bus_space* again.
> > >>>>>>>>>>> 
> > >>>>>>>>>>> My question is simply, which one is correct, or
> > >>>>>>>>>>> should libpciaccess be reworked in a completely
> > >>>>>>>>>>> different way?
> > >>>>>>>>>> 
> > >>>>>>>>>> The latter; in*/out*() are somewhat okay if you
> > >>>>>>>>>> want to restrict this to work on x86 without PCI
> > >>>>>>>>>> domains only. The above approach to using
> > >>>>>>>>>> bus_space(9) is one big hack, though. The right way
> > >>>>>>>>>> for employing that API is to allocate the
> > >>>>>>>>>> corresponding bus resource of a particular device
> > >>>>>>>>>> and then to obtain bus tag and handle via 
> > >>>>>>>>>> rman_get_bus{tag,handle}(9) - which won't work from
> > >>>>>>>>>> userland, however. The shortcut to just stick in 
> > >>>>>>>>>> {AMD64,I386}_BUS_SPACE_IO instead is totally
> > >>>>>>>>>> unportable as generally a bus tag isn't a mere
> > >>>>>>>>>> constant and also may depend on which PCI bus and
> > >>>>>>>>>> domain a particular device is located on/in. What
> > >>>>>>>>>> we really need is a proper interface allowing
> > >>>>>>>>>> userland to access PCI I/O and memory registers, f.
> > >>>>>>>>>> e. via /dev/pci, and for libpciaccess to build upon
> > >>>>>>>>>> that, i. e. essentially the same as things work
> > >>>>>>>>>> on/with Linux and /sys/bus/pci/device. As a 
> > >>>>>>>>>> side-effect this then also permits to properly
> > >>>>>>>>>> sanity check PCI accesses from userland within the
> > >>>>>>>>>> kernel.
> > >>>>>>>>>> 
> > >>>>>>>>> 
> > >>>>>>>>> That is true, however, it won't build itself today,
> > >>>>>>>>> and we need to have this working in the meantime, so
> > >>>>>>>>> what do you suggest we use for now?
> > >>>>>>>> 
> > >>>>>>>> That's hard to say as architecturally neither
> > >>>>>>>> in*/out*() nor bus_space(9) are the way to proceed.
> > >>>>>>>> Tentatively, I'd go with abusing the latter as that
> > >>>>>>>> approach _should_ allow to make things additionally
> > >>>>>>>> work one another one or two architectures - in
> > >>>>>>>> particular powerpc - without introducing an #ifdef
> > >>>>>>>> hell.
> > >>>>>>> 
> > >>>>>>> AFAIK, bus_space(9) can only work for amd64 and i386 in 
> > >>>>>>> userland by pure luck.  It can never work for powerpc if
> > >>>>>>> I'm reading the MD headers correctly.
> > >>>> 
> > >>>>>> Actually, I think that by cloning bs_le_tag in userland as
> > >>>>>> far as necessary, i. e. leaving out things like
> > >>>>>> mapping/unmapping and allocation/deallocation etc., and
> > >>>>>> using that as bus tag, bus_space(9) has a fairly good
> > >>>>>> chance of working in userland for powerpc in this case.
> > >>>>>> Obviously, that's harder to do than faking the bus tag for
> > >>>>>> x86, though.
> > >>>> 
> > >>>>> Please don't forget the point of this thread, i.e., finding
> > >>>>> simple MI interface. ;-)
> > >>>> 
> > >>>>>>> Also, I strongly disagree with abusing bus_space because
> > >>>>>>> it gives a bad example.
> > >>>> 
> > >>>>>> Well, I strongly believe that both in*/out*() and
> > >>>>>> bus_space(9) give very bad examples for userland code :)
> > >>>> 
> > >>>>> If you insist, we can simply use io(4).
> > >>>> 
> > >>>> I went ahead and implemented it:
> > >>>> 
> > >>>> http://people.freebsd.org/~jkim/libpciaccess.diff
> > >>>> 
> > >>>> This should work with powerpc and other platforms with working
> > >>>> io(4).
> > >>> 
> > >>> I just realized powerpc does not have /dev/io, sorry.  Anyway,
> > >>> my point was there is userland API already and we don't have to
> > >>> reinvent the wheel.
> > >>> 
> > >> 
> > >> Essentially, the issue with io(4) is the same as with in*/out*(); 
> > >> you can't implement it in a sane way on architectures such as
> > >> powerpc that have busses of different endianesses, apart from some
> > >> other complications.
> > > 
> > > Why can't we implement it to always get/set in host byte order
> > > regardless of underlying bus?  Can't the MD portion of the driver
> > > figure it out magically?
> > 
> > Not typically. Since all the world is intel, all the world is little 
endian... Plus, the PCI bus is defined to be little endian, not host endian. 
Not all drivers cope well with this.
> > 
> 
> These are good arguments but IMO the real hard part would be to
> derive the endianess and whether it's an I/O or memory mapped
> register etc. solely from the port address supplied via io(4)/in*/
> out*(). We currently don't have any global infrastructure in the
> kernel to query for that information; quite the contrary, with newbus
> and bus_space(9) such information generally is local to a specific
> bus instance. Generally, there also isn't something like the fixed
> ISA range on x86, i. e. for example on one sparc64 machine address
> 0x1000 might refer to a memory mapped register on a big-endian SBus
> while on another one it could translate to a I/O BAR on a little-
> endian PCI bus. Consequently, from a kernel perspective it's way
> simpler if an userland interface would allow to specify "read from
> offset A of BAR B on PCI device C living on BUS D in domain E".

Yes, something like the config access, but having the address be
'offset X in bar Y'.  This would be MI and would use bus_space
in the kernel.  If you wanted a way to mmap BARs you
could implement that (mostly) trivially as well.

What operations does libpciaccess want to do with BARs: read_[1248]
and write_[1248]?  Does it want to do anything else (mmap BARs, etc.)?

-- 
John Baldwin