From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 54F525478B for ; Fri, 12 Apr 2024 21:56:52 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712959013; cv=none; b=CVtHChzzat9pUNxNRjZ52Q1JeY+aEAndDlWXexj55ci73rLTYQWzsLVdtF0CoqSAOFdInzfa8elEjkAEnWnax5hgnw+qmEqsYfYFKovYH6C1h+Hrk+opfM7qJul47xIWbNrhdLmLGOkfaFgOsf8zgKGyON0xkw9mhA8s41Zh+z4= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1712959013; c=relaxed/simple; bh=1PaWGfxyrHB5QHCFj3ZBI3SC8je2iOSCNdYW+yj2zZk=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=lRR3nPnkmq+6hk9MReS94oysuzdNIHGqDvckpSbifXEr3gNytFVw86SjLyGj3vNyiPltCUoGuvhj1WTNGbIXBmWkJOzXf6P26vwYTCYz4FW8RpqWfpQrEm/mjUAHP6EMGuDZmJTvJkYYl9gfCbyh78F+SGXgo+PbhDUG4zP7oMA= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com; spf=pass smtp.mailfrom=rivosinc.com; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b=fidIA4hg; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=none (p=none dis=none) header.from=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=rivosinc.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=rivosinc-com.20230601.gappssmtp.com header.i=@rivosinc-com.20230601.gappssmtp.com header.b="fidIA4hg" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-1e504f58230so12277225ad.2 for ; Fri, 12 Apr 2024 14:56:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1712959012; x=1713563812; darn=vger.kernel.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=KaHJoFfHeKcbZbr8DZBqF2w1WE3ebjk/glljyT7W9ZI=; b=fidIA4hgMjYhSe/iokhz19RcljwGVludiAbQOtT/mqtLkfFJodHOeqLL4bG9FV1bKQ X/j4onsaejAYs3iXIs9uz25e2mYJ1fs4kNEmLSYFck06h0dM9o8dXqFzCQ/T+TnMcenN a+UESJqJM3K4Sudn3WsAgFEy9UudSyCCg4pE5vUAgA6GVeAkfj366YyRAH8Y5um0Usvb eoOD3aawPKyRzOMVXI9/b/8ZdrGDrnjzMJq5CySWWx3fSm3ApumvMVIdjjFTxooJ6gPY L0rjYklLLPmoCzVbfqCARJ6q7aOTRnpIMgEgojA3es0nLHwRUVCUE/kARg+U8dGN1TJE +lqg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712959012; x=1713563812; 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=KaHJoFfHeKcbZbr8DZBqF2w1WE3ebjk/glljyT7W9ZI=; b=v34SFYufdRlirtkb3dc7YnQ0wUxjp4SpLfltF0/UGZxIuo+AeC2CgYqwOVA2rEFlpc 0007+98P/o9NpJ1pvCAw6qdKIUqAVR0TkLcJ2yWOqLn+uvJXS7dirxQJqBPIEu4p2BlJ ht1vIWXMbTRY0SVm4uV70L+B3CLGfEfnqsTUAKfsc2AOFHBrmntv7mWd5xp5B1oBcTES L8it1OWKRw8Ao8YEyie0qQ7TF+ruLMPp4crBwSLS2zWm2oy0lKS/Kc/YcjGLrgjyiKHM nMrH3HNkP8iJpOEFVDNAcEXEjat7v+LRgXpU1dPqOmzndfQ5p8a4ITmduCa7K5Qt2vhz SrgA== X-Forwarded-Encrypted: i=1; AJvYcCVuoG4DX79Fkez9lkPOkhIkWj7oGzr8HkMOjozCl/D6zyYcWE7WmbdwhhneRN2Lo4c/gM2BPS8t/YkkGZcncIHtRiSkiajn+L2R X-Gm-Message-State: AOJu0YxDNlbz0mrqE7Ed6d1PsWV18XXniD/cr+E/RG7lWaK8CYim6Nbu Dg5AX6vWBv9htv9dfy33Owd2vfJHg2bq82kJyUuIrNxWFxr2MQZ5TolIeaUrm7E= X-Google-Smtp-Source: AGHT+IEWqFfAYkiXilmnZ21u0Vqs/Bc8kyXEc8OwbOgXHViFBz7n/2ViIbfUfkCfp7Uh0oJ1KjxbyQ== X-Received: by 2002:a17:903:250:b0:1e4:b1ea:23ef with SMTP id j16-20020a170903025000b001e4b1ea23efmr4487004plh.49.1712959011683; Fri, 12 Apr 2024 14:56:51 -0700 (PDT) Received: from ghost ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id o16-20020a170902d4d000b001e088a9e2bcsm3463749plg.292.2024.04.12.14.56.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Apr 2024 14:56:51 -0700 (PDT) Date: Fri, 12 Apr 2024 14:56:48 -0700 From: Charlie Jenkins To: Conor Dooley Cc: Conor Dooley , Rob Herring , Krzysztof Kozlowski , Paul Walmsley , Palmer Dabbelt , Albert Ou , Guo Ren , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Evan Green , =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= , Jonathan Corbet , Shuah Khan , linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Palmer Dabbelt , linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH 08/19] riscv: Introduce vendor variants of extension helpers Message-ID: References: <20240411-dev-charlie-support_thead_vector_6_9-v1-0-4af9815ec746@rivosinc.com> <20240411-dev-charlie-support_thead_vector_6_9-v1-8-4af9815ec746@rivosinc.com> <20240412-dwarf-shower-5a7300fcd283@wendy> <20240412-drab-french-55d8ff2c8cfa@spud> <20240412-geranium-monotone-b47f414715d1@spud> Precedence: bulk X-Mailing-List: linux-doc@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20240412-geranium-monotone-b47f414715d1@spud> On Fri, Apr 12, 2024 at 10:34:10PM +0100, Conor Dooley wrote: > On Fri, Apr 12, 2024 at 02:03:48PM -0700, Charlie Jenkins wrote: > > On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote: > > > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > > > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > > > > Create vendor variants of the existing extension helpers. If the > > > > > > existing functions were instead modified to support vendor extensions, a > > > > > > branch based on the ext value being greater than > > > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > > > > branch would have an unnecessary performance impact. > > > > > > > > > > > > Signed-off-by: Charlie Jenkins > > > > > > > > > > I've not looked at the "main" patch in the series that adds all of the > > > > > probing and structures for representing this info yet beyond a cursory > > > > > glance, but it feels like we're duplicating a bunch of infrastructure > > > > > here before it is necessary. The IDs are all internal to Linux, so I'd > > > > > rather we kept everything in the same structure until we have more than > > > > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > > > > we will have three vendor extensions which feels like a drop in the > > > > > bucket compared to the standard ones. > > > > > > > > It is not duplicating infrastructure. If we merge this into the existing > > > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > > > > in __riscv_isa_extension_available. This is particularily important > > > > exactly because we have so few vendor extensions currently so this check > > > > would be irrelevant in the vast majority of cases. > > > > > > That's only because of your implementation. The existing vendor extension > > > works fine without this littering. That's another thing actually, you > > > forgot to convert over the user we already have :) > > > > Oh right, I will convert them over. The fundemental goal of this patch > > is to allow a way for vendors to support their own extensions without > > needing to populate riscv_isa_ext. This is to create separation between > > vendors so they do not impact each other. > > The one that needs converting is xandespmu. As I said on the other patch > a minute I don't think isolating vendors for the internal representation > is needed and can be left in hwprobe. I also don't think we can rely on > a behaviour of "SiFive CPUs will always have SiFive's mvendorid" or that > kinda thing, I've heard talk of the SoC vendor getting their mvendorid > for custom CPU cores instead of the CPU vendor and it's possible for > the SBI implementation to "adjust" the values also. Okay that may be possible but that is up to the vendor when that happens. The vendor extensions are fundamentally different from the standard extensions and have even less guarantees of correctness which seems like it would invite more errata if multiple vendors implement the same vendor extensions. I can extract the code into a different file for each vendor so that is more clear. - Charlie > > > xlinuxenvcfg does not fit into this scheme however. This scheme assumes > > that a hart cannot have multiple vendors which that extension breaks. > > xlinuxenvcfg is really filling a hole in the standard isa that is > > applicible to all vendors and does not appear in the device tree so it > > is okay for that to live outside this scheme. > > Ye, xlinuxenvcfg is an internal psuedo-extension that should be treated > more like a standard one than something vendor. > > > > > It is also unecessary to push off the refactoring until we have some > > > > "sufficient" amount of vendor extensions to deem changing the > > > > infrastructure when I already have the patch available here. This does > > > > not introduce any extra overhead to existing functions and will be able > > > > to support vendors into the future. > > > > > > Yeah, maybe that's true but this was my gut reaction before reading the > > > other patch in detail (which I've still yet to do). > > > > - Charlie > > From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id EAF94C4345F for ; Fri, 12 Apr 2024 21:57:06 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=FJmCno2jXsMCy/Oknh/t08duFbQUGpneuLY5MbM5Gp8=; b=Ed4RK7+98PRxKy 8DKyuh855rtFCgcOjzx/R9AuUa76ha87IoEZhOZEFHTx3TjhG4CAqBC7cRHBUXxfHnItSbXG0sLN4 slWCm/q6GmSKF7W3qwSMDaT+gJSmW7hmVviHMN2Kdva3id0LstvRYICFBI+39RKJ6uNbmBMRG3HOJ 3FXwYvkA/UjQt01zZ2C7fQzYOFDCg4fJRgbHq9anchHPBcx0gf8b7Zu5uLBfo9sw2LryJL70DYegu NhoRIO20qXP/7cyfxAonG7KawjyvcDF/WwMPiU6BE37F8JhNVTaOjVoTnuatp9erXC0OSSu9XzAfS QrgBJbIZUD6X5Rm6sbOg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvOtI-00000001T1k-2ogr; Fri, 12 Apr 2024 21:57:00 +0000 Received: from mail-pl1-x631.google.com ([2607:f8b0:4864:20::631]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvOtE-00000001Sz7-1D16 for linux-riscv@lists.infradead.org; Fri, 12 Apr 2024 21:56:58 +0000 Received: by mail-pl1-x631.google.com with SMTP id d9443c01a7336-1e3ca546d40so10252665ad.3 for ; Fri, 12 Apr 2024 14:56:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1712959012; x=1713563812; darn=lists.infradead.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=KaHJoFfHeKcbZbr8DZBqF2w1WE3ebjk/glljyT7W9ZI=; b=3dcDACbioeYzNGrjA7buPEnPMLOyYfcDqwYQtiRkEYGwpKHaQuPv+6LwKGLe2snene iTRDFUktnM4q75Zp2C4xs+pvStU8pDGggyb4L5Mwk7f8fWcxc5ZgVuWSVLK4EKMMT09u psZdhXSlp3EIRFjhoWxFjrPvLg7QZhSJQnb3CnziwQmw/ZDtSe5QGS5Nz0WMxZAQV2BQ /mUDB8cK5bD/3TW6NwhqtygMo3PfICeLj7juUSxM3n6steCv2HvsGtshsJbL1nhzzHvg l8tD2tlgCheXVm7CMOQLvMmwm201O+zuY2LfVyYalsPBER4b0qqvxlnO/kuF10XEyfGI dJng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712959012; x=1713563812; 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=KaHJoFfHeKcbZbr8DZBqF2w1WE3ebjk/glljyT7W9ZI=; b=BtLCEth6S7Vv5I5BL6NFOavp843pRBgnaVeJ6/30zHuQ3yALeL62jTUs2XSxUj/DEp WAKDXL08EuOpKJTL+ZkNhBk1FSoB/W8ZSF6GOaYH3GqTU6E7NRJoOfzXsjFjvFlUDnGW snTVG+kPX4aWQyMn/EDBqE3hXIkLqsSfKkhzr3Xv9KdLTlMpaMxV4pY+VK7TiAJt4gI1 7dNWaNkRg7T0CZAm+pkulKPsiJocbnPepHEXEm+5bHrC+VIWYNPRVIP17u9uyVCRcnDg fd5Fs8s5BU3q0cASGhfD9QMGCddhg5sBd3k3rej+DsCjWRVNBFdXKeuTQj2c4p6Q3sGC 2mVg== X-Forwarded-Encrypted: i=1; AJvYcCU6a7b2z5CnH+Qq7XVC0tNbTGRoHxxCZZhZ7O8Qjn3LYj2ISM20u3L+fif8Dhug3m/r+9Gmu8gx9zvhRJpZT2Grph8ZXl0tO7NXZjasq2H2 X-Gm-Message-State: AOJu0YxL+4lKbqUcj93+2wG+wL+1uW2SvQif851MtvWTPNTbEtThvNIi 8ztU47+kXG4le95sYVlOJ1Oe2mVBtfBJTTTMaBNS6nLL35ST3aL6f9biLpo4d7A= X-Google-Smtp-Source: AGHT+IEWqFfAYkiXilmnZ21u0Vqs/Bc8kyXEc8OwbOgXHViFBz7n/2ViIbfUfkCfp7Uh0oJ1KjxbyQ== X-Received: by 2002:a17:903:250:b0:1e4:b1ea:23ef with SMTP id j16-20020a170903025000b001e4b1ea23efmr4487004plh.49.1712959011683; Fri, 12 Apr 2024 14:56:51 -0700 (PDT) Received: from ghost ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id o16-20020a170902d4d000b001e088a9e2bcsm3463749plg.292.2024.04.12.14.56.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Apr 2024 14:56:51 -0700 (PDT) Date: Fri, 12 Apr 2024 14:56:48 -0700 From: Charlie Jenkins To: Conor Dooley Cc: Conor Dooley , Rob Herring , Krzysztof Kozlowski , Paul Walmsley , Palmer Dabbelt , Albert Ou , Guo Ren , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Evan Green , =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= , Jonathan Corbet , Shuah Khan , linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Palmer Dabbelt , linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH 08/19] riscv: Introduce vendor variants of extension helpers Message-ID: References: <20240411-dev-charlie-support_thead_vector_6_9-v1-0-4af9815ec746@rivosinc.com> <20240411-dev-charlie-support_thead_vector_6_9-v1-8-4af9815ec746@rivosinc.com> <20240412-dwarf-shower-5a7300fcd283@wendy> <20240412-drab-french-55d8ff2c8cfa@spud> <20240412-geranium-monotone-b47f414715d1@spud> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240412-geranium-monotone-b47f414715d1@spud> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240412_145656_371562_231BE1BF X-CRM114-Status: GOOD ( 41.40 ) X-BeenThere: linux-riscv@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-riscv" Errors-To: linux-riscv-bounces+linux-riscv=archiver.kernel.org@lists.infradead.org On Fri, Apr 12, 2024 at 10:34:10PM +0100, Conor Dooley wrote: > On Fri, Apr 12, 2024 at 02:03:48PM -0700, Charlie Jenkins wrote: > > On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote: > > > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > > > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > > > > Create vendor variants of the existing extension helpers. If the > > > > > > existing functions were instead modified to support vendor extensions, a > > > > > > branch based on the ext value being greater than > > > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > > > > branch would have an unnecessary performance impact. > > > > > > > > > > > > Signed-off-by: Charlie Jenkins > > > > > > > > > > I've not looked at the "main" patch in the series that adds all of the > > > > > probing and structures for representing this info yet beyond a cursory > > > > > glance, but it feels like we're duplicating a bunch of infrastructure > > > > > here before it is necessary. The IDs are all internal to Linux, so I'd > > > > > rather we kept everything in the same structure until we have more than > > > > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > > > > we will have three vendor extensions which feels like a drop in the > > > > > bucket compared to the standard ones. > > > > > > > > It is not duplicating infrastructure. If we merge this into the existing > > > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > > > > in __riscv_isa_extension_available. This is particularily important > > > > exactly because we have so few vendor extensions currently so this check > > > > would be irrelevant in the vast majority of cases. > > > > > > That's only because of your implementation. The existing vendor extension > > > works fine without this littering. That's another thing actually, you > > > forgot to convert over the user we already have :) > > > > Oh right, I will convert them over. The fundemental goal of this patch > > is to allow a way for vendors to support their own extensions without > > needing to populate riscv_isa_ext. This is to create separation between > > vendors so they do not impact each other. > > The one that needs converting is xandespmu. As I said on the other patch > a minute I don't think isolating vendors for the internal representation > is needed and can be left in hwprobe. I also don't think we can rely on > a behaviour of "SiFive CPUs will always have SiFive's mvendorid" or that > kinda thing, I've heard talk of the SoC vendor getting their mvendorid > for custom CPU cores instead of the CPU vendor and it's possible for > the SBI implementation to "adjust" the values also. Okay that may be possible but that is up to the vendor when that happens. The vendor extensions are fundamentally different from the standard extensions and have even less guarantees of correctness which seems like it would invite more errata if multiple vendors implement the same vendor extensions. I can extract the code into a different file for each vendor so that is more clear. - Charlie > > > xlinuxenvcfg does not fit into this scheme however. This scheme assumes > > that a hart cannot have multiple vendors which that extension breaks. > > xlinuxenvcfg is really filling a hole in the standard isa that is > > applicible to all vendors and does not appear in the device tree so it > > is okay for that to live outside this scheme. > > Ye, xlinuxenvcfg is an internal psuedo-extension that should be treated > more like a standard one than something vendor. > > > > > It is also unecessary to push off the refactoring until we have some > > > > "sufficient" amount of vendor extensions to deem changing the > > > > infrastructure when I already have the patch available here. This does > > > > not introduce any extra overhead to existing functions and will be able > > > > to support vendors into the future. > > > > > > Yeah, maybe that's true but this was my gut reaction before reading the > > > other patch in detail (which I've still yet to do). > > > > - Charlie > > _______________________________________________ linux-riscv mailing list linux-riscv@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-riscv From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 47983C4345F for ; Fri, 12 Apr 2024 21:57:14 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=7MOKwHXI8/eRB2Z6frP37xWEtPnq0eLW01uw9RJmgEM=; b=rC+9Y65fVj+Wjv /uRzcxxVSFWfwXa/jFdTWr3Dm3nhJkBmvyxSsO8qCnFRwrjg/KFNRGTujeSRJMKL8zYPv0sz3c8zC JFA4zf3iwyQn3CtBg0rGfKkRLiCz2mhZSrOd5n050Mhri4PqjQdFkoaxZITN4fxlFjBYH+CBfOfd3 gHiJs2QbQPUhgQAaFcF1cYmm4hBz3Ebb0HZJxupwXTFfs0tKxTw9pjaQFlD6RtbS7CmbYAk/gZ3cZ n6AmqmlyX8+xGSI+AZj337o2QyxO8Id6X9HEbL94ZLn8hRZAV4c5H/tvRFSeMihWxrSwsALtS0YZj TKQcXGTh0v0sSLQhtYMg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvOtI-00000001T1L-099f; Fri, 12 Apr 2024 21:57:00 +0000 Received: from mail-pl1-x632.google.com ([2607:f8b0:4864:20::632]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvOtE-00000001Sz8-1DH8 for linux-arm-kernel@lists.infradead.org; Fri, 12 Apr 2024 21:56:58 +0000 Received: by mail-pl1-x632.google.com with SMTP id d9443c01a7336-1e3ca546d40so10252675ad.3 for ; Fri, 12 Apr 2024 14:56:52 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1712959012; x=1713563812; darn=lists.infradead.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=KaHJoFfHeKcbZbr8DZBqF2w1WE3ebjk/glljyT7W9ZI=; b=3dcDACbioeYzNGrjA7buPEnPMLOyYfcDqwYQtiRkEYGwpKHaQuPv+6LwKGLe2snene iTRDFUktnM4q75Zp2C4xs+pvStU8pDGggyb4L5Mwk7f8fWcxc5ZgVuWSVLK4EKMMT09u psZdhXSlp3EIRFjhoWxFjrPvLg7QZhSJQnb3CnziwQmw/ZDtSe5QGS5Nz0WMxZAQV2BQ /mUDB8cK5bD/3TW6NwhqtygMo3PfICeLj7juUSxM3n6steCv2HvsGtshsJbL1nhzzHvg l8tD2tlgCheXVm7CMOQLvMmwm201O+zuY2LfVyYalsPBER4b0qqvxlnO/kuF10XEyfGI dJng== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712959012; x=1713563812; 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=KaHJoFfHeKcbZbr8DZBqF2w1WE3ebjk/glljyT7W9ZI=; b=cu9ipNUh5Or3yogu3okv67uq1KzUqyTMbfnHzyprOOFkCrT3YdgPSWBAlQ60PagxoC T/2r3dKtaP6Y9xo2TRFf3o6DsJq1b8ViIE6npv1/a27vEPmZqYq5UnwIZDZXINDNhRh6 2vm9qdXqRFqfGEnVDsxcl6+DAWCQqUBETxWA0nuQSAvFoF991F3+FjnY5pxwNpjeeY3/ zYYdtgGNW6pApPY4Gs+dJl5tArqpguCLHRQxsOmDaxobPaf7L8uRuwjNhshqlJA1NTd0 iBg9wFXJ5T+9W8ysZsmv8tHfEUODyJuLz5gvTmL4KW6F/YTykaIZdT2xm/Vp7YwnCtpU lZbQ== X-Forwarded-Encrypted: i=1; AJvYcCUOhEu/V2vgvjyEtB17AzbJwAxUczVk0hDEirwXvD1Paz+UnxCaoeHxXN6Wy70N6v6CP/RdQCoujLEKyfKut5rm5epWZrzgMgStEM5FCniRE1qV8eM= X-Gm-Message-State: AOJu0Yy5dUA5F5zqw45AOSBQJ7zj5e/fqSGp8kozMWSVel61eOVV5HO+ i5Dtb4wFxy4vJ0P6ZqYtUZ9n6ZRa6G+Jz3c1FHbnjw0B4zULnIxTMgk5rfv1EsY= X-Google-Smtp-Source: AGHT+IEWqFfAYkiXilmnZ21u0Vqs/Bc8kyXEc8OwbOgXHViFBz7n/2ViIbfUfkCfp7Uh0oJ1KjxbyQ== X-Received: by 2002:a17:903:250:b0:1e4:b1ea:23ef with SMTP id j16-20020a170903025000b001e4b1ea23efmr4487004plh.49.1712959011683; Fri, 12 Apr 2024 14:56:51 -0700 (PDT) Received: from ghost ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id o16-20020a170902d4d000b001e088a9e2bcsm3463749plg.292.2024.04.12.14.56.50 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Apr 2024 14:56:51 -0700 (PDT) Date: Fri, 12 Apr 2024 14:56:48 -0700 From: Charlie Jenkins To: Conor Dooley Cc: Conor Dooley , Rob Herring , Krzysztof Kozlowski , Paul Walmsley , Palmer Dabbelt , Albert Ou , Guo Ren , Conor Dooley , Chen-Yu Tsai , Jernej Skrabec , Samuel Holland , Evan Green , =?iso-8859-1?Q?Cl=E9ment_L=E9ger?= , Jonathan Corbet , Shuah Khan , linux-riscv@lists.infradead.org, devicetree@vger.kernel.org, linux-kernel@vger.kernel.org, Palmer Dabbelt , linux-arm-kernel@lists.infradead.org, linux-sunxi@lists.linux.dev, linux-doc@vger.kernel.org, linux-kselftest@vger.kernel.org Subject: Re: [PATCH 08/19] riscv: Introduce vendor variants of extension helpers Message-ID: References: <20240411-dev-charlie-support_thead_vector_6_9-v1-0-4af9815ec746@rivosinc.com> <20240411-dev-charlie-support_thead_vector_6_9-v1-8-4af9815ec746@rivosinc.com> <20240412-dwarf-shower-5a7300fcd283@wendy> <20240412-drab-french-55d8ff2c8cfa@spud> <20240412-geranium-monotone-b47f414715d1@spud> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240412-geranium-monotone-b47f414715d1@spud> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240412_145656_371351_234366FD X-CRM114-Status: GOOD ( 42.96 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org On Fri, Apr 12, 2024 at 10:34:10PM +0100, Conor Dooley wrote: > On Fri, Apr 12, 2024 at 02:03:48PM -0700, Charlie Jenkins wrote: > > On Fri, Apr 12, 2024 at 09:40:03PM +0100, Conor Dooley wrote: > > > On Fri, Apr 12, 2024 at 10:43:02AM -0700, Charlie Jenkins wrote: > > > > On Fri, Apr 12, 2024 at 12:49:57PM +0100, Conor Dooley wrote: > > > > > On Thu, Apr 11, 2024 at 09:11:14PM -0700, Charlie Jenkins wrote: > > > > > > Create vendor variants of the existing extension helpers. If the > > > > > > existing functions were instead modified to support vendor extensions, a > > > > > > branch based on the ext value being greater than > > > > > > RISCV_ISA_VENDOR_EXT_BASE would have to be introduced. This additional > > > > > > branch would have an unnecessary performance impact. > > > > > > > > > > > > Signed-off-by: Charlie Jenkins > > > > > > > > > > I've not looked at the "main" patch in the series that adds all of the > > > > > probing and structures for representing this info yet beyond a cursory > > > > > glance, but it feels like we're duplicating a bunch of infrastructure > > > > > here before it is necessary. The IDs are all internal to Linux, so I'd > > > > > rather we kept everything in the same structure until we have more than > > > > > a handful of vendor extensions. With this patch (and the theadpmu stuff) > > > > > we will have three vendor extensions which feels like a drop in the > > > > > bucket compared to the standard ones. > > > > > > > > It is not duplicating infrastructure. If we merge this into the existing > > > > infrastructure, we would be littering if (ext > RISCV_ISA_VENDOR_EXT_BASE) > > > > in __riscv_isa_extension_available. This is particularily important > > > > exactly because we have so few vendor extensions currently so this check > > > > would be irrelevant in the vast majority of cases. > > > > > > That's only because of your implementation. The existing vendor extension > > > works fine without this littering. That's another thing actually, you > > > forgot to convert over the user we already have :) > > > > Oh right, I will convert them over. The fundemental goal of this patch > > is to allow a way for vendors to support their own extensions without > > needing to populate riscv_isa_ext. This is to create separation between > > vendors so they do not impact each other. > > The one that needs converting is xandespmu. As I said on the other patch > a minute I don't think isolating vendors for the internal representation > is needed and can be left in hwprobe. I also don't think we can rely on > a behaviour of "SiFive CPUs will always have SiFive's mvendorid" or that > kinda thing, I've heard talk of the SoC vendor getting their mvendorid > for custom CPU cores instead of the CPU vendor and it's possible for > the SBI implementation to "adjust" the values also. Okay that may be possible but that is up to the vendor when that happens. The vendor extensions are fundamentally different from the standard extensions and have even less guarantees of correctness which seems like it would invite more errata if multiple vendors implement the same vendor extensions. I can extract the code into a different file for each vendor so that is more clear. - Charlie > > > xlinuxenvcfg does not fit into this scheme however. This scheme assumes > > that a hart cannot have multiple vendors which that extension breaks. > > xlinuxenvcfg is really filling a hole in the standard isa that is > > applicible to all vendors and does not appear in the device tree so it > > is okay for that to live outside this scheme. > > Ye, xlinuxenvcfg is an internal psuedo-extension that should be treated > more like a standard one than something vendor. > > > > > It is also unecessary to push off the refactoring until we have some > > > > "sufficient" amount of vendor extensions to deem changing the > > > > infrastructure when I already have the patch available here. This does > > > > not introduce any extra overhead to existing functions and will be able > > > > to support vendors into the future. > > > > > > Yeah, maybe that's true but this was my gut reaction before reading the > > > other patch in detail (which I've still yet to do). > > > > - Charlie > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel