From owner-freebsd-arch@FreeBSD.ORG Sun Mar 11 17:55:21 2012 Return-Path: Delivered-To: freebsd-arch@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 9E8AA1065672; Sun, 11 Mar 2012 17:55:21 +0000 (UTC) (envelope-from rmh.aybabtu@gmail.com) Received: from mail-iy0-f182.google.com (mail-iy0-f182.google.com [209.85.210.182]) by mx1.freebsd.org (Postfix) with ESMTP id 544FA8FC1A; Sun, 11 Mar 2012 17:55:21 +0000 (UTC) Received: by iahk25 with SMTP id k25so7236599iah.13 for ; Sun, 11 Mar 2012 10:55:21 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; bh=j4FH68a6nSz3SRjJ+ATXD/PzEvCSjivmFoybUt1aLK0=; b=qSbpBFq0xuCtU+YW8+fXW1IJcZ+Bt+A++JtRzE0W8pva7E/3cE3dd7A4ozdEPrQyk7 xn0+yhNs1vojYqoNJlvMutrvn3CGyz89ku+Dg2XvxLSg1JwdK+xl9H0X1rjUekjQcRd2 bRNP2jEzVw1YrYNrjcV/b2bXskji406eRGrb71X9nPNbJ3VkO3HxSZ9+WNef2/isGjym 3RyKfGMCs+mGBRf8w34OO5nd7KrtN/XlJUzOi1pf6fmS06WUJfKEvOsBQYguqYktIUDo wvvGZborX+d+PkPYNu3JCl7S+HRiVM6+tGqF4Mn59inm1I6NVBQOfTYEongV2jKI6ak7 IiAA== MIME-Version: 1.0 Received: by 10.50.87.225 with SMTP id bb1mr15275901igb.13.1331488520802; Sun, 11 Mar 2012 10:55:20 -0700 (PDT) Sender: rmh.aybabtu@gmail.com Received: by 10.43.130.201 with HTTP; Sun, 11 Mar 2012 10:55:20 -0700 (PDT) In-Reply-To: References: Date: Sun, 11 Mar 2012 18:55:20 +0100 X-Google-Sender-Auth: UFiMSlid-6ko9L0wH7QXObibqC4 Message-ID: From: Robert Millan To: Adrian Chadd Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Cc: freebsd-arch@freebsd.org Subject: Re: [PATCH] Add compatibility X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 11 Mar 2012 17:55:21 -0000 Hi Adrian, El 9 de mar=C3=A7 de 2012 23:01, Adrian Chadd 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 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 ), 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 > % #include > % % 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 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. > - has undocumented namespace pollution. It includes > , 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 , IIRC its implementation is compiler-dependant so it's not just a one-liner. How about this: - Create - Move outb() et al to that file, and prefix them with two underscores. - Make include and implement outb() as a #define to __outb(). then can include and use __outb() etc. --=20 Robert Millan