Re: [PATCH] elf loader support for auxvec base platform string

2008-07-20 Thread Benjamin Herrenschmidt 
On Thu, 2008-07-17 at 09:10 -0700, Linus Torvalds wrote:
 
 On Thu, 17 Jul 2008, Benjamin Herrenschmidt wrote:
  
  Should I seek somebody's ack before merging a patch like the one below ?
  
  I'm a bit reluctant to merge via the powerpc.git tree some changes to
  generic files without at least an ack from somebody else :-)
 
 Gaah. Generally we don't, but you're right to ask. The ELF loading keeps 
 on accumulating these things, and I'm not sure we have the right process 
 for things like this. They're all individually small and insignificant, 
 and they are all individually never going away and making the ELF loader 
 messier and messier.

Yeah, annoying. Oh well, this one at least is now getting Andrew's
scrutinity...

Cheers,
Ben.


___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] elf loader support for auxvec base platform string

2008-07-17 Thread Benjamin Herrenschmidt 
Hi Linus, Andrew !

Should I seek somebody's ack before merging a patch like the one below ?

I'm a bit reluctant to merge via the powerpc.git tree some changes to
generic files without at least an ack from somebody else :-)

There have been some debate on whether this AT_BASE_PLATFORM is the
right approach, though I haven't seen them reach any useful conclusion
and our toolchain people internally are screaming for it...

Cheers,
Ben.

On Tue, 2008-07-15 at 18:58 -0500, Nathan Lynch wrote:
 Some IBM POWER-based platforms have the ability to run in a
 mode which mostly appears to the OS as a different processor from the
 actual hardware.  For example, a Power6 system may appear to be a
 Power5+, which makes the AT_PLATFORM value power5+.  This means that
 programs are restricted to the ISA supported by Power5+;
 Power6-specific instructions are treated as illegal.
 
 However, some applications (virtual machines, optimized libraries) can
 benefit from knowledge of the underlying CPU model.  A new aux vector
 entry, AT_BASE_PLATFORM, will denote the actual hardware.  For
 example, on a Power6 system in Power5+ compatibility mode, AT_PLATFORM
 will be power5+ and AT_BASE_PLATFORM will be power6.  The idea is
 that AT_PLATFORM indicates the instruction set supported, while
 AT_BASE_PLATFORM indicates the underlying microarchitecture.
 
 If the architecture has defined ELF_BASE_PLATFORM, copy that value to
 the user stack in the same manner as ELF_PLATFORM.
 
 Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
 ---
  fs/binfmt_elf.c|   23 +++
  include/linux/auxvec.h |5 -
  2 files changed, 27 insertions(+), 1 deletions(-)
 
 diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
 index d48ff5f..834c2c4 100644
 --- a/fs/binfmt_elf.c
 +++ b/fs/binfmt_elf.c
 @@ -131,6 +131,10 @@ static int padzero(unsigned long elf_bss)
  #define STACK_ALLOC(sp, len) ({ sp -= len ; sp; })
  #endif
  
 +#ifndef ELF_BASE_PLATFORM
 +#define ELF_BASE_PLATFORM NULL
 +#endif
 +
  static int
  create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
   unsigned long load_addr, unsigned long interp_load_addr)
 @@ -142,7 +146,9 @@ create_elf_tables(struct linux_binprm *bprm, struct 
 elfhdr *exec,
   elf_addr_t __user *envp;
   elf_addr_t __user *sp;
   elf_addr_t __user *u_platform;
 + elf_addr_t __user *u_base_platform;
   const char *k_platform = ELF_PLATFORM;
 + const char *k_base_platform = ELF_BASE_PLATFORM;
   int items;
   elf_addr_t *elf_info;
   int ei_index = 0;
 @@ -172,6 +178,19 @@ create_elf_tables(struct linux_binprm *bprm, struct 
 elfhdr *exec,
   return -EFAULT;
   }
  
 + /*
 +  * If this architecture has a base platform capability
 +  * string, copy it to userspace.
 +  */
 + u_base_platform = NULL;
 + if (k_base_platform) {
 + size_t len = strlen(k_base_platform) + 1;
 +
 + u_base_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
 + if (__copy_to_user(u_base_platform, k_base_platform, len))
 + return -EFAULT;
 + }
 +
   /* Create the ELF interpreter info */
   elf_info = (elf_addr_t *)current-mm-saved_auxv;
   /* update AT_VECTOR_SIZE_BASE if the number of NEW_AUX_ENT() changes */
 @@ -208,6 +227,10 @@ create_elf_tables(struct linux_binprm *bprm, struct 
 elfhdr *exec,
   NEW_AUX_ENT(AT_PLATFORM,
   (elf_addr_t)(unsigned long)u_platform);
   }
 + if (k_base_platform) {
 + NEW_AUX_ENT(AT_BASE_PLATFORM,
 + (elf_addr_t)(unsigned long)u_base_platform);
 + }
   if (bprm-interp_flags  BINPRM_FLAGS_EXECFD) {
   NEW_AUX_ENT(AT_EXECFD, bprm-interp_data);
   }
 diff --git a/include/linux/auxvec.h b/include/linux/auxvec.h
 index ad89545..1adc61d 100644
 --- a/include/linux/auxvec.h
 +++ b/include/linux/auxvec.h
 @@ -26,8 +26,11 @@
  
  #define AT_SECURE 23   /* secure mode boolean */
  
 +#define AT_BASE_PLATFORM 38  /* string identifying real platform, may
 +  * differ from AT_PLATFORM. */
 +
  #ifdef __KERNEL__
 -#define AT_VECTOR_SIZE_BASE (14 + 2) /* NEW_AUX_ENT entries in auxiliary 
 table */
 +#define AT_VECTOR_SIZE_BASE (14 + 3) /* NEW_AUX_ENT entries in auxiliary 
 table */
  #endif
  
  #endif /* _LINUX_AUXVEC_H */

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] elf loader support for auxvec base platform string

