[PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Miroslav Benes]

GCC 9 introduces a new option, -flive-patching. It disables certain
optimizations which could make a compilation unsafe for later live
patching of the running kernel.

The option is used only if CONFIG_LIVEPATCH is enabled and $(CC)
supports it.

Signed-off-by: Miroslav Benes <mbenes@suse.cz>
---
 Makefile | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/Makefile b/Makefile
index a0650bf79606..53f5ab810efe 100644
--- a/Makefile
+++ b/Makefile
@@ -778,6 +778,10 @@ KBUILD_CFLAGS_KERNEL	+= $(call cc-option,-ffunction-sections,)
 KBUILD_CFLAGS_KERNEL	+= $(call cc-option,-fdata-sections,)
 endif
 
+ifdef CONFIG_LIVEPATCH
+KBUILD_CFLAGS += $(call cc-option, -flive-patching=inline-clone)
+endif
+
 # arch Makefile may override CC so keep this after arch Makefile is included
 NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)
 
-- 
2.19.2

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Josh Poimboeuf]

On Wed, Dec 19, 2018 at 03:17:44PM +0100, Miroslav Benes wrote:
> GCC 9 introduces a new option, -flive-patching. It disables certain
> optimizations which could make a compilation unsafe for later live
> patching of the running kernel.
> 
> The option is used only if CONFIG_LIVEPATCH is enabled and $(CC)
> supports it.
> 
> Signed-off-by: Miroslav Benes <mbenes@suse.cz>
> ---
>  Makefile | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/Makefile b/Makefile
> index a0650bf79606..53f5ab810efe 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -778,6 +778,10 @@ KBUILD_CFLAGS_KERNEL	+= $(call cc-option,-ffunction-sections,)
>  KBUILD_CFLAGS_KERNEL	+= $(call cc-option,-fdata-sections,)
>  endif
>  
> +ifdef CONFIG_LIVEPATCH
> +KBUILD_CFLAGS += $(call cc-option, -flive-patching=inline-clone)
> +endif
> +
>  # arch Makefile may override CC so keep this after arch Makefile is included
>  NOSTDINC_FLAGS += -nostdinc -isystem $(shell $(CC) -print-file-name=include)


This option only makes sense for source-based patch generation, so isn't
it a bit premature to make this change without proper source-based patch
tooling?

Also the commit message needs an analysis of the performance impacts.

-- 
Josh

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Jiri Kosina]

On Wed, 19 Dec 2018, Josh Poimboeuf wrote:

> This option only makes sense for source-based patch generation, so isn't 
> it a bit premature to make this change without proper source-based patch 
> tooling?

The reality is though that before the full-fledged patch tooling exists, 
people are actually already writing livepatches by hand, so this option is 
useful for them.

> Also the commit message needs an analysis of the performance impacts.

Agreed. Especially as it's expected (*) to be completely in the noise 
particularly for the kernel, it'd be good to have that documented in the 
changelog.

(*) actually measured already for some subset of the IPA optimizations

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Josh Poimboeuf]

On Wed, Dec 19, 2018 at 05:58:53PM +0100, Jiri Kosina wrote:
> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
> 
> > This option only makes sense for source-based patch generation, so isn't 
> > it a bit premature to make this change without proper source-based patch 
> > tooling?
> 
> The reality is though that before the full-fledged patch tooling exists, 
> people are actually already writing livepatches by hand, so this option is 
> useful for them.

Fair enough.

Though, upstream, almost everybody seems to use kpatch-build, for which
this patch doesn't help.  And people will continue to do so until we
have decent source-based tooling.  Will the klp-convert patches be
revived soon?

-- 
Josh

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Jiri Kosina]

On Wed, 19 Dec 2018, Josh Poimboeuf wrote:

> > > This option only makes sense for source-based patch generation, so isn't 
> > > it a bit premature to make this change without proper source-based patch 
> > > tooling?
> > 
> > The reality is though that before the full-fledged patch tooling exists, 
> > people are actually already writing livepatches by hand, so this option is 
> > useful for them.
> 
> Fair enough.
> 
> Though, upstream, almost everybody seems to use kpatch-build, for which
> this patch doesn't help.  And people will continue to do so until we
> have decent source-based tooling.  Will the klp-convert patches be
> revived soon?

Let me add Joao, who's working on that.

Joao, I think you had something basically ready for upstream exposure, 
right?

Thanks,

-- 
Jiri Kosina
SUSE Labs

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Miroslav Benes]

On Wed, 19 Dec 2018, Jiri Kosina wrote:

> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
> 
> > Also the commit message needs an analysis of the performance impacts.
> 
> Agreed. Especially as it's expected (*) to be completely in the noise 
> particularly for the kernel, it'd be good to have that documented in the 
> changelog.
> 
> (*) actually measured already for some subset of the IPA optimizations

