Bus space routines

Tue Jun 18 22:03:55 UTC 2013

-----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?

> IMO, we'll run in circles forever without a new userland interface
> or without extending an existing one to be able to deal with all
> these MD requirements (as such, thinking in the direction of the
> MI bus_space(9) at least in theory is the right thing but the
> problem is that it's an _K_PI in the first place).

I am also a big fan of simple API but io(4) isn't too bad, IMHO. :-)

Jung-uk Kim
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2.0.20 (FreeBSD)

iQEcBAEBAgAGBQJRwNksAAoJECXpabHZMqHO5dYH/363uqlIUYMhNF2+o/IkNTd/
1E3uoRqtBuZc9Nl56aBu2X7qCRe7ndlxIzXwqUbOw/MD+ZuGFMxAemVD8HU485l1
17TAIhrLbvZShu7OqjfNYZDY6hi9LJs+7Y0NKFMj/0qQxFZZvrS1/0BrwxmMzQ3E
Jr6LigLaWi5LEekoVEX1Qg7TrXb7fuRXLC3G7SJWtQMi20h/QHpo69wG9+WSugCT
VCPe2t+UrJ07tHa+XFS/SAmPNZHoukIzzkFXzclskqnPXsA6M8eyTG/2SxSoevtS
nob7z4WxlN0Eqkb7EIr04tO+/bu2aKDdg4yCV9SRJ7+9tMruPHFt1TDhuBy1IAg=
=rv89
-----END PGP SIGNATURE-----