2008-07-17 Thread Andrew Morton 
On Thu, 17 Jul 2008 16:35:39 +1000 Benjamin Herrenschmidt [EMAIL PROTECTED] 
wrote:

 Hi Linus, Andrew !
 
 Should I seek somebody's ack before merging a patch like the one below ?

I think it's good to do so.

 I'm a bit reluctant to merge via the powerpc.git tree some changes to
 generic files without at least an ack from somebody else :-)

It tends to happen.  People often don't notice unless it a) crashes or
b) spits warnings or c) screws up my tree or d) all the above plus
more.

 There have been some debate on whether this AT_BASE_PLATFORM is the
 right approach, though I haven't seen them reach any useful conclusion
 and our toolchain people internally are screaming for it...
 
 Cheers,
 Ben.
 
 On Tue, 2008-07-15 at 18:58 -0500, Nathan Lynch wrote:
  Some IBM POWER-based platforms have the ability to run in a
  mode which mostly appears to the OS as a different processor from the
  actual hardware.  For example, a Power6 system may appear to be a
  Power5+, which makes the AT_PLATFORM value power5+.  This means that
  programs are restricted to the ISA supported by Power5+;
  Power6-specific instructions are treated as illegal.
  
  However, some applications (virtual machines, optimized libraries) can
  benefit from knowledge of the underlying CPU model.  A new aux vector
  entry, AT_BASE_PLATFORM, will denote the actual hardware.  For
  example, on a Power6 system in Power5+ compatibility mode, AT_PLATFORM
  will be power5+ and AT_BASE_PLATFORM will be power6.  The idea is
  that AT_PLATFORM indicates the instruction set supported, while
  AT_BASE_PLATFORM indicates the underlying microarchitecture.
  
  If the architecture has defined ELF_BASE_PLATFORM, copy that value to
  the user stack in the same manner as ELF_PLATFORM.
  
  Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
  ---
   fs/binfmt_elf.c|   23 +++
   include/linux/auxvec.h |5 -
   2 files changed, 27 insertions(+), 1 deletions(-)
  
  diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
  index d48ff5f..834c2c4 100644
  --- a/fs/binfmt_elf.c
  +++ b/fs/binfmt_elf.c
  @@ -131,6 +131,10 @@ static int padzero(unsigned long elf_bss)
   #define STACK_ALLOC(sp, len) ({ sp -= len ; sp; })
   #endif
   
  +#ifndef ELF_BASE_PLATFORM
  +#define ELF_BASE_PLATFORM NULL
  +#endif

Please add a comment which explains what this is.

