[PATCH 1/7]: Fix sparc64 setjmp implementation.

[PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

This corrects the sparc64 setjmp implementation.

We need to store the return address register, the
stack pointer, and frame pointer into the jump buf.

And on longjmp we need restore those registers, flush the register
window state, and pull in the top-most register window.

2009-03-03  David S. Miller  <davem@davemloft.net>

	* normal/sparc64/setjmp.S: Fix setjmp implementation.
	* include/grub/sparc64/setjmp.h: Update grub_jmp_buf[].
---
 include/grub/sparc64/setjmp.h |    3 +--
 normal/sparc64/setjmp.S       |   18 +++++++++++++-----
 2 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/include/grub/sparc64/setjmp.h b/include/grub/sparc64/setjmp.h
index 12d8e01..183a820 100644
--- a/include/grub/sparc64/setjmp.h
+++ b/include/grub/sparc64/setjmp.h
@@ -19,8 +19,7 @@
 #ifndef GRUB_SETJMP_CPU_HEADER
 #define GRUB_SETJMP_CPU_HEADER	1
 
-/* FIXME (sparc64).  */
-typedef unsigned long grub_jmp_buf[20];
+typedef unsigned long grub_jmp_buf[3];
 
 int grub_setjmp (grub_jmp_buf env);
 void grub_longjmp (grub_jmp_buf env, int val) __attribute__ ((noreturn));
diff --git a/normal/sparc64/setjmp.S b/normal/sparc64/setjmp.S
index b1a9b6e..02e61b1 100644
--- a/normal/sparc64/setjmp.S
+++ b/normal/sparc64/setjmp.S
@@ -26,13 +26,21 @@
  * int grub_setjmp (grub_jmp_buf env)
  */
 FUNCTION(grub_setjmp)
-        ret
-         nop
+	stx	%o7, [%o0 + 0x00]
+	stx	%sp, [%o0 + 0x08]
+	retl
+	 stx	%fp, [%o0 + 0x10]
 
 /*
  * int grub_longjmp (grub_jmp_buf env, int val)
  */
 FUNCTION(grub_longjmp)
-        ret
-         nop
-
+	ldx	[%o0 + 0x10], %g1
+	movrz	%o1, 1, %o1
+	ta	0x3
+	ldx	[%o0 + 0x00], %o7
+	ldx	[%o0 + 0x08], %fp
+	sub	%fp, 192, %sp
+	stx	%g1, [%sp + 2047 + (14 * 8)]
+        retl
+	 restore %o1, 0, %o0
-- 
1.6.1.2.253.ga34a

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Vesa Jääskeläinen]

David Miller wrote:

> +	stx	%g1, [%sp + 2047 + (14 * 8)]
> +        retl
> +	 restore %o1, 0, %o0

Please check your white spaces. As we can see here those three commands
are on different columns (mixed tabs and spaces). This applies to every
other patch you sent too...

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Vesa Jääskeläinen <chaac@nic.fi>
Date: Wed, 04 Mar 2009 12:59:37 +0200

> David Miller wrote:
> 
> > +	stx	%g1, [%sp + 2047 + (14 * 8)]
> > +        retl
> > +	 restore %o1, 0, %o0
> 
> Please check your white spaces. As we can see here those three commands
> are on different columns (mixed tabs and spaces). This applies to every
> other patch you sent too...

I used plain ASCII text for my patch postings.

I just double checked and they are perfectly fine and applyable in my
email client.

Also note that in the delay slot of control transfer instructions, I
do intentionally use TAB then one space, as this is common practice
for sparc assembler to visually denote the delay slot instruction.

Even GCC outputs assembler using this convention :-)

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Vesa Jääskeläinen]

David Miller wrote:
> This corrects the sparc64 setjmp implementation.
> 
> We need to store the return address register, the
> stack pointer, and frame pointer into the jump buf.
> 
> And on longjmp we need restore those registers, flush the register
> window state, and pull in the top-most register window.
> 
> 2009-03-03  David S. Miller  <davem@davemloft.net>
> 
> 	* normal/sparc64/setjmp.S: Fix setjmp implementation.
> 	* include/grub/sparc64/setjmp.h: Update grub_jmp_buf[].
> ---
>  include/grub/sparc64/setjmp.h |    3 +--
>  normal/sparc64/setjmp.S       |   18 +++++++++++++-----
>  2 files changed, 14 insertions(+), 7 deletions(-)
> 
> diff --git a/include/grub/sparc64/setjmp.h b/include/grub/sparc64/setjmp.h
> index 12d8e01..183a820 100644
> --- a/include/grub/sparc64/setjmp.h
> +++ b/include/grub/sparc64/setjmp.h
> @@ -19,8 +19,7 @@
>  #ifndef GRUB_SETJMP_CPU_HEADER
>  #define GRUB_SETJMP_CPU_HEADER	1
>  
> -/* FIXME (sparc64).  */
> -typedef unsigned long grub_jmp_buf[20];
> +typedef unsigned long grub_jmp_buf[3];

