From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pf1-x431.google.com (mail-pf1-x431.google.com [IPv6:2607:f8b0:4864:20::431]) by sourceware.org (Postfix) with ESMTPS id 207C13858D37 for ; Thu, 20 Jan 2022 02:39:12 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 207C13858D37 Received: by mail-pf1-x431.google.com with SMTP id 78so4051341pfu.10 for ; Wed, 19 Jan 2022 18:39:12 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=x63VAgE/ToXk+RjPJD2xt02YOZCpUpp4dbVNTzJdxzI=; b=JdL0HUpFeWPbYpMLRZfGJ3h7mL8OaFmA046wkMabLWhQM/f0lGIzftrWEsvLJVGjGQ /49OjJvkGE1xShxWPORXovT2VFJOr7Ht4yPboUFKMZ/+HI/knlXFryM8JBFpfs+XLKUx ovUKguoEIukpI+X2M2HShT8NJoAQYqlntaV/PEgnJDkibvoDXhHlSBmPOYI5qZbU+LVx T8xgAVRTsug0JIp8aKDqYns4Dtij+E/Dr03gTlvY4dwUHoZ9DMAbuFnquCrz+98HDw3J BySdcUhLFrzb3pxm5eoCwu27z3w1ADGEH2tDOeNVAHL7CPDHC3j0bR7jFY3ncbmTrq+L y5jg== X-Gm-Message-State: AOAM530+Jx5mkn3/at/fGq8489MQNv/Tj3g3SQJQa+DhSk42UKBoRt9G VRNjcBWxas/Xadtv12TFjEYoHfWs4W26z40OvFR9hq7c X-Google-Smtp-Source: ABdhPJyfW6BrXbJnMMJc+HAo0Ne2MiuWb7xdsKrLDTepaC7xWqG9m56eOn/kuyt8ajQJP1otDTIyfYA8umR+jbj7o8k= X-Received: by 2002:a63:b24e:: with SMTP id t14mr30075564pgo.381.1642646351062; Wed, 19 Jan 2022 18:39:11 -0800 (PST) MIME-Version: 1.0 References: <20220118043159.27521-3-vincent.chen@sifive.com> In-Reply-To: From: "H.J. Lu" Date: Wed, 19 Jan 2022 18:38:35 -0800 Message-ID: Subject: Re: [PATCH v2 2/2] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. To: Palmer Dabbelt Cc: Vincent Chen , GNU C Library , Andrew Waterman , kai.wang@sifive.com, greentime.hu@sifive.com, kito.cheng@sifive.com Content-Type: text/plain; charset="UTF-8" X-Spam-Status: No, score=-3027.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, FREEMAIL_FROM, GIT_PATCH_0, KAM_SHORT, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 20 Jan 2022 02:39:14 -0000 On Wed, Jan 19, 2022 at 6:22 PM Palmer Dabbelt wrote: > > Sorry, I missed the fixed-up patch set (which is why I just sent out a > similar bit of documentation). > > On Mon, 17 Jan 2022 20:31:59 PST (-0800), vincent.chen@sifive.com wrote: > > From: Hsiangkai Wang > > > > In some cases, we do not want to go through the resolver for function > > calls. For example, functions with vector arguments will use vector > > registers to pass arguments. In the resolver, we do not save/restore the > > vector argument registers for lazy binding efficiency. To avoid ruining > > the vector arguments, functions with vector arguments will not go > > through the resolver. > > > > To achieve the goal, we will annotate the function symbols with > > STO_RISCV_VARIANT_CC flag and add DT_RISCV_VARIANT_CC tag in the dynamic > > section. In the first pass on PLT relocations, we do not set up to call > > _dl_runtime_resolve. Instead, we resolve the functions directly. > > > > Signed-off-by: Hsiangkai Wang > > Signed-off-by: Vincent Chen > > --- > > elf/elf.h | 7 +++++++ > > manual/platform.texi | 6 ++++++ > > sysdeps/riscv/dl-dtprocnum.h | 21 +++++++++++++++++++++ > > sysdeps/riscv/dl-machine.h | 26 ++++++++++++++++++++++++++ > > 4 files changed, 60 insertions(+) > > create mode 100644 sysdeps/riscv/dl-dtprocnum.h > > > > diff --git a/elf/elf.h b/elf/elf.h > > index 0735f6b579..9c95544050 100644 > > --- a/elf/elf.h > > +++ b/elf/elf.h > > @@ -3911,6 +3911,13 @@ enum > > > > #define R_TILEGX_NUM 130 > > > > +/* RISC-V specific values for the Dyn d_tag field. */ > > +#define DT_RISCV_VARIANT_CC (DT_LOPROC + 1) > > +#define DT_RISCV_NUM 2 > > + > > +/* RISC-V specific values for the st_other field. */ > > +#define STO_RISCV_VARIANT_CC 0x80 > > + > > /* RISC-V ELF Flags */ > > #define EF_RISCV_RVC 0x0001 > > #define EF_RISCV_FLOAT_ABI 0x0006 > > diff --git a/manual/platform.texi b/manual/platform.texi > > index d5fdc5bd05..a1a740f381 100644 > > --- a/manual/platform.texi > > +++ b/manual/platform.texi > > @@ -121,6 +121,12 @@ when it is not allowed, the priority is set to medium. > > @node RISC-V > > @appendixsec RISC-V-specific Facilities > > > > +Functions that are lazily bound must be compatible with the standard calling > > +convention. When a function is annotated with STO_RISCV_VARIANT_CC, it means > > +this function is not compatible with the standard calling convention. The > > +dynamic linker will directly resolve it instead of using the lazy binding > > +mechanism. > > IMO this is the wrong way to go: we're essentially re-defining a bit > used be the standard ABI to mean something else. I guess we've already > defacto forked from the psABI with that "standard calling convention" > language, but IMO it'd be prudent to use a different bit to represent > this new behavior. In the long term one could imagine trying to get > back in line with the psABI, but if we're repurposing two bit patterns > it'll be a bit harder than if we're just repurposing one. > > > + > > Cache management facilities specific to RISC-V systems that implement the Linux > > ABI are declared in @file{sys/cachectl.h}. > > > > diff --git a/sysdeps/riscv/dl-dtprocnum.h b/sysdeps/riscv/dl-dtprocnum.h > > new file mode 100644 > > index 0000000000..f189fd700a > > --- /dev/null > > +++ b/sysdeps/riscv/dl-dtprocnum.h > > @@ -0,0 +1,21 @@ > > +/* Configuration of lookup functions. RISC-V version. > > + Copyright (C) 2019-2021 Free Software Foundation, Inc. > > + This file is part of the GNU C Library. > > + > > + The GNU C Library is free software; you can redistribute it and/or > > + modify it under the terms of the GNU Lesser General Public > > + License as published by the Free Software Foundation; either > > + version 2.1 of the License, or (at your option) any later version. > > + > > + The GNU C Library is distributed in the hope that it will be useful, > > + but WITHOUT ANY WARRANTY; without even the implied warranty of > > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > > + Lesser General Public License for more details. > > + > > + You should have received a copy of the GNU Lesser General Public > > + License along with the GNU C Library. If not, see > > + . */ > > + > > +/* Number of extra dynamic section entries for this architecture. By > > + default there are none. */ > > +#define DT_THISPROCNUM DT_RISCV_NUM > > diff --git a/sysdeps/riscv/dl-machine.h b/sysdeps/riscv/dl-machine.h > > index 1d3e2e588c..cdbaca6533 100644 > > --- a/sysdeps/riscv/dl-machine.h > > +++ b/sysdeps/riscv/dl-machine.h > > @@ -53,6 +53,9 @@ > > || (__WORDSIZE == 64 && (type) == R_RISCV_TLS_TPREL64))) \ > > | (ELF_RTYPE_CLASS_COPY * ((type) == R_RISCV_COPY))) > > > > +//* Translate a processor specific dynamic tag to the index in l_info array. */ > > +#define DT_RISCV(x) (DT_RISCV_##x - DT_LOPROC + DT_NUM) > > + > > /* Return nonzero iff ELF header is compatible with the running host. */ > > static inline int __attribute_used__ > > elf_machine_matches_host (const ElfW(Ehdr) *ehdr) > > @@ -305,6 +308,29 @@ elf_machine_lazy_rel (struct link_map *map, struct r_scope_elem *scope[], > > /* Check for unexpected PLT reloc type. */ > > if (__glibc_likely (r_type == R_RISCV_JUMP_SLOT)) > > { > > + if (__glibc_unlikely (map->l_info[DT_RISCV (VARIANT_CC)] != NULL)) > > + { > > + /* Check the symbol table for variant CC symbols. */ > > + const Elf_Symndx symndx = ELFW(R_SYM) (reloc->r_info); > > + const ElfW(Sym) *symtab = > > + (const void *)D_PTR (map, l_info[DT_SYMTAB]); > > + const ElfW(Sym) *sym = &symtab[symndx]; > > + if (__glibc_unlikely (sym->st_other & STO_RISCV_VARIANT_CC)) > > + { > > + /* Avoid lazy resolution of variant CC symbols. */ > > + const struct r_found_version *version = NULL; > > + if (map->l_info[VERSYMIDX (DT_VERSYM)] != NULL) > > + { > > + const ElfW(Half) *vernum = > > + (const void *)D_PTR (map, l_info[VERSYMIDX (DT_VERSYM)]); > > + version = &map->l_versions[vernum[symndx] & 0x7fff]; > > + } > > + elf_machine_rela (map, scope, reloc, sym, version, reloc_addr, > > + skip_ifunc); > > + return; > > + } > > + } > > + > > if (__glibc_unlikely (map->l_mach.plt == 0)) > > { > > if (l_addr) > > Aside from that this one looks fine to me. > > Given the complexity around this psABI spec deviation and how close we > are to release I'd prefer to wait and see if we can come up with a > better solution, though -- for example, I'd been kicking around some > ideas related to ELF object attributes saying "this follows the > psABI-1.0" vs "this follows the legacy GNU psABI extensions". That way > we could at least tag binaries that explicitly rely on this new behavior > as such, which would give us a shot at eventually getting rid of them. If you want to go this route, I suggest you use GNU property for this. ld.so supports GNU property. -- H.J.