Re: [PATCH 12/49] image: Create a function to do manual relocation
Hi Rasmus,
On Tue, 4 May 2021 at 01:17, Rasmus Villemoes
wrote:
>
> On 04/05/2021 01.10, Simon Glass wrote:
> > Rather than adding an #ifdef and open-coding this calculation, add a
> > helper function to handle it. Use this in the image code.
> >
> > Signed-off-by: Simon Glass
> > ---
> >
> > (no changes since v1)
> >
> > common/image.c | 33 +++--
> > include/relocate.h | 24 +++-
> > 2 files changed, 30 insertions(+), 27 deletions(-)
> >
> > diff --git a/common/image.c b/common/image.c
> > index 0ef8f30fcfa..086ae609f29 100644
> > --- a/common/image.c
> > +++ b/common/image.c
> > @@ -63,6 +63,7 @@ DECLARE_GLOBAL_DATA_PTR;
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> > #include
> > @@ -563,11 +564,7 @@ const char *genimg_get_cat_name(enum ih_category
> > category, uint id)
> > entry = get_table_entry(table_info[category].table, id);
> > if (!entry)
> > return unknown_msg(category);
> > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> > - return entry->lname;
> > -#else
> > - return entry->lname + gd->reloc_off;
> > -#endif
> > + return manual_reloc(entry->lname);
> > }
> >
> > /**
> > @@ -587,11 +584,7 @@ const char *genimg_get_cat_short_name(enum ih_category
> > category, uint id)
> > entry = get_table_entry(table_info[category].table, id);
> > if (!entry)
> > return unknown_msg(category);
> > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> > - return entry->sname;
> > -#else
> > - return entry->sname + gd->reloc_off;
> > -#endif
> > + return manual_reloc(entry->sname);
> > }
> >
> > int genimg_get_cat_count(enum ih_category category)
> > @@ -641,11 +634,7 @@ char *get_table_entry_name(const table_entry_t *table,
> > char *msg, int id)
> > table = get_table_entry(table, id);
> > if (!table)
> > return msg;
> > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> > - return table->lname;
> > -#else
> > - return table->lname + gd->reloc_off;
> > -#endif
> > + return manual_reloc(table->lname);
> > }
> >
> > const char *genimg_get_os_name(uint8_t os)
> > @@ -675,11 +664,7 @@ static const char *genimg_get_short_name(const
> > table_entry_t *table, int val)
> > table = get_table_entry(table, val);
> > if (!table)
> > return "unknown";
> > -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> > - return table->sname;
> > -#else
> > - return table->sname + gd->reloc_off;
> > -#endif
> > + return manual_reloc(table->sname);
> > }
> >
> > const char *genimg_get_type_short_name(uint8_t type)
> > @@ -722,12 +707,8 @@ int get_table_entry_id(const table_entry_t *table,
> > const table_entry_t *t;
> >
> > for (t = table; t->id >= 0; ++t) {
> > -#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC)
> > - if (t->sname && strcasecmp(t->sname + gd->reloc_off, name) ==
> > 0)
> > -#else
> > - if (t->sname && strcasecmp(t->sname, name) == 0)
> > -#endif
> > - return (t->id);
> > + if (t->sname && !strcasecmp(manual_reloc(t->sname), name))
> > + return t->id;
> > }
> > debug("Invalid %s Type: %s\n", table_name, name);
> >
> > diff --git a/include/relocate.h b/include/relocate.h
> > index 9ceeecdbe71..c4fad336128 100644
> > --- a/include/relocate.h
> > +++ b/include/relocate.h
> > @@ -7,7 +7,11 @@
> > #ifndef _RELOCATE_H_
> > #define _RELOCATE_H_
> >
> > -#include
> > +#ifndef USE_HOSTCC
> > +#include
> > +
> > +DECLARE_GLOBAL_DATA_PTR;
> > +#endif
> >
> > /**
> > * copy_uboot_to_ram() - Copy U-Boot to its new relocated position
> > @@ -35,4 +39,22 @@ int clear_bss(void);
> > */
> > int do_elf_reloc_fixups(void);
> >
> > +/**
> > + * manual_reloc() - Manually relocate a pointer if needed
> > + *
> > + * This is a nop in almost all cases, except for the systems with a broken
> > gcc
> > + * which need to manually relocate some things.
> > + *
> > + * @ptr: Pointer to relocate
> > + * @return new pointer value
> > + */
> > +static inline void *manual_reloc(void *ptr)
> > +{
> > +#ifndef USE_HOSTCC
> > + if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC))
> > + return ptr + gd->reloc_off;
> > +#endif
> > + return ptr;
> > +}
>
> static inlines are preferred in most cases, but you could make this
> const-correct if it was a macro. Dunno, maybe there are no
> const-qualified pointers where this might be used.
I can see what we would want that, but I haven't seen those used yet.
>
> The function also has the advantage of ensuring that the arithmetic is
> done on a void* pointer (i.e. effectively sizeof==1), though if the
> pointer is actually already a u32*, say, the current manual relocation
> code wouldn't be doing a bare ptr+gd->reloc_off anyway.
Right.
Regards,
Re: [PATCH 12/49] image: Create a function to do manual relocation
On 04/05/2021 01.10, Simon Glass wrote:
> Rather than adding an #ifdef and open-coding this calculation, add a
> helper function to handle it. Use this in the image code.
>
> Signed-off-by: Simon Glass
> ---
>
> (no changes since v1)
>
> common/image.c | 33 +++--
> include/relocate.h | 24 +++-
> 2 files changed, 30 insertions(+), 27 deletions(-)
>
> diff --git a/common/image.c b/common/image.c
> index 0ef8f30fcfa..086ae609f29 100644
> --- a/common/image.c
> +++ b/common/image.c
> @@ -63,6 +63,7 @@ DECLARE_GLOBAL_DATA_PTR;
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> @@ -563,11 +564,7 @@ const char *genimg_get_cat_name(enum ih_category
> category, uint id)
> entry = get_table_entry(table_info[category].table, id);
> if (!entry)
> return unknown_msg(category);
> -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> - return entry->lname;
> -#else
> - return entry->lname + gd->reloc_off;
> -#endif
> + return manual_reloc(entry->lname);
> }
>
> /**
> @@ -587,11 +584,7 @@ const char *genimg_get_cat_short_name(enum ih_category
> category, uint id)
> entry = get_table_entry(table_info[category].table, id);
> if (!entry)
> return unknown_msg(category);
> -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> - return entry->sname;
> -#else
> - return entry->sname + gd->reloc_off;
> -#endif
> + return manual_reloc(entry->sname);
> }
>
> int genimg_get_cat_count(enum ih_category category)
> @@ -641,11 +634,7 @@ char *get_table_entry_name(const table_entry_t *table,
> char *msg, int id)
> table = get_table_entry(table, id);
> if (!table)
> return msg;
> -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> - return table->lname;
> -#else
> - return table->lname + gd->reloc_off;
> -#endif
> + return manual_reloc(table->lname);
> }
>
> const char *genimg_get_os_name(uint8_t os)
> @@ -675,11 +664,7 @@ static const char *genimg_get_short_name(const
> table_entry_t *table, int val)
> table = get_table_entry(table, val);
> if (!table)
> return "unknown";
> -#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
> - return table->sname;
> -#else
> - return table->sname + gd->reloc_off;
> -#endif
> + return manual_reloc(table->sname);
> }
>
> const char *genimg_get_type_short_name(uint8_t type)
> @@ -722,12 +707,8 @@ int get_table_entry_id(const table_entry_t *table,
> const table_entry_t *t;
>
> for (t = table; t->id >= 0; ++t) {
> -#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC)
> - if (t->sname && strcasecmp(t->sname + gd->reloc_off, name) == 0)
> -#else
> - if (t->sname && strcasecmp(t->sname, name) == 0)
> -#endif
> - return (t->id);
> + if (t->sname && !strcasecmp(manual_reloc(t->sname), name))
> + return t->id;
> }
> debug("Invalid %s Type: %s\n", table_name, name);
>
> diff --git a/include/relocate.h b/include/relocate.h
> index 9ceeecdbe71..c4fad336128 100644
> --- a/include/relocate.h
> +++ b/include/relocate.h
> @@ -7,7 +7,11 @@
> #ifndef _RELOCATE_H_
> #define _RELOCATE_H_
>
> -#include
> +#ifndef USE_HOSTCC
> +#include
> +
> +DECLARE_GLOBAL_DATA_PTR;
> +#endif
>
> /**
> * copy_uboot_to_ram() - Copy U-Boot to its new relocated position
> @@ -35,4 +39,22 @@ int clear_bss(void);
> */
> int do_elf_reloc_fixups(void);
>
> +/**
> + * manual_reloc() - Manually relocate a pointer if needed
> + *
> + * This is a nop in almost all cases, except for the systems with a broken
> gcc
> + * which need to manually relocate some things.
> + *
> + * @ptr: Pointer to relocate
> + * @return new pointer value
> + */
> +static inline void *manual_reloc(void *ptr)
> +{
> +#ifndef USE_HOSTCC
> + if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC))
> + return ptr + gd->reloc_off;
> +#endif
> + return ptr;
> +}
static inlines are preferred in most cases, but you could make this
const-correct if it was a macro. Dunno, maybe there are no
const-qualified pointers where this might be used.
The function also has the advantage of ensuring that the arithmetic is
done on a void* pointer (i.e. effectively sizeof==1), though if the
pointer is actually already a u32*, say, the current manual relocation
code wouldn't be doing a bare ptr+gd->reloc_off anyway.
Rasmus
[PATCH 12/49] image: Create a function to do manual relocation
Rather than adding an #ifdef and open-coding this calculation, add a
helper function to handle it. Use this in the image code.
Signed-off-by: Simon Glass
---
(no changes since v1)
common/image.c | 33 +++--
include/relocate.h | 24 +++-
2 files changed, 30 insertions(+), 27 deletions(-)
diff --git a/common/image.c b/common/image.c
index 0ef8f30fcfa..086ae609f29 100644
--- a/common/image.c
+++ b/common/image.c
@@ -63,6 +63,7 @@ DECLARE_GLOBAL_DATA_PTR;
#include
#include
#include
+#include
#include
#include
#include
@@ -563,11 +564,7 @@ const char *genimg_get_cat_name(enum ih_category category,
uint id)
entry = get_table_entry(table_info[category].table, id);
if (!entry)
return unknown_msg(category);
-#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
- return entry->lname;
-#else
- return entry->lname + gd->reloc_off;
-#endif
+ return manual_reloc(entry->lname);
}
/**
@@ -587,11 +584,7 @@ const char *genimg_get_cat_short_name(enum ih_category
category, uint id)
entry = get_table_entry(table_info[category].table, id);
if (!entry)
return unknown_msg(category);
-#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
- return entry->sname;
-#else
- return entry->sname + gd->reloc_off;
-#endif
+ return manual_reloc(entry->sname);
}
int genimg_get_cat_count(enum ih_category category)
@@ -641,11 +634,7 @@ char *get_table_entry_name(const table_entry_t *table,
char *msg, int id)
table = get_table_entry(table, id);
if (!table)
return msg;
-#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
- return table->lname;
-#else
- return table->lname + gd->reloc_off;
-#endif
+ return manual_reloc(table->lname);
}
const char *genimg_get_os_name(uint8_t os)
@@ -675,11 +664,7 @@ static const char *genimg_get_short_name(const
table_entry_t *table, int val)
table = get_table_entry(table, val);
if (!table)
return "unknown";
-#if defined(USE_HOSTCC) || !defined(CONFIG_NEEDS_MANUAL_RELOC)
- return table->sname;
-#else
- return table->sname + gd->reloc_off;
-#endif
+ return manual_reloc(table->sname);
}
const char *genimg_get_type_short_name(uint8_t type)
@@ -722,12 +707,8 @@ int get_table_entry_id(const table_entry_t *table,
const table_entry_t *t;
for (t = table; t->id >= 0; ++t) {
-#if !defined(USE_HOSTCC) && defined(CONFIG_NEEDS_MANUAL_RELOC)
- if (t->sname && strcasecmp(t->sname + gd->reloc_off, name) == 0)
-#else
- if (t->sname && strcasecmp(t->sname, name) == 0)
-#endif
- return (t->id);
+ if (t->sname && !strcasecmp(manual_reloc(t->sname), name))
+ return t->id;
}
debug("Invalid %s Type: %s\n", table_name, name);
diff --git a/include/relocate.h b/include/relocate.h
index 9ceeecdbe71..c4fad336128 100644
--- a/include/relocate.h
+++ b/include/relocate.h
@@ -7,7 +7,11 @@
#ifndef _RELOCATE_H_
#define _RELOCATE_H_
-#include
+#ifndef USE_HOSTCC
+#include
+
+DECLARE_GLOBAL_DATA_PTR;
+#endif
/**
* copy_uboot_to_ram() - Copy U-Boot to its new relocated position
@@ -35,4 +39,22 @@ int clear_bss(void);
*/
int do_elf_reloc_fixups(void);
+/**
+ * manual_reloc() - Manually relocate a pointer if needed
+ *
+ * This is a nop in almost all cases, except for the systems with a broken gcc
+ * which need to manually relocate some things.
+ *
+ * @ptr: Pointer to relocate
+ * @return new pointer value
+ */
+static inline void *manual_reloc(void *ptr)
+{
+#ifndef USE_HOSTCC
+ if (IS_ENABLED(CONFIG_NEEDS_MANUAL_RELOC))
+ return ptr + gd->reloc_off;
+#endif
+ return ptr;
+}
+
#endif /* _RELOCATE_H_ */
--
2.31.1.527.g47e6f16901-goog