I assume unsigned long is 64bit in sparc?

Would it be more practical to use grub_uint64_t?

Are there any other registers that needs to be preserved ?

Other than that I cannot decipher more from that asm :). If you have
tested it and it works. It probably ain't bad idea to commit this patch.

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Vesa Jääskeläinen <chaac@nic.fi>
Date: Wed, 04 Mar 2009 13:34:46 +0200

> David Miller wrote:
> > diff --git a/include/grub/sparc64/setjmp.h b/include/grub/sparc64/setjmp.h
> > index 12d8e01..183a820 100644
> > --- a/include/grub/sparc64/setjmp.h
> > +++ b/include/grub/sparc64/setjmp.h
> > @@ -19,8 +19,7 @@
> >  #ifndef GRUB_SETJMP_CPU_HEADER
> >  #define GRUB_SETJMP_CPU_HEADER	1
> >  
> > -/* FIXME (sparc64).  */
> > -typedef unsigned long grub_jmp_buf[20];
> > +typedef unsigned long grub_jmp_buf[3];
> 
> I assume unsigned long is 64bit in sparc?

Yes, for sparc64 long is always 64-bit, no exceptions.

> Would it be more practical to use grub_uint64_t?

This is a "sparc64" file, so I don't think so.

> Are there any other registers that needs to be preserved ?

Not for GRUB, no.

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Javier Martín]

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

El mié, 04-03-2009 a las 03:36 -0800, David Miller escribió:
> From: Vesa Jääskeläinen <chaac@nic.fi>
> Date: Wed, 04 Mar 2009 13:34:46 +0200
> 
> > David Miller wrote:
> > > diff --git a/include/grub/sparc64/setjmp.h b/include/grub/sparc64/setjmp.h
> > > index 12d8e01..183a820 100644
> > > --- a/include/grub/sparc64/setjmp.h
> > > +++ b/include/grub/sparc64/setjmp.h
> > > @@ -19,8 +19,7 @@
> > >  #ifndef GRUB_SETJMP_CPU_HEADER
> > >  #define GRUB_SETJMP_CPU_HEADER	1
> > >  
> > > -/* FIXME (sparc64).  */
> > > -typedef unsigned long grub_jmp_buf[20];
> > > +typedef unsigned long grub_jmp_buf[3];
> > 
> > I assume unsigned long is 64bit in sparc?
> 
> Yes, for sparc64 long is always 64-bit, no exceptions.
> 
> > Would it be more practical to use grub_uint64_t?
> 
> This is a "sparc64" file, so I don't think so.
Next month, or maybe next year, someone will need to fix some bug on
sparc64 code without being sparc64-savvy, maybe because (like it's
already happened) there are no sparc people available. That day people
like Bean, Felix, Marco or Robert (to name just a few of the big gurus
here) may have to decide between editing a sparc64 file or give up on
maintaining the platform. That day it would be really good for them to
visually know that "unsigned long" is 64-bit in sparc, instead of having
to go see the docs. This is the real utility of typedefs (and the
[u]intNN_t types from C99): if you statically _know_ that some variable
is going to be 64 bits long because the _platform_ requires it, why not
make it explicit? It will be easier for readers and maintainers.

As an example of what this entails, the failure to think with the future
in mind has extraordinarily hampered the 32->64 bit transition: many
Windows apps (among them the Vorbis codecs!) had be nearly rewritten
because they used "unsigned long" interchangeably with pointer types. I
still wonder if it would have been so difficult to use "void*", and the
proper types for pointer arithmetic.

-- 
-- Lazy, Oblivious, Rational Disaster -- Habbit

[-- Attachment #2: Esta parte del mensaje está firmada digitalmente --]
[-- Type: application/pgp-signature, Size: 835 bytes --]

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Javier Martín <lordhabbit@gmail.com>
Date: Wed, 04 Mar 2009 16:58:22 +0100

> As an example of what this entails, the failure to think with the future
> in mind has extraordinarily hampered the 32->64 bit transition: many
> Windows apps (among them the Vorbis codecs!) had be nearly rewritten
> because they used "unsigned long" interchangeably with pointer types. I
> still wonder if it would have been so difficult to use "void*", and the
> proper types for pointer arithmetic.

Actually the problem is that they used "int" interchangably with
pointer types.

"unsigned long" is always safe because it is going to be
the largest natural word size on the machine, at least
as large as a pointer will be.

We've been using "unsigned long" for storing virtual addresses in the
Linux kernel for 10+ years and it works just fine.  :-)

