Re: [PATCH v4] lib: utils: fdt_helper: simplify fdt_parse_isa_extensions() helper

[Xiang W <wxjstz@126.com>]

在 2025-09-12五的 16:56 +0800，Peter Lin写道：
> Hi Xiang,
> 
> Still have an issue, when a secondary hart calls hart_detect_features()
> the FDT ISA bits set by the boot hart are cleared here [1]. I think it is still
> necessary to cache the ISA bitmap somewhere before it gets OR'ed
> into hfeatures->extensions.
> 
> [1] https://github.com/riscv-software-src/opensbi/blob/153cdeea5350837c1206a2d2b6189fd0ba06d1f2/lib/sbi/sbi_hart.c#L836-L837

Indeed, let's drop this patch.

Regards,
Xiang W
> 
> On Fri, Sep 12, 2025 at 3:06 PM Xiang W <wxjstz@126.com> wrote:
> > 
> > The fdt_isa_bitmap was previously used as a temporary array
> > to store ISA extensions when parsing device-tree.
> > To reduce scratch space consumption, set the ISA bitmap
> > directly in hfeature->extensions, allows us to remove the
> > fdt_parse_isa_all_harts() as fdt_parse_isa_extensions() can
> > now handle the task directly.
> > 
> > Signed-off-by: Xiang W <wxjstz@126.com>
> > Co-authored-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
> > ---
> >  include/sbi_utils/fdt/fdt_helper.h |  3 +-
> >  lib/utils/fdt/fdt_helper.c         | 63 ++++++++----------------------
> >  platform/generic/platform.c        |  3 +-
> >  3 files changed, 18 insertions(+), 51 deletions(-)
> > 
> > diff --git a/include/sbi_utils/fdt/fdt_helper.h b/include/sbi_utils/fdt/fdt_helper.h
> > index 04c850cc..2ece7f5e 100644
> > --- a/include/sbi_utils/fdt/fdt_helper.h
> > +++ b/include/sbi_utils/fdt/fdt_helper.h
> > @@ -54,8 +54,7 @@ int fdt_parse_cbom_block_size(const void *fdt, int cpu_offset, unsigned long  *c
> > 
> >  int fdt_parse_timebase_frequency(const void *fdt, unsigned long *freq);
> > 
> > -int fdt_parse_isa_extensions(const void *fdt, unsigned int hartid,
> > -                            unsigned long *extensions);
> > +int fdt_parse_isa_extensions(const void *fdt);
> > 
> >  int fdt_parse_gaisler_uart_node(const void *fdt, int nodeoffset,
> >                                 struct platform_uart_data *uart);
> > diff --git a/lib/utils/fdt/fdt_helper.c b/lib/utils/fdt/fdt_helper.c
> > index 0f4859c1..64f928f7 100644
> > --- a/lib/utils/fdt/fdt_helper.c
> > +++ b/lib/utils/fdt/fdt_helper.c
> > @@ -324,9 +324,7 @@ int fdt_parse_timebase_frequency(const void *fdt, unsigned long *freq)
> > 
> >  #define RISCV_ISA_EXT_NAME_LEN_MAX     32
> > 
> > -static unsigned long fdt_isa_bitmap_offset;
> > -
> > -static int fdt_parse_isa_one_hart(const char *isa, unsigned long *extensions)
> > +static int fdt_parse_isa_one_hart(struct sbi_scratch *scratch, const char *isa)
> >  {
> >         size_t i, j, isa_len;
> >         char mstr[RISCV_ISA_EXT_NAME_LEN_MAX];
> > @@ -379,7 +377,8 @@ static int fdt_parse_isa_one_hart(const char *isa, unsigned long *extensions)
> > 
> >  #define set_multi_letter_ext(name, bit)                                \
> >                         if (!strcmp(mstr, name)) {              \
> > -                               __set_bit(bit, extensions);     \
> > +                               sbi_hart_update_extension(      \
> > +                                       scratch, bit, true);    \
> >                                 continue;                       \
> >                         }
> > 
> > @@ -393,29 +392,32 @@ static int fdt_parse_isa_one_hart(const char *isa, unsigned long *extensions)
> >         return 0;
> >  }
> > 
> > -static void fdt_parse_isa_extensions_one_hart(const char *isa,
> > -                                             unsigned long *extensions,
> > -                                             int len)
> > +static void fdt_parse_isa_extensions_one_hart(struct sbi_scratch *scratch,
> > +                                             const char *isa, int len)
> >  {
> >         size_t i;
> > 
> >         for (i = 0; i < SBI_HART_EXT_MAX; i++) {
> >                 if (fdt_stringlist_contains(isa, len, sbi_hart_ext[i].name))
> > -                       __set_bit(sbi_hart_ext[i].id, extensions);
> > +                       sbi_hart_update_extension(scratch, sbi_hart_ext[i].id, true);
> >         }
> >  }
> > 
> > -static int fdt_parse_isa_all_harts(const void *fdt)
> > +int fdt_parse_isa_extensions(const void *fdt)
> >  {
> > +       static bool done;
> >         u32 hartid;
> >         const fdt32_t *val;
> > -       unsigned long *hart_exts;
> >         struct sbi_scratch *scratch;
> >         int err, cpu_offset, cpus_offset, len;
> > 
> > -       if (!fdt || !fdt_isa_bitmap_offset)
> > +       if (!fdt)
> >                 return SBI_EINVAL;
> > 
> > +       if (done)
> > +               return 0;
> > +       done = true;
> > +
> >         cpus_offset = fdt_path_offset(fdt, "/cpus");
> >         if (cpus_offset < 0)
> >                 return cpus_offset;
> > @@ -432,13 +434,10 @@ static int fdt_parse_isa_all_harts(const void *fdt)
> >                 if (!scratch)
> >                         return SBI_ENOENT;
> > 
> > -               hart_exts = sbi_scratch_offset_ptr(scratch,
> > -                                                  fdt_isa_bitmap_offset);
> > -
> >                 val = fdt_getprop(fdt, cpu_offset, "riscv,isa-extensions", &len);
> >                 if (val && len > 0) {
> > -                       fdt_parse_isa_extensions_one_hart((const char *)val,
> > -                                                         hart_exts, len);
> > +                       fdt_parse_isa_extensions_one_hart(scratch,
> > +                                                       (const char *)val, len);
> >                         continue;
> >                 }
> > 
> > @@ -446,7 +445,7 @@ static int fdt_parse_isa_all_harts(const void *fdt)
> >                 if (!val || len <= 0)
> >                         return SBI_ENOENT;
> > 
> > -               err = fdt_parse_isa_one_hart((const char *)val, hart_exts);
> > +               err = fdt_parse_isa_one_hart(scratch, (const char *)val);
> >                 if (err)
> >                         return err;
> >         }
> > @@ -454,36 +453,6 @@ static int fdt_parse_isa_all_harts(const void *fdt)
> >         return 0;
> >  }
> > 
> > -int fdt_parse_isa_extensions(const void *fdt, unsigned int hartid,
> > -                       unsigned long *extensions)
> > -{
> > -       int rc, i;
> > -       unsigned long *hart_exts;
> > -       struct sbi_scratch *scratch;
> > -
> > -       if (!fdt_isa_bitmap_offset) {
> > -               fdt_isa_bitmap_offset = sbi_scratch_alloc_offset(
> > -                                       sizeof(*hart_exts) *
> > -                                       BITS_TO_LONGS(SBI_HART_EXT_MAX));
> > -               if (!fdt_isa_bitmap_offset)
> > -                       return SBI_ENOMEM;
> > -
> > -               rc = fdt_parse_isa_all_harts(fdt);
> > -               if (rc)
> > -                       return rc;
> > -       }
> > -
> > -       scratch = sbi_hartid_to_scratch(hartid);
> > -       if (!scratch)
> > -               return SBI_ENOENT;
> > -
> > -       hart_exts = sbi_scratch_offset_ptr(scratch, fdt_isa_bitmap_offset);
> > -
> > -       for (i = 0; i < BITS_TO_LONGS(SBI_HART_EXT_MAX); i++)
> > -               extensions[i] |= hart_exts[i];
> > -       return 0;
> > -}
> > -
> >  static int fdt_parse_uart_node_common(const void *fdt, int nodeoffset,
> >                                       struct platform_uart_data *uart,
> >                                       unsigned long default_freq,
> > diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> > index 47e771ad..7e3417b0 100644
> > --- a/platform/generic/platform.c
> > +++ b/platform/generic/platform.c
> > @@ -251,8 +251,7 @@ int generic_final_init(bool cold_boot)
> >  int generic_extensions_init(struct sbi_hart_features *hfeatures)
> >  {
> >         /* Parse the ISA string from FDT and enable the listed extensions */
> > -       return fdt_parse_isa_extensions(fdt_get_address(), current_hartid(),
> > -                                       hfeatures->extensions);
> > +       return fdt_parse_isa_extensions(fdt_get_address());
> >  }
> > 
> >  int generic_domains_init(void)
> > --
> > 2.47.3
> > 
-- 
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi