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;
Feedback-ID: i56a14606:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-238-g746678b8b6-fm-20230329.001-g746678b8
Mime-Version: 1.0
Message-Id: <d91ffb1e-261b-4b2f-a78f-f2846600a3e7@app.fastmail.com>
In-Reply-To: <CALs-HstAKtvORKwRWeh97SuAuYR61aiR-3jA2_0JCZGAJXVHbg@mail.gmail.com>
References: <20230221190858.3159617-1-evan@rivosinc.com>
 <20230221190858.3159617-3-evan@rivosinc.com>
 <605fb2fd-bda2-4922-92bf-e3e416d54398@app.fastmail.com>
 <CALs-HstAKtvORKwRWeh97SuAuYR61aiR-3jA2_0JCZGAJXVHbg@mail.gmail.com>
Date:   Fri, 31 Mar 2023 15:21:15 +0200
From:   "Arnd Bergmann" <arnd@arndb.de>
To:     "Evan Green" <evan@rivosinc.com>
Cc:     "Palmer Dabbelt" <palmer@rivosinc.com>,
        =?UTF-8?Q?Heiko_St=C3=BCbner?= <heiko@sntech.de>,
        "Conor Dooley" <conor@kernel.org>, slewis@rivosinc.com,
        "Vineet Gupta" <vineetg@rivosinc.com>,
        "Albert Ou" <aou@eecs.berkeley.edu>,
        "Andrew Bresticker" <abrestic@rivosinc.com>,
        "Andrew Jones" <ajones@ventanamicro.com>,
        "Anup Patel" <apatel@ventanamicro.com>,
        "Atish Patra" <atishp@rivosinc.com>,
        "Bagas Sanjaya" <bagasdotme@gmail.com>,
        "Celeste Liu" <coelacanthus@outlook.com>,
        "Conor.Dooley" <conor.dooley@microchip.com>,
        guoren <guoren@kernel.org>, "Jonathan Corbet" <corbet@lwn.net>,
        "Niklas Cassel" <niklas.cassel@wdc.com>,
        "Palmer Dabbelt" <palmer@dabbelt.com>,
        "Paul Walmsley" <paul.walmsley@sifive.com>,
        "Randy Dunlap" <rdunlap@infradead.org>,
        "Ruizhe Pan" <c141028@gmail.com>,
        "Sunil V L" <sunilvl@ventanamicro.com>,
        "Tobias Klauser" <tklauser@distanz.ch>, linux-doc@vger.kernel.org,
        linux-kernel@vger.kernel.org, linux-riscv@lists.infradead.org
Subject: Re: [PATCH v3 2/7] RISC-V: Add a syscall for HW probing
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Thu, Mar 30, 2023, at 20:30, Evan Green wrote:
> On Thu, Feb 23, 2023 at 2:06=E2=80=AFAM Arnd Bergmann <arnd@arndb.de> =
wrote:
>> > +    long sys_riscv_hwprobe(struct riscv_hwprobe *pairs, size_t
>> > pair_count,
>> > +                           size_t cpu_count, cpu_set_t *cpus,
>> > +                           unsigned long flags);
>>
>> The cpu set argument worries me more: there should never be a
>> need to optimize for broken hardware that has an asymmetric set
>> of features. Just let the kernel figure out the minimum set
>> of features that works across all CPUs and report that like we
>> do with HWCAP. If there is a SoC that is so broken that it has
>> important features on a subset of cores that some user might
>> actually want to rely on, then have them go through the slow
>> sysfs interface for probing the CPUs indidually, but don't make
>> the broken case easier at the expense of normal users that
>> run on working hardware.
>
> I'm not so sure. While I agree with you for major classes of features
> (eg one CPU has floating point support but another does not), I expect
> these bits to contain more subtle details as well, which might vary
> across asymmetric implementations without breaking ABI compatibility
> per-se. Maybe some vendor has implemented exotic video decoding
> acceleration instructions that only work on the big core. Or maybe the
> big cores support v3.1 of some extension (where certain things run
> faster), but the little cores only have v3.0, where it's a little
> slower. Certain apps would likely want to know these things so they
> can allocate their work optimally across cores.

Do you have a specific feature in mind where hardware would be
intentionally designed this way? I still can't come up with a
scenario where this would actually work in practice, as having
asymmetric features is incompatible with so many other things
we normally do.

- In a virtual machine, the VCPU tents to get scheduled arbitrarily
  to physical CPUs, so setting affinity in a guest won't actually
  guarantee that the feature is still there.

- Using a CPU feature from library code is practically impossible
  if it requires special CPU affinity, as the application may
  already be started on specific CPUs for another reason, and
  having a library call sched_setaffinity will conflict with those.

- Even in the simplest case of having a standalone application
  without any shared libraries try to pick a sensible CPU to
  run on is hard to do in a generic way, as it would need to
  weigh availabilty of features on certain cores against the
  number of cores with or without the feature and their current
  and expected system load.

As long as there isn't a specific requirement, I think it's better
to not actually encourage hardware vendors to implement designs
like that, or at least not designing an interface to make getting
this information a few microseconds faster that what already
exists.

      Arnd