Really, nobody is going to touch the grub_jmp_buf[] implementation
other than sparc expert.  So saying this case will cause problems for
grub non-sparc folks is something of a scarecrow.

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Isaac Dupree]

David Miller wrote:
> "unsigned long" is always safe because it is going to be
> the largest natural word size on the machine, at least
> as large as a pointer will be.

er... C standard doesn't guarantee this, and I think that Windows in fact has 
32-bit longs on machines with 64-bit pointers

> We've been using "unsigned long" for storing virtual addresses in the
> Linux kernel for 10+ years and it works just fine.  :-)

and Linux kernel uses GCC compiler in precise ways

I believe that ptrdiff_t is the proper standardized type for an integer the 
size of a pointer.  except... it's always signed :-)

Can you just use pointer types and pointer arithmetic?

also, standards aside, a common way to get such a type, is "configure" script 
testing various possibilities like "unsigned long" and "unsigned long long" 
and seeing which one is the right size for the target architecture. (not sure 
if that works when cross-compiling though)

-Isaac

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Isaac Dupree <ml@isaac.cedarswampstudios.org>
Date: Thu, 5 Mar 2009 08:04:07 -0500

> David Miller wrote:
> > "unsigned long" is always safe because it is going to be
> > the largest natural word size on the machine, at least
> > as large as a pointer will be.
> 
> er... C standard doesn't guarantee this, and I think that Windows in fact has 
> 32-bit longs on machines with 64-bit pointers

I look forward to that MSVC compile of sparc64 GRUB.

Anyways I'm using grub_uint64_t in the current spin of my
patches.

You type lawyers win, so let's just drop this.

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Robert Millan]

On Thu, Mar 05, 2009 at 08:04:07AM -0500, Isaac Dupree wrote:
> > We've been using "unsigned long" for storing virtual addresses in the
> > Linux kernel for 10+ years and it works just fine.  :-)
> 
> and Linux kernel uses GCC compiler in precise ways
> 
> I believe that ptrdiff_t is the proper standardized type for an integer the 
> size of a pointer.  except... it's always signed :-)
> 
> Can you just use pointer types and pointer arithmetic?
> 
> also, standards aside, a common way to get such a type, is "configure" script 
> testing various possibilities like "unsigned long" and "unsigned long long" 
> and seeing which one is the right size for the target architecture. (not sure 
> if that works when cross-compiling though)

I think we're already using longs this way in quite a few places.  It's not
such a big deal IMHO.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Pavel Roskin]

On Fri, 2009-03-06 at 19:55 +0100, Robert Millan wrote:
> On Thu, Mar 05, 2009 at 08:04:07AM -0500, Isaac Dupree wrote:
> > > We've been using "unsigned long" for storing virtual addresses in the
> > > Linux kernel for 10+ years and it works just fine.  :-)
> > 
> > and Linux kernel uses GCC compiler in precise ways
> > 
> > I believe that ptrdiff_t is the proper standardized type for an integer the 
> > size of a pointer.  except... it's always signed :-)
> > 
> > Can you just use pointer types and pointer arithmetic?
> > 
> > also, standards aside, a common way to get such a type, is "configure" script 
> > testing various possibilities like "unsigned long" and "unsigned long long" 
> > and seeing which one is the right size for the target architecture. (not sure 
> > if that works when cross-compiling though)
> 
> I think we're already using longs this way in quite a few places.  It's not
> such a big deal IMHO.

I was considering making grub_size_t long and grub_ssize_t unsigned
long.  I remember that it required many changes in string formats, so I
didn't feel it would be justified.  But we could try it again.

-- 
Regards,
Pavel Roskin

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Robert Millan <rmh@aybabtu.com>
Date: Fri, 6 Mar 2009 19:55:53 +0100

> On Thu, Mar 05, 2009 at 08:04:07AM -0500, Isaac Dupree wrote:
> > > We've been using "unsigned long" for storing virtual addresses in the
> > > Linux kernel for 10+ years and it works just fine.  :-)
> > 
> > and Linux kernel uses GCC compiler in precise ways
> > 
> > I believe that ptrdiff_t is the proper standardized type for an integer the 
> > size of a pointer.  except... it's always signed :-)
> > 
> > Can you just use pointer types and pointer arithmetic?
> > 
> > also, standards aside, a common way to get such a type, is "configure" script 
> > testing various possibilities like "unsigned long" and "unsigned long long" 
> > and seeing which one is the right size for the target architecture. (not sure 
> > if that works when cross-compiling though)
> 
> I think we're already using longs this way in quite a few places.  It's not
> such a big deal IMHO.

