Re: [PATCH] [AArch64] support -mfentry feature for arm64

[Andrew Pinski <pinskia at gmail dot com>]

On Thu, Apr 14, 2016 at 9:08 PM, Maxim Kuvyrkov
<maxim.kuvyrkov@linaro.org> wrote:
> On Mar 14, 2016, at 11:14 AM, Li Bin <huawei.libin@huawei.com> wrote:
>>
>> As ARM64 is entering enterprise world, machines can not be stopped for
>> some critical enterprise production environment, that is, live patch as
>> one of the RAS features is increasing more important for ARM64 arch now.
>>
>> Now, the mainstream live patch implementation which has been merged in
>> Linux kernel (x86/s390) is based on the 'ftrace with regs' feature, and
>> this feature needs the help of gcc.
>>
>> This patch proposes a generic solution for arm64 gcc which called mfentry,
>> following the example of x86, mips, s390, etc. and on these archs, this
>> feature has been used to implement the ftrace feature 'ftrace with regs'
>> to support live patch.
>>
>> By now, there is an another solution from linaro [1], which proposes to
>> implement a new option -fprolog-pad=N that generate a pad of N nops at the
>> beginning of each function. This solution is a arch-independent way for gcc,
>> but there may be some limitations which have not been recognized for Linux
>> kernel to adapt to this solution besides the discussion on [2]
>
> It appears that implementing -fprolog-pad=N option in GCC will not enable kernel live-patching support for AArch64.  The proposal for the option was to make GCC output a given number of NOPs at the beginning of each function, and then the kernel could use that NOP pad to insert whatever instructions it needs.  The modification of kernel instruction stream needs to be done atomically, and, unfortunately, it seems the kernel can use only architecture-provided atomicity primitives -- i.e., changing at most 8 bytes at a time.
>

Can't we add a 16byte atomic primitive for ARM64 to the kernel?
Though you need to align all functions to a 16 byte boundary if the
-fprolog-pag=N needs to happen.  Do you know what the size that needs
to be modified?  It does seem to be either 12 or 16 bytes.

> From the kernel discussion thread it appears that the pad needs to be more than 8 bytes, and that the kernel can't update that atomically.  However if -mfentry approach is used, then we need to update only 4 (or 8) bytes of the pad, and we avoid the atomicity problem.

I think you are incorrect, you could add a 16 byte atomic primitive if needed.

>
> Therefore, [unless there is a clever multi-stage update process to atomically change NOPs to whatever we need,] I think we have to go with Li's -mfentry approach.

Please consider the above of having a 16 byte (128bit) atomic
instructions be available would that be enough?

Thanks,
Andrew

>
> Comments?
>
> --
> Maxim Kuvyrkov
> www.linaro.org
>
>
>> , typically
>> for powerpc archs. Furthermore I think there are no good reasons to promote
>> the other archs (such as x86) which have implemented the feature 'ftrace with regs'
>> to replace the current method with the new option, which may bring heavily
>> target-dependent code adaption, as a result it becomes a arm64 dedicated
>> solution, leaving kernel with two different forms of implementation.
>>
>> [1] https://gcc.gnu.org/ml/gcc/2015-10/msg00090.html
>> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2016-January/401854.html
>