On Sat, Apr 12, 2025 at 6:06 PM Andrew Jones <ajones@xxxxxxxxxxxxxxxx> wrote:
>
> On Wed, Mar 26, 2025 at 12:26:42PM +0530, Anup Patel wrote:
> > Currently, the isa_info_arr[] only include multi-letter extensions but
> > the KVM ONE_REG interface covers both single-letter and multi-letter
> > extensions so extend isa_info_arr[] to include single-letter extensions.
> >
> > Signed-off-by: Anup Patel <apatel@xxxxxxxxxxxxxxxx>
> > ---
> > riscv/fdt.c | 138 +++++++++++++++++++++++++++++-----------------------
> > 1 file changed, 76 insertions(+), 62 deletions(-)
> >
> > diff --git a/riscv/fdt.c b/riscv/fdt.c
> > index 251821e..46efb47 100644
> > --- a/riscv/fdt.c
> > +++ b/riscv/fdt.c
> > @@ -12,71 +12,81 @@
> > struct isa_ext_info {
> > const char *name;
> > unsigned long ext_id;
> > + bool multi_letter;
> > };
> >
> > struct isa_ext_info isa_info_arr[] = {
> > - /* sorted alphabetically */
> > - {"smnpm", KVM_RISCV_ISA_EXT_SMNPM},
> > - {"smstateen", KVM_RISCV_ISA_EXT_SMSTATEEN},
> > - {"ssaia", KVM_RISCV_ISA_EXT_SSAIA},
> > - {"sscofpmf", KVM_RISCV_ISA_EXT_SSCOFPMF},
> > - {"ssnpm", KVM_RISCV_ISA_EXT_SSNPM},
> > - {"sstc", KVM_RISCV_ISA_EXT_SSTC},
> > - {"svade", KVM_RISCV_ISA_EXT_SVADE},
> > - {"svadu", KVM_RISCV_ISA_EXT_SVADU},
> > - {"svinval", KVM_RISCV_ISA_EXT_SVINVAL},
> > - {"svnapot", KVM_RISCV_ISA_EXT_SVNAPOT},
> > - {"svpbmt", KVM_RISCV_ISA_EXT_SVPBMT},
> > - {"svvptc", KVM_RISCV_ISA_EXT_SVVPTC},
> > - {"zabha", KVM_RISCV_ISA_EXT_ZABHA},
> > - {"zacas", KVM_RISCV_ISA_EXT_ZACAS},
> > - {"zawrs", KVM_RISCV_ISA_EXT_ZAWRS},
> > - {"zba", KVM_RISCV_ISA_EXT_ZBA},
> > - {"zbb", KVM_RISCV_ISA_EXT_ZBB},
> > - {"zbc", KVM_RISCV_ISA_EXT_ZBC},
> > - {"zbkb", KVM_RISCV_ISA_EXT_ZBKB},
> > - {"zbkc", KVM_RISCV_ISA_EXT_ZBKC},
> > - {"zbkx", KVM_RISCV_ISA_EXT_ZBKX},
> > - {"zbs", KVM_RISCV_ISA_EXT_ZBS},
> > - {"zca", KVM_RISCV_ISA_EXT_ZCA},
> > - {"zcb", KVM_RISCV_ISA_EXT_ZCB},
> > - {"zcd", KVM_RISCV_ISA_EXT_ZCD},
> > - {"zcf", KVM_RISCV_ISA_EXT_ZCF},
> > - {"zcmop", KVM_RISCV_ISA_EXT_ZCMOP},
> > - {"zfa", KVM_RISCV_ISA_EXT_ZFA},
> > - {"zfh", KVM_RISCV_ISA_EXT_ZFH},
> > - {"zfhmin", KVM_RISCV_ISA_EXT_ZFHMIN},
> > - {"zicbom", KVM_RISCV_ISA_EXT_ZICBOM},
> > - {"zicboz", KVM_RISCV_ISA_EXT_ZICBOZ},
> > - {"ziccrse", KVM_RISCV_ISA_EXT_ZICCRSE},
> > - {"zicntr", KVM_RISCV_ISA_EXT_ZICNTR},
> > - {"zicond", KVM_RISCV_ISA_EXT_ZICOND},
> > - {"zicsr", KVM_RISCV_ISA_EXT_ZICSR},
> > - {"zifencei", KVM_RISCV_ISA_EXT_ZIFENCEI},
> > - {"zihintntl", KVM_RISCV_ISA_EXT_ZIHINTNTL},
> > - {"zihintpause", KVM_RISCV_ISA_EXT_ZIHINTPAUSE},
> > - {"zihpm", KVM_RISCV_ISA_EXT_ZIHPM},
> > - {"zimop", KVM_RISCV_ISA_EXT_ZIMOP},
> > - {"zknd", KVM_RISCV_ISA_EXT_ZKND},
> > - {"zkne", KVM_RISCV_ISA_EXT_ZKNE},
> > - {"zknh", KVM_RISCV_ISA_EXT_ZKNH},
> > - {"zkr", KVM_RISCV_ISA_EXT_ZKR},
> > - {"zksed", KVM_RISCV_ISA_EXT_ZKSED},
> > - {"zksh", KVM_RISCV_ISA_EXT_ZKSH},
> > - {"zkt", KVM_RISCV_ISA_EXT_ZKT},
> > - {"ztso", KVM_RISCV_ISA_EXT_ZTSO},
> > - {"zvbb", KVM_RISCV_ISA_EXT_ZVBB},
> > - {"zvbc", KVM_RISCV_ISA_EXT_ZVBC},
> > - {"zvfh", KVM_RISCV_ISA_EXT_ZVFH},
> > - {"zvfhmin", KVM_RISCV_ISA_EXT_ZVFHMIN},
> > - {"zvkb", KVM_RISCV_ISA_EXT_ZVKB},
> > - {"zvkg", KVM_RISCV_ISA_EXT_ZVKG},
> > - {"zvkned", KVM_RISCV_ISA_EXT_ZVKNED},
> > - {"zvknha", KVM_RISCV_ISA_EXT_ZVKNHA},
> > - {"zvknhb", KVM_RISCV_ISA_EXT_ZVKNHB},
> > - {"zvksed", KVM_RISCV_ISA_EXT_ZVKSED},
> > - {"zvksh", KVM_RISCV_ISA_EXT_ZVKSH},
> > - {"zvkt", KVM_RISCV_ISA_EXT_ZVKT},
> > + /* single-letter */
> > + {"a", KVM_RISCV_ISA_EXT_A, false},
> > + {"c", KVM_RISCV_ISA_EXT_C, false},
> > + {"d", KVM_RISCV_ISA_EXT_D, false},
> > + {"f", KVM_RISCV_ISA_EXT_F, false},
> > + {"h", KVM_RISCV_ISA_EXT_H, false},
> > + {"i", KVM_RISCV_ISA_EXT_I, false},
> > + {"m", KVM_RISCV_ISA_EXT_M, false},
> > + {"v", KVM_RISCV_ISA_EXT_V, false},
> > + /* multi-letter sorted alphabetically */
> > + {"smnpm", KVM_RISCV_ISA_EXT_SMNPM, true},
> > + {"smstateen", KVM_RISCV_ISA_EXT_SMSTATEEN, true},
> > + {"ssaia", KVM_RISCV_ISA_EXT_SSAIA, true},
> > + {"sscofpmf", KVM_RISCV_ISA_EXT_SSCOFPMF, true},
> > + {"ssnpm", KVM_RISCV_ISA_EXT_SSNPM, true},
> > + {"sstc", KVM_RISCV_ISA_EXT_SSTC, true},
> > + {"svade", KVM_RISCV_ISA_EXT_SVADE, true},
> > + {"svadu", KVM_RISCV_ISA_EXT_SVADU, true},
> > + {"svinval", KVM_RISCV_ISA_EXT_SVINVAL, true},
> > + {"svnapot", KVM_RISCV_ISA_EXT_SVNAPOT, true},
> > + {"svpbmt", KVM_RISCV_ISA_EXT_SVPBMT, true},
> > + {"svvptc", KVM_RISCV_ISA_EXT_SVVPTC, true},
> > + {"zabha", KVM_RISCV_ISA_EXT_ZABHA, true},
> > + {"zacas", KVM_RISCV_ISA_EXT_ZACAS, true},
> > + {"zawrs", KVM_RISCV_ISA_EXT_ZAWRS, true},
> > + {"zba", KVM_RISCV_ISA_EXT_ZBA, true},
> > + {"zbb", KVM_RISCV_ISA_EXT_ZBB, true},
> > + {"zbc", KVM_RISCV_ISA_EXT_ZBC, true},
> > + {"zbkb", KVM_RISCV_ISA_EXT_ZBKB, true},
> > + {"zbkc", KVM_RISCV_ISA_EXT_ZBKC, true},
> > + {"zbkx", KVM_RISCV_ISA_EXT_ZBKX, true},
> > + {"zbs", KVM_RISCV_ISA_EXT_ZBS, true},
> > + {"zca", KVM_RISCV_ISA_EXT_ZCA, true},
> > + {"zcb", KVM_RISCV_ISA_EXT_ZCB, true},
> > + {"zcd", KVM_RISCV_ISA_EXT_ZCD, true},
> > + {"zcf", KVM_RISCV_ISA_EXT_ZCF, true},
> > + {"zcmop", KVM_RISCV_ISA_EXT_ZCMOP, true},
> > + {"zfa", KVM_RISCV_ISA_EXT_ZFA, true},
> > + {"zfh", KVM_RISCV_ISA_EXT_ZFH, true},
> > + {"zfhmin", KVM_RISCV_ISA_EXT_ZFHMIN, true},
> > + {"zicbom", KVM_RISCV_ISA_EXT_ZICBOM, true},
> > + {"zicboz", KVM_RISCV_ISA_EXT_ZICBOZ, true},
> > + {"ziccrse", KVM_RISCV_ISA_EXT_ZICCRSE, true},
> > + {"zicntr", KVM_RISCV_ISA_EXT_ZICNTR, true},
> > + {"zicond", KVM_RISCV_ISA_EXT_ZICOND, true},
> > + {"zicsr", KVM_RISCV_ISA_EXT_ZICSR, true},
> > + {"zifencei", KVM_RISCV_ISA_EXT_ZIFENCEI, true},
> > + {"zihintntl", KVM_RISCV_ISA_EXT_ZIHINTNTL, true},
> > + {"zihintpause", KVM_RISCV_ISA_EXT_ZIHINTPAUSE, true},
> > + {"zihpm", KVM_RISCV_ISA_EXT_ZIHPM, true},
> > + {"zimop", KVM_RISCV_ISA_EXT_ZIMOP, true},
> > + {"zknd", KVM_RISCV_ISA_EXT_ZKND, true},
> > + {"zkne", KVM_RISCV_ISA_EXT_ZKNE, true},
> > + {"zknh", KVM_RISCV_ISA_EXT_ZKNH, true},
> > + {"zkr", KVM_RISCV_ISA_EXT_ZKR, true},
> > + {"zksed", KVM_RISCV_ISA_EXT_ZKSED, true},
> > + {"zksh", KVM_RISCV_ISA_EXT_ZKSH, true},
> > + {"zkt", KVM_RISCV_ISA_EXT_ZKT, true},
> > + {"ztso", KVM_RISCV_ISA_EXT_ZTSO, true},
> > + {"zvbb", KVM_RISCV_ISA_EXT_ZVBB, true},
> > + {"zvbc", KVM_RISCV_ISA_EXT_ZVBC, true},
> > + {"zvfh", KVM_RISCV_ISA_EXT_ZVFH, true},
> > + {"zvfhmin", KVM_RISCV_ISA_EXT_ZVFHMIN, true},
> > + {"zvkb", KVM_RISCV_ISA_EXT_ZVKB, true},
> > + {"zvkg", KVM_RISCV_ISA_EXT_ZVKG, true},
> > + {"zvkned", KVM_RISCV_ISA_EXT_ZVKNED, true},
> > + {"zvknha", KVM_RISCV_ISA_EXT_ZVKNHA, true},
> > + {"zvknhb", KVM_RISCV_ISA_EXT_ZVKNHB, true},
> > + {"zvksed", KVM_RISCV_ISA_EXT_ZVKSED, true},
> > + {"zvksh", KVM_RISCV_ISA_EXT_ZVKSH, true},
> > + {"zvkt", KVM_RISCV_ISA_EXT_ZVKT, true},
>
> nit: I think I would add a 'misa' boolean member instead of 'multi_letter'
> and then rework this table to look like this
"misa" is not appropriate because misa CSR is only available
in M-mode.
We do have the notion of "single-letter" extensions
in RISC-V unpriv ISA conventions chapter so let me
use that instead.
>
> {"a", KVM_RISCV_ISA_EXT_A, .misa = true },
> {"c", KVM_RISCV_ISA_EXT_C, .misa = true },
> {"d", KVM_RISCV_ISA_EXT_D, .misa = true },
> {"f", KVM_RISCV_ISA_EXT_F, .misa = true },
> {"h", KVM_RISCV_ISA_EXT_H, .misa = true },
> {"i", KVM_RISCV_ISA_EXT_I, .misa = true },
> {"m", KVM_RISCV_ISA_EXT_M, .misa = true },
> {"v", KVM_RISCV_ISA_EXT_V, .misa = true },
>
> /* multi-letter sorted alphabetically */
> {"smnpm", KVM_RISCV_ISA_EXT_SMNPM, },
> {"smstateen", KVM_RISCV_ISA_EXT_SMSTATEEN, },
> ...
>
> The benefit is that only the misa extensions need another field.
See above comment.
>
> > };
> >
> > static void dump_fdt(const char *dtb_file, void *fdt)
> > @@ -129,6 +139,10 @@ static void generate_cpu_nodes(void *fdt, struct kvm *kvm)
> > }
> >
> > for (i = 0; i < arr_sz; i++) {
> > + /* Skip single-letter extensions since these are taken care */
>
> We should finish the comment with 'of by "whatever takes care of them"'
Okay, I will update.
>
> > + if (!isa_info_arr[i].multi_letter)
> > + continue;
> > +
> > reg.id = RISCV_ISA_EXT_REG(isa_info_arr[i].ext_id);
> > reg.addr = (unsigned long)&isa_ext_out;
> > if (ioctl(vcpu->vcpu_fd, KVM_GET_ONE_REG, ®) < 0)
> > --
> > 2.43.0
> >
>
> Thanks,
> drew
Regards,
Anup
