Re: [Xen-devel] [PATCH v1 6/9] livepatch: Initial ARM64 support.

[Julien Grall <julien.grall@xxxxxxx>]

On 15/08/2016 17:17, Konrad Rzeszutek Wilk wrote:
> On Mon, Aug 15, 2016 at 04:57:26PM +0200, Julien Grall wrote:
> > Hi Jan and Konrad,
> >
> > On 15/08/2016 16:23, Jan Beulich wrote:
> > > > > > On 15.08.16 at 16:09, <konrad.wilk@xxxxxxxxxx> wrote:
> > > > On Mon, Aug 15, 2016 at 02:21:48AM -0600, Jan Beulich wrote:
> > > > > > > > On 15.08.16 at 01:07, <konrad.wilk@xxxxxxxxxx> wrote:
> > > > > > @@ -711,9 +711,15 @@ static int prepare_payload(struct payload *payload,
> > > > > >                  return -EINVAL;
> > > > > >              }
> > > > > >          }
> > > > > > +#ifndef CONFIG_ARM
> > > > > >          apply_alternatives_nocheck(start, end);
> > > > > > +#else
> > > > > > +        apply_alternatives(start, sec->sec->sh_size);
> > > > > > +#endif
> > > > > Conditionals like this are ugly - can't this be properly abstracted?
> > > > Yes, I can introduce an apply_alternatives_nocheck on ARM that will
> > > > hava the same set of arguments on x86.
> > > >
> > > > Or I can make a new function name?
> > > Either way is fine with me, with a slight preference to the former
> > > one.
> > I am fine with the prototype of the function apply_alternatives_nocheck but
> > I don't think the name is relevant for ARM.
> >
> > Is there any reason we don't want to call directly apply_alternatives in
> > x86?
> It assumes (and has an ASSERT) that it is called with interrupts disabled.
> And we don't need to do that (as during livepatch loading we can modify the
> livepatch payload without worrying about interrupts).
Oh, it makes more sense now.

> P.S.
> loading != applying.
>
> I could do a patch where we rename 'apply_alternatives' -> 
> 'apply_alternatives_boot'
> and 'apply_alternatives_nocheck' to 'apply_alternatives'.
>
> Also the x86 version instead of having:
>
> apply_alternatives(struct alt_instr *start, struct alt_instr *end)
>
> would end up with:
>
> apply_alternatives(void *start, size_t length)
I would be fine with the prototype
apply_alternatives(struct alt_instr *start, struct alt_instr *end)

for ARM. It is up to you.

Cheers,

--
Julien Grall

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