Date: Fri, 22 Feb 2002 18:25:22 +0100 From: Thomas Moestl <tmoestl@gmx.net> To: Bruce Evans <bde@zeta.org.au> Cc: freebsd-arch@FreeBSD.ORG Subject: Re: adding more endian conversion and bus space functions Message-ID: <20020222172522.GA302@crow.dom2ip.de> In-Reply-To: <20020223004643.V25362-100000@gamplex.bde.org> References: <20020222033455.GA289@crow.dom2ip.de> <20020223004643.V25362-100000@gamplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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 <libkern/alpha/byte_swap_2.S> > > 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20020222172522.GA302>