[PATCH] sparc32: fix __context corruption in __switch_context_nosave

[Artyom Tarasenko]

__switch_context_nosave shall not write to __context, only read it.

Signed-off-by: Artyom Tarasenko 
--- arch/sparc32/switch.S       (revision 1024)
+++ arch/sparc32/switch.S       (working copy)
@@ -79,11 +79,14 @@
        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
        wr      %g2, 0x0, %psr
 #endif
-
+        set     __context, %g1
+        swap      [%g1], %fp
+        ba     __set_context
 __switch_context_nosave:
-       set     __context, %g1
+        set     __context, %g1
         /* Swap ctx pointer with %fp */
-        swap      [%g1], %fp
+        ld      [%g1], %fp
+__set_context:
        /* Load all registers */
         /* offset 0: %g0, no need to load */
         ld      [%fp + 4], %g1

---
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

[Blue Swirl]

On Wed, Feb 9, 2011 at 8:37 PM, Artyom Tarasenko  wrote:
> __switch_context_nosave shall not write to __context, only read it.
>
> Signed-off-by: Artyom Tarasenko 
> --- arch/sparc32/switch.S       (revision 1024)
> +++ arch/sparc32/switch.S       (working copy)
> @@ -79,11 +79,14 @@
>        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
>        wr      %g2, 0x0, %psr
>  #endif
> set     __context, %g1
> swap      [%g1], %fp
> ba     __set_context
>
> __switch_context_nosave:
> set     __context, %g1
> set     __context, %g1
>
> /* Swap ctx pointer with %fp */

Shouldn't this comment be moved earlier, close to swap?
> swap      [%g1], %fp
> ld      [%g1], %fp
>
> +__set_context:
>        /* Load all registers */
>         /* offset 0: %g0, no need to load */
>         ld      [%fp + 4], %g1
>
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/

[Artyom Tarasenko]

On Fri, Feb 11, 2011 at 7:52 PM, Blue Swirl  wrote:
> On Wed, Feb 9, 2011 at 8:37 PM, Artyom Tarasenko  wrote:
> > __switch_context_nosave shall not write to __context, only read it.
> >
> > Signed-off-by: Artyom Tarasenko 
> > --- arch/sparc32/switch.S       (revision 1024)
> > +++ arch/sparc32/switch.S       (working copy)
> > @@ -79,11 +79,14 @@
> >        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
> >        wr      %g2, 0x0, %psr
> >  #endif
> > set     __context, %g1
> > swap      [%g1], %fp
> > ba     __set_context
> >
> > __switch_context_nosave:
> > set     __context, %g1
> > set     __context, %g1
> >
> > /* Swap ctx pointer with %fp */
>
> Shouldn't this comment be moved earlier, close to swap?

Good catch. I'll send an updated version.
> > swap      [%g1], %fp
> > ld      [%g1], %fp
> >
> > +__set_context:
> >        /* Load all registers */
> >         /* offset 0: %g0, no need to load */
> >         ld      [%fp + 4], %g1

[PATCH v2] sparc32: fix __context corruption in __switch_context_nosave

[Artyom Tarasenko]

__switch_context_nosave shall not write to __context, only read it.

Signed-off-by: Artyom Tarasenko 
--- arch/sparc32/switch.S       (revision 1024)
+++ arch/sparc32/switch.S       (working copy)
@@ -79,11 +79,15 @@
        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
        wr      %g2, 0x0, %psr
 #endif
-
-__switch_context_nosave:
-       set     __context, %g1
+        set     __context, %g1
         /* Swap ctx pointer with %fp */
         swap      [%g1], %fp
+        ba     __set_context
+__switch_context_nosave:
+        set     __context, %g1
+        /* Load %fp from ctx pointer */
+        ld      [%g1], %fp
+__set_context:
        /* Load all registers */
         /* offset 0: %g0, no need to load */
         ld      [%fp + 4], %g1
---
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

[PATCH v2] sparc32: fix __context corruption in __switch_context_nosave

[Blue Swirl]

On Fri, Feb 11, 2011 at 9:35 PM, Artyom Tarasenko  wrote:
> __switch_context_nosave shall not write to __context, only read it.
>
> Signed-off-by: Artyom Tarasenko 
> --- arch/sparc32/switch.S       (revision 1024)
> +++ arch/sparc32/switch.S       (working copy)
> @@ -79,11 +79,15 @@
>        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
>        wr      %g2, 0x0, %psr
>  #endif
>
> -__switch_context_nosave:
> set     __context, %g1
> set     __context, %g1
>
> /* Swap ctx pointer with %fp */
>         swap      [%g1], %fp
> ba     __set_context

This will execute also 'sethi' part of the following 'set', please add
a 'nop' (with indent increased by one space).
> +__switch_context_nosave:
> set     __context, %g1
> /* Load %fp from ctx pointer */
> ld      [%g1], %fp
>
> +__set_context:
>        /* Load all registers */
>         /* offset 0: %g0, no need to load */
>         ld      [%fp + 4], %g1
>
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/

[PATCH v2] sparc32: fix __context corruption in __switch_context_nosave

[Artyom Tarasenko]

