live-patching.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Joe Lawrence <joe.lawrence@redhat.com>
To: Nicolas Schier <nicolas@fjasle.eu>
Cc: Petr Mladek <pmladek@suse.com>,
	live-patching@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kbuild@vger.kernel.org
Subject: Re: [RFC PATCH v6 02/12] kbuild: Support for symbols.klp creation
Date: Mon, 18 Apr 2022 14:12:27 -0400	[thread overview]
Message-ID: <Yl2qC7p7NDq4i+9B@redhat.com> (raw)
In-Reply-To: <YlhhHSQIWpLG0Cgn@fjasle.eu>

On Thu, Apr 14, 2022 at 07:59:57PM +0200, Nicolas Schier wrote:
> On Thu, Apr 14, 2022 at 11:35:42AM +0200 Petr Mladek wrote:
> > On Wed 2022-02-16 11:39:30, Joe Lawrence wrote:
> > > From: Joao Moreira <jmoreira@suse.de>
> > > 
> > > For automatic resolution of livepatch relocations, a file called
> > > symbols.klp is used. This file maps symbols within every compiled kernel
> > > object allowing the identification of symbols whose name is unique, thus
> > > relocation can be automatically inferred, or providing information that
> > > helps developers when code annotation is required for solving the
> > > matter.
> > > 
> > > Add support for creating symbols.klp in the main Makefile. First, ensure
> > > that built-in is compiled when CONFIG_LIVEPATCH is enabled (as required
> > > to achieve a complete symbols.klp file). Define the command to build
> > > symbols.klp (cmd_klp_map) and hook it in the modules rule.
> > > 
> > > As it is undesirable to have symbols from livepatch objects inside
> > > symbols.klp, make livepatches discernible by modifying
> > > scripts/Makefile.build to create a .livepatch file for each livepatch in
> > > $(MODVERDIR). This file then used by cmd_klp_map to identify and bypass
> > > livepatches.
> > >
> > > For identifying livepatches during the build process, a flag variable
> > > LIVEPATCH_$(basetarget).o is considered in scripts/Makefile.build. This
> > > way, set this flag for the livepatch sample Makefile in
> > > samples/livepatch/Makefile.
> > 
> > I do not see the related code in scripts/Makefile.build.
> > 
> > > Finally, Add a clean rule to ensure that symbols.klp is removed during
> > > clean.
> > > 
> > > Notes:
> > > 
> > > To achieve a correct symbols.klp file, all kernel objects must be
> > > considered, thus, its construction require these objects to be priorly
> > > built. On the other hand, invoking scripts/Makefile.modpost without
> > > having a complete symbols.klp in place would occasionally lead to
> > > in-tree livepatches being post-processed incorrectly.
> > 
> > Honestly, I do not understand what it exactly means that "in-tree
> > livepatches would occasionally be post-processed incorrectly".
> > 
> > Is it the problem that modpost is not able to handle the unresolved
> > symbols that have to be updated by klp-convert?
> > 
> > > To prevent this
> > > from becoming a circular dependency, the construction of symbols.klp
> > > uses non-post-processed kernel objects and such does not cause harm as
> > > the symbols normally referenced from within livepatches are visible at
> > > this stage. Also due to these requirements, the spot in-between modules
> > > compilation and the invocation of scripts/Makefile.modpost was picked
> > > for hooking cmd_klp_map.
> > > 
> > > The approach based on .livepatch files was proposed as an alternative to
> > > using MODULE_INFO statements. This approach was originally proposed by
> > > Miroslav Benes as a workaround for identifying livepathes without
> > > depending on modinfo during the modpost stage. It was moved to this
> > > patch as the approach also shown to be useful while building
> > > symbols.klp.
> > 
> > All the tricky code is removed in the 5th patch. My understanding is
> > that the problem causing the cyclic dependency is solved by modifying
> > modpost.
> > 
> > It looks like this patch is outdated and mostly obsoleted. On the
> > other hand, the commit message in 5th patch is too short.
> > 
> > What about merging the two patches and updating the commit message?
> 
> +1
> 
> Yes, please merge those patches.  These '$(shell ...)' side-effect lines in the
> definition of 'cmd_klp_map' are quite confusing.
> 

Sure.  Admittedly the kbuild integration is most confusing to me, so I
leaned heavily on Joao's original notes and Masahiro's gracious tips and
refactored code.  I'll try cutting to the final version in later patches
rather than providing all the (confusing) code evolution along the way.

Thanks,
 
-- Joe


  reply	other threads:[~2022-04-18 18:12 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-02-16 16:39 [RFC PATCH v6 00/12] livepatch: klp-convert tool Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 01/12] livepatch: Create and include UAPI headers Joe Lawrence
2022-04-14  8:50   ` Petr Mladek
2022-02-16 16:39 ` [RFC PATCH v6 02/12] kbuild: Support for symbols.klp creation Joe Lawrence
2022-04-14  9:35   ` Petr Mladek
2022-04-14 17:59     ` Nicolas Schier
2022-04-18 18:12       ` Joe Lawrence [this message]
2022-02-16 16:39 ` [RFC PATCH v6 03/12] livepatch: Add klp-convert tool Joe Lawrence
2022-02-16 16:46   ` Joe Lawrence
2022-02-16 16:56   ` Joe Lawrence
2022-04-14 15:03   ` elf API: was: " Petr Mladek
2022-04-18 18:01     ` Joe Lawrence
2023-02-06 18:16   ` Marcos Paulo de Souza
2022-02-16 16:39 ` [RFC PATCH v6 04/12] livepatch: Add klp-convert annotation helpers Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 05/12] modpost: Integrate klp-convert Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 06/12] livepatch: Add sample livepatch module Joe Lawrence
2023-02-07 12:52   ` Marcos Paulo de Souza
2022-02-16 16:39 ` [RFC PATCH v6 07/12] documentation: Update on livepatch elf format Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 08/12] livepatch/selftests: add klp-convert Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 09/12] livepatch/selftests: test multiple sections Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 10/12] livepatch/selftests: add __asm__ symbol renaming examples Joe Lawrence
2022-02-16 17:03   ` Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 11/12] livepatch/selftests: add data relocations test Joe Lawrence
2022-02-16 17:12   ` Joe Lawrence
2022-02-16 16:39 ` [RFC PATCH v6 12/12] livepatch/selftests: add static keys test Joe Lawrence
2022-02-16 17:17 ` [RFC PATCH v6 00/12] livepatch: klp-convert tool Joe Lawrence
2023-02-07 12:57 ` Marcos Paulo de Souza
2023-02-07 15:54   ` Joe Lawrence

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=Yl2qC7p7NDq4i+9B@redhat.com \
    --to=joe.lawrence@redhat.com \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=live-patching@vger.kernel.org \
    --cc=nicolas@fjasle.eu \
    --cc=pmladek@suse.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).