Re: [Xen-devel] [PATCH v3] xen/arm32: Introduce alternative runtime patching

[Julien Grall <julien.grall@xxxxxxx>]

Hi Wei,

On 30/03/17 10:02, Wei Chen wrote:
> +/* Unconditional branch instructions */
> +/*
> + * From ARM DDI 0406C.c Section A8.8.25. We can see blx has a H bit.
> + * In an ARM/Thumb instructions mixed running environment, this bit
> + * can be 1 or 0. Although Xen is only using the ARM instructions
> + * and the H bit is always 0. We mask this bit to catch both of these
> + * two encodings for future-proof.
> + */
> +__AARCH32_INSN_FUNCS(blx, 0x0E000000, 0x0A000000)
> +
> +int32_t aarch32_get_branch_offset(uint32_t insn);
> +uint32_t aarch32_set_branch_offset(uint32_t insn, int32_t offset);
> +
> +/* Wrapper for common code */
> +static inline bool insn_is_branch_imm(uint32_t insn)
> +{
> +    /* Check conditional branch instructions */
> +    if ( __CONDITIONAL_INSN(insn) )
This is not what I asked. Whilst I am happy to see a different solution 
this one is completely wrong. b and bl instruction cannot be 
unconditional and a user of the helpers aarch32_insn_is_{b,bl} would 
expect them to do the right things.
By "open-coding" I meant dropping the macro __AARCH32_INS_FUNCS and 
directly implement insn_is_branch_imm with:
return ( (insn & 0x0E000000) == 0x0A000000 );

With the comment on top explain the check.

Cheers,

--
Julien Grall

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