All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Jiri Slaby <jslaby@suse.cz>
Cc: mingo@redhat.com, tglx@linutronix.de, hpa@zytor.com,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	Juergen Gross <jgross@suse.com>,
	xen-devel@lists.xenproject.org,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	Len Brown <len.brown@intel.com>, Pavel Machek <pavel@ucw.cz>,
	linux-pm@vger.kernel.org
Subject: Re: [PATCH v2 02/10] x86: assembly, FUNC_START for fn, DATA_START for data
Date: Mon, 20 Mar 2017 11:07:19 -0500	[thread overview]
Message-ID: <20170320160719.57xlo2stclruyzwo@treble> (raw)
In-Reply-To: <cf9b5094-a7b0-76ae-714a-7e4c8a0f79fb@suse.cz>

On Mon, Mar 20, 2017 at 04:32:09PM +0100, Jiri Slaby wrote:
> On 03/20/2017, 02:32 PM, Josh Poimboeuf wrote:
> > On Mon, Mar 20, 2017 at 01:32:14PM +0100, Jiri Slaby wrote:
> >> This is a start of series to cleanup macros used for starting functions,
> >> data, globals etc. across x86. When we have all this sorted out, this
> >> will help to inject DWARF unwinding info by objtool later.
> >>
> >> The goal is forcing SYM_FUNC_START to emit .cfi_startproc and
> >> SYM_FUNC_END to emit .cfi_endproc. Automatically at best.
> > 
> > Do we still want to emit .cfi_startproc/endproc from the macro?  From
> > our last discussion, that seemed to be up in the air.
> > 
> >   https://lkml.kernel.org/r/20170217211804.j6l2d7t5mfzqzmbt@treble
> 
> "Automatically at best" above means "completely from objtool". I am
> still uncertain whether it will work 100% or we would have to help by
> generating some pieces from the added macros. In particular, the ALIASes
> are evil which cause harm here:
> 
> fun_alias:
> fun:
> <code>
> .size fun, .-fun
> .type fun STT_FUNC
> .size fun_alias, .-fun_alias
> .type fun_alias STT_FUNC
> 
> Both cannot create (overlapping) .cfi_startproc/endproc, only the inner
> shall.
> 
> But it seems so far, that we might be able to deal with all of that from
> objtool... (I have not been thinking about this particular thing deep
> enough yet.) Some sort of "from the last label that is marked as
> STT_FUNC till its .size" might work.

Ok.

> > What did you think about making CFI read-only for .c object files and
> > write-only for .S object files?
> 
> There are those functions like sync_core() or native_save_fl() with
> inline asm. And they seem to need a) read-write support, or b) manual
> annotation. I would like to avoid b) for sure.

Ah, so I guess those inline asm functions cause problems because they
muck with the stack pointer with pushes and pops?

I don't think manual annotation of inline asm would be so bad.  IIUC, it
would only mean replacing the pushes and pops with a macro which does
the CFI-annotated version, like PUSH_CFI and POP_CFI.  And the benefit
would be that objtool doesn't have to try to rewrite a bunch of .c
object files.

Objtool read-write worries me because it gives more responsibility to
objtool.  It could be tricky to insert CFI instructions within the ones
already created by gcc.  Also, while unlikely, a bug in objtool could
theoretically corrupt an object file and brick the kernel.  Also I
wonder how all those extra file writes would affect build performance.

If at all possible, I would rather objtool stay out of the way of the
compiler and let gcc do its job of generating CFI.

-- 
Josh

WARNING: multiple messages have this Message-ID (diff)
From: Josh Poimboeuf <jpoimboe@redhat.com>
To: Jiri Slaby <jslaby@suse.cz>
Cc: Juergen Gross <jgross@suse.com>, Len Brown <len.brown@intel.com>,
	linux-pm@vger.kernel.org, x86@kernel.org,
	"Rafael J. Wysocki" <rjw@rjwysocki.net>,
	linux-kernel@vger.kernel.org, mingo@redhat.com,
	Pavel Machek <pavel@ucw.cz>,
	hpa@zytor.com, xen-devel@lists.xenproject.org,
	Boris Ostrovsky <boris.ostrovsky@oracle.com>,
	tglx@linutronix.de
Subject: Re: [PATCH v2 02/10] x86: assembly, FUNC_START for fn, DATA_START for data
Date: Mon, 20 Mar 2017 11:07:19 -0500	[thread overview]
Message-ID: <20170320160719.57xlo2stclruyzwo@treble> (raw)
In-Reply-To: <cf9b5094-a7b0-76ae-714a-7e4c8a0f79fb@suse.cz>

