From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-ot1-x331.google.com (mail-ot1-x331.google.com [IPv6:2607:f8b0:4864:20::331]) by sourceware.org (Postfix) with ESMTPS id CDD7E385C409 for ; Thu, 2 Dec 2021 21:09:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org CDD7E385C409 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=sifive.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=sifive.com Received: by mail-ot1-x331.google.com with SMTP id 47-20020a9d0332000000b005798ac20d72so1336426otv.9 for ; Thu, 02 Dec 2021 13:09:52 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=sifive.com; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=FZo80TedjpkfxkOgczMD0TXmk5pKSNXxGQiu1SwP+N0=; b=XL3mv3iZp+VNZhCmAd1CttQ1dYO78osrrvz+pbgc+id3WeWHg01gFtSHsH9TQb8typ lgk/ep74wjGtu4/pruhW3e3ojYANPJvGqG50xD8tqyPLWojVTv3X6cF+dY10OJyKH+Xh WfMwif2NS89q7S1f/8mpD1b16U4/euX9bBtbvuv2KdRzLLxx+ANIJCRQncoT2vpHsMgM ZHa6OIKplMT8Ub0g7dHhTzNgg/96JuJkppa/hkWZiNn//L3omw6Dq+KEiIooPL4j3NF0 m88vLTNPiPwpz2gL8XgxuUfoqK07WoFePX8N/9TuXtd8WlN9oVFmDZAzBM+Ic4gQEi+2 Q4kQ== 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:content-transfer-encoding; bh=FZo80TedjpkfxkOgczMD0TXmk5pKSNXxGQiu1SwP+N0=; b=p+joWphdAYAP3V9xieIZ6dnu96tyCW6YYn4yvfNRTgFiW/CEJuGECSa1nbOUGmX+ff gP6QtTOxXiPbt3TgIfMbQRGu58SUyxkU9u3/AD9YvZ4NdmssVUlWUqh2c9fZb+OnzswM d+E3qLcqf4u/cw+Ty3YDhuFdqgVXaEUkqE7PJUM2VP+CbhXrmW9qE+RulO++4nW3Z+p3 gArW3a1vtson8Fvoq7BsdRt4PqsusHb2BBw5LPdrU3CO4ofWBqTkzIeSfaEbb8bfIH3B 6npzVPuFbcJk2SLAYSIeO8zNe2MpS9Ho6fsy2w7S6Um8sOqs/qjuHhfon1lnTwmBw5fS ICQA== X-Gm-Message-State: AOAM5333DAz99innbTMbuCTe9cMrEjPDYoFQTUF2dInaX4U9jkDp1YY0 E9j9+orFSUTO7T37sEoMUOgFUVP9aZBluQ== X-Google-Smtp-Source: ABdhPJzBKmW5cuu6PwF1i56vSuuCYsnBbpTx5V+MnAe0gxQ6yn7HM+FYb3lZXIi1wl5t5dBsTWHUpw== X-Received: by 2002:a9d:824:: with SMTP id 33mr12794303oty.124.1638479391978; Thu, 02 Dec 2021 13:09:51 -0800 (PST) Received: from mail-oi1-f170.google.com (mail-oi1-f170.google.com. [209.85.167.170]) by smtp.gmail.com with ESMTPSA id z20sm248355ote.28.2021.12.02.13.09.51 for (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Thu, 02 Dec 2021 13:09:51 -0800 (PST) Received: by mail-oi1-f170.google.com with SMTP id bj13so1710697oib.4 for ; Thu, 02 Dec 2021 13:09:51 -0800 (PST) X-Received: by 2002:a54:4486:: with SMTP id v6mr6441820oiv.90.1638479391007; Thu, 02 Dec 2021 13:09:51 -0800 (PST) MIME-Version: 1.0 References: <60359D93-BA59-486D-BCD5-8EB582700FA9@jrtc27.com> In-Reply-To: <60359D93-BA59-486D-BCD5-8EB582700FA9@jrtc27.com> From: Andrew Waterman Date: Thu, 2 Dec 2021 13:09:39 -0800 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH] riscv: Resolve symbols directly for symbols with STO_RISCV_VARIANT_CC. To: Jessica Clarke Cc: Palmer Dabbelt , Jim Wilson , Kai Wang , libc-alpha@sourceware.org, Kito Cheng Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Spam-Status: No, score=-10.3 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, 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, 02 Dec 2021 21:09:54 -0000 On Thu, Dec 2, 2021 at 1:06 PM Jessica Clarke wrote: > > On 2 Dec 2021, at 20:21, Palmer Dabbelt wrote: > > > > [Changing to Jim's new address] > > > > On Mon, 25 Oct 2021 14:08:26 PDT (-0700), jimw@sifive.com wrote: > >> On Wed, Aug 11, 2021 at 4:52 PM Palmer Dabbelt wr= ote: > >> > >>> IIUC the ABI patch still isn't merged, which is what makes it actuall= y > >>> official. I'll take this when it lands. > >>> > >> > >> Just following up, the psABI patch was merged in > >> https://github.com/riscv/riscv-elf-psabi-doc/pull/190 > >> and the psABI has gone into public review for the first official relea= se. > >> So we would like this patch from Kai merged in. > >> https://sourceware.org/pipermail/libc-alpha/2021-August/129931.html > >> in case you lost track of the original message on this thread. > > > > Sorry this took a while, there's a lot of context here and I figured > > it'd be good to run through that as the issue in play has a pretty > > roundabout effect on this patch. This is a little long, but I promise > > it gets there at the end: > > > > We had a long-term outstanding issue on the psABI spec > > , which > > essentially boils down to glibc relying on lazily bound functions to fo= llow the > > standard calling convention. The actual issue isn't specific to vector > > registers, we're relying on this for X and F registers too, but AFAIK t= here's > > no users of non-standard X/F register calling conventions for lazily bo= und > > functions so we haven't had any user-visible bugs yet. This all become= s a lot > > more important with the recent addition of V registers, which are both = more > > expensive to save and all clobbered by the standard ABI. > > > > In order to address the desire for non-standard calling conventions > > driven by the standard V register map, the following text was added to > > the psABI spec: > > > > Run-time linkers that use lazy binding must preserve all argument > > registers used in the standard calling convention for the ABI in > > use. Any functions that use additional argument registers must be > > annotated with STO_RISCV_VARIANT_CC, as defined in Section 8.4. > > > > As a result of adding that language the bug in question about the lazil= y > > bound functions was resolved. We're relying on lazily bound functions > > following a lot more of the standard calling convention than that in > > glibc (sp, and ra for example; along with all the temporaries). The > > text in question doesn't directly address any of those assumptions, and > > given that there's historically been user-visible breakages around thes= e > > ambiguities I think it's prudent to be more explicit about what the > > glibc ABI is here. > > > > As far as I can see, there are really three ways to go about this: > > > > * We can just define the defacto divergence that glibc has always had > > from the psABI in this area. That's the smallest change, as IMO > > that's as simple as > > > > diff --git a/manual/platform.texi b/manual/platform.texi > > index d5fdc5bd05..9153408966 100644 > > --- a/manual/platform.texi > > +++ b/manual/platform.texi > > @@ -121,6 +121,9 @@ when it is not allowed, the priority is set to m= edium. > > @node RISC-V > > @appendixsec RISC-V-specific Facilities > > > > +Functions that are lazily bound must be compatible with the standar= d calling > > +convention. > > + > > Cache management facilities specific to RISC-V systems that impleme= nt the Linux > > ABI are declared in @file{sys/cachectl.h}. > > > > to start, adding something like "unless those functions are > > annotated with STO_RISCV_VARIANT_CC" after merging something very > > similar to the patch in question (I haven't looked closely at the > > code, but it seems pretty-straight-forward). > > * We can do our best to allow for the additional functions this new tex= t > > implicitly allows. I don't see anything we can do about RA and SP, > > but we could save the registers we clobber in _dl_runtime_resolve > > (including the F registers on non-F libraries running on F systems, > > which will be a headache). We'd still need some additional > > glibc-specific ABI for the bits of the standard calling convention we > > can't otherwise handle, but IMO those are pedantic enough that we > > could just disallow them entirely. > > * We can just stop doing any sort of lazy binding. That would allow us= to > > any relying on any unspecified behavior. > > > > The first option is preferable from my end, as the second option would > > require a lot of complexity (both specification and implementation) as > > well as come with a likely performance hit. > > Thanks for the detailed review of this corner of the spec, it=E2=80=99s > especially valuable at the moment whilst we=E2=80=99re trying to reach a > ratified version (though all the confusing/inconsistent/ill-defined > terminology is even less appropriate here, at the end of the day the > ABI is what the toolchains and runtimes say it is so it=E2=80=99s really = just > about getting the necessary rubber stamps to get the next label and > reviewing the document style and exact language used...). > > The intent of the spec is not to make repurposing just ra/sp/gp/tp as > anything other than their ABI-defined meanings (per table 1 of riscv-cc > and, in the case of the stack pointer, the alignment required by the > integer calling convention) legal unless you use STO_RISCV_VARIANT_CC, > so the glibc requirement, which is also true of FreeBSD for exactly the > same reasons, is intended to be what is specified. Upon re-reading what > was written I can see that requirement was lost or forgotten, so I=E2=80= =99ll > look at tightening it up, probably changing > > Any functions that use additional argument registers ... > > to > > Any functions that use registers in a way that is incompatible with > the register convention of the ABI in use ... > > I also note we currently only talk about the run-time linker preserving > argument registers, and say nothing about preserved registers, nor the > return address. I=E2=80=99m not sure quite why we do the former (maybe I= =E2=80=99ve > just forgotten a past conversation), I feel like that=E2=80=99s implied b= y lazy > binding being an implementation optimisation that must not break the > calling convention, but if we want to keep that language then it should > probably be changed to: > > Run-time linkers that use lazy binding must only clobber registers > defined as temporary registers for the ABI in use. > > Does that all sound sensible, and sufficient, to you? I think so. > > Jess >