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

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:
> >>> 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?

OK, that would make it easier.
> 
> > @@ -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?

Yes, I can introduce an apply_alternatives_nocheck on ARM that will
hava the same set of arguments on x86.

Or I can make a new function name?
> 
> > --- 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?

Correct. Let me move that and also the 
arch/arm/arm32/livepatch:arch_livepatch_verify_elf
code out of this patch.
> 
> > @@ -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?

I didnt' see it in the ELF of xen-syms or the livepatch tests cases so I 
omitted them.
But then I added some that weren't in the ELF files either: LDST16 and LDST8.

I will add the two missing ones and also sort this.

Thanks!
> 
> Jan
> 

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