On 12/02/2018 20:12, Mirela Simonovic wrote:
Thank you for the review.
I've done pretty much the same work in parallel, but there are few
additional minor changes I've made. Briefly, the difference is in return
values that some already implemented functions should return starting
from v1.0 (and even v0.2 errata). Please let me know whether you omitted
Could you give a bit more details here? From a brief look we don't seem
to implement correctly:
- CPU_OFF: PSCI_DENY should be return on failure (though it should
never fail in Xen case) and the check on the vCPU state is pointless.
- MIGRATE_INFO_TYPE: should technically return int32_t instead of
uint32_t. That not really matter for now.
If you speak about denying SMC64 call from AArch32, then this is already
done in vsmccc.c (see vsmccc_call).
I can submit these patches if you want. Currently I have few - one for
each fix, easier to review. I guess all of them should be squashed with
the patch you submitted.
One more note - starting from v1.0, PSCI_NOT_SUPPORTED error should be
returned for all optional functions that are not implemented. Is that
the case? I.e. when there is no case for a particular function ID in
do_vpsci_0_2_call the PSCI_NOT_SUPPORTED error will be returned?
This is done by vmsccc_handle_call().
See set_user_regs(regs, 0, ARM_SMCC_ERR_UNKNOWN_FUNCTION) which is
equivalent to PSCI_NOT_SUPPORTED.
+static int32_t do_psci_1_0_features(uint32_t psci_func_id)
+ /* /!\ Ordered by function ID and not name */
+ switch ( psci_func_id )
+ case PSCI_0_2_FN32_PSCI_VERSION:
+ case PSCI_0_2_FN32_CPU_SUSPEND:
+ case PSCI_0_2_FN64_CPU_SUSPEND:
Just a note here - PSCI_FEATURES should return additional information
just for CPU_SUSPEND (supported power state and mode). AFAIU, that value
is also 0, so the return code should be fine.
I think so, from what I understood this is inline with CPU_SUSPEND only
supports 0.2 format.
Xen-devel mailing list