arm64:, Re: [RFC] Kernel livepatching support in GCC

arm64:, Re: [RFC] Kernel livepatching support in GCC

[AKASHI Takahiro]

Li,
(added linux-arm-kernel to Cc.)

On 10/22/2015 06:07 PM, libin wrote:
>
>
> ? 2015/5/28 16:39, Maxim Kuvyrkov ??:
>> Hi,
>>
>> Akashi-san and I have been discussing required GCC changes to make kernel's livepatching work for AArch64 and other
>> architectures.  At the moment livepatching is supported for x86[_64] using the following options: "-pg -mfentry
>> -mrecord-mcount -mnop-mcount" which is geek-speak for "please add several NOPs at the very beginning of each function,
>> and make a section with addresses of all those NOP pads".
>>
>> The above long-ish list of options is a historical artifact of how livepatching support evolved for x86.  The end
>> result is that for livepatching (or ftrace, or possible future kernel features) to work compiler needs to generate a
>> little bit of empty code space at the beginning of each function.  Kernel can later use that space to insert call
>> sequences for various hooks.
>>
>> Our proposal is that instead of adding -mfentry/-mnop-count/-mrecord-mcount options to other architectures, we should
>> implement a target-independent option -fprolog-pad=N, which will generate a pad of N nops at the beginning of each
>> function and add a section entry describing the pad similar to -mrecord-mcount [1].
>>
>> Since adding NOPs is much less architecture-specific then outputting call instruction sequences, this option can be
>> handled in a target-independent way at least for some/most architectures.
>>
>> Comments?
>>
>> As I found out today, the team from Huawei has implemented [2], which follows x86 example of -mfentry option
>> generating a hard-coded call sequence.  I hope that this proposal can be easily incorporated into their work since
>> most of the livepatching changes are in the kernel.
>>
>
> Thanks very much for your effort for this, and the arch-independed implementation
> is very good to me, but only have one question that how to enture the atomic
> replacement of multi instructions in kernel side?

I have one idea, but we'd better discuss this topic in, at least including, linux-arm-kernel.

> And before this arch-independed option, can we consider the arch-depended -mfentry
> implemention for arm64 like arch x86 firstly? I will post it soon.
>
> livepatch for arm64 based on this arm64 -mfentry feature on github:
> https://github.com/libin2015/livepatch-for-arm64.git  master


I also have my own version of livepatch support for arm64 using yet-coming "-fprolog-add=N" option :)
As we discussed before, the main difference will be how we should preserve LR register when invoking
a ftrace hook (ftrace_regs_caller).
But again, this is a topic to discuss mainly in linux-arm-kernel.
(I have no intention of excluding gcc ml from the discussions.)

Thanks,
-Takahiro AKASHI

> discussions on this topic:
> https://lkml.org/lkml/2015/5/28/54
>
> Thanks,
> Li Bin
>
>> [1] Technically, generating a NOP pad and adding a section entry in .__mcount_loc are two separate actions, so we may
>> want to have a -fprolog-pad-record option.  My instinct is to stick with a single option for now, since we can always
>> add more later.
>>
>> [2] http://lists.infradead.org/pipermail/linux-arm-kernel/2015-May/346905.html
>>
>> --
>> Maxim Kuvyrkov
>> www.linaro.org
>>
>>
>>
>>
>>
>

arm64:, Re: [RFC] Kernel livepatching support in GCC

[Szabolcs Nagy]

On 22/10/15 11:14, AKASHI Takahiro wrote:
> On 10/22/2015 06:07 PM, libin wrote:
>> ? 2015/5/28 16:39, Maxim Kuvyrkov ??:
>>> Our proposal is that instead of adding -mfentry/-mnop-count/-mrecord-mcount options to other architectures,
>>> we should
>>> implement a target-independent option -fprolog-pad=N, which will generate a pad of N nops at the beginning
>>> of each
>>> function and add a section entry describing the pad similar to -mrecord-mcount [1].
>>>
>>> Since adding NOPs is much less architecture-specific then outputting call instruction sequences, this option
>>> can be
>>> handled in a target-independent way at least for some/most architectures.
>>>
>>> Comments?
>>>
>>> As I found out today, the team from Huawei has implemented [2], which follows x86 example of -mfentry option
>>> generating a hard-coded call sequence.  I hope that this proposal can be easily incorporated into their work
>>> since
>>> most of the livepatching changes are in the kernel.
>>>
>>
>> Thanks very much for your effort for this, and the arch-independed implementation
>> is very good to me, but only have one question that how to enture the atomic
>> replacement of multi instructions in kernel side?
>
> I have one idea, but we'd better discuss this topic in, at least including, linux-arm-kernel.
>
>> And before this arch-independed option, can we consider the arch-depended -mfentry
>> implemention for arm64 like arch x86 firstly? I will post it soon.
>>
>> livepatch for arm64 based on this arm64 -mfentry feature on github:
>> https://github.com/libin2015/livepatch-for-arm64.git  master
>
>
> I also have my own version of livepatch support for arm64 using yet-coming "-fprolog-add=N" option :)
> As we discussed before, the main difference will be how we should preserve LR register when invoking
> a ftrace hook (ftrace_regs_caller).
> But again, this is a topic to discuss mainly in linux-arm-kernel.
> (I have no intention of excluding gcc ml from the discussions.)

is -fprolog-add=N enough from gcc?

