* [PATCH v2 0/3] xen/arm: SMCCC fixes and PSCI clean-up
@ 2018-02-02 11:41 Julien Grall
2018-02-02 11:41 ` [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU Julien Grall
` (2 more replies)
0 siblings, 3 replies; 16+ messages in thread
From: Julien Grall @ 2018-02-02 11:41 UTC (permalink / raw)
To: xen-devel; +Cc: sstabellini, volodymyr_babchuk, andre.przywara, Julien Grall
Hi all,
This small patch series contains SMCCC fixes (see #2) and PSCI clean-up.
Cheers,
Julien Grall (3):
xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU
xen/arm: vsmc: Don't implement function ID that doesn't exist
xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c
xen/arch/arm/vpsci.c | 155 +++++++++++++++++++++++++++++++++------
xen/arch/arm/vsmc.c | 126 ++++---------------------------
xen/include/asm-arm/perfc_defn.h | 2 -
xen/include/asm-arm/psci.h | 23 ------
xen/include/asm-arm/smccc.h | 20 ++++-
xen/include/asm-arm/vpsci.h | 13 ++++
6 files changed, 177 insertions(+), 162 deletions(-)
create mode 100644 xen/include/asm-arm/vpsci.h
--
2.11.0
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel
^ permalink raw reply [flat|nested] 16+ messages in thread* [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU 2018-02-02 11:41 [PATCH v2 0/3] xen/arm: SMCCC fixes and PSCI clean-up Julien Grall @ 2018-02-02 11:41 ` Julien Grall 2018-02-02 13:44 ` Volodymyr Babchuk 2018-02-02 11:41 ` [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist Julien Grall 2018-02-02 11:41 ` [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c Julien Grall 2 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-02 11:41 UTC (permalink / raw) To: xen-devel; +Cc: sstabellini, volodymyr_babchuk, andre.przywara, Julien Grall The PSCI call MIGRATE and MIGRATE_INFO_UP_CPU are optional and implemented as just returning PSCI_NOT_SUPPORTED (aka UNKNOWN_FUNCTION for SMCCC). The new SMCCC framework is able to deal with unimplemented function and return the proper error code. So remove the implementations for both function. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v2: - Remove define in psci.h - Update SSSC_SMCCC_FUNCTION_COUNT --- xen/arch/arm/vpsci.c | 10 ---------- xen/arch/arm/vsmc.c | 16 +--------------- xen/include/asm-arm/perfc_defn.h | 2 -- xen/include/asm-arm/psci.h | 4 ---- 4 files changed, 1 insertion(+), 31 deletions(-) diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c index cd724904ef..979d32ed6d 100644 --- a/xen/arch/arm/vpsci.c +++ b/xen/arch/arm/vpsci.c @@ -172,21 +172,11 @@ int32_t do_psci_0_2_affinity_info(register_t target_affinity, return PSCI_0_2_AFFINITY_LEVEL_OFF; } -int32_t do_psci_0_2_migrate(uint32_t target_cpu) -{ - return PSCI_NOT_SUPPORTED; -} - uint32_t do_psci_0_2_migrate_info_type(void) { return PSCI_0_2_TOS_MP_OR_NOT_PRESENT; } -register_t do_psci_0_2_migrate_info_up_cpu(void) -{ - return PSCI_NOT_SUPPORTED; -} - void do_psci_0_2_system_off( void ) { struct domain *d = current->domain; diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c index c9064de37a..997f2e0ebc 100644 --- a/xen/arch/arm/vsmc.c +++ b/xen/arch/arm/vsmc.c @@ -28,7 +28,7 @@ #define XEN_SMCCC_FUNCTION_COUNT 3 /* Number of functions currently supported by Standard Service Service Calls. */ -#define SSSC_SMCCC_FUNCTION_COUNT 13 +#define SSSC_SMCCC_FUNCTION_COUNT 11 static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid) { @@ -157,11 +157,6 @@ static bool handle_sssc(struct cpu_user_regs *regs) PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); return true; - case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU: - perfc_incr(vpsci_migrate_info_up_cpu); - PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu()); - return true; - case PSCI_0_2_FN_SYSTEM_OFF: perfc_incr(vpsci_system_off); do_psci_0_2_system_off(); @@ -206,15 +201,6 @@ static bool handle_sssc(struct cpu_user_regs *regs) return true; } - case PSCI_0_2_FN_MIGRATE: - { - uint32_t tcpu = PSCI_ARG32(regs, 1); - - perfc_incr(vpsci_cpu_migrate); - PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu)); - return true; - } - case ARM_SMCCC_FUNC_CALL_COUNT: return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT); diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h index 5f957ee6ec..a7acb7d21c 100644 --- a/xen/include/asm-arm/perfc_defn.h +++ b/xen/include/asm-arm/perfc_defn.h @@ -27,12 +27,10 @@ PERFCOUNTER(vpsci_cpu_on, "vpsci: cpu_on") PERFCOUNTER(vpsci_cpu_off, "vpsci: cpu_off") PERFCOUNTER(vpsci_version, "vpsci: version") PERFCOUNTER(vpsci_migrate_info_type, "vpsci: migrate_info_type") -PERFCOUNTER(vpsci_migrate_info_up_cpu, "vpsci: migrate_info_up_cpu") PERFCOUNTER(vpsci_system_off, "vpsci: system_off") PERFCOUNTER(vpsci_system_reset, "vpsci: system_reset") PERFCOUNTER(vpsci_cpu_suspend, "vpsci: cpu_suspend") PERFCOUNTER(vpsci_cpu_affinity_info, "vpsci: cpu_affinity_info") -PERFCOUNTER(vpsci_cpu_migrate, "vpsci: cpu_migrate") PERFCOUNTER(vgicd_reads, "vgicd: read") PERFCOUNTER(vgicd_writes, "vgicd: write") diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h index 635ea5dae4..32c1f81f21 100644 --- a/xen/include/asm-arm/psci.h +++ b/xen/include/asm-arm/psci.h @@ -37,9 +37,7 @@ int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, register_t context_id); int32_t do_psci_0_2_affinity_info(register_t target_affinity, uint32_t lowest_affinity_level); -int32_t do_psci_0_2_migrate(uint32_t target_cpu); uint32_t do_psci_0_2_migrate_info_type(void); -register_t do_psci_0_2_migrate_info_up_cpu(void); void do_psci_0_2_system_off(void); void do_psci_0_2_system_reset(void); @@ -57,9 +55,7 @@ void do_psci_0_2_system_reset(void); #define PSCI_0_2_FN_CPU_OFF 2 #define PSCI_0_2_FN_CPU_ON 3 #define PSCI_0_2_FN_AFFINITY_INFO 4 -#define PSCI_0_2_FN_MIGRATE 5 #define PSCI_0_2_FN_MIGRATE_INFO_TYPE 6 -#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU 7 #define PSCI_0_2_FN_SYSTEM_OFF 8 #define PSCI_0_2_FN_SYSTEM_RESET 9 -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU 2018-02-02 11:41 ` [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU Julien Grall @ 2018-02-02 13:44 ` Volodymyr Babchuk 0 siblings, 0 replies; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-02 13:44 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara Hi Julien, On 02.02.18 13:41, Julien Grall wrote: > The PSCI call MIGRATE and MIGRATE_INFO_UP_CPU are optional and > implemented as just returning PSCI_NOT_SUPPORTED (aka UNKNOWN_FUNCTION > for SMCCC). > > The new SMCCC framework is able to deal with unimplemented function and > return the proper error code. So remove the implementations for both > function. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` > --- > Changes in v2: > - Remove define in psci.h > - Update SSSC_SMCCC_FUNCTION_COUNT > --- > xen/arch/arm/vpsci.c | 10 ---------- > xen/arch/arm/vsmc.c | 16 +--------------- > xen/include/asm-arm/perfc_defn.h | 2 -- > xen/include/asm-arm/psci.h | 4 ---- > 4 files changed, 1 insertion(+), 31 deletions(-) > > diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c > index cd724904ef..979d32ed6d 100644 > --- a/xen/arch/arm/vpsci.c > +++ b/xen/arch/arm/vpsci.c > @@ -172,21 +172,11 @@ int32_t do_psci_0_2_affinity_info(register_t target_affinity, > return PSCI_0_2_AFFINITY_LEVEL_OFF; > } > > -int32_t do_psci_0_2_migrate(uint32_t target_cpu) > -{ > - return PSCI_NOT_SUPPORTED; > -} > - > uint32_t do_psci_0_2_migrate_info_type(void) > { > return PSCI_0_2_TOS_MP_OR_NOT_PRESENT; > } > > -register_t do_psci_0_2_migrate_info_up_cpu(void) > -{ > - return PSCI_NOT_SUPPORTED; > -} > - > void do_psci_0_2_system_off( void ) > { > struct domain *d = current->domain; > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c > index c9064de37a..997f2e0ebc 100644 > --- a/xen/arch/arm/vsmc.c > +++ b/xen/arch/arm/vsmc.c > @@ -28,7 +28,7 @@ > #define XEN_SMCCC_FUNCTION_COUNT 3 > > /* Number of functions currently supported by Standard Service Service Calls. */ > -#define SSSC_SMCCC_FUNCTION_COUNT 13 > +#define SSSC_SMCCC_FUNCTION_COUNT 11 > > static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid) > { > @@ -157,11 +157,6 @@ static bool handle_sssc(struct cpu_user_regs *regs) > PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); > return true; > > - case PSCI_0_2_FN_MIGRATE_INFO_UP_CPU: > - perfc_incr(vpsci_migrate_info_up_cpu); > - PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_up_cpu()); > - return true; > - > case PSCI_0_2_FN_SYSTEM_OFF: > perfc_incr(vpsci_system_off); > do_psci_0_2_system_off(); > @@ -206,15 +201,6 @@ static bool handle_sssc(struct cpu_user_regs *regs) > return true; > } > > - case PSCI_0_2_FN_MIGRATE: > - { > - uint32_t tcpu = PSCI_ARG32(regs, 1); > - > - perfc_incr(vpsci_cpu_migrate); > - PSCI_SET_RESULT(regs, do_psci_0_2_migrate(tcpu)); > - return true; > - } > - > case ARM_SMCCC_FUNC_CALL_COUNT: > return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT); > > diff --git a/xen/include/asm-arm/perfc_defn.h b/xen/include/asm-arm/perfc_defn.h > index 5f957ee6ec..a7acb7d21c 100644 > --- a/xen/include/asm-arm/perfc_defn.h > +++ b/xen/include/asm-arm/perfc_defn.h > @@ -27,12 +27,10 @@ PERFCOUNTER(vpsci_cpu_on, "vpsci: cpu_on") > PERFCOUNTER(vpsci_cpu_off, "vpsci: cpu_off") > PERFCOUNTER(vpsci_version, "vpsci: version") > PERFCOUNTER(vpsci_migrate_info_type, "vpsci: migrate_info_type") > -PERFCOUNTER(vpsci_migrate_info_up_cpu, "vpsci: migrate_info_up_cpu") > PERFCOUNTER(vpsci_system_off, "vpsci: system_off") > PERFCOUNTER(vpsci_system_reset, "vpsci: system_reset") > PERFCOUNTER(vpsci_cpu_suspend, "vpsci: cpu_suspend") > PERFCOUNTER(vpsci_cpu_affinity_info, "vpsci: cpu_affinity_info") > -PERFCOUNTER(vpsci_cpu_migrate, "vpsci: cpu_migrate") > > PERFCOUNTER(vgicd_reads, "vgicd: read") > PERFCOUNTER(vgicd_writes, "vgicd: write") > diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h > index 635ea5dae4..32c1f81f21 100644 > --- a/xen/include/asm-arm/psci.h > +++ b/xen/include/asm-arm/psci.h > @@ -37,9 +37,7 @@ int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, > register_t context_id); > int32_t do_psci_0_2_affinity_info(register_t target_affinity, > uint32_t lowest_affinity_level); > -int32_t do_psci_0_2_migrate(uint32_t target_cpu); > uint32_t do_psci_0_2_migrate_info_type(void); > -register_t do_psci_0_2_migrate_info_up_cpu(void); > void do_psci_0_2_system_off(void); > void do_psci_0_2_system_reset(void); > > @@ -57,9 +55,7 @@ void do_psci_0_2_system_reset(void); > #define PSCI_0_2_FN_CPU_OFF 2 > #define PSCI_0_2_FN_CPU_ON 3 > #define PSCI_0_2_FN_AFFINITY_INFO 4 > -#define PSCI_0_2_FN_MIGRATE 5 > #define PSCI_0_2_FN_MIGRATE_INFO_TYPE 6 > -#define PSCI_0_2_FN_MIGRATE_INFO_UP_CPU 7 > #define PSCI_0_2_FN_SYSTEM_OFF 8 > #define PSCI_0_2_FN_SYSTEM_RESET 9 > > -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist 2018-02-02 11:41 [PATCH v2 0/3] xen/arm: SMCCC fixes and PSCI clean-up Julien Grall 2018-02-02 11:41 ` [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU Julien Grall @ 2018-02-02 11:41 ` Julien Grall 2018-02-02 13:46 ` Volodymyr Babchuk 2018-02-02 11:41 ` [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c Julien Grall 2 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-02 11:41 UTC (permalink / raw) To: xen-devel; +Cc: sstabellini, volodymyr_babchuk, andre.przywara, Julien Grall The current implementation of SMCCC relies on the fact only function number (bits [15:0]) is enough to identify what to implement. However, PSCI call are only available in the range 0x84000000-0x8400001F and 0xC4000000-0xC400001F. Furthermore, not all SMC32 functions have equivalent in the SMC64. This is the case of: * PSCI_VERSION * CPU_OFF * MIGRATE_INFO_TYPE * SYSTEM_OFF * SYSTEM_RESET Similarly call count, call uid, revision can only be query using smc32/hvc32 fast calls (See 6.2 in ARM DEN 0028B). Xen should only implement identifier existing in the specification in order to avoid potential clash with later revision. Therefore rework the vsmc code to use the whole function identifier rather than only the function number. At the same time, the new macros for call count, call uid, revision are renamed to better suit the spec. Lastly, update SSSC_SMCCC_FUNCTION_COUNT to match the correct number of funtions. Note that version is not updated because the number has always been wrong, and nobody could properly use it. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- This should be backported to Xen 4.10 as we should not implement functions identifier that does not exist toprevent clash with a later revision. Changes in v2: - Rename the call count, call uid, revision macros - Update SSSC_SMCCC_FUNCTION_COUNT --- xen/arch/arm/vsmc.c | 39 ++++++++++++++++++++++----------------- xen/include/asm-arm/smccc.h | 20 +++++++++++++++++--- 2 files changed, 39 insertions(+), 20 deletions(-) diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c index 997f2e0ebc..3d8cbcc808 100644 --- a/xen/arch/arm/vsmc.c +++ b/xen/arch/arm/vsmc.c @@ -28,7 +28,7 @@ #define XEN_SMCCC_FUNCTION_COUNT 3 /* Number of functions currently supported by Standard Service Service Calls. */ -#define SSSC_SMCCC_FUNCTION_COUNT 11 +#define SSSC_SMCCC_FUNCTION_COUNT 14 static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid) { @@ -84,13 +84,15 @@ static bool fill_function_call_count(struct cpu_user_regs *regs, uint32_t cnt) /* SMCCC interface for hypervisor. Tell about itself. */ static bool handle_hypervisor(struct cpu_user_regs *regs) { - switch ( smccc_get_fn(get_user_reg(regs, 0)) ) + uint32_t fid = (uint32_t)get_user_reg(regs, 0); + + switch ( fid ) { - case ARM_SMCCC_FUNC_CALL_COUNT: + case ARM_SMCCC_CALL_COUNT_FID(HYPERVISOR): return fill_function_call_count(regs, XEN_SMCCC_FUNCTION_COUNT); - case ARM_SMCCC_FUNC_CALL_UID: + case ARM_SMCCC_CALL_UID_FID(HYPERVISOR): return fill_uid(regs, XEN_SMCCC_UID); - case ARM_SMCCC_FUNC_CALL_REVISION: + case ARM_SMCCC_REVISION_FID(HYPERVISOR): return fill_revision(regs, XEN_SMCCC_MAJOR_REVISION, XEN_SMCCC_MINOR_REVISION); default: @@ -140,36 +142,37 @@ static bool handle_sssc(struct cpu_user_regs *regs) { uint32_t fid = (uint32_t)get_user_reg(regs, 0); - switch ( smccc_get_fn(fid) ) + switch ( fid ) { - case PSCI_0_2_FN_PSCI_VERSION: + case PSCI_0_2_FN32(PSCI_VERSION): perfc_incr(vpsci_version); PSCI_SET_RESULT(regs, do_psci_0_2_version()); return true; - case PSCI_0_2_FN_CPU_OFF: + case PSCI_0_2_FN32(CPU_OFF): perfc_incr(vpsci_cpu_off); PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off()); return true; - case PSCI_0_2_FN_MIGRATE_INFO_TYPE: + case PSCI_0_2_FN32(MIGRATE_INFO_TYPE): perfc_incr(vpsci_migrate_info_type); PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); return true; - case PSCI_0_2_FN_SYSTEM_OFF: + case PSCI_0_2_FN32(SYSTEM_OFF): perfc_incr(vpsci_system_off); do_psci_0_2_system_off(); PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); return true; - case PSCI_0_2_FN_SYSTEM_RESET: + case PSCI_0_2_FN32(SYSTEM_RESET): perfc_incr(vpsci_system_reset); do_psci_0_2_system_reset(); PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); return true; - case PSCI_0_2_FN_CPU_ON: + case PSCI_0_2_FN32(CPU_ON): + case PSCI_0_2_FN64(CPU_ON): { register_t vcpuid = PSCI_ARG(regs, 1); register_t epoint = PSCI_ARG(regs, 2); @@ -180,7 +183,8 @@ static bool handle_sssc(struct cpu_user_regs *regs) return true; } - case PSCI_0_2_FN_CPU_SUSPEND: + case PSCI_0_2_FN32(CPU_SUSPEND): + case PSCI_0_2_FN64(CPU_SUSPEND): { uint32_t pstate = PSCI_ARG32(regs, 1); register_t epoint = PSCI_ARG(regs, 2); @@ -191,7 +195,8 @@ static bool handle_sssc(struct cpu_user_regs *regs) return true; } - case PSCI_0_2_FN_AFFINITY_INFO: + case PSCI_0_2_FN32(AFFINITY_INFO): + case PSCI_0_2_FN64(AFFINITY_INFO): { register_t taff = PSCI_ARG(regs, 1); uint32_t laff = PSCI_ARG32(regs, 2); @@ -201,13 +206,13 @@ static bool handle_sssc(struct cpu_user_regs *regs) return true; } - case ARM_SMCCC_FUNC_CALL_COUNT: + case ARM_SMCCC_CALL_COUNT_FID(STANDARD): return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT); - case ARM_SMCCC_FUNC_CALL_UID: + case ARM_SMCCC_CALL_UID_FID(STANDARD): return fill_uid(regs, SSSC_SMCCC_UID); - case ARM_SMCCC_FUNC_CALL_REVISION: + case ARM_SMCCC_REVISION_FID(STANDARD): return fill_revision(regs, SSSC_SMCCC_MAJOR_REVISION, SSSC_SMCCC_MINOR_REVISION); diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h index f543dea0bb..62b3a8cdf5 100644 --- a/xen/include/asm-arm/smccc.h +++ b/xen/include/asm-arm/smccc.h @@ -82,9 +82,23 @@ static inline uint32_t smccc_get_owner(register_t funcid) #define ARM_SMCCC_OWNER_TRUSTED_OS_END 63 /* List of generic function numbers */ -#define ARM_SMCCC_FUNC_CALL_COUNT 0xFF00 -#define ARM_SMCCC_FUNC_CALL_UID 0xFF01 -#define ARM_SMCCC_FUNC_CALL_REVISION 0xFF03 +#define ARM_SMCCC_CALL_COUNT_FID(owner) \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_CONV_32, \ + ARM_SMCCC_OWNER_##owner, \ + 0xFF00) + +#define ARM_SMCCC_CALL_UID_FID(owner) \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_CONV_32, \ + ARM_SMCCC_OWNER_##owner, \ + 0xFF01) + +#define ARM_SMCCC_REVISION_FID(owner) \ + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ + ARM_SMCCC_CONV_32, \ + ARM_SMCCC_OWNER_##owner, \ + 0xFF03) /* Only one error code defined in SMCCC */ #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION (-1) -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist 2018-02-02 11:41 ` [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist Julien Grall @ 2018-02-02 13:46 ` Volodymyr Babchuk 2018-02-06 14:53 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-02 13:46 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara On 02.02.18 13:41, Julien Grall wrote: > The current implementation of SMCCC relies on the fact only function > number (bits [15:0]) is enough to identify what to implement. > > However, PSCI call are only available in the range 0x84000000-0x8400001F > and 0xC4000000-0xC400001F. Furthermore, not all SMC32 functions have > equivalent in the SMC64. This is the case of: > * PSCI_VERSION > * CPU_OFF > * MIGRATE_INFO_TYPE > * SYSTEM_OFF > * SYSTEM_RESET > > Similarly call count, call uid, revision can only be query using smc32/hvc32 > fast calls (See 6.2 in ARM DEN 0028B). > > Xen should only implement identifier existing in the specification in > order to avoid potential clash with later revision. Therefore rework the > vsmc code to use the whole function identifier rather than only the > function number. > > At the same time, the new macros for call count, call uid, revision are > renamed to better suit the spec. > > Lastly, update SSSC_SMCCC_FUNCTION_COUNT to match the correct number of > funtions. Note that version is not updated because the number has always > been wrong, and nobody could properly use it. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` > --- > This should be backported to Xen 4.10 as we should not implement > functions identifier that does not exist toprevent clash with a > later revision. > > Changes in v2: > - Rename the call count, call uid, revision macros > - Update SSSC_SMCCC_FUNCTION_COUNT > --- > xen/arch/arm/vsmc.c | 39 ++++++++++++++++++++++----------------- > xen/include/asm-arm/smccc.h | 20 +++++++++++++++++--- > 2 files changed, 39 insertions(+), 20 deletions(-) > > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c > index 997f2e0ebc..3d8cbcc808 100644 > --- a/xen/arch/arm/vsmc.c > +++ b/xen/arch/arm/vsmc.c > @@ -28,7 +28,7 @@ > #define XEN_SMCCC_FUNCTION_COUNT 3 > > /* Number of functions currently supported by Standard Service Service Calls. */ > -#define SSSC_SMCCC_FUNCTION_COUNT 11 > +#define SSSC_SMCCC_FUNCTION_COUNT 14 > > static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid) > { > @@ -84,13 +84,15 @@ static bool fill_function_call_count(struct cpu_user_regs *regs, uint32_t cnt) > /* SMCCC interface for hypervisor. Tell about itself. */ > static bool handle_hypervisor(struct cpu_user_regs *regs) > { > - switch ( smccc_get_fn(get_user_reg(regs, 0)) ) > + uint32_t fid = (uint32_t)get_user_reg(regs, 0); > + > + switch ( fid ) > { > - case ARM_SMCCC_FUNC_CALL_COUNT: > + case ARM_SMCCC_CALL_COUNT_FID(HYPERVISOR): > return fill_function_call_count(regs, XEN_SMCCC_FUNCTION_COUNT); > - case ARM_SMCCC_FUNC_CALL_UID: > + case ARM_SMCCC_CALL_UID_FID(HYPERVISOR): > return fill_uid(regs, XEN_SMCCC_UID); > - case ARM_SMCCC_FUNC_CALL_REVISION: > + case ARM_SMCCC_REVISION_FID(HYPERVISOR): > return fill_revision(regs, XEN_SMCCC_MAJOR_REVISION, > XEN_SMCCC_MINOR_REVISION); > default: > @@ -140,36 +142,37 @@ static bool handle_sssc(struct cpu_user_regs *regs) > { > uint32_t fid = (uint32_t)get_user_reg(regs, 0); > > - switch ( smccc_get_fn(fid) ) > + switch ( fid ) > { > - case PSCI_0_2_FN_PSCI_VERSION: > + case PSCI_0_2_FN32(PSCI_VERSION): > perfc_incr(vpsci_version); > PSCI_SET_RESULT(regs, do_psci_0_2_version()); > return true; > > - case PSCI_0_2_FN_CPU_OFF: > + case PSCI_0_2_FN32(CPU_OFF): > perfc_incr(vpsci_cpu_off); > PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off()); > return true; > > - case PSCI_0_2_FN_MIGRATE_INFO_TYPE: > + case PSCI_0_2_FN32(MIGRATE_INFO_TYPE): > perfc_incr(vpsci_migrate_info_type); > PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); > return true; > > - case PSCI_0_2_FN_SYSTEM_OFF: > + case PSCI_0_2_FN32(SYSTEM_OFF): > perfc_incr(vpsci_system_off); > do_psci_0_2_system_off(); > PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); > return true; > > - case PSCI_0_2_FN_SYSTEM_RESET: > + case PSCI_0_2_FN32(SYSTEM_RESET): > perfc_incr(vpsci_system_reset); > do_psci_0_2_system_reset(); > PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); > return true; > > - case PSCI_0_2_FN_CPU_ON: > + case PSCI_0_2_FN32(CPU_ON): > + case PSCI_0_2_FN64(CPU_ON): > { > register_t vcpuid = PSCI_ARG(regs, 1); > register_t epoint = PSCI_ARG(regs, 2); > @@ -180,7 +183,8 @@ static bool handle_sssc(struct cpu_user_regs *regs) > return true; > } > > - case PSCI_0_2_FN_CPU_SUSPEND: > + case PSCI_0_2_FN32(CPU_SUSPEND): > + case PSCI_0_2_FN64(CPU_SUSPEND): > { > uint32_t pstate = PSCI_ARG32(regs, 1); > register_t epoint = PSCI_ARG(regs, 2); > @@ -191,7 +195,8 @@ static bool handle_sssc(struct cpu_user_regs *regs) > return true; > } > > - case PSCI_0_2_FN_AFFINITY_INFO: > + case PSCI_0_2_FN32(AFFINITY_INFO): > + case PSCI_0_2_FN64(AFFINITY_INFO): > { > register_t taff = PSCI_ARG(regs, 1); > uint32_t laff = PSCI_ARG32(regs, 2); > @@ -201,13 +206,13 @@ static bool handle_sssc(struct cpu_user_regs *regs) > return true; > } > > - case ARM_SMCCC_FUNC_CALL_COUNT: > + case ARM_SMCCC_CALL_COUNT_FID(STANDARD): > return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT); > > - case ARM_SMCCC_FUNC_CALL_UID: > + case ARM_SMCCC_CALL_UID_FID(STANDARD): > return fill_uid(regs, SSSC_SMCCC_UID); > > - case ARM_SMCCC_FUNC_CALL_REVISION: > + case ARM_SMCCC_REVISION_FID(STANDARD): > return fill_revision(regs, SSSC_SMCCC_MAJOR_REVISION, > SSSC_SMCCC_MINOR_REVISION); > > diff --git a/xen/include/asm-arm/smccc.h b/xen/include/asm-arm/smccc.h > index f543dea0bb..62b3a8cdf5 100644 > --- a/xen/include/asm-arm/smccc.h > +++ b/xen/include/asm-arm/smccc.h > @@ -82,9 +82,23 @@ static inline uint32_t smccc_get_owner(register_t funcid) > #define ARM_SMCCC_OWNER_TRUSTED_OS_END 63 > > /* List of generic function numbers */ > -#define ARM_SMCCC_FUNC_CALL_COUNT 0xFF00 > -#define ARM_SMCCC_FUNC_CALL_UID 0xFF01 > -#define ARM_SMCCC_FUNC_CALL_REVISION 0xFF03 > +#define ARM_SMCCC_CALL_COUNT_FID(owner) \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_CONV_32, \ > + ARM_SMCCC_OWNER_##owner, \ > + 0xFF00) > + > +#define ARM_SMCCC_CALL_UID_FID(owner) \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_CONV_32, \ > + ARM_SMCCC_OWNER_##owner, \ > + 0xFF01) > + > +#define ARM_SMCCC_REVISION_FID(owner) \ > + ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > + ARM_SMCCC_CONV_32, \ > + ARM_SMCCC_OWNER_##owner, \ > + 0xFF03) > > /* Only one error code defined in SMCCC */ > #define ARM_SMCCC_ERR_UNKNOWN_FUNCTION (-1) > -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist 2018-02-02 13:46 ` Volodymyr Babchuk @ 2018-02-06 14:53 ` Julien Grall 2018-02-06 15:15 ` Volodymyr Babchuk 0 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-06 14:53 UTC (permalink / raw) To: Volodymyr Babchuk, xen-devel; +Cc: sstabellini, andre.przywara Hi Volodymyr, On 02/02/2018 01:46 PM, Volodymyr Babchuk wrote: > > > On 02.02.18 13:41, Julien Grall wrote: >> The current implementation of SMCCC relies on the fact only function >> number (bits [15:0]) is enough to identify what to implement. >> >> However, PSCI call are only available in the range 0x84000000-0x8400001F >> and 0xC4000000-0xC400001F. Furthermore, not all SMC32 functions have >> equivalent in the SMC64. This is the case of: >> * PSCI_VERSION >> * CPU_OFF >> * MIGRATE_INFO_TYPE >> * SYSTEM_OFF >> * SYSTEM_RESET >> >> Similarly call count, call uid, revision can only be query using >> smc32/hvc32 >> fast calls (See 6.2 in ARM DEN 0028B). >> >> Xen should only implement identifier existing in the specification in >> order to avoid potential clash with later revision. Therefore rework the >> vsmc code to use the whole function identifier rather than only the >> function number. >> >> At the same time, the new macros for call count, call uid, revision are >> renamed to better suit the spec. >> >> Lastly, update SSSC_SMCCC_FUNCTION_COUNT to match the correct number of >> funtions. Note that version is not updated because the number has always >> been wrong, and nobody could properly use it. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> > > `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` Thank you for the reviewed-by. I noticed that you put ` at the beginning and end of the line in your reviewed-by tag so far. Is there any reason for that? Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist 2018-02-06 14:53 ` Julien Grall @ 2018-02-06 15:15 ` Volodymyr Babchuk 2018-02-06 15:29 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-06 15:15 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara Hi Julien, On 06.02.18 16:53, Julien Grall wrote: > Hi Volodymyr, > > On 02/02/2018 01:46 PM, Volodymyr Babchuk wrote: >> >> >> On 02.02.18 13:41, Julien Grall wrote: >>> The current implementation of SMCCC relies on the fact only function >>> number (bits [15:0]) is enough to identify what to implement. >>> >>> However, PSCI call are only available in the range 0x84000000-0x8400001F >>> and 0xC4000000-0xC400001F. Furthermore, not all SMC32 functions have >>> equivalent in the SMC64. This is the case of: >>> * PSCI_VERSION >>> * CPU_OFF >>> * MIGRATE_INFO_TYPE >>> * SYSTEM_OFF >>> * SYSTEM_RESET >>> >>> Similarly call count, call uid, revision can only be query using >>> smc32/hvc32 >>> fast calls (See 6.2 in ARM DEN 0028B). >>> >>> Xen should only implement identifier existing in the specification in >>> order to avoid potential clash with later revision. Therefore rework the >>> vsmc code to use the whole function identifier rather than only the >>> function number. >>> >>> At the same time, the new macros for call count, call uid, revision are >>> renamed to better suit the spec. >>> >>> Lastly, update SSSC_SMCCC_FUNCTION_COUNT to match the correct number of >>> funtions. Note that version is not updated because the number has always >>> been wrong, and nobody could properly use it. >>> >>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> >> `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` > > Thank you for the reviewed-by. I noticed that you put ` at the beginning > and end of the line in your reviewed-by tag so far. Is there any reason > for that? It is out of habit. You see, this is preferred way to add R-b tag at GitHub (or at least in OP-TEE repos). Github uses Markdown, and text in ` becomes formatted as a code. I do code review mostly at github, so it become a habit to add ` around R-b tags. You can ignore this, of course. I'll try to omit ` in the future. -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist 2018-02-06 15:15 ` Volodymyr Babchuk @ 2018-02-06 15:29 ` Julien Grall 0 siblings, 0 replies; 16+ messages in thread From: Julien Grall @ 2018-02-06 15:29 UTC (permalink / raw) To: Volodymyr Babchuk, xen-devel; +Cc: sstabellini, andre.przywara On 02/06/2018 03:15 PM, Volodymyr Babchuk wrote: > Hi Julien, Hi, > On 06.02.18 16:53, Julien Grall wrote: >> On 02/02/2018 01:46 PM, Volodymyr Babchuk wrote: >>> >>> >>> On 02.02.18 13:41, Julien Grall wrote: >>>> The current implementation of SMCCC relies on the fact only function >>>> number (bits [15:0]) is enough to identify what to implement. >>>> >>>> However, PSCI call are only available in the range >>>> 0x84000000-0x8400001F >>>> and 0xC4000000-0xC400001F. Furthermore, not all SMC32 functions have >>>> equivalent in the SMC64. This is the case of: >>>> * PSCI_VERSION >>>> * CPU_OFF >>>> * MIGRATE_INFO_TYPE >>>> * SYSTEM_OFF >>>> * SYSTEM_RESET >>>> >>>> Similarly call count, call uid, revision can only be query using >>>> smc32/hvc32 >>>> fast calls (See 6.2 in ARM DEN 0028B). >>>> >>>> Xen should only implement identifier existing in the specification in >>>> order to avoid potential clash with later revision. Therefore rework >>>> the >>>> vsmc code to use the whole function identifier rather than only the >>>> function number. >>>> >>>> At the same time, the new macros for call count, call uid, revision are >>>> renamed to better suit the spec. >>>> >>>> Lastly, update SSSC_SMCCC_FUNCTION_COUNT to match the correct number of >>>> funtions. Note that version is not updated because the number has >>>> always >>>> been wrong, and nobody could properly use it. >>>> >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>> >>> `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` >> >> Thank you for the reviewed-by. I noticed that you put ` at the >> beginning and end of the line in your reviewed-by tag so far. Is there >> any reason for that? > > It is out of habit. You see, this is preferred way to add R-b tag at > GitHub (or at least in OP-TEE repos). Github uses Markdown, and text in > ` becomes formatted as a code. I do code review mostly at github, > so it become a habit to add ` around R-b tags. You can ignore this, of > course. I'll try to omit ` in the future. Oh, good to know. I am usually copying the full line and my editor was not happy at all with the `. For Xen, we tend to only have the Reviewed-by tag on a line. Anyway, thank you for reviewing the series :). I will resend a version soon. If you have time, you might want to have a look at "xen/arm: PSCI 1.1 and SMCCC-1.1 support and XSA-254 variant 2 update" [1]. Cheers, [1] https://lists.xen.org/archives/html/xen-devel/2018-02/msg00285.html -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-02 11:41 [PATCH v2 0/3] xen/arm: SMCCC fixes and PSCI clean-up Julien Grall 2018-02-02 11:41 ` [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU Julien Grall 2018-02-02 11:41 ` [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist Julien Grall @ 2018-02-02 11:41 ` Julien Grall 2018-02-02 14:23 ` Volodymyr Babchuk 2 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-02 11:41 UTC (permalink / raw) To: xen-devel; +Cc: sstabellini, volodymyr_babchuk, andre.przywara, Julien Grall At the moment PSCI function dispatching is done in vsmc.c and the function implementation in vpsci.c. Some bits of the implementation is even done in vsmc.c (see PSCI_SYSTEM_RESET). This means that it is difficult to follow the implementation and also requires to export functions for each PSCI functions. Therefore move PSCI dispatching in two new functions do_vpsci_0_1_call and do_vpsci_0_2_call. The former will handle PSCI 0.1 call while the latter 0.2 or later call. At the same time, a new header vpsci.h was created to contain all definitions for virtual PSCI and avoid confusion with the host PSCI. Signed-off-by: Julien Grall <julien.grall@linaro.org> --- Changes in v2: - Add a 'v' in the function names to help distinguish virtual vs physical PSCI - Introduce vpsci.h and VSCPI_NR_FUNCS --- xen/arch/arm/vpsci.c | 147 +++++++++++++++++++++++++++++++++++++++----- xen/arch/arm/vsmc.c | 99 ++--------------------------- xen/include/asm-arm/psci.h | 19 ------ xen/include/asm-arm/vpsci.h | 13 ++++ 4 files changed, 152 insertions(+), 126 deletions(-) create mode 100644 xen/include/asm-arm/vpsci.h diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c index 979d32ed6d..884f0fa710 100644 --- a/xen/arch/arm/vpsci.c +++ b/xen/arch/arm/vpsci.c @@ -16,7 +16,7 @@ #include <asm/current.h> #include <asm/vgic.h> -#include <asm/psci.h> +#include <asm/vpsci.h> #include <asm/event.h> #include <public/sched.h> @@ -91,12 +91,12 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point, return PSCI_SUCCESS; } -int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) +static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) { return do_common_cpu_on(vcpuid, entry_point, 0 , PSCI_VERSION(0, 1)); } -int32_t do_psci_cpu_off(uint32_t power_state) +static int32_t do_psci_cpu_off(uint32_t power_state) { struct vcpu *v = current; if ( !test_and_set_bit(_VPF_down, &v->pause_flags) ) @@ -104,13 +104,14 @@ int32_t do_psci_cpu_off(uint32_t power_state) return PSCI_SUCCESS; } -uint32_t do_psci_0_2_version(void) +static uint32_t do_psci_0_2_version(void) { return PSCI_VERSION(0, 2); } -register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, - register_t context_id) +static register_t do_psci_0_2_cpu_suspend(uint32_t power_state, + register_t entry_point, + register_t context_id) { struct vcpu *v = current; @@ -123,13 +124,14 @@ register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, return PSCI_SUCCESS; } -int32_t do_psci_0_2_cpu_off(void) +static int32_t do_psci_0_2_cpu_off(void) { return do_psci_cpu_off(0); } -int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, - register_t context_id) +static int32_t do_psci_0_2_cpu_on(register_t target_cpu, + register_t entry_point, + register_t context_id) { return do_common_cpu_on(target_cpu, entry_point, context_id, PSCI_VERSION(0, 2)); @@ -144,8 +146,8 @@ static const unsigned long target_affinity_mask[] = { #endif }; -int32_t do_psci_0_2_affinity_info(register_t target_affinity, - uint32_t lowest_affinity_level) +static int32_t do_psci_0_2_affinity_info(register_t target_affinity, + uint32_t lowest_affinity_level) { struct domain *d = current->domain; struct vcpu *v; @@ -172,23 +174,140 @@ int32_t do_psci_0_2_affinity_info(register_t target_affinity, return PSCI_0_2_AFFINITY_LEVEL_OFF; } -uint32_t do_psci_0_2_migrate_info_type(void) +static uint32_t do_psci_0_2_migrate_info_type(void) { return PSCI_0_2_TOS_MP_OR_NOT_PRESENT; } -void do_psci_0_2_system_off( void ) +static void do_psci_0_2_system_off( void ) { struct domain *d = current->domain; domain_shutdown(d,SHUTDOWN_poweroff); } -void do_psci_0_2_system_reset(void) +static void do_psci_0_2_system_reset(void) { struct domain *d = current->domain; domain_shutdown(d,SHUTDOWN_reboot); } +#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val) +#define PSCI_ARG(reg, n) get_user_reg(reg, n) + +#ifdef CONFIG_ARM_64 +#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n)) +#else +#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n) +#endif + +/* + * PSCI 0.1 calls. It will return false if the function ID is not + * handled. + */ +bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid) +{ + switch ( (uint32_t)get_user_reg(regs, 0) ) + { + case PSCI_cpu_off: + { + uint32_t pstate = PSCI_ARG32(regs, 1); + + perfc_incr(vpsci_cpu_off); + PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate)); + return true; + } + case PSCI_cpu_on: + { + uint32_t vcpuid = PSCI_ARG32(regs, 1); + register_t epoint = PSCI_ARG(regs, 2); + + perfc_incr(vpsci_cpu_on); + PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint)); + return true; + } + default: + return false; + } +} + +/* + * PSCI 0.2 or later calls. It will return false if the function ID is + * not handled. + */ +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) +{ + /* + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when + * adding/removing a function + */ + switch ( fid ) + { + case PSCI_0_2_FN32(PSCI_VERSION): + perfc_incr(vpsci_version); + PSCI_SET_RESULT(regs, do_psci_0_2_version()); + return true; + + case PSCI_0_2_FN32(CPU_OFF): + perfc_incr(vpsci_cpu_off); + PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off()); + return true; + + case PSCI_0_2_FN32(MIGRATE_INFO_TYPE): + perfc_incr(vpsci_migrate_info_type); + PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); + return true; + + case PSCI_0_2_FN32(SYSTEM_OFF): + perfc_incr(vpsci_system_off); + do_psci_0_2_system_off(); + PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); + return true; + + case PSCI_0_2_FN32(SYSTEM_RESET): + perfc_incr(vpsci_system_reset); + do_psci_0_2_system_reset(); + PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); + return true; + + case PSCI_0_2_FN32(CPU_ON): + case PSCI_0_2_FN64(CPU_ON): + { + register_t vcpuid = PSCI_ARG(regs, 1); + register_t epoint = PSCI_ARG(regs, 2); + register_t cid = PSCI_ARG(regs, 3); + + perfc_incr(vpsci_cpu_on); + PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid)); + return true; + } + + case PSCI_0_2_FN32(CPU_SUSPEND): + case PSCI_0_2_FN64(CPU_SUSPEND): + { + uint32_t pstate = PSCI_ARG32(regs, 1); + register_t epoint = PSCI_ARG(regs, 2); + register_t cid = PSCI_ARG(regs, 3); + + perfc_incr(vpsci_cpu_suspend); + PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid)); + return true; + } + + case PSCI_0_2_FN32(AFFINITY_INFO): + case PSCI_0_2_FN64(AFFINITY_INFO): + { + register_t taff = PSCI_ARG(regs, 1); + uint32_t laff = PSCI_ARG32(regs, 2); + + perfc_incr(vpsci_cpu_affinity_info); + PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff)); + return true; + } + default: + return false; + } +} + /* * Local variables: * mode: C diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c index 3d8cbcc808..3d3bd95fee 100644 --- a/xen/arch/arm/vsmc.c +++ b/xen/arch/arm/vsmc.c @@ -19,16 +19,16 @@ #include <xen/types.h> #include <public/arch-arm/smccc.h> #include <asm/monitor.h> -#include <asm/psci.h> #include <asm/regs.h> #include <asm/smccc.h> #include <asm/traps.h> +#include <asm/vpsci.h> /* Number of functions currently supported by Hypervisor Service. */ #define XEN_SMCCC_FUNCTION_COUNT 3 /* Number of functions currently supported by Standard Service Service Calls. */ -#define SSSC_SMCCC_FUNCTION_COUNT 14 +#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS) static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid) { @@ -100,41 +100,13 @@ static bool handle_hypervisor(struct cpu_user_regs *regs) } } -#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val) -#define PSCI_ARG(reg, n) get_user_reg(reg, n) - -#ifdef CONFIG_ARM_64 -#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n)) -#else -#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n) -#endif - /* Existing (pre SMCCC) APIs. This includes PSCI 0.1 interface */ static bool handle_existing_apis(struct cpu_user_regs *regs) { /* Only least 32 bits are significant (ARM DEN 0028B, page 12) */ - switch ( (uint32_t)get_user_reg(regs, 0) ) - { - case PSCI_cpu_off: - { - uint32_t pstate = PSCI_ARG32(regs, 1); - - perfc_incr(vpsci_cpu_off); - PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate)); - return true; - } - case PSCI_cpu_on: - { - uint32_t vcpuid = PSCI_ARG32(regs, 1); - register_t epoint = PSCI_ARG(regs, 2); + uint32_t fid = (uint32_t)get_user_reg(regs, 0); - perfc_incr(vpsci_cpu_on); - PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint)); - return true; - } - default: - return false; - } + return do_vpsci_0_1_call(regs, fid); } /* PSCI 0.2 interface and other Standard Secure Calls */ @@ -142,70 +114,11 @@ static bool handle_sssc(struct cpu_user_regs *regs) { uint32_t fid = (uint32_t)get_user_reg(regs, 0); - switch ( fid ) - { - case PSCI_0_2_FN32(PSCI_VERSION): - perfc_incr(vpsci_version); - PSCI_SET_RESULT(regs, do_psci_0_2_version()); + if ( do_vpsci_0_2_call(regs, fid) ) return true; - case PSCI_0_2_FN32(CPU_OFF): - perfc_incr(vpsci_cpu_off); - PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off()); - return true; - - case PSCI_0_2_FN32(MIGRATE_INFO_TYPE): - perfc_incr(vpsci_migrate_info_type); - PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); - return true; - - case PSCI_0_2_FN32(SYSTEM_OFF): - perfc_incr(vpsci_system_off); - do_psci_0_2_system_off(); - PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); - return true; - - case PSCI_0_2_FN32(SYSTEM_RESET): - perfc_incr(vpsci_system_reset); - do_psci_0_2_system_reset(); - PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); - return true; - - case PSCI_0_2_FN32(CPU_ON): - case PSCI_0_2_FN64(CPU_ON): - { - register_t vcpuid = PSCI_ARG(regs, 1); - register_t epoint = PSCI_ARG(regs, 2); - register_t cid = PSCI_ARG(regs, 3); - - perfc_incr(vpsci_cpu_on); - PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid)); - return true; - } - - case PSCI_0_2_FN32(CPU_SUSPEND): - case PSCI_0_2_FN64(CPU_SUSPEND): - { - uint32_t pstate = PSCI_ARG32(regs, 1); - register_t epoint = PSCI_ARG(regs, 2); - register_t cid = PSCI_ARG(regs, 3); - - perfc_incr(vpsci_cpu_suspend); - PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid)); - return true; - } - - case PSCI_0_2_FN32(AFFINITY_INFO): - case PSCI_0_2_FN64(AFFINITY_INFO): + switch ( fid ) { - register_t taff = PSCI_ARG(regs, 1); - uint32_t laff = PSCI_ARG32(regs, 2); - - perfc_incr(vpsci_cpu_affinity_info); - PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff)); - return true; - } - case ARM_SMCCC_CALL_COUNT_FID(STANDARD): return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT); diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h index 32c1f81f21..3c44468e72 100644 --- a/xen/include/asm-arm/psci.h +++ b/xen/include/asm-arm/psci.h @@ -22,25 +22,6 @@ int call_psci_cpu_on(int cpu); void call_psci_system_off(void); void call_psci_system_reset(void); -/* functions to handle guest PSCI requests */ -int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point); -int32_t do_psci_cpu_off(uint32_t power_state); -int32_t do_psci_cpu_suspend(uint32_t power_state, register_t entry_point); -int32_t do_psci_migrate(uint32_t vcpuid); - -/* PSCI 0.2 functions to handle guest PSCI requests */ -uint32_t do_psci_0_2_version(void); -register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, - register_t context_id); -int32_t do_psci_0_2_cpu_off(void); -int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, - register_t context_id); -int32_t do_psci_0_2_affinity_info(register_t target_affinity, - uint32_t lowest_affinity_level); -uint32_t do_psci_0_2_migrate_info_type(void); -void do_psci_0_2_system_off(void); -void do_psci_0_2_system_reset(void); - /* PSCI v0.2 interface */ #define PSCI_0_2_FN32(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ ARM_SMCCC_CONV_32, \ diff --git a/xen/include/asm-arm/vpsci.h b/xen/include/asm-arm/vpsci.h new file mode 100644 index 0000000000..d6a890f6a2 --- /dev/null +++ b/xen/include/asm-arm/vpsci.h @@ -0,0 +1,13 @@ +#ifndef __ASM_VPSCI_H__ +#define __ASM_VPSCI_H__ + +#include <asm/psci.h> + +/* Number of function implemented by virtual PSCI (only 0.2 or later) */ +#define VPSCI_NR_FUNCS 11 + +/* Functions handle PSCI calls from the guests */ +bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid); +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid); + +#endif /* __ASM_VPSCI_H__ */ -- 2.11.0 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-02 11:41 ` [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c Julien Grall @ 2018-02-02 14:23 ` Volodymyr Babchuk 2018-02-02 14:31 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-02 14:23 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara On 02.02.18 13:41, Julien Grall wrote: > At the moment PSCI function dispatching is done in vsmc.c and the > function implementation in vpsci.c. Some bits of the implementation is > even done in vsmc.c (see PSCI_SYSTEM_RESET). > > This means that it is difficult to follow the implementation and also > requires to export functions for each PSCI functions. > > Therefore move PSCI dispatching in two new functions do_vpsci_0_1_call > and do_vpsci_0_2_call. The former will handle PSCI 0.1 call while the latter > 0.2 or later call. > > At the same time, a new header vpsci.h was created to contain all > definitions for virtual PSCI and avoid confusion with the host PSCI. > > Signed-off-by: Julien Grall <julien.grall@linaro.org> > I'm okay with this generally, you can have my r-b: `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` But there are couple of minor questions below. > --- > Changes in v2: > - Add a 'v' in the function names to help distinguish virtual vs > physical PSCI > - Introduce vpsci.h and VSCPI_NR_FUNCS > --- > xen/arch/arm/vpsci.c | 147 +++++++++++++++++++++++++++++++++++++++----- > xen/arch/arm/vsmc.c | 99 ++--------------------------- > xen/include/asm-arm/psci.h | 19 ------ > xen/include/asm-arm/vpsci.h | 13 ++++ > 4 files changed, 152 insertions(+), 126 deletions(-) > create mode 100644 xen/include/asm-arm/vpsci.h > > diff --git a/xen/arch/arm/vpsci.c b/xen/arch/arm/vpsci.c > index 979d32ed6d..884f0fa710 100644 > --- a/xen/arch/arm/vpsci.c > +++ b/xen/arch/arm/vpsci.c > @@ -16,7 +16,7 @@ > > #include <asm/current.h> > #include <asm/vgic.h> > -#include <asm/psci.h> > +#include <asm/vpsci.h> > #include <asm/event.h> > > #include <public/sched.h> > @@ -91,12 +91,12 @@ static int do_common_cpu_on(register_t target_cpu, register_t entry_point, > return PSCI_SUCCESS; > } > > -int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > +static int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point) > { > return do_common_cpu_on(vcpuid, entry_point, 0 , PSCI_VERSION(0, 1)); > } > > -int32_t do_psci_cpu_off(uint32_t power_state) > +static int32_t do_psci_cpu_off(uint32_t power_state) > { > struct vcpu *v = current; > if ( !test_and_set_bit(_VPF_down, &v->pause_flags) ) > @@ -104,13 +104,14 @@ int32_t do_psci_cpu_off(uint32_t power_state) > return PSCI_SUCCESS; > } > > -uint32_t do_psci_0_2_version(void) > +static uint32_t do_psci_0_2_version(void) > { > return PSCI_VERSION(0, 2); > } > > -register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, > - register_t context_id) > +static register_t do_psci_0_2_cpu_suspend(uint32_t power_state, > + register_t entry_point, > + register_t context_id) > { > struct vcpu *v = current; > > @@ -123,13 +124,14 @@ register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, > return PSCI_SUCCESS; > } > > -int32_t do_psci_0_2_cpu_off(void) > +static int32_t do_psci_0_2_cpu_off(void) > { > return do_psci_cpu_off(0); > } > > -int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, > - register_t context_id) > +static int32_t do_psci_0_2_cpu_on(register_t target_cpu, > + register_t entry_point, > + register_t context_id) > { > return do_common_cpu_on(target_cpu, entry_point, context_id, > PSCI_VERSION(0, 2)); > @@ -144,8 +146,8 @@ static const unsigned long target_affinity_mask[] = { > #endif > }; > > -int32_t do_psci_0_2_affinity_info(register_t target_affinity, > - uint32_t lowest_affinity_level) > +static int32_t do_psci_0_2_affinity_info(register_t target_affinity, > + uint32_t lowest_affinity_level) > { > struct domain *d = current->domain; > struct vcpu *v; > @@ -172,23 +174,140 @@ int32_t do_psci_0_2_affinity_info(register_t target_affinity, > return PSCI_0_2_AFFINITY_LEVEL_OFF; > } > > -uint32_t do_psci_0_2_migrate_info_type(void) > +static uint32_t do_psci_0_2_migrate_info_type(void) > { > return PSCI_0_2_TOS_MP_OR_NOT_PRESENT; > } > > -void do_psci_0_2_system_off( void ) > +static void do_psci_0_2_system_off( void ) > { > struct domain *d = current->domain; > domain_shutdown(d,SHUTDOWN_poweroff); > } > > -void do_psci_0_2_system_reset(void) > +static void do_psci_0_2_system_reset(void) > { > struct domain *d = current->domain; > domain_shutdown(d,SHUTDOWN_reboot); > } > > +#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val) > +#define PSCI_ARG(reg, n) get_user_reg(reg, n) > + > +#ifdef CONFIG_ARM_64 > +#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n)) > +#else > +#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n) > +#endif > + > +/* > + * PSCI 0.1 calls. It will return false if the function ID is not > + * handled. > + */ > +bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid) > +{ > + switch ( (uint32_t)get_user_reg(regs, 0) ) > + { > + case PSCI_cpu_off: > + { > + uint32_t pstate = PSCI_ARG32(regs, 1); > + > + perfc_incr(vpsci_cpu_off); > + PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate)); > + return true; > + } > + case PSCI_cpu_on: > + { > + uint32_t vcpuid = PSCI_ARG32(regs, 1); > + register_t epoint = PSCI_ARG(regs, 2); > + > + perfc_incr(vpsci_cpu_on); > + PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint)); > + return true; > + } > + default: > + return false; > + } > +} > + > +/* > + * PSCI 0.2 or later calls. It will return false if the function ID is > + * not handled. > + */ > +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) > +{ > + /* > + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when > + * adding/removing a function > + */ Should we also update revision of SSSC interface as well? SMCCC requires this. > + switch ( fid ) > + { > + case PSCI_0_2_FN32(PSCI_VERSION): > + perfc_incr(vpsci_version); > + PSCI_SET_RESULT(regs, do_psci_0_2_version()); > + return true; > + > + case PSCI_0_2_FN32(CPU_OFF): > + perfc_incr(vpsci_cpu_off); > + PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off()); > + return true; > + > + case PSCI_0_2_FN32(MIGRATE_INFO_TYPE): > + perfc_incr(vpsci_migrate_info_type); > + PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); > + return true; > + > + case PSCI_0_2_FN32(SYSTEM_OFF): > + perfc_incr(vpsci_system_off); > + do_psci_0_2_system_off(); > + PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); > + return true; > + > + case PSCI_0_2_FN32(SYSTEM_RESET): > + perfc_incr(vpsci_system_reset); > + do_psci_0_2_system_reset(); > + PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); > + return true; > + > + case PSCI_0_2_FN32(CPU_ON): > + case PSCI_0_2_FN64(CPU_ON): > + { > + register_t vcpuid = PSCI_ARG(regs, 1); > + register_t epoint = PSCI_ARG(regs, 2); > + register_t cid = PSCI_ARG(regs, 3); > + > + perfc_incr(vpsci_cpu_on); > + PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid)); > + return true; > + } > + > + case PSCI_0_2_FN32(CPU_SUSPEND): > + case PSCI_0_2_FN64(CPU_SUSPEND): > + { > + uint32_t pstate = PSCI_ARG32(regs, 1); > + register_t epoint = PSCI_ARG(regs, 2); > + register_t cid = PSCI_ARG(regs, 3); > + > + perfc_incr(vpsci_cpu_suspend); > + PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid)); > + return true; > + } > + > + case PSCI_0_2_FN32(AFFINITY_INFO): > + case PSCI_0_2_FN64(AFFINITY_INFO): > + { > + register_t taff = PSCI_ARG(regs, 1); > + uint32_t laff = PSCI_ARG32(regs, 2); > + > + perfc_incr(vpsci_cpu_affinity_info); > + PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff)); > + return true; > + } > + default: > + return false; > + } > +} > + > /* > * Local variables: > * mode: C > diff --git a/xen/arch/arm/vsmc.c b/xen/arch/arm/vsmc.c > index 3d8cbcc808..3d3bd95fee 100644 > --- a/xen/arch/arm/vsmc.c > +++ b/xen/arch/arm/vsmc.c > @@ -19,16 +19,16 @@ > #include <xen/types.h> > #include <public/arch-arm/smccc.h> > #include <asm/monitor.h> > -#include <asm/psci.h> > #include <asm/regs.h> > #include <asm/smccc.h> > #include <asm/traps.h> > +#include <asm/vpsci.h> > > /* Number of functions currently supported by Hypervisor Service. */ > #define XEN_SMCCC_FUNCTION_COUNT 3 > > /* Number of functions currently supported by Standard Service Service Calls. */ > -#define SSSC_SMCCC_FUNCTION_COUNT 14 > +#define SSSC_SMCCC_FUNCTION_COUNT (3 + VPSCI_NR_FUNCS) > > static bool fill_uid(struct cpu_user_regs *regs, xen_uuid_t uuid) > { > @@ -100,41 +100,13 @@ static bool handle_hypervisor(struct cpu_user_regs *regs) > } > } > > -#define PSCI_SET_RESULT(reg, val) set_user_reg(reg, 0, val) > -#define PSCI_ARG(reg, n) get_user_reg(reg, n) > - > -#ifdef CONFIG_ARM_64 > -#define PSCI_ARG32(reg, n) (uint32_t)(get_user_reg(reg, n)) > -#else > -#define PSCI_ARG32(reg, n) PSCI_ARG(reg, n) > -#endif > - > /* Existing (pre SMCCC) APIs. This includes PSCI 0.1 interface */ > static bool handle_existing_apis(struct cpu_user_regs *regs) > { > /* Only least 32 bits are significant (ARM DEN 0028B, page 12) */ > - switch ( (uint32_t)get_user_reg(regs, 0) ) > - { > - case PSCI_cpu_off: > - { > - uint32_t pstate = PSCI_ARG32(regs, 1); > - > - perfc_incr(vpsci_cpu_off); > - PSCI_SET_RESULT(regs, do_psci_cpu_off(pstate)); > - return true; > - } > - case PSCI_cpu_on: > - { > - uint32_t vcpuid = PSCI_ARG32(regs, 1); > - register_t epoint = PSCI_ARG(regs, 2); > + uint32_t fid = (uint32_t)get_user_reg(regs, 0); > > - perfc_incr(vpsci_cpu_on); > - PSCI_SET_RESULT(regs, do_psci_cpu_on(vcpuid, epoint)); > - return true; > - } > - default: > - return false; > - } > + return do_vpsci_0_1_call(regs, fid); > } > > /* PSCI 0.2 interface and other Standard Secure Calls */ > @@ -142,70 +114,11 @@ static bool handle_sssc(struct cpu_user_regs *regs) > { > uint32_t fid = (uint32_t)get_user_reg(regs, 0); > > - switch ( fid ) > - { > - case PSCI_0_2_FN32(PSCI_VERSION): > - perfc_incr(vpsci_version); > - PSCI_SET_RESULT(regs, do_psci_0_2_version()); > + if ( do_vpsci_0_2_call(regs, fid) ) > return true; > > - case PSCI_0_2_FN32(CPU_OFF): > - perfc_incr(vpsci_cpu_off); > - PSCI_SET_RESULT(regs, do_psci_0_2_cpu_off()); > - return true; > - > - case PSCI_0_2_FN32(MIGRATE_INFO_TYPE): > - perfc_incr(vpsci_migrate_info_type); > - PSCI_SET_RESULT(regs, do_psci_0_2_migrate_info_type()); > - return true; > - > - case PSCI_0_2_FN32(SYSTEM_OFF): > - perfc_incr(vpsci_system_off); > - do_psci_0_2_system_off(); > - PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); > - return true; > - > - case PSCI_0_2_FN32(SYSTEM_RESET): > - perfc_incr(vpsci_system_reset); > - do_psci_0_2_system_reset(); > - PSCI_SET_RESULT(regs, PSCI_INTERNAL_FAILURE); > - return true; > - > - case PSCI_0_2_FN32(CPU_ON): > - case PSCI_0_2_FN64(CPU_ON): > - { > - register_t vcpuid = PSCI_ARG(regs, 1); > - register_t epoint = PSCI_ARG(regs, 2); > - register_t cid = PSCI_ARG(regs, 3); > - > - perfc_incr(vpsci_cpu_on); > - PSCI_SET_RESULT(regs, do_psci_0_2_cpu_on(vcpuid, epoint, cid)); > - return true; > - } > - > - case PSCI_0_2_FN32(CPU_SUSPEND): > - case PSCI_0_2_FN64(CPU_SUSPEND): > - { > - uint32_t pstate = PSCI_ARG32(regs, 1); > - register_t epoint = PSCI_ARG(regs, 2); > - register_t cid = PSCI_ARG(regs, 3); > - > - perfc_incr(vpsci_cpu_suspend); > - PSCI_SET_RESULT(regs, do_psci_0_2_cpu_suspend(pstate, epoint, cid)); > - return true; > - } > - > - case PSCI_0_2_FN32(AFFINITY_INFO): > - case PSCI_0_2_FN64(AFFINITY_INFO): > + switch ( fid ) > { > - register_t taff = PSCI_ARG(regs, 1); > - uint32_t laff = PSCI_ARG32(regs, 2); > - > - perfc_incr(vpsci_cpu_affinity_info); > - PSCI_SET_RESULT(regs, do_psci_0_2_affinity_info(taff, laff)); > - return true; > - } > - > case ARM_SMCCC_CALL_COUNT_FID(STANDARD): > return fill_function_call_count(regs, SSSC_SMCCC_FUNCTION_COUNT); > > diff --git a/xen/include/asm-arm/psci.h b/xen/include/asm-arm/psci.h > index 32c1f81f21..3c44468e72 100644 > --- a/xen/include/asm-arm/psci.h > +++ b/xen/include/asm-arm/psci.h > @@ -22,25 +22,6 @@ int call_psci_cpu_on(int cpu); > void call_psci_system_off(void); > void call_psci_system_reset(void); > > -/* functions to handle guest PSCI requests */ > -int32_t do_psci_cpu_on(uint32_t vcpuid, register_t entry_point); > -int32_t do_psci_cpu_off(uint32_t power_state); > -int32_t do_psci_cpu_suspend(uint32_t power_state, register_t entry_point); > -int32_t do_psci_migrate(uint32_t vcpuid); > - > -/* PSCI 0.2 functions to handle guest PSCI requests */ > -uint32_t do_psci_0_2_version(void); > -register_t do_psci_0_2_cpu_suspend(uint32_t power_state, register_t entry_point, > - register_t context_id); > -int32_t do_psci_0_2_cpu_off(void); > -int32_t do_psci_0_2_cpu_on(register_t target_cpu, register_t entry_point, > - register_t context_id); > -int32_t do_psci_0_2_affinity_info(register_t target_affinity, > - uint32_t lowest_affinity_level); > -uint32_t do_psci_0_2_migrate_info_type(void); > -void do_psci_0_2_system_off(void); > -void do_psci_0_2_system_reset(void); > - > /* PSCI v0.2 interface */ > #define PSCI_0_2_FN32(name) ARM_SMCCC_CALL_VAL(ARM_SMCCC_FAST_CALL, \ > ARM_SMCCC_CONV_32, \ > diff --git a/xen/include/asm-arm/vpsci.h b/xen/include/asm-arm/vpsci.h > new file mode 100644 > index 0000000000..d6a890f6a2 > --- /dev/null > +++ b/xen/include/asm-arm/vpsci.h > @@ -0,0 +1,13 @@ I'm not sure, should you add license information? > +#ifndef __ASM_VPSCI_H__ > +#define __ASM_VPSCI_H__ > + > +#include <asm/psci.h> > + > +/* Number of function implemented by virtual PSCI (only 0.2 or later) */ > +#define VPSCI_NR_FUNCS 11 > + > +/* Functions handle PSCI calls from the guests */ > +bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid); > +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid); > + > +#endif /* __ASM_VPSCI_H__ */ > And the same about Emacs tags. -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-02 14:23 ` Volodymyr Babchuk @ 2018-02-02 14:31 ` Julien Grall 2018-02-02 15:40 ` Volodymyr Babchuk 0 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-02 14:31 UTC (permalink / raw) To: Volodymyr Babchuk, xen-devel; +Cc: sstabellini, andre.przywara Hi, On 02/02/18 14:23, Volodymyr Babchuk wrote: > On 02.02.18 13:41, Julien Grall wrote: >> At the moment PSCI function dispatching is done in vsmc.c and the >> function implementation in vpsci.c. Some bits of the implementation is >> even done in vsmc.c (see PSCI_SYSTEM_RESET). >> >> This means that it is difficult to follow the implementation and also >> requires to export functions for each PSCI functions. >> >> Therefore move PSCI dispatching in two new functions do_vpsci_0_1_call >> and do_vpsci_0_2_call. The former will handle PSCI 0.1 call while the >> latter >> 0.2 or later call. >> >> At the same time, a new header vpsci.h was created to contain all >> definitions for virtual PSCI and avoid confusion with the host PSCI. >> >> Signed-off-by: Julien Grall <julien.grall@linaro.org> >> > I'm okay with this generally, you can have my r-b: > `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` I would not have gave a reviewed-by with the comments you gave below ;). >> +/* >> + * PSCI 0.2 or later calls. It will return false if the function ID is >> + * not handled. >> + */ >> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) >> +{ >> + /* >> + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when >> + * adding/removing a function >> + */ > Should we also update revision of SSSC interface as well? SMCCC requires > this. Meh, you can't rely on the SSSC revision most of the time as the version for PSCI is done through PSCI_get_version. I can add a comment if you want. [...] >> diff --git a/xen/include/asm-arm/vpsci.h b/xen/include/asm-arm/vpsci.h >> new file mode 100644 >> index 0000000000..d6a890f6a2 >> --- /dev/null >> +++ b/xen/include/asm-arm/vpsci.h >> @@ -0,0 +1,13 @@ > I'm not sure, should you add license information? I think so. > >> +#ifndef __ASM_VPSCI_H__ >> +#define __ASM_VPSCI_H__ >> + >> +#include <asm/psci.h> >> + >> +/* Number of function implemented by virtual PSCI (only 0.2 or later) */ >> +#define VPSCI_NR_FUNCS 11 >> + >> +/* Functions handle PSCI calls from the guests */ >> +bool do_vpsci_0_1_call(struct cpu_user_regs *regs, uint32_t fid); >> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid); >> + >> +#endif /* __ASM_VPSCI_H__ */ >> > > And the same about Emacs tags. Good catch. I will send a new version with the comments addressed. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-02 14:31 ` Julien Grall @ 2018-02-02 15:40 ` Volodymyr Babchuk 2018-02-06 14:45 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-02 15:40 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara Hi On 02.02.18 16:31, Julien Grall wrote: > Hi, > > On 02/02/18 14:23, Volodymyr Babchuk wrote: >> On 02.02.18 13:41, Julien Grall wrote: >>> At the moment PSCI function dispatching is done in vsmc.c and the >>> function implementation in vpsci.c. Some bits of the implementation is >>> even done in vsmc.c (see PSCI_SYSTEM_RESET). >>> >>> This means that it is difficult to follow the implementation and also >>> requires to export functions for each PSCI functions. >>> >>> Therefore move PSCI dispatching in two new functions do_vpsci_0_1_call >>> and do_vpsci_0_2_call. The former will handle PSCI 0.1 call while the >>> latter >>> 0.2 or later call. >>> >>> At the same time, a new header vpsci.h was created to contain all >>> definitions for virtual PSCI and avoid confusion with the host PSCI. >>> >>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>> >> I'm okay with this generally, you can have my r-b: >> `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` > > I would not have gave a reviewed-by with the comments you gave below ;). > Point taken :) >>> +/* >>> + * PSCI 0.2 or later calls. It will return false if the function ID is >>> + * not handled. >>> + */ >>> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) >>> +{ >>> + /* >>> + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when >>> + * adding/removing a function >>> + */ >> Should we also update revision of SSSC interface as well? SMCCC >> requires this. > > Meh, you can't rely on the SSSC revision most of the time as the version > for PSCI is done through PSCI_get_version. I can add a comment if you want. > Yes, I agree with you. But section 5.4 of SMCCC 1.0 applies to SSSC as well. So you can write something like "ARM SMCCC requires that SSSC revision and function call count should be updated every time you add or remove a function" [...] -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-02 15:40 ` Volodymyr Babchuk @ 2018-02-06 14:45 ` Julien Grall 2018-02-06 15:28 ` Volodymyr Babchuk 0 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-06 14:45 UTC (permalink / raw) To: Volodymyr Babchuk, xen-devel; +Cc: sstabellini, andre.przywara On 02/02/2018 03:40 PM, Volodymyr Babchuk wrote: > Hi Hi, > On 02.02.18 16:31, Julien Grall wrote: >> Hi, >> >> On 02/02/18 14:23, Volodymyr Babchuk wrote: >>> On 02.02.18 13:41, Julien Grall wrote: >>>> At the moment PSCI function dispatching is done in vsmc.c and the >>>> function implementation in vpsci.c. Some bits of the implementation is >>>> even done in vsmc.c (see PSCI_SYSTEM_RESET). >>>> >>>> This means that it is difficult to follow the implementation and also >>>> requires to export functions for each PSCI functions. >>>> >>>> Therefore move PSCI dispatching in two new functions do_vpsci_0_1_call >>>> and do_vpsci_0_2_call. The former will handle PSCI 0.1 call while >>>> the latter >>>> 0.2 or later call. >>>> >>>> At the same time, a new header vpsci.h was created to contain all >>>> definitions for virtual PSCI and avoid confusion with the host PSCI. >>>> >>>> Signed-off-by: Julien Grall <julien.grall@linaro.org> >>>> >>> I'm okay with this generally, you can have my r-b: >>> `Reviewed-by: Volodymyr Babchuk <volodymyr_babchuk@epam.com>` >> >> I would not have gave a reviewed-by with the comments you gave below ;). >> > > Point taken :) > >>>> +/* >>>> + * PSCI 0.2 or later calls. It will return false if the function ID is >>>> + * not handled. >>>> + */ >>>> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) >>>> +{ >>>> + /* >>>> + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when >>>> + * adding/removing a function >>>> + */ >>> Should we also update revision of SSSC interface as well? SMCCC >>> requires this. >> >> Meh, you can't rely on the SSSC revision most of the time as the >> version for PSCI is done through PSCI_get_version. I can add a comment >> if you want. >> > Yes, I agree with you. But section 5.4 of SMCCC 1.0 applies to SSSC as > well. So you can write something like "ARM SMCCC requires that SSSC > revision and function call count should be updated every time you add or > remove a function" Usually we update versioning number only once per release. I would follow the same approach for this one. So I would suggest "VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when adding removing a function. SSCCC_SMCCC_*_REVISION should be updated once per Xen release". Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-06 14:45 ` Julien Grall @ 2018-02-06 15:28 ` Volodymyr Babchuk 2018-02-06 15:39 ` Julien Grall 0 siblings, 1 reply; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-06 15:28 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara Hi Julien, On 06.02.18 16:45, Julien Grall wrote: [...] >>>>> +/* >>>>> + * PSCI 0.2 or later calls. It will return false if the function >>>>> ID is >>>>> + * not handled. >>>>> + */ >>>>> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) >>>>> +{ >>>>> + /* >>>>> + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when >>>>> + * adding/removing a function >>>>> + */ >>>> Should we also update revision of SSSC interface as well? SMCCC >>>> requires this. >>> >>> Meh, you can't rely on the SSSC revision most of the time as the >>> version for PSCI is done through PSCI_get_version. I can add a >>> comment if you want. >>> >> Yes, I agree with you. But section 5.4 of SMCCC 1.0 applies to SSSC as >> well. So you can write something like "ARM SMCCC requires that SSSC >> revision and function call count should be updated every time you add >> or remove a function" > > Usually we update versioning number only once per release. I would > follow the same approach for this one. So I would suggest > It is not stated clearly in SMCCC, but I've got a felling that revision belongs not to product version, but to SMC interface version. So I see no reason to change revision, if there was no changes to interface itself. I'm okay witch changing revision only one time per release, but only if there was changes to PSCI interface. On other hand, person responsible for release need to track if there was any changes in PSCI and act accordingly. This is not very convenient. So, I would prefer to change revision in the same patch (or patch series) which alters the interface. > "VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when adding > removing a function. SSCCC_SMCCC_*_REVISION should be updated once per > Xen release". And who will be responsible for this? -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-06 15:28 ` Volodymyr Babchuk @ 2018-02-06 15:39 ` Julien Grall 2018-02-06 15:50 ` Volodymyr Babchuk 0 siblings, 1 reply; 16+ messages in thread From: Julien Grall @ 2018-02-06 15:39 UTC (permalink / raw) To: Volodymyr Babchuk, xen-devel; +Cc: sstabellini, andre.przywara On 02/06/2018 03:28 PM, Volodymyr Babchuk wrote: > Hi Julien, > > On 06.02.18 16:45, Julien Grall wrote: > > [...] >>>>>> +/* >>>>>> + * PSCI 0.2 or later calls. It will return false if the function >>>>>> ID is >>>>>> + * not handled. >>>>>> + */ >>>>>> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) >>>>>> +{ >>>>>> + /* >>>>>> + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated >>>>>> when >>>>>> + * adding/removing a function >>>>>> + */ >>>>> Should we also update revision of SSSC interface as well? SMCCC >>>>> requires this. >>>> >>>> Meh, you can't rely on the SSSC revision most of the time as the >>>> version for PSCI is done through PSCI_get_version. I can add a >>>> comment if you want. >>>> >>> Yes, I agree with you. But section 5.4 of SMCCC 1.0 applies to SSSC >>> as well. So you can write something like "ARM SMCCC requires that >>> SSSC revision and function call count should be updated every time >>> you add or remove a function" >> >> Usually we update versioning number only once per release. I would >> follow the same approach for this one. So I would suggest >> > > It is not stated clearly in SMCCC, but I've got a felling that revision > belongs not to product version, but to SMC interface version. So I see > no reason to change revision, if there was no changes to interface itself. > I'm okay witch changing revision only one time per release, but only > if there was changes to PSCI interface. On other hand, person > responsible for release need to track if there was any changes in PSCI > and act accordingly. This is not very convenient. > So, I would prefer to change revision in the same patch (or patch > series) which alters the interface. At the end of the day this is closely tie to product revision. A new revision may produce change in the implementation and therefore the version will be bumped. But there are always exception when you do a release just for bug fix. This is the same for Xen, we only change interface version if something major as changed. If nothing, then the version stays the same. > >> "VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when adding >> removing a function. SSCCC_SMCCC_*_REVISION should be updated once per >> Xen release". > > And who will be responsible for this? The first patch modifying the SMCCC implementation after the tree is re-opened. This is how we deal the rest of the versions in Xen, and it works. Cheers, -- Julien Grall _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c 2018-02-06 15:39 ` Julien Grall @ 2018-02-06 15:50 ` Volodymyr Babchuk 0 siblings, 0 replies; 16+ messages in thread From: Volodymyr Babchuk @ 2018-02-06 15:50 UTC (permalink / raw) To: Julien Grall, xen-devel; +Cc: sstabellini, andre.przywara On 06.02.18 17:39, Julien Grall wrote: > > > On 02/06/2018 03:28 PM, Volodymyr Babchuk wrote: >> Hi Julien, >> >> On 06.02.18 16:45, Julien Grall wrote: >> >> [...] >>>>>>> +/* >>>>>>> + * PSCI 0.2 or later calls. It will return false if the function >>>>>>> ID is >>>>>>> + * not handled. >>>>>>> + */ >>>>>>> +bool do_vpsci_0_2_call(struct cpu_user_regs *regs, uint32_t fid) >>>>>>> +{ >>>>>>> + /* >>>>>>> + * /!\ VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated >>>>>>> when >>>>>>> + * adding/removing a function >>>>>>> + */ >>>>>> Should we also update revision of SSSC interface as well? SMCCC >>>>>> requires this. >>>>> >>>>> Meh, you can't rely on the SSSC revision most of the time as the >>>>> version for PSCI is done through PSCI_get_version. I can add a >>>>> comment if you want. >>>>> >>>> Yes, I agree with you. But section 5.4 of SMCCC 1.0 applies to SSSC >>>> as well. So you can write something like "ARM SMCCC requires that >>>> SSSC revision and function call count should be updated every time >>>> you add or remove a function" >>> >>> Usually we update versioning number only once per release. I would >>> follow the same approach for this one. So I would suggest >>> >> >> It is not stated clearly in SMCCC, but I've got a felling that >> revision belongs not to product version, but to SMC interface version. >> So I see no reason to change revision, if there was no changes to >> interface itself. >> I'm okay witch changing revision only one time per release, but only >> if there was changes to PSCI interface. On other hand, person >> responsible for release need to track if there was any changes in PSCI >> and act accordingly. This is not very convenient. >> So, I would prefer to change revision in the same patch (or patch >> series) which alters the interface. > At the end of the day this is closely tie to product revision. A new > revision may produce change in the implementation and therefore the > version will be bumped. But there are always exception when you do a > release just for bug fix. > > This is the same for Xen, we only change interface version if something > major as changed. If nothing, then the version stays the same. > Then I'm okay with this. I just wanted to sure that we are on the same page. >> >>> "VPSCI_NR_FUNCS (in asm-arm/vpsci.h) should be updated when adding >>> removing a function. SSCCC_SMCCC_*_REVISION should be updated once >>> per Xen release". >> >> And who will be responsible for this? > > The first patch modifying the SMCCC implementation after the tree is > re-opened. This is how we deal the rest of the versions in Xen, and it > works. I'm good with this then. -- Volodymyr Babchuk _______________________________________________ Xen-devel mailing list Xen-devel@lists.xenproject.org https://lists.xenproject.org/mailman/listinfo/xen-devel ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2018-02-06 15:50 UTC | newest] Thread overview: 16+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-02-02 11:41 [PATCH v2 0/3] xen/arm: SMCCC fixes and PSCI clean-up Julien Grall 2018-02-02 11:41 ` [PATCH v2 1/3] xen/arm: vpsci: Removing dummy MIGRATE and MIGRATE_INFO_UP_CPU Julien Grall 2018-02-02 13:44 ` Volodymyr Babchuk 2018-02-02 11:41 ` [PATCH v2 2/3] xen/arm: vsmc: Don't implement function ID that doesn't exist Julien Grall 2018-02-02 13:46 ` Volodymyr Babchuk 2018-02-06 14:53 ` Julien Grall 2018-02-06 15:15 ` Volodymyr Babchuk 2018-02-06 15:29 ` Julien Grall 2018-02-02 11:41 ` [PATCH v2 3/3] xen/arm: vpsci: Move PSCI function dispatching from vsmc.c to vpsci.c Julien Grall 2018-02-02 14:23 ` Volodymyr Babchuk 2018-02-02 14:31 ` Julien Grall 2018-02-02 15:40 ` Volodymyr Babchuk 2018-02-06 14:45 ` Julien Grall 2018-02-06 15:28 ` Volodymyr Babchuk 2018-02-06 15:39 ` Julien Grall 2018-02-06 15:50 ` Volodymyr Babchuk
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).