lkml.org 
[lkml]   [2014]   [Nov]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH, RFC] x86: also CFI-annotate certain inline asm()s

* Jan Beulich <JBeulich@suse.com> wrote:

> @@ -11,6 +14,7 @@
> static inline unsigned long native_save_fl(void)
> {
> unsigned long flags;
> + CFI_DECL;
>
> /*
> * "=rm" is safe here, because "pop" adjusts the stack before
> @@ -18,9 +22,10 @@ static inline unsigned long native_save_
> * documented behavior of the "pop" instruction.
> */
> asm volatile("# __raw_save_flags\n\t"
> - "pushf ; pop %0"
> + "pushf" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
> + "pop %0" CFI_ADJUST_CFA_OFFSET(-1)
> : "=rm" (flags)
> - : /* no input */
> + : CFI_INPUTS()
> : "memory");
>
> return flags;
> @@ -28,10 +33,13 @@ static inline unsigned long native_save_
>
> static inline void native_restore_fl(unsigned long flags)
> {
> - asm volatile("push %0 ; popf"
> + CFI_DECL;
> +
> + asm volatile("push %[flags]" CFI_ADJUST_CFA_OFFSET(1) "\n\t"
> + "popf" CFI_ADJUST_CFA_OFFSET(-1)
> : /* no output */
> - :"g" (flags)
> - :"memory", "cc");
> + : CFI_INPUTS([flags] "g" (flags))
> + : "memory", "cc");
> }

In principle I'm not against generating better debug info for our
assembly code, but I think it should be more readable - for
example I would not hide 'cfi_var' in the CFI_DECL above but I'd
make it something like:

CFI_DECL(cfi_var);

...
CFI_ADJUST_CFA_OFFSET(cfi_var, ...);

etc. - even if this is slightly more verbose than what you wrote.

There's few things worse than hidden state connections between
various primitives - even if this is build only.

I'd also suggest splitting up the patch into 'add new primitives'
and 'use them to improve stuff' parts.

(Assuming nobody objects strongly.)

Thanks,

Ingo


\
 
 \ /
  Last update: 2014-11-10 11:41    [W:0.123 / U:17.092 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site