lkml.org 
[lkml]   [2008]   [Jul]   [17]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH] elf loader support for auxvec base platform string
    On Thu, 17 Jul 2008 16:35:39 +1000 Benjamin Herrenschmidt <benh@kernel.crashing.org> 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 <ntl@pobox.com>
    > > ---
    > > 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


    \
     
     \ /
      Last update: 2008-07-17 09:13    [W:0.026 / U:0.148 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site