Ok, we can do that. I don't expect the results to be different from the 
last measurement as Jiri mentions. The sets of disabled optimizations are 
similar.

I'll add it to v2.

On Wed, 19 Dec 2018, Jiri Kosina wrote:

> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
> 
> > > > This option only makes sense for source-based patch generation, so isn't 
> > > > it a bit premature to make this change without proper source-based patch 
> > > > tooling?
> > > 
> > > The reality is though that before the full-fledged patch tooling exists, 
> > > people are actually already writing livepatches by hand, so this option is 
> > > useful for them.
> > 
> > Fair enough.

Yes, that was the reason I sent it. It would not make sense to wait for 
the tooling in this case, because -flive-patching is useful even now, 
since there is a way to prepare livepatches without any tooling.

> > Though, upstream, almost everybody seems to use kpatch-build, for which
> > this patch doesn't help.  And people will continue to do so until we
> > have decent source-based tooling.  Will the klp-convert patches be
> > revived soon?
>
> Let me add Joao, who's working on that.
> 
> Joao, I think you had something basically ready for upstream exposure, 
> right?

I think that when Joao posted it a long time ago, the conclusion was that 
it would be better to wait for the source-based tooling and have the 
complete solution. I may misremember though.

If Josh thinks that it would be acceptable to have klp-convert merged even 
without the tooling, I'm all for it.

We're about to start using it at SUSE and staying close to upstream would 
definitely be better.

Miroslav

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Josh Poimboeuf]

On Thu, Dec 20, 2018 at 09:33:05AM +0100, Miroslav Benes wrote:
> > > Though, upstream, almost everybody seems to use kpatch-build, for which
> > > this patch doesn't help.  And people will continue to do so until we
> > > have decent source-based tooling.  Will the klp-convert patches be
> > > revived soon?
> >
> > Let me add Joao, who's working on that.
> > 
> > Joao, I think you had something basically ready for upstream exposure, 
> > right?
> 
> I think that when Joao posted it a long time ago, the conclusion was that 
> it would be better to wait for the source-based tooling and have the 
> complete solution. I may misremember though.
> 
> If Josh thinks that it would be acceptable to have klp-convert merged even 
> without the tooling, I'm all for it.
> 
> We're about to start using it at SUSE and staying close to upstream would 
> definitely be better.

A complete toolchain should definitely be the end goal.

But as a usable first step, only *some* of the tooling is required,
since some of the steps can be done manually, right?

So starting out, for something usable, I believe the following would be
the bare minimum:

  * -flive-patching

  * The analysis tool which analyzes the -fdump-ipa-clones files

  * klp-convert

  * Documentation describing the end-to-end patch creation process,
    including the manual steps and how to use the above tools

Did I miss anything?

Then over time we can fill in the gaps (manual steps) with automation.

-- 
Josh

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Miroslav Benes]

On Thu, 20 Dec 2018, Josh Poimboeuf wrote:

> On Thu, Dec 20, 2018 at 09:33:05AM +0100, Miroslav Benes wrote:
> > > > Though, upstream, almost everybody seems to use kpatch-build, for which
> > > > this patch doesn't help.  And people will continue to do so until we
> > > > have decent source-based tooling.  Will the klp-convert patches be
> > > > revived soon?
> > >
> > > Let me add Joao, who's working on that.
> > > 
> > > Joao, I think you had something basically ready for upstream exposure, 
> > > right?
> > 
> > I think that when Joao posted it a long time ago, the conclusion was that 
> > it would be better to wait for the source-based tooling and have the 
> > complete solution. I may misremember though.
> > 
> > If Josh thinks that it would be acceptable to have klp-convert merged even 
> > without the tooling, I'm all for it.
> > 
> > We're about to start using it at SUSE and staying close to upstream would 
> > definitely be better.
> 
> A complete toolchain should definitely be the end goal.
> 
> But as a usable first step, only *some* of the tooling is required,
> since some of the steps can be done manually, right?
> 
> So starting out, for something usable, I believe the following would be
> the bare minimum:
> 
>   * -flive-patching
> 
>   * The analysis tool which analyzes the -fdump-ipa-clones files
> 
>   * klp-convert
> 
>   * Documentation describing the end-to-end patch creation process,
>     including the manual steps and how to use the above tools
> 
> Did I miss anything?
> 
> Then over time we can fill in the gaps (manual steps) with automation.

Agreed.

Miroslav

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Joao Moreira]

