Re: [PATCH v5 3/4] livepatch: NOP if func->new_addr is zero.

["Jan Beulich" <JBeulich@suse.com>]

>>> On 11.09.16 at 17:48, <konrad.wilk@oracle.com> wrote:
> The NOP functionality will NOP any of the code at
> the 'old_addr' or at 'name' if the 'new_addr' is zero.
> The purpose of this is to NOP out calls, such as:
> 
>  e8 <4-bytes-offset>
> 
> (5 byte insn), or on ARM a 4 byte insn for branching.
> 
> We need the EIP of where we need to the NOP, and that can
> be provided via the `old_addr` or `name`.
> 
> If the `old_addr` is provided we will NOP 'new_size'
> amount of bytes at that location.
> 
> The amount is up to 31 instructions if desired (which is
> the size of the opaque member). If there is a need to NOP
> more then either more 'struct livepatch_func' structures need
> to be present or we have to implement a variable size buffer.

Actually I meanwhile realized that if we wanted to void more than
this many, it would probably be better to simply make the first bytes
an unconditional branch skipping the to be disabled code (of course
provided there are no branch targets in the middle).

> --- a/xen/include/xen/livepatch.h
> +++ b/xen/include/xen/livepatch.h
> @@ -66,7 +66,16 @@ int arch_livepatch_secure(const void *va, unsigned int pages, enum va_type types
>  void arch_livepatch_init(void);
>  
>  #include <public/sysctl.h> /* For struct livepatch_func. */
> +#include <asm/livepatch.h> /* For PATCH_INSN_SIZE. */
>  int arch_livepatch_verify_func(const struct livepatch_func *func);
> +
> +static inline size_t arch_livepatch_insn_len(const struct livepatch_func *func)

This being defined (rather than just declared) here makes me question
the arch_ prefix.

> +{
> +    if ( !func->new_addr )
> +        return func->new_size;
> +
> +    return PATCH_INSN_SIZE;

This, otoh, now might better be ARCH_PATCH_INSN_SIZE.

Jan


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