linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Gabriel Paubert <paubert@iram.es>
To: Wrobel Heinz-R39252 <r39252@freescale.com>
Cc: Michael Ellerman <michael@ellerman.id.au>,
	Steffen Rumler <steffen.rumler.ext@nsn.com>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: kernel panic during kernel module load (powerpc specific part)
Date: Thu, 31 May 2012 13:04:39 +0200	[thread overview]
Message-ID: <20120531110439.GA17373@visitor2.iram.es> (raw)
In-Reply-To: <192298D25D96A042975E372855100DB70FEA87@039-SN2MPN1-011.039d.mgd.msft.net>

On Thu, May 31, 2012 at 07:04:42AM +0000, Wrobel Heinz-R39252 wrote:
> Michael,
> 
> > On Wed, 2012-05-30 at 16:33 +0200, Steffen Rumler wrote:
> > > I've found the following root cause:
> > >
> > >      (6) Unfortunately, the trampoline code (do_plt_call()) is using
> > register r11 to setup the jump.
> > >            It looks like the prologue and epilogue are using also the
> > register r11, in order to point to the previous stack frame.
> > >            This is a conflict !!! The trampoline code is damaging the
> > content of r11.
> > 
> > Hi Steffen,
> > 
> > Great bug report!
> > 
> > I can't quite work out what the standards say, the versions I'm looking
> > at are probably old anyway.
> 
> The ABI supplement from https://www.power.org/resources/downloads/ is explicit about r11 being a requirement for the statically lined save/restore functions in section 3.3.4 on page 59.
> 
> This means that any trampoline code must not ever use r11 or it can't be used to get to such save/restore functions safely from far away.

I believe that the basic premise is that you should provide a directly reachable copy 
of the save/rstore functions, even if this means that you need several copies of the functions.

> 
> Unfortunately the same doc and predecessors show r11 in all basic examples for PLT/trampoline code AFAICS, which is likely why all trampoline code uses r11 in any known case.
> 
> I would guess that it was never envisioned that compiler generated code would be in a different section than save/restore functions, i.e., the Linux module "__init" assumptions for Power break the ABI. Or does the ABI break the __init concept?!
> 
> Using r12 in the trampoline seems to be the obvious solution for module loading.
> 
> But what about other code loading done? If, e.g., a user runs any app from bash it gets loaded and relocated and trampolines might get set up somehow.

I don't think so. The linker/whatever should generate a copy of the save/restore functions for every
executable code area (shared library), and probably more than one copy if the text becomes too large.

For 64 bit code, these functions are actually inserted by the linker.

[Ok, I just recompiled my 64 bit kernel with -Os and I see that vmlinux gets one copy
of the save/restore functions and every module also gets its copy.]

This makes sense, really these functions are there for a compromise between locality 
and performance, there should be one per code section, otherwise the cache line 
used by the trampoline negates a large part of their advantage.

> 
> Wouldn't we have to find fix ANY trampoline code generator remotely related to a basic Power Architecture Linux?
> Or is it a basic assumption for anything but modules that compiler generated code may never ever be outside the .text section? I am not sure that would be a safe assumption.
> 
> Isn't this problem going beyond just module loading for Power Architecture Linux?

I don't think so. It really seems to be a 32 bit kernel problem.

	Regards,
	Gabriel

  reply	other threads:[~2012-05-31 11:05 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-30 14:33 kernel panic during kernel module load (powerpc specific part) Steffen Rumler
2012-05-30 23:24 ` Michael Ellerman
2012-05-31  7:04   ` Wrobel Heinz-R39252
2012-05-31 11:04     ` Gabriel Paubert [this message]
2012-06-01  9:18       ` Benjamin Herrenschmidt
2012-06-01 11:33         ` Wrobel Heinz-R39252
2012-06-04  7:43           ` Steffen Rumler
2012-06-04 10:53             ` Paul Mackerras
2012-06-04 11:03           ` Gabriel Paubert
2012-06-04 22:00             ` Benjamin Herrenschmidt
2012-06-05 10:44               ` Gabriel Paubert
2012-06-05 22:47                 ` Benjamin Herrenschmidt
2012-06-05 11:32               ` Gabriel Paubert
2012-06-05 22:14                 ` Benjamin Herrenschmidt
2012-06-06  7:36             ` Steffen Rumler
2012-06-06 11:32               ` Benjamin Herrenschmidt
2012-06-06 14:37                 ` [PATCH] " Steffen Rumler
2012-06-21 15:27                   ` roger blofeld

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=20120531110439.GA17373@visitor2.iram.es \
    --to=paubert@iram.es \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=michael@ellerman.id.au \
    --cc=r39252@freescale.com \
    --cc=steffen.rumler.ext@nsn.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).