On Fri, Feb 11, 2011 at 8:41 PM, Blue Swirl  wrote:
> On Fri, Feb 11, 2011 at 9:35 PM, Artyom Tarasenko  wrote:
> > __switch_context_nosave shall not write to __context, only read it.
> >
> > Signed-off-by: Artyom Tarasenko 
> > --- arch/sparc32/switch.S       (revision 1024)
> > +++ arch/sparc32/switch.S       (working copy)
> > @@ -79,11 +79,15 @@
> >        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
> >        wr      %g2, 0x0, %psr
> >  #endif
> >
> > -__switch_context_nosave:
> > set     __context, %g1
> > set     __context, %g1
> >
> > /* Swap ctx pointer with %fp */
> >         swap      [%g1], %fp
> > ba     __set_context
>
> This will execute also 'sethi' part of the following 'set', please add
> a 'nop' (with indent increased by one space).

Maybe just switch them?
ba     __set_context
swap      [%g1], %fp

Indentation is broken in the original file I guess. It has mixed tabs
and spaces.
> > +__switch_context_nosave:
> > set     __context, %g1
> > /* Load %fp from ctx pointer */
> > ld      [%g1], %fp
> >
> > +__set_context:
> >        /* Load all registers */
> >         /* offset 0: %g0, no need to load */
> >         ld      [%fp + 4], %g1

[PATCH v2] sparc32: fix __context corruption in __switch_context_nosave

[Blue Swirl]

On Fri, Feb 11, 2011 at 9:50 PM, Artyom Tarasenko  wrote:
> On Fri, Feb 11, 2011 at 8:41 PM, Blue Swirl  wrote:
> > On Fri, Feb 11, 2011 at 9:35 PM, Artyom Tarasenko  wrote:
> > > __switch_context_nosave shall not write to __context, only read it.
> > >
> > > Signed-off-by: Artyom Tarasenko 
> > > --- arch/sparc32/switch.S       (revision 1024)
> > > +++ arch/sparc32/switch.S       (working copy)
> > > @@ -79,11 +79,15 @@
> > >        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
> > >        wr      %g2, 0x0, %psr
> > >  #endif
> > >
> > > -__switch_context_nosave:
> > > set     __context, %g1
> > > set     __context, %g1
> > >
> > > /* Swap ctx pointer with %fp */
> > >         swap      [%g1], %fp
> > > ba     __set_context
> >
> > This will execute also 'sethi' part of the following 'set', please add
> > a 'nop' (with indent increased by one space).
>
> Maybe just switch them?
> ba     __set_context
> swap      [%g1], %fp

That's even better.
> Indentation is broken in the original file I guess. It has mixed tabs
> and spaces.

Well, the formatting convention for instructions in delay slots is
that they should be indented slightly to make them stand out.

[PATCH v3] sparc32: fix __context corruption in __switch_context_nosave

[Artyom Tarasenko]

__switch_context_nosave shall not write to __context, only read it.

Signed-off-by: Artyom Tarasenko 
--- arch/sparc32/switch.S       (revision 1024)
+++ arch/sparc32/switch.S       (working copy)
@@ -79,11 +79,15 @@
        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
        wr      %g2, 0x0, %psr
 #endif
-
+        set     __context, %g1
+        /* Swap ctx pointer with %fp and jump*/
+        ba     __set_context
+         swap      [%g1], %fp
 __switch_context_nosave:
-       set     __context, %g1
-        /* Swap ctx pointer with %fp */
-        swap      [%g1], %fp
+        set     __context, %g1
+        /* load %fp from ctx pointer */
+        ld      [%g1], %fp
+__set_context:
        /* Load all registers */
         /* offset 0: %g0, no need to load */
         ld      [%fp + 4], %g1

---
Regards,
Artyom Tarasenko

solaris/sparc under qemu blog: http://tyom.blogspot.com/

[PATCH v3] sparc32: fix __context corruption in __switch_context_nosave

[Blue Swirl]

Thanks, applied.

On Sat, Feb 12, 2011 at 3:21 PM, Artyom Tarasenko  wrote:
> __switch_context_nosave shall not write to __context, only read it.
>
> Signed-off-by: Artyom Tarasenko 
> --- arch/sparc32/switch.S       (revision 1024)
> +++ arch/sparc32/switch.S       (working copy)
> @@ -79,11 +79,15 @@
>        set     (PSR_PS | PSR_S | PSR_PIL | PSR_EF), %g2
>        wr      %g2, 0x0, %psr
>  #endif
> set     __context, %g1
> /* Swap ctx pointer with %fp and jump*/
> ba     __set_context
> swap      [%g1], %fp
>
> __switch_context_nosave:
> set     __context, %g1
> /* Swap ctx pointer with %fp */
> swap      [%g1], %fp
> set     __context, %g1
> /* load %fp from ctx pointer */
> ld      [%g1], %fp
>
> +__set_context:
>        /* Load all registers */
>         /* offset 0: %g0, no need to load */
>         ld      [%fp + 4], %g1
>
> Regards,
> Artyom Tarasenko
>
> solaris/sparc under qemu blog: http://tyom.blogspot.com/