Hi Sandeep, Thanks for the clarification. I too think this has general utility and since you need access to internal PSCI data to perform the functionality , exporting a utility function from PSCI for platforms to invoke seems the right thing to do. The small concern I have is using the `plat_*` namespace as this is a utility function invoked by the platform and another porting layer beneath it seems out of place.
I would suggest to add a parameter to stop_other_cores(), this could be either the IPI number or a callback function to trigger the IPI. This allows to provide the platform specific details without `plat_*` API.
Best Regards Soby Mathew
-----Original Message----- From: Sandeep Tripathy sandeep.tripathy@broadcom.com Sent: 21 August 2020 07:17 To: Soby Mathew Soby.Mathew@arm.com Cc: tf-a@lists.trustedfirmware.org Subject: RE: [TF-A] [RFC] Api to power down all cores
Hi Soby, I realize using term 'PSCI API' in rfc tag is misleading like Achin mentioned in gerrit.
Here I wanted to have a generic API to 'stop_other_cores' in secure world. The usage is platform firmware specific. Implementation of 'stop_other_cores' depends on a generic 'IPI' support (1). It leverages the existing EHF. So I feel it is not adding and complexity or overhead in normal execution path.
'stop_other_cores' API implementation depends on some psci private functions to traverse the pd nodes and extract MPIDRs for target pe list. That was the reason to put the function within psci lib. So there are two things. 1- Does this idea of 'stop_other_core' api qualify to be generic 2- Does a generic IPI layer make sense
Thanks Sandeep
-----Original Message----- From: Soby Mathew [mailto:Soby.Mathew@arm.com] Sent: Thursday, August 20, 2020 8:54 PM To: Sandeep Tripathy Cc: tf-a@lists.trustedfirmware.org Subject: RE: [TF-A] [RFC] psci: api to power down all cores
Hi Sandeep, Just to understand better, if there is a secure side panic/watchdog interrupt, then the secure side is already able to do such an intervention without the availability of a PSCI API to the NS side.
In case the NS world has crashed, then PSCI_SYSTEM_RESET and PSCI_SYSTEM_OFF APIs can be invoked which then does the appropriate actions. From my reading, the PSCI specification doesn't prevent firmware implementation of the reset and off API's from doing the kind of implementation as per your proposal.
I intend to do 'stop_other_cores' in platform extension of 'plat_system_resetx()', secure side watchdog expiry/ plat_panic_handler().
Best Regards Soby Mathew
-----Original Message----- From: TF-A tf-a-bounces@lists.trustedfirmware.org On Behalf Of
Sandeep
Tripathy via TF-A Sent: 19 August 2020 16:42 To: tf-a@lists.trustedfirmware.org Subject: [TF-A] [RFC] psci: api to power down all cores
Hi, I am proposing to have a generic api in psci lib which can be used to
force
power down all other cores from any initiating core analogous to 'smp_cpu_stop' in linux. It is immune to interrupt lock by EL1/EL2 software.
Platforms may use this api in case of secure side panic, secure watchdog interrupt handling or if required in certain types of warm resets. The usage
is
platform dependent.
This depends on a generic implementation of secure IPI (1) which uses EHF
to
handle IPI at platform defined priority. We probably require more types of secure IPIs.
Please review the series Ref: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/5324
diff --git a/lib/psci/psci_system_off.c b/lib/psci/psci_system_off.c index 141d69e..011aaa6 100644 --- a/lib/psci/psci_system_off.c +++ b/lib/psci/psci_system_off.c @@ -10,10 +10,44 @@ #include <arch_helpers.h> #include <common/debug.h> #include <drivers/console.h> +#include <drivers/delay_timer.h> #include <plat/common/platform.h>
#include "psci_private.h"
+#ifndef PLAT_CORES_PWRDWN_WAIT_TIMEOUT_MS #define +PLAT_CORES_PWRDWN_WAIT_TIMEOUT_MS 1000 #endif
+#if IMAGE_BL31 +void psci_stop_other_cores(void) +{ +#define PLAT_CORES_PWRDWN_WAIT_TIMEOUT_MS 1000 #endif
+#if IMAGE_BL31 +void psci_stop_other_cores(void) +{
int idx, this_cpu_idx, cnt;this_cpu_idx = plat_my_core_pos();/* Raise G0 IPI cpustop to all cores but self */for (idx = 0; idx < psci_plat_core_count; idx++) {if ((idx != this_cpu_idx) &&(psci_get_aff_info_state_by_idx(idx) ==AFF_STATE_ON)) {
plat_ipi_send_cpu_stop(psci_cpu_pd_nodes[idx].mpidr);
}}/* Wait for others cores to shutdown */for (cnt = 0; cnt < PLAT_CORES_PWRDWN_WAIT_TIMEOUT_MS;- cnt++)
{
if (psci_is_last_on_cpu())break;mdelay(1);}if (!psci_is_last_on_cpu()) {WARN("Failed to stop all cores!\n");psci_print_power_domain_map();}+} +#endif
(1) RFC: ipi: add ipi feature Ref: https://review.trustedfirmware.org/c/TF-A/trusted-firmware-
a/+/5323/1
Thanks Sandeep -- TF-A mailing list TF-A@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-a