Re: [Minios-devel] [UNIKRAFT PATCHv5 40/46] plat/common: Implement CPU reset for arm64

[Simon Kuenzer <simon.kuenzer@xxxxxxxxx>]

On 12.09.2018 08:19, Wei Chen (Arm Technology China) wrote:
> > -----Original Message-----
> > From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> > Sent: 2018年9月12日 4:23
> > To: Wei Chen (Arm Technology China) <Wei.Chen@xxxxxxx>; minios-
> > devel@xxxxxxxxxxxxxxxxxxxx
> > Cc: Kaly Xin (Arm Technology China) <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> > Subject: Re: [Minios-devel] [UNIKRAFT PATCHv5 40/46] plat/common: Implement
> > CPU reset for arm64
> >
> >
> >
> > On 11.09.2018 22:20, Simon Kuenzer wrote:
> > > On 10.08.2018 09:08, Wei Chen wrote:
> > > > From: Wei Chen <Wei.Chen@xxxxxxx>
> > > >
> > > > Using PSCI despatch function to call PSCI_CPU_RESET
> > > > function to reset CPU.
> > > >
> > > > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > > > ---
> > > >    plat/common/arm/cpu_native.c        | 7 +++++++
> > > >    plat/common/include/arm/arm64/cpu.h | 1 +
> > > >    2 files changed, 8 insertions(+)
> > > >
> > > > diff --git a/plat/common/arm/cpu_native.c b/plat/common/arm/cpu_native.c
> > > > index 2d1a1aa..429b172 100644
> > > > --- a/plat/common/arm/cpu_native.c
> > > > +++ b/plat/common/arm/cpu_native.c
> > > > @@ -43,3 +43,10 @@ void halt(void)
> > > >    {
> > > >        __asm__ __volatile__("wfi");
> > > >    }
> > > > +
> > > > +/* Systems support PSCI >= 0.2 can do system reset from PSCI */
> > > > +void reset(void)
> > > > +{
> > > > +    UK_BUGON (!smcc_psci_call);
> > > Maybe we should have failed earlier during boot when we failed to detect
> > > the mode. Then you could use UK_ASSERT() instead. What do you think?
> > Alternatively we could enter a busy loop if we fail to get the mode
> > (same for shutdown in the next patch).
> I am considering let return directly, and give its caller the chance
> to do some remedy if reset failed.
Hum... we defined ukplat_terminate() with '__noreturn' attribute. The 
question would also be, what else would you like to do if you cannot 
reset the system. This function is also called from trap handlers.
Because of this attribute, returning wouldn't be possible without 
changing the ukplat API. Personally, I would prefer to print that reset 
failed (same for shutdown) and enter an endless halt loop. I think this 
should also be the fallback behavior for the case were we failed to 
detect the PSCI mode with the DTB.
So, I agree it is fine (or even better) if you prefer continue booting 
whenever you could not detect the PSCI mode but I would prefer that the 
ukplat_terminate keeps the __noreturn attribute.
What do you think?

> > > > +    smcc_psci_call(PSCI_FNID_SYSTEM_RESET, 0, 0, 0);
> > > > +}
> > > > diff --git a/plat/common/include/arm/arm64/cpu.h
> > > > b/plat/common/include/arm/arm64/cpu.h
> > > > index b231d32..c156fd3 100644
> > > > --- a/plat/common/include/arm/arm64/cpu.h
> > > > +++ b/plat/common/include/arm/arm64/cpu.h
> > > > @@ -102,3 +102,4 @@ int32_t smcc_psci_smc_call(uint32_t, uint64_t,
> > > > uint64_t, uint64_t);
> > > >    /* CPU native APIs */
> > > >    void halt(void);
> > > > +void reset(void);
> > > _______________________________________________
> > > Minios-devel mailing list
> > > Minios-devel@xxxxxxxxxxxxxxxxxxxx
> > > https://lists.xenproject.org/mailman/listinfo/minios-devel
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel