Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 15.08.16 at 01:07, <konrad.wilk@xxxxxxxxxx> wrote:
> --- a/xen/common/Kconfig
> +++ b/xen/common/Kconfig
> @@ -222,7 +222,7 @@ endmenu
>  config LIVEPATCH
>       bool "Live patching support (TECH PREVIEW)"
>       default n
> -     depends on X86 && HAS_BUILD_ID = "y"
> +     depends on (X86 || ARM_64) && HAS_BUILD_ID = "y"

Would this better become a black list?

> @@ -711,9 +711,15 @@ static int prepare_payload(struct payload *payload,
>                  return -EINVAL;
>              }
>          }
> +#ifndef CONFIG_ARM
>          apply_alternatives_nocheck(start, end);
> +#else
> +        apply_alternatives(start, sec->sec->sh_size);
> +#endif

Conditionals like this are ugly - can't this be properly abstracted?

> --- a/xen/include/xen/elfstructs.h
> +++ b/xen/include/xen/elfstructs.h
> @@ -103,6 +103,15 @@ typedef uint64_t Elf64_Xword;
>                        (ehdr).e_ident[EI_MAG2] == ELFMAG2 && \
>                        (ehdr).e_ident[EI_MAG3] == ELFMAG3)
>  
> +/* e_flags */
> +#define EF_ARM_EABI_MASK     0xff000000
> +#define EF_ARM_EABI_UNKNOWN  0x00000000
> +#define EF_ARM_EABI_VER1     0x01000000
> +#define EF_ARM_EABI_VER2     0x02000000
> +#define EF_ARM_EABI_VER3     0x03000000
> +#define EF_ARM_EABI_VER4     0x04000000
> +#define EF_ARM_EABI_VER5     0x05000000

Aren't these ARM32 definitions, which should be unneeded for
ARM64 support?

> @@ -171,6 +180,7 @@ typedef struct {
>  #define EM_PPC               20              /* PowerPC */
>  #define EM_PPC64     21              /* PowerPC 64-bit */
>  #define EM_ARM               40              /* Advanced RISC Machines ARM */
> +#define EM_AARCH64   183             /* ARM 64-bit */
>  #define EM_ALPHA     41              /* DEC ALPHA */
>  #define EM_SPARCV9   43              /* SPARC version 9 */
>  #define EM_ALPHA_EXP 0x9026          /* DEC ALPHA */

I think this tries to be sorted by number.

> +/*
> + * S - address of symbol.
> + * A - addend for relocation (r_addend)
> + * P - address of the dest being relocated (derieved from r_offset)
> + * NC -  No check for overflow.
> + *
> + * The defines also use _PREL for PC-relative address, and _NC is No Check.
> + */
> +#define R_AARCH64_ABS64                      257 /* Direct 64 bit. S+A, NC*/
> +#define R_AARCH64_ABS32                      258 /* Direct 32 bit. S+A */
> +#define R_AARCH64_PREL64             260 /* S+A-P, NC */
> +#define R_AARCH64_PREL32             261 /* S+A-P */
> +
> +#define R_AARCH64_ADR_PREL_LO21              274 /* ADR imm, [20:0]. S+A-P */
> +#define R_AARCH64_ADR_PREL_PG_HI21   275 /* ADRP imm, [32:12]. Page(S+A) - 
> Page(P).*/
> +#define R_AARCH64_ADD_ABS_LO12_NC    277 /* ADD imm. [11:0]. S+A, NC */
> +
> +#define R_AARCH64_CONDBR19           280 /* Bits 20:2, S+A-P */
> +#define R_AARCH64_JUMP26             282 /* Bits 27:2, S+A-P */
> +#define R_AARCH64_CALL26             283 /* Bits 27:2, S+A-P */

No R_AARCH64_TSTBR14?

> +#define R_AARCH64_LDST16_ABS_LO12_NC 284 /* LD/ST to bits 11:1, S+A, NC */
> +#define R_AARCH64_LDST32_ABS_LO12_NC 285 /* LD/ST to bits 11:2, S+A, NC */
> +#define R_AARCH64_LDST64_ABS_LO12_NC 286 /* LD/ST to bits 11:3, S+A, NC */
> +#define R_AARCH64_LDST8_ABS_LO12_NC  278 /* LD/ST to bits 11:0, S+A, NC */

What about R_AARCH64_LDST128_ABS_LO12_NC?

Jan


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