Re: [PATCH v4] RISC-V: Show accurate per-hart isa in /proc/cpuinfo

[Evan Green]

On Tue, Aug 29, 2023 at 1:48 AM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote:
>
> Hi Evan,
>
> Here's my stab at new wording.
>
> On Tue, Jul 11, 2023 at 01:18:30PM -0700, Evan Green wrote:
> ...
> > diff --git a/Documentation/riscv/uabi.rst b/Documentation/riscv/uabi.rst
> > index 8960fac42c40..afdda580e5a2 100644
> > --- a/Documentation/riscv/uabi.rst
> > +++ b/Documentation/riscv/uabi.rst
> > @@ -42,6 +42,16 @@ An example string following the order is::
> >
> >     rv64imadc_zifoo_zigoo_zafoo_sbar_scar_zxmbaz_xqux_xrux
> >
> > +"isa" vs "hart isa" lines in /proc/cpuinfo
> > +------------------------------------------
> > +
> > +The "isa" line in /proc/cpuinfo describes the lowest common denominator of
> > +RISC-V ISA extensions understood by the kernel and implemented on all harts. The
> > +"hart isa" line, in contrast, describes the set of extensions understood by the
> > +kernel on the particular hart being described, even if those extensions may not
> > +be present on all harts in the system. The "hart isa" line is consistent with
> > +what's returned by __riscv_hwprobe() when querying for that specific CPU.
> > +
>
> The "isa" and "hart isa" lines in /proc/cpuinfo list RISC-V ISA extensions
> which the kernel can identify (the kernel recognizes the extension's name)
> and have not been filtered out due to effectively not being present.  An
> extension is effectively not present when it is unusable, either due to
> defects (which the kernel is aware of), due to missing information which
> is necessary to complete the extension's description, or due to being
> explicitly "hidden", such as when a kernel command line parameter
> instructs the kernel to pretend the extension is not present.  Note, an
> extension's presence in a list does not imply the kernel is using the
> extension, nor does it imply that userspace or guest kernels may use the
> extension (__riscv_hwprobe() should be queried for userspace usability.
> The hypervisor should be queried, using hypervisor-specific APIs, to
> check guest kernel usability.)
>
> The "isa" line describes the lowest common denominator of extensions,
> which are the extensions implemented on all harts.  In contrast, the
> extensions listed in the "hart isa" line need not be implemented by
> any other hart than the hart corresponding to the line.
>
> ---
>
> I've specifically dropped the 'The "hart isa" line is consistent with
> what's returned by __riscv_hwprobe()...' part because I suspect hwprobe
> could at least lag what gets put in "hart isa", since the kernel may be
> taught about an extension for a different purpose first, neglecting
> hwprobe. And, there may be cases that hwprobe never enumerates an
> extension which the kernel does.

Thanks for this. My v5 had also dropped the hwprobe part. A few thoughts:

 * It seems like you want to make sure we call out the fact that the
kernel may trim out, for various reasons, ISA extensions that the
hardware does in fact support. This seems reasonable, but I don't
think we need to enumerate the complete list of "why" this might
happen, as that list is likely to go stale.
 * The "kernel is using the extension" part is a slightly confusing
perspective in this context, as it sort of implies the kernel has its
own agenda :). I'd expect users looking at /proc/cpuinfo are mostly
trying to figure out what extensions they themselves can use, and the
kernel's behavior factors in only insofar as it's required to support
the user in using a feature. Mostly I guess this is a phrasing nit.
 * The bringing up of guest kernels also seems confusing to me in the
context of /proc/cpuinfo. I'd expect discussions on how host ISA
extensions filter into guest OSes to be in a hypervisor-specifc
document, or at least a section dedicated to virtualization.
 * I hesitated in adding prescriptive guidance on what users should
do, as I think this section will hold up better over time if it just
describes current characteristics, rather than attempting to prescribe
behavior. If we want a prescriptive documentation on "use this for
that", that should probably be its own section

If I try to fold the gist of what you wrote into v5, I get something
like this (also with a very slight section heading change). Let me
know what you think:

"isa" and "hart isa" lines in /proc/cpuinfo
------------------------------------------

The "isa" line in /proc/cpuinfo describes the lowest common denominator of
RISC-V ISA extensions recognized by the kernel and implemented on all harts. The
"hart isa" line, in contrast, describes the set of extensions recognized by the
kernel on the particular hart being described, even if those extensions may not
be present on all harts in the system.

In both lines, the presence of an extension guarantees only that the
hardware has the described capability.
Additional kernel support or policy changes may be required before an
extension's capability is fully usable by userspace programs.

Inversely, the absence of an extension in these lines does not
necessarily mean the hardware does not support that feature. The
running kernel may not recognize the extension, or may have
deliberately disabled access to it.

-Evan