[2/8] amd/rtld: update the ELF representation of LDS symbols

Submitted by Marek Olšák on June 20, 2019, 4:19 a.m.

Details

Message ID 20190620041941.14001-2-maraeo@gmail.com
State New
Headers show
Series "Series without cover letter" ( rev: 1 ) in Mesa

Not browsing as part of any series.

Commit Message

Marek Olšák June 20, 2019, 4:19 a.m.
From: Nicolai Hähnle <nicolai.haehnle@amd.com>

The initial prototype used a processor-specific symbol type, but
feedback suggests that an approach using processor-specific section
name that encodes the alignment analogous to SHN_COMMON symbols is
preferred.

This patch keeps both variants around for now to reduce problems
with LLVM compatibility as we switch branches around.

This also cleans up the error reporting in this function.
---
 src/amd/common/ac_rtld.c | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

Patch hide | download patch | download mbox

diff --git a/src/amd/common/ac_rtld.c b/src/amd/common/ac_rtld.c
index 57d6b0151b4..ebf64d91658 100644
--- a/src/amd/common/ac_rtld.c
+++ b/src/amd/common/ac_rtld.c
@@ -32,21 +32,25 @@ 
 
 #include "ac_binary.h"
 #include "ac_gpu_info.h"
 #include "util/u_dynarray.h"
 #include "util/u_math.h"
 
 // Old distributions may not have this enum constant
 #define MY_EM_AMDGPU 224
 
 #ifndef STT_AMDGPU_LDS
-#define STT_AMDGPU_LDS 13
+#define STT_AMDGPU_LDS 13 // this is deprecated -- remove
+#endif
+
+#ifndef SHN_AMDGPU_LDS
+#define SHN_AMDGPU_LDS 0xff00
 #endif
 
 #ifndef R_AMDGPU_NONE
 #define R_AMDGPU_NONE 0
 #define R_AMDGPU_ABS32_LO 1
 #define R_AMDGPU_ABS32_HI 2
 #define R_AMDGPU_ABS64 3
 #define R_AMDGPU_REL32 4
 #define R_AMDGPU_REL64 5
 #define R_AMDGPU_ABS32 6
