[PATCH 1/5] lib: sbi: Allow specifying start mode to sbi_hsm_hart_start() API
Anup Patel
Anup.Patel at wdc.com
Wed Sep 16 01:30:46 EDT 2020
> -----Original Message-----
> From: Atish Patra <atishp at atishpatra.org>
> Sent: 15 September 2020 13:04
> To: Anup Patel <Anup.Patel at wdc.com>
> Cc: Atish Patra <Atish.Patra at wdc.com>; Alistair Francis
> <Alistair.Francis at wdc.com>; Anup Patel <anup at brainfault.org>; OpenSBI
> <opensbi at lists.infradead.org>
> Subject: Re: [PATCH 1/5] lib: sbi: Allow specifying start mode to
> sbi_hsm_hart_start() API
>
> On Mon, Sep 14, 2020 at 12:44 AM Anup Patel <Anup.Patel at wdc.com>
> wrote:
> >
> >
> >
> > > -----Original Message-----
> > > From: Atish Patra <atishp at atishpatra.org>
> > > Sent: 11 September 2020 06:29
> > > To: Anup Patel <Anup.Patel at wdc.com>
> > > Cc: Atish Patra <Atish.Patra at wdc.com>; Alistair Francis
> > > <Alistair.Francis at wdc.com>; Anup Patel <anup at brainfault.org>;
> > > OpenSBI <opensbi at lists.infradead.org>
> > > Subject: Re: [PATCH 1/5] lib: sbi: Allow specifying start mode to
> > > sbi_hsm_hart_start() API
> > >
> > > On Wed, Sep 9, 2020 at 7:51 AM Anup Patel <anup.patel at wdc.com>
> wrote:
> > > >
> > > > The sbi_scratch already has provision to specify the next stage
> > > > mode so we can leverage this to specify start mode to
> sbi_hsm_hart_start().
> > > >
> > > > In future, this will be useful in providing SBI calls to U-mode on
> > > > embedded cores where we M-mode and U-mode but no S-mode.
> > > >
> > > > Signed-off-by: Anup Patel <anup.patel at wdc.com>
> > > > ---
> > > > include/sbi/sbi_hsm.h | 2 +-
> > > > lib/sbi/sbi_ecall_hsm.c | 6 +++++-
> > > > lib/sbi/sbi_hsm.c | 6 +++++-
> > > > 3 files changed, 11 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/include/sbi/sbi_hsm.h b/include/sbi/sbi_hsm.h index
> > > > 57d41ff..18d129b 100644
> > > > --- a/include/sbi/sbi_hsm.h
> > > > +++ b/include/sbi/sbi_hsm.h
> > > > @@ -25,7 +25,7 @@ int sbi_hsm_init(struct sbi_scratch *scratch,
> > > > u32 hartid, bool cold_boot); void __noreturn sbi_hsm_exit(struct
> > > > sbi_scratch *scratch);
> > > >
> > > > int sbi_hsm_hart_start(struct sbi_scratch *scratch, u32 hartid,
> > > > - ulong saddr, ulong priv);
> > > > + ulong saddr, ulong smode, ulong priv);
> > > > int sbi_hsm_hart_stop(struct sbi_scratch *scratch, bool exitnow);
> > > > int sbi_hsm_hart_get_state(u32 hartid); int
> > > > sbi_hsm_hart_state_to_status(int state); diff --git
> > > > a/lib/sbi/sbi_ecall_hsm.c b/lib/sbi/sbi_ecall_hsm.c index
> > > > 028bf68..992c93a 100644
> > > > --- a/lib/sbi/sbi_ecall_hsm.c
> > > > +++ b/lib/sbi/sbi_ecall_hsm.c
> > > > @@ -19,12 +19,16 @@ static int sbi_ecall_hsm_handler(unsigned long
> > > extid, unsigned long funcid,
> > > > unsigned long *args, unsigned long *out_val,
> > > > struct sbi_trap_info *out_trap)
> > > > {
> > > > + ulong smode;
> > > > int ret = 0, hstate;
> > > > struct sbi_scratch *scratch = sbi_scratch_thishart_ptr();
> > > >
> > > > switch (funcid) {
> > > > case SBI_EXT_HSM_HART_START:
> > > > - ret = sbi_hsm_hart_start(scratch, args[0], args[1], args[2]);
> > > > + smode = csr_read(CSR_MSTATUS);
> > > > + smode = (smode & MSTATUS_MPP) >>
> MSTATUS_MPP_SHIFT;
> > > > + ret = sbi_hsm_hart_start(scratch, args[0], args[1],
> > > > + smode, args[2]);
> > > > break;
> > > > case SBI_EXT_HSM_HART_STOP:
> > > > ret = sbi_hsm_hart_stop(scratch, TRUE); diff --git
> > > > a/lib/sbi/sbi_hsm.c b/lib/sbi/sbi_hsm.c index 013647a..b430793
> > > > 100644
> > > > --- a/lib/sbi/sbi_hsm.c
> > > > +++ b/lib/sbi/sbi_hsm.c
> > > > @@ -203,7 +203,7 @@ fail_exit:
> > > > }
> > > >
> > > > int sbi_hsm_hart_start(struct sbi_scratch *scratch, u32 hartid,
> > > > - ulong saddr, ulong priv)
> > > > + ulong saddr, ulong smode, ulong priv)
> > > > {
> > > > int rc;
> > > > unsigned long init_count;
> > > > @@ -212,6 +212,9 @@ int sbi_hsm_hart_start(struct sbi_scratch
> > > > *scratch,
> > > u32 hartid,
> > > > struct sbi_hsm_data *hdata;
> > > > const struct sbi_platform *plat =
> > > > sbi_platform_ptr(scratch);
> > > >
> > > > + if (smode != PRV_M && smode != PRV_S && smode != PRV_U)
> > > > + return SBI_EINVAL;
> > > > +
> > > > rscratch = sbi_hartid_to_scratch(hartid);
> > > > if (!rscratch)
> > > > return SBI_EINVAL; @@ -236,6 +239,7 @@ int
> > > > sbi_hsm_hart_start(struct sbi_scratch *scratch,
> > > u32 hartid,
> > > > init_count = sbi_init_count(hartid);
> > > > rscratch->next_arg1 = priv;
> > > > rscratch->next_addr = saddr;
> > > > + rscratch->next_mode = smode;
> > > >
> > > > if (sbi_platform_has_hart_hotplug(plat) ||
> > > > (sbi_platform_has_hart_secondary_boot(plat) &&
> > > > !init_count)) {
> > > > --
> > > > 2.25.1
> > > >
> > > >
> > > > --
> > > > opensbi mailing list
> > > > opensbi at lists.infradead.org
> > > > http://lists.infradead.org/mailman/listinfo/opensbi
> > >
> > > The change as such looks okay to me. But if it is done inside an
> > > internal function instead of changing sbi_hsm_hart_start,we can avoid
> confusion.
> > > We should keep the function names/arguments same as the spec if
> possible.
> >
> > The purpose of adding parameter in sbi_hsm_hart_start(), was to have
> > capability to start boot HART of all non-root domains from init_coldboot().
> > That's why this series is a preparatory series for OpenSBI domain support.
> >
>
> Got it. We are getting the priv mode of the caller hart for ecall use case but
> domains will get that information domain information. That's why the mode
> has to be extracted before sbi_hsm_hart_start not inside it. Thanks for the
> explanation.
>
> > The sbi_ecall_hsm.c is the one which follows SBI spec but sbi_hsm.c is
> > our Internal library for HSM functionality. (Similar example is
> > sbi_ecall_replace.c and sbi_tlb.c)
> >
> > Regards,
> > Anup
>
>
> Reviewed-by: Atish Patra <atish.patra at wdc.com>
Applied this patch to the riscv/opensbi repo
Regards,
Anup
More information about the opensbi
mailing list