From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-pg1-x52e.google.com (mail-pg1-x52e.google.com [IPv6:2607:f8b0:4864:20::52e]) by sourceware.org (Postfix) with ESMTPS id 5ADB63865C22 for ; Thu, 15 Feb 2024 18:56:06 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org 5ADB63865C22 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=rivosinc.com ARC-Filter: OpenARC Filter v1.0.0 sourceware.org 5ADB63865C22 Authentication-Results: server2.sourceware.org; arc=none smtp.remote-ip=2607:f8b0:4864:20::52e ARC-Seal: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708023368; cv=none; b=Ooo7xLqHTKk2lAIyab+6UO7nkVqduhZFFmlWPb86ByGkwUttcZ2R3BigmtUP1m6wWY1XU4I+R2Tkau9O5TGNUUK+3oDJIufYDfRkYxRZeZuwoO28y5pzheYkS53kfwsc+S+8cdwOlZvyRjSe+TwelTopsjlBAtP+omgUMbowa5I= ARC-Message-Signature: i=1; a=rsa-sha256; d=sourceware.org; s=key; t=1708023368; c=relaxed/simple; bh=JWw2sDJ2qWbSfXYUTokkh7lXRfkw780AVG2KtfCDnk8=; h=DKIM-Signature:Date:Subject:From:To:Message-ID:Mime-Version; b=RSmrW5Ixq5Nlc5Pj8PLZLw5sTZG6kFJzH0CzoByoq8T7IYc3DFq6qOmV+78Qlz1d6BEMloHluwjmIi8WONxi1tfaqmiuLiENhK2rSV3I0ruaoEDYygyJZUMboR3dIPE3PLdW0ObJ9hp3D6OQHV/PSq1UR8oDTBil141ApgD8J88= ARC-Authentication-Results: i=1; server2.sourceware.org Received: by mail-pg1-x52e.google.com with SMTP id 41be03b00d2f7-5bdbe2de25fso1057200a12.3 for ; Thu, 15 Feb 2024 10:56:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1708023365; x=1708628165; darn=sourceware.org; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:from:to:cc:subject:date:message-id :reply-to; bh=pBAj5Qopg2v7lIehQCLtwlpkbXnEGRfiKBavQTmBg74=; b=FI9+oPwN6N9UxPfvDpltlwnCp+KqpTR3usixkYNCNG5AFj1oXdwuq72X56gGUEII+J B1+r5tWdz4VUwzpGOsKpE69MGlZDzinh4MAnLRigZOmf9iB9GZmc1e0IcIoKPJKgEF1K +BHVJT1gDauMKwLTibzeQtwAZLxO3peuXsJ/aBhlq/xEnhRdsa/VC0XXL0dlvdWQHjge dbZPANv+VwEWkpBBoNLT11ljY2aCUCwoQjl5utYtKjTARQYbG0zgd+hH7t9JmfWLjiKI txQr9c/4LL0BkHidFP0MY0rywunwP9B+lHGxCbktRJycHnU2ci7iIQt55wD6qFwBrr6x sLiA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1708023365; x=1708628165; h=content-transfer-encoding:mime-version:message-id:to:from:cc :in-reply-to:subject:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=pBAj5Qopg2v7lIehQCLtwlpkbXnEGRfiKBavQTmBg74=; b=fjt7E51RuUnmoT9beBOyePlG2SH+U4YmRKiQpV4RCngY318b4po2ipx29QgGkKz6bh vZdGTYs8mladog66cvhCUV7qCWgKFn0yWCYh1u8ObrLdJDvS5/Wz1CGzSP9At2jpz2G5 Ik7SD7ZSYrFWlWIIZKBESTwDy5X47PjXqrtEoz1UiY0nIv3eK/AGyaF8/cQf5zoJ2zWr 6x8f8Piyey6J0ZA8RSDVTLVjsB2hH8QtXSaYLwxrefcqaeuHngvDz0KzxzmIiCFf17Y/ uPkvEVqWyGRcQQVAzZnr4dNsZitzSPQMOP5r+BIf7DvRIvU9snt2bmOYV6FfSibXgbGU 92/Q== X-Forwarded-Encrypted: i=1; AJvYcCWrSoODm8VbwKEKDjmTX2zDj5uHNnyG4+qRmbd1Sia02NaOXfc0nzESMDnxpSl0WSgcnarr3hJGltZQlXlT5F1wAp6REzFC2m6x X-Gm-Message-State: AOJu0YwXSlFOR17dZGdHhSIvmcvaIUqge4najysvYerke4vF3xcj1nnS 4ThN/ChKdIFrGtUBftUyvPhXvXm8eSp4qgQZGd0ZUqS4KftrpJp4VaE6B5aRWiM= X-Google-Smtp-Source: AGHT+IFZvHSs8TQD4EfVTBayp3Z4IhyIgFAyDoWrCNRH4HhsdKc7c55DLekETI7EHlpJa4gHlNpCZg== X-Received: by 2002:a05:6a20:49a5:b0:19e:cdca:3e56 with SMTP id fs37-20020a056a2049a500b0019ecdca3e56mr2155092pzb.27.1708023365206; Thu, 15 Feb 2024 10:56:05 -0800 (PST) Received: from localhost ([192.184.165.199]) by smtp.gmail.com with ESMTPSA id cb15-20020a056a02070f00b005dbd0facb4dsm1539573pgb.61.2024.02.15.10.56.04 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Feb 2024 10:56:04 -0800 (PST) Date: Thu, 15 Feb 2024 10:56:04 -0800 (PST) X-Google-Original-Date: Thu, 15 Feb 2024 10:56:02 PST (-0800) Subject: Re: [PATCH v12 0/7] RISC-V: ifunced memcpy using new kernel hwprobe interface In-Reply-To: CC: Evan Green , adhemerval.zanella@linaro.org, libc-alpha@sourceware.org, Vineet Gupta , fweimer@redhat.com, slewis@rivosinc.com, jrtc27@debian.org From: Palmer Dabbelt To: enh@google.com Message-ID: Mime-Version: 1.0 (MHng) Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 Thu, 15 Feb 2024 10:45:10 PST (-0800), enh@google.com wrote: > On Thu, Feb 15, 2024 at 9:22 AM Palmer Dabbelt wrote: >> >> On Thu, 15 Feb 2024 09:00:03 PST (-0800), enh@google.com wrote: >> > On Thu, Feb 15, 2024 at 8:50 AM Palmer Dabbelt wrote: >> >> >> >> On Thu, 15 Feb 2024 07:48:03 PST (-0800), Evan Green wrote: >> >> > On Wed, Feb 14, 2024 at 10:16 AM Adhemerval Zanella Netto >> >> > wrote: >> >> >> >> >> >> >> >> >> >> >> >> On 14/02/24 11:31, Evan Green wrote: >> >> >> > >> >> >> > This series illustrates the use of a recently accepted Linux syscall that >> >> >> > enumerates architectural information about the RISC-V cores the system >> >> >> > is running on. In this series we expose a small wrapper function around >> >> >> > the syscall. An ifunc selector for memcpy queries it to see if unaligned >> >> >> > access is "fast" on this hardware. If it is, it selects a newly provided >> >> >> > implementation of memcpy that doesn't work hard at aligning the src and >> >> >> > destination buffers. >> >> >> > >> >> >> > For applications and libraries outside of glibc that want to use >> >> >> > __riscv_hwprobe() in ifunc selectors, this series also sends a pointer >> >> >> > to the riscv_hwprobe() function in as the second argument to ifunc >> >> >> > selectors. A new inline convenience function can help application and >> >> >> > library callers to check for validity and quickly probe a single key. >> >> >> >> >> >> I still think we should address Jessica Clarke remarks for the ifunc ABI [1]. >> >> >> I recall that Florian has tried to address the ifunc ordering and that >> >> >> Jessica proposed solutions was not fully sufficient to address all the >> >> >> ifunc corner cases. >> >> >> >> >> >> [1] https://sourceware.org/pipermail/libc-alpha/2024-January/154082.html >> >> > >> >> > I haven't invested the time yet in studying the resolver to understand >> >> > how feasible Jessica's suggestion is. I was sort of hoping Florian >> >> > would chime in with an "oh yeah let's do that" or "no, it doesn't >> >> > work". I suppose I still am :) >> >> >> >> The discussion's over here: >> >> https://inbox.sourceware.org/libc-alpha/20231017044641.pw2ccr6exvhtmhkk@google.com/ >> >> >> >> I was inclined to just ignore it: Florian explained what's going on with >> >> the multiple libraries constraint, so we're kind of just going around in >> >> circles at that point. >> >> >> >> I'm also not sure the argument makes a whole lot of sense in the first >> >> place. The core of the argument seems to be around binary compatibility >> >> with other kernels/libcs, but there's a ton of reasons why binaries >> >> aren't compatible between those systems. So if glibc just has a more >> >> complex IFUNC resolving scheme and that ends up requiring the extra >> >> agrument, then users of glibc have to go deal with that -- other >> >> systems/libcs might make different decisions, but that's just how these >> >> things go. >> > >> > yeah, i struggle with the portability premise too. not least because >> > macOS/iOS (which is obviously the most common "other platform" for me >> > with my Android hat on) doesn't have ifuncs at all. that's quite a big >> > blocker to any dream of portability. >> > >> > as i've said before, in a survey of all the open source libraries that >> > go into Android, there are none that don't support macOS/iOS, and so >> > there are none that actually rely on ifuncs. ifunc usage is limited to >> > libc (which is why we libc maintainers get bikeshedded by stuff like >> > this) and compiler-generated FMV stuff. i'd argue that "real people" >> > (app developers) should probably be looking at the latter anyway, and >> > it's our job to make that work (which happens once^Wtwice, in llvm and >> > gcc). >> > >> > also, the fact that Android is doing what's proposed here (with the >> > extra argument) means there'd be _some_ incompatibility even if glibc >> > and FreeBSD didn't do that. >> >> So you're already committed to that ABI? > > "pretty much" given glibc release cycles and Android release cycles... Ya, cool -- I was just curious, I figured it'd be something along those lines (we also talked a bit off list). > the current API addresses a real problem with current dynamic linkers, > and -- though it would have been great to make a different decision 16 > years ago -- between the already stated reasons and the potential for > app compat issues (and a distaste for having the riscv64 linker > behavior differ from the already-shipped architectures), i think we'll > want something like that... Ya, I'm generally in favor of making RISC-V as unsurprising as possible. Given how complex the IFUNC stuff is it seems like a recipe for disaster to have different rules around linker ordering or cross-object symbol resoltion for RISC-V. Maybe folks want to change that for all the ports, but that seems like a fairly complex project and I'm worried we'll miss another release waiting on it. As far as I can tell the downside here is just that we have an extra argument pointing to a hwprobe function, that doesn't seem so painful to just support forever. It's not like this possible change to the resolution rules would make calling via the pre-resolved function argument invalid, it'd just be unnecessary. Thus I'm generally in favor of merging this, so Acked-by: Palmer Dabbelt though I'm cool holding off a bit on merging it, in case anyone thinks that's a bad idea. >> > (Android's only source incompatibility with glibc at the moment is the >> > lack of the single-probe helper inline, which i fear will encourage >> > _worse_ code, but suspect will actually be unused in practice anyway >> > for the same "ifuncs are 'assembler' for libc/toolchain, and >> > library/app developers doing stuff themselves will continue to use >> > regular function pointers like they do today" reasons.) >> > >> >> Looks like Maskray is proposing making glibc's IFUNC resolving. That's >> >> a generic glibc decision and I don't really understand this stuff well >> >> enough to have much of an opinion -- sure maybe the multi-library IFUNC >> >> resolving is overkill, but aside from doing some similar grep of all the >> >> sources (or trying some builds and such) I'm not sure how to tell if >> >> we're safe breaking the ABI there. >> >> >> >> That said, we're not comitting to ABI stability until the release. So >> >> if we decide to make a generic glibc change we can always go drop the >> >> argument, that should be easy. Maybe we even just throw out a RFC patch >> >> to do it, unless I'm missing something the code is the easy part here >> >> (we're essentially just going back to one of the early versions, from >> >> before we knew about this resolving order complexity). >> >> >> >> > Alternatively, patches 1-3 of this series stand on their own. If the >> >> > ifunc aspect of this is gated on me doing a bunch of research, it >> >> > might at least make sense to land the first half now, to get Linux >> >> > users easy access to the __riscv_hwprobe() syscall and vDSO. >> >> >> >> I'm OK with that, too. >> >> >> >> > >> >> > -Evan