Re: [PATCH v1 01/11] xsplice: Design document (v2).

[Konrad Rzeszutek Wilk <konrad.wilk@oracle.com>]

On Thu, Nov 05, 2015 at 10:49:51AM +0000, Ross Lagerwall wrote:
> On 11/04/2015 09:10 PM, Konrad Rzeszutek Wilk wrote:
> snip
> >>+The payload **MUST** contain enough data to allow us to apply the update
> >>+and also safely reverse it. As such we **MUST** know:
> >>+
> >>+ * The locations in memory to be patched. This can be determined dynamically
> >>+   via symbols or via virtual addresses.
> >>+ * The new code that will be patched in.
> >>+ * Signature to verify the payload.
> >
> >Argh. We need to move the 'Signature to verify' in the 'v2' section
> >as I don't think we can get that done in time.
> 
> No, not for V1.
> 
> >
> >>+
> >>+This binary format can be constructed using an custom binary format but
> >>+there are severe disadvantages of it:
> >>+
> >>+ * The format might need to be changed and we need an mechanism to accommodate
> >>+   that.
> >>+ * It has to be platform agnostic.
> >>+ * Easily constructed using existing tools.
> >>+
> >>+As such having the payload in an ELF file is the sensible way. We would be
> >>+carrying the various sets of structures (and data) in the ELF sections under
> >>+different names and with definitions. The prefix for the ELF section name
> >>+would always be: *.xsplice* to match up to the names of the structures.
> >>+
> >>+Note that every structure has padding. This is added so that the hypervisor
> >>+can re-use those fields as it sees fit.
> >>+
> >>+Earlier design attempted to ineptly explain the relations of the ELF sections
> >>+to each other without using proper ELF mechanism (sh_info, sh_link, data
> >>+structures using Elf types, etc). This design will explain in detail
> >>+the structures and how they are used together and not dig in the ELF
> >>+format - except mention that the section names should match the
> >>+structure names.
> >>+
> >>+The xSplice payload is a relocatable ELF binary. A typical binary would have:
> >>+
> >>+ * One or more .text sections
> >>+ * Zero or more read-only data sections
> >>+ * Zero or more data sections
> >>+ * Relocations for each of these sections
> >>+
> >>+It may also have some architecture-specific sections. For example:
> >>+
> >>+ * Alternatives instructions
> >>+ * Bug frames
> >>+ * Exception tables
> >>+ * Relocations for each of these sections
> >>+
> >>+The xSplice core code loads the payload as a standard ELF binary, relocates it
> >>+and handles the architecture-specifc sections as needed. This process is much
> >>+like what the Linux kernel module loader does. It contains no xSplice-specific
> >>+details and thus will not be discussed further.
> >
> >What is 'it'? The 'process of what module loader does'?
> 
> 'It' refers to the process of module loading in the previous sentence.
> 
> >
> >>+
> >>+Importantly, the payload also contains a section with an array of structures
> >>+describing the functions to be patched:
> >>+<pre>
> >>+struct xsplice_patch_func {
> >>+    unsigned long new_addr;
> >>+    unsigned long new_size;
> >>+    unsigned long old_addr;
> >>+    unsigned long old_size;
> >>+    char *name;
> >>+    uint8_t pad[64];
> >>+};
> >>+<pre>
> >
> >Uh, so 104 bytes ? Or did you mean to s/64/24/ so the structure is nicely
> >padded to 64-bytes?
> >
> >I think that is what you meant.
> 
> OK. I'm not too fussed about exact sizes for V1 anyway, it's likely to
> change at some point.

Right. How do we want to handle that?

The original structure I wrote up was mostly copied from ksplice kmodsrc with
modifications. Which meant it was capable of patching data, code, your
grandmothers sofa, you name it. But for v1 we want to have the basic -
patching code.

However going forward we will want to expand and make changes - and some
companies may want to add fields and structures (and naturally also
add code to the hypervisor for that) and so on.

And at some point we will make mistakes and realize that some structures
are to be deprecated.

To make the live easier for them we ought to provide some way for this.

Thoughts?


> 
> -- 
> Ross Lagerwall