linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jed Davis <jld@mozilla.com>
To: Dave Martin <Dave.Martin@arm.com>, Will Deacon <will.deacon@arm.com>
Cc: Robert Richter <rric@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	Ingo Molnar <mingo@redhat.com>, Paul Mackerras <paulus@samba.org>,
	Arnaldo Carvalho de Melo <acme@ghostprotocols.net>,
	Russell King <linux@arm.linux.org.uk>,
	"oprofile-list@lists.sf.net" <oprofile-list@lists.sf.net>,
	"linux-arm-kernel@lists.infradead.org" 
	<linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH] ARM: Fix r7/r11 confusion when CONFIG_THUMB2_KERNEL=y
Date: Mon, 29 Jul 2013 14:21:40 -0700	[thread overview]
Message-ID: <20130729212140.GB12681@mozilla.com> (raw)
In-Reply-To: <20130722185234.GA14519@localhost.localdomain>

On Mon, Jul 22, 2013 at 07:52:39PM +0100, Dave Martin wrote:
> On Sun, Jul 21, 2013 at 10:37:53PM +0100, Will Deacon wrote:
> > Ok, I think I'm with you now. I also think that a better solution would be
> > to try and limit the r7/fp confusion to one place, perhaps behind something
> > like:
> > 
> > void arm_get_current_stackframe(struct pt_regs *regs, struct stackframe *frame);
> > 
> > then that function can act as the bridge between pt_regs (where we leave
> > everything as it is) and stackframe (where we assign either r7 or fp into
> > the fp member). Then we just fix up the call sites to pass the regs they're
> > interested in to our new function.
> > 
> > What do you think?

I can see that being useful if we wanted to opacify struct stackframe,
but... all uses of stackframe that I see involve passing it to
unwind_frame, which expands it back out into an array of registers.
(Except with CONFIG_FRAME_POINTER, but "APCS variants that require a
frame pointer register are obsolete.")

So... why not make pt_regs and stackframe the same, and avoid the
translations entirely?

> Do the ARM unwind tables guarantee not to need knowledge of any
> virtual registers for the purpose of processing the opcodes of a single
> function's unwind table entry, except those virtual regs that we happen
> to initialise?  Or do we just rely on luck?

Yes, for some value of "luck".  The spec documents 0x90|N, for N a core
register number other than 13 or 15, as setting the vSP to the value of
virtual register N.  We can get away with some omissions for kernel code
(e.g., unwind.c doesn't handle saved floating point registers, nor adding
constants larger than 1024 to vSP), but is this one of them?

[...]
> The purist answer seems to be: we might need all the regs in struct
> stackframe.
> 
> The pragmatic answer might be that we definitely need r7 for Thumb code,
> but given the nimbleness of GCC to evolve we might get away with not
> including extra registers for a long time yet.

The other question to ask here might be: what does the "pragmatic
answer" gain us over the "purist answer"?

> A review of existing custom unwind annotations might be a good idea
> anyway, to check whether any of them requires another register right now.

`egrep -r '\.(setf|movs)p' arch/arm` finds nothing, for what it's worth.

--Jed


  reply	other threads:[~2013-07-29 21:21 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-07-13  3:18 [PATCH] ARM: Fix r7/r11 confusion when CONFIG_THUMB2_KERNEL=y Jed Davis
2013-07-15 13:54 ` Will Deacon
2013-07-20  4:46   ` Jed Davis
2013-07-21 21:37     ` Will Deacon
2013-07-22 13:56       ` Robert Richter
2013-07-22 18:52       ` Dave Martin
2013-07-29 21:21         ` Jed Davis [this message]
2013-07-30  9:25           ` Dave Martin
2013-07-30  9:38             ` [PATCH] ARM: Fix r7/r11 confusion when CONFIG_THUMB2_KERNEL=y [OT] Jean-Francois Moine
2013-07-30  9:44               ` Dave Martin
2013-07-30 10:09                 ` Jean-Francois Moine
2013-07-30 11:46                   ` Dave Martin
2013-07-30 17:50                   ` Christopher Covington
2013-07-30  9:49               ` Will Deacon
2013-07-31  9:03                 ` Jean-Francois Moine
2013-07-31 10:38                   ` Will Deacon
2014-01-06  9:54                   ` walimis

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=20130729212140.GB12681@mozilla.com \
    --to=jld@mozilla.com \
    --cc=Dave.Martin@arm.com \
    --cc=a.p.zijlstra@chello.nl \
    --cc=acme@ghostprotocols.net \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@arm.linux.org.uk \
    --cc=mingo@redhat.com \
    --cc=oprofile-list@lists.sf.net \
    --cc=paulus@samba.org \
    --cc=rric@kernel.org \
    --cc=will.deacon@arm.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).