Hi
> -----Original Message-----
> From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of Soby Mathew
> via TF-A
> Sent: 06 January 2020 12:34
> To: Olivier Deprez <Olivier.Deprez(a)arm.com>; tf-a(a)lists.trustedfirmware.org;
> Sheetal Tigadoli <sheetal.tigadoli(a)broadcom.com>
> Cc: nd <nd(a)arm.com>
> Subject: Re: [TF-A] [Question] Why are the log related macros defined in
> multiple of 10's
>
> > Hi All,
> >
> > I wanted to understand the reason behind defining LOG_LEVEL_* macros
> > as multiple of 10's, also same goes with LOG_MARKER_* macros.
> > If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
> > division operation when printing the msgs to console.
> >
> > Please let me your thoughts on this.
> >
> > Link to this question on Phabricator -
> > https://developer.trustedfirmware.org/T624
> >
> > Thanks
> > Sheetal
>
As Sandrine said in Phabricator, the reason was simply to leave space between the levels to allow adding intermediate levels without breaking compatibility. Compatibility was a possible concern if external tools used the log level and you only rebuilt some of the TF-A components (components could end up using different values). However, I suspect the actual level value is never emitted in practice (just the text prefix) so this may not be a real concern.
> Hi Sheetal
> I didn't quite get reason for the optimization from multiples of 10 to
> consecutive numbers. The mod operation you refer to are assertions which will
> be compiled out when DEBUG=0 or when ENABLE_ASSSERTIONS=0. Other than that
> there is a single divide by 10 operation within the default implementation of
> plat_log_get_prefix().
>
>
> If the logging utility is considered performance critical, removing the
> single divide by 10 will not result in any gain. Usually the bottleneck for
> logs are UART baudrates and the CPU spends most of time spinning in a wait
> loop for UART to be free. Hence I don't see any need for change for log
> levels macro if the intent is performance optimization.
>
+1. Unless there's an actual performance gain, this sounds like an unnecessary change.
Dan.
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
On 06/01/2020 12:34, Soby Mathew via TF-A wrote:
>> Hi All,
>>
>> I wanted to understand the reason behind defining LOG_LEVEL_* macros as
>> multiple of 10's, also same goes with LOG_MARKER_* macros.
>> If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
>> division operation when printing the msgs to console.
>>
>> Please let me your thoughts on this.
>>
>> Link to this question on Phabricator -
>> https://developer.trustedfirmware.org/T624
>>
>> Thanks
>> Sheetal
>
> Hi Sheetal
> I didn't quite get reason for the optimization from multiples of 10 to
> consecutive numbers. The mod operation you refer to are assertions which
> will be compiled out when DEBUG=0 or when ENABLE_ASSSERTIONS=0. Other
> than that there is a single divide by 10 operation within the default
> implementation of plat_log_get_prefix().
>
>
> If the logging utility is considered performance critical, removing the
> single divide by 10 will not result in any gain. Usually the bottleneck
> for logs are UART baudrates and the CPU spends most of time spinning in
> a wait loop for UART to be free. Hence I don't see any need for change
> for log levels macro if the intent is performance optimization.
>
> Best Regards
> Soby Mathew
>
Hi Sheetal
There is some discussion related to this here :
https://github.com/ARM-software/tf-issues/issues/462 and see associated
issues and PRs for more details.
Best Regards
Soby Mathew
>> --
>> TF-A mailing list
>> TF-A(a)lists.trustedfirmware.org
>> https://lists.trustedfirmware.org/mailman/listinfo/tf-a
>>
>
> Hi All,
>
> I wanted to understand the reason behind defining LOG_LEVEL_* macros as
> multiple of 10's, also same goes with LOG_MARKER_* macros.
> If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
> division operation when printing the msgs to console.
>
> Please let me your thoughts on this.
>
> Link to this question on Phabricator -
> https://developer.trustedfirmware.org/T624
>
> Thanks
> Sheetal
Hi Sheetal
I didn't quite get reason for the optimization from multiples of 10 to
consecutive numbers. The mod operation you refer to are assertions which
will be compiled out when DEBUG=0 or when ENABLE_ASSSERTIONS=0. Other
than that there is a single divide by 10 operation within the default
implementation of plat_log_get_prefix().
If the logging utility is considered performance critical, removing the
single divide by 10 will not result in any gain. Usually the bottleneck
for logs are UART baudrates and the CPU spends most of time spinning in
a wait loop for UART to be free. Hence I don't see any need for change
for log levels macro if the intent is performance optimization.
Best Regards
Soby Mathew
> --
> TF-A mailing list
> TF-A(a)lists.trustedfirmware.org
> https://lists.trustedfirmware.org/mailman/listinfo/tf-a
>
Hi Varun
Could you take a look at the Nvidia driver defect reported by the Coverity scan?
Regards.
Alexei.
________________________________
From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> on behalf of scan-admin--- via TF-A <tf-a(a)lists.trustedfirmware.org>
Sent: 03 January 2020 08:19
To: tf-a(a)lists.trustedfirmware.org <tf-a(a)lists.trustedfirmware.org>
Subject: [TF-A] New Defects reported by Coverity Scan for ARM-software/arm-trusted-firmware
Hi,
Please find the latest report on new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan.
1 new defect(s) introduced to ARM-software/arm-trusted-firmware found with Coverity Scan.
New defect(s) Reported-by: Coverity Scan
Showing 1 of 1 defect(s)
** CID 352832: Uninitialized variables (UNINIT)
/plat/nvidia/tegra/soc/t194/drivers/mce/nvg.c: 197 in nvg_update_ccplex_gsc()
________________________________________________________________________________________________________
*** CID 352832: Uninitialized variables (UNINIT)
/plat/nvidia/tegra/soc/t194/drivers/mce/nvg.c: 197 in nvg_update_ccplex_gsc()
191 ret = EINVAL;
192 } else {
193 nvg_set_request_data((uint64_t)TEGRA_NVG_CHANNEL_UPDATE_CCPLEX_GSC,
194 (uint64_t)gsc_idx);
195 }
196
>>> CID 352832: Uninitialized variables (UNINIT)
>>> Using uninitialized value "ret".
197 return ret;
198 }
199
200 /*
201 * Cache clean operation for all CCPLEX caches.
202 */
________________________________________________________________________________________________________
To view the defects in Coverity Scan visit, https://u2389337.ct.sendgrid.net/wf/click?upn=08onrYu34A-2BWcWUl-2F-2BfV0V0…
--
TF-A mailing list
TF-A(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-a
IMPORTANT NOTICE: The contents of this email and any attachments are confidential and may also be privileged. If you are not the intended recipient, please notify the sender immediately and do not disclose the contents to any other person, use it for any purpose, or store or copy the information in any medium. Thank you.
Hi Sheetal,
Your remark/question makes sense.
The original requirement is described here: https://github.com/ARM-software/tf-issues/issues/232
We may get further update from Dan, but I sense this was to provision for intermediate log levels (as stated by Sandrine on phabricator).
Will you be able to submit a patch for this?
Regards,
Olivier.
________________________________________
From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> on behalf of Sheetal Tigadoli via TF-A <tf-a(a)lists.trustedfirmware.org>
Sent: 20 December 2019 15:48
To: tf-a(a)lists.trustedfirmware.org
Subject: [TF-A] [Question] Why are the log related macros defined in multiple of 10's
Hi All,
I wanted to understand the reason behind defining LOG_LEVEL_* macros as
multiple of 10's, also same goes with LOG_MARKER_* macros.
If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
division operation when printing the msgs to console.
Please let me your thoughts on this.
Link to this question on Phabricator -
https://developer.trustedfirmware.org/T624
Thanks
Sheetal
--
TF-A mailing list
TF-A(a)lists.trustedfirmware.org
https://lists.trustedfirmware.org/mailman/listinfo/tf-a
Hi All,
I wanted to understand the reason behind defining LOG_LEVEL_* macros as
multiple of 10's, also same goes with LOG_MARKER_* macros.
If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
division operation when printing the msgs to console.
Please let me your thoughts on this.
Link to this question on Phabricator -
https://developer.trustedfirmware.org/T624
Thanks
Sheetal
Hi All,
I wanted to understand the reason behind defining LOG_LEVEL_* macros as
multiple of 10's, also same goes with LOG_MARKER_* macros.
If we set the LOG macros to 1,2,3 and so on, we can avoid the mod and
division operation when printing the msgs to console.
Please let me your thoughts on this.
Link to this question on Phabricator -
https://developer.trustedfirmware.org/T624
Thanks
Sheetal
>
> Subject: [PATCH] rpi3/4: Add support for offlining CPUs
> From: Jan Kiszka <jan.kiszka(a)siemens.com>
>
> The hooks were populated but the power down left the CPU in limbo-land.
> What we need to do - until there is a way to actually power off - is to
> turn off the MMU and enter the spinning loop as if we were cold-booted.
> This allows the on-call to pick up the CPU again.
>
> Signed-off-by: Jan Kiszka <jan.kiszka(a)siemens.com>
> ---
> plat/rpi/common/rpi3_pm.c | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/plat/rpi/common/rpi3_pm.c b/plat/rpi/common/rpi3_pm.c
> index 8c2d070c4..2a6bf076b 100644
> --- a/plat/rpi/common/rpi3_pm.c
> +++ b/plat/rpi/common/rpi3_pm.c
> @@ -123,6 +123,15 @@ static void rpi3_pwr_domain_off(const psci_power_state_t *target_state)
> #endif
> }
>
> +void __dead2 plat_secondary_cold_boot_setup(void);
> +
> +static void __dead2
> +rpi3_pwr_domain_pwr_down_wfi(const psci_power_state_t *target_state)
> +{
> + disable_mmu_el3();
> + plat_secondary_cold_boot_setup();
> +}
> +
> /*******************************************************************************
> * Platform handler called when a power domain is about to be turned on. The
> * mpidr determines the CPU to be turned on.
> @@ -224,6 +233,7 @@ static void __dead2 rpi3_system_off(void)
> static const plat_psci_ops_t plat_rpi3_psci_pm_ops = {
> .cpu_standby = rpi3_cpu_standby,
> .pwr_domain_off = rpi3_pwr_domain_off,
> + .pwr_domain_pwr_down_wfi = rpi3_pwr_domain_pwr_down_wfi,
> .pwr_domain_on = rpi3_pwr_domain_on,
> .pwr_domain_on_finish = rpi3_pwr_domain_on_finish,
> .system_off = rpi3_system_off,
> --
> 2.16.4
>
Hi Jan,
I have put in a suggestion for the change. Basically, if you are able to
do a `reset` before spinning in `plat_secondary_cold_boot_setup()`, that
would be ideal. Also, if `plat_secondary_cold_boot_setup()` can trap the
primary CPU as well if it were to be offlined, then I think your changes
should be good.
Best Regards
Soby Mathew