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

["Wei Chen (Arm Technology China)" <Wei.Chen@xxxxxxx>]

Hi Simon,

> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: 2018年9月12日 16:59
> 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 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?
> 

I agree with you.

> >
> >>>
> >>>> +    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