[parisc-linux] byte swapping redux

[parisc-linux] byte swapping redux

[Grant Grundler]

Hi all,

Can someone explain why include/asm-parisc/byteorder.h uses the following?

#if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
#  define __BYTEORDER_HAS_U64__
#  define __SWAB_64_THRU_32__
#endif

vs include/asm-sparc64/byeorder.h:
#define __BYTEORDER_HAS_U64__

I'm pretty sure we don't want __SWAB_64_THRU_32__ defined.
PA-RISC version of ___arch__swab64() is appropriately defined for
both 32 and 64-bit compiles.

For reference include/linux/byteorder/swab.h says:
#ifdef __BYTEORDER_HAS_U64__
static __inline__ __const__ __u64 __fswab64(__u64 x)
{
#  ifdef __SWAB_64_THRU_32__
        __u32 h = x >> 32;
        __u32 l = x & ((1ULL<<32)-1);
        return (((__u64)__swab32(l)) << 32) |
		((__u64)(__swab32(h)));
#  else
        return __arch__swab64(x);
#  endif
}
...

thanks,
grant

Re: [parisc-linux] byte swapping redux

[Grant Grundler]

On Sun, Jun 29, 2003 at 12:14:15PM -0600, Grant Grundler wrote:
> #if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
> #  define __BYTEORDER_HAS_U64__
> #  define __SWAB_64_THRU_32__
> #endif
> 
> vs include/asm-sparc64/byeorder.h:
> #define __BYTEORDER_HAS_U64__

BTW, I forgot to mention parisc is picking generic 64-bit swap macros.
Instead of 4 cycles, we are talking 30 or so.

grant

Re: [parisc-linux] byte swapping redux

[Matthew Wilcox]

On Sun, Jun 29, 2003 at 12:14:15PM -0600, Grant Grundler wrote:
> Hi all,
> 
> Can someone explain why include/asm-parisc/byteorder.h uses the following?
> 
> #if !defined(__STRICT_ANSI__) || defined(__KERNEL__)
> #  define __BYTEORDER_HAS_U64__
> #  define __SWAB_64_THRU_32__
> #endif

Protection against userspace inclusion.  Which shouldn't be happening
of course, but some userspace programs are crap.

> vs include/asm-sparc64/byeorder.h:
> #define __BYTEORDER_HAS_U64__
> 
> I'm pretty sure we don't want __SWAB_64_THRU_32__ defined.
> PA-RISC version of ___arch__swab64() is appropriately defined for
> both 32 and 64-bit compiles.

Take it out then; if it works, commit it (to 2.5 anyway).

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk

Re: [parisc-linux] byte swapping redux

[Grant Grundler]

On Sun, Jun 29, 2003 at 07:41:50PM +0100, Matthew Wilcox wrote:
> Protection against userspace inclusion.  Which shouldn't be happening
> of course, but some userspace programs are crap.

What are they protecting against?
I'll remove it.

> > I'm pretty sure we don't want __SWAB_64_THRU_32__ defined.
> > PA-RISC version of ___arch__swab64() is appropriately defined for
> > both 32 and 64-bit compiles.
> 
> Take it out then; if it works, commit it (to 2.5 anyway).

well, I'm still looking at 2.4.x...you know about the problems I had
finding my boot disk on the rx2600 (ia64) when trying to boot 2.5.72.
I'm not keen on reproducing experience on parisc this weekend.

I was actually trying to get up to speed to test/review Joel's patch.

BTW, Prelimary diff on ftp://ftp.p-l.o/patches/diff-2.4.21-pa1
attempts to fix the byteorder.h mess and first cut at trying
to get intermezzo to build. But intermezzo won't assemble because
it has field alignment (ldd 52(x) instruction) problems.


thanks,
grant

Re: [parisc-linux] byte swapping redux

[Joel Soete]

> BTW, Prelimary diff on ftp://ftp.p-l.o/patches/diff-2.4.21-pa1
> attempts to fix the byteorder.h mess and first cut at trying
> to get intermezzo to build. But intermezzo won't assemble because
> it has field alignment (ldd 52(x) instruction) problems.

