Hi David,
As this thread is specifically for weak symbols, it would be helpful to provide some more reasoning behind the new guidance. Could be a good opportunity to discuss further as well.
Thanks, Abhishek
-----Original Message----- From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of David Hu (Arm Technology China) via TF-M Sent: 01 November 2019 10:24 To: Christopher Brand chris.brand@cypress.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: Re: [TF-M] Weak symbols
Hi Chris,
Sorry for the inconvenience. The discussion to figure out the new guidance did cost some time.
The initial conclusion is that the weak symbol/function should be avoided in new code added to TF-M. Platform support code may use weak symbols as an exception to make platform specific implementation flexible. The code guidance will be updated soon to address this issue formally.
Best regards, Hu Ziji
-----Original Message----- From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of Christopher Brand via TF-M Sent: Wednesday, October 30, 2019 7:50 PM To: tf-m@lists.trustedfirmware.org Subject: [TF-M] Weak symbols
Hi,
There are a couple of code reviews that I've been involved with that seem to be being held up due to the use of weak symbols. https://review.trustedfirmware.org/c/trusted-firmware-m/+/1973 https://review.trustedfirmware.org/c/trusted-firmware-m/+/2212 There seems to be a feeling among some reviewers that weak symbols are discouraged in TF-M, due to them not being in the C standard.
There's no mention of weak symbols in docs/coding_guide.rst. There is a brief mention in docs/user_guides/tfm_secure_irq_handling.rst: In Library model a function with the name derived from the value of the ``line_num`` or ``line_name`` property is generated. This function will be put in the vector table by the linker (as the handlers in the startup assembly are defined as weak symbols). The code generated for this function will forward the call to the function with the name of the value of the ``signal`` property post-fixed with ``_isr``. So that document explicitly states that weak symbols will be used in TF-M.
TFM already uses weak symbols fairly extensively. A quick grep shows two instances under "interface": interface/src/tfm_ns_interface.c: __attribute__((weak)) int32_t tfm_ns_interface_dispatch(veneer_fn fn, [...] __attribute__((weak)) enum tfm_status_e tfm_ns_interface_init(void) and several hundred under "platform": $ grep -ri weak platform/ --exclude=tfm_mbedcrypto_config.h --exclude=cmsis_*.h | wc 747 5902 109938
Looking at Phabricator, the only mention of weak symbols I can find is in https://developer.trustedfirmware.org/T363 and https://developer.trustedfirmware.org/T198, both of which explicitly state that new weak functions will be added, and https://developer.trustedfirmware.org/T463, which doesn't take a position either way.
Weak symbols have been discussed a little on the mailing list in the thread at https://lists.trustedfirmware.org/pipermail/tf-m/2019-October/000430.html but not very extensively.
My feeling is that it wouldn't be unreasonable to decide that weak symbols should be discouraged, and it might even make sense for somebody to take on the task of removing them from TF-M, although that looks like a significant amount of work. Of course it would be nice to see that documented, presumably in docs/coding_guide.rst (and presumably corresponding changes to docs/user_guides/tfm_secure_irq_handling.rst and tasks T198 and T463). As things stand right now, they do seem to be an accepted mechanism within TF-M, and worrying about adding a handful more to the nearly 750 we have now seems unreasonable - if we do decide at some point not to use weak symbols, the work involved in removing 760 or even 800 Is not significantly more than would be needed now.
The code reviews I cited have been out there for 6 weeks now. It would be really nice to find a way forward...
Thanks,
Chris
This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message. -- TF-M mailing list TF-M@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-m
Hi,
In my own words, :) it is one of the steps to be compliant with some coding guidelines with more strict requirements. We are updating the doc to specify the details.
Best regards, Hi Ziji ________________________________ From: Abhishek Pandit Abhishek.Pandit@arm.com Sent: Friday, November 1, 2019 6:59:32 PM To: David Hu (Arm Technology China) David.Hu@arm.com; Christopher Brand chris.brand@cypress.com; tf-m@lists.trustedfirmware.org tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: RE: Weak symbols
Hi David,
As this thread is specifically for weak symbols, it would be helpful to provide some more reasoning behind the new guidance. Could be a good opportunity to discuss further as well.
Thanks, Abhishek
-----Original Message----- From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of David Hu (Arm Technology China) via TF-M Sent: 01 November 2019 10:24 To: Christopher Brand chris.brand@cypress.com; tf-m@lists.trustedfirmware.org Cc: nd nd@arm.com Subject: Re: [TF-M] Weak symbols
Hi Chris,
Sorry for the inconvenience. The discussion to figure out the new guidance did cost some time.
The initial conclusion is that the weak symbol/function should be avoided in new code added to TF-M. Platform support code may use weak symbols as an exception to make platform specific implementation flexible. The code guidance will be updated soon to address this issue formally.
Best regards, Hu Ziji
-----Original Message----- From: TF-M tf-m-bounces@lists.trustedfirmware.org On Behalf Of Christopher Brand via TF-M Sent: Wednesday, October 30, 2019 7:50 PM To: tf-m@lists.trustedfirmware.org Subject: [TF-M] Weak symbols
Hi,
There are a couple of code reviews that I've been involved with that seem to be being held up due to the use of weak symbols. https://review.trustedfirmware.org/c/trusted-firmware-m/+/1973 https://review.trustedfirmware.org/c/trusted-firmware-m/+/2212 There seems to be a feeling among some reviewers that weak symbols are discouraged in TF-M, due to them not being in the C standard.
There's no mention of weak symbols in docs/coding_guide.rst. There is a brief mention in docs/user_guides/tfm_secure_irq_handling.rst: In Library model a function with the name derived from the value of the ``line_num`` or ``line_name`` property is generated. This function will be put in the vector table by the linker (as the handlers in the startup assembly are defined as weak symbols). The code generated for this function will forward the call to the function with the name of the value of the ``signal`` property post-fixed with ``_isr``. So that document explicitly states that weak symbols will be used in TF-M.
TFM already uses weak symbols fairly extensively. A quick grep shows two instances under "interface": interface/src/tfm_ns_interface.c: __attribute__((weak)) int32_t tfm_ns_interface_dispatch(veneer_fn fn, [...] __attribute__((weak)) enum tfm_status_e tfm_ns_interface_init(void) and several hundred under "platform": $ grep -ri weak platform/ --exclude=tfm_mbedcrypto_config.h --exclude=cmsis_*.h | wc 747 5902 109938
Looking at Phabricator, the only mention of weak symbols I can find is in https://developer.trustedfirmware.org/T363 and https://developer.trustedfirmware.org/T198, both of which explicitly state that new weak functions will be added, and https://developer.trustedfirmware.org/T463, which doesn't take a position either way.
Weak symbols have been discussed a little on the mailing list in the thread at https://lists.trustedfirmware.org/pipermail/tf-m/2019-October/000430.html but not very extensively.
My feeling is that it wouldn't be unreasonable to decide that weak symbols should be discouraged, and it might even make sense for somebody to take on the task of removing them from TF-M, although that looks like a significant amount of work. Of course it would be nice to see that documented, presumably in docs/coding_guide.rst (and presumably corresponding changes to docs/user_guides/tfm_secure_irq_handling.rst and tasks T198 and T463). As things stand right now, they do seem to be an accepted mechanism within TF-M, and worrying about adding a handful more to the nearly 750 we have now seems unreasonable - if we do decide at some point not to use weak symbols, the work involved in removing 760 or even 800 Is not significantly more than would be needed now.
The code reviews I cited have been out there for 6 weeks now. It would be really nice to find a way forward...
Thanks,
Chris
This message and any attachments may contain confidential information from Cypress or its subsidiaries. If it has been received in error, please advise the sender and immediately delete this message. -- TF-M mailing list TF-M@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-m -- TF-M mailing list TF-M@lists.trustedfirmware.org https://lists.trustedfirmware.org/mailman/listinfo/tf-m
tf-m@lists.trustedfirmware.org
-
Abhishek Pandit -
David Hu (Arm Technology China)