On 12/20/18 12:33 AM, Miroslav Benes wrote:
> On Wed, 19 Dec 2018, Jiri Kosina wrote:
> 
>> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
>>
>>> Also the commit message needs an analysis of the performance impacts.
>>
>> Agreed. Especially as it's expected (*) to be completely in the noise
>> particularly for the kernel, it'd be good to have that documented in the
>> changelog.
>>
>> (*) actually measured already for some subset of the IPA optimizations
> 
> Ok, we can do that. I don't expect the results to be different from the
> last measurement as Jiri mentions. The sets of disabled optimizations are
> similar.
> 
> I'll add it to v2.
> 
> On Wed, 19 Dec 2018, Jiri Kosina wrote:
> 
>> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
>>
>>>>> This option only makes sense for source-based patch generation, so isn't
>>>>> it a bit premature to make this change without proper source-based patch
>>>>> tooling?
>>>>
>>>> The reality is though that before the full-fledged patch tooling exists,
>>>> people are actually already writing livepatches by hand, so this option is
>>>> useful for them.
>>>
>>> Fair enough.
> 
> Yes, that was the reason I sent it. It would not make sense to wait for
> the tooling in this case, because -flive-patching is useful even now,
> since there is a way to prepare livepatches without any tooling.
> 
>>> Though, upstream, almost everybody seems to use kpatch-build, for which
>>> this patch doesn't help.  And people will continue to do so until we
>>> have decent source-based tooling.  Will the klp-convert patches be
>>> revived soon?
>>
>> Let me add Joao, who's working on that.
>>
>> Joao, I think you had something basically ready for upstream exposure,
>> right?
> 
> I think that when Joao posted it a long time ago, the conclusion was that
> it would be better to wait for the source-based tooling and have the
> complete solution. I may misremember though.

Your memories match mine, Miroslav.

FTR, we recently integrated klp-convert to SLE. There were some fixes in 
comparison with the version which was submitted upstream, thus a v2 of 
the patches is necessary.

> 
> If Josh thinks that it would be acceptable to have klp-convert merged even
> without the tooling, I'm all for it.
> 
Of course I can work on that and I'll be glad to do so / submit this new 
version, if this is now something considered useful.

> We're about to start using it at SUSE and staying close to upstream would
> definitely be better.
> 
> Miroslav
> 

Re: [PATCH] kbuild: use -flive-patching when CONFIG_LIVEPATCH is enabled

[Miroslav Benes]

On Thu, 20 Dec 2018, Joao Moreira wrote:

> On 12/20/18 12:33 AM, Miroslav Benes wrote:
> > On Wed, 19 Dec 2018, Jiri Kosina wrote:
> > 
> >> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
> >>
> >>> Also the commit message needs an analysis of the performance impacts.
> >>
> >> Agreed. Especially as it's expected (*) to be completely in the noise
> >> particularly for the kernel, it'd be good to have that documented in the
> >> changelog.
> >>
> >> (*) actually measured already for some subset of the IPA optimizations
> > 
> > Ok, we can do that. I don't expect the results to be different from the
> > last measurement as Jiri mentions. The sets of disabled optimizations are
> > similar.
> > 
> > I'll add it to v2.
> > 
> > On Wed, 19 Dec 2018, Jiri Kosina wrote:
> > 
> >> On Wed, 19 Dec 2018, Josh Poimboeuf wrote:
> >>
> >>>>> This option only makes sense for source-based patch generation, so isn't
> >>>>> it a bit premature to make this change without proper source-based patch
> >>>>> tooling?
> >>>>
> >>>> The reality is though that before the full-fledged patch tooling exists,
> >>>> people are actually already writing livepatches by hand, so this option
> >>>> is
> >>>> useful for them.
> >>>
> >>> Fair enough.
> > 
> > Yes, that was the reason I sent it. It would not make sense to wait for
> > the tooling in this case, because -flive-patching is useful even now,
> > since there is a way to prepare livepatches without any tooling.
> > 
> >>> Though, upstream, almost everybody seems to use kpatch-build, for which
> >>> this patch doesn't help.  And people will continue to do so until we
> >>> have decent source-based tooling.  Will the klp-convert patches be
> >>> revived soon?
> >>
> >> Let me add Joao, who's working on that.
> >>
> >> Joao, I think you had something basically ready for upstream exposure,
> >> right?
> > 
> > I think that when Joao posted it a long time ago, the conclusion was that
> > it would be better to wait for the source-based tooling and have the
> > complete solution. I may misremember though.
> 
> Your memories match mine, Miroslav.
> 
> FTR, we recently integrated klp-convert to SLE. There were some fixes in
> comparison with the version which was submitted upstream, thus a v2 of the
> patches is necessary.
> 
> > 
> > If Josh thinks that it would be acceptable to have klp-convert merged even
> > without the tooling, I'm all for it.
> > 
> Of course I can work on that and I'll be glad to do so / submit this new
> version, if this is now something considered useful.

Yes, please.

Some context first. We decided to remove the integration into kbuild at 
SUSE. klp-convert is called from rpm .spec file directly after a livepatch 
module is compiled.

I think we should preserve kbuild process in upstream though. It makes 
more sense there.

Miroslav