Re: [RFC v2] xSplice design

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

On Fri, Jun 12, 2015 at 08:36:29PM +0200, Martin Pohlack wrote:
> On 12.06.2015 18:39, Konrad Rzeszutek Wilk wrote:
> > On Fri, Jun 12, 2015 at 05:17:13PM +0100, Andrew Cooper wrote:
> >> On 12/06/15 17:09, Konrad Rzeszutek Wilk wrote:
> >>>
> >>>>> The _GET_STATUS does not enforce this and can take longer giving us
> >>>>> more breathing room - and also unbounded time - which means if
> >>>>> we were to try to cancel it (say it had run for an hour and still
> >>>>> could not patch it)- we have to add some hairy code to
> >>>>> deal with cancelling asynchronous code.
> >>>>>
> >>>>> Your way is simpler - but I would advocate expanding the -EAGAIN to _all_
> >>>>> the xSplice hypercalls. Thoughts?
> >>>> In my experience, you only need the EAGAIN for hypercalls that use the
> >>>> quiet state.  Depending on the design, that would be the operations that
> >>>> do hotpatch activation and deactivation (i.e., the actual splicing).
> >>> The uploading of the patch could be slow - as in the checking to be done
> >>> and on an big patch (2MB or more?) it would be good to try again.
> >>
> >> If a patch is greater than a few kb, it is probably not something
> >> sensible to be patching.
> > 
> > Potentially. It could be an cumlative update containing mulitple XSAs.
> > 
> >>
> >> However, an upload_patch/apply_patch split in the hypercall ABI might be
> >> a sensible idea.
> > 
> > The design has that (it has four hypercalls actually).
> > 
> > The question is whether that upload_patch hypercall should also
> > have the EAGAIN mechansim baked in the design.
> 
> Why would you need it?  Do you envision any complex blocking operation
> to happen when loading a module?  I can't think of any off the top of my
> head.

We have to ELF load the payload, do a signature verification. If that
is somehow tied in SecureBoot perhaps we need to use the shim to verify
the payload. Hopefully that won't take forever, but it is unbounded
and we have no clue how long it could take.

> 
> > The other one (GET_LIST) has it too - and can return EAGAIN with an
> > count of how many there are left so the user-space can pick up.
> 
> I don't understand how that relates to the async nature of the other
> interface parts.  Is that similar to the readdir syscall where a second
> invocation would continue from the current seek pointer?

That, but without guarantees. That is if during this operation another
patch is loaded - we would not see it. Unless we did another GET_LIST
hypercall.
> 
> Or do you have something like a pread for a subarray of list entries in
> mind?
> 
> It might be a bit tricky to reliably deliver atomic snapshots if a
> potentially larger list to userland.  Maybe a version field might be
> desirable here.

Sure, a version field would work nicely.
> 
> Martin
>