On Mon, Aug 28, 2023 at 09:44:55AM -0700, Evan Green wrote: > On Sat, Aug 26, 2023 at 1:01 AM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote: > > > > On Fri, Aug 25, 2023 at 03:51:28PM -0700, Evan Green wrote: > > > On Fri, Aug 25, 2023 at 1:16 AM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote: > > > > > > > > On Thu, Aug 24, 2023 at 03:06:53PM -0700, Evan Green wrote: > > > > > On Thu, Aug 24, 2023 at 10:29 AM Conor Dooley <conor@xxxxxxxxxx> wrote: > > > > ... > > > > > > Do you want to have this new thing in cpuinfo tell the user "this hart > > > > > > has xyz extensions that are supported by a kernel, but maybe not this > > > > > > kernel" or to tell the user "this hart has xyz extensions that are > > > > > > supported by this kernel"? Your text above says "understood by the > > > > > > kernel", but I think that's a poor definition that needs to be improved > > > > > > to spell out exactly what you mean. IOW does "understood" mean the > > > > > > kernel will parse them into a structure, or does it mean "yes you can > > > > > > use this extension on this particular hart". > > > > > > > > > > I'm imagining /proc/cpuinfo being closer to "the CPU has it and the > > > > > kernel at least vaguely understands it, but may not have full support > > > > > for it enabled". I'm assuming /proc/cpuinfo is mostly used by 1) > > > > > humans wanting to know if they have hardware support for a feature, > > > > > and 2) administrators collecting telemetry to manage fleets (ie do I > > > > > have any hardware deployed that supports X). > > > > > > > > Is (2) a special case of (1)? (I want to make sure I understand all the > > > > cases.) > > > > > > More or less, yes. In bucket two are also folks wondering things like > > > "are all these crash reports I'm getting specific to machines with X". > > > > > > > > > > > > Programmers looking to > > > > > see "is the kernel support for this feature ready right now" would > > > > > ideally not be parsing /proc/cpuinfo text, as more direct mechanisms > > > > > like specific hwprobe bits for "am I fully ready to go" would be > > > > > easier to work with. Feel free to yell at me if this overall vision > > > > > seems flawed. > > > > > > > > > > I tried to look to see if there was consensus among the other > > > > > architectures. Aarch64 seems to go with "supported and fully enabled", > > > > > as their cpu_has_feature() directly tests elf_hwcap, and elements in > > > > > arm64_elf_hwcaps[] are Kconfig gated. X86 is complicated, but IIRC is > > > > > more along the lines of "hardware has it". They have two macros, > > > > > cpu_has() for "raw capability" and cpu_feature_enabled() for "kernel > > > > > can do it too", and they use cpu_has() for /proc/cpuinfo flags. > > > > > > > > > > > > > I'd lean more towards the way AArch64 goes, because, unless /proc/cpuinfo > > > > is just a blind regurgitation of an isa string from DT / ACPI, then the > > > > kernel must at least know something about it. Advertising a feature which > > > > is known, but also known not to work, seems odd to me. So my vote is that > > > > only features which are present and enabled in the kernel or present and > > > > not necessary to be enabled in the kernel in order for userspace or > > > > virtual machines to use be advertised in /proc/cpuinfo. > > > > > > > > We still have SMBIOS (dmidecode) to blindly dump what the hardware > > > > supports for cases (1) and (2) above. > > > > > > Yeah, there's an argument to be made for that. My worry is it's a > > > difficult line to hold. The bar you're really trying to describe (or > > > at least what people might take away from it) is "if it's listed here > > > then it's fully ready to be used in userspace". But then things get > > > squishy when there are additional ways to control the use of the > > > feature (prctls() in init to turn it on, usermode policy to turn it > > > off, security doodads that disable it, etc). I'm assuming nobody wants > > > a version of /proc/cpuinfo that changes depending on which process is > > > asking. So then the line would have to be more carefully described as > > > "well, the hardware can do it, and the kernel COULD do it under some > > > circumstances, but YMMV", which ends up falling somewhat short of the > > > original goal. In my mind keeping /proc/cpuinfo as close to "here's > > > what the hardware can do" seems like a more defensible position. > > > -Evan > > > > I agree with that. I was actually even trying to say the same thing, > > but only by bringing up virtual machines. Once we decide we'll expose > > extensions to VMs, whether or not the host kernel enables them, then > > none of the other host kernel configurations matter with respect to > > advertising the feature, since the guest kernel may have a completely > > different set of configurations. > > My head spins a little when I try to picture a feature which 1) > requires kernel support to use, 2) has that kernel support turned off > in the host kernel, but 3) is passed down into guest kernels. Mine did too, but apparently these already exist for kvm guests. I can't find the exact email, but either Drew or Anup told me that Svpbmt can be used by a guest even if support for it is not present in the host kernel. Thanks, Conor. > Generally though, I agree that trying to tie the guarantees of > features in /proc/cpuinfo too much to software gets confusing when > viewed through the double lens of virtualization. > > > > > So I think we should only be filtering out extensions that are disabled > > because they're broken (have a detected erratum), have been "hidden" > > (have a kernel command line allowing them to be treated as if not > > present), or cannot be used at all due to missing accompanying hardware > > descriptions (such as block size info for CBO extensions). In all cases, > > I presume we'd wire checks up in riscv_isa_extension_check() and no > > checks would be gated on Kconfigs or anything else. And, since > > /proc/cpuinfo gets its list from the bitmap that's already filtered by > > riscv_isa_extension_check(), then, long story short, we're good to go :-) > > > > But maybe we can try to spell that policy out a bit more in > > Documentation/riscv/uabi.rst. > > Right, that sounds reasonable to me, and is consistent with the > behavior we already have. With this documentation change I was only > trying to define the lower bound, rather than the complete definition > for every case. In other words, seeing a feature in cpuinfo guarantees > only that the hardware (or virtualized hardware) supports the feature, > but that's all the language says. So for instance NOT seeing a feature > in cpuinfo doesn't necessarily mean the hardware doesn't support it. > Software turning it off for the reasons you describe IMO doesn't > contradict what's written here. I was planning to leave that tacit, > but if you have suggestions on how to spell that out I'd take them. > > -Evan
Attachment:
signature.asc
Description: PGP signature
