On Wed, Aug 30, 2023 at 10:33:04AM -0700, Evan Green wrote: > On Wed, Aug 30, 2023 at 2:03 AM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote: > > > > On Tue, Aug 29, 2023 at 10:20:04AM -0700, Evan Green wrote: > > > 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. > > > > I agree it's better to not [try to] list all the possibilities, assuming > > we can come up with good, general words to capture the idea. > > > > > * 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. > > > > We'll have plenty of S-mode extensions listed in these strings. Users > > who recognize S-mode extensions may want to know if they're listed because > > the kernel is applying them (and wouldn't be listed otherwise), or whether > > they're listed simply because they exist on the hart(s). > > I see. You're right I was thinking only about U-mode extensions. > > > > > > * 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. > > > > If there weren't S-mode extensions being listed, then I would agree, > > but, since there are, it seems odd to not explain what it means for > > them to be there wrt host and guest kernels. > > I'm not a virtualization guy, but my impression was people didn't have > expectations that everything they saw in cpuinfo would be wholesale > presented to guest VMs. There's always that layer of hypervisor > configuration that may strip out some features. So I'm still not super > convinced guest VMs need a carveout/caveat here, but let me see if I > can fold it in. > > > > > > * 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 > > > > I guess the guidance you're referring to is the "(__riscv_hwprobe() should > > be queried for userspace usability. The hypervisor should be queried, > > using hypervisor-specific APIs, to check guest kernel usability.)" bit. > > I'm fine with dropping that or moving it to another section, but I think > > the more we point out hwprobe, the better. If developers are reading this > > /proc/cpuinfo section because they want to detect extensions, then I'd > > prefer the section redirects them to hwprobe. > > Fair enough. I still haven't brought myself to wedge in an ad for > hwprobe, but I also don't disagree with this :) > > > > > > > > > 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 > > > ------------------------------------------ > > > > need one more _ > > > > > > > > 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. > > > > or guest kernels in the case of S-mode extensions. > > > > > > > > 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. > > > > I'm not sure about the word "disabled". The kernel can only disable U-mode > > extensions and S-mode extensions for guests. S-mode extensions for the > > current kernel would have to be disabled by its next higher privilege > > level. > > > > How about "...may not recognize the extension, or may have deliberately > > removed it from the listing." > > Perfect. > > > > > (But then readers will wonder why an extension would be deliberately > > removed from the listing, which brings us back to trying to come up > > with general words to capture the cases I listed. Or, maybe we don't > > have to care if they wonder why in this section/document.) > > I know, I felt the same pull to explain why as well. But I think given > that our goal with this section is mostly to make the distinction of > "presence != active", explaining exactly why the kernel may remove it > is not strictly necessary. All of my attempts to tack something on end > up being enumerated lists, which don't come out well and muddle the > message. > > Here's another shot (line endings may be wonky): > > "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. > Similarly, for S-mode extensions, presence in one of these lines does > not guarantee that the kernel is taking advantage of the extension, or > that the feature will be visible in guest VMs managed by this kernel. > > 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 removed it from the listing. Looks good to me! Thanks, drew