Hmm here no pb to build intermezzo (as a module) neither for 2.4.21-pa1 neither
2.4.22-pre2-pa1 (gcc-3.3 32bits at least). So I suspect that it is a pb with
binutils (I still have to downgrade it to binutils-2.13.90.0.18-1.7_hppa.deb)
which Dave fixed in upstream a time ago but (I don't know why) not yet included
in debian dpkg.

hth,
    Joel


---------------------------------
Tiscali ADSL: 19,50 euros/mois...abonnez-vous sur www.tiscali.be

Re: [parisc-linux] byte swapping redux

[Grant Grundler]

On Mon, Jun 30, 2003 at 12:19:49PM +0200, Joel Soete wrote:
> Hmm here no pb to build intermezzo (as a module) neither for 2.4.21-pa1 neither
> 2.4.22-pre2-pa1 (gcc-3.3 32bits at least). So I suspect that it is a pb with
> binutils (I still have to downgrade it to binutils-2.13.90.0.18-1.7_hppa.deb)

For the record, here are the toolchain versions I'm using:
grundler@ios:~$ ld -v
GNU ld version 2.14.90.0.1 20030505 Debian GNU/Linux
grundler@ios:~$ gcc -v
Reading specs from /usr/lib/gcc-lib/hppa-linux/3.2.3/specs
Configured with: ../src/configure -v
--enable-languages=c,c++,f77,objc,ada --prefix=/usr
--mandir=/usr/share/man --infodir=/usr/share/info
--with-gxx-include-dir=/usr/include/c++/3.2 --enable-shared
--with-system-zlib --enable-nls --without-included-gettext
--enable-__cxa_atexit --enable-clocale=gnu --enable-objc-gc hppa-linux
Thread model: posix
gcc version 3.2.3
grundler@ios:~$ 


> which Dave fixed in upstream a time ago but (I don't know why) not yet
> included in debian dpkg.

Dave isn't responsible for the debian package...find the maintainer.

thanks,
grant

Re: [parisc-linux] byte swapping redux

[Joel Soete]

Hi Grant,

> For the record, here are the toolchain versions I'm using:
> grundler@ios:~$ ld -v
> GNU ld version 2.14.90.0.1 20030505 Debian GNU/Linux
> grundler@ios:~$ gcc -v
> Reading specs from /usr/lib/gcc-lib/hppa-linux/3.2.3/specs
> Configured with: ../src/configure -v
> --enable-languages=c,c++,f77,objc,ada --prefix=/usr
> --mandir=/usr/share/man --infodir=/usr/share/info
> --with-gxx-include-dir=/usr/include/c++/3.2 --enable-shared
> --with-system-zlib --enable-nls --without-included-gettext
> --enable-__cxa_atexit --enable-clocale=gnu --enable-objc-gc hppa-linux
> Thread model: posix
> gcc version 3.2.3
> grundler@ios:~$ 

Ok, and so is it the same pb with binutils 2.13.90 (if you have the opportunity
to test it)?

> > which Dave fixed in upstream a time ago but (I don't know why) not yet
> > included in debian dpkg.
> 
> Dave isn't responsible for the debian package...find the maintainer.

I will try to find some time to open a pr near debian maintainer ;-)

Thanks,
    Joel

PS: The disk I used to test 64bits kernel on teh N class is broken :(, so
awaiting a spare part and rebuild a system, here I will go to build the kernel
2.4.21-pa0 and 2.4.21-pa1 (in 32bits mode) then objdump all object and diff
them in the hope to find the pb

---------------------------------
Tiscali ADSL: 19,50 euros/mois...abonnez-vous sur www.tiscali.be

Re: [parisc-linux] byte swapping redux

[Grant Grundler]

On Mon, Jun 30, 2003 at 04:58:01PM +0200, Joel Soete wrote:
> I don't understand why the 32bits kernel crashed with this patche:
> (gcc-3.3 and binutils-2.13.90)

uhm...perhaps the 64-bit swap is buggy for 32-bit mode?
I didn't test that. It's a different code path.

> the log of the boot sequence shows me that the kernel was launched twice:

probably not. It's probably the kernel crash path just dumping out
the dmesg buffer "again".

grant

Re: [parisc-linux] byte swapping redux

[Joel Soete]

Hi Grant,

> > I don't understand why the 32bits kernel crashed with this patche:
> > (gcc-3.3 and binutils-2.13.90)
> 
> uhm...perhaps the 64-bit swap is buggy for 32-bit mode?
> I didn't test that. It's a different code path.

I hope I am not going to anoying you.

I just found related interesting stuff: <http://lwn.net/Articles/38384/>

And may be also the source of pb.

I also write this small testcase to try to point out the pb in 32bits mode:

#include <stdlib.h>
#include <stdio.h>
#include <string.h>
#include <errno.h>
#include <asm/types.h>

typedef unsigned long long u64;
typedef u_int8_t  BOOLEAN;

typedef struct
{
    unsigned char b7, b6, b5, b4, b3, b2, b1, b0;
}
EightBytes;

static __inline__ __const__ __u32 ___arch__swab32(__u32 x)
{
        unsigned int temp;
        __asm__("shd %0, %0, 16, %1\n\t"        /* shift abcdabcd -> cdab
*/
                "dep %1, 15, 8, %1\n\t"         /* deposit cdab -> cbab */
                "shd %0, %1, 8, %0"             /* shift abcdcbab -> dcba
*/
                : "=r" (x), "=&r" (temp)
                : "0" (x));
        return x;
}

#define __arch__swab32(x) ___arch__swab32(x)

static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
{
        __u32 t1 = (__u32) x;
        __u32 t2 = (__u32) ((x) >> 32);
        ___arch__swab32(t1);
        ___arch__swab32(t2);
        return (((__u64) t1 << 32) + ((__u64) t2));
}

static __inline__ __const__ __u32 __fswab32(__u32 x)
{
        return __arch__swab32(x);
}

#  define __swab32(x) __fswab32(x)

static __inline__ __const__ __u64 __fswab64(__u64 x)
{
        __u32 h = x >> 32;
        __u32 l = x & ((1ULL<<32)-1);
        return (((__u64)__swab32(l)) << 32) | ((__u64)(__swab32(h)));
}

int main(int argc, char * * argv, char * * env) {

    unsigned long long TU64;
    int err;

    TU64=0xfedcba9876543210LL;

    printf("Val of TU64: %0Lx\n", TU64);
    printf("Val of ___arch__swab64(TU64): %0Lx\n", ___arch__swab64(TU64));
    printf("Val of __fswab64(TU64): %0Lx\n", __fswab64(TU64));

}

And here is the results:
Val of TU64: fedcba9876543210
Val of ___arch__swab64(TU64): 76543210fedcba98
Val of __fswab64(TU64): 1032547698badcfe

What is right (I could not rebuild with hppa64-linux-gcc :( )?

Thanks,
    Joel


---------------------------------
Tiscali ADSL: 19,50 euros/mois...abonnez-vous sur www.tiscali.be

Re: [parisc-linux] byte swapping redux

[Grant Grundler]

On Tue, Jul 01, 2003 at 06:33:07PM +0200, Joel Soete wrote:
> I hope I am not going to anoying you.

:^)

> I just found related interesting stuff: <http://lwn.net/Articles/38384/>
> And may be also the source of pb.

Maybe our userspace will encounter this problem as well since I've
ripped out the __STRICT_ANSI__ test...can you work this out?


> I also write this small testcase to try to point out the pb in 32bits mode:
...
> And here is the results:
> Val of TU64: fedcba9876543210
> Val of ___arch__swab64(TU64): 76543210fedcba98
> Val of __fswab64(TU64): 1032547698badcfe
> 
> What is right (I could not rebuild with hppa64-linux-gcc :( )?

__fswab64() is correct.
___arch__swab64() didn't actually swap within the  32-bit words.


asm-parisc/byteorder.h has:
static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
{
	__u32 t1 = (__u32) x;
	__u32 t2 = (__u32) ((x) >> 32);
	___arch__swab32(t1);
	___arch__swab32(t2);
	return (((__u64) t2 << 32) + ((__u64) t1));
}


DOH! *smack*
Probably my bad.
That should read:

static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
{
	__u32 t1 = (__u32) x;
	__u32 t2 = (__u32) ((x) >> 32);
	return ((__u64) ___arch__swab32(t1) << 32) + ((__u64) ___arch__swab32(t2));     
}


Could you please test this?

thanks,
grant

Re: [parisc-linux] byte swapping redux

[Joel Soete]

[-- Attachment #1: Type: text/plain, Size: 1800 bytes --]


> Maybe our userspace will encounter this problem as well since I've
> ripped out the __STRICT_ANSI__ test...can you work this out?

I will see what I can do for this 

> > I also write this small testcase to try to point out the pb in 32bits
mode:
> ...
> > And here is the results:
> > Val of TU64: fedcba9876543210
> > Val of ___arch__swab64(TU64): 76543210fedcba98
> > Val of __fswab64(TU64): 1032547698badcfe
[...]
> __fswab64() is correct.
> ___arch__swab64() didn't actually swap within the  32-bit word
[...]

static __inline__ __const__ __u64 ___arch__swab64(__u64 x)
{
	__u32 t1 = (__u32) x;
	__u32 t2 = (__u32) ((x) >> 32);
	return ((__u64) ___arch__swab32(t1) << 32) + ((__u64) ___arch__swab32(t2))
>     
}

That works for my 32bits kernel on my b2k :-) (just replace 't1' by 'l' and
't2' by 'h' ;) )

btw here is also a small patch I just test also for cdrom pb encounter by
somebody else:
--- system_irqsave.h.orig	2003-07-02 17:34:02.000000000 +0200
+++ system_irqsave.h	2003-07-02 17:35:59.000000000 +0200
@@ -7,8 +7,13 @@
 #define __cli()	__asm__ __volatile__("rsm %0,%%r0\n" : : "i" (PSW_I) : "memory"
)
 #define __sti()	__asm__ __volatile__("ssm %0,%%r0\n" : : "i" (PSW_I) : "memory"
)
 
+#define __save_and_cli(x)  do { __save_flags(x); __cli(); } while(0);
+#define __save_and_sti(x)  do { __save_flags(x); __sti(); } while(0);
+
+/* For spinlocks etc */
 #define local_irq_save(x) \
 	__asm__ __volatile__("rsm %1,%0" : "=r" (x) :"i" (PSW_I) : "memory" )
+#define local_irq_set(x)   __save_and_sti(x)
 #define local_irq_restore(x) \
 	__asm__ __volatile__("mtsm %0" : : "r" (x) : "memory" )
 #define local_irq_disable() __cli()

( may be possible to do better ??)

Joel





---------------------------------
Tiscali ADSL: 19,50 euros/mois...abonnez-vous sur www.tiscali.be



[-- Attachment #2: system_irqsave-patch --]
[-- Type: application/octet-stream, Size: 747 bytes --]

--- system_irqsave.h.orig	2003-07-02 17:34:02.000000000 +0200
+++ system_irqsave.h	2003-07-02 17:35:59.000000000 +0200
@@ -7,8 +7,13 @@
 #define __cli()	__asm__ __volatile__("rsm %0,%%r0\n" : : "i" (PSW_I) : "memory" )
 #define __sti()	__asm__ __volatile__("ssm %0,%%r0\n" : : "i" (PSW_I) : "memory" )
 
+#define __save_and_cli(x)  do { __save_flags(x); __cli(); } while(0);
+#define __save_and_sti(x)  do { __save_flags(x); __sti(); } while(0);
+
+/* For spinlocks etc */
 #define local_irq_save(x) \
 	__asm__ __volatile__("rsm %1,%0" : "=r" (x) :"i" (PSW_I) : "memory" )
+#define local_irq_set(x)   __save_and_sti(x)
 #define local_irq_restore(x) \
 	__asm__ __volatile__("mtsm %0" : : "r" (x) : "memory" )
 #define local_irq_disable() __cli()