[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v8.1 12/27] xsplice: Implement support for applying/reverting/replacing patches.



>>> On 21.04.16 at 22:27, <konrad@xxxxxxxxxx> wrote:
> On Thu, Apr 21, 2016 at 12:44:41AM -0600, Jan Beulich wrote:
>> >>> On 21.04.16 at 02:28, <konrad.wilk@xxxxxxxxxx> wrote:
>> >> >+    ASSERT(sec);
>> >> >+    if ( sec->sec->sh_size % sizeof(*payload->funcs) )
>> >> >+    {
>> >> >+        dprintk(XENLOG_ERR, XSPLICE "%s: Wrong size of 
>> >> >.xsplice.funcs!\n",
>> >> >+                elf->name);
>> >> >+        return -EINVAL;
>> >> >+    }
>> >> >+
>> >> >+    payload->funcs = sec->load_addr;
>> >> >+    payload->nfuncs = sec->sec->sh_size / sizeof(*payload->funcs);
>> >> 
>> >> Following to our discussion yesterday - can't we (ab)use the section merge
>> >> flag here to report the structure size, along the lines of what relocation
>> >> sections do for their elements?
>> > 
>> > 
>> > In other words - the xsplice-tools.git and the test-cases (in
>> > arch/x86/xen/test) would modify the sec->sh_entsize to have the
>> > sizeof(xsplice_patch_func) aka 64 (0x40) ?
>> > 
>> > [Note this could also extend to .xsplice.hooks.load and
>> > .xsplice.hooks.unload]
>> > 
>> > The xsplice-tools.git could surely do it, but the built-in test-cases -
>> > not so much. I would need to do some in-place binary handrolling - unless 
>> > you
>> > know of some tool or some __section modifiers?
>> 
>> Well, that's why I suggest (ab) using the SHF_MERGE flag, i.e. to
>> see whether we can make gas do what we want. Fiddling with the
>> binary I would not recommend, i.e. if we can't get gas to do what
>> we want we should rather stay with what there is.
> 
> This:
> 
> diff --git a/xen/arch/x86/test/xen_hello_world.c 
> b/xen/arch/x86/test/xen_hello_world.c
> index a3da85d..92f2e97 100644
> --- a/xen/arch/x86/test/xen_hello_world.c
> +++ b/xen/arch/x86/test/xen_hello_world.c
> @@ -47,6 +47,13 @@ XSPLICE_UNLOAD_HOOK(hi_func);
>  
>  XSPLICE_UNLOAD_HOOK(Z_check_fnc);
>  
> +#define XSPLICE_FUNC() \
> +    asm ( \
> +    ".pushsection .xsplice.funcs, \"awM\", @progbits, 64\n " \
> +    ".popsection")
> +
> +XSPLICE_FUNC();
> +
>  struct xsplice_patch_func __section(".xsplice.funcs") 
> xsplice_xen_hello_world = {
>      .version = XSPLICE_PAYLOAD_VERSION,
>      .name = hello_world_patch_this_fnc,
> 
> Makes the .xsplice.funcs have sh_entsize have the size of 64 bytes:
> 
> 
>   [ 6] .xsplice.funcs    PROGBITS         0000000000000000  000000a0
>        0000000000000040  0000000000000000  WA       0     0     32
> vs
> 
>   [ 6] .xsplice.funcs    PROGBITS         0000000000000000  000000a0
>        0000000000000040  0000000000000040 WAM       0     0     32
> 
> Unfortunatly the compilation throws this:
> 
> FC-64 <konrad@build-external:~/xtt-x86_64/xen/xen> make -j4 tests 1>1
> dnsdomainname: Name or service not known
> {standard input}: Assembler messages:
> {standard input}:184: Warning: ignoring changed section attributes for 
> .xsplice.funcs
> 
> 
> This is with:
> binutils-2.20.51.0.2-15.fc13.x86_64
> and with:
> binutils-2.25-9.fc22.x86_64
> 
> Ideas?

Well, this presumably being a redundant .section directive
somewhere it would require seeing the full .c together with the
intermediate .s in order to tell. But I guess to avoid you spending
more time on this secondary aspect, let's just drop this for now
and just put it on the todo list (as long as the thing is tech preview
we can still alter requirements we place on the actual payloads).

Jan


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.