Date: Sun, 11 Mar 2012 18:55:20 +0100 From: Robert Millan <rmh@freebsd.org> To: Adrian Chadd <adrian@freebsd.org> Cc: freebsd-arch@freebsd.org Subject: Re: [PATCH] Add compatibility <sys/io.h> Message-ID: <CAOfDtXOCyjga5QHz98Re3jXkefNzB-MbULcAVUQH89ToVLkw9g@mail.gmail.com> In-Reply-To: <CAJ-Vmonu_ApSd192cjvsW6k3eNNK4Kz=MmAMe_e=zmwbrS8Ayw@mail.gmail.com> References: <CAOfDtXPGPP0reN9NTBw_5%2BNwXZ56Yy0oyx_fH%2BDOvmpc1O%2BQdQ@mail.gmail.com> <CAJ-Vmonu_ApSd192cjvsW6k3eNNK4Kz=MmAMe_e=zmwbrS8Ayw@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
Hi Adrian,
El 9 de mar=C3=A7 de 2012 23:01, Adrian Chadd <adrian@freebsd.org> ha escri=
t:
> I really, really don't like the idea of having the same function have
> different argument orders based on the include file you're using.
>
> That will lead to all kinds of weird ass debugging issues later on.
Definitely. This disparity is terribly harmful. I think everybody
will agree this is the biggest problem here.
I don't think this problem is introduced by my patch though, rather I
think my proposal is one of the possible ways to address it. But of
course it's not the only way :-)
> Why is it that we can't fix the upstream code?
Well even with current FreeBSD codebase I see two possible kind of
trouble in upstream code:
- fails to build
- builds successfully but sends I/O to the wrong ports
there are many ways upstream code can missbehave and reach either of
these problems. As the second one is clearly the worst result, I
think that preventing that should be the priority.
Bruce made some interesting suggestions....
El 10 de mar=C3=A7 de 2012 14:26, Bruce Evans <brde@optusnet.com.au> ha esc=
rit:
> cpufunc.h has no user-serving parts inside, so it can't conflict with
> any legal include. However, abusing it in userland is convenient.
>
> Even using the port i/o functions in it in the kernel is an error.
> Bus space functions should be used instead. However, the i/o functions
> in it are much older than bus space, and abusing them in MD code in
> the kernel is convenient.
>
> [...]
> #error "no user serving parts inside" would prevent cpufunc.h being abuse=
d
> at any time :-).
I think this would give the expected results, as long as we provide a
suitable alternative for userland users.
>>> FreeBSD code: outw(port, data);
>>> Glibc code: outw(data, port);
>
> The FreeBSD order seemed natural to me since I was familiar with
> Intel/DOS and they never used AT&T asm order. I put the i/o functions
> in cpufunc.h, but 386BSD seems to have had them in the Intel order,
> since 4.4BSD has them in Intel order.
Either way in both cases it looks arbitrary to me. The problem is
that they don't match :-(
> A sys header is more unsuitable, since this is very machine-dependent.
> Even bus space's header is not in sys (it is <machine/bus.h>), though
> many of its APIs are MI (with very MD internals for the typeded types).
>
> Userland has even more reasons to avoid the machine dependencies.
> Bus space is more complex, but seems to work OK in userland:
>
> % #include <sys/types.h>
> % #include <machine/bus.h>
> % % void
> % my_outb(unsigned int port, unsigned char data)
> % {
> % bus_space_write_1(X86_BUS_SPACE_IO, port, 0, data);
> % }
>
> This gives only minor pessimizations relative to the raw inline asm
> outb().
Looks good. So you suggest we tell userspace users to switch to
bus_space_write_*?
Btw what are the pessimizations? I can't see anything in that
function which can't be optimized away after inlining.
> To get this to work, I had to work around the following problems:
> - the prerequisite <sys/types.h> is undocumented
> - someone renamed I386_BUS_SPACE_IO and AMD64_BUS_SPACE_IO to
> X86_BUS_SPACE_IO. This gives portability problems, and I only remembere=
d
> the old names. Having the machine name in the API at all is a bug.
> Especially for BUS_SPACE_MEM, the machine arch can't make any difference=
.
> There might be different types of memory mapped i/o, but a plain MEM sho=
uld
> mean "ordinary" memory, or the least-extraordinary memory that the machi=
ne
> has, or if that is too extraordinary, then just don't implement
> BUS_TYPE_MEM. Similarly for IO. It means port i/o for x86, and jusr
> including the x86 header says that you want the x86 version of "ordinary=
"
> ports.
I could prepare a patch to fix those.
> - <machine/bus.h> has undocumented namespace pollution. It includes
> <machine/cpufunc.h>, and even uses outb() from it. So I had to
> rename my function my_outb() to get it to compile.
I wouldn't duplicate outb() in <machine/bus.h>, IIRC its
implementation is compiler-dependant so it's not just a one-liner.
How about this:
- Create <machine/_cpufunc.h>
- Move outb() et al to that file, and prefix them with two underscores.
- Make <machine/cpufunc.h> include <machine/_cpufunc.h> and implement
outb() as a #define to __outb().
then <machine/bus.h> can include <machine/_cpufunc.h> and use __outb() etc.
--=20
Robert Millan
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAOfDtXOCyjga5QHz98Re3jXkefNzB-MbULcAVUQH89ToVLkw9g>