From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm1-x32b.google.com (mail-wm1-x32b.google.com [IPv6:2a00:1450:4864:20::32b]) by sourceware.org (Postfix) with ESMTPS id 3EBEC3858D32 for ; Fri, 29 Sep 2023 18:22:02 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 3EBEC3858D32 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=jrtc27.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=jrtc27.com Received: by mail-wm1-x32b.google.com with SMTP id 5b1f17b1804b1-4066241289bso4599055e9.0 for ; Fri, 29 Sep 2023 11:22:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=jrtc27.com; s=gmail.jrtc27.user; t=1696011721; x=1696616521; darn=sourceware.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=dFUu5kcp31wDTHGbs24MNKAwzIRG0nEbMtdBenUaDDs=; b=GfRam/PHKpS6QcEd2VBlMAYphQYAfStcNi3/EDWB0gxCRat7t4sej++Mbz4ZVYRezs FxZNiYZnAzW+nB9um6QiuNis/uGAP+aAVhh0b64DnPjDyn/UpKBjJKne/vfKtQBF1N6A xv79m4AvOcX6NlhwNAT7Xl3/O066hVUVXSOqEy4UNOqGwoE2gSB5Lb6PzaLHsN80XS0L Jrot5J/RggRFayaU5SQb1Eg2vczc5hY+g3j/HHlK3y3dffr/g18m5MM8WOc87ndnUNV3 3r6zbpO7SUjESavQ3JGstwSx1RIQMd+xzNwQ5byPkrQSCO3PcFUVlRC8aXTFJalqESnB ISjA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1696011721; x=1696616521; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=dFUu5kcp31wDTHGbs24MNKAwzIRG0nEbMtdBenUaDDs=; b=Gl2TJ10iEymh5PtrMUmWtTrh3iZZDiU0FLWO9ox6l3nn5c4ai3nViCdL/sNLPtxO5d PKK3UG9eVqq0pXXIa8kSM3VV/rjoARiAO6Ack+JuL6ne1+F5h5mIb5AB2aZypVe/0irO EF8ZTT5fkTZolkoF3hf3bUv+UeEuwPowWhiNWzlTQr3pS20i6hh6a4RZCoO5GtDySHII qQj5mKcmxiWfx/iv2oHh93QuE1T8kfJu8ZS3qoKoVAWArEJBE2hNCQt9dcVM+ikUkW2T Te9ZXLXstW0K7yGAVrhJJwzxRvB27pITqHNbgKyzm1vhxSx477GtWvzZIyfOfNl6QFsK JeSQ== X-Gm-Message-State: AOJu0YxjiK3xo9gGoD862WLv8T9atxMTCvr++hGZmh+jDSG0raSyPB3v lIZbdrk7KidfU+1gAGo14nzSKA== X-Google-Smtp-Source: AGHT+IFwR+jGGWApZfLVKrdRTTrmLzm/umHK4S37N+jyEDgR5oS2B2P8Ap1TZ3BNJHNrsiQKdqBCSA== X-Received: by 2002:a1c:7715:0:b0:405:5ffd:21d2 with SMTP id t21-20020a1c7715000000b004055ffd21d2mr5348820wmi.19.1696011720837; Fri, 29 Sep 2023 11:22:00 -0700 (PDT) Received: from Jessicas-MacBook-Pro.localdomain ([131.111.5.246]) by smtp.gmail.com with ESMTPSA id a8-20020a05600c224800b0040607da271asm1940145wmm.31.2023.09.29.11.22.00 (version=TLS1_3 cipher=TLS_CHACHA20_POLY1305_SHA256 bits=256/256); Fri, 29 Sep 2023 11:22:00 -0700 (PDT) Received: by Jessicas-MacBook-Pro.localdomain (Postfix, from userid 501) id A4BD370FB64E; Fri, 29 Sep 2023 19:21:59 +0100 (BST) Date: Fri, 29 Sep 2023 19:21:58 +0100 From: Jessica Clarke To: Evan Green Cc: libc-alpha@sourceware.org, slewis@rivosinc.com, Florian Weimer , vineetg@rivosinc.com, palmer@rivosinc.com Subject: Re: [PATCH v8 3/6] riscv: Add __riscv_hwprobe pointer to ifunc calls Message-ID: References: <20230901235224.3304592-1-evan@rivosinc.com> <20230901235224.3304592-4-evan@rivosinc.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20230901235224.3304592-4-evan@rivosinc.com> X-Spam-Status: No, score=-11.7 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Fri, Sep 01, 2023 at 04:52:21PM -0700, Evan Green wrote: > The new __riscv_hwprobe() function is designed to be used by ifunc > selector functions. This presents a challenge for applications and > libraries, as ifunc selectors are invoked before all relocations have > been performed, so an external call to __riscv_hwprobe() from an ifunc > selector won't work. To address this, pass a pointer to the > __riscv_hwprobe() vDSO function into ifunc selectors as the second > argument (alongside dl_hwcap, which was already being passed). > > Include a typedef as well for convenience, so that ifunc users don't > have to go through contortions to call this routine. Users will need to > remember to check the second argument for NULL, both to account for > older glibcs that don't pass the function, and older kernels that don't > have the vDSO pointer. This makes the IFUNC interface Linux-specific, which seems gratuitous. Being able to write portable IFUNC resolvers across Linux, BSDs and other similar OSes is valuable, and this would preclude that. In FreeBSD there are no issues with calling functions from IFUNC resolvers; we just process all non-IRELATIVE relocations before IRELATIVE ones. It sounds like glibc could benefit from the same; then you wouldn't have to hack things up like this. Alternatively we could have a standardised "what extensions do I have" interface passed in the pointer instead, but the Linux's hwprobe (with the Linux-defined IDs for every extension Linux chooses to support and poor extensibility for vendors) is not and will never be that. I urge you to not continue down the "who cares about portability" path. Jess > Signed-off-by: Evan Green > > --- > > (no changes since v7) > > Changes in v7: > - Remove __THROW from function pointer type, as it creates warnings > together with __fortified_attr_access. > > sysdeps/riscv/dl-irel.h | 8 ++++---- > sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h | 10 ++++++++++ > 2 files changed, 14 insertions(+), 4 deletions(-) > > diff --git a/sysdeps/riscv/dl-irel.h b/sysdeps/riscv/dl-irel.h > index eaeec5467c..2147504458 100644 > --- a/sysdeps/riscv/dl-irel.h > +++ b/sysdeps/riscv/dl-irel.h > @@ -31,10 +31,10 @@ static inline ElfW(Addr) > __attribute ((always_inline)) > elf_ifunc_invoke (ElfW(Addr) addr) > { > - /* The second argument is a void pointer to preserve the extension > - fexibility. */ > - return ((ElfW(Addr) (*) (uint64_t, void *)) (addr)) > - (GLRO(dl_hwcap), NULL); > + /* The third argument is a void pointer to preserve the extension > + flexibility. */ > + return ((ElfW(Addr) (*) (uint64_t, void *, void *)) (addr)) > + (GLRO(dl_hwcap), GLRO(dl_vdso_riscv_hwprobe), NULL); > } > > static inline void > diff --git a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h > index aa189a4818..fd3be5a411 100644 > --- a/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h > +++ b/sysdeps/unix/sysv/linux/riscv/sys/hwprobe.h > @@ -67,6 +67,16 @@ extern int __riscv_hwprobe (struct riscv_hwprobe *__pairs, size_t __pair_count, > __fortified_attr_access (__read_write__, 1, 2) > __fortified_attr_access (__read_only__, 4, 3); > > +/* A pointer to the __riscv_hwprobe vDSO function is passed as the second > + argument to ifunc selector routines. Include a function pointer type for > + convenience in calling the function in those settings. */ > +typedef int (*__riscv_hwprobe_t) (struct riscv_hwprobe *__pairs, size_t __pair_count, > + size_t __cpu_count, unsigned long int *__cpus, > + unsigned int __flags) > + __nonnull ((1)) __wur > + __fortified_attr_access (__read_write__, 1, 2) > + __fortified_attr_access (__read_only__, 4, 3); > + > __END_DECLS > > #endif /* sys/hwprobe.h */ > -- > 2.34.1 >