Please also add a comment telling the world in which header file the
architecture *must* define this macro and then ensure that that header is
included into this file by reliable means.  asm/elf.h looks OK.

   static int
   create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
  unsigned long load_addr, unsigned long interp_load_addr)
  @@ -142,7 +146,9 @@ create_elf_tables(struct linux_binprm *bprm, struct 
  elfhdr *exec,
  elf_addr_t __user *envp;
  elf_addr_t __user *sp;
  elf_addr_t __user *u_platform;
  +   elf_addr_t __user *u_base_platform;
  const char *k_platform = ELF_PLATFORM;
  +   const char *k_base_platform = ELF_BASE_PLATFORM;
  int items;
  elf_addr_t *elf_info;
  int ei_index = 0;
  @@ -172,6 +178,19 @@ create_elf_tables(struct linux_binprm *bprm, struct 
  elfhdr *exec,
  return -EFAULT;
  }
   
  +   /*
  +* If this architecture has a base platform capability
  +* string, copy it to userspace.
  +*/
  +   u_base_platform = NULL;
  +   if (k_base_platform) {
  +   size_t len = strlen(k_base_platform) + 1;
  +
  +   u_base_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
  +   if (__copy_to_user(u_base_platform, k_base_platform, len))
  +   return -EFAULT;
  +   }

From my reading, this change will result in no additional code
generation on non-powerpc architectures.  This is good.  If poss, could
you please verify that theory and perhaps drop a note in the changelog
about that?


Apart from that - acked-by-me
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] elf loader support for auxvec base platform string

2008-07-17 Thread Linus Torvalds 


On Thu, 17 Jul 2008, Benjamin Herrenschmidt wrote:
 
 Should I seek somebody's ack before merging a patch like the one below ?
 
 I'm a bit reluctant to merge via the powerpc.git tree some changes to
 generic files without at least an ack from somebody else :-)

Gaah. Generally we don't, but you're right to ask. The ELF loading keeps 
on accumulating these things, and I'm not sure we have the right process 
for things like this. They're all individually small and insignificant, 
and they are all individually never going away and making the ELF loader 
messier and messier.

I dunno. 

Linus
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] elf loader support for auxvec base platform string

2008-07-17 Thread Nathan Lynch 
Andrew Morton wrote:
  On Tue, 2008-07-15 at 18:58 -0500, Nathan Lynch wrote:
   diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
   index d48ff5f..834c2c4 100644
   --- a/fs/binfmt_elf.c
   +++ b/fs/binfmt_elf.c
   @@ -131,6 +131,10 @@ static int padzero(unsigned long elf_bss)
#define STACK_ALLOC(sp, len) ({ sp -= len ; sp; })
#endif

   +#ifndef ELF_BASE_PLATFORM
   +#define ELF_BASE_PLATFORM NULL
   +#endif
 
 Please add a comment which explains what this is.
 
 Please also add a comment telling the world in which header file the
 architecture *must* define this macro and then ensure that that header is
 included into this file by reliable means.  asm/elf.h looks OK.

Okay.


   @@ -172,6 +178,19 @@ create_elf_tables(struct linux_binprm *bprm, struct 
   elfhdr *exec,
 return -EFAULT;
 }

   + /*
   +  * If this architecture has a base platform capability
   +  * string, copy it to userspace.
   +  */
   + u_base_platform = NULL;
   + if (k_base_platform) {
   + size_t len = strlen(k_base_platform) + 1;
   +
   + u_base_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
   + if (__copy_to_user(u_base_platform, k_base_platform, len))
   + return -EFAULT;
   + }
 
 From my reading, this change will result in no additional code
 generation on non-powerpc architectures.  This is good.  If poss, could
 you please verify that theory and perhaps drop a note in the changelog
 about that?

That was the intent, yes.  However:

$ scripts/bloat-o-meter vmlinux-x86-{before,after}
add/remove: 0/0 grow/shrink: 2/0 up/down: 18/0 (18)
function old new   delta
init_mm  784 800 +16
load_elf_binary11946   11948  +2

(x86_64_defconfig, gcc 4.2.3)

The init_mm/mm_struct bloat is expected (although I'd like to avoid
that), but evidently it has some small effect on load_elf_binary.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

Re: [PATCH] elf loader support for auxvec base platform string

2008-07-17 Thread Nathan Lynch 
Linus Torvalds wrote:
 
 
 On Thu, 17 Jul 2008, Benjamin Herrenschmidt wrote:
  
  Should I seek somebody's ack before merging a patch like the one below ?
  
  I'm a bit reluctant to merge via the powerpc.git tree some changes to
  generic files without at least an ack from somebody else :-)
 
 Gaah. Generally we don't, but you're right to ask. The ELF loading keeps 
 on accumulating these things, and I'm not sure we have the right process 
 for things like this. They're all individually small and insignificant, 
 and they are all individually never going away and making the ELF loader 
 messier and messier.

FWIW, I was initially reluctant to do it this way, but the ELF aux
vector seems to be the only reasonable mechanism for getting this
information to all interested users.  And the only reason the patch
touches the generic code is because we have to copy a string to
userspace (just like AT_PLATFORM).  Otherwise it could be done in
powerpc's ARCH_DLINFO.
___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev

