[Mesa-dev] [PATCH 2/8] amd/rtld: update the ELF representation of LDS symbols

[Bas Nieuwenhuizen]

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 at gmail.com> wrote:
>
> From: Nicolai Hähnle <nicolai.haehnle at 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 at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev