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>