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

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

On Fri, 21 Sep 2012, Cyril Chemparathy wrote:

quoted

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 <redacted>
Reviewed-by: Nicolas Pitre <redacted>

There is another problem with this.

[...]

quoted

diff --git a/arch/arm/include/asm/runtime-patch.h b/arch/arm/include/asm/runtime-patch.h
new file mode 100644
index 0000000..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.