[PATCH] elf loader support for auxvec base platform string

2008-07-15 Thread Nathan Lynch 
Some IBM POWER-based platforms have the ability to run in a
mode which mostly appears to the OS as a different processor from the
actual hardware.  For example, a Power6 system may appear to be a
Power5+, which makes the AT_PLATFORM value power5+.  This means that
programs are restricted to the ISA supported by Power5+;
Power6-specific instructions are treated as illegal.

However, some applications (virtual machines, optimized libraries) can
benefit from knowledge of the underlying CPU model.  A new aux vector
entry, AT_BASE_PLATFORM, will denote the actual hardware.  For
example, on a Power6 system in Power5+ compatibility mode, AT_PLATFORM
will be power5+ and AT_BASE_PLATFORM will be power6.  The idea is
that AT_PLATFORM indicates the instruction set supported, while
AT_BASE_PLATFORM indicates the underlying microarchitecture.

If the architecture has defined ELF_BASE_PLATFORM, copy that value to
the user stack in the same manner as ELF_PLATFORM.

Signed-off-by: Nathan Lynch [EMAIL PROTECTED]
---
 fs/binfmt_elf.c|   23 +++
 include/linux/auxvec.h |5 -
 2 files changed, 27 insertions(+), 1 deletions(-)

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index d48ff5f..834c2c4 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -131,6 +131,10 @@ static int padzero(unsigned long elf_bss)
 #define STACK_ALLOC(sp, len) ({ sp -= len ; sp; })
 #endif
 
+#ifndef ELF_BASE_PLATFORM
+#define ELF_BASE_PLATFORM NULL
+#endif
+
 static int
 create_elf_tables(struct linux_binprm *bprm, struct elfhdr *exec,
unsigned long load_addr, unsigned long interp_load_addr)
@@ -142,7 +146,9 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr 
*exec,
elf_addr_t __user *envp;
elf_addr_t __user *sp;
elf_addr_t __user *u_platform;
+   elf_addr_t __user *u_base_platform;
const char *k_platform = ELF_PLATFORM;
+   const char *k_base_platform = ELF_BASE_PLATFORM;
int items;
elf_addr_t *elf_info;
int ei_index = 0;
@@ -172,6 +178,19 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr 
*exec,
return -EFAULT;
}
 
+   /*
+* If this architecture has a base platform capability
+* string, copy it to userspace.
+*/
+   u_base_platform = NULL;
+   if (k_base_platform) {
+   size_t len = strlen(k_base_platform) + 1;
+
+   u_base_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
+   if (__copy_to_user(u_base_platform, k_base_platform, len))
+   return -EFAULT;
+   }
+
/* Create the ELF interpreter info */
elf_info = (elf_addr_t *)current-mm-saved_auxv;
/* update AT_VECTOR_SIZE_BASE if the number of NEW_AUX_ENT() changes */
@@ -208,6 +227,10 @@ create_elf_tables(struct linux_binprm *bprm, struct elfhdr 
*exec,
NEW_AUX_ENT(AT_PLATFORM,
(elf_addr_t)(unsigned long)u_platform);
}
+   if (k_base_platform) {
+   NEW_AUX_ENT(AT_BASE_PLATFORM,
+   (elf_addr_t)(unsigned long)u_base_platform);
+   }
if (bprm-interp_flags  BINPRM_FLAGS_EXECFD) {
NEW_AUX_ENT(AT_EXECFD, bprm-interp_data);
}
diff --git a/include/linux/auxvec.h b/include/linux/auxvec.h
index ad89545..1adc61d 100644
--- a/include/linux/auxvec.h
+++ b/include/linux/auxvec.h
@@ -26,8 +26,11 @@
 
 #define AT_SECURE 23   /* secure mode boolean */
 
+#define AT_BASE_PLATFORM 38/* string identifying real platform, may
+* differ from AT_PLATFORM. */
+
 #ifdef __KERNEL__
-#define AT_VECTOR_SIZE_BASE (14 + 2) /* NEW_AUX_ENT entries in auxiliary table 
*/
+#define AT_VECTOR_SIZE_BASE (14 + 3) /* NEW_AUX_ENT entries in auxiliary table 
*/
 #endif
 
 #endif /* _LINUX_AUXVEC_H */
-- 
1.5.6.2

___
Linuxppc-dev mailing list
Linuxppc-dev@ozlabs.org
https://ozlabs.org/mailman/listinfo/linuxppc-dev