- TF-A - lists.trustedfirmware.org

Re: [TF-A] psci shutdown do not follow graceful power off sequence

by Dan Handley

Hi Sandeep (I accidentally dropped the TF-A list in my last reply - now re-adding). > -----Original Message----- > From: Sandeep Tripathy <sandeep.tripathy(a)broadcom.com> > Sent: 05 December 2019 17:17 > > On Thu, Dec 5, 2019 at 9:54 PM Dan Handley <Dan.Handley(a)arm.com> wrote: > > > > Hi Sandeep > > > > > -----Original Message----- > > > From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of > > > Sandeep Tripathy via TF-A > > > Sent: 05 December 2019 12:00 > > > > > > My query is more on the spec. > > > The OS (eg: linux) and atf and psci spec seem to have assumed that > > > it is managing an independent system or managing 'all' the masters > > > in a coherent domain. > > > What other > > > reason could possibly encourage to not to follow a shutdown sequence. > > > > > Do you mean "to not follow a *graceful* shutdown sequence"? > Yes, exactly. Thanks! > > If so I can think of 3 reasons: > > 1. It's much slower than a non-graceful shutdown. > But this is certainly not a concern for smaller embedded systems. True, but TF-A tries to be a reference for all systems. > > 2. There is no observable difference between a graceful and non-graceful > > shutdown from the calling OS's point of view. The OS presumably has no > > knowledge of other masters it does not manage. > > Can CCN state machine go bad because one participating entity just goes off > without marking its exit ? > Please note I have not seen the issue and it is my assumption. > It depends on the interconnect. Arm interconnects designed for pre-v8.2 systems required explicit programming to take the master our of the coherency domain. Arm interconnects for v8.2+ systems do this automatically via hardware system coherency signals. The TF-A off/reset platform interfaces have provision to do this programming if necessary, but only for the running cluster, which is another reason not to use these PSCI functions in this scenario. > > 3. It's hard for firmware to implement in the multicore situation. > Agree. It is complex to initiate and ensure 'other cores' power down in > firmware. > > > > I haven't yet seen a reason why SYSTEM_SUSPEND won't work instead. > > > I think you are suggesting to use psci system suspend hook in reboot /power > off path Or use system suspend from the OS itself ? Should work. > I'm suggesting to just do a normal SYSTEM_SUSPEND (suspend to RAM) from the OS. > @Sudeep, I agree alternate approaches to solve data loss problem works and > may be those are the best suited. > The past thread[1] is somewhat related but diverged in multiple directions. > I wanted to know and focus the above 3 points especially point 2. Regards 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.

5 years, 4 months

3
14
0 0

RPi4 and CPU hotplugging

by Jan Kiszka

Hi Andre, I'm trying to get CPU offlining work for Linux on the RPi4. In fact, this is working already with current ATF master and the Raspberry kernel 4.19.85. Not it would be "nice" if onlining worked as well: [ 94.959294] CPU1: shutdown [ 94.959311] psci: CPU1 killed. [ 106.750960] CPU1: failed to come online [ 106.755425] CPU1: failed in unknown state : 0x0 Is this a known limitation? Or am I using the wrong kernel (obviously, it's a downstream one ;) )? Thanks, Jan PS: Current RPi firmware requires an explicit "armstub=armstub8.bin" in config.txt, no automatic pickup.

5 years, 4 months

3
6
0 0

Re: [TF-A] [RFC] New feature in TF-A to load encrypted FIP payloads

by Sandrine Bailleux

