All of lore.kernel.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] powerpc/irq: don't use current_stack_pointer() in check_stack_overflow()
Date: Fri, 24 Jan 2020 02:58:30 -0600	[thread overview]
Message-ID: <20200124085830.GT3191@gate.crashing.org> (raw)
In-Reply-To: <74cb4227-1a24-6fe1-2df4-3d4b069453c4@c-s.fr>

On Fri, Jan 24, 2020 at 07:03:36AM +0000, Christophe Leroy wrote:
> >Le 24/01/2020 à 06:46, Michael Ellerman a écrit :
> >>
> >>If I do this it seems to work, but feels a little dicey:
> >>
> >>    asm ("" : "=r" (r1));
> >>    sp = r1 & (THREAD_SIZE - 1);
> >
> >
> >Or we could do add in asm/reg.h what we have in boot/reg.h:
> >
> >register void *__stack_pointer asm("r1");
> >#define get_sp()    (__stack_pointer)
> >
> >And use get_sp()
> >
> 
> It works, and I guess doing it this way is acceptable as it's exactly 
> what's done for current in asm/current.h with register r2.

That is a *global* register variable.  That works.  We still need to
document a bit better what it does exactly, but this is the expected
use case, so that will work.

> Now I (still) get:
> 
> 	sp = get_sp() & (THREAD_SIZE - 1);
>  b9c:	54 24 04 fe 	clrlwi  r4,r1,19
> 	if (unlikely(sp < 2048)) {
>  ba4:	2f 84 07 ff 	cmpwi   cr7,r4,2047
> 
> Allthough GCC 8.1 what doing exactly the same with the form CLANG don't 
> like:
> 
> 	register unsigned long r1 asm("r1");
> 	long sp = r1 & (THREAD_SIZE - 1);
>  b84:	54 24 04 fe 	clrlwi  r4,r1,19
> 	if (unlikely(sp < 2048)) {
>  b8c:	2f 84 07 ff 	cmpwi   cr7,r4,2047

Sure, if it did what you expected, things will usually work out fine ;-)

(Pity that the compiler didn't come up with
    rlwinm. r4,r1,0,19,20
    bne bad
Or are the low bits of r4 used later again?)


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Christophe Leroy <christophe.leroy@c-s.fr>
Cc: Michael Ellerman <mpe@ellerman.id.au>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	linuxppc-dev@lists.ozlabs.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 1/2] powerpc/irq: don't use current_stack_pointer() in check_stack_overflow()
Date: Fri, 24 Jan 2020 02:58:30 -0600	[thread overview]
Message-ID: <20200124085830.GT3191@gate.crashing.org> (raw)
In-Reply-To: <74cb4227-1a24-6fe1-2df4-3d4b069453c4@c-s.fr>

On Fri, Jan 24, 2020 at 07:03:36AM +0000, Christophe Leroy wrote:
> >Le 24/01/2020 à 06:46, Michael Ellerman a écrit :
> >>
> >>If I do this it seems to work, but feels a little dicey:
> >>
> >>    asm ("" : "=r" (r1));
> >>    sp = r1 & (THREAD_SIZE - 1);
> >
> >
> >Or we could do add in asm/reg.h what we have in boot/reg.h:
> >
> >register void *__stack_pointer asm("r1");
> >#define get_sp()    (__stack_pointer)
> >
> >And use get_sp()
> >
> 
> It works, and I guess doing it this way is acceptable as it's exactly 
> what's done for current in asm/current.h with register r2.

That is a *global* register variable.  That works.  We still need to
document a bit better what it does exactly, but this is the expected
use case, so that will work.

> Now I (still) get:
> 
> 	sp = get_sp() & (THREAD_SIZE - 1);
>  b9c:	54 24 04 fe 	clrlwi  r4,r1,19
> 	if (unlikely(sp < 2048)) {
>  ba4:	2f 84 07 ff 	cmpwi   cr7,r4,2047
> 
> Allthough GCC 8.1 what doing exactly the same with the form CLANG don't 
> like:
> 
> 	register unsigned long r1 asm("r1");
> 	long sp = r1 & (THREAD_SIZE - 1);
>  b84:	54 24 04 fe 	clrlwi  r4,r1,19
> 	if (unlikely(sp < 2048)) {
>  b8c:	2f 84 07 ff 	cmpwi   cr7,r4,2047

Sure, if it did what you expected, things will usually work out fine ;-)

(Pity that the compiler didn't come up with
    rlwinm. r4,r1,0,19,20
    bne bad
Or are the low bits of r4 used later again?)


Segher

  reply	other threads:[~2020-01-24  9:00 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-09  6:07 [PATCH 1/2] powerpc/irq: don't use current_stack_pointer() in check_stack_overflow() Christophe Leroy
2019-12-09  6:07 ` Christophe Leroy
2019-12-09  6:07 ` [PATCH 2/2] powerpc/irq: use IS_ENABLED() " Christophe Leroy
2019-12-09  6:07   ` Christophe Leroy
2020-01-24  5:46 ` [PATCH 1/2] powerpc/irq: don't use current_stack_pointer() " Michael Ellerman
2020-01-24  5:46   ` Michael Ellerman
2020-01-24  6:19   ` Christophe Leroy
2020-01-24  6:19     ` Christophe Leroy
2020-01-24  7:03     ` Christophe Leroy
2020-01-24  8:58       ` Segher Boessenkool [this message]
2020-01-24  8:58         ` Segher Boessenkool
2020-01-24  8:44   ` Segher Boessenkool
2020-01-24  8:44     ` Segher Boessenkool

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=20200124085830.GT3191@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=christophe.leroy@c-s.fr \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=paulus@samba.org \
    /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.