Date: Wed, 19 Jun 2013 18:17:09 -0400 From: Jung-uk Kim <jkim@FreeBSD.org> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-arch@freebsd.org, Robert Millan <rmh@freebsd.org>, Niclas Zeising <zeising@freebsd.org>, "arch@freebsd.org" <arch@freebsd.org>, Marius Strobl <marius@alchemy.franken.de> Subject: Re: Bus space routines Message-ID: <51C22DE5.2040306@FreeBSD.org> In-Reply-To: <201306191056.41700.jhb@freebsd.org> References: <51C044DA.8030406@freebsd.org> <F65ADD1F-3B27-44C1-A6EE-3A9E1E3C33A4@bsdimp.com> <20130618224501.GC53058@alchemy.franken.de> <201306191056.41700.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA1 On 2013-06-19 10:56:41 -0400, John Baldwin wrote: > 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]? Yes. > Does it want to do anything else (mmap BARs, etc.)? Not ATM. http://cgit.freedesktop.org/xorg/lib/libpciaccess/commit/src/common_io.c?id=5e8d4c1 for (bar = 0; bar < 6; bar++) { struct pci_mem_region *region = &(dev->regions[bar]); if (!region->is_IO) continue; ... Please note we do not support I/O via PCI BARs yet on *any* platform. In fact, we only support ISA I/O ports, i.e., just enough to support VGA/VESA for x86, and possibly ia64. AFAICT, the API is available but X.org does not need it ATM. In other words, there is no hurry at all. In fact, I don't think they will ever use the API because they already moved these things to kernel-side anyway. ;-) Jung-uk Kim -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.20 (FreeBSD) iQEcBAEBAgAGBQJRwi3lAAoJECXpabHZMqHO2wYIAJURutCs8gKsuxrfycAR4vDL T8hFcA5sR4WFJBuyMLsKxagcF0A6+HJtyTW8V7yC1dqt9lqMWLUAPHrAFv9Fdcsg Ztbe3N8FGIbKva6VrtfdYWHjE4kv3Qb5OblVsW1s/kXm4Cetei5dQ4HXA1Q340LT 7OAlFxLaWMfLjkSj939BlBeXJ2vApnEEkYEFJiEGxDMm6laFeB/B1vNfSQbF2ncd x8VYf5qwal1kBLs8w5mdAG7Eo9RAatWNa5/Cycr4qdALKpqRZXb/pTob07pBAaPH 1GZC4x1KFDwM/m3T81T+ntgQ2lJNyPuZZaO29s5mcppugdFUBF6yX1ISAWQ/ovQ= =8LTg -----END PGP SIGNATURE-----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?51C22DE5.2040306>