Hi Sumit, Thanks for your answer. On 12/2/19 2:22 PM, Sumit Garg via TF-A wrote: > First of all, the authenticated encryption framework for FIP payloads > allows for algorithm parameter to be passed in the header (see struct > fip_toc_dec_data here [1]). So it should be easy to add support for > CCM algorithm too. > > Now coming on to additional reason to choose AES-GCM only (apart from > reasons that you have already mentioned) being: > - Currently mbedTLS only exposes partial decryption APIs for GCM > (mbedtls_gcm_starts(), mbedtls_gcm_update() and mbedtls_gcm_finish()) > but not CCM. And we need to rely on stack based partial decryption > approach (see gcm_decrypt() here [2]) as I think we can't afford large > buffers for both encrypted and plain firmware image. > > [1] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2495/2/inclu… > [2] https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2494/2/drive… I see, that makes sense to me. >> Also, I am still trying to get my head around how this would integrate >> with a cryptographic engine where the key does not leave the chip. I can >> imagine that we could get the address of the encrypted firmware image >> from the FIP, pass that to a cryptographic engine, request it to decrypt >> it and store the result somewhere in Trusted RAM. In this case, we >> wouldn't call plat_get_fip_encryption_key(). Do you have any idea how we >> would pull this off? Like how the different modules (IO layer, crypto >> module, image parser module, ...) would integrate together? > > In this case, I would expect platform to provide key identifier rather > than actual key as part of plat_get_fip_encryption_key() which is then > passed onto auth_decrypt() that is implementation specific to > cryptographic engine in similar terms as currently done for mbedTLS > backend. Ah I see, so plat_get_fip_encryption_key() could return either the key itself or a key identifier. Just like plat_get_rotpk_info() can return either the key or a hash of it today. However, in the case of plat_get_rotpk_info(), it also returns some flags indicating which one it is. Don't we need something similar for plat_get_fip_encryption_key()? How will the crypto module be able to tell the difference between a key and a key ID otherwise? Or would you expect a given crypto module backend to always use either keys or key IDs, but not both? >> I have some concerns around the generation of the initialization vectors >> in the encrypt_fw tool. Right now, IVs are simply a random sequence of >> bytes (obtained through a call to OpenSSL's RAND_bytes() API). Now, I >> would imagine that RAND_bytes() is typically based on a good random >> number generator and thus will generate different sequences every time >> it is called. At least, as long as it is called from the same machine >> every time. But what if we encrypt a new FIP bundle from a different >> machine, say in the context of a firmware update? Is it not possible >> that it might choose the same IV out of bad luck? >> >> Perhaps that's an issue left to provisioning/manufacturing time and is >> out of the scope here. But it worries me because AFAIU, the security of >> AES-GCM is critically undermined if the same nonce is used multiple >> times with the same key (see section 5.1.1. "Nonce reuse" in RFC 5116). >> If the encryption key is the SSK (rather than the BSSK) then I guess the >> probability is even higher, as it is shared amongst a class of devices. >> > > Agree that "nonce" should be unique and using a random number > generator available on build machine was an effort towards that. But > thinking about the case that you have mentioned, I think we could have > an optional user provided "nonce" as an input to "encrypt_fw" tool, so > that the user is aware to randomly generate and provide a unique > nonce. Yes, I like your suggestion of specifying the nonce to the tool. But I think it should be the default behaviour then. You mention an *optional* user-provided nonce, I would like to suggest we make this mandatory instead. We could provide an option to request the tool to generate the nonce, intended for development purposes. >> Impact on memory footprint and performance >> ------------------------------------------ >> >> Do you know what the performance impact is when this feature is enabled >> in TF-A, to decrypt images at boot time? Obviously it depends on the >> platform and whether there is a dedicated cryptographic engine, and I >> suppose you cannot really get any relevant measurements out of QEMU but >> I would be interested if you've got any rough numbers. > > Following are measurements based on qemu for mbedTLS software library > based authenticated decryption: > > BL31 plain: > NOTICE: Load image time: 137us, size: 28KB > BL31 encrypted: > NOTICE: Load image time: 3979us, size: 28KB > > BL32 plain: > NOTICE: Load image time: 1791us, size: 360KB > BL32 encrypted: > NOTICE: Load image time: 36339us, size: 360KB Thanks. So it's a 29% increase for BL31 and 20% for BL32. I would have naively expected similar percentages, any idea why we get such a difference between BL31 and BL32? I am just curious. Maybe it's down to how the crypto algorithm/mode of operation works under the hood? > Following is the patch I used to take measurements in case someone is > interested to try it out on actual platform: > > diff --git a/common/bl_common.c b/common/bl_common.c > index e6f9802..d7303d2 100644 > --- a/common/bl_common.c > +++ b/common/bl_common.c > @@ -148,6 +148,7 @@ static int load_auth_image_internal(unsigned int image_id, > int is_parent_image) > { > int rc; > + unsigned long int ticks; > > #if TRUSTED_BOARD_BOOT > if (dyn_is_auth_disabled() == 0) { > @@ -164,11 +165,16 @@ static int load_auth_image_internal(unsigned int image_id, > } > #endif /* TRUSTED_BOARD_BOOT */ > > + ticks = read_cntpct_el0(); > /* Load the image */ > rc = load_image(image_id, image_data); > if (rc != 0) { > return rc; > } > +#define SYS_COUNTER_FREQ_IN_TICKS ((1000 * 1000 * 1000) / 16) > + NOTICE("Load image time: %ldus, size: %dKB\n", > + (read_cntpct_el0() - ticks) * 1000000 / > SYS_COUNTER_FREQ_IN_TICKS, > + image_data->image_size / 1024); > > #if TRUSTED_BOARD_BOOT > if (dyn_is_auth_disabled() == 0) { > >> >> And what's the memory footprint impact? IIUC, AES-GCM almost does not >> inflate the size of the data it encrypts. The size of the ciphertext >> seems to be the same as the plaintext + the size of the authentication >> tag. So I guess there's no real impact on flash storage and Trusted RAM >> usage to hold decrypted firmware. But what about the mbedTLS primitives >> to decrypt the images? How much code and data does this add? > > Following is my analysis on code and data increase due to mbedTLS primitives: > > Binary size: > ========= > > $ ls -lh build/qemu/release/*.bin > -rwxrwxr-x 1 sumit sumit 58K Dec 2 12:53 build/qemu/release/bl1.bin > -rwxrwxr-x 1 sumit sumit 66K Dec 2 12:53 build/qemu/release/bl2.bin > -rwxrwxr-x 1 sumit sumit 29K Dec 2 12:53 build/qemu/release/bl31.bin > -rw-rw-r-- 1 sumit sumit 2.5M Dec 2 12:53 build/qemu/release/fip.bin > -rw-rw-r-- 1 sumit sumit 32 Dec 2 12:53 build/qemu/release/rotpk_sha256.bin > > After importing mbedTLS primitives to support AES-GCM algo: > > $ ls -lh build/qemu/release/*.bin > -rwxrwxr-x 1 sumit sumit 67K Dec 2 12:56 build/qemu/release/bl1.bin > -rwxrwxr-x 1 sumit sumit 78K Dec 2 12:56 build/qemu/release/bl2.bin > -rwxrwxr-x 1 sumit sumit 29K Dec 2 12:56 build/qemu/release/bl31.bin > -rw-rw-r-- 1 sumit sumit 2.5M Dec 2 12:56 build/qemu/release/fip.bin > -rw-rw-r-- 1 sumit sumit 32 Dec 2 12:53 build/qemu/release/rotpk_sha256.bin > > Stack and heap: Works fine with default allocations on qemu. > ============ OK thanks. >> encrypt_fw tool >> --------------- >> >> We have some floating ideas around re-implementing the tools (fiptool, >> certtool) in a scripting language (possibly python) in the future and >> also doing a better job at sharing a common description of the list of >> images to boot/authenticate between the firmware and the host tools. But >> we're not there yet, so I agree that implementing this new tool in C >> from the same "mold" as fiptool and certtool is what makes the most >> sense today. It's just another tool we will have to rework if and when >> we get there. > > Sounds like a good idea to have these tools being python based. BTW, we noticed some copyright headers attributed to both Arm and Linaro and pre-dating year 2019, e.g. in https://review.trustedfirmware.org/c/TF-A/trusted-firmware-a/+/2496/3/tools… : * Copyright (c) 2015, ARM Limited and Contributors. All rights reserved. * Copyright (c) 2019, Linaro Limited I am guessing this is because this tool was derived from the existing cert_create tool code, is that right? >> I did not understand why this new tool needs to know what image it is >> encrypting. For example, one possible invocation could be: >> >> tools/encrypt_fw/encrypt_fw \ >> -k 1234567890abcdef1234567890abcdef1234567890abcdef1234567890abcdef \ >> --soc-fw bl31.bin \ >> --soc-fw-enc bl31_enc.bin \ >> --tos-fw bl32.bin \ >> --tos-fw-enc bl32_enc.bin >> >> Why not invoking the tool once per image instead? As in: >> >> encrypt_fw -k key -in ifile -out ofile >> >> for BL31, then for BL32? Does the tool do anything different based on >> the type of image it receives? > > "encrypt_fw" tool doesn't infer anything based on image type but image > types were added for more user visibility and ease of use as follows: > - Provides the capability to encrypt multiple firmwares on single invocation. I am not really convinced that this is a useful feature. I would rather have an external script (or the build system) invoking the tool multiple times, once per firmware image. Putting that complexity in the tool itself seems unnecessary to me. Also, it makes the tool TBBR specific, as it has to know the list of images it's allowed to encrypt. Unfortunately, we already have this TBBR knowledge embedded into the fiptool/cert_create tool today but we would like to change that in the future. It does not scale well with new images or alternative chains of trust. > - Restricts usage of tool for FIP payloads only. Why would we want that? > - Better align with Makefile framework to build command line args > while building different images and finally invoke tool at once before > creating FIP payload. Could we not invoke the tool for each image as we go along? As in, we build BL31 and just after we generate the encrypted version of it. Then we build BL32 and its encrypted version. And so on. While we do that, we build the fiptool command line that will indeed put all the encrypted images in the final FIP image. How does that sound? Regards, Sandrine

5 years, 4 months

2
1
0 0

Re: [TF-A] psci shutdown do not follow graceful power off sequence

by Sudeep Holla

On Thu, Dec 05, 2019 at 10:47:14PM +0530, Sandeep Tripathy wrote: > On Thu, Dec 5, 2019 at 9:54 PM Dan Handley <Dan.Handley(a)arm.com> wrote: > > > > Hi Sandeep > > > > > -----Original Message----- > > > From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of Sandeep > > > Tripathy via TF-A > > > Sent: 05 December 2019 12:00 > > > > > > My query is more on the spec. > > > The OS (eg: linux) and atf and psci spec seem to have assumed that it is > > > managing an independent system or managing 'all' the masters in a coherent > > > domain. > > > What other > > > reason could possibly encourage to not to follow a shutdown sequence. > > > > > Do you mean "to not follow a *graceful* shutdown sequence"? > Yes, exactly. Thanks! > If so I can think of 3 reasons: > > 1. It's much slower than a non-graceful shutdown. > But this is certainly not a concern for smaller embedded systems. But we are talking about generic solution here, aren't we ? If so, it remains concern. [..] > I think you are suggesting to use psci system suspend hook in reboot > /power off path Or use system suspend from the OS itself ? Should work. > Just suspend, don't try to use PSCI SYSTEM_SUSPEND in reboot/poweroff path, not at-least in the generic code. If you think it works fine to address your issue, you can use it in your custom solution :) > @Sudeep, I agree alternate approaches to solve data loss problem works > and may be those are the best suited. No, the data loss issue had more open questions and I haven't understood the solution you have there. > The past thread[1] is somewhat related but diverged in multiple directions. OK, details again ? -- Regards, Sudeep

5 years, 4 months

1
0
0 0

Re: [TF-A] SMC to intentionally trigger a panic in TF-A

by Sandrine Bailleux

Hi Julius, As you were mentioning that the Linux kernel uses /proc/sysrq-trigger for a similar purpose, I was wondering whether you'd be open to a solution based on a "DebugFS" entry. As you may have seen on the mailing list, Olivier posted a proposal for introducing a firmware debug interface, which has many similarities to how /proc or /sys works in the kernel world: https://lists.trustedfirmware.org/pipermail/tf-a/2019-October/000120.html TF-A patches for this feature are up for review right now and Olivier has also posted some TF-A Tests patches that demonstrate how this can be used from normal world. In addition, we are also working on a Linux driver for this. As you can imagine, DebugFS uses an SMC interface under the hood (currently allocated in the SiP range). But being an abstraction over the SMC layer, which specific SMC function ID is used does not matter so much and it does not need to be standardized by any Arm specification. You'd need to mandate all Chrome OS devices to have this DebugFS entry in the firmware but the backend could vary from platform to platform. Would that suit your use case? Regards, Sandrine 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.

5 years, 4 months

2
1
0 0

Build config tool

by Balint Dobszay

Hi All, In TF-A we have a large set of build options (over 90). Dependency and incompatibility checks for these are currently done in the Makefile system. This can be hard to follow and difficult to maintain. We are investigating if there is a better way to describe the build options in a structured way, handle the dependencies/relations between them, and provide a more user-friendly configuration interface. A possible good solution for this is Kconfig, more specifically a Python implementation called Kconfiglib [1]. We plan to do more prototyping work in this area. Any feedback or comments on this topic are welcome. Regards, Balint [1] https://github.com/ulfalizer/Kconfiglib

5 years, 4 months

1
0
0 0

Re: [TF-A] psci shutdown do not follow graceful power off sequence

by Sudeep Holla

On Tue, Dec 3, 2019 at 4:58 PM Sandeep Tripathy via TF-A <tf-a(a)lists.trustedfirmware.org> wrote: > > Hi, > system shutdown or system reset PSCI calls are invoked by the last > core in the system. PSCI lib does not do cache flush for the last core > /cluster and does not follow the core / cluster power down sequence. > This may cause issue in a system if the system is not standalone one > ie if the system is a slave or node in a bigger system with other > coherent masters/PEs. > I am not sure if system off/reset is the right API to call in such a setup. > Please suggest if the PSCI spec expected 'shutdown/reset/reset2' to > deliberately skip the core/cluster shutdown sequence. > Yes and IIRC this has been discussed in the past[1]. I expect to get some closure on open question on that thread. Quite a few questions were raised by few people and I am not sure if all were answered. I need to dig but but AFAIK it wasn't all answered. -- Regards, Sudeep [1] https://lkml.org/lkml/2019/1/18/16

5 years, 4 months

2
2
0 0

About disable gic cpuif in psci suspend or off

by Jisheng Zhang

Hi, In ATF-A, I usually see below code in psci suspend or off code path: /* Prevent interrupts from spuriously waking up this cpu */ plat_arm_gic_cpuif_disable(); But per my understanding, before calling psci_suspend(), the NW, e.g linux kernel has disabled all interrupts from cpu level, so here preventing interrupt is to prevent the interrupts from secure world? Another question is: for Cortex A55, this is not necessary. Because CA55 TRM says when the core_pwrdn_en bit is set, executing WFI automatically masks all interrupts and wake-up events in the core. Am I right? Thanks in advance

5 years, 4 months

1
0
0 0

Re: [TF-A] Insufficient ordering guarantees in bakery_lock_get() and bakery_lock_release() in bakery_lock_coherent.c

by Raghupathy Krishnamurthy

Hi Soby, Thanks for your response. >>it is needed to ensure the ordering of the succeeding sev(). Agree. Thanks for the clarification. >>Was this an issue that actually manifested on a hardware or is this something that you caught while reviewing the code? Noticed it while reviewing code and I have not observed it on hardware. Thanks -Raghu On November 26, 2019 at 8:55 AM, Soby Mathew <Soby.Mathew(a)arm.com> wrote: On 26/11/2019 16:30, Raghupathy Krishnamurthy via TF-A wrote: Hello! Reposting this from (https://developer.trustedfirmware.org/T589). bakery_lock_get() uses a dmbld() after lock acquisition which is insufficient in a lock acquire situation. With just dmbld(), stores in a critical section can be reordered before the dmbld() and indeed before the lock acquisition has taken place. similarly, bakery_lock_release() only uses dmbst(). A load in the critical section could be reordered after the dmbst() and write to the lock data structure releasing the lock. This is likely less of a problem but lock release needs to provide release semantics, and dmbst() is insufficient. For ex: A load in the critical section of CPU1 can be reordered after the store for the lock release, and it could read from a store that is executed on CPU2 in the same critical section, since CPU2 saw the store for the lock release first, and raced into the critical section. Hi Raghu, You are right on this. The dmbld() and dmbst() does not provide sufficient guarantees in the cases you mention. Was this an issue that actually manifested on a hardware or is this something that you caught while reviewing the code ? Also the dsb() after the write to the lock seems unnecessary. Am I missing something here ? It looks like the same issue is present even in bakery_lock_normal. If you are referring to the dsb() at this line : https://git.trustedfirmware.org/TF-A/trusted-firmware-a.git/tree/lib/locks/… it is needed to ensure the ordering of the succeeding sev(). Best Regards Soby Mathew Thanks Raghu

5 years, 4 months

2
2
0 0

Re: [TF-A] psci shutdown do not follow graceful power off sequence

by Dan Handley

Hi Sandeep > -----Original Message----- > From: TF-A <tf-a-bounces(a)lists.trustedfirmware.org> On Behalf Of Sandeep > Tripathy via TF-A > Sent: 03 December 2019 16:59 > > Hi, > system shutdown or system reset PSCI calls are invoked by the last core in > the system. That may be the case in practice but these functions can be called by any core in the system at any time, not just the last core. > PSCI lib does not do cache flush for the last core /cluster and > does not follow the core / cluster power down sequence. Correct. Not only that, it doesn't do anything with the other cores in the system that may be running at that time. > This may cause issue in a system if the system is not standalone one ie if > the system is a slave or node in a bigger system with other coherent > masters/PEs. > What issue specifically? Are the other coherent masters expecting to have the same view of the node's memory that the node had prior to calling system off/suspend? If so, then the node is not really independent to the wider system and system off/reset are probably not the right calls to use. Perhaps SYSTEM_SUSPEND is what you're looking for? That function requires all other cores to be off before the call and will perform cache maintenance on the last core. > Please suggest if the PSCI spec expected 'shutdown/reset/reset2' to > deliberately skip the core/cluster shutdown sequence. > Yes, the spec was deliberately relaxed between version 0.2 and 1.0 to allow implementations to just "pull the plug" since there would be no observable difference to the normal world caller between this behaviour and attempting to shutdown cleanly, which would be difficult and inherently racy anyway. > Otherwise in case it makes sense following is a patch to take care of this. > https://review.trustedfirmware.org/#/c/TF-A/trusted-firmware-a/+/2364/ > issue: https://developer.trustedfirmware.org/T566 Thanks but I don't think we want this. Regards 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.

5 years, 4 months

1
0
0 0

2025

2024

2023

2022

2021

2020

2019

2018

TF-A