Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-24 Thread Dave Martin 
On Mon, Sep 24, 2012 at 10:49:34AM -0400, Cyril Chemparathy wrote:
> Hi Dave,
> 
> Thanks for the detailed review...
> 
> On 9/24/2012 8:06 AM, Dave Martin wrote:
> >On Fri, Sep 21, 2012 at 11:55:59AM -0400, Cyril Chemparathy wrote:
> >>The original phys_to_virt/virt_to_phys patching implementation relied on 
> >>early
> >>patching prior to MMU initialization.  On PAE systems running out of >4G
> >>address space, this would have entailed an additional round of patching 
> >>after
> >>switching over to the high address space.
> >>
> >>The approach implemented here conceptually extends the original PHYS_OFFSET
> >>patching implementation with the introduction of "early" patch stubs.  Early
> >>patch code is required to be functional out of the box, even before the 
> >>patch
> >>is applied.  This is implemented by inserting functional (but inefficient)
> >>load code into the .runtime.patch.code init section.  Having functional code
> >>out of the box then allows us to defer the init time patch application until
> >>later in the init sequence.
> >
> >There are currently a few different patching mechanisms in the kernel, and
> >it would be good if we could collect more of them under some common
> >framework.
> >
> 
> That would be great!  I could use some pointers here.  I've looked
> at the kprobes code, and there doesn't appear to be much of an
> intersection there...

Well, I think we shouldn't try it for now.  I think kprobes is
doing something different enough that it doesn't necessarily make
sense to unify that.

If your patches are useful as-is, we shouldn't worry about adding
features until later.

> >For example, it might be possible to do the SMP-on-UP fixups using the same
> >framework you propose.  Best not to attempt that yet, though.
> >
> 
> ... and I've looked at the ALT_SMP/ALT_UP stuff as well.  The
> problem here appears to be that ALT_SMP is needed way early in boot
> - up in head.S assembly-land.

Indeed, this would need some thought.  Again, this is just a "maybe".
No need to try it right now.

> 
> The third place is probably the jump label mechanism.  That may be a
> fit with some work, but I'm not sure yet.
> 
> >Overall, this looks well thought out and useful, though it looks like it
> >has a few issues that need attention.
> >
> >Comments below.
> >
> >Cheers
> >---Dave
> >
> [...]
> >>Note: the obtuse use of stringified symbols in patch_stub() and
> >>early_patch_stub() is intentional.  Theoretically this should have been
> >>accomplished with formal operands passed into the asm block, but this 
> >>requires
> >>the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
> >>However, the 'c' modifier has been found to ICE certain versions of GCC, and
> >>therefore we resort to stringified symbols here.
> >
> >You might find that the "n" constraint works.  The explanation in the
> >GCC docs is pretty incomprehensible, but at least it exists.
> >
> >__stringify hacks are not uncommon anyway though, so it's not a problem
> >either way.
> >
> 
> The stringify hack is nasty, and I'd like to get rid of it if I could...
> 
> I looked up constraints.md, and couldn't find "n" defined as a
> constraint.  For that matter, I couldn't find "n" being handled as a
> modifier in arm_print_operand() either.  BTW, I'm looking at GCC 4.6
> for these.
> 
> Could you please point me to the doc in question that lists this
> constraint/modifier?  Is this specific to newer (or older) versions
> of GCC.

See the "Simple Constraints" section in the GCC docs.

However, it looks like "n" only works for operands which really do
evaluate to constant integers.  It doesn't work for expressions based
on symbols:

void g(int);
asm(".long %0" :: "n" (g));

tst.c:6:2: warning: asm operand 0 probably doesn't match constraints
tst.c:6:2: error: impossible constraint in 'asm'


So scratch that suggestion...  the __stringify() method looks better.

> 
> [...]
> >>+#define early_patch_stub(type, code, pad, patch_data, ...) \
> >>+   __asm__("@ patch stub\n"\
> >>+   "1:\n"  \
> >>+   "   b   6f\n"   \
> >>+   "   .fill   " __stringify(pad) ", 1, 0\n"   \
> >
> >What is the pad argument for?  It never seems to be set to anything other
> >than 0 in your series.
> >
> 
> The pad argument is used when we need more than one 32-bit
> instruction slot in the straight line (post patch) code path.
> Please look at patch 05/17 of the series (ARM: LPAE: support 64-bit
> virt_to_phys patching). When we patch 64-bit virt_to_phys(), we need
> that extra slot to fill up the upper 32-bits of the result.

I see -- it looks like I missed that.

I guess this is probably OK, given that for every instruction-sized
bit of padding, there should be a corresponding patch data definition
in the same asm block which increases GCC's guess about the

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-24 Thread Cyril Chemparathy 

Hi Dave,

Thanks for the detailed review...

On 9/24/2012 8:06 AM, Dave Martin wrote:

On Fri, Sep 21, 2012 at 11:55:59AM -0400, Cyril Chemparathy wrote:

The original phys_to_virt/virt_to_phys patching implementation relied on early
patching prior to MMU initialization.  On PAE systems running out of >4G
address space, this would have entailed an additional round of patching after
switching over to the high address space.

The approach implemented here conceptually extends the original PHYS_OFFSET
patching implementation with the introduction of "early" patch stubs.  Early
patch code is required to be functional out of the box, even before the patch
is applied.  This is implemented by inserting functional (but inefficient)
load code into the .runtime.patch.code init section.  Having functional code
out of the box then allows us to defer the init time patch application until
later in the init sequence.


There are currently a few different patching mechanisms in the kernel, and
it would be good if we could collect more of them under some common
framework.



That would be great!  I could use some pointers here.  I've looked at 
the kprobes code, and there doesn't appear to be much of an intersection 
there...



For example, it might be possible to do the SMP-on-UP fixups using the same
framework you propose.  Best not to attempt that yet, though.



... and I've looked at the ALT_SMP/ALT_UP stuff as well.  The problem 
here appears to be that ALT_SMP is needed way early in boot - up in 
head.S assembly-land.


The third place is probably the jump label mechanism.  That may be a fit 
with some work, but I'm not sure yet.



Overall, this looks well thought out and useful, though it looks like it
has a few issues that need attention.

Comments below.

Cheers
---Dave


[...]

Note: the obtuse use of stringified symbols in patch_stub() and
early_patch_stub() is intentional.  Theoretically this should have been
accomplished with formal operands passed into the asm block, but this requires
the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
However, the 'c' modifier has been found to ICE certain versions of GCC, and
therefore we resort to stringified symbols here.


You might find that the "n" constraint works.  The explanation in the
GCC docs is pretty incomprehensible, but at least it exists.

__stringify hacks are not uncommon anyway though, so it's not a problem
either way.



The stringify hack is nasty, and I'd like to get rid of it if I could...

I looked up constraints.md, and couldn't find "n" defined as a 
constraint.  For that matter, I couldn't find "n" being handled as a 
modifier in arm_print_operand() either.  BTW, I'm looking at GCC 4.6 for 
these.


Could you please point me to the doc in question that lists this 
constraint/modifier?  Is this specific to newer (or older) versions of GCC.


[...]

+#define early_patch_stub(type, code, pad, patch_data, ...) \
+   __asm__("@ patch stub\n"  \
+   "1:\n"\
+   "  b   6f\n"  \
+   "  .fill   " __stringify(pad) ", 1, 0\n"\


What is the pad argument for?  It never seems to be set to anything other
than 0 in your series.



The pad argument is used when we need more than one 32-bit instruction 
slot in the straight line (post patch) code path.  Please look at patch 
05/17 of the series (ARM: LPAE: support 64-bit virt_to_phys patching). 
When we patch 64-bit virt_to_phys(), we need that extra slot to fill up 
the upper 32-bits of the result.



The compiler uses a pretty dumb heuristic to guess the size of asms:
4 * (number of ; or \n in the string)

Directives that the compiler can't predict the size of are not safe if
they output into any segment that the compiler uses.  .fill/.skip are
obvious candidates, but macro expansions, .rept, .irp etc. can cause
these problems too.

For example:

void g(int);
void f(void)
{
g(0xd00dfeed);
asm(".skip 0x1000");
}

If you try building this with gcc -marm -Os for example:

/tmp/ccXYm1uP.s: Assembler messages:
/tmp/ccXYm1uP.s:21: Error: bad immediate value for offset (4100)

...because the assembler assumes that it can dump a literal at the end
of the function and reference it from the g() callsite.


It may be that you have some intended future use for pad (such as
pasting one instruction sequence in place of another possibly
differently-sized sequence at fixup time), in which case this might
require a bit more thought.



Good point.  Thanks.

I'm not sure if this helps, but I don't think pad should be used to 
insert more than a couple of instruction words into the code.



+   "2:\n"\
+   "  .pushsection .runtime.patch.table, \"a\"\n"  \
+

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-24 Thread Dave Martin 
On Fri, Sep 21, 2012 at 11:55:59AM -0400, Cyril Chemparathy wrote:
> The original phys_to_virt/virt_to_phys patching implementation relied on early
> patching prior to MMU initialization.  On PAE systems running out of >4G
> address space, this would have entailed an additional round of patching after
> switching over to the high address space.
> 
> The approach implemented here conceptually extends the original PHYS_OFFSET
> patching implementation with the introduction of "early" patch stubs.  Early
> patch code is required to be functional out of the box, even before the patch
> is applied.  This is implemented by inserting functional (but inefficient)
> load code into the .runtime.patch.code init section.  Having functional code
> out of the box then allows us to defer the init time patch application until
> later in the init sequence.

There are currently a few different patching mechanisms in the kernel, and
it would be good if we could collect more of them under some common
framework.

For example, it might be possible to do the SMP-on-UP fixups using the same
framework you propose.  Best not to attempt that yet, though.


Overall, this looks well thought out and useful, though it looks like it
has a few issues that need attention.

Comments below.

Cheers
---Dave

> In addition to fitting better with our need for physical address-space
> switch-over, this implementation should be somewhat more extensible by virtue
> of its more readable (and hackable) C implementation.  This should prove
> useful for other similar init time specialization needs, especially in light
> of our multi-platform kernel initiative.
> 
> This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
> (Cortex-A8) device.
> 
> Note: the obtuse use of stringified symbols in patch_stub() and
> early_patch_stub() is intentional.  Theoretically this should have been
> accomplished with formal operands passed into the asm block, but this requires
> the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
> However, the 'c' modifier has been found to ICE certain versions of GCC, and
> therefore we resort to stringified symbols here.

You might find that the "n" constraint works.  The explanation in the
GCC docs is pretty incomprehensible, but at least it exists.

__stringify hacks are not uncommon anyway though, so it's not a problem
either way.

> Signed-off-by: Cyril Chemparathy 
> Reviewed-by: Nicolas Pitre 
> ---
>  arch/arm/Kconfig |3 +
>  arch/arm/include/asm/module.h|7 ++
>  arch/arm/include/asm/runtime-patch.h |  208 
> ++
>  arch/arm/kernel/Makefile |1 +
>  arch/arm/kernel/module.c |9 +-
>  arch/arm/kernel/runtime-patch.c  |  193 +++
>  arch/arm/kernel/setup.c  |3 +
>  arch/arm/kernel/vmlinux.lds.S|   10 ++
>  8 files changed, 433 insertions(+), 1 deletion(-)
>  create mode 100644 arch/arm/include/asm/runtime-patch.h
>  create mode 100644 arch/arm/kernel/runtime-patch.c
> 
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index 2f88d8d..98a3a1a 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -60,6 +60,9 @@ config ARM
>  config ARM_HAS_SG_CHAIN
>   bool
>  
> +config ARM_RUNTIME_PATCH
> + bool
> +
>  config NEED_SG_DMA_LENGTH
>   bool
>  
> diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
> index 6c6809f..2090486 100644
> --- a/arch/arm/include/asm/module.h
> +++ b/arch/arm/include/asm/module.h
> @@ -43,9 +43,16 @@ struct mod_arch_specific {
>  #define MODULE_ARCH_VERMAGIC_ARMTHUMB ""
>  #endif
>  
> +#ifdef CONFIG_ARM_RUNTIME_PATCH
> +#define MODULE_ARCH_VERMAGIC_RT_PATCH "rt-patch "
> +#else
> +#define MODULE_ARCH_VERMAGIC_RT_PATCH ""
> +#endif
> +
>  #define MODULE_ARCH_VERMAGIC \
>   MODULE_ARCH_VERMAGIC_ARMVSN \
>   MODULE_ARCH_VERMAGIC_ARMTHUMB \
> + MODULE_ARCH_VERMAGIC_RT_PATCH \
>   MODULE_ARCH_VERMAGIC_P2V
>  
>  #endif /* _ASM_ARM_MODULE_H */
> diff --git a/arch/arm/include/asm/runtime-patch.h 
> b/arch/arm/include/asm/runtime-patch.h
> new file mode 100644
> index 000..366444d
> --- /dev/null
> +++ b/arch/arm/include/asm/runtime-patch.h
> @@ -0,0 +1,208 @@
> +/*
> + * arch/arm/include/asm/runtime-patch.h
> + * Note: this file should not be included by non-asm/.h files
> + *
> + * Copyright 2012 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-24 Thread Dave Martin 
On Fri, Sep 21, 2012 at 11:55:59AM -0400, Cyril Chemparathy wrote:
 The original phys_to_virt/virt_to_phys patching implementation relied on early
 patching prior to MMU initialization.  On PAE systems running out of 4G
 address space, this would have entailed an additional round of patching after
 switching over to the high address space.
 
 The approach implemented here conceptually extends the original PHYS_OFFSET
 patching implementation with the introduction of early patch stubs.  Early
 patch code is required to be functional out of the box, even before the patch
 is applied.  This is implemented by inserting functional (but inefficient)
 load code into the .runtime.patch.code init section.  Having functional code
 out of the box then allows us to defer the init time patch application until
 later in the init sequence.

There are currently a few different patching mechanisms in the kernel, and
it would be good if we could collect more of them under some common
framework.

For example, it might be possible to do the SMP-on-UP fixups using the same
framework you propose.  Best not to attempt that yet, though.


Overall, this looks well thought out and useful, though it looks like it
has a few issues that need attention.

Comments below.

Cheers
---Dave

 In addition to fitting better with our need for physical address-space
 switch-over, this implementation should be somewhat more extensible by virtue
 of its more readable (and hackable) C implementation.  This should prove
 useful for other similar init time specialization needs, especially in light
 of our multi-platform kernel initiative.
 
 This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
 (Cortex-A8) device.
 
 Note: the obtuse use of stringified symbols in patch_stub() and
 early_patch_stub() is intentional.  Theoretically this should have been
 accomplished with formal operands passed into the asm block, but this requires
 the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
 However, the 'c' modifier has been found to ICE certain versions of GCC, and
 therefore we resort to stringified symbols here.

You might find that the n constraint works.  The explanation in the
GCC docs is pretty incomprehensible, but at least it exists.

__stringify hacks are not uncommon anyway though, so it's not a problem
either way.

 Signed-off-by: Cyril Chemparathy cy...@ti.com
 Reviewed-by: Nicolas Pitre n...@linaro.org
 ---
  arch/arm/Kconfig |3 +
  arch/arm/include/asm/module.h|7 ++
  arch/arm/include/asm/runtime-patch.h |  208 
 ++
  arch/arm/kernel/Makefile |1 +
  arch/arm/kernel/module.c |9 +-
  arch/arm/kernel/runtime-patch.c  |  193 +++
  arch/arm/kernel/setup.c  |3 +
  arch/arm/kernel/vmlinux.lds.S|   10 ++
  8 files changed, 433 insertions(+), 1 deletion(-)
  create mode 100644 arch/arm/include/asm/runtime-patch.h
  create mode 100644 arch/arm/kernel/runtime-patch.c
 
 diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
 index 2f88d8d..98a3a1a 100644
 --- a/arch/arm/Kconfig
 +++ b/arch/arm/Kconfig
 @@ -60,6 +60,9 @@ config ARM
  config ARM_HAS_SG_CHAIN
   bool
  
 +config ARM_RUNTIME_PATCH
 + bool
 +
  config NEED_SG_DMA_LENGTH
   bool
  
 diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
 index 6c6809f..2090486 100644
 --- a/arch/arm/include/asm/module.h
 +++ b/arch/arm/include/asm/module.h
 @@ -43,9 +43,16 @@ struct mod_arch_specific {
  #define MODULE_ARCH_VERMAGIC_ARMTHUMB 
  #endif
  
 +#ifdef CONFIG_ARM_RUNTIME_PATCH
 +#define MODULE_ARCH_VERMAGIC_RT_PATCH rt-patch 
 +#else
 +#define MODULE_ARCH_VERMAGIC_RT_PATCH 
 +#endif
 +
  #define MODULE_ARCH_VERMAGIC \
   MODULE_ARCH_VERMAGIC_ARMVSN \
   MODULE_ARCH_VERMAGIC_ARMTHUMB \
 + MODULE_ARCH_VERMAGIC_RT_PATCH \
   MODULE_ARCH_VERMAGIC_P2V
  
  #endif /* _ASM_ARM_MODULE_H */
 diff --git a/arch/arm/include/asm/runtime-patch.h 
 b/arch/arm/include/asm/runtime-patch.h
 new file mode 100644
 index 000..366444d
 --- /dev/null
 +++ b/arch/arm/include/asm/runtime-patch.h
 @@ -0,0 +1,208 @@
 +/*
 + * arch/arm/include/asm/runtime-patch.h
 + * Note: this file should not be included by non-asm/.h files
 + *
 + * Copyright 2012 Texas Instruments, Inc.
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms and conditions of the GNU General Public License,
 + * version 2, as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope 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 http://www.gnu.org/licenses/.
 +

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-24 Thread Cyril Chemparathy 

Hi Dave,

Thanks for the detailed review...

On 9/24/2012 8:06 AM, Dave Martin wrote:

On Fri, Sep 21, 2012 at 11:55:59AM -0400, Cyril Chemparathy wrote:

The original phys_to_virt/virt_to_phys patching implementation relied on early
patching prior to MMU initialization.  On PAE systems running out of 4G
address space, this would have entailed an additional round of patching after
switching over to the high address space.

The approach implemented here conceptually extends the original PHYS_OFFSET
patching implementation with the introduction of early patch stubs.  Early
patch code is required to be functional out of the box, even before the patch
is applied.  This is implemented by inserting functional (but inefficient)
load code into the .runtime.patch.code init section.  Having functional code
out of the box then allows us to defer the init time patch application until
later in the init sequence.


There are currently a few different patching mechanisms in the kernel, and
it would be good if we could collect more of them under some common
framework.



That would be great!  I could use some pointers here.  I've looked at 
the kprobes code, and there doesn't appear to be much of an intersection 
there...



For example, it might be possible to do the SMP-on-UP fixups using the same
framework you propose.  Best not to attempt that yet, though.



... and I've looked at the ALT_SMP/ALT_UP stuff as well.  The problem 
here appears to be that ALT_SMP is needed way early in boot - up in 
head.S assembly-land.


The third place is probably the jump label mechanism.  That may be a fit 
with some work, but I'm not sure yet.



Overall, this looks well thought out and useful, though it looks like it
has a few issues that need attention.

Comments below.

Cheers
---Dave


[...]

Note: the obtuse use of stringified symbols in patch_stub() and
early_patch_stub() is intentional.  Theoretically this should have been
accomplished with formal operands passed into the asm block, but this requires
the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
However, the 'c' modifier has been found to ICE certain versions of GCC, and
therefore we resort to stringified symbols here.


You might find that the n constraint works.  The explanation in the
GCC docs is pretty incomprehensible, but at least it exists.

__stringify hacks are not uncommon anyway though, so it's not a problem
either way.



The stringify hack is nasty, and I'd like to get rid of it if I could...

I looked up constraints.md, and couldn't find n defined as a 
constraint.  For that matter, I couldn't find n being handled as a 
modifier in arm_print_operand() either.  BTW, I'm looking at GCC 4.6 for 
these.


Could you please point me to the doc in question that lists this 
constraint/modifier?  Is this specific to newer (or older) versions of GCC.


[...]

+#define early_patch_stub(type, code, pad, patch_data, ...) \
+   __asm__(@ patch stub\n  \
+   1:\n\
+ b   6f\n  \
+ .fill__stringify(pad) , 1, 0\n\


What is the pad argument for?  It never seems to be set to anything other
than 0 in your series.



The pad argument is used when we need more than one 32-bit instruction 
slot in the straight line (post patch) code path.  Please look at patch 
05/17 of the series (ARM: LPAE: support 64-bit virt_to_phys patching). 
When we patch 64-bit virt_to_phys(), we need that extra slot to fill up 
the upper 32-bits of the result.



The compiler uses a pretty dumb heuristic to guess the size of asms:
4 * (number of ; or \n in the string)

Directives that the compiler can't predict the size of are not safe if
they output into any segment that the compiler uses.  .fill/.skip are
obvious candidates, but macro expansions, .rept, .irp etc. can cause
these problems too.

For example:

void g(int);
void f(void)
{
g(0xd00dfeed);
asm(.skip 0x1000);
}

If you try building this with gcc -marm -Os for example:

/tmp/ccXYm1uP.s: Assembler messages:
/tmp/ccXYm1uP.s:21: Error: bad immediate value for offset (4100)

...because the assembler assumes that it can dump a literal at the end
of the function and reference it from the g() callsite.


It may be that you have some intended future use for pad (such as
pasting one instruction sequence in place of another possibly
differently-sized sequence at fixup time), in which case this might
require a bit more thought.



Good point.  Thanks.

I'm not sure if this helps, but I don't think pad should be used to 
insert more than a couple of instruction words into the code.



+   2:\n\
+ .pushsection .runtime.patch.table, \a\\n  \
+   3:\n

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-24 Thread Dave Martin 
On Mon, Sep 24, 2012 at 10:49:34AM -0400, Cyril Chemparathy wrote:
 Hi Dave,
 
 Thanks for the detailed review...
 
 On 9/24/2012 8:06 AM, Dave Martin wrote:
 On Fri, Sep 21, 2012 at 11:55:59AM -0400, Cyril Chemparathy wrote:
 The original phys_to_virt/virt_to_phys patching implementation relied on 
 early
 patching prior to MMU initialization.  On PAE systems running out of 4G
 address space, this would have entailed an additional round of patching 
 after
 switching over to the high address space.
 
 The approach implemented here conceptually extends the original PHYS_OFFSET
 patching implementation with the introduction of early patch stubs.  Early
 patch code is required to be functional out of the box, even before the 
 patch
 is applied.  This is implemented by inserting functional (but inefficient)
 load code into the .runtime.patch.code init section.  Having functional code
 out of the box then allows us to defer the init time patch application until
 later in the init sequence.
 
 There are currently a few different patching mechanisms in the kernel, and
 it would be good if we could collect more of them under some common
 framework.
 
 
 That would be great!  I could use some pointers here.  I've looked
 at the kprobes code, and there doesn't appear to be much of an
 intersection there...

Well, I think we shouldn't try it for now.  I think kprobes is
doing something different enough that it doesn't necessarily make
sense to unify that.

If your patches are useful as-is, we shouldn't worry about adding
features until later.

 For example, it might be possible to do the SMP-on-UP fixups using the same
 framework you propose.  Best not to attempt that yet, though.
 
 
 ... and I've looked at the ALT_SMP/ALT_UP stuff as well.  The
 problem here appears to be that ALT_SMP is needed way early in boot
 - up in head.S assembly-land.

Indeed, this would need some thought.  Again, this is just a maybe.
No need to try it right now.

 
 The third place is probably the jump label mechanism.  That may be a
 fit with some work, but I'm not sure yet.
 
 Overall, this looks well thought out and useful, though it looks like it
 has a few issues that need attention.
 
 Comments below.
 
 Cheers
 ---Dave
 
 [...]
 Note: the obtuse use of stringified symbols in patch_stub() and
 early_patch_stub() is intentional.  Theoretically this should have been
 accomplished with formal operands passed into the asm block, but this 
 requires
 the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
 However, the 'c' modifier has been found to ICE certain versions of GCC, and
 therefore we resort to stringified symbols here.
 
 You might find that the n constraint works.  The explanation in the
 GCC docs is pretty incomprehensible, but at least it exists.
 
 __stringify hacks are not uncommon anyway though, so it's not a problem
 either way.
 
 
 The stringify hack is nasty, and I'd like to get rid of it if I could...
 
 I looked up constraints.md, and couldn't find n defined as a
 constraint.  For that matter, I couldn't find n being handled as a
 modifier in arm_print_operand() either.  BTW, I'm looking at GCC 4.6
 for these.
 
 Could you please point me to the doc in question that lists this
 constraint/modifier?  Is this specific to newer (or older) versions
 of GCC.

See the Simple Constraints section in the GCC docs.

However, it looks like n only works for operands which really do
evaluate to constant integers.  It doesn't work for expressions based
on symbols:

void g(int);
asm(.long %0 :: n (g));

tst.c:6:2: warning: asm operand 0 probably doesn't match constraints
tst.c:6:2: error: impossible constraint in 'asm'


So scratch that suggestion...  the __stringify() method looks better.

 
 [...]
 +#define early_patch_stub(type, code, pad, patch_data, ...) \
 +   __asm__(@ patch stub\n\
 +   1:\n  \
 +  b   6f\n   \
 +  .fill__stringify(pad) , 1, 0\n   \
 
 What is the pad argument for?  It never seems to be set to anything other
 than 0 in your series.
 
 
 The pad argument is used when we need more than one 32-bit
 instruction slot in the straight line (post patch) code path.
 Please look at patch 05/17 of the series (ARM: LPAE: support 64-bit
 virt_to_phys patching). When we patch 64-bit virt_to_phys(), we need
 that extra slot to fill up the upper 32-bits of the result.

I see -- it looks like I missed that.

I guess this is probably OK, given that for every instruction-sized
bit of padding, there should be a corresponding patch data definition
in the same asm block which increases GCC's guess about the size of
the asm at least enough to avoid fixup problems.

Used like this, there should probably be no problem.

 The compiler uses a pretty dumb heuristic to guess the size of asms:
 4 * (number of ; or \n

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-22 Thread Cyril Chemparathy 

On 09/22/12 11:10, Nicolas Pitre wrote:

On Fri, 21 Sep 2012, Cyril Chemparathy wrote:


The original phys_to_virt/virt_to_phys patching implementation relied on early
patching prior to MMU initialization.  On PAE systems running out of >4G
address space, this would have entailed an additional round of patching after
switching over to the high address space.

The approach implemented here conceptually extends the original PHYS_OFFSET
patching implementation with the introduction of "early" patch stubs.  Early
patch code is required to be functional out of the box, even before the patch
is applied.  This is implemented by inserting functional (but inefficient)
load code into the .runtime.patch.code init section.  Having functional code
out of the box then allows us to defer the init time patch application until
later in the init sequence.

In addition to fitting better with our need for physical address-space
switch-over, this implementation should be somewhat more extensible by virtue
of its more readable (and hackable) C implementation.  This should prove
useful for other similar init time specialization needs, especially in light
of our multi-platform kernel initiative.

This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
(Cortex-A8) device.

Note: the obtuse use of stringified symbols in patch_stub() and
early_patch_stub() is intentional.  Theoretically this should have been
accomplished with formal operands passed into the asm block, but this requires
the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
However, the 'c' modifier has been found to ICE certain versions of GCC, and
therefore we resort to stringified symbols here.

Signed-off-by: Cyril Chemparathy 
Reviewed-by: Nicolas Pitre 


There is another problem with this.

[...]

diff --git a/arch/arm/include/asm/runtime-patch.h 
b/arch/arm/include/asm/runtime-patch.h
new file mode 100644
index 000..366444d
--- /dev/null
+++ b/arch/arm/include/asm/runtime-patch.h
@@ -0,0 +1,208 @@
+/*
+ * arch/arm/include/asm/runtime-patch.h
+ * Note: this file should not be included by non-asm/.h files
+ *
+ * Copyright 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 .
+ */
+#ifndef __ASM_ARM_RUNTIME_PATCH_H
+#define __ASM_ARM_RUNTIME_PATCH_H
+
+#include 
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_ARM_RUNTIME_PATCH
+
+struct patch_info {
+   void*insn;
+   u16  type;
+   u8   insn_size;
+   u8   data_size;
+   u32  data[0];
+};


This causes the following compilation error:

   CC  sound/core/pcm.o
In file included from sound/core/pcm.c:293:0:
include/linux/soundcard.h:223:8: error: redefinition of 'struct patch_info'
arch/arm/include/asm/runtime-patch.h:28:8: note: originally defined here
make[2]: *** [sound/core/pcm.o] Error 1

The problem is that asm/runtime-patch.h gets included by asm/memory.h
and asm/memory.h is included by almost the entire kernel. Something like
"struct patch_info" is a bit too generic a name to be exported to the
world as the likelihood of a name collision with some private definition
in a driver or the like is rather high.

In that context it might be worth moving everything that is not required
for the patch stub definitions out of asm/runtime-patch.h.  For example,
the definition of struct patch_info, struct patch_info_imm8,
patch_next() and patch_data() could be moved to runtime-patch.c directly
instead.  And then patch_stub() should be renamed to
runtime_patch_stub(), early_patch_stub() to early_runtime_patch_stub(),
patch_imm8() to runtime_patch_imm8(), etc.  Even the __IMM8 symbol name
is rather weak for kernel wide scope.



Thanks for trying this out.  I'll take this up in the next rev.

-- Cyril.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-22 Thread Nicolas Pitre 
On Fri, 21 Sep 2012, Cyril Chemparathy wrote:

> The original phys_to_virt/virt_to_phys patching implementation relied on early
> patching prior to MMU initialization.  On PAE systems running out of >4G
> address space, this would have entailed an additional round of patching after
> switching over to the high address space.
> 
> The approach implemented here conceptually extends the original PHYS_OFFSET
> patching implementation with the introduction of "early" patch stubs.  Early
> patch code is required to be functional out of the box, even before the patch
> is applied.  This is implemented by inserting functional (but inefficient)
> load code into the .runtime.patch.code init section.  Having functional code
> out of the box then allows us to defer the init time patch application until
> later in the init sequence.
> 
> In addition to fitting better with our need for physical address-space
> switch-over, this implementation should be somewhat more extensible by virtue
> of its more readable (and hackable) C implementation.  This should prove
> useful for other similar init time specialization needs, especially in light
> of our multi-platform kernel initiative.
> 
> This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
> (Cortex-A8) device.
> 
> Note: the obtuse use of stringified symbols in patch_stub() and
> early_patch_stub() is intentional.  Theoretically this should have been
> accomplished with formal operands passed into the asm block, but this requires
> the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
> However, the 'c' modifier has been found to ICE certain versions of GCC, and
> therefore we resort to stringified symbols here.
> 
> Signed-off-by: Cyril Chemparathy 
> Reviewed-by: Nicolas Pitre 

There is another problem with this.

[...]
> diff --git a/arch/arm/include/asm/runtime-patch.h 
> b/arch/arm/include/asm/runtime-patch.h
> new file mode 100644
> index 000..366444d
> --- /dev/null
> +++ b/arch/arm/include/asm/runtime-patch.h
> @@ -0,0 +1,208 @@
> +/*
> + * arch/arm/include/asm/runtime-patch.h
> + * Note: this file should not be included by non-asm/.h files
> + *
> + * Copyright 2012 Texas Instruments, Inc.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms and conditions of the GNU General Public License,
> + * version 2, as published by the Free Software Foundation.
> + *
> + * This program is distributed in the hope 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 .
> + */
> +#ifndef __ASM_ARM_RUNTIME_PATCH_H
> +#define __ASM_ARM_RUNTIME_PATCH_H
> +
> +#include 
> +
> +#ifndef __ASSEMBLY__
> +
> +#ifdef CONFIG_ARM_RUNTIME_PATCH
> +
> +struct patch_info {
> + void*insn;
> + u16  type;
> + u8   insn_size;
> + u8   data_size;
> + u32  data[0];
> +};

This causes the following compilation error:

  CC  sound/core/pcm.o
In file included from sound/core/pcm.c:293:0:
include/linux/soundcard.h:223:8: error: redefinition of 'struct patch_info'
arch/arm/include/asm/runtime-patch.h:28:8: note: originally defined here
make[2]: *** [sound/core/pcm.o] Error 1

The problem is that asm/runtime-patch.h gets included by asm/memory.h 
and asm/memory.h is included by almost the entire kernel. Something like 
"struct patch_info" is a bit too generic a name to be exported to the 
world as the likelihood of a name collision with some private definition 
in a driver or the like is rather high.  

In that context it might be worth moving everything that is not required 
for the patch stub definitions out of asm/runtime-patch.h.  For example, 
the definition of struct patch_info, struct patch_info_imm8, 
patch_next() and patch_data() could be moved to runtime-patch.c directly 
instead.  And then patch_stub() should be renamed to 
runtime_patch_stub(), early_patch_stub() to early_runtime_patch_stub(), 
patch_imm8() to runtime_patch_imm8(), etc.  Even the __IMM8 symbol name 
is rather weak for kernel wide scope.


Nicolas
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-22 Thread Nicolas Pitre 
On Fri, 21 Sep 2012, Cyril Chemparathy wrote:

 The original phys_to_virt/virt_to_phys patching implementation relied on early
 patching prior to MMU initialization.  On PAE systems running out of 4G
 address space, this would have entailed an additional round of patching after
 switching over to the high address space.
 
 The approach implemented here conceptually extends the original PHYS_OFFSET
 patching implementation with the introduction of early patch stubs.  Early
 patch code is required to be functional out of the box, even before the patch
 is applied.  This is implemented by inserting functional (but inefficient)
 load code into the .runtime.patch.code init section.  Having functional code
 out of the box then allows us to defer the init time patch application until
 later in the init sequence.
 
 In addition to fitting better with our need for physical address-space
 switch-over, this implementation should be somewhat more extensible by virtue
 of its more readable (and hackable) C implementation.  This should prove
 useful for other similar init time specialization needs, especially in light
 of our multi-platform kernel initiative.
 
 This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
 (Cortex-A8) device.
 
 Note: the obtuse use of stringified symbols in patch_stub() and
 early_patch_stub() is intentional.  Theoretically this should have been
 accomplished with formal operands passed into the asm block, but this requires
 the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
 However, the 'c' modifier has been found to ICE certain versions of GCC, and
 therefore we resort to stringified symbols here.
 
 Signed-off-by: Cyril Chemparathy cy...@ti.com
 Reviewed-by: Nicolas Pitre n...@linaro.org

There is another problem with this.

[...]
 diff --git a/arch/arm/include/asm/runtime-patch.h 
 b/arch/arm/include/asm/runtime-patch.h
 new file mode 100644
 index 000..366444d
 --- /dev/null
 +++ b/arch/arm/include/asm/runtime-patch.h
 @@ -0,0 +1,208 @@
 +/*
 + * arch/arm/include/asm/runtime-patch.h
 + * Note: this file should not be included by non-asm/.h files
 + *
 + * Copyright 2012 Texas Instruments, Inc.
 + *
 + * This program is free software; you can redistribute it and/or modify it
 + * under the terms and conditions of the GNU General Public License,
 + * version 2, as published by the Free Software Foundation.
 + *
 + * This program is distributed in the hope 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 http://www.gnu.org/licenses/.
 + */
 +#ifndef __ASM_ARM_RUNTIME_PATCH_H
 +#define __ASM_ARM_RUNTIME_PATCH_H
 +
 +#include linux/stringify.h
 +
 +#ifndef __ASSEMBLY__
 +
 +#ifdef CONFIG_ARM_RUNTIME_PATCH
 +
 +struct patch_info {
 + void*insn;
 + u16  type;
 + u8   insn_size;
 + u8   data_size;
 + u32  data[0];
 +};

This causes the following compilation error:

  CC  sound/core/pcm.o
In file included from sound/core/pcm.c:293:0:
include/linux/soundcard.h:223:8: error: redefinition of 'struct patch_info'
arch/arm/include/asm/runtime-patch.h:28:8: note: originally defined here
make[2]: *** [sound/core/pcm.o] Error 1

The problem is that asm/runtime-patch.h gets included by asm/memory.h 
and asm/memory.h is included by almost the entire kernel. Something like 
struct patch_info is a bit too generic a name to be exported to the 
world as the likelihood of a name collision with some private definition 
in a driver or the like is rather high.  

In that context it might be worth moving everything that is not required 
for the patch stub definitions out of asm/runtime-patch.h.  For example, 
the definition of struct patch_info, struct patch_info_imm8, 
patch_next() and patch_data() could be moved to runtime-patch.c directly 
instead.  And then patch_stub() should be renamed to 
runtime_patch_stub(), early_patch_stub() to early_runtime_patch_stub(), 
patch_imm8() to runtime_patch_imm8(), etc.  Even the __IMM8 symbol name 
is rather weak for kernel wide scope.


Nicolas
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-22 Thread Cyril Chemparathy 

On 09/22/12 11:10, Nicolas Pitre wrote:

On Fri, 21 Sep 2012, Cyril Chemparathy wrote:


The original phys_to_virt/virt_to_phys patching implementation relied on early
patching prior to MMU initialization.  On PAE systems running out of 4G
address space, this would have entailed an additional round of patching after
switching over to the high address space.

The approach implemented here conceptually extends the original PHYS_OFFSET
patching implementation with the introduction of early patch stubs.  Early
patch code is required to be functional out of the box, even before the patch
is applied.  This is implemented by inserting functional (but inefficient)
load code into the .runtime.patch.code init section.  Having functional code
out of the box then allows us to defer the init time patch application until
later in the init sequence.

In addition to fitting better with our need for physical address-space
switch-over, this implementation should be somewhat more extensible by virtue
of its more readable (and hackable) C implementation.  This should prove
useful for other similar init time specialization needs, especially in light
of our multi-platform kernel initiative.

This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
(Cortex-A8) device.

Note: the obtuse use of stringified symbols in patch_stub() and
early_patch_stub() is intentional.  Theoretically this should have been
accomplished with formal operands passed into the asm block, but this requires
the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
However, the 'c' modifier has been found to ICE certain versions of GCC, and
therefore we resort to stringified symbols here.

Signed-off-by: Cyril Chemparathy cy...@ti.com
Reviewed-by: Nicolas Pitre n...@linaro.org


There is another problem with this.

[...]

diff --git a/arch/arm/include/asm/runtime-patch.h 
b/arch/arm/include/asm/runtime-patch.h
new file mode 100644
index 000..366444d
--- /dev/null
+++ b/arch/arm/include/asm/runtime-patch.h
@@ -0,0 +1,208 @@
+/*
+ * arch/arm/include/asm/runtime-patch.h
+ * Note: this file should not be included by non-asm/.h files
+ *
+ * Copyright 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 http://www.gnu.org/licenses/.
+ */
+#ifndef __ASM_ARM_RUNTIME_PATCH_H
+#define __ASM_ARM_RUNTIME_PATCH_H
+
+#include linux/stringify.h
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_ARM_RUNTIME_PATCH
+
+struct patch_info {
+   void*insn;
+   u16  type;
+   u8   insn_size;
+   u8   data_size;
+   u32  data[0];
+};


This causes the following compilation error:

   CC  sound/core/pcm.o
In file included from sound/core/pcm.c:293:0:
include/linux/soundcard.h:223:8: error: redefinition of 'struct patch_info'
arch/arm/include/asm/runtime-patch.h:28:8: note: originally defined here
make[2]: *** [sound/core/pcm.o] Error 1

The problem is that asm/runtime-patch.h gets included by asm/memory.h
and asm/memory.h is included by almost the entire kernel. Something like
struct patch_info is a bit too generic a name to be exported to the
world as the likelihood of a name collision with some private definition
in a driver or the like is rather high.

In that context it might be worth moving everything that is not required
for the patch stub definitions out of asm/runtime-patch.h.  For example,
the definition of struct patch_info, struct patch_info_imm8,
patch_next() and patch_data() could be moved to runtime-patch.c directly
instead.  And then patch_stub() should be renamed to
runtime_patch_stub(), early_patch_stub() to early_runtime_patch_stub(),
patch_imm8() to runtime_patch_imm8(), etc.  Even the __IMM8 symbol name
is rather weak for kernel wide scope.



Thanks for trying this out.  I'll take this up in the next rev.

-- Cyril.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-21 Thread Cyril Chemparathy 
The original phys_to_virt/virt_to_phys patching implementation relied on early
patching prior to MMU initialization.  On PAE systems running out of >4G
address space, this would have entailed an additional round of patching after
switching over to the high address space.

The approach implemented here conceptually extends the original PHYS_OFFSET
patching implementation with the introduction of "early" patch stubs.  Early
patch code is required to be functional out of the box, even before the patch
is applied.  This is implemented by inserting functional (but inefficient)
load code into the .runtime.patch.code init section.  Having functional code
out of the box then allows us to defer the init time patch application until
later in the init sequence.

In addition to fitting better with our need for physical address-space
switch-over, this implementation should be somewhat more extensible by virtue
of its more readable (and hackable) C implementation.  This should prove
useful for other similar init time specialization needs, especially in light
of our multi-platform kernel initiative.

This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
(Cortex-A8) device.

Note: the obtuse use of stringified symbols in patch_stub() and
early_patch_stub() is intentional.  Theoretically this should have been
accomplished with formal operands passed into the asm block, but this requires
the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
However, the 'c' modifier has been found to ICE certain versions of GCC, and
therefore we resort to stringified symbols here.

Signed-off-by: Cyril Chemparathy 
Reviewed-by: Nicolas Pitre 
---
 arch/arm/Kconfig |3 +
 arch/arm/include/asm/module.h|7 ++
 arch/arm/include/asm/runtime-patch.h |  208 ++
 arch/arm/kernel/Makefile |1 +
 arch/arm/kernel/module.c |9 +-
 arch/arm/kernel/runtime-patch.c  |  193 +++
 arch/arm/kernel/setup.c  |3 +
 arch/arm/kernel/vmlinux.lds.S|   10 ++
 8 files changed, 433 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/asm/runtime-patch.h
 create mode 100644 arch/arm/kernel/runtime-patch.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2f88d8d..98a3a1a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -60,6 +60,9 @@ config ARM
 config ARM_HAS_SG_CHAIN
bool
 
+config ARM_RUNTIME_PATCH
+   bool
+
 config NEED_SG_DMA_LENGTH
bool
 
diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index 6c6809f..2090486 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -43,9 +43,16 @@ struct mod_arch_specific {
 #define MODULE_ARCH_VERMAGIC_ARMTHUMB ""
 #endif
 
+#ifdef CONFIG_ARM_RUNTIME_PATCH
+#define MODULE_ARCH_VERMAGIC_RT_PATCH "rt-patch "
+#else
+#define MODULE_ARCH_VERMAGIC_RT_PATCH ""
+#endif
+
 #define MODULE_ARCH_VERMAGIC \
MODULE_ARCH_VERMAGIC_ARMVSN \
MODULE_ARCH_VERMAGIC_ARMTHUMB \
+   MODULE_ARCH_VERMAGIC_RT_PATCH \
MODULE_ARCH_VERMAGIC_P2V
 
 #endif /* _ASM_ARM_MODULE_H */
diff --git a/arch/arm/include/asm/runtime-patch.h 
b/arch/arm/include/asm/runtime-patch.h
new file mode 100644
index 000..366444d
--- /dev/null
+++ b/arch/arm/include/asm/runtime-patch.h
@@ -0,0 +1,208 @@
+/*
+ * arch/arm/include/asm/runtime-patch.h
+ * Note: this file should not be included by non-asm/.h files
+ *
+ * Copyright 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 .
+ */
+#ifndef __ASM_ARM_RUNTIME_PATCH_H
+#define __ASM_ARM_RUNTIME_PATCH_H
+
+#include 
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_ARM_RUNTIME_PATCH
+
+struct patch_info {
+   void*insn;
+   u16  type;
+   u8   insn_size;
+   u8   data_size;
+   u32  data[0];
+};
+
+#define PATCH_IMM8 0x0001
+struct patch_info_imm8 {
+   u32 *imm;
+   u32  insn;
+};
+
+#define patch_next(p)  ((void *)(p) + sizeof(*(p)) + (p)->data_size)
+#define patch_data(p)  ((void *)&(p)->data[0])
+
+#define patch_stub(type, code, patch_data, ...)
\
+   __asm__("@ patch stub\n"\
+   "1:\n"  \
+

[PATCH v3 RESEND 01/17] ARM: add mechanism for late code patching

2012-09-21 Thread Cyril Chemparathy 
The original phys_to_virt/virt_to_phys patching implementation relied on early
patching prior to MMU initialization.  On PAE systems running out of 4G
address space, this would have entailed an additional round of patching after
switching over to the high address space.

The approach implemented here conceptually extends the original PHYS_OFFSET
patching implementation with the introduction of early patch stubs.  Early
patch code is required to be functional out of the box, even before the patch
is applied.  This is implemented by inserting functional (but inefficient)
load code into the .runtime.patch.code init section.  Having functional code
out of the box then allows us to defer the init time patch application until
later in the init sequence.

In addition to fitting better with our need for physical address-space
switch-over, this implementation should be somewhat more extensible by virtue
of its more readable (and hackable) C implementation.  This should prove
useful for other similar init time specialization needs, especially in light
of our multi-platform kernel initiative.

This code has been boot tested in both ARM and Thumb-2 modes on an ARMv7
(Cortex-A8) device.

Note: the obtuse use of stringified symbols in patch_stub() and
early_patch_stub() is intentional.  Theoretically this should have been
accomplished with formal operands passed into the asm block, but this requires
the use of the 'c' modifier for instantiating the long (e.g. .long %c0).
However, the 'c' modifier has been found to ICE certain versions of GCC, and
therefore we resort to stringified symbols here.

Signed-off-by: Cyril Chemparathy cy...@ti.com
Reviewed-by: Nicolas Pitre n...@linaro.org
---
 arch/arm/Kconfig |3 +
 arch/arm/include/asm/module.h|7 ++
 arch/arm/include/asm/runtime-patch.h |  208 ++
 arch/arm/kernel/Makefile |1 +
 arch/arm/kernel/module.c |9 +-
 arch/arm/kernel/runtime-patch.c  |  193 +++
 arch/arm/kernel/setup.c  |3 +
 arch/arm/kernel/vmlinux.lds.S|   10 ++
 8 files changed, 433 insertions(+), 1 deletion(-)
 create mode 100644 arch/arm/include/asm/runtime-patch.h
 create mode 100644 arch/arm/kernel/runtime-patch.c

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 2f88d8d..98a3a1a 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -60,6 +60,9 @@ config ARM
 config ARM_HAS_SG_CHAIN
bool
 
+config ARM_RUNTIME_PATCH
+   bool
+
 config NEED_SG_DMA_LENGTH
bool
 
diff --git a/arch/arm/include/asm/module.h b/arch/arm/include/asm/module.h
index 6c6809f..2090486 100644
--- a/arch/arm/include/asm/module.h
+++ b/arch/arm/include/asm/module.h
@@ -43,9 +43,16 @@ struct mod_arch_specific {
 #define MODULE_ARCH_VERMAGIC_ARMTHUMB 
 #endif
 
+#ifdef CONFIG_ARM_RUNTIME_PATCH
+#define MODULE_ARCH_VERMAGIC_RT_PATCH rt-patch 
+#else
+#define MODULE_ARCH_VERMAGIC_RT_PATCH 
+#endif
+
 #define MODULE_ARCH_VERMAGIC \
MODULE_ARCH_VERMAGIC_ARMVSN \
MODULE_ARCH_VERMAGIC_ARMTHUMB \
+   MODULE_ARCH_VERMAGIC_RT_PATCH \
MODULE_ARCH_VERMAGIC_P2V
 
 #endif /* _ASM_ARM_MODULE_H */
diff --git a/arch/arm/include/asm/runtime-patch.h 
b/arch/arm/include/asm/runtime-patch.h
new file mode 100644
index 000..366444d
--- /dev/null
+++ b/arch/arm/include/asm/runtime-patch.h
@@ -0,0 +1,208 @@
+/*
+ * arch/arm/include/asm/runtime-patch.h
+ * Note: this file should not be included by non-asm/.h files
+ *
+ * Copyright 2012 Texas Instruments, Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope 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 http://www.gnu.org/licenses/.
+ */
+#ifndef __ASM_ARM_RUNTIME_PATCH_H
+#define __ASM_ARM_RUNTIME_PATCH_H
+
+#include linux/stringify.h
+
+#ifndef __ASSEMBLY__
+
+#ifdef CONFIG_ARM_RUNTIME_PATCH
+
+struct patch_info {
+   void*insn;
+   u16  type;
+   u8   insn_size;
+   u8   data_size;
+   u32  data[0];
+};
+
+#define PATCH_IMM8 0x0001
+struct patch_info_imm8 {
+   u32 *imm;
+   u32  insn;
+};
+
+#define patch_next(p)  ((void *)(p) + sizeof(*(p)) + (p)-data_size)
+#define patch_data(p)  ((void *)(p)-data[0])
+
+#define patch_stub(type, code, patch_data, ...)
\
+   __asm__(@ patch stub\n\
+   1:\n