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

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





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

 


Rackspace

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