Re: [Minios-devel] [UNIKRAFT PATCHv4 34/43] plat/common: Implement PSCI despatch functions for arm64

[Wei Chen <Wei.Chen@xxxxxxx>]

Hi Julien,

> -----Original Message-----
> From: Julien Grall <julien.grall@xxxxxxxxxx>
> Sent: 2018年7月16日 21:32
> 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 34/43] plat/common: Implement
> PSCI despatch functions for arm64
> 
> Hi,
> 
> On 06/07/18 10:03, Wei Chen wrote:
> > Implement PSCI despatch functions for different conduits.
> > The platforms can select correct despatch function as the
> > PSCI conduit they are using.
> >
> > Signed-off-by: Wei Chen <Wei.Chen@xxxxxxx>
> > ---
> >   plat/common/arm/psci_arm64.S             | 20 ++++++++++++++++++++
> >   plat/common/include/arm/arm64/cpu.h      |  5 +++++
> >   plat/common/include/arm/arm64/cpu_defs.h | 14 ++++++++++++++
> >   3 files changed, 39 insertions(+)
> >   create mode 100644 plat/common/arm/psci_arm64.S
> >
> > diff --git a/plat/common/arm/psci_arm64.S b/plat/common/arm/psci_arm64.S
> > new file mode 100644
> > index 0000000..eb3b7ac
> > --- /dev/null
> > +++ b/plat/common/arm/psci_arm64.S
> > @@ -0,0 +1,20 @@
> > +#include <uk/arch/limits.h>
> > +#include <arm/cpu_defs.h>
> > +
> > +/*
> > + * Use HVC to call PSCI functions:
> > + * uint64_t psci_hvc_call(uint64_t psci_func_id);
> > + */
> > +ENTRY(psci_hvc_call)
> > +   hvc #0
> > +   ret
> > +END(psci_hvc_call)
> > +
> > +/*
> > + * Use SMC to call PSCI functions:
> > + * uint64_t psci_smc_call(uint64_t psci_func_id);
> > + */
> > +ENTRY(psci_smc_call)
> > +   smc #0
> > +   ret
> > +END(psci_smc_call)
> > diff --git a/plat/common/include/arm/arm64/cpu.h
> b/plat/common/include/arm/arm64/cpu.h
> > index cf0f6a2..397c81f 100644
> > --- a/plat/common/include/arm/arm64/cpu.h
> > +++ b/plat/common/include/arm/arm64/cpu.h
> > @@ -72,6 +72,11 @@
> >                                     : : "r" ((uint64_t)(val)))
> >
> >
> > +/* PSCI conduit method to call functions */
> > +extern int psci_method;
> > +uint64_t psci_hvc_call(uint64_t psci_func_id);
> > +uint64_t psci_smc_call(uint64_t psci_func_id);
> 
> The prototype looks a bit strange:
>       - It is possible to have multiple arguments

Actually, yes. On FreeBSD, the psci_xxx_call have multiple arguments.
But I just want to use one argument here.

>       - The function identifiers are 32-bit

I think the psci_func_id would be placed to x0, I want to use a similar
data type like register_t. But we haven't defined it in Unikraft, So I
use the uint64_t here.

>       - The PSCI spec (5.2.2 in ARM DEN 0022D) describes the error code as
> 32-bit signed integers. So the return should be int32_t or int.
> 

Ok, you're right, I will change to int.

> Lastly, this is based on the SMC Calling Convention. So I would rename
> all of this to "smccc_*_call".
> 

Can we use "int __smcc psci_hvc_call"? Just like we use __cdecl, __stdcall
in C programming? I don't like to put calling convention to the function
name.

> > +
> >   /* PSCI conduit types */
> >   #define PSCI_METHOD_NONE  0x0
> >   #define PSCI_METHOD_HVC           0x1
> > diff --git a/plat/common/include/arm/arm64/cpu_defs.h
> b/plat/common/include/arm/arm64/cpu_defs.h
> > index 591d632..315d027 100644
> > --- a/plat/common/include/arm/arm64/cpu_defs.h
> > +++ b/plat/common/include/arm/arm64/cpu_defs.h
> > @@ -105,6 +105,20 @@ END(name)
> >   #define PSR_N             0x80000000
> >   #define PSR_FLAGS 0xf0000000
> >
> > +/*
> > + * Power State Coordination Interface (PSCI v0.2) function codes
> > + */
> > +#define PSCI_FNID_VERSION          0x84000000
> > +#define PSCI_FNID_CPU_SUSPEND              0xc4000001
> > +#define PSCI_FNID_CPU_OFF          0x84000002
> > +#define PSCI_FNID_CPU_ON           0xc4000003
> > +#define PSCI_FNID_AFFINITY_INFO            0xc4000004
> > +#define PSCI_FNID_MIGRATE          0xc4000005
> > +#define PSCI_FNID_MIGRATE_INFO_TYPE        0x84000006
> > +#define PSCI_FNID_MIGRATE_INFO_UP_CPU      0xc4000007
> > +#define PSCI_FNID_SYSTEM_OFF               0x84000008
> > +#define PSCI_FNID_SYSTEM_RESET             0x84000009
> > +
> >   /*
> >    * The supported virtual address bits.
> >    * We will do 1:1 VA to PA Mapping, so we define the same address size
> >
> 
> Cheers,
> 
> --
> Julien Grall
_______________________________________________
Minios-devel mailing list
Minios-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/minios-devel