All of lore.kernel.org
 help / color / mirror / Atom feed
From: James Hogan <james.hogan@mips.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Kees Cook <keescook@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jiri Kosina <jkosina@suse.cz>, Al Viro <viro@zeniv.linux.org.uk>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@redhat.com>,
	Baoquan He <bhe@redhat.com>, <linux-metag@vger.kernel.org>
Subject: Re: [PATCH 1/2] fs, elf: drop MAP_FIXED usage from elf_map
Date: Mon, 16 Oct 2017 21:02:09 +0100	[thread overview]
Message-ID: <20171016200208.GL15235@jhogan-linux> (raw)
In-Reply-To: <20171016190047.76ry2inllsacd3pv@dhcp22.suse.cz>

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

On Mon, Oct 16, 2017 at 09:00:47PM +0200, Michal Hocko wrote:
> [CCing metag people for the metag elf_map implementation specific. The thread
> starts here http://lkml.kernel.org/r/20171016134446.19910-1-mhocko@kernel.org]
> 
> On Mon 16-10-17 09:39:14, Kees Cook wrote:
> > On Mon, Oct 16, 2017 at 6:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > +               return -EAGAIN;
> > > +       }
> > > +
> > > +       return map_addr;
> > > +}
> > > +
> > >  static unsigned long elf_map(struct file *filep, unsigned long addr,
> > >                 struct elf_phdr *eppnt, int prot, int type,
> > >                 unsigned long total_size)
> > > @@ -366,11 +389,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
> > 
> > elf_map is redirected on metag -- it should probably have its vm_mmap
> > calls adjust too.
> 
> Thanks for spotting this. I am not really familiar with metag. It seems
> to clear MAP_FIXED already
> 	tcm_tag = tcm_lookup_tag(addr);
> 
> 	if (tcm_tag != TCM_INVALID_TAG)
> 		type &= ~MAP_FIXED;
> 
> So if there is a tag the flag is cleared. I do not understand this code
> (and git log doesn't help) but why is this MAP_FIXED code really needed?

This function was added to the metag port in mid-2010 to support ELFs
with tightly coupled memory (TCM) segments, for example metag "core"
memories are at fixed virtual addresses and aren't MMU mappable (i.e.
globally accessible), and are outside of the usual userland address
range, but are as fast as cache. The commit message says this:

> Override the definition of the elf_map() function to special case
> sections that are loaded at the address of the internal memories.
> If we have such a section, map it at a different address and copy
> the contents of the section into the appropriate memory.

So yeh, it looks like if the section is meant to use TCM based on the
virtual address, it drops MAP_FIXED so that the vm_mmap can succeed
(because its outside the normally valid range), and then copies it
directly to the desired TCM so the program can use it.

Hope that helps add some context to understand whats needed.

There was some description of this in an ELCE-2010 talk by the original
author Will Newton that may also be of interest [1].

Cheers
James

[1] http://free-electrons.com/blog/elce-2010-videos/
    "Exploiting On-chip Memories in Embedded Linux Applications"
    See slides about "core memories".

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

WARNING: multiple messages have this Message-ID (diff)
From: James Hogan <james.hogan@mips.com>
To: Michal Hocko <mhocko@kernel.org>
Cc: Kees Cook <keescook@chromium.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Jiri Kosina <jkosina@suse.cz>, Al Viro <viro@zeniv.linux.org.uk>,
	Oleg Nesterov <oleg@redhat.com>, Ingo Molnar <mingo@redhat.com>,
	Baoquan He <bhe@redhat.com>,
	linux-metag@vger.kernel.org
Subject: Re: [PATCH 1/2] fs, elf: drop MAP_FIXED usage from elf_map
Date: Mon, 16 Oct 2017 21:02:09 +0100	[thread overview]
Message-ID: <20171016200208.GL15235@jhogan-linux> (raw)
In-Reply-To: <20171016190047.76ry2inllsacd3pv@dhcp22.suse.cz>

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

