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>