On Mon, Mar 20, 2017 at 04:32:09PM +0100, Jiri Slaby wrote:
> On 03/20/2017, 02:32 PM, Josh Poimboeuf wrote:
> > On Mon, Mar 20, 2017 at 01:32:14PM +0100, Jiri Slaby wrote:
> >> This is a start of series to cleanup macros used for starting functions,
> >> data, globals etc. across x86. When we have all this sorted out, this
> >> will help to inject DWARF unwinding info by objtool later.
> >>
> >> The goal is forcing SYM_FUNC_START to emit .cfi_startproc and
> >> SYM_FUNC_END to emit .cfi_endproc. Automatically at best.
> > 
> > Do we still want to emit .cfi_startproc/endproc from the macro?  From
> > our last discussion, that seemed to be up in the air.
> > 
> >   https://lkml.kernel.org/r/20170217211804.j6l2d7t5mfzqzmbt@treble
> 
> "Automatically at best" above means "completely from objtool". I am
> still uncertain whether it will work 100% or we would have to help by
> generating some pieces from the added macros. In particular, the ALIASes
> are evil which cause harm here:
> 
> fun_alias:
> fun:
> <code>
> .size fun, .-fun
> .type fun STT_FUNC
> .size fun_alias, .-fun_alias
> .type fun_alias STT_FUNC
> 
> Both cannot create (overlapping) .cfi_startproc/endproc, only the inner
> shall.
> 
> But it seems so far, that we might be able to deal with all of that from
> objtool... (I have not been thinking about this particular thing deep
> enough yet.) Some sort of "from the last label that is marked as
> STT_FUNC till its .size" might work.

Ok.

> > What did you think about making CFI read-only for .c object files and
> > write-only for .S object files?
> 
> There are those functions like sync_core() or native_save_fl() with
> inline asm. And they seem to need a) read-write support, or b) manual
> annotation. I would like to avoid b) for sure.

Ah, so I guess those inline asm functions cause problems because they
muck with the stack pointer with pushes and pops?

I don't think manual annotation of inline asm would be so bad.  IIUC, it
would only mean replacing the pushes and pops with a macro which does
the CFI-annotated version, like PUSH_CFI and POP_CFI.  And the benefit
would be that objtool doesn't have to try to rewrite a bunch of .c
object files.

Objtool read-write worries me because it gives more responsibility to
objtool.  It could be tricky to insert CFI instructions within the ones
already created by gcc.  Also, while unlikely, a bug in objtool could
theoretically corrupt an object file and brick the kernel.  Also I
wonder how all those extra file writes would affect build performance.

If at all possible, I would rather objtool stay out of the way of the
compiler and let gcc do its job of generating CFI.

-- 
Josh

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

  reply	other threads:[~2017-03-20 16:08 UTC|newest]

Thread overview: 108+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-02-17 10:47 [PATCH 01/10] x86: assembly, ENTRY for fn, GLOBAL for data Jiri Slaby
2017-02-17 10:47 ` [PATCH 02/10] x86: assembly, use ENDPROC for functions Jiri Slaby
2017-02-17 10:47   ` Jiri Slaby
2017-02-17 11:08   ` Juergen Gross
2017-02-17 11:08   ` Juergen Gross
2017-02-17 10:47 ` [PATCH 03/10] x86: boot, annotate functions properly Jiri Slaby
2017-02-17 10:47 ` [PATCH 04/10] linkage: introduce ENTRY_LOCAL Jiri Slaby
2017-02-17 10:47 ` [PATCH 05/10] x86: kernel, annotate local functions Jiri Slaby
2017-02-17 10:47 ` [PATCH 06/10] x86: crypto, " Jiri Slaby
2017-02-17 10:47 ` [PATCH 07/10] linkage: introduce ALIASes Jiri Slaby
2017-02-17 10:47 ` [PATCH 08/10] x86: assembly, annotate aliases Jiri Slaby
2017-02-17 10:47   ` Jiri Slaby
2017-02-17 11:52   ` Juergen Gross
2017-02-17 11:52   ` Juergen Gross
2017-02-17 10:47 ` [RFC 09/10] x86: boot, extract efi_pe_entry from startup_64 Jiri Slaby
2017-02-17 10:47 ` [PREVIEW 10/10] linkage: add .cfi_{start/end}proc to ENTRY/ENDPROC Jiri Slaby
2017-02-17 13:16   ` Josh Poimboeuf
2017-02-17 13:36     ` Jiri Slaby
2017-02-17 14:07       ` Josh Poimboeuf
2017-02-17 14:26         ` Jiri Slaby
2017-02-17 21:18           ` Josh Poimboeuf
2017-02-17 11:06 ` [PATCH 01/10] x86: assembly, ENTRY for fn, GLOBAL for data Juergen Gross
2017-02-17 11:06 ` Juergen Gross
2017-03-01  9:38 ` Ingo Molnar
2017-03-01  9:38   ` Ingo Molnar
2017-03-01  9:50   ` Jiri Slaby
2017-03-01  9:50     ` Jiri Slaby
2017-03-01 10:09   ` Thomas Gleixner
2017-03-01 10:09   ` Thomas Gleixner
2017-03-01 10:27     ` Ingo Molnar
2017-03-01 10:27       ` Ingo Molnar
2017-03-03 12:22       ` Jiri Slaby
2017-03-03 12:22         ` Jiri Slaby
2017-03-03 18:20       ` hpa
2017-03-03 18:20         ` hpa
2017-03-06 14:09         ` Jiri Slaby
2017-03-06 14:09           ` Jiri Slaby
2017-03-07  7:57         ` Ingo Molnar
2017-03-07  7:57           ` Ingo Molnar
2017-03-03 18:24       ` hpa
2017-03-03 18:24         ` hpa
2017-03-07  8:27         ` Ingo Molnar
2017-03-07  8:27           ` Ingo Molnar
2017-03-07 17:24           ` [RFC] linkage: new macros for functions and data Jiri Slaby
2017-03-07 17:24             ` Jiri Slaby
2017-03-16  8:02             ` Ingo Molnar
2017-03-16  8:02               ` Ingo Molnar
2017-03-16  8:13               ` Jiri Slaby
2017-03-20 12:32                 ` [PATCH v2 01/10] linkage: new macros for assembler symbols Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 02/10] x86: assembly, FUNC_START for fn, DATA_START for data Jiri Slaby
2017-03-20 12:32                     ` Jiri Slaby
2017-03-20 13:32                     ` Josh Poimboeuf
2017-03-20 13:32                       ` Josh Poimboeuf
2017-03-20 15:32                       ` Jiri Slaby
2017-03-20 15:32                         ` Jiri Slaby
2017-03-20 16:07                         ` Josh Poimboeuf [this message]
2017-03-20 16:07                           ` Josh Poimboeuf
2017-03-21 14:08                     ` Pavel Machek
2017-03-21 14:08                       ` Pavel Machek
2017-03-22  7:25                       ` Ingo Molnar
2017-03-22  7:25                         ` Ingo Molnar
2017-03-22  7:39                         ` Jiri Slaby
2017-03-22  7:39                         ` Jiri Slaby
2017-03-22  7:46                           ` Ingo Molnar
2017-03-22  7:46                           ` Ingo Molnar
2017-03-22 14:11                             ` Josh Poimboeuf
2017-03-22 15:01                               ` Jiri Slaby
2017-03-22 15:33                                 ` Josh Poimboeuf
2017-03-22 15:33                                 ` Josh Poimboeuf
2017-03-22 15:01                               ` Jiri Slaby
2017-03-23  7:38                               ` Ingo Molnar
2017-03-23  7:38                               ` Ingo Molnar
2017-03-23 13:24                                 ` Josh Poimboeuf
2017-03-23 13:24                                 ` Josh Poimboeuf
2017-03-22 14:11                             ` Josh Poimboeuf
2017-03-22  7:25                       ` Ingo Molnar
2017-03-22 12:06                       ` Jiri Slaby
2017-03-22 12:06                       ` Jiri Slaby
2017-03-22 15:52                         ` Pavel Machek
2017-03-22 15:52                         ` Pavel Machek
2017-03-20 12:32                   ` [PATCH v2 03/10] x86: assembly, use SYM_FUNC_END for functions Jiri Slaby
2017-03-21 14:48                     ` Josh Poimboeuf
2017-03-21 14:48                     ` Josh Poimboeuf
2017-03-22  7:29                       ` Ingo Molnar
2017-03-22  7:29                         ` Ingo Molnar
2017-03-22 14:26                     ` Josh Poimboeuf
2017-03-22 15:44                       ` Jiri Slaby
2017-03-22 15:44                         ` Jiri Slaby
2017-04-10 11:23                         ` Jiri Slaby
2017-04-10 19:35                           ` Josh Poimboeuf
2017-04-10 19:35                           ` Josh Poimboeuf
2017-04-12  6:24                             ` Jiri Slaby
2017-04-12  6:52                               ` Ingo Molnar
2017-04-12  6:52                               ` Ingo Molnar
2017-04-12  6:24                             ` Jiri Slaby
2017-04-10 11:23                         ` Jiri Slaby
2017-03-22 14:26                     ` Josh Poimboeuf
2017-03-20 12:32                   ` Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 04/10] x86: boot, annotate functions properly Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 05/10] x86: kernel+lib, annotate local functions Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 06/10] x86: crypto, " Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 07/10] x86: assembly, annotate aliases Jiri Slaby
2017-03-20 12:32                   ` Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 08/10] x86: entry, annotate THUNKs Jiri Slaby
2017-03-20 12:32                   ` [PATCH v2 09/10] x86: entry, annotate interrupt symbols properly Jiri Slaby
2017-03-20 12:32                   ` [RFC v2 10/10] x86: boot, extract efi_pe_entry from startup_64 Jiri Slaby
2017-03-20 12:32                 ` [PATCH v2 01/10] linkage: new macros for assembler symbols Jiri Slaby
2017-03-16  8:13               ` [RFC] linkage: new macros for functions and data Jiri Slaby

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=20170320160719.57xlo2stclruyzwo@treble \
    --to=jpoimboe@redhat.com \
    --cc=boris.ostrovsky@oracle.com \
    --cc=hpa@zytor.com \
    --cc=jgross@suse.com \
    --cc=jslaby@suse.cz \
    --cc=len.brown@intel.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=pavel@ucw.cz \
    --cc=rjw@rjwysocki.net \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.org \
    --cc=xen-devel@lists.xenproject.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.