On Wed, Dec 30, 2015 at 09:59:29AM -0800, Andy Lutomirski wrote:
> This adds two bits of fixup class information to a fixup entry,
> generalizing the uaccess_err hack currently in place.
> 
> Forward-ported-from-3.9-by: Tony Luck <tony.l...@intel.com>
> Signed-off-by: Andy Lutomirski <l...@amacapital.net>
> ---
>  arch/x86/include/asm/asm.h | 70 
> ++++++++++++++++++++++++++++++----------------
>  arch/x86/mm/extable.c      | 21 ++++++++------
>  2 files changed, 59 insertions(+), 32 deletions(-)
> 
> diff --git a/arch/x86/include/asm/asm.h b/arch/x86/include/asm/asm.h
> index 189679aba703..b64121ffb2da 100644
> --- a/arch/x86/include/asm/asm.h
> +++ b/arch/x86/include/asm/asm.h
> @@ -43,19 +43,47 @@
>  #define _ASM_DI              __ASM_REG(di)
>  
>  /* Exception table entry */
> -#ifdef __ASSEMBLY__
> -# define _ASM_EXTABLE(from,to)                                       \
> -     .pushsection "__ex_table","a" ;                         \
> -     .balign 8 ;                                             \
> -     .long (from) - . ;                                      \
> -     .long (to) - . ;                                        \
> -     .popsection
>  
> -# define _ASM_EXTABLE_EX(from,to)                            \
> -     .pushsection "__ex_table","a" ;                         \
> -     .balign 8 ;                                             \
> -     .long (from) - . ;                                      \
> -     .long (to) - . + 0x7ffffff0 ;                           \
> +/*
> + * An exception table entry is 64 bits.  The first 32 bits are the offset

Two 32-bit ints, to be exact.

Also, there's text in arch/x86/include/asm/uaccess.h where the exception
table entry is defined so you probably should sync with it so that the
nomenclature is the same.

> + * from that entry to the potentially faulting instruction.  sortextable

                                                                sortextable.c ?

> + * relies on that exact encoding.  The second 32 bits encode the fault
> + * handler address.
> + *
> + * We want to stick two extra bits of handler class into the fault handler
> + * address.  All of these are generated by relocations, so we can only
> + * rely on addition.  We therefore emit:
> + *
> + * (target - here) + (class) + 0x20000000

I still don't understand that bit 29 thing.

Because the offset is negative?

The exception table currently looks like this here:

insn offset: 0xff91a7c4, fixup offset: 0xffffd57a
insn offset: 0xff91bac3, fixup offset: 0xffffd57e
insn offset: 0xff91bac0, fixup offset: 0xffffd57d
insn offset: 0xff91baba, fixup offset: 0xffffd57c
insn offset: 0xff91bfca, fixup offset: 0xffffd57c
insn offset: 0xff91bfff, fixup offset: 0xffffd57e
insn offset: 0xff91c049, fixup offset: 0xffffd580
insn offset: 0xff91c141, fixup offset: 0xffffd57f
insn offset: 0xff91c24e, fixup offset: 0xffffd581
insn offset: 0xff91c262, fixup offset: 0xffffd580
insn offset: 0xff91c261, fixup offset: 0xffffd57f
...

It probably will dawn on me when I look at the rest of the patch...

> + * This has the property that the two high bits are the class and the
> + * rest is easy to decode.
> + */
> +
> +/* There are two bits of extable entry class, added to a signed offset. */
> +#define _EXTABLE_CLASS_DEFAULT       0               /* standard uaccess 
> fixup */
> +#define _EXTABLE_CLASS_EX    0x80000000      /* uaccess + set uaccess_err */

                                BIT(31) is more readable.

> +
> +/*
> + * The biases are the class constants + 0x20000000, as signed integers.
> + * This can't use ordinary arithmetic -- the assembler isn't that smart.
> + */
> +#define _EXTABLE_BIAS_DEFAULT        0x20000000
> +#define _EXTABLE_BIAS_EX     0x20000000 - 0x80000000

Ditto.

> +
> +#define _ASM_EXTABLE(from,to)                                                
> \
> +     _ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_DEFAULT)
> +
> +#define _ASM_EXTABLE_EX(from,to)                                     \
> +     _ASM_EXTABLE_CLASS(from, to, _EXTABLE_BIAS_EX)
> +
> +#ifdef __ASSEMBLY__
> +# define _EXPAND_EXTABLE_BIAS(x) x
> +# define _ASM_EXTABLE_CLASS(from,to,bias)                            \
> +     .pushsection "__ex_table","a" ;                                 \
> +     .balign 8 ;                                                     \
> +     .long (from) - . ;                                              \
> +     .long (to) - . + _EXPAND_EXTABLE_BIAS(bias) ;                   \

Why not simply:

        .long (to) - . + (bias) ;

and

        " .long (" #to ") - . + "(" #bias ") "\n"

below and get rid of that _EXPAND_EXTABLE_BIAS()?

>       .popsection
>  
>  # define _ASM_NOKPROBE(entry)                                        \
> @@ -89,18 +117,12 @@
>       .endm
>  
>  #else
> -# define _ASM_EXTABLE(from,to)                                       \
> -     " .pushsection \"__ex_table\",\"a\"\n"                  \
> -     " .balign 8\n"                                          \
> -     " .long (" #from ") - .\n"                              \
> -     " .long (" #to ") - .\n"                                \
> -     " .popsection\n"
> -
> -# define _ASM_EXTABLE_EX(from,to)                            \
> -     " .pushsection \"__ex_table\",\"a\"\n"                  \
> -     " .balign 8\n"                                          \
> -     " .long (" #from ") - .\n"                              \
> -     " .long (" #to ") - . + 0x7ffffff0\n"                   \
> +# define _EXPAND_EXTABLE_BIAS(x) #x
> +# define _ASM_EXTABLE_CLASS(from,to,bias)                            \
> +     " .pushsection \"__ex_table\",\"a\"\n"                          \
> +     " .balign 8\n"                                                  \
> +     " .long (" #from ") - .\n"                                      \
> +     " .long (" #to ") - . + " _EXPAND_EXTABLE_BIAS(bias) "\n"       \
>       " .popsection\n"
>  /* For C file, we already have NOKPROBE_SYMBOL macro */
>  #endif
> diff --git a/arch/x86/mm/extable.c b/arch/x86/mm/extable.c
> index 903ec1e9c326..95e2ede71206 100644
> --- a/arch/x86/mm/extable.c
> +++ b/arch/x86/mm/extable.c
> @@ -8,16 +8,24 @@ ex_insn_addr(const struct exception_table_entry *x)
>  {
>       return (unsigned long)&x->insn + x->insn;
>  }
> +static inline unsigned int
> +ex_class(const struct exception_table_entry *x)
> +{
> +     return (unsigned int)x->fixup & 0xC0000000;
> +}
> +
>  static inline unsigned long
>  ex_fixup_addr(const struct exception_table_entry *x)
>  {
> -     return (unsigned long)&x->fixup + x->fixup;
> +     long offset = (long)((u32)x->fixup & 0x3fffffff) - (long)0x20000000;

So basically:

        x->fixup & 0x1fffffff

Why the explicit subtraction of bit 29?

IOW, I was expecting something simpler for the whole scheme like:

ex_class:

        return x->fixup & 0xC0000000;

ex_fixup_addr:

        return x->fixup | 0xC0000000;

Why can't it be done this way?

-- 
Regards/Gruss,
    Boris.

ECO tip #101: Trim your mails when you reply.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to