[PATCH] lib: utils: fdt_helper: simplify fdt_parse_isa_extensions() helper
Xiang W
wxjstz at 126.com
Thu Sep 4 02:54:33 PDT 2025
在 2025-09-04四的 17:23 +0800，Peter Lin写道：
> Hi Xiang,
>
> On Wed, Aug 20, 2025 at 12:03 PM Xiang W <wxjstz at 126.com> wrote:
> >
> > 在 2025-08-15五的 10:44 +0800，Yu-Chien Peter Lin写道：
> > > 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.
> > >
> > > This change makes the helper function more versatile, even
> > > before scratch space is initialized.
> > >
> > > Signed-off-by: Yu-Chien Peter Lin <peter.lin at sifive.com>
> > NACK
> >
> > Originally, the extensions parsed by fdt_parse_isa_extensions
> > were related to hartid. After the modification, it will collect
> > the extensions supported by all the HART.
> >
> > To remove fdt_isa_bitmap_offset, call sbi_hart_update_extension
> > in fdt_parse_isa_extensions_one_hart/fdt_parse_isa_one_hart
>
> Thanks for catching this, but the function
> already has direct access to the hart_exts
> (which corresponds to hfeature->extenions
> of current HART), so we may not need to use
> sbi_hart_update_extension() which locates
> hfeature->extensions from scratch?
>
> So I plan to skip other harts when parsing:
>
> --- a/lib/utils/fdt/fdt_helper.c
> +++ b/lib/utils/fdt/fdt_helper.c
> @@ -421,6 +421,9 @@ int fdt_parse_isa_extensions(const void *fdt,
> unsigned long *hart_exts)
> if (err)
> continue;
>
> + if (hartid != current_hartid())
> + continue;
> +
> if (!fdt_node_is_enabled(fdt, cpu_offset))
> continue;
>
LGTM
Regards,
Xiang W
> It should fix the issue of collecting ISA from all
> HARTs.
>
> Thanks,
> Peter Lin
>
> > Regards,
> > Xiang W
> >
> > > ---
> > > include/sbi_utils/fdt/fdt_helper.h | 3 +-
> > > lib/utils/fdt/fdt_helper.c | 45 ++----------------------------
> > > platform/generic/platform.c | 2 +-
> > > 3 files changed, 4 insertions(+), 46 deletions(-)
> > >
> > > diff --git a/include/sbi_utils/fdt/fdt_helper.h b/include/sbi_utils/fdt/fdt_helper.h
> > > index 04c850cc..24ad1a94 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, unsigned long *hart_exts);
> > >
> > > 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..61a5962c 100644
> > > --- a/lib/utils/fdt/fdt_helper.c
> > > +++ b/lib/utils/fdt/fdt_helper.c
> > > @@ -324,8 +324,6 @@ 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)
> > > {
> > > size_t i, j, isa_len;
> > > @@ -405,15 +403,13 @@ static void fdt_parse_isa_extensions_one_hart(const char *isa,
> > > }
> > > }
> > >
> > > -static int fdt_parse_isa_all_harts(const void *fdt)
> > > +int fdt_parse_isa_extensions(const void *fdt, unsigned long *hart_exts)
> > > {
> > > 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 || !hart_exts)
> > > return SBI_EINVAL;
> > >
> > > cpus_offset = fdt_path_offset(fdt, "/cpus");
> > > @@ -428,13 +424,6 @@ static int fdt_parse_isa_all_harts(const void *fdt)
> > > if (!fdt_node_is_enabled(fdt, cpu_offset))
> > > continue;
> > >
> > > - scratch = sbi_hartid_to_scratch(hartid);
> > > - 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,
> > > @@ -454,36 +443,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..2e8c6051 100644
> > > --- a/platform/generic/platform.c
> > > +++ b/platform/generic/platform.c
> > > @@ -251,7 +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(),
> > > + return fdt_parse_isa_extensions(fdt_get_address(),
> > > hfeatures->extensions);
> > > }
> > >
> > > --
> > > 2.48.0
> > >
> >
More information about the opensbi
mailing list