[RFC PATCH 0/2] kvmtool: Rework guest/init integration

[RFC PATCH 0/2] kvmtool: Rework guest/init integration

[Andre Przywara]

Hi,

this mini series aims at solving long standing issues with compiling
and linking the guest/init binary for MIPS.
It seems that many MIPS toolchains use different default ELF targets
for the compiler and the linker, rendering the approach of linking
the guest/init executable into the lkvm binary moot.
One could work around this by specifying some magic ELF target
options to the linker so it matches the compiler ones, but I couldn't
find a way of automatically determining those, so this approach is
only valid on a particular toolchain.
Another approach would be to fixup the ELF header, but that sounds
dodgy and fragile to me.

Instead of using "ld" this series transforms the generated guest
binary into a C file, which gets compiled (with CC) and thus
automatically links fine with the other object files.
Patch 2/2 implements this, patch 1/2 cleans up the rules for the
guest binary generation in the Makefile.

This uses the "xxd" tool, which has a special command line option
to generate a C array out of a binary blob. On the distributions
I checked, this comes with the vim package, not sure if that is a
restriction.

I compile tested this on on PowerPC64, MIPS64, ARM, ARM64, i386 and
x86_64.

Please test whether this works with your toolchain / system!

Cheers,
Andre.

Andre Przywara (2):
  Makefile: cleanup guest/init generation
  Makefile: use xxd for converting guest/init

 Makefile        | 21 ++++++++++++---------
 builtin-run.c   |  8 ++++----
 builtin-setup.c |  8 ++++----
 3 files changed, 20 insertions(+), 17 deletions(-)

-- 
2.3.5

[RFC PATCH 1/2] Makefile: cleanup guest/init generation

[Andre Przywara]

The dependencies and targets for the guest userland binary are
currently not correct, some are redundant.
Fix them by splitting up guest/guest_init.o creation into its two
steps and describe the dependencies properly.
On the way use automatic variables in some rules.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/Makefile b/Makefile
index 151fa9d..b9480ff 100644
--- a/Makefile
+++ b/Makefile
@@ -337,7 +337,7 @@ ifneq ($(WERROR),0)
 	CFLAGS += -Werror
 endif
 
-all: $(PROGRAM) $(PROGRAM_ALIAS) $(GUEST_INIT)
+all: $(PROGRAM) $(PROGRAM_ALIAS)
 
 # CFLAGS used when building objects
 # This is intentionally not assigned using :=
@@ -352,22 +352,25 @@ c_flags	= -Wp,-MD,$(depfile) $(CFLAGS)
 STATIC_OBJS = $(patsubst %.o,%.static.o,$(OBJS) $(OBJS_STATOPT))
 GUEST_OBJS = guest/guest_init.o
 
-$(PROGRAM)-static:  $(STATIC_OBJS) $(OTHEROBJS) $(GUEST_INIT)
+$(PROGRAM)-static:  $(STATIC_OBJS) $(OTHEROBJS) $(GUEST_OBJS)
 	$(E) "  LINK    " $@
-	$(Q) $(CC) -static $(CFLAGS) $(STATIC_OBJS) $(OTHEROBJS) $(GUEST_OBJS) $(LIBS) $(LIBS_STATOPT) -o $@
+	$(Q) $(CC) -static $(CFLAGS) $^ $(LIBS) $(LIBS_STATOPT) -o $@
 
-$(PROGRAM): $(OBJS) $(OBJS_DYNOPT) $(OTHEROBJS) $(GUEST_INIT)
+$(PROGRAM): $(OBJS) $(OBJS_DYNOPT) $(OTHEROBJS) $(GUEST_OBJS)
 	$(E) "  LINK    " $@
-	$(Q) $(CC) $(CFLAGS) $(OBJS) $(OBJS_DYNOPT) $(OTHEROBJS) $(GUEST_OBJS) $(LIBS) $(LIBS_DYNOPT) -o $@
+	$(Q) $(CC) $(CFLAGS) $^ $(LIBS) $(LIBS_DYNOPT) -o $@
 
 $(PROGRAM_ALIAS): $(PROGRAM)
 	$(E) "  LN      " $@
