[Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Robert Reif]

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

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?

[-- Attachment #2: mxcc.diff.txt --]
[-- Type: text/plain, Size: 1284 bytes --]

Index: target-sparc/op_helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v
retrieving revision 1.52
diff -p -u -r1.52 op_helper.c
--- target-sparc/op_helper.c	11 Nov 2007 19:46:09 -0000	1.52
+++ target-sparc/op_helper.c	15 Nov 2007 12:27:05 -0000
@@ -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;

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Blue Swirl]

On 11/15/07, Robert Reif <reif@earthlink.net> wrote:
> 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,

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.

> 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.

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Robert Reif]

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

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


[-- Attachment #2: mxcc.diff.txt --]
[-- Type: text/plain, Size: 1258 bytes --]

Index: target-sparc/op_helper.c
===================================================================
RCS file: /sources/qemu/qemu/target-sparc/op_helper.c,v
retrieving revision 1.52
diff -p -u -r1.52 op_helper.c
--- target-sparc/op_helper.c	11 Nov 2007 19:46:09 -0000	1.52
+++ target-sparc/op_helper.c	15 Nov 2007 23:04:48 -0000
@@ -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;

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Blue Swirl]

On 11/16/07, Robert Reif <reif@earthlink.net> wrote:
> >
> >
> >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

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Robert Reif]

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

Blue Swirl wrote:

>On 11/16/07, Robert Reif <reif@earthlink.net> wrote:
>  
>
>>>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
>
>
>
>  
>
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.



[-- Attachment #2: obio.diff.txt --]
[-- Type: text/plain, Size: 533 bytes --]

Index: drivers/obio.c
===================================================================
--- drivers/obio.c	(revision 178)
+++ drivers/obio.c	(working copy)
@@ -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");

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Blue Swirl]

On 11/18/07, Robert Reif <reif@earthlink.net> wrote:
> Blue Swirl wrote:
>
> >On 11/16/07, Robert Reif <reif@earthlink.net> wrote:
> >
> >
> >>>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
> >
> >
> >
> >
> >
> 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?

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Robert Reif]

Blue Swirl wrote:

>On 11/18/07, Robert Reif <reif@earthlink.net> wrote:
>  
>
>>Blue Swirl wrote:
>>
>>    
>>
>>>On 11/16/07, Robert Reif <reif@earthlink.net> wrote:
>>>
>>>
>>>      
>>>
>>>>>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
>>>
>>>
>>>
>>>
>>>
>>>      
>>>
>>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.

Re: [Qemu-devel] [RFC][PATCH] fix sparc32 mxcc 64 bit read word order

[Blue Swirl]

On 11/18/07, Robert Reif <reif@earthlink.net> wrote:
> Blue Swirl wrote:
>
> >On 11/16/07, Robert Reif <reif@earthlink.net> wrote:
> >
> >
> >>>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
> >
> >
> >
> >
> >
> 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
> ===================================================================
> --- drivers/obio.c      (revision 178)
> +++ drivers/obio.c      (working copy)
> @@ -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");
>
>

Thanks, both patches applied.