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 70259C4345F for ; Fri, 12 Apr 2024 21:32: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=fS6mvHC8pjmEkic653ALDqq0Mop8F3uHJAlByXcyJm8=; b=atQsB8I+CNZkfb FWZTBes56WBEfnZXvl/hPsxvsgS4JG+U9a/1JmygRGduqtYPjbVIG3ueYeBcsvJ1vjMgXl/ZLTTlZ 67TIBkohPIQ0Hu6NN/s9DqR3cMGQNJ0ytWaHXmuwBQj4XxMGogrBvOKKCteIw52vjFuMZ9uAqxK3Y 6okg2oqwSDxue9DODWMvNLHMn7KhHYUhyyf+r2bbd/MsPKSq+D/0RIhsWT48dVFHBIv/Ws7FM3EsA x1ELT4Ypiz80ZccjxOhjKqM4whkN3t13QjY1zREQoLQ3HU//EIIcBxbCpERSC/egsu8QeZUm6gWB3 9VyNjT53RlY8oLEFESyQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvOUy-00000001OcN-1rq8; Fri, 12 Apr 2024 21:31:52 +0000 Received: from mail-pf1-x434.google.com ([2607:f8b0:4864:20::434]) by bombadil.infradead.org with esmtps (Exim 4.97.1 #2 (Red Hat Linux)) id 1rvOUu-00000001OYM-2K4L for linux-arm-kernel@lists.infradead.org; Fri, 12 Apr 2024 21:31:50 +0000 Received: by mail-pf1-x434.google.com with SMTP id d2e1a72fcca58-6ece8991654so1289614b3a.3 for ; Fri, 12 Apr 2024 14:31:46 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20230601.gappssmtp.com; s=20230601; t=1712957505; x=1713562305; 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=voVXHLevuqJNzT+bs7/EqrNPPg6u1fkprZIxe/QalLU=; b=WM9oG7NF8CwNDThqTuvJWxR2eJACH9Hb5U5WEtntycbRpT5uQw/k5RQW3EhnYf7837 YSiqYYGpvhWBM7SqYlIkZ9JlxmW/S2QeI+wpZgwpgJtA88mgtj14nmQNHqboocwHukek qB265OG9gpqBa0HbsDiK23fRBtJtGoVZhqcmDvlSw3z+al8AWp5ZDrTU0gL2sFu5RbP0 WBMcGO8p+2GPASQ8uHt+14IBpKLREctFTI22zhNtOVloJnaOIwc2yRKFNCFEDUG29rwW qvS9FSsoPqxJD50AJgoC+K2R8PuG28GfsgmP1B1lEb/syUpLXQXx5CsZW3KHfPv2ywto A0nw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1712957505; x=1713562305; 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=voVXHLevuqJNzT+bs7/EqrNPPg6u1fkprZIxe/QalLU=; b=ON/u9H3gy6q1OWG4+FufFI1Cx8qXk0LvXK9VSl6mHnoD7/McAm6BdrxXYoCQYJuH+w 9S4KgMz6VJ9TDDsU9TKMpYAn7xLUkA0jAl5LiDlySuT0Vmdhde43d70kqoyNbhDgODi2 I0UFIyJls6ReZnOf+h0HsVIccQRqPLfcfWVFSr+jcUVPtFMQcv9wbDWoAvg3FcbRARXX 6xveiCzKFqA9syX9Ot/RcaJrCGA0cnZbnDlzvvcOUGc/WdHxJDRvcctUm4Tle+aML5av Q1O+pM4ef/VAF5NNlkeATTkzdlug4tUIx69KjgBl4nU//OAVzUuq/GBi9Rf/ImKw1MWb dVbg== X-Forwarded-Encrypted: i=1; AJvYcCUuVwLlpD/c1SF1DzkRjFSASx9TSsMApElbhLPy+0wbHSvQtumolewKzD5Gzw79bq3qgqKRtb67AwQRGdCyvIUgxwVZ4erUqCjtKZ/I7UxspAgrTX0= X-Gm-Message-State: AOJu0YzSWUYtULvP+Jdd/VkzBwRTSyt6ATOuzKRB4xHQuT/mI+Cd+VZN NFj/u5MGiBssWl9ZWdWsPsDqDZu1r0x4Q359jelye/7EsAI29Ak2VWP9ZCM9Az0= X-Google-Smtp-Source: AGHT+IERI9B+BrmtTh0aeho0vx1NlGjhWinYXb/3YQQZhSK7SAPG1+V9dZjVR23iA+HJ2H/wGMNAgA== X-Received: by 2002:a05:6a20:5d92:b0:1a7:3d5f:dd3b with SMTP id km18-20020a056a205d9200b001a73d5fdd3bmr3547889pzb.16.1712957505558; Fri, 12 Apr 2024 14:31:45 -0700 (PDT) Received: from ghost ([50.145.13.30]) by smtp.gmail.com with ESMTPSA id h15-20020a63384f000000b005dc5289c4edsm3063267pgn.64.2024.04.12.14.31.44 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 12 Apr 2024 14:31:45 -0700 (PDT) Date: Fri, 12 Apr 2024 14:31:42 -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 02/19] riscv: cpufeature: Fix thead vector hwcap removal 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-2-4af9815ec746@rivosinc.com> <20240412-tuesday-resident-d9d07e75463c@wendy> <20240412-eastcoast-disparity-9c9e7d178df5@spud> <20240412-chemist-haunt-0a30a8f280ca@spud> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20240412-chemist-haunt-0a30a8f280ca@spud> X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20240412_143148_604681_8AC2A5D9 X-CRM114-Status: GOOD ( 57.20 ) 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:27:47PM +0100, Conor Dooley wrote: > On Fri, Apr 12, 2024 at 01:48:46PM -0700, Charlie Jenkins wrote: > > On Fri, Apr 12, 2024 at 07:47:48PM +0100, Conor Dooley wrote: > > > On Fri, Apr 12, 2024 at 10:12:20AM -0700, Charlie Jenkins wrote: > > > > On Fri, Apr 12, 2024 at 11:25:47AM +0100, Conor Dooley wrote: > > > > > On Thu, Apr 11, 2024 at 09:11:08PM -0700, Charlie Jenkins wrote: > > > > > > The riscv_cpuinfo struct that contains mvendorid and marchid is not > > > > > > populated until all harts are booted which happens after the DT parsing. > > > > > > Use the vendorid/archid values from the DT if available or assume all > > > > > > harts have the same values as the boot hart as a fallback. > > > > > > > > > > > > Fixes: d82f32202e0d ("RISC-V: Ignore V from the riscv,isa DT property on older T-Head CPUs") > > > > > > > > > > If this is our only use case for getting the mvendorid/marchid stuff > > > > > from dt, then I don't think we should add it. None of the devicetrees > > > > > that the commit you're fixing here addresses will have these properties > > > > > and if they did have them, they'd then also be new enough to hopefully > > > > > not have "v" either - the issue is they're using whatever crap the > > > > > vendor shipped. > > > > > > > > Yes, the DT those shipped with will not have the property in the DT so > > > > will fall back on the boot hart. The addition of the DT properties allow > > > > future heterogenous systems to be able to function. > > > > > > I think you've kinda missed the point about what the original code was > > > actually doing here. Really the kernel should not be doing validation of > > > the devicetree at all, but I was trying to avoid people shooting > > > themselves in the foot by doing something simple that would work for > > > their (incorrect) vendor dtbs. > > > Future heterogenous systems should be using riscv,isa-extensions, which > > > is totally unaffected by this codepath (and setting actual values for > > > mimpid/marchid too ideally!). > > > > > > > I am on the same page with you about that. > > > > > > > If we're gonna get the information from DT, we already have something > > > > > that we can look at to perform the disable as the cpu compatibles give > > > > > us enough information to make the decision. > > > > > > > > > > I also think that we could just cache the boot CPU's marchid/mvendorid, > > > > > since we already have to look at it in riscv_fill_cpu_mfr_info(), avoid > > > > > repeating these ecalls on all systems. > > > > > > > > Yeah that is a minor optimization that can I can apply. > > > > > > > > > > > > > > Perhaps for now we could just look at the boot CPU alone? To my > > > > > knowledge the systems that this targets all have homogeneous > > > > > marchid/mvendorid values of 0x0. > > > > > > > > They have an mvendorid of 0x5b7. > > > > > > That was a braino, clearly I should have typed "mimpid". > > > > > > > This is already falling back on the boot CPU, but that is not a solution > > > > that scales. Even though all systems currently have homogenous > > > > marchid/mvendorid I am hesitant to assert that all systems are > > > > homogenous without providing an option to override this. > > > > > > There are already is an option. Use the non-deprecated property in your > > > new system for describing what extesions you support. We don't need to > > > add any more properties (for now at least). > > > > The issue is that it is not possible to know which vendor extensions are > > associated with a vendor. That requires a global namespace where each > > extension can be looked up in a table. I have opted to have a > > vendor-specific namespace so that vendors don't have to worry about > > stepping on other vendor's toes (or the other way around). In order to > > support that, the vendorid of the hart needs to be known prior. > > Nah, I think you're mixing up something like hwprobe and having > namespaces there with needing namespacing on the devicetree probing side > too. You don't need any vendor namespacing, it's perfectly fine (IMO) > for a vendor to implement someone else's extension and I think we should > allow probing any vendors extension on any CPU. I am not mixing it up. Sure a vendor can implement somebody else's extension, they just need to add it to their namespace too. - Charlie > > > I know a rebuttal here is that this is taking away from the point of > > the original patch. I can split this patch up if so. The goal here is to > > allow vendor extensions to play nicely with the rest of the system. > > There are two uses of the mvendorid DT value, this fix, and the patch > > that adds vendor extension support. I felt that it was applicable to > > wrap the mvendorid DT value into this patch, but if you would prefer > > that to live separate of this fix then that is fine too. > > > > - Charlie > > > > > > > > > The overhead is > > > > looking for a field in the DT which does not seem to be impactful enough > > > > to prevent the addition of this option. > > > > > > > > > > > > > > > Signed-off-by: Charlie Jenkins > > > > > > > > > > > @@ -514,12 +521,23 @@ static void __init riscv_fill_hwcap_from_isa_string(unsigned long *isa2hwcap) > > > > > > pr_warn("Unable to find \"riscv,isa\" devicetree entry\n"); > > > > > > continue; > > > > > > } > > > > > > + if (of_property_read_u64(node, "riscv,vendorid", &this_vendorid) < 0) { > > > > > > + pr_warn("Unable to find \"riscv,vendorid\" devicetree entry, using boot hart mvendorid instead\n"); > > > > > > > > > > This should 100% not be a warning, it's not a required property in the > > > > > binding. > > > > > > > > Yes definitely, thank you. > > > > > > > > - Charlie > > > > > > > > > > > > > > Cheers, > > > > > Conor. > > > > > > > > > > > + this_vendorid = boot_vendorid; > > > > > > + } > > > > > > > > > > > > > > > > > _______________________________________________ linux-arm-kernel mailing list linux-arm-kernel@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-arm-kernel