On Tue, Feb 15, 2022 at 11:06:24AM -0800, Atish Kumar Patra wrote: > On Tue, Feb 15, 2022 at 8:04 AM Jisheng Zhang <jszhang@xxxxxxxxxx> wrote: > > > > On Tue, Feb 15, 2022 at 01:02:05AM -0800, Atish Patra wrote: > > > This series implements a generic framework to parse multi-letter ISA > > > extensions. This series is based on Tsukasa's v3 isa extension improvement > > > series[1]. I have fixed few bugs and improved comments from that series > > > (PATCH1-3). I have not used PATCH 4 from that series as we are not using > > > ISA extension versioning as of now. We can add that later if required. > > > > > > PATCH 4 allows the probing of multi-letter extensions via a macro. > > > It continues to use the common isa extensions between all the harts. > > > Thus hetergenous hart systems will only see the common ISA extensions. > > > > > > PATCH 6 improves the /proc/cpuinfo interface for the available ISA extensions > > > via /proc/cpuinfo. > > > > > > Here is the example output of /proc/cpuinfo: > > > (with debug patches in Qemu and Linux kernel) > > > > > > / # cat /proc/cpuinfo > > > processor : 0 > > > hart : 0 > > > isa : rv64imafdcsu > > > isa-ext : sstc,sscofpmf > > > mmu : sv48 > > > > > > processor : 1 > > > hart : 1 > > > isa : rv64imafdcsu > > > isa-ext : sstc,sscofpmf > > > mmu : sv48 > > > > > > processor : 2 > > > hart : 2 > > > isa : rv64imafdcsu > > > isa-ext : sstc,sscofpmf > > > mmu : sv48 > > > > > > processor : 3 > > > hart : 3 > > > isa : rv64imafdcsu > > > isa-ext : sstc,sscofpmf > > > mmu : sv48 > > > > > > Anybody adding support for any new multi-letter extensions should add an > > > entry to the riscv_isa_ext_id and the isa extension array. > > > E.g. The patch[2] adds the support for various ISA extensions. > > > > Hi Atish, > > > > Thanks for this series. I'm thinking cpu features VS ISA extenstions. > > I'm converting the sv48 to static key: > > https://lore.kernel.org/linux-riscv/20220125165036.987-1-jszhang@xxxxxxxxxx/ > > > > Previously, I thought the SV48 as a cpu feature, and there will be > > more and more cpu features, so I implemented an unified static key > > mechanism for CPU features. But after reading this series, I think > > I may need to rebase(even reimplement) the above patch to your series. > > But I'm a bit confused by CPU features VS ISA extenstions now: > > > > 1. Is cpu feature == ISA extension? > > > > 2. Is SV48 considered as ISA extension? > > If yes, now SV48 or not is determined during runtime, but current ISA > > extensions seem parsed from DT. So how to support those ISA extensions > > which can be determined during runtime? > > > > Could you please share your thought? > > > > Here are my two cents: > > I think the cpu feature is a superset of the ISA extension. > cpu feature != ISA extension. > > While all ISA extensions are cpu features, all CPU features may not be > an ISA extension. > e.g. sv48 is not a ISA extension but F/D are (used to set the > cpu_hwcap_fpu static key) > > Moreover, not all cpu feature/ISA extension requires a static key. > e.g SSTC extension will require a static key because the check has to > happen in the hot path. > However, sscofpmf extension don't need a static key as the check > happens only one time during boot. > > We should keep these two separate but a common static framework would > be very useful. > > Here is the flow that I have in my mind. > 1. All ISA extensions will be parsed through riscv,isa DT property > 2. Any supported/enabled extension will be set in riscv_isa bitmap > 3. Any extension requiring a static key will invoke the cpus_set_cap. > > cpus_set_cap will be invoked from a different code path that uses a > static key for a specific ISA > extension or a CPU feature. > > The only problem I see here is that we have to set a bit in both > cpu_hwcaps & riscv_isa bitmap. > We also have to define the value of that bit for any extension > requiring a static key twice as well. > > I think that should be okay. But I would like to hear what everybody > else thinks as well. > Thank Atish's input. I notice that SV57 support is merged, I'll send a new version to apply static mechanism to both SV48 and SV57 once rc1 is released.
