Skip to contentSkip to conversation

fix sparc32 mxcc 64 bit read word order

RFC PATCH In review Robert Reif 2 participants 8 messages Nov 15, 2007 – Nov 19, 2007

lore
Share & tools
lore

Robert Reif

author
#0 lore raw

RFC PATCH 1/1 fix sparc32 mxcc 64 bit read word order

This patch fixes the word order for 64 bit reads of the mxcc registers.

It returns the high 32 bits in ret and the lower 32 bits in T0 just like other places in the same function.

T0 is defined as: register uint32_t T0 asm(AREG1);

T0 on my machine has a sizeof = 4. Because of this, I don't think it is necessary to mask off the high bits with 0xffffffff like other places in the same function. You should probably use 0xffffffffULL to mask off the upper 32 bits.

I would remove the & 0xffffffff but I hesitate because T0 is defined "register" uint32_t and I'm not sure what that would really be on 64 bit machines,

Is this patch correct or should I remove the & 0xffffffff here and in the other places in the same function or change them to 0xffffffffULL?

Index: target-sparc/op_helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v retrieving revision 1.52

op_helper.c +4 -4
diff -p -u -r1.52 op_helper.c
@@ -196,8 +196,8 @@ void helper_ld_asi(int asi, int size, in
         switch (T0) {
         case 0x01c00a00: /* MXCC control register */
             if (size == 8) {
-                ret = env->mxccregs[3];
-                T0 = env->mxccregs[3] >> 32;
+                ret = env->mxccregs[3] >> 32;
+                T0 = env->mxccregs[3] & 0xffffffff;
             } else
                 DPRINTF_MXCC("%08x: unimplemented access size: %d\n", T0, size);
             break;

@@ -209,8 +209,8 @@ void helper_ld_asi(int asi, int size, in
             break;
         case 0x01c00f00: /* MBus port address register */
             if (size == 8) {
-                ret = env->mxccregs[7];
-                T0 = env->mxccregs[7] >> 32;
+                ret = env->mxccregs[7] >> 32;
+                T0 = env->mxccregs[7] & 0xffffffff;
             } else
                 DPRINTF_MXCC("%08x: unimplemented access size: %d\n", T0, size);
             break;
#1 lore raw
Quoted 7 lines · Robert Reif

This patch fixes the word order for 64 bit reads of the mxcc registers.

It returns the high 32 bits in ret and the lower 32 bits in T0 just like other places in the same function.

T0 is defined as: register uint32_t T0 asm(AREG1);

Quoted 8 lines

T0 on my machine has a sizeof = 4. Because of this, I don't think it is necessary to mask off the high bits with 0xffffffff like other places in the same function. You should probably use 0xffffffffULL to mask off the upper 32 bits.

I would remove the & 0xffffffff but I hesitate because T0 is defined "register" uint32_t and I'm not sure what that would really be on 64 bit machines,

On my x86_64 sizeof(T0) is also 4. Only if I replace uint32_t with long or uint64_t the size becomes 8, with short the size is 2.

Quoted 3 lines

Is this patch correct or should I remove the & 0xffffffff here and in the other
places in the same function or change them to 0xffffffffULL?

I think it's better to remove them, preferably with a different patch without other changes.

author
#2 lore raw
Quoted 5 lines

This patch fixes the word order for 64 bit reads of the mxcc registers.

Index: target-sparc/op_helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v retrieving revision 1.52

op_helper.c +4 -4
diff -p -u -r1.52 op_helper.c
@@ -196,8 +196,8 @@ void helper_ld_asi(int asi, int size, in
         switch (T0) {
         case 0x01c00a00: /* MXCC control register */
             if (size == 8) {
-                ret = env->mxccregs[3];
-                T0 = env->mxccregs[3] >> 32;
+                ret = env->mxccregs[3] >> 32;
+                T0 = env->mxccregs[3];
             } else
                 DPRINTF_MXCC("%08x: unimplemented access size: %d\n", T0, size);
             break;

@@ -209,8 +209,8 @@ void helper_ld_asi(int asi, int size, in
             break;
         case 0x01c00f00: /* MBus port address register */
             if (size == 8) {
-                ret = env->mxccregs[7];
-                T0 = env->mxccregs[7] >> 32;
+                ret = env->mxccregs[7] >> 32;
+                T0 = env->mxccregs[7];
             } else
                 DPRINTF_MXCC("%08x: unimplemented access size: %d\n", T0, size);
             break;
#3 lore raw
Quoted 3 lines · Robert Reif
Quoted 3 lines

This patch fixes the word order for 64 bit reads of the mxcc registers.

Otherwise everything seems OK, but it breaks NetBSD version 3 on SS10:
clock0 at obio0 slot 0 offset 0x200000: mk48t08
timer0 at obio0 slot 0 offset 0x300000data fault: pc=0xf0111a0c addr=0x0 sfsr=126<PERR=0,LVL=1,AT=1,FT=1,FAV,OW>
panic: kernel fault
halted

halt, power off

Without the patch I get:
clock0 at obio0 slot 0 offset 0x200000: mk48t08 timer0 at obio0 slot 0 offset 0x300000: delay constant 99 zs0 at obio0 slot 0 offset 0x100000 level 12 softpri 6 zstty0 at zs0 channel 0 (console i/o)
zstty1 at zs0 channel 1
scsi-disk: Unsupported command length, command 79

author
#4 lore raw
Quoted 16 lines · Blue Swirl
Quoted 3 lines · Robert Reif
Quoted 3 lines

This patch fixes the word order for 64 bit reads of the mxcc registers.

Otherwise everything seems OK, but it breaks NetBSD version 3 on SS10:
clock0 at obio0 slot 0 offset 0x200000: mk48t08
timer0 at obio0 slot 0 offset 0x300000data fault: pc=0xf0111a0c addr=0x0 sfsr=126<PERR=0,LVL=1,AT=1,FT=1,FAV,OW>
panic: kernel fault
halted

halt, power off

Quoted 12 lines

Without the patch I get:
clock0 at obio0 slot 0 offset 0x200000: mk48t08 timer0 at obio0 slot 0 offset 0x300000: delay constant 99 zs0 at obio0 slot 0 offset 0x100000 level 12 softpri 6 zstty0 at zs0 channel 0 (console i/o)
zstty1 at zs0 channel 1
scsi-disk: Unsupported command length, command 79

This is a classic case of two wrongs make a right. OpenBios need to be fixed to set mbus module id to start at 8, not 0 for mbus based machines.

obio.c +9 -1
Index: drivers/obio.c
===================================================================
@@ -891,7 +891,15 @@
         push_str("cache-coherence?");
         fword("property");
 
-        PUSH(i);
+	switch (machine_id) {
+        case 0x71:
+        case 0x72:
+            PUSH(i + 8);
+            break;
+        case 0x80:
+            PUSH(i);
+            break;
+        }
         fword("encode-int");
         push_str("mid");
         fword("property");
#5 lore raw
Quoted 30 lines · Robert Reif
Quoted 16 lines · Blue Swirl
Quoted 3 lines · Robert Reif
Quoted 3 lines

This patch fixes the word order for 64 bit reads of the mxcc registers.

Otherwise everything seems OK, but it breaks NetBSD version 3 on SS10:
clock0 at obio0 slot 0 offset 0x200000: mk48t08
timer0 at obio0 slot 0 offset 0x300000data fault: pc=0xf0111a0c addr=0x0 sfsr=126<PERR=0,LVL=1,AT=1,FT=1,FAV,OW>
panic: kernel fault
halted

halt, power off

Quoted 12 lines

Without the patch I get:
clock0 at obio0 slot 0 offset 0x200000: mk48t08 timer0 at obio0 slot 0 offset 0x300000: delay constant 99 zs0 at obio0 slot 0 offset 0x100000 level 12 softpri 6 zstty0 at zs0 channel 0 (console i/o)
zstty1 at zs0 channel 1
scsi-disk: Unsupported command length, command 79

Quoted 2 lines

This is a classic case of two wrongs make a right. OpenBios need to be fixed to set mbus module id to start at 8, not 0 for mbus based machines.

Turbosparc manual says that the module id is hardwired to 0x8, so would it be OK if mid was i + 8 for all machines?

author
#6 lore raw
Quoted 40 lines · Blue Swirl
Quoted 37 lines · Robert Reif
Quoted 33 lines · Blue Swirl
Quoted 5 lines · Robert Reif
Quoted 5 lines

This patch fixes the word order for 64 bit reads of the mxcc registers.

Otherwise everything seems OK, but it breaks NetBSD version 3 on SS10:
clock0 at obio0 slot 0 offset 0x200000: mk48t08
timer0 at obio0 slot 0 offset 0x300000data fault: pc=0xf0111a0c addr=0x0 sfsr=126<PERR=0,LVL=1,AT=1,FT=1,FAV,OW>
panic: kernel fault
halted

halt, power off

Without the patch I get:
clock0 at obio0 slot 0 offset 0x200000: mk48t08 timer0 at obio0 slot 0 offset 0x300000: delay constant 99 zs0 at obio0 slot 0 offset 0x100000 level 12 softpri 6 zstty0 at zs0 channel 0 (console i/o)
zstty1 at zs0 channel 1
scsi-disk: Unsupported command length, command 79

Quoted 12 lines
Quoted 4 lines

This is a classic case of two wrongs make a right. OpenBios need to be fixed to set mbus module id to start at 8, not 0 for mbus based machines.

Turbosparc manual says that the module id is hardwired to 0x8, so would it be OK if mid was i + 8 for all machines?

Probably but I don't know for sure. I just fired up a SPARCclassic X which has a microSPARC CPU and the mid was 0. You may need to check the CPU type for the SS5. I don't have the time right now to dig out a microSPARC SS5 to check it out.

#7 lore raw
In reply to Robert Reif
Quoted 48 lines · Robert Reif
Quoted 16 lines · Blue Swirl
Quoted 3 lines · Robert Reif
Quoted 3 lines

This patch fixes the word order for 64 bit reads of the mxcc registers.

Otherwise everything seems OK, but it breaks NetBSD version 3 on SS10:
clock0 at obio0 slot 0 offset 0x200000: mk48t08
timer0 at obio0 slot 0 offset 0x300000data fault: pc=0xf0111a0c addr=0x0 sfsr=126<PERR=0,LVL=1,AT=1,FT=1,FAV,OW>
panic: kernel fault
halted

halt, power off

Quoted 12 lines

Without the patch I get:
clock0 at obio0 slot 0 offset 0x200000: mk48t08 timer0 at obio0 slot 0 offset 0x300000: delay constant 99 zs0 at obio0 slot 0 offset 0x100000 level 12 softpri 6 zstty0 at zs0 channel 0 (console i/o)
zstty1 at zs0 channel 1
scsi-disk: Unsupported command length, command 79

This is a classic case of two wrongs make a right. OpenBios need to be fixed to set mbus module id to start at 8, not 0 for mbus based machines.

Index: drivers/obio.c
===================================================================

obio.c (working copy) renamed from obio.c (revision 178) +2 -1
@@ -891,7 +891,15 @@
         push_str("cache-coherence?");
         fword("property");
 
-        PUSH(i);
+       switch (machine_id) {
+        case 0x71:

obio.c (working copy) line 905

Quoted 10 lines
obio.c (working copy) renamed from obio.c (revision 178) +7 -0
+        case 0x72:
+            PUSH(i + 8);
+            break;
+        case 0x80:
+            PUSH(i);
+            break;
+        }
         fword("encode-int");
         push_str("mid");
         fword("property");

Thanks, both patches applied.