Message ID | 20230623222353.3742384-1-evan@rivosinc.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:994d:0:b0:3d9:f83d:47d9 with SMTP id k13csp6074158vqr; Fri, 23 Jun 2023 15:27:11 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ6TtGtLNE/yEC5TiVNN180C6IaJTmCOmjKSHjqv501d+/+D/qtwKYC53acFCXQRoWLP2GT4 X-Received: by 2002:a05:6a21:3293:b0:10a:ee1b:fdc4 with SMTP id yt19-20020a056a21329300b0010aee1bfdc4mr28205018pzb.47.1687559231384; Fri, 23 Jun 2023 15:27:11 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1687559231; cv=none; d=google.com; s=arc-20160816; b=FB4sooLdmr5edhh0NvSHiCakW3hxB663tYGI18Ti5LJKqU+4yONbhPkuCroJqY1XUE ZkzTJrfw6tS5JHBAUwbaowbNyuFqVKTPPcs6g2Jw4l8Wcb9luFjv0JIQstBPd3Und8Uy WRv+jxVORAO+wXptXWrbjzD70hbhLqK590ax96hkCiilGbqgmYyYZDJUuDBwMEv0/bNr PqA4DYgdzTI0husgI0iX8tCyY/d4nGcgW0nztEoPoUmkU2sqKQNJZIqKYZaEiFSOF0XK 50PWuBJLhMcGJq9fEtRJAN4SfViGDzTIS4MASLRnMV0tIiMiy0n5PWhkNOW+uESR7lb/ sHZw== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=2T74NvB61j3v2x3seml7lOG7zGtTOHQZiKI9MSy1E1w=; fh=10xrWwVTi0XDrgrqj+Y6IQXzz3alrsIME2XN+GlTKaA=; b=delnGZtsQPbk1B/s2Q+wkAd8GttfUN3bwkyyoOurBEuNVs1oeS7L0SyN+9Wouw+6fT r7ekxEbrlFkTl47xf5rw5edburW3xrl1vH7USB+g0tHpM9K2FOj95j5+vSzxPhjfOkpH RFglluNuSmhUhBcH7YI67KLI0hOkgX7bNY9AoEZ6f5+j7Yqrs5uUElRBP5fxlM+0dsCS LBZr0rSO/8RYc4oq8kp1uVRzwf72okS1VQSQ8bp+V99Lczta2w+Y5qa480XO/RaYoc52 JFVxN9VLaAA+SGQuFpQhwMD9ajnjdjtmW6/evCThAh1cPocuJ5y3atsVePHgBgAd0s5w v5OQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@rivosinc-com.20221208.gappssmtp.com header.s=20221208 header.b=2EFwKpEp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id c3-20020a170902848300b001b22c8d0dc5si120844plo.437.2023.06.23.15.26.59; Fri, 23 Jun 2023 15:27:11 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@rivosinc-com.20221208.gappssmtp.com header.s=20221208 header.b=2EFwKpEp; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229786AbjFWWYE (ORCPT <rfc822;maxin.john@gmail.com> + 99 others); Fri, 23 Jun 2023 18:24:04 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54430 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229558AbjFWWYD (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 23 Jun 2023 18:24:03 -0400 Received: from mail-pg1-x52f.google.com (mail-pg1-x52f.google.com [IPv6:2607:f8b0:4864:20::52f]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 2FA71172D for <linux-kernel@vger.kernel.org>; Fri, 23 Jun 2023 15:24:02 -0700 (PDT) Received: by mail-pg1-x52f.google.com with SMTP id 41be03b00d2f7-54fb3c168fcso966413a12.0 for <linux-kernel@vger.kernel.org>; Fri, 23 Jun 2023 15:24:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=rivosinc-com.20221208.gappssmtp.com; s=20221208; t=1687559041; x=1690151041; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=2T74NvB61j3v2x3seml7lOG7zGtTOHQZiKI9MSy1E1w=; b=2EFwKpEpn1GuiCAmqPCRobOJDLmPT0lTU5+mX/qqokC8SaZkni1sEd6wjT1x8qKcRY dNT0eYjJCgXS/Q/dR3chEQTYwi4xFRQJbu5blR4pzfE3h6HZPLkY4jlTRHuF3pyF7lWp 4Zn3U4H7KcZy5oRilvTBMSFAHNsEwnk9XJAUccADwRq0dFw1AqHMlUoXkddAv025GINS 8mY0ueJmqA5CxgM7M71zf6Oj6uKQj1WmH72AgwMqxEwrePfKsVOYBPJApmjFeatj6s4J YPAA6nyMGs2zJqY3GgSPTNUIce+PgAOwH839GsTPnttAOCmSHoxNSB/3q7IY70UxqN6/ wCfg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1687559041; x=1690151041; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=2T74NvB61j3v2x3seml7lOG7zGtTOHQZiKI9MSy1E1w=; b=SHSByPasC8vHNuAlD2ugjbZqbah3hkn3+uBsSd3S5BWAiMKXun/geotAmjEA2XlMDb flWz9rI9GK6BG6vrNzz7XLfILamPq6rW6o6/M6y9qtifdD8h8XhwURVxHYdgeRfzfXJo 5Jwb1YD+qODl2fv0G9ST62uqCMv9bfUV1WTe31OjYMuRMKy2eIv+KlwMXt9ksrnM7Iz6 m5+8bhWOTJIfY8CXO1SCtaSrBSQgxgEEGuLMLMFPMtBmTaM5s7dhATN/zN/uN2/s/yXo FmZmpKRl2Rd7147tT3gViZYiPJKjmHuhkZPOEVqz5ry9jd+vv81FAxHUHwAY/x/W9hyr dvaQ== X-Gm-Message-State: AC+VfDxdygjvm9LDNnMK1GzdFmWUog9RFXy8pPlaG7+kZLx0yxT/uAQC QzGC3gIoB4WWfVM9E65oNVU9Yg== X-Received: by 2002:a05:6a21:3293:b0:10a:ee1b:fdc4 with SMTP id yt19-20020a056a21329300b0010aee1bfdc4mr28198131pzb.47.1687559041720; Fri, 23 Jun 2023 15:24:01 -0700 (PDT) Received: from evan.ba.rivosinc.com ([66.220.2.162]) by smtp.gmail.com with ESMTPSA id m7-20020aa78a07000000b0063f1a1e3003sm19429pfa.166.2023.06.23.15.24.00 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Fri, 23 Jun 2023 15:24:01 -0700 (PDT) From: Evan Green <evan@rivosinc.com> To: Palmer Dabbelt <palmer@rivosinc.com> Cc: Evan Green <evan@rivosinc.com>, Albert Ou <aou@eecs.berkeley.edu>, Andrew Jones <ajones@ventanamicro.com>, Anup Patel <apatel@ventanamicro.com>, Conor Dooley <conor.dooley@microchip.com>, Heiko Stuebner <heiko.stuebner@vrull.eu>, Palmer Dabbelt <palmer@dabbelt.com>, Paul Walmsley <paul.walmsley@sifive.com>, Sunil V L <sunilvl@ventanamicro.com>, linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org Subject: [PATCH] RISC-V: Show accurate per-hart isa in /proc/cpuinfo Date: Fri, 23 Jun 2023 15:23:53 -0700 Message-Id: <20230623222353.3742384-1-evan@rivosinc.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_NONE,SPF_HELO_NONE,SPF_PASS, 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 lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1769534108354090844?= X-GMAIL-MSGID: =?utf-8?q?1769534108354090844?= |
Series |
RISC-V: Show accurate per-hart isa in /proc/cpuinfo
|
|
Commit Message
Evan Green
June 23, 2023, 10:23 p.m. UTC
In /proc/cpuinfo, most of the information we show for each processor is
specific to that hart: marchid, mvendorid, mimpid, processor, hart,
compatible, and the mmu size. But the ISA string gets filtered through a
lowest common denominator mask, so that if one CPU is missing an ISA
extension, no CPUs will show it.
Now that we track the ISA extensions for each hart, let's report ISA
extension info accurately per-hart in /proc/cpuinfo.
Signed-off-by: Evan Green <evan@rivosinc.com>
---
arch/riscv/kernel/cpu.c | 16 +++++++++-------
1 file changed, 9 insertions(+), 7 deletions(-)
Comments
Hey Evan, On Fri, Jun 23, 2023 at 03:23:53PM -0700, Evan Green wrote: > In /proc/cpuinfo, most of the information we show for each processor is > specific to that hart: marchid, mvendorid, mimpid, processor, hart, > compatible, and the mmu size. But the ISA string gets filtered through a > lowest common denominator mask, so that if one CPU is missing an ISA > extension, no CPUs will show it. > > Now that we track the ISA extensions for each hart, let's report ISA > extension info accurately per-hart in /proc/cpuinfo. No, you can't do this as it breaks the assumptions of userspace that this shows the set supported across all harts. Sorry, but NAK. Cheers, Conor.
On Fri, Jun 23, 2023 at 5:12 PM Conor Dooley <conor@kernel.org> wrote: > > Hey Evan, > > On Fri, Jun 23, 2023 at 03:23:53PM -0700, Evan Green wrote: > > In /proc/cpuinfo, most of the information we show for each processor is > > specific to that hart: marchid, mvendorid, mimpid, processor, hart, > > compatible, and the mmu size. But the ISA string gets filtered through a > > lowest common denominator mask, so that if one CPU is missing an ISA > > extension, no CPUs will show it. > > > > Now that we track the ISA extensions for each hart, let's report ISA > > extension info accurately per-hart in /proc/cpuinfo. > > No, you can't do this as it breaks the assumptions of userspace that > this shows the set supported across all harts. > Sorry, but NAK. My hope was that we were still early enough that no production systems existed (yet) that actually had different ISA extensions in the set we track, and therefore usermode would have been unable to make those assumptions at this point. If such a system exists, and I don't know if it does or not, then I agree it's too late to make a change like this. I thought I'd put this out here and see if someone could point at such a system; but if not it'd be great to keep /proc/cpuinfo accurate and consistent with hwprobe (which does return accurate per-hart ISA extension info). -Evan > > Cheers, > Conor.
On Mon, Jun 26, 2023 at 12:25:42PM -0700, Evan Green wrote: > On Fri, Jun 23, 2023 at 5:12 PM Conor Dooley <conor@kernel.org> wrote: > > On Fri, Jun 23, 2023 at 03:23:53PM -0700, Evan Green wrote: > > > In /proc/cpuinfo, most of the information we show for each processor is > > > specific to that hart: marchid, mvendorid, mimpid, processor, hart, > > > compatible, and the mmu size. But the ISA string gets filtered through a > > > lowest common denominator mask, so that if one CPU is missing an ISA > > > extension, no CPUs will show it. > > > > > > Now that we track the ISA extensions for each hart, let's report ISA > > > extension info accurately per-hart in /proc/cpuinfo. > > > > No, you can't do this as it breaks the assumptions of userspace that > > this shows the set supported across all harts. > > Sorry, but NAK. > My hope was that we were still early enough that no production systems > existed (yet) that actually had different ISA extensions in the set we > track, and therefore usermode would have been unable to make those > assumptions at this point. If such a system exists, and I don't know > if it does or not, then I agree it's too late to make a change like > this. You should put this information into your commit messages & not just hope that people understand your intent. Userspace does actually make these assumptions already, see for example this Google "cpu features" repo: https://github.com/google/cpu_features/tree/main To be quite honest, I really dislike the fragility of what they have implemented - with only one of the reasons being they made the mistake of assuming homogeneity. There's got to be a line somewhere for what constitutes buggy userspace and what's a regression. Up to Palmer I suppose as to what constitutes which. > I thought I'd put this out here and see if someone could point at such > a system; but if not it'd be great to keep /proc/cpuinfo accurate and > consistent with hwprobe (which does return accurate per-hart ISA > extension info). Just another nail in the coffin for a bad interface :) There are apparently some mixed c906 chips that support vector on one core and not the other - although it is thead vector which is not supported upstream yet... Other than that, SiFive stuff technically can be mixed - rv64imac & rv64imafdc on a bunch of the older stuff. I don't think anyone actually runs those sort of configurations on them though.
On Mon, 26 Jun 2023 13:34:24 PDT (-0700), Conor Dooley wrote: > On Mon, Jun 26, 2023 at 12:25:42PM -0700, Evan Green wrote: >> On Fri, Jun 23, 2023 at 5:12 PM Conor Dooley <conor@kernel.org> wrote: >> > On Fri, Jun 23, 2023 at 03:23:53PM -0700, Evan Green wrote: >> > > In /proc/cpuinfo, most of the information we show for each processor is >> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart, >> > > compatible, and the mmu size. But the ISA string gets filtered through a >> > > lowest common denominator mask, so that if one CPU is missing an ISA >> > > extension, no CPUs will show it. >> > > >> > > Now that we track the ISA extensions for each hart, let's report ISA >> > > extension info accurately per-hart in /proc/cpuinfo. >> > >> > No, you can't do this as it breaks the assumptions of userspace that >> > this shows the set supported across all harts. >> > Sorry, but NAK. > >> My hope was that we were still early enough that no production systems >> existed (yet) that actually had different ISA extensions in the set we >> track, and therefore usermode would have been unable to make those >> assumptions at this point. If such a system exists, and I don't know >> if it does or not, then I agree it's too late to make a change like >> this. > > You should put this information into your commit messages & not just > hope that people understand your intent. > Userspace does actually make these assumptions already, see for example > this Google "cpu features" repo: > https://github.com/google/cpu_features/tree/main > To be quite honest, I really dislike the fragility of what they have > implemented - with only one of the reasons being they made the mistake > of assuming homogeneity. > > There's got to be a line somewhere for what constitutes buggy userspace > and what's a regression. Up to Palmer I suppose as to what constitutes > which. Maybe let's just add a pretty printed version of the hwprobe info to /proc/cpuinfo, and then leave the ISA string alone as a legacy interface? Having something so poorly defined as uABI is a bit embarassing, but it's our mistake so we've got to live with it. >> I thought I'd put this out here and see if someone could point at such >> a system; but if not it'd be great to keep /proc/cpuinfo accurate and >> consistent with hwprobe (which does return accurate per-hart ISA >> extension info). > > Just another nail in the coffin for a bad interface :) > There are apparently some mixed c906 chips that support vector on one > core and not the other - although it is thead vector which is not > supported upstream yet... > > Other than that, SiFive stuff technically can be mixed - rv64imac & > rv64imafdc on a bunch of the older stuff. I don't think anyone actually > runs those sort of configurations on them though.
On Mon, Jun 26, 2023 at 1:48 PM Palmer Dabbelt <palmer@dabbelt.com> wrote: > > On Mon, 26 Jun 2023 13:34:24 PDT (-0700), Conor Dooley wrote: > > On Mon, Jun 26, 2023 at 12:25:42PM -0700, Evan Green wrote: > >> On Fri, Jun 23, 2023 at 5:12 PM Conor Dooley <conor@kernel.org> wrote: > >> > On Fri, Jun 23, 2023 at 03:23:53PM -0700, Evan Green wrote: > >> > > In /proc/cpuinfo, most of the information we show for each processor is > >> > > specific to that hart: marchid, mvendorid, mimpid, processor, hart, > >> > > compatible, and the mmu size. But the ISA string gets filtered through a > >> > > lowest common denominator mask, so that if one CPU is missing an ISA > >> > > extension, no CPUs will show it. > >> > > > >> > > Now that we track the ISA extensions for each hart, let's report ISA > >> > > extension info accurately per-hart in /proc/cpuinfo. > >> > > >> > No, you can't do this as it breaks the assumptions of userspace that > >> > this shows the set supported across all harts. > >> > Sorry, but NAK. > > > >> My hope was that we were still early enough that no production systems > >> existed (yet) that actually had different ISA extensions in the set we > >> track, and therefore usermode would have been unable to make those > >> assumptions at this point. If such a system exists, and I don't know > >> if it does or not, then I agree it's too late to make a change like > >> this. > > > > You should put this information into your commit messages & not just > > hope that people understand your intent. Fair enough. > > Userspace does actually make these assumptions already, see for example > > this Google "cpu features" repo: > > https://github.com/google/cpu_features/tree/main > > To be quite honest, I really dislike the fragility of what they have > > implemented - with only one of the reasons being they made the mistake > > of assuming homogeneity. > > > > There's got to be a line somewhere for what constitutes buggy userspace > > and what's a regression. Up to Palmer I suppose as to what constitutes > > which. > > Maybe let's just add a pretty printed version of the hwprobe info to > /proc/cpuinfo, and then leave the ISA string alone as a legacy > interface? I like it! I'll aim for that for v2. I'll resist the urge to name the row isa_for_real. > > Having something so poorly defined as uABI is a bit embarassing, but > it's our mistake so we've got to live with it. > > >> I thought I'd put this out here and see if someone could point at such > >> a system; but if not it'd be great to keep /proc/cpuinfo accurate and > >> consistent with hwprobe (which does return accurate per-hart ISA > >> extension info). > > > > Just another nail in the coffin for a bad interface :) > > There are apparently some mixed c906 chips that support vector on one > > core and not the other - although it is thead vector which is not > > supported upstream yet... > > > > Other than that, SiFive stuff technically can be mixed - rv64imac & > > rv64imafdc on a bunch of the older stuff. I don't think anyone actually > > runs those sort of configurations on them though.
diff --git a/arch/riscv/kernel/cpu.c b/arch/riscv/kernel/cpu.c index a2fc952318e9..7bb386f94f01 100644 --- a/arch/riscv/kernel/cpu.c +++ b/arch/riscv/kernel/cpu.c @@ -226,7 +226,7 @@ static struct riscv_isa_ext_data isa_ext_arr[] = { __RISCV_ISA_EXT_DATA("", RISCV_ISA_EXT_MAX), }; -static void print_isa_ext(struct seq_file *f) +static void print_isa_ext(struct seq_file *f, unsigned long cpu) { struct riscv_isa_ext_data *edata; int i = 0, arr_sz; @@ -239,7 +239,8 @@ static void print_isa_ext(struct seq_file *f) for (i = 0; i <= arr_sz; i++) { edata = &isa_ext_arr[i]; - if (!__riscv_isa_extension_available(NULL, edata->isa_ext_id)) + if (!__riscv_isa_extension_available(hart_isa[cpu].isa, + edata->isa_ext_id)) continue; seq_printf(f, "_%s", edata->uprop); } @@ -253,7 +254,7 @@ static void print_isa_ext(struct seq_file *f) */ static const char base_riscv_exts[13] = "imafdqcbkjpvh"; -static void print_isa(struct seq_file *f, const char *isa) +static void print_isa(struct seq_file *f, const char *isa, unsigned long cpu) { int i; @@ -261,11 +262,12 @@ static void print_isa(struct seq_file *f, const char *isa) /* Print the rv[64/32] part */ seq_write(f, isa, 4); for (i = 0; i < sizeof(base_riscv_exts); i++) { - if (__riscv_isa_extension_available(NULL, base_riscv_exts[i] - 'a')) + if (__riscv_isa_extension_available(hart_isa[cpu].isa, + base_riscv_exts[i] - 'a')) /* Print only enabled the base ISA extensions */ seq_write(f, &base_riscv_exts[i], 1); } - print_isa_ext(f); + print_isa_ext(f, cpu); seq_puts(f, "\n"); } @@ -324,7 +326,7 @@ static int c_show(struct seq_file *m, void *v) if (acpi_disabled) { node = of_get_cpu_node(cpu_id, NULL); if (!of_property_read_string(node, "riscv,isa", &isa)) - print_isa(m, isa); + print_isa(m, isa, cpu_id); print_mmu(m); if (!of_property_read_string(node, "compatible", &compat) && @@ -334,7 +336,7 @@ static int c_show(struct seq_file *m, void *v) of_node_put(node); } else { if (!acpi_get_riscv_isa(NULL, cpu_id, &isa)) - print_isa(m, isa); + print_isa(m, isa, cpu_id); print_mmu(m); }