Received-SPF: pass (google.com: domain of linux-kernel+bounces-194228-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.48.161 as permitted sender) client-ip=147.75.48.161;
From: Marcos Paulo de Souza <mpdesouza@suse.com>
To: Lukas Hruska <lhruska@suse.cz>
Cc: mpdesouza@suse.com,
	pmladek@suse.com,
	mbenes@suse.cz,
	jpoimboe@kernel.org,
	joe.lawrence@redhat.com,
	live-patching@vger.kernel.org,
	linux-kernel@vger.kernel.org,
	linux-kbuild@vger.kernel.org
Subject: Re: [PATCH v2 0/5] livepatch: klp-convert tool - Minimal version
Date: Wed, 29 May 2024 11:05:27 -0300
Message-ID: <20240529140536.18137-1-mpdesouza@suse.com>
In-Reply-To: <20240516133009.20224-1-lhruska@suse.cz>
References: 
Precedence: bulk
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit

From: mpdesouza@suse.com

On Thu, 16 May 2024 15:30:03 +0200 Lukas Hruska <lhruska@suse.cz> wrote:

> Summary
> -------
> 
> This is a significantly simplified version of the original klp-convert tool.
> The klp-convert code has never got a proper review and also clean ups
> were not easy. The last version was v7, see
> https://lore.kernel.org/r/20230306140824.3858543-1-joe.lawrence@redhat.com
> 
> The main change is that the tool does not longer search for the
> symbols which would need the livepatch specific relocation entry.
> Also klp.symbols file is not longer needed.
> 
> Instead, the needed information is appended to the symbol declaration
> via a new macro KLP_RELOC_SYMBOL(). It creates symbol with all needed
> metadata. For example:
> 
>   extern char *saved_command_line \
>                  KLP_RELOC_SYMBOL(vmlinux, vmlinux, saved_command_line, 0);
> 
> would create symbol
> 
> $>readelf -r -W <compiled livepatch module>:
> Relocation section '.rela.text' at offset 0x32e60 contains 10 entries:
>     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
> [...]
> 0000000000000068  0000003c00000002 R_X86_64_PC32          0000000000000000 .klp.sym.rela.vmlinux.vmlinux.saved_command_line,0 - 4
> [...]
> 
> 
> The simplified klp-convert tool just transforms symbols
> created by KLP_RELOC_SYMBOL() to object specific rela sections
> and rela entries which would later be proceed when the livepatch
> or the livepatched object is loaded.
> 
> For example, klp-convert would replace the above symbols with:
> 
> $> readelf -r -W <livepatch_module_proceed_by_klp_convert>
> Relocation section '.klp.rela.vmlinux.text' at offset 0x5cb60 contains 1 entry:
>     Offset             Info             Type               Symbol's Value  Symbol's Name + Addend
> 0000000000000068  0000003c00000002 R_X86_64_PC32          0000000000000000 .klp.sym.vmlinux.saved_command_line,0 - 4
> 
> 
> Note that similar macro was needed also in the original version
> to handle more symbols of the same name (sympos).
> 
> Given the above, add klp-convert tool; integrate klp-convert tool into
> kbuild; add data-structure and macros to enable users to annotate
> livepatch source code; make modpost stage compatible with livepatches;
> update livepatch-sample and update documentation.
> 
> 
> Testing
> -------
> 
> The patchset selftests build and execute on x86_64, s390x, and ppc64le
> for both default config (with added livepatch dependencies) and a larger
> SLE-15-ish config.
> 
> 
> Summary of changes in this minimal version v2
> ------------------------
> 
> - rebase for v6.9
> - cleaned-up SoB chains (suggested by pmladek)
> - klp-convert: remove the symbol map auto-resolving solution
> - klp-convert: add macro for flagging variables inside a LP src to be resolved by this tool
> - klp-convert: code simplification
> - selftests: add selftest livepatching function using an external symbol

Thanks for sending this new version Lukas! It currently fails to apply on
current Linux master, but the conflict is very simple to address (attached
patch).

Joe, Josh, other people, can you also take a look in the patchset? It would be
good we can move this forward.

Thanks,
  Marcos

commit 1a1cf8b0967c26857b17e8ceb02f6a1bd854667d
Author: Marcos Paulo de Souza <mpdesouza@suse.com>
Date:   Wed May 29 10:18:38 2024 -0300

    Solve merge problem
    
    Signed-off-by: Marcos Paulo de Souza <mpdesouza@suse.com>

diff --git a/Makefile b/Makefile
index f975b6396328..579dfb46e691 100644
--- a/Makefile
+++ b/Makefile
@@ -1491,7 +1491,7 @@ endif # CONFIG_MODULES
 # Directories & files removed with 'make clean'
 CLEAN_FILES += vmlinux.symvers modules-only.symvers \
 	       modules.builtin modules.builtin.modinfo modules.nsdeps \
-	       compile_commands.json rust/test \
+	       compile_commands.json .thinlto-cache rust/test \
 	       rust-project.json .vmlinux.objs .vmlinux.export.c
 
 # Directories & files removed with 'make mrproper'

> 
> Previous versions
> -----------------
> 
> RFC:
>   https://lore.kernel.org/r/cover.1477578530.git.jpoimboe@redhat.com/
> v2:
>   https://lore.kernel.org/r/f52d29f7-7d1b-ad3d-050b-a9fa8878faf2@redhat.com/
> v3:
>   https://lore.kernel.org/r/20190410155058.9437-1-joe.lawrence@redhat.com/
> v4:
>   https://lore.kernel.org/r/20190509143859.9050-1-joe.lawrence@redhat.com/
> v5:
>   (not posted)
>   https://github.com/joe-lawrence/klp-convert-tree/tree/klp-convert-v5-devel
> v6:
>   https://lore.kernel.org/r/20220216163940.228309-1-joe.lawrence@redhat.com/
> v7:
>   https://lore.kernel.org/r/20230306140824.3858543-1-joe.lawrence@redhat.com/
> v1 minimal:
>   https://lore.kernel.org/r/20231106162513.17556-1-lhruska@suse.cz/