@@ -169,47 +173,60 @@  static bool layout_symbols(struct ac_rtld_symbol *symbols, unsigned num_symbols,
  * Read LDS symbols from the given \p section of the ELF of \p part and append
  * them to the LDS symbols list.
  *
  * Shared LDS symbols are filtered out.
  */
 static bool read_private_lds_symbols(struct ac_rtld_binary *binary,
 				     unsigned part_idx,
 				     Elf_Scn *section,
 				     uint32_t *lds_end_align)
 {
-#define report_elf_if(cond) \
+#define report_if(cond) \
 	do { \
 		if ((cond)) { \
 			report_errorf(#cond); \
 			return false; \
 		} \
 	} while (false)
+#define report_elf_if(cond) \
+	do { \
+		if ((cond)) { \
+			report_elf_errorf(#cond); \
+			return false; \
+		} \
+	} while (false)
 
 	struct ac_rtld_part *part = &binary->parts[part_idx];
 	Elf64_Shdr *shdr = elf64_getshdr(section);
 	uint32_t strtabidx = shdr->sh_link;
 	Elf_Data *symbols_data = elf_getdata(section, NULL);
 	report_elf_if(!symbols_data);
 
 	const Elf64_Sym *symbol = symbols_data->d_buf;
 	size_t num_symbols = symbols_data->d_size / sizeof(Elf64_Sym);
 
 	for (size_t j = 0; j < num_symbols; ++j, ++symbol) {
-		if (ELF64_ST_TYPE(symbol->st_info) != STT_AMDGPU_LDS)
+		struct ac_rtld_symbol s = {};
+
+		if (ELF64_ST_TYPE(symbol->st_info) == STT_AMDGPU_LDS) {
+			/* old-style LDS symbols from initial prototype -- remove eventually */
+			s.align = MIN2(1u << (symbol->st_other >> 3), 1u << 16);
+		} else if (symbol->st_shndx == SHN_AMDGPU_LDS) {
+			s.align = MIN2(symbol->st_value, 1u << 16);
+			report_if(!util_is_power_of_two_nonzero(s.align));
+		} else
 			continue;
 
-		report_elf_if(symbol->st_size > 1u << 29);
+		report_if(symbol->st_size > 1u << 29);
 
-		struct ac_rtld_symbol s = {};
 		s.name = elf_strptr(part->elf, strtabidx, symbol->st_name);
 		s.size = symbol->st_size;
-		s.align = MIN2(1u << (symbol->st_other >> 3), 1u << 16);
 		s.part_idx = part_idx;
 
 		if (!strcmp(s.name, "__lds_end")) {
 			report_elf_if(s.size != 0);
 			*lds_end_align = MAX2(*lds_end_align, s.align);
 			continue;
 		}
 
 		const struct ac_rtld_symbol *shared =
 			find_symbol(&binary->lds_symbols, s.name, part_idx);
@@ -217,20 +234,21 @@  static bool read_private_lds_symbols(struct ac_rtld_binary *binary,
 			report_elf_if(s.align > shared->align);
 			report_elf_if(s.size > shared->size);
 			continue;
 		}
 
 		util_dynarray_append(&binary->lds_symbols, struct ac_rtld_symbol, s);
 	}
 
 	return true;
 
+#undef report_if
 #undef report_elf_if
 }
 
 /**
  * Open a binary consisting of one or more shader parts.
  *
  * \param binary the uninitialized struct
  * \param i binary opening parameters
  */
 bool ac_rtld_open(struct ac_rtld_binary *binary,
@@ -515,21 +533,23 @@  bool ac_rtld_read_config(struct ac_rtld_binary *binary,
 		config->rsrc2 = c.rsrc2;
 	}
 
 	return true;
 }
 
 static bool resolve_symbol(const struct ac_rtld_upload_info *u,
 			   unsigned part_idx, const Elf64_Sym *sym,
 			   const char *name, uint64_t *value)
 {
-	if (sym->st_shndx == SHN_UNDEF) {
+	/* TODO: properly disentangle the undef and the LDS cases once
+	 * STT_AMDGPU_LDS is retired. */
+	if (sym->st_shndx == SHN_UNDEF || sym->st_shndx == SHN_AMDGPU_LDS) {
 		const struct ac_rtld_symbol *lds_sym =
 			find_symbol(&u->binary->lds_symbols, name, part_idx);
 
 		if (lds_sym) {
 			*value = lds_sym->offset;
 			return true;
 		}
 
 		/* TODO: resolve from other parts */
 

Comments

Marek, I thought you also r-b'd this?

Either way r-b.

On Thu, Jun 20, 2019 at 6:20 AM Marek Olšák <maraeo@gmail.com> wrote:
>
> From: Nicolai Hähnle <nicolai.haehnle@amd.com>
>
> The initial prototype used a processor-specific symbol type, but
> feedback suggests that an approach using processor-specific section
> name that encodes the alignment analogous to SHN_COMMON symbols is
> preferred.
>
> This patch keeps both variants around for now to reduce problems
> with LLVM compatibility as we switch branches around.
>
> This also cleans up the error reporting in this function.
> ---
>  src/amd/common/ac_rtld.c | 34 +++++++++++++++++++++++++++-------
>  1 file changed, 27 insertions(+), 7 deletions(-)
>
> diff --git a/src/amd/common/ac_rtld.c b/src/amd/common/ac_rtld.c
> index 57d6b0151b4..ebf64d91658 100644
> --- a/src/amd/common/ac_rtld.c
> +++ b/src/amd/common/ac_rtld.c
> @@ -32,21 +32,25 @@
>
>  #include "ac_binary.h"
>  #include "ac_gpu_info.h"
>  #include "util/u_dynarray.h"
>  #include "util/u_math.h"
>
>  // Old distributions may not have this enum constant
>  #define MY_EM_AMDGPU 224
>
>  #ifndef STT_AMDGPU_LDS
> -#define STT_AMDGPU_LDS 13
> +#define STT_AMDGPU_LDS 13 // this is deprecated -- remove
> +#endif
> +
> +#ifndef SHN_AMDGPU_LDS
> +#define SHN_AMDGPU_LDS 0xff00
>  #endif
>
>  #ifndef R_AMDGPU_NONE
>  #define R_AMDGPU_NONE 0
>  #define R_AMDGPU_ABS32_LO 1
>  #define R_AMDGPU_ABS32_HI 2
>  #define R_AMDGPU_ABS64 3
>  #define R_AMDGPU_REL32 4
>  #define R_AMDGPU_REL64 5
>  #define R_AMDGPU_ABS32 6
> @@ -169,47 +173,60 @@ static bool layout_symbols(struct ac_rtld_symbol *symbols, unsigned num_symbols,
>   * Read LDS symbols from the given \p section of the ELF of \p part and append
>   * them to the LDS symbols list.
>   *
>   * Shared LDS symbols are filtered out.
>   */
>  static bool read_private_lds_symbols(struct ac_rtld_binary *binary,
>                                      unsigned part_idx,
>                                      Elf_Scn *section,
>                                      uint32_t *lds_end_align)
>  {
> -#define report_elf_if(cond) \
> +#define report_if(cond) \
>         do { \
>                 if ((cond)) { \
>                         report_errorf(#cond); \
>                         return false; \
>                 } \
>         } while (false)
> +#define report_elf_if(cond) \
> +       do { \
> +               if ((cond)) { \
> +                       report_elf_errorf(#cond); \
> +                       return false; \
> +               } \
> +       } while (false)
>
>         struct ac_rtld_part *part = &binary->parts[part_idx];
>         Elf64_Shdr *shdr = elf64_getshdr(section);
>         uint32_t strtabidx = shdr->sh_link;
>         Elf_Data *symbols_data = elf_getdata(section, NULL);
>         report_elf_if(!symbols_data);
>
>         const Elf64_Sym *symbol = symbols_data->d_buf;
>         size_t num_symbols = symbols_data->d_size / sizeof(Elf64_Sym);
>
>         for (size_t j = 0; j < num_symbols; ++j, ++symbol) {
> -               if (ELF64_ST_TYPE(symbol->st_info) != STT_AMDGPU_LDS)
> +               struct ac_rtld_symbol s = {};
> +
> +               if (ELF64_ST_TYPE(symbol->st_info) == STT_AMDGPU_LDS) {
> +                       /* old-style LDS symbols from initial prototype -- remove eventually */
> +                       s.align = MIN2(1u << (symbol->st_other >> 3), 1u << 16);
> +               } else if (symbol->st_shndx == SHN_AMDGPU_LDS) {
> +                       s.align = MIN2(symbol->st_value, 1u << 16);
> +                       report_if(!util_is_power_of_two_nonzero(s.align));
> +               } else
>                         continue;
>
> -               report_elf_if(symbol->st_size > 1u << 29);
> +               report_if(symbol->st_size > 1u << 29);
>
> -               struct ac_rtld_symbol s = {};
>                 s.name = elf_strptr(part->elf, strtabidx, symbol->st_name);
>                 s.size = symbol->st_size;
> -               s.align = MIN2(1u << (symbol->st_other >> 3), 1u << 16);
>                 s.part_idx = part_idx;
>
>                 if (!strcmp(s.name, "__lds_end")) {
>                         report_elf_if(s.size != 0);
>                         *lds_end_align = MAX2(*lds_end_align, s.align);
>                         continue;
>                 }
>
>                 const struct ac_rtld_symbol *shared =
>                         find_symbol(&binary->lds_symbols, s.name, part_idx);
> @@ -217,20 +234,21 @@ static bool read_private_lds_symbols(struct ac_rtld_binary *binary,
>                         report_elf_if(s.align > shared->align);
>                         report_elf_if(s.size > shared->size);
>                         continue;
>                 }
>
>                 util_dynarray_append(&binary->lds_symbols, struct ac_rtld_symbol, s);
>         }
>
>         return true;
>
> +#undef report_if
>  #undef report_elf_if
>  }
>
>  /**
>   * Open a binary consisting of one or more shader parts.
>   *
>   * \param binary the uninitialized struct
>   * \param i binary opening parameters
>   */
>  bool ac_rtld_open(struct ac_rtld_binary *binary,
> @@ -515,21 +533,23 @@ bool ac_rtld_read_config(struct ac_rtld_binary *binary,
>                 config->rsrc2 = c.rsrc2;
>         }
>
>         return true;
>  }
>
>  static bool resolve_symbol(const struct ac_rtld_upload_info *u,
>                            unsigned part_idx, const Elf64_Sym *sym,
>                            const char *name, uint64_t *value)
>  {
> -       if (sym->st_shndx == SHN_UNDEF) {
> +       /* TODO: properly disentangle the undef and the LDS cases once
> +        * STT_AMDGPU_LDS is retired. */
> +       if (sym->st_shndx == SHN_UNDEF || sym->st_shndx == SHN_AMDGPU_LDS) {
>                 const struct ac_rtld_symbol *lds_sym =
>                         find_symbol(&u->binary->lds_symbols, name, part_idx);
>
>                 if (lds_sym) {
>                         *value = lds_sym->offset;
>                         return true;
>                 }
>
>                 /* TODO: resolve from other parts */
>
> --
> 2.17.1
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev