uml and -regparm=3

uml and -regparm=3

[Miklos Szeredi]

FASTCALL is defined empty in -mm, but UML is not compiled with
-mregparm=3 and so this breaks things (I noticed problems with
rwsem_down_write_failed).

Tried recompiling UML with -mregparm=3, but that resulted in a strange
failure immediately after startup:

|^[%G�^[%@: Invalid argument

What's up?

Miklos

Re: uml and -regparm=3

[Andi Kleen]

Miklos Szeredi <miklos@szeredi.hu> writes:

> FASTCALL is defined empty in -mm, but UML is not compiled with
> -mregparm=3 and so this breaks things (I noticed problems with
> rwsem_down_write_failed).
>
> Tried recompiling UML with -mregparm=3, but that resulted in a strange
> failure immediately after startup:
>
> |%G�%@: Invalid argument
>
> What's up?

UML links with glibc and that does not use -mregparm. 

You can only use -mregparm in user space if you recompile
all libraries too.

-Andi

Re: uml and -regparm=3

[Miklos Szeredi]

> Miklos Szeredi <miklos@szeredi.hu> writes:
> 
> > FASTCALL is defined empty in -mm, but UML is not compiled with
> > -mregparm=3 and so this breaks things (I noticed problems with
> > rwsem_down_write_failed).
> >
> > Tried recompiling UML with -mregparm=3, but that resulted in a strange
> > failure immediately after startup:
> >
> > |%G�%@: Invalid argument
> >
> > What's up?
> 
> UML links with glibc and that does not use -mregparm. 
> 
> You can only use -mregparm in user space if you recompile
> all libraries too.

Ah, right.  I didn't think of that.

Does that mean that FASTCALL removals will have to be undone?  Or is
there an alternative?

Miklos

Re: uml and -regparm=3

[Andi Kleen]

On Wed, Jan 09, 2008 at 10:20:49PM +0100, Miklos Szeredi wrote:
> > Miklos Szeredi <miklos@szeredi.hu> writes:
> > 
> > > FASTCALL is defined empty in -mm, but UML is not compiled with
> > > -mregparm=3 and so this breaks things (I noticed problems with
> > > rwsem_down_write_failed).
> > >
> > > Tried recompiling UML with -mregparm=3, but that resulted in a strange
> > > failure immediately after startup:
> > >
> > > |%G�%@: Invalid argument
> > >
> > > What's up?
> > 
> > UML links with glibc and that does not use -mregparm. 
> > 
> > You can only use -mregparm in user space if you recompile
> > all libraries too.
> 
> Ah, right.  I didn't think of that.
> 
> Does that mean that FASTCALL removals will have to be undone?  Or is
> there an alternative?

FASTCALL is useless and should not make a difference. It enables
regparm on specific functions, but that should not make a difference
if it works or not.

For working -mregparm you will need asmlinkage on all
functions that interface with assembler code to disable regparms for them.
The only exception is system calls which have the arguments on the stack
too.

-Andi

Re: uml and -regparm=3

[Miklos Szeredi]

> FASTCALL is useless and should not make a difference. It enables
> regparm on specific functions, but that should not make a difference
> if it works or not.

__down_write() in include/asm-x86/rwsem.h seems to assume, that the
semaphore pointer is passed in %eax down to rwsem_down_write_failed(),
so regparm does make a difference there.

There's also some intervening magic in arch/x86/lib/semaphore_32.S,
that I don't quite understand, but which doesn't seem to make a
difference.

Miklos

Re: uml and -regparm=3

[Jeff Dike]

On Wed, Jan 09, 2008 at 10:50:48PM +0100, Miklos Szeredi wrote:
> > FASTCALL is useless and should not make a difference. It enables
> > regparm on specific functions, but that should not make a difference
> > if it works or not.
> 
> __down_write() in include/asm-x86/rwsem.h seems to assume, that the
> semaphore pointer is passed in %eax down to rwsem_down_write_failed(),
> so regparm does make a difference there.

And rwsem_down_write_failed seems to think it's getting the pointer in
%eax:

Dump of assembler code for function rwsem_down_write_failed:
0x08193599 <rwsem_down_write_failed+0>: push   %ebp
0x0819359a <rwsem_down_write_failed+1>: mov    %esp,%ebp
0x0819359c <rwsem_down_write_failed+3>: push   %ebx
0x0819359d <rwsem_down_write_failed+4>: mov    %eax,%ebx
0x0819359f <rwsem_down_write_failed+6>: sub    $0x10,%esp
0x081935a2 <rwsem_down_write_failed+9>: push   $0xffffffff
0x081935a4 <rwsem_down_write_failed+11>:        lea    0xffffffec(%ebp),%eax
0x081935a7 <rwsem_down_write_failed+14>:        push   %eax
0x081935a8 <rwsem_down_write_failed+15>:        push   %ebx
0x081935a9 <rwsem_down_write_failed+16>:        movl   $0x2,0xfffffff8(%ebp)
0x081935b0 <rwsem_down_write_failed+23>:        call   0x8193423 <rwsem_down_failed_common>

This is clearly taking something from %eax and something on the stack
(and a -1) and passing it to rwsem_down_failed_common, corresponding
to this:
	rwsem_down_failed_common(sem, &waiter,
				RWSEM_WAITING_BIAS - RWSEM_ACTIVE_BIAS);

So, this does look right to me.

				Jeff

-- 
Work email - jdike at linux dot intel dot com

Re: uml and -regparm=3

[Andi Kleen]

On Wed, Jan 09, 2008 at 09:14:04PM -0500, Jeff Dike wrote:
> On Wed, Jan 09, 2008 at 10:50:48PM +0100, Miklos Szeredi wrote:
> > > FASTCALL is useless and should not make a difference. It enables
> > > regparm on specific functions, but that should not make a difference
> > > if it works or not.
> > 
> > __down_write() in include/asm-x86/rwsem.h seems to assume, that the
> > semaphore pointer is passed in %eax down to rwsem_down_write_failed(),
> > so regparm does make a difference there.
> 
> And rwsem_down_write_failed seems to think it's getting the pointer in
> %eax:

That's a left over from when it was called by magic inline assembly.
I suspect it would be better to switch it just over to stack args
for consistent and then declare it asmlinkage

-Andi

Re: uml and -regparm=3

[H. Peter Anvin]

Andi Kleen wrote:
> On Wed, Jan 09, 2008 at 09:14:04PM -0500, Jeff Dike wrote:
>> On Wed, Jan 09, 2008 at 10:50:48PM +0100, Miklos Szeredi wrote:
>>>> FASTCALL is useless and should not make a difference. It enables
>>>> regparm on specific functions, but that should not make a difference
>>>> if it works or not.
>>> __down_write() in include/asm-x86/rwsem.h seems to assume, that the
>>> semaphore pointer is passed in %eax down to rwsem_down_write_failed(),
>>> so regparm does make a difference there.
>> And rwsem_down_write_failed seems to think it's getting the pointer in
>> %eax:
> 
> That's a left over from when it was called by magic inline assembly.
> I suspect it would be better to switch it just over to stack args
> for consistent and then declare it asmlinkage
> 

That seems to go in the wrong direction to me.  It would make more sense 
to gradually shift to asmlinkage actually being regparm=3.  This, of 
course, would require a different name (such as FASTCALL) until the 
conversion is complete -- even if done as a single patch series.

	-hpa

Re: uml and -regparm=3

[Adrian Bunk]

On Wed, Jan 09, 2008 at 10:20:49PM +0100, Miklos Szeredi wrote:
> > Miklos Szeredi <miklos@szeredi.hu> writes:
> > 
> > > FASTCALL is defined empty in -mm, but UML is not compiled with
> > > -mregparm=3 and so this breaks things (I noticed problems with
> > > rwsem_down_write_failed).
> > >
> > > Tried recompiling UML with -mregparm=3, but that resulted in a strange
> > > failure immediately after startup:
> > >
> > > |%G�%@: Invalid argument
> > >
> > > What's up?
> > 
> > UML links with glibc and that does not use -mregparm. 
> > 
> > You can only use -mregparm in user space if you recompile
> > all libraries too.
> 
> Ah, right.  I didn't think of that.
> 
> Does that mean that FASTCALL removals will have to be undone?  Or is
> there an alternative?

It's enough when we keep fastcall/FASTCALL in the few cases where UML 
calls assembler code with this calling convention. [1]

> Miklos

cu
Adrian

[1] http://lkml.org/lkml/2007/12/4/425

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: uml and -regparm=3

[linux-os (Dick Johnson)]

On Wed, 9 Jan 2008, Miklos Szeredi wrote:

> FASTCALL is defined empty in -mm, but UML is not compiled with
> -mregparm=3 and so this breaks things (I noticed problems with
> rwsem_down_write_failed).
>
> Tried recompiling UML with -mregparm=3, but that resulted in a strange
> failure immediately after startup:
>
> |^[%Gÿÿÿÿÿÿ^[%@: Invalid argument
>
> What's up?
>
> Miklos
> --

UML gets linked with the 'C' runtime library. It doesn't
pass parameters in registers unless you get the source and
recompile the whole thing --and then you would have to
recompile all your applications, too. Yikes!


Cheers,
Dick Johnson
Penguin : Linux version 2.6.22.1 on an i686 machine (5588.29 BogoMips).
My book : http://www.AbominableFirebug.com/
_

****************************************************************
The information transmitted in this message is confidential and may be privileged.  Any review, retransmission, dissemination, or other use of this information by persons or entities other than the intended recipient is prohibited.  If you are not the intended recipient, please notify Analogic Corporation immediately - by replying to this message or by sending an email to DeliveryErrors@analogic.com - and destroy all copies of this information, including any attachments, without reading or disclosing them.

Thank you.

Re: uml and -regparm=3

[Ingo Molnar]

* Miklos Szeredi <miklos@szeredi.hu> wrote:

> FASTCALL is defined empty in -mm, but UML is not compiled with 
> -mregparm=3 and so this breaks things (I noticed problems with 
> rwsem_down_write_failed).
> 
> Tried recompiling UML with -mregparm=3, but that resulted in a strange 
> failure immediately after startup:
> 
> |^[%G�^[%@: Invalid argument
> 
> What's up?

Miklos, could you try the fix below?

In general most FASTCALL/fastcall uses are bogus, except for code where 
a function that takes parameters is implemented in assembly with a 
regparm calling convention. The fix is to introduce the "asmregparm" 
attribute to mark such function prototypes with regparm(3). This is the 
opposite of asmlinkage. [asmlinkage forced regparm(0)]

	Ingo

------------>
Subject: x86: fix UML calling convention
From: Ingo Molnar <mingo@elte.hu>

introduce the "asmregparm" calling convention: for functions
implemented in assembly with a fixed regparm input parameters
calling convention.

mark the semaphore and rwsem slowpath functions with that.

Reported-by: Miklos Szeredi <miklos@szeredi.hu>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 include/asm-x86/linkage.h      |    5 +++++
 include/asm-x86/rwsem.h        |   12 ++++++++----
 include/asm-x86/semaphore_32.h |    8 ++++----
 3 files changed, 17 insertions(+), 8 deletions(-)

Index: linux-x86.q/include/asm-x86/linkage.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/linkage.h
+++ linux-x86.q/include/asm-x86/linkage.h
@@ -9,6 +9,11 @@
 #ifdef CONFIG_X86_32
 #define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
 #define prevent_tail_call(ret) __asm__ ("" : "=r" (ret) : "0" (ret))
+/*
+ * For 32-bit UML - mark functions implemented in assembly that use
+ * regparm input parameters:
+ */
+#define asmregparm __attribute__((regparm(3)))
 #endif
 
 #ifdef CONFIG_X86_ALIGNMENT_16
Index: linux-x86.q/include/asm-x86/rwsem.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/rwsem.h
+++ linux-x86.q/include/asm-x86/rwsem.h
@@ -44,10 +44,14 @@
 
 struct rwsem_waiter;
 
-extern struct rw_semaphore *FASTCALL(rwsem_down_read_failed(struct rw_semaphore *sem));
-extern struct rw_semaphore *FASTCALL(rwsem_down_write_failed(struct rw_semaphore *sem));
-extern struct rw_semaphore *FASTCALL(rwsem_wake(struct rw_semaphore *));
-extern struct rw_semaphore *FASTCALL(rwsem_downgrade_wake(struct rw_semaphore *sem));
+extern asmregparm struct rw_semaphore *
+ rwsem_down_read_failed(struct rw_semaphore *sem);
+extern asmregparm struct rw_semaphore *
+ rwsem_down_write_failed(struct rw_semaphore *sem);
+extern asmregparm struct rw_semaphore *
+ rwsem_wake(struct rw_semaphore *);
+extern asmregparm struct rw_semaphore *
+ rwsem_downgrade_wake(struct rw_semaphore *sem);
 
 /*
  * the semaphore definition
Index: linux-x86.q/include/asm-x86/semaphore_32.h
===================================================================
--- linux-x86.q.orig/include/asm-x86/semaphore_32.h
+++ linux-x86.q/include/asm-x86/semaphore_32.h
@@ -83,10 +83,10 @@ static inline void init_MUTEX_LOCKED (st
 	sema_init(sem, 0);
 }
 
-void __down_failed(void /* special register calling convention */);
-int  __down_failed_interruptible(void  /* params in registers */);
-int  __down_failed_trylock(void  /* params in registers */);
-void __up_wakeup(void /* special register calling convention */);
+extern asmregparm void __down_failed(atomic_t *count_ptr);
+extern asmregparm int  __down_failed_interruptible(atomic_t *count_ptr);
+extern asmregparm int  __down_failed_trylock(atomic_t *count_ptr);
+extern asmregparm void __up_wakeup(atomic_t *count_ptr);
 
 /*
  * This is ugly, but we want the default case to fall through.

Re: uml and -regparm=3

[Miklos Szeredi]

> * Miklos Szeredi <miklos@szeredi.hu> wrote:
> 
> > FASTCALL is defined empty in -mm, but UML is not compiled with 
> > -mregparm=3 and so this breaks things (I noticed problems with 
> > rwsem_down_write_failed).
> > 
> > Tried recompiling UML with -mregparm=3, but that resulted in a strange 
> > failure immediately after startup:
> > 
> > |^[%G�^[%@: Invalid argument
> > 
> > What's up?
> 
> Miklos, could you try the fix below?
> 
> In general most FASTCALL/fastcall uses are bogus, except for code where 
> a function that takes parameters is implemented in assembly with a 
> regparm calling convention. The fix is to introduce the "asmregparm" 
> attribute to mark such function prototypes with regparm(3). This is the 
> opposite of asmlinkage. [asmlinkage forced regparm(0)]

Thanks.  Patch works, but needed to add asmregparm to the definitions
as well, plus added default define into <linux/linkage.h> (updated
patch below).

Miklos
----

Subject: x86: fix UML calling convention
From: Ingo Molnar <mingo@elte.hu>

introduce the "asmregparm" calling convention: for functions
implemented in assembly with a fixed regparm input parameters
calling convention.

mark the semaphore and rwsem slowpath functions with that.

Signed-off-by: Ingo Molnar <mingo@elte.hu>
Signed-off-by: Miklos Szeredi <mszeredi@suse.cz>
---

Index: linux/include/asm-x86/linkage.h
===================================================================
--- linux.orig/include/asm-x86/linkage.h	2008-01-09 21:16:29.000000000 +0100
+++ linux/include/asm-x86/linkage.h	2008-01-10 09:38:17.000000000 +0100
@@ -9,6 +9,11 @@
 #ifdef CONFIG_X86_32
 #define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))
 #define prevent_tail_call(ret) __asm__ ("" : "=r" (ret) : "0" (ret))
+/*
+ * For 32-bit UML - mark functions implemented in assembly that use
+ * regparm input parameters:
+ */
+#define asmregparm __attribute__((regparm(3)))
 #endif
 
 #ifdef CONFIG_X86_ALIGNMENT_16
Index: linux/include/asm-x86/rwsem.h
===================================================================
--- linux.orig/include/asm-x86/rwsem.h	2008-01-09 21:16:29.000000000 +0100
+++ linux/include/asm-x86/rwsem.h	2008-01-10 09:55:07.000000000 +0100
@@ -44,10 +44,14 @@
 
 struct rwsem_waiter;
 
-extern struct rw_semaphore *FASTCALL(rwsem_down_read_failed(struct rw_semaphore *sem));
-extern struct rw_semaphore *FASTCALL(rwsem_down_write_failed(struct rw_semaphore *sem));
-extern struct rw_semaphore *FASTCALL(rwsem_wake(struct rw_semaphore *));
-extern struct rw_semaphore *FASTCALL(rwsem_downgrade_wake(struct rw_semaphore *sem));
+extern asmregparm struct rw_semaphore *
+ rwsem_down_read_failed(struct rw_semaphore *sem);
+extern asmregparm struct rw_semaphore *
+ rwsem_down_write_failed(struct rw_semaphore *sem);
+extern asmregparm struct rw_semaphore *
+ rwsem_wake(struct rw_semaphore *);
+extern asmregparm struct rw_semaphore *
+ rwsem_downgrade_wake(struct rw_semaphore *sem);
 
 /*
  * the semaphore definition
Index: linux/include/asm-x86/semaphore_32.h
===================================================================
--- linux.orig/include/asm-x86/semaphore_32.h	2008-01-09 21:16:29.000000000 +0100
+++ linux/include/asm-x86/semaphore_32.h	2008-01-10 09:38:17.000000000 +0100
@@ -83,10 +83,10 @@ static inline void init_MUTEX_LOCKED (st
 	sema_init(sem, 0);
 }
 
-void __down_failed(void /* special register calling convention */);
-int  __down_failed_interruptible(void  /* params in registers */);
-int  __down_failed_trylock(void  /* params in registers */);
-void __up_wakeup(void /* special register calling convention */);
+extern asmregparm void __down_failed(atomic_t *count_ptr);
+extern asmregparm int  __down_failed_interruptible(atomic_t *count_ptr);
+extern asmregparm int  __down_failed_trylock(atomic_t *count_ptr);
+extern asmregparm void __up_wakeup(atomic_t *count_ptr);
 
 /*
  * This is ugly, but we want the default case to fall through.
Index: linux/lib/rwsem.c
===================================================================
--- linux.orig/lib/rwsem.c	2008-01-09 21:16:30.000000000 +0100
+++ linux/lib/rwsem.c	2008-01-10 09:55:55.000000000 +0100
@@ -187,7 +187,7 @@ rwsem_down_failed_common(struct rw_semap
 /*
  * wait for the read lock to be granted
  */
-struct rw_semaphore __sched *
+asmregparm struct rw_semaphore __sched *
 rwsem_down_read_failed(struct rw_semaphore *sem)
 {
 	struct rwsem_waiter waiter;
@@ -201,7 +201,7 @@ rwsem_down_read_failed(struct rw_semapho
 /*
  * wait for the write lock to be granted
  */
-struct rw_semaphore __sched *
+asmregparm struct rw_semaphore __sched *
 rwsem_down_write_failed(struct rw_semaphore *sem)
 {
 	struct rwsem_waiter waiter;
@@ -216,7 +216,7 @@ rwsem_down_write_failed(struct rw_semaph
  * handle waking up a waiter on the semaphore
  * - up_read/up_write has decremented the active part of count if we come here
  */
-struct rw_semaphore *rwsem_wake(struct rw_semaphore *sem)
+asmregparm struct rw_semaphore *rwsem_wake(struct rw_semaphore *sem)
 {
 	unsigned long flags;
 
@@ -236,7 +236,7 @@ struct rw_semaphore *rwsem_wake(struct r
  * - caller incremented waiting part of count and discovered it still negative
  * - just wake up any readers at the front of the queue
  */
-struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
+asmregparm struct rw_semaphore *rwsem_downgrade_wake(struct rw_semaphore *sem)
 {
 	unsigned long flags;
 
Index: linux/include/linux/linkage.h
===================================================================
--- linux.orig/include/linux/linkage.h	2008-01-09 21:16:04.000000000 +0100
+++ linux/include/linux/linkage.h	2008-01-10 10:01:52.000000000 +0100
@@ -13,6 +13,10 @@
 #define asmlinkage CPP_ASMLINKAGE
 #endif
 
+#ifndef asmregparm
+#define asmregparm
+#endif
+
 #ifndef prevent_tail_call
 # define prevent_tail_call(ret) do { } while (0)
 #endif

Re: uml and -regparm=3

[Ingo Molnar]

* Miklos Szeredi <miklos@szeredi.hu> wrote:

> Thanks.  Patch works, but needed to add asmregparm to the definitions 
> as well, plus added default define into <linux/linkage.h> (updated 
> patch below).

thanks, applied.

	Ingo