And ironically enough, using "unsigned long" would allow us to
share this header with any future 32-bit sparc GRUB support.

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Pavel Roskin <proski@gnu.org>
Date: Fri, 06 Mar 2009 14:03:57 -0500

> On Fri, 2009-03-06 at 19:55 +0100, Robert Millan wrote:
> > On Thu, Mar 05, 2009 at 08:04:07AM -0500, Isaac Dupree wrote:
> > > > We've been using "unsigned long" for storing virtual addresses in the
> > > > Linux kernel for 10+ years and it works just fine.  :-)
> > > 
> > > and Linux kernel uses GCC compiler in precise ways
> > > 
> > > I believe that ptrdiff_t is the proper standardized type for an integer the 
> > > size of a pointer.  except... it's always signed :-)
> > > 
> > > Can you just use pointer types and pointer arithmetic?
> > > 
> > > also, standards aside, a common way to get such a type, is "configure" script 
> > > testing various possibilities like "unsigned long" and "unsigned long long" 
> > > and seeing which one is the right size for the target architecture. (not sure 
> > > if that works when cross-compiling though)
> > 
> > I think we're already using longs this way in quite a few places.  It's not
> > such a big deal IMHO.
> 
> I was considering making grub_size_t long and grub_ssize_t unsigned
> long.  I remember that it required many changes in string formats, so I
> didn't feel it would be justified.  But we could try it again.

You should use whatever is the appropriate size_t/ssize_t type on
the given platform+ABI, and that way GRUB could use "%Zd" and/or
"%zd" throughout.  Was that the plan?

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Pavel Roskin]

Quoting David Miller <davem@davemloft.net>:

>> I was considering making grub_size_t long and grub_ssize_t unsigned
>> long.  I remember that it required many changes in string formats, so I
>> didn't feel it would be justified.  But we could try it again.
>
> You should use whatever is the appropriate size_t/ssize_t type on
> the given platform+ABI, and that way GRUB could use "%Zd" and/or
> "%zd" throughout.  Was that the plan?

No, the plan was to use long.  We don't support architectures where  
long and size_t have different sizes.  I'm not aware of such  
architectures.  I don't care about win64, as it would need some very  
special handling (perhaps redefinition of long).

I was trying to fix an inconsistency of the userspace, where size_t  
can be unsigned int or unsigned long.  Perhaps the plan was too  
ambitious.

-- 
Regards,
Pavel Roskin

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Robert Millan]

On Sat, Mar 07, 2009 at 12:56:43AM -0500, Pavel Roskin wrote:
> Quoting David Miller <davem@davemloft.net>:
>
>>> I was considering making grub_size_t long and grub_ssize_t unsigned
>>> long.  I remember that it required many changes in string formats, so I
>>> didn't feel it would be justified.  But we could try it again.
>>
>> You should use whatever is the appropriate size_t/ssize_t type on
>> the given platform+ABI, and that way GRUB could use "%Zd" and/or
>> "%zd" throughout.  Was that the plan?
>
> No, the plan was to use long.  We don't support architectures where long 
> and size_t have different sizes.  I'm not aware of such architectures.  I 
> don't care about win64, as it would need some very special handling 
> (perhaps redefinition of long).

Maybe Christian can shred some light on this ;-)

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[Robert Millan]

On Fri, Mar 06, 2009 at 04:34:47PM -0800, David Miller wrote:
> > 
> > I think we're already using longs this way in quite a few places.  It's not
> > such a big deal IMHO.
> 
> And ironically enough, using "unsigned long" would allow us to
> share this header with any future 32-bit sparc GRUB support.

Are there any GNU/Linux distributions still supporting 32-bit sparc?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.

[David Miller]

From: Robert Millan <rmh@aybabtu.com>
Date: Sat, 7 Mar 2009 11:38:38 +0100

> On Sat, Mar 07, 2009 at 12:56:43AM -0500, Pavel Roskin wrote:
> > Quoting David Miller <davem@davemloft.net>:
> >
> >>> I was considering making grub_size_t long and grub_ssize_t unsigned
> >>> long.  I remember that it required many changes in string formats, so I
> >>> didn't feel it would be justified.  But we could try it again.
> >>
> >> You should use whatever is the appropriate size_t/ssize_t type on
> >> the given platform+ABI, and that way GRUB could use "%Zd" and/or
> >> "%zd" throughout.  Was that the plan?
> >
> > No, the plan was to use long.  We don't support architectures where long 
> > and size_t have different sizes.  I'm not aware of such architectures.  I 
> > don't care about win64, as it would need some very special handling 
> > (perhaps redefinition of long).
> 
> Maybe Christian can shred some light on this ;-)

I've checked a lot of cases (other than win64) and this assumption
that sizeof(long)==sizeof(size_t) seems to hold.