On Mon, Oct 16, 2017 at 09:00:47PM +0200, Michal Hocko wrote:
> [CCing metag people for the metag elf_map implementation specific. The thread
> starts here http://lkml.kernel.org/r/20171016134446.19910-1-mhocko@kernel.org]
> 
> On Mon 16-10-17 09:39:14, Kees Cook wrote:
> > On Mon, Oct 16, 2017 at 6:44 AM, Michal Hocko <mhocko@kernel.org> wrote:
> > > +               return -EAGAIN;
> > > +       }
> > > +
> > > +       return map_addr;
> > > +}
> > > +
> > >  static unsigned long elf_map(struct file *filep, unsigned long addr,
> > >                 struct elf_phdr *eppnt, int prot, int type,
> > >                 unsigned long total_size)
> > > @@ -366,11 +389,11 @@ static unsigned long elf_map(struct file *filep, unsigned long addr,
> > 
> > elf_map is redirected on metag -- it should probably have its vm_mmap
> > calls adjust too.
> 
> Thanks for spotting this. I am not really familiar with metag. It seems
> to clear MAP_FIXED already
> 	tcm_tag = tcm_lookup_tag(addr);
> 
> 	if (tcm_tag != TCM_INVALID_TAG)
> 		type &= ~MAP_FIXED;
> 
> So if there is a tag the flag is cleared. I do not understand this code
> (and git log doesn't help) but why is this MAP_FIXED code really needed?

This function was added to the metag port in mid-2010 to support ELFs
with tightly coupled memory (TCM) segments, for example metag "core"
memories are at fixed virtual addresses and aren't MMU mappable (i.e.
globally accessible), and are outside of the usual userland address
range, but are as fast as cache. The commit message says this:

> Override the definition of the elf_map() function to special case
> sections that are loaded at the address of the internal memories.
> If we have such a section, map it at a different address and copy
> the contents of the section into the appropriate memory.

So yeh, it looks like if the section is meant to use TCM based on the
virtual address, it drops MAP_FIXED so that the vm_mmap can succeed
(because its outside the normally valid range), and then copies it
directly to the desired TCM so the program can use it.

Hope that helps add some context to understand whats needed.

There was some description of this in an ELCE-2010 talk by the original
author Will Newton that may also be of interest [1].

Cheers
James

[1] http://free-electrons.com/blog/elce-2010-videos/
    "Exploiting On-chip Memories in Embedded Linux Applications"
    See slides about "core memories".

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2017-10-16 20:03 UTC|newest]

Thread overview: 39+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-04  7:50 MAP_FIXED for ELF mappings Michal Hocko
2017-10-04  7:59 ` Michal Hocko
2017-10-04 15:03 ` Baoquan He
2017-10-04 15:11   ` Michal Hocko
2017-10-04 15:12   ` Baoquan He
2017-10-04 15:17     ` Michal Hocko
2017-10-04 15:37       ` Baoquan He
2017-10-04 17:12         ` Michal Hocko
2017-10-04 17:15           ` Linus Torvalds
2017-10-04 17:28             ` Michal Hocko
2017-10-05 16:33       ` Oleg Nesterov
2017-10-05 16:42         ` Michal Hocko
2017-10-16 13:44 ` [PATCH 0/2] fs, elf: get rid of MAP_FIXED from the loader Michal Hocko
2017-10-16 13:44   ` [PATCH 1/2] fs, elf: drop MAP_FIXED usage from elf_map Michal Hocko
2017-10-16 16:39     ` Kees Cook
2017-10-16 19:00       ` Michal Hocko
2017-10-16 19:00         ` Michal Hocko
2017-10-16 20:02         ` James Hogan [this message]
2017-10-16 20:02           ` James Hogan
2017-10-17  7:37           ` Michal Hocko
2017-10-17  7:37             ` Michal Hocko
2017-10-17  8:35             ` James Hogan
2017-10-17  8:35               ` James Hogan
2017-10-17  8:56               ` Michal Hocko
2017-10-17 12:26     ` Baoquan He
2017-10-17 12:56       ` Michal Hocko
2017-10-17 13:22         ` Baoquan He
2017-10-17 13:33           ` Michal Hocko
2017-10-17 13:42             ` Baoquan He
2017-10-16 13:44   ` [PATCH 2/2] fs, elf: drop MAP_FIXED from initial ET_DYN segment Michal Hocko
2017-10-16 16:44     ` Kees Cook
2017-10-16 18:43       ` Michal Hocko
2017-10-16 19:38         ` Kees Cook
2017-10-17  9:04           ` Michal Hocko
2017-10-17 20:01             ` Kees Cook
2017-10-19 11:20               ` Michal Hocko
2017-10-19 17:19                 ` Kees Cook
2017-10-20  8:45                   ` Michal Hocko
2017-10-20 14:12                     ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171016200208.GL15235@jhogan-linux \
    --to=james.hogan@mips.com \
    --cc=bhe@redhat.com \
    --cc=jkosina@suse.cz \
    --cc=keescook@chromium.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-metag@vger.kernel.org \
    --cc=mhocko@kernel.org \
    --cc=mingo@redhat.com \
    --cc=oleg@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.