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

[Julien Grall <julien.grall@xxxxxxx>]

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.
Cheers,

--
Julien Grall

_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel