[PATCH v3 02/15] sbi: sbi_pmu: Improve sbi_pmu_init() error handling

[Yu-Chien Peter Lin]

Hi Atish,

Thanks for the review.

On Wed, Nov 22, 2023 at 03:47:43PM -0800, Atish Patra wrote:
> On Tue, Nov 21, 2023 at 11:39 PM Yu Chien Peter Lin
> <peterlin at andestech.com> wrote:
> >
> > This patch makes the following changes:
> >
> > - As sbi_platform_pmu_init() returns a negative error code on
> >   failure, let sbi_pmu_init() to hang by propagating the error
> >   code.
> >
> 
> Why ? PMU is not a critical resource for booting the system. A system
> can function without it.
> Do we really want to hang the system booting in case sbi_pmu_init fails ?

Agreed, I will simply print the error code with sbi_dprintf() rather
than returning error.

Best regards,
Peter Lin

> > - In order to distinguish the SBI_EFAIL error returned by
> >   sbi_pmu_add_*_counter_map(), return SBI_ENOENT to indicate
> >   that fdt_pmu_setup() failed to locate "riscv,pmu" node, and
> >   generic_pmu_init() ignores such case.
> >
> > Signed-off-by: Yu Chien Peter Lin <peterlin at andestech.com>
> > Reviewed-by: Anup Patel <anup at brainfault.org>
> > ---
> > Changes v1 -> v2:
> >   - New patch
> > Changes v2 -> v3:
> >   - Include Anup's RB tag
> > ---
> >  lib/sbi/sbi_pmu.c           | 5 ++++-
> >  lib/utils/fdt/fdt_pmu.c     | 2 +-
> >  platform/generic/platform.c | 8 +++++++-
> >  3 files changed, 12 insertions(+), 3 deletions(-)
> >
> > diff --git a/lib/sbi/sbi_pmu.c b/lib/sbi/sbi_pmu.c
> > index f4c8fc4..3cbd4ff 100644
> > --- a/lib/sbi/sbi_pmu.c
> > +++ b/lib/sbi/sbi_pmu.c
> > @@ -957,6 +957,7 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
> >         int hpm_count = sbi_fls(sbi_hart_mhpm_mask(scratch));
> >         struct sbi_pmu_hart_state *phs;
> >         const struct sbi_platform *plat;
> > +       int rc;
> >
> >         if (cold_boot) {
> >                 hw_event_map = sbi_calloc(sizeof(*hw_event_map),
> > @@ -972,7 +973,9 @@ int sbi_pmu_init(struct sbi_scratch *scratch, bool cold_boot)
> >
> >                 plat = sbi_platform_ptr(scratch);
> >                 /* Initialize hw pmu events */
> > -               sbi_platform_pmu_init(plat);
> > +               rc = sbi_platform_pmu_init(plat);
> > +               if (rc)
> > +                       return rc;
> >
> >                 /* mcycle & minstret is available always */
> >                 if (!hpm_count)
> > diff --git a/lib/utils/fdt/fdt_pmu.c b/lib/utils/fdt/fdt_pmu.c
> > index 83301bb..a8d7648 100644
> > --- a/lib/utils/fdt/fdt_pmu.c
> > +++ b/lib/utils/fdt/fdt_pmu.c
> > @@ -74,7 +74,7 @@ int fdt_pmu_setup(void *fdt)
> >
> >         pmu_offset = fdt_node_offset_by_compatible(fdt, -1, "riscv,pmu");
> >         if (pmu_offset < 0)
> > -               return SBI_EFAIL;
> > +               return SBI_ENOENT;
> >
> >         event_ctr_map = fdt_getprop(fdt, pmu_offset,
> >                                     "riscv,event-to-mhpmcounters", &len);
> > diff --git a/platform/generic/platform.c b/platform/generic/platform.c
> > index 85acecd..fa400b9 100644
> > --- a/platform/generic/platform.c
> > +++ b/platform/generic/platform.c
> > @@ -265,7 +265,13 @@ static u32 generic_tlb_num_entries(void)
> >
> >  static int generic_pmu_init(void)
> >  {
> > -       return fdt_pmu_setup(fdt_get_address());
> > +       int rc;
> > +
> > +       rc = fdt_pmu_setup(fdt_get_address());
> > +       if (rc && rc != SBI_ENOENT)
> > +               return rc;
> > +
> > +       return 0;
> >  }
> >
> >  static uint64_t generic_pmu_xlate_to_mhpmevent(uint32_t event_idx,
> > --
> > 2.34.1
> >
> 
> 
> -- 
> Regards,
> Atish