Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-06-06 Thread Julien Grall 

Hi Konrad,

On 06/06/16 15:17, Konrad Rzeszutek Wilk wrote:

On Thu, Jun 02, 2016 at 04:14:04PM +0100, Julien Grall wrote:



On 02/06/16 16:04, Julien Grall wrote:

Hi Konrad,

On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:

On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:

On 31/05/16 10:21, Stefano Stabellini wrote:

On Mon, 30 May 2016, Julien Grall wrote:

By spinning in __apply_alternatives_multi_stop we protect us against
modification in the common code and tricky bug (yes it might be
difficult to
hit and debug).


I feel that you are moving down the stack to try to make the
impact of doing modifications have no impact (or as low as possible).

And it would work now, but I am concerned that in the future it may be
not soo future proof.


Can you detail here?


Would it perhaps make sense to make some of the livepatching mechanism
be exposed as general code? And use it for alternative asm as well?


The code to sync the CPU looks very similar to stop_machine, so why
would we want to get yet another mechanism to sync the CPUs and execute
a specific function?


I forgot to mention that apply_alternatives_all is only called one time
during the boot and before CPU0 goes in idle_loop. If we have to patch
afterward, we would use apply_alternatives which consider that all the CPUs
have been parked somewhere else.


Oh, in that case please just mention that in the commit description.


Will do in the next version.

Cheers,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-06-06 Thread Konrad Rzeszutek Wilk 
On Thu, Jun 02, 2016 at 04:14:04PM +0100, Julien Grall wrote:
> 
> 
> On 02/06/16 16:04, Julien Grall wrote:
> >Hi Konrad,
> >
> >On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:
> >>On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
> >>>On 31/05/16 10:21, Stefano Stabellini wrote:
> On Mon, 30 May 2016, Julien Grall wrote:
> >>>By spinning in __apply_alternatives_multi_stop we protect us against
> >>>modification in the common code and tricky bug (yes it might be
> >>>difficult to
> >>>hit and debug).
> >>
> >>I feel that you are moving down the stack to try to make the
> >>impact of doing modifications have no impact (or as low as possible).
> >>
> >>And it would work now, but I am concerned that in the future it may be
> >>not soo future proof.
> >
> >Can you detail here?
> >
> >>Would it perhaps make sense to make some of the livepatching mechanism
> >>be exposed as general code? And use it for alternative asm as well?
> >
> >The code to sync the CPU looks very similar to stop_machine, so why
> >would we want to get yet another mechanism to sync the CPUs and execute
> >a specific function?
> 
> I forgot to mention that apply_alternatives_all is only called one time
> during the boot and before CPU0 goes in idle_loop. If we have to patch
> afterward, we would use apply_alternatives which consider that all the CPUs
> have been parked somewhere else.

Oh, in that case please just mention that in the commit description.

> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-06-02 Thread Julien Grall 



On 02/06/16 16:04, Julien Grall wrote:

Hi Konrad,

On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:

On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:

On 31/05/16 10:21, Stefano Stabellini wrote:

On Mon, 30 May 2016, Julien Grall wrote:

By spinning in __apply_alternatives_multi_stop we protect us against
modification in the common code and tricky bug (yes it might be
difficult to
hit and debug).


I feel that you are moving down the stack to try to make the
impact of doing modifications have no impact (or as low as possible).

And it would work now, but I am concerned that in the future it may be
not soo future proof.


Can you detail here?


Would it perhaps make sense to make some of the livepatching mechanism
be exposed as general code? And use it for alternative asm as well?


The code to sync the CPU looks very similar to stop_machine, so why
would we want to get yet another mechanism to sync the CPUs and execute
a specific function?


I forgot to mention that apply_alternatives_all is only called one time 
during the boot and before CPU0 goes in idle_loop. If we have to patch 
afterward, we would use apply_alternatives which consider that all the 
CPUs have been parked somewhere else.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-06-02 Thread Julien Grall 

Hi Konrad,