-	$(Q) ln -f $(PROGRAM) $@
+	$(Q) ln -f $< $@
 
-$(GUEST_INIT): guest/init.c
+$(GUEST_OBJS): $(GUEST_INIT)
 	$(E) "  LINK    " $@
-	$(Q) $(CC) -static guest/init.c -o $@
-	$(Q) $(LD) $(LDFLAGS) -r -b binary -o guest/guest_init.o $(GUEST_INIT)
+	$(Q) $(LD) $(LDFLAGS) -r -b binary -o $@ $<
+
+$(GUEST_INIT): guest/init.c
+	$(E) "  CC      " $@
+	$(Q) $(CC) -static $^ -o $@
 
 %.s: %.c
 	$(Q) $(CC) -o $@ -S $(CFLAGS) -fverbose-asm $<
-- 
2.3.5

[RFC PATCH 2/2] Makefile: use xxd for converting guest/init

[Andre Przywara]

Currently we use ld to convert the static guest/init binary back
into an object file, which we can embed as a binary blob into our
lkvm binary. This works fine as long as compiler and linker use the
same ELF target format, which seems to be not true for most of the
MIPS toolchains.
Use a different approach instead, which converts the guest/init
binary into a C array, from which the compiler generates an .o
representation. As the compiler is now the same, this naturally links
together fine on all architectures.
We use the "xxd" tool for generating a C array representation out of
the binary file. If this turns out to be not widely installed (it
seems to be part of the vim package in most distributions), we could
think about switching to a scripted implementation using "od" or some
printf trickery.

Signed-off-by: Andre Przywara <andre.przywara@arm.com>
---
 Makefile        | 4 ++--
 builtin-run.c   | 8 ++++----
 builtin-setup.c | 8 ++++----
 3 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/Makefile b/Makefile
index b9480ff..7f2a0ea 100644
--- a/Makefile
+++ b/Makefile
@@ -365,8 +365,8 @@ $(PROGRAM_ALIAS): $(PROGRAM)
 	$(Q) ln -f $< $@
 
 $(GUEST_OBJS): $(GUEST_INIT)
-	$(E) "  LINK    " $@
-	$(Q) $(LD) $(LDFLAGS) -r -b binary -o $@ $<
+	$(E) "  CONVERT " $@
+	$(Q) xxd -i $< | $(CC) -c -x c - -o $@
 
 $(GUEST_INIT): guest/init.c
 	$(E) "  CC      " $@
diff --git a/builtin-run.c b/builtin-run.c
index 1ee75ad..0a48663 100644
--- a/builtin-run.c
+++ b/builtin-run.c
@@ -59,8 +59,8 @@ static int  kvm_run_wrapper;
 
 bool do_debug_print = false;
 
-extern char _binary_guest_init_start;
-extern char _binary_guest_init_size;
+extern char guest_init;
+extern char guest_init_len;
 
 static const char * const run_usage[] = {
 	"lkvm run [<options>] [<kernel image>]",
@@ -354,8 +354,8 @@ static int kvm_setup_guest_init(struct kvm *kvm)
 	char *data;
 
 	/* Setup /virt/init */
-	size = (size_t)&_binary_guest_init_size;
-	data = (char *)&_binary_guest_init_start;
+	size = (size_t)&guest_init_len;
+	data = (char *)&guest_init;
 	snprintf(tmp, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), rootfs);
 	remove(tmp);
 	fd = open(tmp, O_CREAT | O_WRONLY, 0755);
diff --git a/builtin-setup.c b/builtin-setup.c
index 8b45c56..fd7ca54 100644
--- a/builtin-setup.c
+++ b/builtin-setup.c
@@ -16,8 +16,8 @@
 #include <sys/mman.h>
 #include <fcntl.h>
 
-extern char _binary_guest_init_start;
-extern char _binary_guest_init_size;
+extern char guest_init;
+extern char guest_init_len;
 
 static const char *instance_name;
 
@@ -131,8 +131,8 @@ static int copy_init(const char *guestfs_name)
 	int fd, ret;
 	char *data;
 
-	size = (size_t)&_binary_guest_init_size;
-	data = (char *)&_binary_guest_init_start;
+	size = (size_t)&guest_init;
+	data = (char *)&guest_init_len;
 	snprintf(path, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), guestfs_name);
 	remove(path);
 	fd = open(path, O_CREAT | O_WRONLY, 0755);
-- 
2.3.5

Re: [RFC PATCH 2/2] Makefile: use xxd for converting guest/init

[Andreas Herrmann]

On Tue, Jun 23, 2015 at 04:06:27PM +0100, Andre Przywara wrote:
> Currently we use ld to convert the static guest/init binary back
> into an object file, which we can embed as a binary blob into our
> lkvm binary. This works fine as long as compiler and linker use the
> same ELF target format, which seems to be not true for most of the
> MIPS toolchains.
> Use a different approach instead, which converts the guest/init
> binary into a C array, from which the compiler generates an .o
> representation. As the compiler is now the same, this naturally links
> together fine on all architectures.
> We use the "xxd" tool for generating a C array representation out of
> the binary file. If this turns out to be not widely installed (it
> seems to be part of the vim package in most distributions), we could
> think about switching to a scripted implementation using "od" or some
> printf trickery.

Works for me. (tested with an octeon-sdk toolchain)

Thanks,
Andreas

> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
> ---
>  Makefile        | 4 ++--
>  builtin-run.c   | 8 ++++----
>  builtin-setup.c | 8 ++++----
>  3 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index b9480ff..7f2a0ea 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -365,8 +365,8 @@ $(PROGRAM_ALIAS): $(PROGRAM)
>  	$(Q) ln -f $< $@
>  
>  $(GUEST_OBJS): $(GUEST_INIT)
> -	$(E) "  LINK    " $@
> -	$(Q) $(LD) $(LDFLAGS) -r -b binary -o $@ $<
> +	$(E) "  CONVERT " $@
> +	$(Q) xxd -i $< | $(CC) -c -x c - -o $@
>  
>  $(GUEST_INIT): guest/init.c
>  	$(E) "  CC      " $@
> diff --git a/builtin-run.c b/builtin-run.c
> index 1ee75ad..0a48663 100644
> --- a/builtin-run.c
> +++ b/builtin-run.c
> @@ -59,8 +59,8 @@ static int  kvm_run_wrapper;
>  
>  bool do_debug_print = false;
>  
> -extern char _binary_guest_init_start;
> -extern char _binary_guest_init_size;
> +extern char guest_init;
> +extern char guest_init_len;
>  
>  static const char * const run_usage[] = {
>  	"lkvm run [<options>] [<kernel image>]",
> @@ -354,8 +354,8 @@ static int kvm_setup_guest_init(struct kvm *kvm)
>  	char *data;
>  
>  	/* Setup /virt/init */
> -	size = (size_t)&_binary_guest_init_size;
> -	data = (char *)&_binary_guest_init_start;
> +	size = (size_t)&guest_init_len;
> +	data = (char *)&guest_init;
>  	snprintf(tmp, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), rootfs);
>  	remove(tmp);
>  	fd = open(tmp, O_CREAT | O_WRONLY, 0755);
> diff --git a/builtin-setup.c b/builtin-setup.c
> index 8b45c56..fd7ca54 100644
> --- a/builtin-setup.c
> +++ b/builtin-setup.c
> @@ -16,8 +16,8 @@
>  #include <sys/mman.h>
>  #include <fcntl.h>
>  
> -extern char _binary_guest_init_start;
> -extern char _binary_guest_init_size;
> +extern char guest_init;
> +extern char guest_init_len;
>  
>  static const char *instance_name;
>  
> @@ -131,8 +131,8 @@ static int copy_init(const char *guestfs_name)
>  	int fd, ret;
>  	char *data;
>  
> -	size = (size_t)&_binary_guest_init_size;
> -	data = (char *)&_binary_guest_init_start;
> +	size = (size_t)&guest_init;
> +	data = (char *)&guest_init_len;
>  	snprintf(path, PATH_MAX, "%s%s/virt/init", kvm__get_dir(), guestfs_name);
>  	remove(path);
>  	fd = open(path, O_CREAT | O_WRONLY, 0755);
> -- 
> 2.3.5
>