i assume it solves the live patching, but i thought -mfentry
might be still necessary when live patching is not used.

or is the kernel fine with the current mcount abi for that?
(note that changes the code generation in leaf functions
and currently the kernel relies on frame pointers etc.)

arm64:, Re: [RFC] Kernel livepatching support in GCC

[AKASHI Takahiro]

On 10/22/2015 07:26 PM, Szabolcs Nagy wrote:
> On 22/10/15 11:14, AKASHI Takahiro wrote:
>> On 10/22/2015 06:07 PM, libin wrote:
>>> ? 2015/5/28 16:39, Maxim Kuvyrkov ??:
>>>> Our proposal is that instead of adding -mfentry/-mnop-count/-mrecord-mcount options to other architectures,
>>>> we should
>>>> implement a target-independent option -fprolog-pad=N, which will generate a pad of N nops at the beginning
>>>> of each
>>>> function and add a section entry describing the pad similar to -mrecord-mcount [1].
>>>>
>>>> Since adding NOPs is much less architecture-specific then outputting call instruction sequences, this option
>>>> can be
>>>> handled in a target-independent way at least for some/most architectures.
>>>>
>>>> Comments?
>>>>
>>>> As I found out today, the team from Huawei has implemented [2], which follows x86 example of -mfentry option
>>>> generating a hard-coded call sequence.  I hope that this proposal can be easily incorporated into their work
>>>> since
>>>> most of the livepatching changes are in the kernel.
>>>>
>>>
>>> Thanks very much for your effort for this, and the arch-independed implementation
>>> is very good to me, but only have one question that how to enture the atomic
>>> replacement of multi instructions in kernel side?
>>
>> I have one idea, but we'd better discuss this topic in, at least including, linux-arm-kernel.
>>
>>> And before this arch-independed option, can we consider the arch-depended -mfentry
>>> implemention for arm64 like arch x86 firstly? I will post it soon.
>>>
>>> livepatch for arm64 based on this arm64 -mfentry feature on github:
>>> https://github.com/libin2015/livepatch-for-arm64.git  master
>>
>>
>> I also have my own version of livepatch support for arm64 using yet-coming "-fprolog-add=N" option :)
>> As we discussed before, the main difference will be how we should preserve LR register when invoking
>> a ftrace hook (ftrace_regs_caller).
>> But again, this is a topic to discuss mainly in linux-arm-kernel.
>> (I have no intention of excluding gcc ml from the discussions.)
>
> is -fprolog-add=N enough from gcc?

Yes, as far as I correctly understand this option.

> i assume it solves the live patching, but i thought -mfentry
> might be still necessary when live patching is not used.

No.
- Livepatch depends on ftrace's DYNAMIC_FTRACE_WITH_REGS feature
- DYNAMIC_FTRACE_WITH_REGS can be implemented either with -fprolog-add=N or -mfentry
- x86 is the only architecture that supports -mfentry AFAIK
- and it is used in the kernel solely to implement this ftrace feature AFAIK
- So once a generic option, fprolog-add=N, is supported, we have no reason to add arch-specific -mfentry.

> or is the kernel fine with the current mcount abi for that?
> (note that changes the code generation in leaf functions

Can you please elaborate your comments in more details?
I didn't get your point here.

Thanks,
-Takahiro AKASHI

> and currently the kernel relies on frame pointers etc.)
>

arm64:, Re: [RFC] Kernel livepatching support in GCC

[Szabolcs Nagy]

On 23/10/15 10:11, AKASHI Takahiro wrote:
> On 10/22/2015 07:26 PM, Szabolcs Nagy wrote:
>> On 22/10/15 11:14, AKASHI Takahiro wrote:
>>>
>>> I also have my own version of livepatch support for arm64 using yet-coming "-fprolog-add=N" option :)
>>> As we discussed before, the main difference will be how we should preserve LR register when invoking
>>> a ftrace hook (ftrace_regs_caller).
>>> But again, this is a topic to discuss mainly in linux-arm-kernel.
>>> (I have no intention of excluding gcc ml from the discussions.)
>>
>> is -fprolog-add=N enough from gcc?
>
> Yes, as far as I correctly understand this option.
>
>> i assume it solves the live patching, but i thought -mfentry
>> might be still necessary when live patching is not used.
>
> No.
> - Livepatch depends on ftrace's DYNAMIC_FTRACE_WITH_REGS feature
> - DYNAMIC_FTRACE_WITH_REGS can be implemented either with -fprolog-add=N or -mfentry
> - x86 is the only architecture that supports -mfentry AFAIK
> - and it is used in the kernel solely to implement this ftrace feature AFAIK
> - So once a generic option, fprolog-add=N, is supported, we have no reason to add arch-specific -mfentry.
>
>> or is the kernel fine with the current mcount abi for that?
>> (note that changes the code generation in leaf functions
>
> Can you please elaborate your comments in more details?
> I didn't get your point here.
>

ok, i may be confused.

i thought there is a static ftrace (functions are
instrumented with mcount using -pg) and a dynamic one
where the code is modified at runtime.

then i thought adding -fprolog-pad=N would be good for the
dynamic case, but not for the static case.

the static case may need improvements too because the
current way (using regular c call abi for mcount) affects
code generation more significantly than the proposed
-mfentry solution would (e.g. leaf functions turn into
non-leaf ones).

hence the question: is the kernel satisfied with -pg mcount
for the static ftrace or does it want -mfentry behaviour
instead?