Re: [PATCH v2 10/13] xen_hello_world.xsplice: Test payload for patching 'xen_extra_version'.

[Ross Lagerwall <ross.lagerwall@citrix.com>]

On 01/14/2016 09:47 PM, Konrad Rzeszutek Wilk wrote:
> This change demonstrates how to generate an xSplice ELF payload.
>
> The idea here is that we want to patch in the hypervisor
> the 'xen_version_extra' function with an function that will
> return 'Hello World'. The 'xl info | grep extraversion'
> will reflect the new value after the patching.
>
snip
> +### Example
> +
> +A simple example of what a payload file can be:
> +
> +<pre>
> +/* MUST be in sync with hypervisor. */
> +struct xsplice_patch_func {
> +    const char *name;
> +    unsigned long new_addr;
> +    const unsigned long old_addr;
> +    uint32_t new_size;
> +    const uint32_t old_size;
> +    uint8_t pad[32];
> +};
> +
> +/* Our replacement function for xen_extra_version. */
> +const char *xen_hello_world(void)
> +{
> +    return "Hello World";
> +}
> +
> +struct xsplice_patch_func xsplice_hello_world = {
> +    .name = "xen_extra_version",
> +    .new_addr = &xen_hello_world,
> +    .old_addr = 0xffff82d08013963c, /* Extracted from xen-syms. */
> +    .new_size = 13, /* To be be computed by scripts. */
> +    .old_size = 13, /* -----------""---------------  */
> +};
> +</pre>
> +
> +With the linker script as follow to change the `xsplice_hello_world`
> +do be `.xsplice.funcs` :
> +
> +<pre>
> +OUTPUT_FORMAT("elf64-x86-64", "elf64-x86-64", "elf64-x86-64")
> +OUTPUT_ARCH(i386:x86-64)
> +ENTRY(xsplice_hello_world)
> +SECTIONS
> +{
> +    /* The hypervisor expects ".xsplice.func", so change
> +     * the ".data.xsplice_hello_world" to it. */
> +
> +    .xsplice.funcs : { *(*.xsplice_hello_world) }
> +    }
> +}
> +</pre>

You should be able to use __attribute__((__section__(".xsplice.funcs"))) 
on the structure to avoid needing to use a linker script.

> +
> +Code must be compiled with -fPIC.
> +
>   ## Hypercalls
>
>   We will employ the sub operations of the system management hypercall (sysctl).
> diff --git a/tools/misc/Makefile b/tools/misc/Makefile
> index c46873e..8385830 100644
> --- a/tools/misc/Makefile
> +++ b/tools/misc/Makefile
> @@ -36,6 +36,10 @@ INSTALL_SBIN += $(INSTALL_SBIN-y)
>   # Everything to be installed in a private bin/
>   INSTALL_PRIVBIN                += xenpvnetboot
>
> +# We need the hypervisor - and only 64-bit builds have it.
> +ifeq ($(XEN_COMPILE_ARCH),x86_64)
> +INSTALL_PRIVBIN                += xen_hello_world.xsplice
> +endif
>   # Everything to be installed
>   TARGETS_ALL := $(INSTALL_BIN) $(INSTALL_SBIN) $(INSTALL_PRIVBIN)
>
> @@ -49,7 +53,7 @@ TARGETS_COPY += xenpvnetboot
>   # Everything which needs to be built
>   TARGETS_BUILD := $(filter-out $(TARGETS_COPY),$(TARGETS_ALL))
>
> -.PHONY: all build
> +.PHONY: all build xsplice
>   all build: $(TARGETS_BUILD)
>
>   .PHONY: install
> @@ -111,4 +115,23 @@ gtraceview: gtraceview.o
>   xencov: xencov.o
>   	$(CC) $(LDFLAGS) -o $@ $< $(LDLIBS_libxenctrl) $(APPEND_LDFLAGS)
>
> +.PHONY: xsplice
> +xsplice:
> +ifeq ($(XEN_COMPILE_ARCH),x86_64)
> +	# We MUST regenerate the file everytime we build - in case the hypervisor
> +	# is rebuilt too.
> +	$(RM) *.xplice
> +	$(MAKE) xen_hello_world.xsplice

Can't you depend on xen-syms to avoid recompiling this every time.

> +endif
> +
> +XEN_EXTRA_VERSION_ADDR=$(shell nm --defined $(XEN_ROOT)/xen/xen-syms | grep xen_extra_version | awk '{print "0x"$$1}')
> +
> +xen_hello_world.xsplice: xen_hello_world.c
> +	$(CC) -DOLD_CODE=$(XEN_EXTRA_VERSION_ADDR) -I$(XEN_ROOT)/tools/include \
> +		-fPIC -Wl,--emit-relocs \
> +		-Wl,-r -Wl,--entry=xsplice_hello_world \
> +		-fdata-sections -ffunction-sections \
> +		-nostdlib -Txsplice.lds \
> +		-o $@ $<
> +	@objdump -x --section=.xsplice.funcs $@

If you use __attribute__((__section__(".xsplice.funcs"))) on the struct, 
you can drop the custom linker script and simplify the command-line to 
something like:
$(CC) -DOLD_CODE=$(XEN_EXTRA_VERSION_ADDR) -I$(XEN_ROOT)/tools/include \
	-c -o $@ $< $(CFLAGS)

Having mostly the same CFLAGS that Xen uses is important because it 
contains things like -mno-red-zone, -fno-asynchronous-unwind-tables, and 
-mno-sse, etc which affect the way the code is compiled.

-- 
Ross Lagerwall