Re: [Minios-devel] [UNIKRAFT PATCHv4 35/43] plat/common: Implement CPU suspend for arm64

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxx>
> Sent: 2018年7月18日 21:37
> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
> simon.kuenzer@xxxxxxxxx
> Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 35/43] plat/common: Implement
> CPU suspend for arm64
> 
> 
> 
> On 18/07/18 14:28, Julien Grall wrote:
> >
> >
> > On 18/07/18 10:11, Wei Chen wrote:
> >> Hi Julien,
> >
> > Hi Wei,
> >
> >>
> >>> -----Original Message-----
> >>> From: Julien Grall <julien.grall@xxxxxxx>
> >>> Sent: 2018年7月16日 21:48
> >>> To: Wei Chen <Wei.Chen@xxxxxxx>; minios-devel@xxxxxxxxxxxxxxxxxxxx;
> >>> simon.kuenzer@xxxxxxxxx
> >>> Cc: Kaly Xin <Kaly.Xin@xxxxxxx>; nd <nd@xxxxxxx>
> >>> Subject: Re: [Minios-devel] [UNIKRAFT PATCHv4 35/43] plat/common:
> >>> Implement
> >>> CPU suspend for arm64
> >>>
> >>> Hi Wei,
> >>>
> >>> On 06/07/18 10:03, Wei Chen wrote:
> >>>> Using PSCI despatch function to call PSCI_CPU_SUSPEND
> >>>> function to halt CPU.
> >>>>
> >>>> Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> >>>> ---
> >>>>    plat/common/arm/cpu_native.c        | 44
> >>>> +++++++++++++++++++++++++++++
> >>>>    plat/common/include/arm/arm64/cpu.h |  2 ++
> >>>>    2 files changed, 46 insertions(+)
> >>>>    create mode 100644 plat/common/arm/cpu_native.c
> >>>>
> >>>> diff --git a/plat/common/arm/cpu_native.c
> >>>> b/plat/common/arm/cpu_native.c
> >>>> new file mode 100644
> >>>> index 0000000..07548bf
> >>>> --- /dev/null
> >>>> +++ b/plat/common/arm/cpu_native.c
> >>>> @@ -0,0 +1,44 @@
> >>>> +/* SPDX-License-Identifier: BSD-3-Clause */
> >>>> +/*
> >>>> + * Authors: Wei Chen <wei.chen@xxxxxxx>
> >>>> + *
> >>>> + * Copyright (c) 2018, Arm Ltd., All rights reserved.
> >>>> + *
> >>>> + * Redistribution and use in source and binary forms, with or without
> >>>> + * modification, are permitted provided that the following conditions
> >>>> + * are met:
> >>>> + *
> >>>> + * 1. Redistributions of source code must retain the above copyright
> >>>> + *    notice, this list of conditions and the following disclaimer.
> >>>> + * 2. Redistributions in binary form must reproduce the above
> >>>> copyright
> >>>> + *    notice, this list of conditions and the following disclaimer
> >>>> in the
> >>>> + *    documentation and/or other materials provided with the
> >>>> distribution.
> >>>> + * 3. Neither the name of the copyright holder nor the names of its
> >>>> + *    contributors may be used to endorse or promote products
> >>>> derived from
> >>>> + *    this software without specific prior written permission.
> >>>> + *
> >>>> + * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND
> >>>> CONTRIBUTORS "AS
> >>> IS"
> >>>> + * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >>>> LIMITED TO,
> >>> THE
> >>>> + * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR
> >>> PURPOSE
> >>>> + * ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR
> >>>> CONTRIBUTORS
> >>> BE
> >>>> + * LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR
> >>>> + * CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
> >>>> PROCUREMENT OF
> >>>> + * SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR
> >>>> BUSINESS
> >>>> + * INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
> >>>> WHETHER IN
> >>>> + * CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
> >>>> OTHERWISE)
> >>>> + * ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
> >>>> ADVISED OF
> >>> THE
> >>>> + * POSSIBILITY OF SUCH DAMAGE.
> >>>> + *
> >>>> + * THIS HEADER MAY NOT BE EXTRACTED OR MODIFIED IN ANY WAY.
> >>>> + */
> >>>> +
> >>>> +#include <arm/cpu.h>
> >>>> +#include <arm/cpu_defs.h>
> >>>> +
> >>>> +void halt(void)
> >>>
> >>> Looking at the usage of halt, I think PSCI CPU suspend is too expensive
> >>> for brief idling.
> >>>
> >>> I am not entirely sure of the semantic expected, but it looks like "wfi"
> >>> would be more suitable here.
> >>
> >> Does timer interrupt can wake up wfi? If yes, I think I can't use wfi
> >> here.
> >
> > Yes interrupt will wake-up the CPU. This is the same with PSCI suspend.
> >
> > The main difference with the two is the latter will allow you to go in
> > deeper state (depending on the arguments you passed).
> 
> I forgot to mention that PSCI CPU_SUSPEND will have some latency. Along
> with the states, this is usually encoded in the DT. So if you don't
> parse the DT and have some knowledge how long you will likely wait in
> suspend state, then calling CPU_SUSPEND will not be the best solutions.
> 

I have commented in previous reply.

> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel