[PATCH] binfmt_flat: minimum support for the Blackfin relocations

[PATCH] binfmt_flat: minimum support for the Blackfin relocations

[Bryan Wu]

From: Bernd Schmidt <bernd.schmidt@analog.com>

This just adds minimum support for the Blackfin relocations,
since we don't have enough space in each reloc. The idea
is to store a value with one relocation so that subsequent ones can
access it.

Signed-off-by: Bernd Schmidt <bernd.schmidt@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
Cc: David McCullough <davidm@snapgear.com>
Cc: Greg Ungerer <gerg@snapgear.com>
---
 fs/binfmt_flat.c             |    5 ++++-
 include/asm-h8300/flat.h     |    3 ++-
 include/asm-m32r/flat.h      |    3 ++-
 include/asm-m68knommu/flat.h |    3 ++-
 include/asm-sh/flat.h        |    3 ++-
 include/asm-v850/flat.h      |    4 +++-
 6 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 7b0265d..13b58f8 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 	 * __start to address 4 so that is okay).
 	 */
 	if (rev > OLD_FLAT_VERSION) {
+		unsigned long persistent = 0;
 		for (i=0; i < relocs; i++) {
 			unsigned long addr, relval;
 
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 			   relocated (of course, the address has to be
 			   relocated first).  */
 			relval = ntohl(reloc[i]);
+			if (flat_set_persistent (relval, &persistent))
+				continue;
 			addr = flat_get_relocate_addr(relval);
 			rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
 			if (rp == (unsigned long *)RELOC_FAILED) {
@@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 			}
 
 			/* Get the pointer's value.  */
-			addr = flat_get_addr_from_rp(rp, relval, flags);
+			addr = flat_get_addr_from_rp(rp, relval, flags, &persistent);
 			if (addr != 0) {
 				/*
 				 * Do the relocation.  PIC relocs in the data section are
diff --git a/include/asm-h8300/flat.h b/include/asm-h8300/flat.h
index c20eee7..2a87350 100644
--- a/include/asm-h8300/flat.h
+++ b/include/asm-h8300/flat.h
@@ -9,6 +9,7 @@
 #define	flat_argvp_envp_on_stack()		1
 #define	flat_old_ram_flag(flags)		1
 #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
+#define	flat_set_persistent(relval, p)		0
 
 /*
  * on the H8 a couple of the relocations have an instruction in the
@@ -18,7 +19,7 @@
  */
 
 #define	flat_get_relocate_addr(rel)		(rel)
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
         (get_unaligned(rp) & ((flags & FLAT_FLAG_GOTPIC) ? 0xffffffff: 0x00ffffff))
 #define flat_put_addr_at_rp(rp, addr, rel) \
 	put_unaligned (((*(char *)(rp)) << 24) | ((addr) & 0x00ffffff), rp)
diff --git a/include/asm-m32r/flat.h b/include/asm-m32r/flat.h
index 1b285f6..d851cf0 100644
--- a/include/asm-m32r/flat.h
+++ b/include/asm-m32r/flat.h
@@ -15,9 +15,10 @@
 #define	flat_stack_align(sp)		(*sp += (*sp & 3 ? (4 - (*sp & 3)): 0))
 #define	flat_argvp_envp_on_stack()		0
 #define	flat_old_ram_flag(flags)		(flags)
+#define	flat_set_persistent(relval, p)		0
 #define	flat_reloc_valid(reloc, size)		\
 	(((reloc) - textlen_for_m32r_lo16_data) <= (size))
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
 	m32r_flat_get_addr_from_rp(rp, relval, (text_len) )
 
 #define flat_put_addr_at_rp(rp, addr, relval) \
diff --git a/include/asm-m68knommu/flat.h b/include/asm-m68knommu/flat.h
index 2d836ed..814b517 100644
--- a/include/asm-m68knommu/flat.h
+++ b/include/asm-m68knommu/flat.h
@@ -9,8 +9,9 @@
 #define	flat_argvp_envp_on_stack()		1
 #define	flat_old_ram_flag(flags)		(flags)
 #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
-#define	flat_get_addr_from_rp(rp, relval, flags)	get_unaligned(rp)
+#define	flat_get_addr_from_rp(rp, relval, flags, p)	get_unaligned(rp)
 #define	flat_put_addr_at_rp(rp, val, relval)	put_unaligned(val,rp)
 #define	flat_get_relocate_addr(rel)		(rel)
+#define	flat_set_persistent(relval, p)		0
 
 #endif /* __M68KNOMMU_FLAT_H__ */
diff --git a/include/asm-sh/flat.h b/include/asm-sh/flat.h
index 0d5cc04..dc4f595 100644
--- a/include/asm-sh/flat.h
+++ b/include/asm-sh/flat.h
@@ -16,8 +16,9 @@
 #define	flat_argvp_envp_on_stack()		0
 #define	flat_old_ram_flag(flags)		(flags)
 #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
-#define	flat_get_addr_from_rp(rp, relval, flags)	get_unaligned(rp)
+#define	flat_get_addr_from_rp(rp, relval, flags, p)	get_unaligned(rp)
 #define	flat_put_addr_at_rp(rp, val, relval)	put_unaligned(val,rp)
 #define	flat_get_relocate_addr(rel)		(rel)
+#define	flat_set_persistent(relval, p)		0
 
 #endif /* __ASM_SH_FLAT_H */
diff --git a/include/asm-v850/flat.h b/include/asm-v850/flat.h
index 3888f59..17f0ea5 100644
--- a/include/asm-v850/flat.h
+++ b/include/asm-v850/flat.h
@@ -25,6 +25,7 @@
 #define	flat_stack_align(sp)		/* nothing needed */
 #define	flat_argvp_envp_on_stack()	0
 #define	flat_old_ram_flag(flags)	(flags)
+#define	flat_set_persistent(relval, p)	0
 
 /* We store the type of relocation in the top 4 bits of the `relval.' */
 
@@ -46,7 +47,8 @@ flat_get_relocate_addr (unsigned long relval)
    For the v850, RP should always be half-word aligned.  */
 static inline unsigned long flat_get_addr_from_rp (unsigned long *rp,
 						   unsigned long relval,
-						   unsigned long flags)
+						   unsigned long flags,
+						   unsigned long *persistent)
 {
 	short *srp = (short *)rp;
 
-- 
1.5.2

[PATCH] binfmt_flat: minimum support for the Blackfin relocations

[Bryan Wu]

From: Bernd Schmidt <bernd.schmidt@analog.com>

This just adds minimum support for the Blackfin relocations,
since we don't have enough space in each reloc. The idea
is to store a value with one relocation so that subsequent ones can
access it.

Actually, this patch is required for Blackfin. Currently if BINFMT_FLAT 
is enabled, git-tree kernel will fail to compile. Please consider to
accept it.

Signed-off-by: Bernd Schmidt <bernd.schmidt@analog.com>
Signed-off-by: Bryan Wu <bryan.wu@analog.com>
Cc: David McCullough <davidm@snapgear.com>
Cc: Greg Ungerer <gerg@snapgear.com>
---
 fs/binfmt_flat.c             |    5 ++++-
 include/asm-h8300/flat.h     |    3 ++-
 include/asm-m32r/flat.h      |    3 ++-
 include/asm-m68knommu/flat.h |    3 ++-
 include/asm-sh/flat.h        |    3 ++-
 include/asm-v850/flat.h      |    4 +++-
 6 files changed, 15 insertions(+), 6 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 7b0265d..13b58f8 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 	 * __start to address 4 so that is okay).
 	 */
 	if (rev > OLD_FLAT_VERSION) {
+		unsigned long persistent = 0;
 		for (i=0; i < relocs; i++) {
 			unsigned long addr, relval;
 
@@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
 			   relocated (of course, the address has to be
 			   relocated first).  */
 			relval = ntohl(reloc[i]);
+			if (flat_set_persistent (relval, &persistent))
+				continue;
 			addr = flat_get_relocate_addr(relval);
 			rp = (unsigned long *) calc_reloc(addr, libinfo, id, 1);
 			if (rp == (unsigned long *)RELOC_FAILED) {
@@ -757,7 +760,7 @@ static int load_flat_file(struct linux_binprm * bprm,
 			}
 
 			/* Get the pointer's value.  */
-			addr = flat_get_addr_from_rp(rp, relval, flags);
+			addr = flat_get_addr_from_rp(rp, relval, flags, &persistent);
 			if (addr != 0) {
 				/*
 				 * Do the relocation.  PIC relocs in the data section are
diff --git a/include/asm-h8300/flat.h b/include/asm-h8300/flat.h
index c20eee7..2a87350 100644
--- a/include/asm-h8300/flat.h
+++ b/include/asm-h8300/flat.h
@@ -9,6 +9,7 @@
 #define	flat_argvp_envp_on_stack()		1
 #define	flat_old_ram_flag(flags)		1
 #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
+#define	flat_set_persistent(relval, p)		0
 
 /*
  * on the H8 a couple of the relocations have an instruction in the
@@ -18,7 +19,7 @@
  */
 
 #define	flat_get_relocate_addr(rel)		(rel)
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
         (get_unaligned(rp) & ((flags & FLAT_FLAG_GOTPIC) ? 0xffffffff: 0x00ffffff))
 #define flat_put_addr_at_rp(rp, addr, rel) \
 	put_unaligned (((*(char *)(rp)) << 24) | ((addr) & 0x00ffffff), rp)
diff --git a/include/asm-m32r/flat.h b/include/asm-m32r/flat.h
index 1b285f6..d851cf0 100644
--- a/include/asm-m32r/flat.h
+++ b/include/asm-m32r/flat.h
@@ -15,9 +15,10 @@
 #define	flat_stack_align(sp)		(*sp += (*sp & 3 ? (4 - (*sp & 3)): 0))
 #define	flat_argvp_envp_on_stack()		0
 #define	flat_old_ram_flag(flags)		(flags)
+#define	flat_set_persistent(relval, p)		0
 #define	flat_reloc_valid(reloc, size)		\
 	(((reloc) - textlen_for_m32r_lo16_data) <= (size))
-#define flat_get_addr_from_rp(rp, relval, flags) \
+#define flat_get_addr_from_rp(rp, relval, flags, persistent) \
 	m32r_flat_get_addr_from_rp(rp, relval, (text_len) )
 
 #define flat_put_addr_at_rp(rp, addr, relval) \
diff --git a/include/asm-m68knommu/flat.h b/include/asm-m68knommu/flat.h
index 2d836ed..814b517 100644
--- a/include/asm-m68knommu/flat.h
+++ b/include/asm-m68knommu/flat.h
@@ -9,8 +9,9 @@
 #define	flat_argvp_envp_on_stack()		1
 #define	flat_old_ram_flag(flags)		(flags)
 #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
-#define	flat_get_addr_from_rp(rp, relval, flags)	get_unaligned(rp)
+#define	flat_get_addr_from_rp(rp, relval, flags, p)	get_unaligned(rp)
 #define	flat_put_addr_at_rp(rp, val, relval)	put_unaligned(val,rp)
 #define	flat_get_relocate_addr(rel)		(rel)
+#define	flat_set_persistent(relval, p)		0
 
 #endif /* __M68KNOMMU_FLAT_H__ */
diff --git a/include/asm-sh/flat.h b/include/asm-sh/flat.h
index 0d5cc04..dc4f595 100644
--- a/include/asm-sh/flat.h
+++ b/include/asm-sh/flat.h
@@ -16,8 +16,9 @@
 #define	flat_argvp_envp_on_stack()		0
 #define	flat_old_ram_flag(flags)		(flags)
 #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
-#define	flat_get_addr_from_rp(rp, relval, flags)	get_unaligned(rp)
+#define	flat_get_addr_from_rp(rp, relval, flags, p)	get_unaligned(rp)
 #define	flat_put_addr_at_rp(rp, val, relval)	put_unaligned(val,rp)
 #define	flat_get_relocate_addr(rel)		(rel)
+#define	flat_set_persistent(relval, p)		0
 
 #endif /* __ASM_SH_FLAT_H */
diff --git a/include/asm-v850/flat.h b/include/asm-v850/flat.h
index 3888f59..17f0ea5 100644
--- a/include/asm-v850/flat.h
+++ b/include/asm-v850/flat.h
@@ -25,6 +25,7 @@
 #define	flat_stack_align(sp)		/* nothing needed */
 #define	flat_argvp_envp_on_stack()	0
 #define	flat_old_ram_flag(flags)	(flags)
+#define	flat_set_persistent(relval, p)	0
 
 /* We store the type of relocation in the top 4 bits of the `relval.' */
 
@@ -46,7 +47,8 @@ flat_get_relocate_addr (unsigned long relval)
    For the v850, RP should always be half-word aligned.  */
 static inline unsigned long flat_get_addr_from_rp (unsigned long *rp,
 						   unsigned long relval,
-						   unsigned long flags)
+						   unsigned long flags,
+						   unsigned long *persistent)
 {
 	short *srp = (short *)rp;
 
-- 
1.5.2

Re: [PATCH] binfmt_flat: minimum support for the Blackfin relocations

[Andrew Morton]

On Tue, 18 Sep 2007 16:09:25 +0800
Bryan Wu <bryan.wu@analog.com> wrote:

> From: Bernd Schmidt <bernd.schmidt@analog.com>
> 
> This just adds minimum support for the Blackfin relocations,
> since we don't have enough space in each reloc. The idea
> is to store a value with one relocation so that subsequent ones can
> access it.
> 
> Actually, this patch is required for Blackfin. Currently if BINFMT_FLAT 
> is enabled, git-tree kernel will fail to compile. Please consider to
> accept it.
> 

A few things

> diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
> index 7b0265d..13b58f8 100644
> --- a/fs/binfmt_flat.c
> +++ b/fs/binfmt_flat.c
> @@ -742,6 +742,7 @@ static int load_flat_file(struct linux_binprm * bprm,
>  	 * __start to address 4 so that is okay).
>  	 */
>  	if (rev > OLD_FLAT_VERSION) {
> +		unsigned long persistent = 0;

`persistent' here only has meaning inside the next nesting level, so should
be moved down into that scope for readability reasons.

Also, the initialisation to zero is, afaict, unneeded and wastes
instructions.  I suspect that's just there to suppress a gcc warning, which
is better done with uninitialized_var().

>  		for (i=0; i < relocs; i++) {
>  			unsigned long addr, relval;
>  
> @@ -749,6 +750,8 @@ static int load_flat_file(struct linux_binprm * bprm,
>  			   relocated (of course, the address has to be
>  			   relocated first).  */
>  			relval = ntohl(reloc[i]);
> +			if (flat_set_persistent (relval, &persistent))
> +				continue;

If this correct?  flat_set_persistent() returns zero if it didn't write
anything to `persistent'.  It seems strange that in the case where
flat_set_persistent() _does_ write something to `persistent', we just throw
it away by doing `continue'.

Either that, or I've misread the code and you really did mean to put
`persistent' in the outer scope, and its value is supposed to propagate
over into the next iteration of the loop.  If so, that's all a bit too
tricky for it to be implemented with zero code comments, dontcha think?

Also, given that you are proposing that flat_set_persistent() becomes part
of the kernel's core<->arch interface (for all architectures which want to
implement binfmt_flat()), it is no longer appropriate that the reference
implementation of flat_set_persistent() (ie: blackfins's implementation) be
completely undocumented.  IMO.

> --- a/include/asm-h8300/flat.h
> +++ b/include/asm-h8300/flat.h
> @@ -9,6 +9,7 @@
>  #define	flat_argvp_envp_on_stack()		1
>  #define	flat_old_ram_flag(flags)		1
>  #define	flat_reloc_valid(reloc, size)		((reloc) <= (size))
> +#define	flat_set_persistent(relval, p)		0

ug.  those macros are crap.  who did that.  They will generate compiler
warnings and runtime failures in a whole host of well-known scenarios.  

My kingdom to the person who invents a keyboard which delivers 12 kV when
it detects the sequence # d e f i n e.  There is no reason why these
"functions" need to be implemented as crappy #defines and converting them
to C will fix bug, warnings and will clean stuff up.

Sigh.

Re: [PATCH] binfmt_flat: minimum support for the Blackfin relocations

[Bernd Schmidt]

Andrew Morton wrote:
>>  	if (rev > OLD_FLAT_VERSION) {
>> +		unsigned long persistent = 0;
> 
> `persistent' here only has meaning inside the next nesting level, so should
> be moved down into that scope for readability reasons.

See below.

>> +			if (flat_set_persistent (relval, &persistent))
>> +				continue;
> 
> If this correct?  flat_set_persistent() returns zero if it didn't write
> anything to `persistent'.  It seems strange that in the case where
> flat_set_persistent() _does_ write something to `persistent', we just throw
> it away by doing `continue'.
> 
> Either that, or I've misread the code and you really did mean to put
> `persistent' in the outer scope, and its value is supposed to propagate
> over into the next iteration of the loop.  If so, that's all a bit too
> tricky for it to be implemented with zero code comments, dontcha think?

The latter.  We need to be able to use more data than we can fit into a 
single reloc, so we store a value with one reloc and reuse it with the 
next.  There'd be no point in having this function otherwise since you 
could perform whatever needs to be done in flat_get_relocate_addr.

This seemed fairly obvious at the time... when you're familiar with the 
flat format, the loop isn't all that hard to understand.  I'll add 
comments in the next version.


Bernd
-- 
This footer brought to you by insane German lawmakers.
Analog Devices GmbH      Wilhelm-Wagenfeld-Str. 6      80807 Muenchen
Sitz der Gesellschaft Muenchen, Registergericht Muenchen HRB 40368
Geschaeftsfuehrer Thomas Wessel, William A. Martin, Margaret Seif