All of lore.kernel.org
 help / color / mirror / Atom feed
From: Steffen Rumler <steffen.rumler.ext@nsn.com>
To: ext Gabriel Paubert <paubert@iram.es>
Cc: Wrobel Heinz-R39252 <r39252@freescale.com>,
	Michael Ellerman <michael@ellerman.id.au>,
	"linuxppc-dev@lists.ozlabs.org" <linuxppc-dev@lists.ozlabs.org>
Subject: Re: kernel panic during kernel module load (powerpc specific part)
Date: Wed, 06 Jun 2012 09:36:55 +0200	[thread overview]
Message-ID: <4FCF0897.2060405@nsn.com> (raw)
In-Reply-To: <20120604110305.GA16707@visitor2.iram.es>


> On Fri, Jun 01, 2012 at 11:33:37AM +0000, Wrobel Heinz-R39252 wrote:
>>>> 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.
>>>
>>> I just fixed a very similar problem with grub2 in fact. It was using r0
>>> and trashing the saved LR that way.
>>>
>>> The real fix is indeed to statically link those gcc "helpers", we
>>> shouldn't generate things like cross-module calls inside function prologs
>>> and epilogues, when stackframes aren't even guaranteed to be reliable.
>>>
>>> However, in the grub2 case, it was easier to just use r12 :-)
>> For not just the module loading case, I believe r12 is the only real solution now. I checked one debugger capable of doing ELF download. It also uses r12 for trampoline code. I am guessing for the reason that prompted this discussion.
>>
> I disagree. Look carefully at Be's answer: cross-module calls
> are intrinsically dangerous when stack frames are in a transient
> state.
>
>> Without r12 we'd have to change standard libraries to automagically link in gcc helpers for any conceivable non-.text section, which I am not sure is feasible. How would you write section independent helper functions which link to any section needing them?!
> I don't thnk that it is tha bad: the helpers should be linked to the default .text section
> when needed, typically the init code and so on are mapped within the reach of that
> section (otherwise you'll end up with the linker complaining that it finds overflowing
> branch offsets between .text and .init.text).
>
>> Asking users to create their own section specific copy of helper functions is definitely not portable if the module or other code is not architecture dependent.
> Well, it automagically works on 64 bit. There is is performed by magic built into the linker.
>
>> It is a normal gcc feature that you can assign specific code to non-.text sections and it is not documented that it may crash depending on the OS arch the ELF is built for, so asking for a Power Architecture specific change on tool libs to make Power Architecture Linux happy seems a bit much to ask.
>>
> Once again I disagree.
>
>> Using r12 in any Linux related trampoline code seems a reachable goal, and it would eliminate the conflict to the ABI.
>>
> There is no conflict to the ABI. These functions are supposed to be directly reachable from whatever code
> section may need them.
>
> Now I have a question: how did you get the need for this?
>
> None of my kernels uses them:
> - if I compile with -O2, the compiler simply expands epilogue and prologue to series of lwz and stw
> - if I compile with -Os, the compiler generates lmw/stmw which give the smallest possible cache footprint
>
> Neither did I find a single reference to these functions in several systems that I grepped for.
>
> 	Regards,
> 	Gabriel
>
Hi,

how should we continue here ?
There is the kernel panic, I've described.

Technically, there is an conflict between the code generated by the compiler and the loader in module_32.c, at least by using -Os.
Because the prologue/epilogue is part of the .text and init_module() is part of .init.text (in the case __init is applied, as usual),
a directly reachable call is not always possible.

Thanks
Steffen

  parent reply	other threads:[~2012-06-06  7:37 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
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 [this message]
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=4FCF0897.2060405@nsn.com \
    --to=steffen.rumler.ext@nsn.com \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=michael@ellerman.id.au \
    --cc=paubert@iram.es \
    --cc=r39252@freescale.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 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.