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

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

> -----Original Message-----
> From: Simon Kuenzer <simon.kuenzer@xxxxxxxxx>
> Sent: 2018年9月12日 17:18
> 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 11:11, Wei Chen (Arm Technology China) wrote:
> > 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.
> 
> ;-) Then I think this UK_BUGON() is anyway going to get removed from
> these functions. It could have even caused a endless recursion problem
> because UK_BUGON() is calling ukplat_terminate() in an error case.
> 

Haha, Yes, that's real. I would remove it.

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