All of lore.kernel.org
 help / color / mirror / Atom feed
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Paul Mackerras <paulus@samba.org>,
	Michael Ellerman <mpe@ellerman.id.au>,
	nathanl@linux.ibm.com, anton@ozlabs.org,
	linux-arch@vger.kernel.org, arnd@arndb.de,
	linux-kernel@vger.kernel.org, luto@kernel.org,
	tglx@linutronix.de, vincenzo.frascino@arm.com,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v10 2/5] powerpc/vdso: Prepare for switching VDSO to generic C implementation.
Date: Wed, 5 Aug 2020 15:55:56 -0500	[thread overview]
Message-ID: <20200805205556.GR6753@gate.crashing.org> (raw)
In-Reply-To: <7d409421-6396-8eba-8250-b6c9ff8232d9@csgroup.eu>

Hi!

On Wed, Aug 05, 2020 at 06:51:44PM +0200, Christophe Leroy wrote:
> Le 05/08/2020 à 16:03, Segher Boessenkool a écrit :
> >On Wed, Aug 05, 2020 at 07:09:23AM +0000, Christophe Leroy wrote:
> >>+/*
> >>+ * The macros sets two stack frames, one for the caller and one for the 
> >>callee
> >>+ * because there are no requirement for the caller to set a stack frame 
> >>when
> >>+ * calling VDSO so it may have omitted to set one, especially on PPC64
> >>+ */
> >
> >If the caller follows the ABI, there always is a stack frame.  So what
> >is going on?
> 
> Looks like it is not the case. See discussion at 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/2a67c333893454868bbfda773ba4b01c20272a5d.1588079622.git.christophe.leroy@c-s.fr/
> 
> Seems like GCC uses the redzone and doesn't set a stack frame. I guess 
> it doesn't know that the inline assembly contains a function call so it 
> doesn't set the frame.

Yes, that is the problem.  See
https://gcc.gnu.org/onlinedocs/gcc-10.2.0/gcc/Extended-Asm.html#AssemblerTemplate
where this is (briefly) discussed:
  "Accessing data from C programs without using input/output operands
  (such as by using global symbols directly from the assembler
  template) may not work as expected. Similarly, calling functions
  directly from an assembler template requires a detailed understanding
  of the target assembler and ABI."

I don't know of a good way to tell GCC some function needs a frame (that
is, one that doesn't result in extra code other than to set up the
frame).  I'll think about it.


Segher

WARNING: multiple messages have this Message-ID (diff)
From: Segher Boessenkool <segher@kernel.crashing.org>
To: Christophe Leroy <christophe.leroy@csgroup.eu>
Cc: nathanl@linux.ibm.com, linux-arch@vger.kernel.org,
	vincenzo.frascino@arm.com, arnd@arndb.de,
	linux-kernel@vger.kernel.org, Paul Mackerras <paulus@samba.org>,
	luto@kernel.org, tglx@linutronix.de,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH v10 2/5] powerpc/vdso: Prepare for switching VDSO to generic C implementation.
Date: Wed, 5 Aug 2020 15:55:56 -0500	[thread overview]
Message-ID: <20200805205556.GR6753@gate.crashing.org> (raw)
In-Reply-To: <7d409421-6396-8eba-8250-b6c9ff8232d9@csgroup.eu>

Hi!

On Wed, Aug 05, 2020 at 06:51:44PM +0200, Christophe Leroy wrote:
> Le 05/08/2020 à 16:03, Segher Boessenkool a écrit :
> >On Wed, Aug 05, 2020 at 07:09:23AM +0000, Christophe Leroy wrote:
> >>+/*
> >>+ * The macros sets two stack frames, one for the caller and one for the 
> >>callee
> >>+ * because there are no requirement for the caller to set a stack frame 
> >>when
> >>+ * calling VDSO so it may have omitted to set one, especially on PPC64
> >>+ */
> >
> >If the caller follows the ABI, there always is a stack frame.  So what
> >is going on?
> 
> Looks like it is not the case. See discussion at 
> https://patchwork.ozlabs.org/project/linuxppc-dev/patch/2a67c333893454868bbfda773ba4b01c20272a5d.1588079622.git.christophe.leroy@c-s.fr/
> 
> Seems like GCC uses the redzone and doesn't set a stack frame. I guess 
> it doesn't know that the inline assembly contains a function call so it 
> doesn't set the frame.

Yes, that is the problem.  See
https://gcc.gnu.org/onlinedocs/gcc-10.2.0/gcc/Extended-Asm.html#AssemblerTemplate
where this is (briefly) discussed:
  "Accessing data from C programs without using input/output operands
  (such as by using global symbols directly from the assembler
  template) may not work as expected. Similarly, calling functions
  directly from an assembler template requires a detailed understanding
  of the target assembler and ABI."

I don't know of a good way to tell GCC some function needs a frame (that
is, one that doesn't result in extra code other than to set up the
frame).  I'll think about it.


Segher

  reply	other threads:[~2020-08-05 20:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-05  7:09 [PATCH v10 0/5] powerpc: switch VDSO to C implementation Christophe Leroy
2020-08-05  7:09 ` Christophe Leroy
2020-08-05  7:09 ` [PATCH v10 1/5] powerpc/processor: Move cpu_relax() into asm/vdso/processor.h Christophe Leroy
2020-08-05  7:09   ` Christophe Leroy
2020-08-05  7:09 ` [PATCH v10 2/5] powerpc/vdso: Prepare for switching VDSO to generic C implementation Christophe Leroy
2020-08-05  7:09   ` Christophe Leroy
2020-08-05 14:03   ` Segher Boessenkool
2020-08-05 14:03     ` Segher Boessenkool
2020-08-05 16:40     ` Christophe Leroy
2020-08-05 16:40       ` Christophe Leroy
2020-08-05 18:40       ` Segher Boessenkool
2020-08-05 18:40         ` Segher Boessenkool
2020-08-06  5:46         ` Christophe Leroy
2020-08-06  5:46           ` Christophe Leroy
2020-08-05 16:51     ` Christophe Leroy
2020-08-05 16:51       ` Christophe Leroy
2020-08-05 20:55       ` Segher Boessenkool [this message]
2020-08-05 20:55         ` Segher Boessenkool
2020-08-05  7:09 ` [PATCH v10 3/5] powerpc/vdso: Save and restore TOC pointer on PPC64 Christophe Leroy
2020-08-05  7:09   ` Christophe Leroy
2020-08-05  7:09 ` [PATCH v10 4/5] powerpc/vdso: Switch VDSO to generic C implementation Christophe Leroy
2020-08-05  7:09   ` Christophe Leroy
2020-08-05  7:09 ` [PATCH v10 5/5] powerpc/vdso: Provide __kernel_clock_gettime64() on vdso32 Christophe Leroy
2020-08-05  7:09   ` Christophe Leroy

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=20200805205556.GR6753@gate.crashing.org \
    --to=segher@kernel.crashing.org \
    --cc=anton@ozlabs.org \
    --cc=arnd@arndb.de \
    --cc=benh@kernel.crashing.org \
    --cc=christophe.leroy@csgroup.eu \
    --cc=linux-arch@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=luto@kernel.org \
    --cc=mpe@ellerman.id.au \
    --cc=nathanl@linux.ibm.com \
    --cc=paulus@samba.org \
    --cc=tglx@linutronix.de \
    --cc=vincenzo.frascino@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 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.