[PATCH] lib: sbi: Ensure SBI extension is available

[Andrew Jones]

On Thu, Apr 27, 2023 at 09:02:14AM +0800, Xiang W wrote:
> 在 2023-04-26星期三的 19:27 +0200，Andrew Jones写道：
> > Ensure attempts to invoke SBI extension functions fail with
> > SBI_ERR_NOT_SUPPORTED when the extension's probe function has
> > reported that the extension is not available. By adding a new
> > status member to the extension which has three states
> > (uninitialized, available, unavailable), we ensure that the
> > probe function is only invoked once, lazily, upon first use.
> 
> Adding this state may cause errors. For example: the id numberof an extension is from extid_start to extid_end, which has an
> unused ID. Searching for an ID in this extension will return NULL
> if an unused ID is searched first.

Hi Xiang,

You're right. Either sbi_ecall_extension::status needs to be a function of
extid, like probe is, or we should just always call probe (when it's
available) and leave any optimizations to the individual probe functions.
How about this?

diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c
index 76a1ae9ab733..c86e4e8c9134 100644
--- a/lib/sbi/sbi_ecall.c
+++ b/lib/sbi/sbi_ecall.c
@@ -42,16 +42,18 @@ static SBI_LIST_HEAD(ecall_exts_list);
 
 struct sbi_ecall_extension *sbi_ecall_find_extension(unsigned long extid)
 {
-	struct sbi_ecall_extension *t, *ret = NULL;
+	struct sbi_ecall_extension *t;
+	unsigned long out_val;
 
 	sbi_list_for_each_entry(t, &ecall_exts_list, head) {
 		if (t->extid_start <= extid && extid <= t->extid_end) {
-			ret = t;
-			break;
+			if (t->probe && (t->probe(extid, &out_val) || !out_val))
+				return NULL;
+			return t;
 		}
 	}
 
-	return ret;
+	return NULL;
 }
 
 int sbi_ecall_register_extension(struct sbi_ecall_extension *ext)

Thanks,
drew

> 
> Regards,
> Xiang W
> > 
> > Signed-off-by: Andrew Jones <ajones at ventanamicro.com>
> > ---
> >  include/sbi/sbi_ecall.h |  6 ++++++
> >  lib/sbi/sbi_ecall.c     | 18 ++++++++++++++----
> >  2 files changed, 20 insertions(+), 4 deletions(-)
> > 
> > diff --git a/include/sbi/sbi_ecall.h b/include/sbi/sbi_ecall.h
> > index ff9bf8e2b435..8e31d0d91620 100644
> > --- a/include/sbi/sbi_ecall.h
> > +++ b/include/sbi/sbi_ecall.h
> > @@ -20,10 +20,16 @@
> >  struct sbi_trap_regs;
> >  struct sbi_trap_info;
> >  
> > +enum sbi_ext_status {
> > +       SBI_EXT_AVAILABLE = 1,
> > +       SBI_EXT_UNAVAILABLE,
> > +};
> > +
> >  struct sbi_ecall_extension {
> >         struct sbi_dlist head;
> >         unsigned long extid_start;
> >         unsigned long extid_end;
> > +       enum sbi_ext_status status;
> >         int (* probe)(unsigned long extid, unsigned long *out_val);
> >         int (* handle)(unsigned long extid, unsigned long funcid,
> >                        const struct sbi_trap_regs *regs,
> > diff --git a/lib/sbi/sbi_ecall.c b/lib/sbi/sbi_ecall.c
> > index 76a1ae9ab733..20ff06f32dad 100644
> > --- a/lib/sbi/sbi_ecall.c
> > +++ b/lib/sbi/sbi_ecall.c
> > @@ -42,16 +42,26 @@ static SBI_LIST_HEAD(ecall_exts_list);
> >  
> >  struct sbi_ecall_extension *sbi_ecall_find_extension(unsigned long extid)
> >  {
> > -       struct sbi_ecall_extension *t, *ret = NULL;
> > +       struct sbi_ecall_extension *t;
> > +       unsigned long out_val;
> >  
> >         sbi_list_for_each_entry(t, &ecall_exts_list, head) {
> >                 if (t->extid_start <= extid && extid <= t->extid_end) {
> > -                       ret = t;
> > -                       break;
> > +                       if (t->status == SBI_EXT_AVAILABLE)
> > +                               return t;
> > +                       if (t->status == SBI_EXT_UNAVAILABLE)
> > +                               return NULL;
> > +                       if (t->probe && (t->probe(extid, &out_val) || !out_val)) {
> > +                               t->status = SBI_EXT_UNAVAILABLE;
> > +                               return NULL;
> > +                       }
> > +
> > +                       t->status = SBI_EXT_AVAILABLE;
> > +                       return t;
> >                 }
> >         }
> >  
> > -       return ret;
> > +       return NULL;
> >  }
> >  
> >  int sbi_ecall_register_extension(struct sbi_ecall_extension *ext)
> > -- 
> > 2.39.2
> > 
> > 
> 
>