linux-kernel - Re: [PATCH] x86: clean up after: move entry_64.S register saving out of the macros

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20081120153954.GA22511@elte.hu>
Date:	Thu, 20 Nov 2008 16:39:54 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Alexander van Heukelum <heukelum@...lshack.com>
Cc:	LKML <linux-kernel@...r.kernel.org>,
	Andi Kleen <andi@...stfloor.org>,
	"H. Peter Anvin" <hpa@...or.com>,
	Jan Beulich <jbeulich@...ell.com>,
	Glauber Costa <gcosta@...hat.com>,
	Matt Mackall <mpm@...enic.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Nick Piggin <nickpiggin@...oo.com.au>,
	Cyrill Gorcunov <gorcunov@...il.com>
Subject: Re: [PATCH] x86: clean up after: move entry_64.S register saving
	out of the macros


* Alexander van Heukelum <heukelum@...lshack.com> wrote:

> On Thu, Nov 20, 2008 at 04:04:12PM +0100, Ingo Molnar wrote:
> > 
> > * Alexander van Heukelum <heukelum@...lshack.com> wrote:
> > 
> > > This add-on patch to x86: move entry_64.S register saving out of the 
> > > macros visually cleans up the appearance of the code by introducing 
> > > some basic helper macro's. It also adds some cfi annotations which 
> > > were missing.
> > > 
> > > Signed-off-by: Alexander van Heukelum <heukelum@...tmail.fm>
> > > ---
> > >  arch/x86/kernel/entry_64.S |  220 ++++++++++++++++++++++----------------------
> > >  1 files changed, 112 insertions(+), 108 deletions(-)
> > > 
> > > Hello Ingo,
> > > 
> > > This patch improves the CFI-situation in entry_64.S, but restricted 
> > > mostly to the areas touched by "x86: move entry_64.S register saving 
> > > out of the macros". I'm sure there will be some small errors 
> > > somewhere, but it compiles and runs fine.
> > 
> > very nice cleanup! This is exactly what should be done. Applied to 
> > tip/x86/irq.
> > 
> > Note, i did a small rename:
> > 
> >     cfi_pushq  =>  pushq_cfi
> >     cfi_popq   =>  popq_cfi
> >     cfi_store  =>  movq_cfi
> > 
> > as the goal is to have the actual source code read mostly as regular 
> > assembly code. The fact that the macro is equivalent to a 
> > default-annotated pushq/popq/movq instruction is much more important 
> > than the fact that it also does CFI annotations.
> > 
> > Also, while cfi_store is correct as well, the usual x86 assembly term 
> > (and instruction used here) is movq.
> 
> Now I have a little problem with my next patch... I wanted to 
> introduce cfi_load. Guess what assembly instruction that maps to ;).

heh ;-)

the restore direction could be named movq_cfi_restore, and have the same 
order of arguments as the regular movq that it replaces. I.e.:

        movq 8(%rsp),%r11
        CFI_RESTORE r11

would map to:

	movq_cfi_restore 8, r11

or so.

cfi_store has really a bad name: it's confusing whether it's the CFI 
info we are storing/registering (which we are), or a 'store' instruction 
(which this is too).

If then we should name it movq_cfi_store or movq_cfi_register - but 
that's too long.

movq_cfi for the frame construction direction and movq_cfi_restore for 
the frame deconstruction phase sounds like a good naming compromise, hm?

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/