From owner-freebsd-arch Fri Feb 22 9:25:31 2002 Delivered-To: freebsd-arch@freebsd.org Received: from mail.gmx.net (mail.gmx.net [213.165.64.20]) by hub.freebsd.org (Postfix) with SMTP id 4389F37B400 for ; Fri, 22 Feb 2002 09:25:25 -0800 (PST) Received: (qmail 8454 invoked by uid 0); 22 Feb 2002 17:25:23 -0000 Received: from pd953899c.dip.t-dialin.net (HELO forge.local) (217.83.137.156) by mail.gmx.net (mp014-rz3) with SMTP; 22 Feb 2002 17:25:23 -0000 Received: from tmm by forge.local with local (Exim 3.34 #1) id 16eJRj-0000pg-00; Fri, 22 Feb 2002 18:25:23 +0100 Date: Fri, 22 Feb 2002 18:25:22 +0100 From: Thomas Moestl To: Bruce Evans Cc: freebsd-arch@FreeBSD.ORG Subject: Re: adding more endian conversion and bus space functions Message-ID: <20020222172522.GA302@crow.dom2ip.de> Mail-Followup-To: Bruce Evans , freebsd-arch@FreeBSD.ORG References: <20020222033455.GA289@crow.dom2ip.de> <20020223004643.V25362-100000@gamplex.bde.org> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20020223004643.V25362-100000@gamplex.bde.org> User-Agent: Mutt/1.3.27i Sender: owner-freebsd-arch@FreeBSD.ORG Precedence: bulk List-ID: List-Archive: (Web Archive) List-Help: (List Instructions) List-Subscribe: List-Unsubscribe: X-Loop: FreeBSD.ORG On Sat, 2002/02/23 at 01:22:30 +1100, Bruce Evans wrote: > On Fri, 22 Feb 2002, Thomas Moestl wrote: > > > I've attached an updated version of the patch, which implements some > > helpful suggestions by Mike Barcroft to reduce name space pollution > > when the new functions will be exposed to userland, as well as some > > cleanups... > > > ==== //depot/projects/sparc64/sys/alpha/include/bus.h#1 - /home/tmm/p4/sparc64/sys/alpha/include/bus.h ==== > > --- /tmp/tmp.8072.1 Thu Feb 21 19:06:08 2002 > > +++ /home/tmm/p4/sparc64/sys/alpha/include/bus.h Wed Feb 20 01:37:00 2002 > > @@ -366,6 +366,70 @@ > > (t)->ab_ops->abo_barrier(t, (h)+(o), l, f) > > > > /* > > + * Stream accesses are the same as normal accesses on alpha; there are no > > + * supported bus systems with an endianess different from the host one. > > + */ > > +#define bus_space_read_stream_1(t, h, o) bus_space_read_1((t), (h), (o)) > > ... > > I think these definitions should be in a common header. They are only present on some architectures; powerpc and sparc64 have bus systems with a different endianesses than the CPU, so these defines are different there. I think that an extra header only to be included by those architectures for which the stream and non-stream variants are identical may be a bit of overkill. > > ==== //depot/projects/sparc64/sys/i386/include/endian.h#6 - /home/tmm/p4/sparc64/sys/i386/include/endian.h ==== > > --- /tmp/tmp.8072.10 Thu Feb 21 19:06:10 2002 > > +++ /home/tmm/p4/sparc64/sys/i386/include/endian.h Thu Feb 21 01:53:24 2002 > > @@ -58,12 +58,13 @@ > > #define BYTE_ORDER LITTLE_ENDIAN > > #endif /* ! _POSIX_SOURCE */ > > > > +#ifdef _KERNEL > > #ifdef __GNUC__ > > > > -__BEGIN_DECLS > > - > > +#ifndef _BSWAP32_DEFINED > > +#define _BSWAP32_DEFINED > > static __inline __uint32_t > > -__htonl(__uint32_t __x) > > +__bswap32(__uint32_t __x) > > { > > #if defined(_KERNEL) && (defined(I486_CPU) || defined(I586_CPU) || defined(I686_CPU)) && !defined(I386_CPU) > > __asm ("bswap %0" : "+r" (__x)); > > Can _BSWAP32_DEFINED be already defined here? I think only this file > should define it (for i386's), and the multiple-inclusion protection > prevents it being redefined. Yes, the #ifndef protection is not really needed. I will remove it. > > ==== //depot/projects/sparc64/sys/sys/imgact_aout.h#3 - /home/tmm/p4/sparc64/sys/sys/imgact_aout.h ==== > > --- /tmp/tmp.8072.20 Thu Feb 21 19:06:12 2002 > > +++ /home/tmm/p4/sparc64/sys/sys/imgact_aout.h Wed Feb 20 00:51:10 2002 > > @@ -50,13 +50,13 @@ > > ((mag) & 0xffff) ) > > > > #define N_GETMAGIC_NET(ex) \ > > - (__ntohl((ex).a_midmag) & 0xffff) > > + (ntohl((ex).a_midmag) & 0xffff) > > #define N_GETMID_NET(ex) \ > > - ((__ntohl((ex).a_midmag) >> 16) & 0x03ff) > > + ((ntohl((ex).a_midmag) >> 16) & 0x03ff) > > #define N_GETFLAG_NET(ex) \ > > - ((__ntohl((ex).a_midmag) >> 26) & 0x3f) > > + ((ntohl((ex).a_midmag) >> 26) & 0x3f) > > #define N_SETMAGIC_NET(ex,mag,mid,flag) \ > > - ( (ex).a_midmag = __htonl( (((flag)&0x3f)<<26) | (((mid)&0x03ff)<<16) \ > > + ( (ex).a_midmag = htonl( (((flag)&0x3f)<<26) | (((mid)&0x03ff)<<16) \ > > | (((mag)&0xffff)) ) ) > > > > #define N_ALIGN(ex,x) \ > > This seems to be a regression. I think the change should be from __hton* > to __bswap*. Hmmm, judging from the comments in imgact_aout.c, that code was added to deal with NetBSD executables, which apparently use network byte order for the header fields; however, other executables would presumably be in host byte order on big-endian architectures, too, so executables with little-endian header fields on big-endian architectures should be non-existent. In any case, there would be other modifications required to handle this case in at least imgact_aout.c, so I would rather not do this right now, at least not in that commit. > > +++ /home/tmm/p4/sparc64/sys/libkern/alpha/bswap16.S Tue Feb 19 20:08:13 2002 > > @@ -0,0 +1,35 @@ > > +/* > > + * Copyright (c) 1996 Carnegie-Mellon University. > > + * All rights reserved. > > + * > > + * Author: Chris G. Demetriou > > ... > > + * $FreeBSD$ > > + */ > > + > > +#define NAME __bswap16 > > + > > +#include > > Is this and similar files from NetBSD? It is too trivial to deserve > having a long copyright. In libc/i386/string/memcpy.S, similar > functionality takes 2 lines: > > #define MEMCPY > #include "bcopy.S" They were copied directly from ntoh*.S from libkern, which had this copyright, so I felt uncomfortable to remove it. - thomas To Unsubscribe: send mail to majordomo@FreeBSD.org with "unsubscribe freebsd-arch" in the body of the message