| Message ID | 1474479154-20991-8-git-send-email-konrad.wilk@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Konrad, On 21/09/16 18:32, Konrad Rzeszutek Wilk wrote: > Those symbols are used to help final linkers to replace insn. > The ARM ELF specification mandates that they are present > to denote the start of certain CPU features. There are two > variants of it - short and long format. > > Either way - we can ignore these symbols. > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> [x86 bits] > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> Acked-by: Julien Grall <julien.grall@arm.com> Regards,
On 09/21/2016 06:32 PM, Konrad Rzeszutek Wilk wrote: > Those symbols are used to help final linkers to replace insn. > The ARM ELF specification mandates that they are present > to denote the start of certain CPU features. There are two > variants of it - short and long format. > > Either way - we can ignore these symbols. > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com> [x86 bits] > Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > > --- > Cc: Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> > Cc: Ross Lagerwall <ross.lagerwall@citrix.com> > Cc: Stefano Stabellini <sstabellini@kernel.org> > Cc: Julien Grall <julien.grall@arm.com > Cc: Jan Beulich <jbeulich@suse.com> > Cc: Andrew Cooper <andrew.cooper3@citrix.com> > > v1: First submission > v2: Update the order of symbols, fix title > Add {} in after the first if - per Jan's recommendation. > v3: Add Andrew's Review tag > Make the function return an bool_t. > Skip check for '$t' > Fix spelling of comments. > s/arch_is_payload_symbol/arch_livepatch_symbol_ok/ > v4: s/bool_t/bool/ > v5: Removed an extra space in the conditional. > --- > xen/arch/arm/livepatch.c | 33 +++++++++++++++++++++++++++++++++ > xen/arch/x86/livepatch.c | 7 +++++++ > xen/common/livepatch.c | 2 +- > xen/include/xen/livepatch.h | 2 ++ > 4 files changed, 43 insertions(+), 1 deletion(-) > > diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c > index 9284766..9959315 100644 > --- a/xen/arch/arm/livepatch.c > +++ b/xen/arch/arm/livepatch.c > @@ -84,6 +84,39 @@ void arch_livepatch_unmask(void) > local_abort_enable(); > } > > +bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf, > + const struct livepatch_elf_sym *sym) > +{ > + /* > + * - Mapping symbols - denote the "start of a sequence of bytes of the > + * appropriate type" to mark certain features - such as start of region > + * containing data ($d); ARM ($a), or A64 ($x) instructions. > + * We ignore Thumb instructions ($t) as we shouldn't have them. > + * > + * The format is either short: '$x' or long: '$x.<any>'. We do not > + * need this and more importantly - each payload will contain this > + * resulting in symbol collisions. > + */ > + if ( *sym->name == '$' && sym->name[1] != '\0' ) This would be clear (IMO) as sym->name[0]. Either way, Reviewed-by: Ross Lagerwall <ross.lagerwall@citrix.com>
diff --git a/xen/arch/arm/livepatch.c b/xen/arch/arm/livepatch.c index 9284766..9959315 100644 --- a/xen/arch/arm/livepatch.c +++ b/xen/arch/arm/livepatch.c @@ -84,6 +84,39 @@ void arch_livepatch_unmask(void) local_abort_enable(); } +bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym) +{ + /* + * - Mapping symbols - denote the "start of a sequence of bytes of the + * appropriate type" to mark certain features - such as start of region + * containing data ($d); ARM ($a), or A64 ($x) instructions. + * We ignore Thumb instructions ($t) as we shouldn't have them. + * + * The format is either short: '$x' or long: '$x.<any>'. We do not + * need this and more importantly - each payload will contain this + * resulting in symbol collisions. + */ + if ( *sym->name == '$' && sym->name[1] != '\0' ) + { + char p = sym->name[1]; + size_t len = strlen(sym->name); + + if ( (len >= 3 && (sym->name[2] == '.' )) || (len == 2) ) + { + if ( p == 'd' || +#ifdef CONFIG_ARM_32 + p == 'a' +#else + p == 'x' +#endif + ) + return false; + } + } + return true; +} + int arch_livepatch_perform_rel(struct livepatch_elf *elf, const struct livepatch_elf_sec *base, const struct livepatch_elf_sec *rela) diff --git a/xen/arch/x86/livepatch.c b/xen/arch/x86/livepatch.c index b0d81d7..7a369a0 100644 --- a/xen/arch/x86/livepatch.c +++ b/xen/arch/x86/livepatch.c @@ -124,6 +124,13 @@ int arch_livepatch_verify_elf(const struct livepatch_elf *elf) return 0; } +bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym) +{ + /* No special checks on x86. */ + return true; +} + int arch_livepatch_perform_rel(struct livepatch_elf *elf, const struct livepatch_elf_sec *base, const struct livepatch_elf_sec *rela) diff --git a/xen/common/livepatch.c b/xen/common/livepatch.c index f2f866c..082f553 100644 --- a/xen/common/livepatch.c +++ b/xen/common/livepatch.c @@ -749,7 +749,7 @@ static bool_t is_payload_symbol(const struct livepatch_elf *elf, !strncmp(sym->name, ".L", 2) ) return 0; - return 1; + return arch_livepatch_symbol_ok(elf, sym); } static int build_symbol_table(struct payload *payload, diff --git a/xen/include/xen/livepatch.h b/xen/include/xen/livepatch.h index b7b84e7..e8c67d6 100644 --- a/xen/include/xen/livepatch.h +++ b/xen/include/xen/livepatch.h @@ -48,6 +48,8 @@ bool_t is_patch(const void *addr); /* Arch hooks. */ int arch_livepatch_verify_elf(const struct livepatch_elf *elf); +bool arch_livepatch_symbol_ok(const struct livepatch_elf *elf, + const struct livepatch_elf_sym *sym); int arch_livepatch_perform_rel(struct livepatch_elf *elf, const struct livepatch_elf_sec *base, const struct livepatch_elf_sec *rela);