On 02/06/16 15:46, Konrad Rzeszutek Wilk wrote:

On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:

On 31/05/16 10:21, Stefano Stabellini wrote:

On Mon, 30 May 2016, Julien Grall wrote:

By spinning in __apply_alternatives_multi_stop we protect us against
modification in the common code and tricky bug (yes it might be difficult to
hit and debug).


I feel that you are moving down the stack to try to make the
impact of doing modifications have no impact (or as low as possible).

And it would work now, but I am concerned that in the future it may be
not soo future proof.


Can you detail here?


Would it perhaps make sense to make some of the livepatching mechanism
be exposed as general code? And use it for alternative asm as well?


The code to sync the CPU looks very similar to stop_machine, so why 
would we want to get yet another mechanism to sync the CPUs and execute 
a specific function?


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-06-02 Thread Konrad Rzeszutek Wilk 
On Tue, May 31, 2016 at 11:24:10AM +0100, Julien Grall wrote:
> Hi Stefano,
> 
> On 31/05/16 10:21, Stefano Stabellini wrote:
> >On Mon, 30 May 2016, Julien Grall wrote:
> >>Hi Stefano,
> >>
> >>On 30/05/2016 15:45, Stefano Stabellini wrote:
> >>>On Mon, 23 May 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 21/05/16 16:09, Stefano Stabellini wrote:
> >On Thu, 5 May 2016, Julien Grall wrote:
> >>+void __init apply_alternatives_all(void)
> >>+{
> >>+int ret;
> >>+
> >>+   /* better not try code patching on a live SMP system */
> >>+ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
> >>NR_CPUS);
> >
> >Why not just call stop_machine_run, passing 0 as the cpu to run
> >__apply_alternatives_multi_stop on? Given that you already have
> >secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> >missing?
> 
> Someone as to call __apply_alternatives_multi_stop on secondary CPUs.
> >>>
> >>>Why? Secondary cpus would be just left spinning at the beginning of the
> >>>function. You might as well leave them spinning in
> >>>stopmachine_wait_state.
> >>>
> >>
> >>Because, we may need to patch the stop_machine state machine (spinlock,...).
> >>So the safest place whilst the code is patched is
> >>__apply_alternatives_multi_stop.
> >>
> >>Note that there is a comment on top of __apply_alternatives_multi_stop to
> >>explain the reason.
> >
> >Have you tried patching stop_machine? What if you need to patch
> >__apply_alternatives_multi_stop? :-)
> 
> We have to define a safe place where the CPUs could spin. I.e the
> instructions will not be patched.
> Whilst stop_machine may not be patched today, it is common code and we don't
> know how this will evolve in the future.

Right, which is why livepatching persued a different path as it may
be patched. And instead choose a simpler mechanism to trigger IPIs and
either patched it from idle_loop() or from the VMX assembler code.


> 
> By spinning in __apply_alternatives_multi_stop we protect us against
> modification in the common code and tricky bug (yes it might be difficult to
> hit and debug).

I feel that you are moving down the stack to try to make the
impact of doing modifications have no impact (or as low as possible).

And it would work now, but I am concerned that in the future it may be
not soo future proof.

Would it perhaps make sense to make some of the livepatching mechanism
be exposed as general code? And use it for alternative asm as well?


> 
> Regards,
> 
> -- 
> Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-31 Thread Julien Grall 

Hi Stefano,

On 31/05/16 10:21, Stefano Stabellini wrote:

On Mon, 30 May 2016, Julien Grall wrote:

Hi Stefano,

On 30/05/2016 15:45, Stefano Stabellini wrote:

On Mon, 23 May 2016, Julien Grall wrote:

Hi Stefano,

On 21/05/16 16:09, Stefano Stabellini wrote:

On Thu, 5 May 2016, Julien Grall wrote:

+void __init apply_alternatives_all(void)
+{
+int ret;
+
+   /* better not try code patching on a live SMP system */
+ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
NR_CPUS);


Why not just call stop_machine_run, passing 0 as the cpu to run
__apply_alternatives_multi_stop on? Given that you already have
secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
missing?


Someone as to call __apply_alternatives_multi_stop on secondary CPUs.


Why? Secondary cpus would be just left spinning at the beginning of the
function. You might as well leave them spinning in
stopmachine_wait_state.



Because, we may need to patch the stop_machine state machine (spinlock,...).
So the safest place whilst the code is patched is
__apply_alternatives_multi_stop.

Note that there is a comment on top of __apply_alternatives_multi_stop to
explain the reason.


Have you tried patching stop_machine? What if you need to patch
__apply_alternatives_multi_stop? :-)


We have to define a safe place where the CPUs could spin. I.e the 
instructions will not be patched.
Whilst stop_machine may not be patched today, it is common code and we 
don't know how this will evolve in the future.


By spinning in __apply_alternatives_multi_stop we protect us against 
modification in the common code and tricky bug (yes it might be 
difficult to hit and debug).


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-31 Thread Stefano Stabellini 
On Mon, 30 May 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 30/05/2016 15:45, Stefano Stabellini wrote:
> > On Mon, 23 May 2016, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 21/05/16 16:09, Stefano Stabellini wrote:
> > > > On Thu, 5 May 2016, Julien Grall wrote:
> > > > > +void __init apply_alternatives_all(void)
> > > > > +{
> > > > > +int ret;
> > > > > +
> > > > > + /* better not try code patching on a live SMP system */
> > > > > +ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
> > > > > NR_CPUS);
> > > > 
> > > > Why not just call stop_machine_run, passing 0 as the cpu to run
> > > > __apply_alternatives_multi_stop on? Given that you already have
> > > > secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> > > > missing?
> > > 
> > > Someone as to call __apply_alternatives_multi_stop on secondary CPUs.
> > 
> > Why? Secondary cpus would be just left spinning at the beginning of the
> > function. You might as well leave them spinning in
> > stopmachine_wait_state.
> > 
> 
> Because, we may need to patch the stop_machine state machine (spinlock,...).
> So the safest place whilst the code is patched is
> __apply_alternatives_multi_stop.
> 
> Note that there is a comment on top of __apply_alternatives_multi_stop to
> explain the reason.

Have you tried patching stop_machine? What if you need to patch
__apply_alternatives_multi_stop? :-)

I'll refer to Konrad on whether this is a good approach or not.

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-30 Thread Julien Grall 

Hi Stefano,

On 30/05/2016 15:45, Stefano Stabellini wrote:

On Mon, 23 May 2016, Julien Grall wrote:

Hi Stefano,

On 21/05/16 16:09, Stefano Stabellini wrote:

On Thu, 5 May 2016, Julien Grall wrote:

+void __init apply_alternatives_all(void)
+{
+int ret;
+
+   /* better not try code patching on a live SMP system */
+ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
NR_CPUS);


Why not just call stop_machine_run, passing 0 as the cpu to run
__apply_alternatives_multi_stop on? Given that you already have
secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
missing?


Someone as to call __apply_alternatives_multi_stop on secondary CPUs.


Why? Secondary cpus would be just left spinning at the beginning of the
function. You might as well leave them spinning in
stopmachine_wait_state.



Because, we may need to patch the stop_machine state machine 
(spinlock,...). So the safest place whilst the code is patched is 
__apply_alternatives_multi_stop.


Note that there is a comment on top of __apply_alternatives_multi_stop 
to explain the reason.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-30 Thread Stefano Stabellini 
On Mon, 23 May 2016, Julien Grall wrote:
> Hi Stefano,
> 
> On 21/05/16 16:09, Stefano Stabellini wrote:
> > On Thu, 5 May 2016, Julien Grall wrote:
> > > +void __init apply_alternatives_all(void)
> > > +{
> > > +int ret;
> > > +
> > > + /* better not try code patching on a live SMP system */
> > > +ret = stop_machine_run(__apply_alternatives_multi_stop, NULL,
> > > NR_CPUS);
> > 
> > Why not just call stop_machine_run, passing 0 as the cpu to run
> > __apply_alternatives_multi_stop on? Given that you already have
> > secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
> > missing?
> 
> Someone as to call __apply_alternatives_multi_stop on secondary CPUs.

Why? Secondary cpus would be just left spinning at the beginning of the
function. You might as well leave them spinning in
stopmachine_wait_state.

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-23 Thread Julien Grall 

Hi Stefano,

On 21/05/16 16:09, Stefano Stabellini wrote:

On Thu, 5 May 2016, Julien Grall wrote:

+void __init apply_alternatives_all(void)
+{
+int ret;
+
+   /* better not try code patching on a live SMP system */
+ret = stop_machine_run(__apply_alternatives_multi_stop, NULL, NR_CPUS);


Why not just call stop_machine_run, passing 0 as the cpu to run
__apply_alternatives_multi_stop on? Given that you already have
secondary cpus spinning in __apply_alternatives_multi_stop.  What am I
missing?


Someone as to call __apply_alternatives_multi_stop on secondary CPUs. 
apply_alternatives is called by the boot CPU, the rest are spinning 
within the idle_loop.


The function stop_machine_run will stop all the CPUs, and call 
__apply_alternatives_multi_stop on each of them.


Regards,

--
Julien Grall

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

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-21 Thread Stefano Stabellini 
On Thu, 5 May 2016, Julien Grall wrote:
> Some of the processor erratum will require to modify code sequence.
> As those modifications may impact the performance, they should only
> be enabled on affected cores. Furthermore, Xen may also want to take
> advantage of new hardware features coming up with v8.1 and v8.2.
> 
> This patch adds an infrastructure to patch Xen during boot time
> depending on the "features" available on the platform.
> 
> This code is based on the file arch/arm64/kernel/alternative.c in
> Linux 4.6-rc3. Any references to arm64 have been dropped to make the
> code as generic as possible.
> 
> Furthermore, in Xen the executable sections (.text and .init.text)
> are always mapped read-only and enforced by SCTLR.WNX. So it is not
> possible to directly patch Xen.
> 
> To by-pass this restriction, a temporary writeable mapping is made with
> vmap. This mapping will be used to write the new instructions.
> 
> Lastly, runtime patching is currently not necessary for ARM32. So the
> code is only enabled for ARM64.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/Kconfig  |   5 +
>  xen/arch/arm/Makefile |   1 +
>  xen/arch/arm/alternative.c| 217 
> ++
>  xen/arch/arm/setup.c  |   8 ++
>  xen/arch/arm/xen.lds.S|   7 ++
>  xen/include/asm-arm/alternative.h | 165 +
>  xen/include/asm-arm/arm64/insn.h  |  16 +++
>  7 files changed, 419 insertions(+)
>  create mode 100644 xen/arch/arm/alternative.c
>  create mode 100644 xen/include/asm-arm/alternative.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 6231cd5..9b3e66b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -14,6 +14,7 @@ config ARM_64
>   def_bool y
>   depends on 64BIT
>   select HAS_GICV3
> +select ALTERNATIVE
>  
>  config ARM
>   def_bool y
> @@ -46,6 +47,10 @@ config ACPI
>  config HAS_GICV3
>   bool
>  
> +# Select ALTERNATIVE if the architecture supports runtime patching
> +config ALTERNATIVE
> +bool
> +
>  endmenu
>  
>  source "common/Kconfig"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 9122f78..2d330aa 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -4,6 +4,7 @@ subdir-y += platforms
>  subdir-$(CONFIG_ARM_64) += efi
>  subdir-$(CONFIG_ACPI) += acpi
>  
> +obj-$(CONFIG_ALTERNATIVE) += alternative.o
>  obj-y += bootfdt.o
>  obj-y += cpu.o
>  obj-y += cpufeature.o
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> new file mode 100644
> index 000..0a5d1c8
> --- /dev/null
> +++ b/xen/arch/arm/alternative.c
> @@ -0,0 +1,217 @@
> +/*
> + * alternative runtime patching
> + * inspired by the x86 version
> + *
> + * Copyright (C) 2014 ARM Ltd.
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define __ALT_PTR(a,f)  (u32 *)((void *)&(a)->f + (a)->f)
> +#define ALT_ORIG_PTR(a) __ALT_PTR(a, orig_offset)
> +#define ALT_REPL_PTR(a) __ALT_PTR(a, alt_offset)
> +
> +extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
> +
> +struct alt_region {
> +const struct alt_instr *begin;
> +const struct alt_instr *end;
> +};
> +
> +/*
> + * Check if the target PC is within an alternative block.
> + */
> +static bool_t branch_insn_requires_update(const struct alt_instr *alt,
> +  unsigned long pc)
> +{
> +unsigned long replptr;
> +
> +if ( is_active_kernel_text(pc) )
> +return 1;
> +
> +replptr = (unsigned long)ALT_REPL_PTR(alt);
> +if ( pc >= replptr && pc <= (replptr + alt->alt_len) )
> +return 0;
> +
> +/*
> + * Branching into *another* alternate sequence is doomed, and
> + * we're not even trying to fix it up.
> + */
> +BUG();
> +}
> +
> +static u32 get_alt_insn(const struct alt_instr *alt,
> +const u32 *insnptr, const u32 *altinsnptr)
> +{
> +u32 insn;
> +
> +insn = le32_to_cpu(*altinsnptr);
> +
> +if ( insn_is_branch_imm(insn) )
> +{
> +s32 offset = insn_get_branch_offset(insn);
> +unsigned long target;
> +
> +

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-14 Thread Julien Grall 

Hi Konrad,

On 13/05/2016 21:26, Konrad Rzeszutek Wilk wrote:

On Thu, May 05, 2016 at 05:34:19PM +0100, Julien Grall wrote:

Some of the processor erratum will require to modify code sequence.
As those modifications may impact the performance, they should only
be enabled on affected cores. Furthermore, Xen may also want to take
advantage of new hardware features coming up with v8.1 and v8.2.

This patch adds an infrastructure to patch Xen during boot time
depending on the "features" available on the platform.

This code is based on the file arch/arm64/kernel/alternative.c in
Linux 4.6-rc3. Any references to arm64 have been dropped to make the
code as generic as possible.

Furthermore, in Xen the executable sections (.text and .init.text)
are always mapped read-only and enforced by SCTLR.WNX. So it is not
possible to directly patch Xen.


Is it not possible to temporarily 'unenforce' the SCTLR.WNX?


The problem is not because of SCTLR.WNX but because the entries are 
change from read-write to read-only afterwards.


The ARM ARM (see D4-1732 in ARM DDI 0487A.i) recommends the use of 
break-before-make if either the old or new entry is writable when the 
page table is shared between multiples processor. This is to avoid 
possible TLB conflicts on platform have aggressive prefetcher.


The break-before-make approach requires the following steps:
   - Replacing the old entry with an invalid entry
   - Invalidate the entry by flush the TLBs
   - Write the new entry

However, with the current approach, secondary CPUs runs at the same, so 
even with the break-before-make approach it might be possible to hit a 
data abort if the CPU execute code on an entry that is currently been 
changed.


It would be possible to make the CPUs spinning outside of the 
executable, but I think this make the code more complicate. Note that 
you would have the same issue with XSplice.


You may wonder why Xen is patched when all the CPUs are online. This is 
because in big.LITTLE environment, big and little cores will have 
different errata. The only way to know the list of errata is to probe 
each CPU, and therefore bring them up.


I hope this gives you more insights on why I choose this approach. I can 
update the commit message with it.






To by-pass this restriction, a temporary writeable mapping is made with
vmap. This mapping will be used to write the new instructions.

Lastly, runtime patching is currently not necessary for ARM32. So the
code is only enabled for ARM64.

Signed-off-by: Julien Grall 
---
  xen/arch/arm/Kconfig  |   5 +
  xen/arch/arm/Makefile |   1 +
  xen/arch/arm/alternative.c| 217 ++
  xen/arch/arm/setup.c  |   8 ++
  xen/arch/arm/xen.lds.S|   7 ++
  xen/include/asm-arm/alternative.h | 165 +
  xen/include/asm-arm/arm64/insn.h  |  16 +++
  7 files changed, 419 insertions(+)
  create mode 100644 xen/arch/arm/alternative.c
  create mode 100644 xen/include/asm-arm/alternative.h

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 6231cd5..9b3e66b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -14,6 +14,7 @@ config ARM_64
def_bool y
depends on 64BIT
select HAS_GICV3
+select ALTERNATIVE


Hard tabs, not spaces.


Right, I will change in the next version.



  config ARM
def_bool y
@@ -46,6 +47,10 @@ config ACPI
  config HAS_GICV3
bool

+# Select ALTERNATIVE if the architecture supports runtime patching
+config ALTERNATIVE
+bool


Ditto.


Ok.


+
  endmenu

  source "common/Kconfig"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 9122f78..2d330aa 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -4,6 +4,7 @@ subdir-y += platforms
  subdir-$(CONFIG_ARM_64) += efi
  subdir-$(CONFIG_ACPI) += acpi

+obj-$(CONFIG_ALTERNATIVE) += alternative.o


This probably needs to be alternative.init.o ?

  obj-y += bootfdt.o
  obj-y += cpu.o
  obj-y += cpufeature.o
diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
new file mode 100644
index 000..0a5d1c8
--- /dev/null
+++ b/xen/arch/arm/alternative.c
@@ -0,0 +1,217 @@
+/*
+ * alternative runtime patching
+ * inspired by the x86 version
+ *
+ * Copyright (C) 2014 ARM Ltd.


Not 2016?


The code has been taken from Linux. I was not sure what to do here. I 
will update with 2014-2016.





+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ *

Re: [Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-13 Thread Konrad Rzeszutek Wilk 
On Thu, May 05, 2016 at 05:34:19PM +0100, Julien Grall wrote:
> Some of the processor erratum will require to modify code sequence.
> As those modifications may impact the performance, they should only
> be enabled on affected cores. Furthermore, Xen may also want to take
> advantage of new hardware features coming up with v8.1 and v8.2.
> 
> This patch adds an infrastructure to patch Xen during boot time
> depending on the "features" available on the platform.
> 
> This code is based on the file arch/arm64/kernel/alternative.c in
> Linux 4.6-rc3. Any references to arm64 have been dropped to make the
> code as generic as possible.
> 
> Furthermore, in Xen the executable sections (.text and .init.text)
> are always mapped read-only and enforced by SCTLR.WNX. So it is not
> possible to directly patch Xen.

Is it not possible to temporarily 'unenforce' the SCTLR.WNX?

> 
> To by-pass this restriction, a temporary writeable mapping is made with
> vmap. This mapping will be used to write the new instructions.
> 
> Lastly, runtime patching is currently not necessary for ARM32. So the
> code is only enabled for ARM64.
> 
> Signed-off-by: Julien Grall 
> ---
>  xen/arch/arm/Kconfig  |   5 +
>  xen/arch/arm/Makefile |   1 +
>  xen/arch/arm/alternative.c| 217 
> ++
>  xen/arch/arm/setup.c  |   8 ++
>  xen/arch/arm/xen.lds.S|   7 ++
>  xen/include/asm-arm/alternative.h | 165 +
>  xen/include/asm-arm/arm64/insn.h  |  16 +++
>  7 files changed, 419 insertions(+)
>  create mode 100644 xen/arch/arm/alternative.c
>  create mode 100644 xen/include/asm-arm/alternative.h
> 
> diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
> index 6231cd5..9b3e66b 100644
> --- a/xen/arch/arm/Kconfig
> +++ b/xen/arch/arm/Kconfig
> @@ -14,6 +14,7 @@ config ARM_64
>   def_bool y
>   depends on 64BIT
>   select HAS_GICV3
> +select ALTERNATIVE

Hard tabs, not spaces.
>  
>  config ARM
>   def_bool y
> @@ -46,6 +47,10 @@ config ACPI
>  config HAS_GICV3
>   bool
>  
> +# Select ALTERNATIVE if the architecture supports runtime patching
> +config ALTERNATIVE
> +bool

Ditto. 
> +
>  endmenu
>  
>  source "common/Kconfig"
> diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
> index 9122f78..2d330aa 100644
> --- a/xen/arch/arm/Makefile
> +++ b/xen/arch/arm/Makefile
> @@ -4,6 +4,7 @@ subdir-y += platforms
>  subdir-$(CONFIG_ARM_64) += efi
>  subdir-$(CONFIG_ACPI) += acpi
>  
> +obj-$(CONFIG_ALTERNATIVE) += alternative.o

This probably needs to be alternative.init.o ?
>  obj-y += bootfdt.o
>  obj-y += cpu.o
>  obj-y += cpufeature.o
> diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
> new file mode 100644
> index 000..0a5d1c8
> --- /dev/null
> +++ b/xen/arch/arm/alternative.c
> @@ -0,0 +1,217 @@
> +/*
> + * alternative runtime patching
> + * inspired by the x86 version
> + *
> + * Copyright (C) 2014 ARM Ltd.

Not 2016?

> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License version 2 as
> + * published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program.  If not, see .
> + */
> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#define __ALT_PTR(a,f)  (u32 *)((void *)&(a)->f + (a)->f)
> +#define ALT_ORIG_PTR(a) __ALT_PTR(a, orig_offset)
> +#define ALT_REPL_PTR(a) __ALT_PTR(a, alt_offset)
> +
> +extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
> +
> +struct alt_region {
> +const struct alt_instr *begin;
> +const struct alt_instr *end;
> +};
> +
> +/*
> + * Check if the target PC is within an alternative block.
> + */
> +static bool_t branch_insn_requires_update(const struct alt_instr *alt,

__init?
> +  unsigned long pc)
> +{
> +unsigned long replptr;
> +
> +if ( is_active_kernel_text(pc) )
> +return 1;
> +
> +replptr = (unsigned long)ALT_REPL_PTR(alt);
> +if ( pc >= replptr && pc <= (replptr + alt->alt_len) )
> +return 0;
> +
> +/*
> + * Branching into *another* alternate sequence is doomed, and
> + * we're not even trying to fix it up.
> + */
> +BUG();
> +}
> +
> +static u32 get_alt_insn(const struct alt_instr *alt,
> +const u32 *insnptr, const u32 *altinsnptr)
> +{
> +u32 insn;
> +
>

[Xen-devel] [RFC 10/16] xen/arm: Introduce alternative runtime patching

2016-05-05 Thread Julien Grall 
Some of the processor erratum will require to modify code sequence.
As those modifications may impact the performance, they should only
be enabled on affected cores. Furthermore, Xen may also want to take
advantage of new hardware features coming up with v8.1 and v8.2.

This patch adds an infrastructure to patch Xen during boot time
depending on the "features" available on the platform.

This code is based on the file arch/arm64/kernel/alternative.c in
Linux 4.6-rc3. Any references to arm64 have been dropped to make the
code as generic as possible.

Furthermore, in Xen the executable sections (.text and .init.text)
are always mapped read-only and enforced by SCTLR.WNX. So it is not
possible to directly patch Xen.

To by-pass this restriction, a temporary writeable mapping is made with
vmap. This mapping will be used to write the new instructions.

Lastly, runtime patching is currently not necessary for ARM32. So the
code is only enabled for ARM64.

Signed-off-by: Julien Grall 
---
 xen/arch/arm/Kconfig  |   5 +
 xen/arch/arm/Makefile |   1 +
 xen/arch/arm/alternative.c| 217 ++
 xen/arch/arm/setup.c  |   8 ++
 xen/arch/arm/xen.lds.S|   7 ++
 xen/include/asm-arm/alternative.h | 165 +
 xen/include/asm-arm/arm64/insn.h  |  16 +++
 7 files changed, 419 insertions(+)
 create mode 100644 xen/arch/arm/alternative.c
 create mode 100644 xen/include/asm-arm/alternative.h

diff --git a/xen/arch/arm/Kconfig b/xen/arch/arm/Kconfig
index 6231cd5..9b3e66b 100644
--- a/xen/arch/arm/Kconfig
+++ b/xen/arch/arm/Kconfig
@@ -14,6 +14,7 @@ config ARM_64
def_bool y
depends on 64BIT
select HAS_GICV3
+select ALTERNATIVE
 
 config ARM
def_bool y
@@ -46,6 +47,10 @@ config ACPI
 config HAS_GICV3
bool
 
+# Select ALTERNATIVE if the architecture supports runtime patching
+config ALTERNATIVE
+bool
+
 endmenu
 
 source "common/Kconfig"
diff --git a/xen/arch/arm/Makefile b/xen/arch/arm/Makefile
index 9122f78..2d330aa 100644
--- a/xen/arch/arm/Makefile
+++ b/xen/arch/arm/Makefile
@@ -4,6 +4,7 @@ subdir-y += platforms
 subdir-$(CONFIG_ARM_64) += efi
 subdir-$(CONFIG_ACPI) += acpi
 
+obj-$(CONFIG_ALTERNATIVE) += alternative.o
 obj-y += bootfdt.o
 obj-y += cpu.o
 obj-y += cpufeature.o
diff --git a/xen/arch/arm/alternative.c b/xen/arch/arm/alternative.c
new file mode 100644
index 000..0a5d1c8
--- /dev/null
+++ b/xen/arch/arm/alternative.c
@@ -0,0 +1,217 @@
+/*
+ * alternative runtime patching
+ * inspired by the x86 version
+ *
+ * Copyright (C) 2014 ARM Ltd.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see .
+ */
+
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+#include 
+
+#define __ALT_PTR(a,f)  (u32 *)((void *)&(a)->f + (a)->f)
+#define ALT_ORIG_PTR(a) __ALT_PTR(a, orig_offset)
+#define ALT_REPL_PTR(a) __ALT_PTR(a, alt_offset)
+
+extern const struct alt_instr __alt_instructions[], __alt_instructions_end[];
+
+struct alt_region {
+const struct alt_instr *begin;
+const struct alt_instr *end;
+};
+
+/*
+ * Check if the target PC is within an alternative block.
+ */
+static bool_t branch_insn_requires_update(const struct alt_instr *alt,
+  unsigned long pc)
+{
+unsigned long replptr;
+
+if ( is_active_kernel_text(pc) )
+return 1;
+
+replptr = (unsigned long)ALT_REPL_PTR(alt);
+if ( pc >= replptr && pc <= (replptr + alt->alt_len) )
+return 0;
+
+/*
+ * Branching into *another* alternate sequence is doomed, and
+ * we're not even trying to fix it up.
+ */
+BUG();
+}
+
+static u32 get_alt_insn(const struct alt_instr *alt,
+const u32 *insnptr, const u32 *altinsnptr)
+{
+u32 insn;
+
+insn = le32_to_cpu(*altinsnptr);
+
+if ( insn_is_branch_imm(insn) )
+{
+s32 offset = insn_get_branch_offset(insn);
+unsigned long target;
+
+target = (unsigned long)altinsnptr + offset;
+
+/*
+ * If we're branching inside the alternate sequence,
+ * do not rewrite the instruction, as it is already
+ * correct. Otherwise, generate the new instruction.
+ */
+if ( branch_insn_requires_update(alt, target) )
+{
+