Re: [Xen-devel] [PATCH v5 07/16] livepatch: ARM 32|64: Ignore mapping symbols: $[d, a, x]

[Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>]

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@xxxxxxxxxx> [x86 bits]
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
>
> ---
> Cc: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Cc: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
> Cc: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> Cc: Julien Grall <julien.grall@xxxxxxx
> Cc: Jan Beulich <jbeulich@xxxxxxxx>
> Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>
> 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@xxxxxxxxxx>

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