Hi,
I am looking at how console flags are used and setup. In porting guide I see
Function : bl31_plat_runtime_setup() [optional] ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
::
Argument : void Return : void
The purpose of this function is allow the platform to perform any BL31 runtime setup just prior to BL31 exit during cold boot. The default weak implementation of this function will invoke ``console_switch_state()`` to switch console output to consoles marked for use in the ``runtime`` state.
Some platform are calling it but some of them not (like our Xilinx one).
Tegra has in tegra_pwr_domain_power_down_wfi() console_flush(); console_switch_state(0);
which is what none other has. Should console_flush() be called by default all the time when console is switched and also disabled when system goes down? Why console_switch_state(CONSOLE_FLAG_RUNTIME) is not called from bl31_main() when before bl31_plat_runtime_setup() is called we have console_flush() already?
The second part of this how console scope is setup. Implementation is clear and set.
void console_set_scope(console_t *console, unsigned int scope) { assert(console != NULL);
console->flags = (console->flags & ~CONSOLE_FLAG_SCOPE_MASK) | scope; }
The commit cc5859ca19ff ("Multi-console: Deprecate the `finish_console_register` macro") when finish_console_register is called (DCC is exception here) is setting up CONSOLE_FLAG_BOOT and CONSOLE_FLAG_CRASH by default.
And most of platforms is calling console registration with calling console_set_scope() where new flags are recorded BOOT only, BOOT/RUNTIME, RUNTIME only or BOOT/RUNTIME/CRASH.
I would like to understand what should be the right behavior. Why are platforms removing CRASH flag after registration? (I see that a lot of platforms are having private plat_crash_console_init() but pretty much crash console is the same with regular console). Why runtime console is setup directly in bl31_early_platform_setup2 when guidance is saying that it should be done much later?
Also commit 63c52d0071ef ("plat/common/crash_console_helpers.S: Fix MULTI_CONSOLE_API support") removed CONSOLE_FLAG_CRASH from plat_crash_console_init but only from 64bit version. In 32bit version there is still there. It suggest that any C code should be called. Do we really need CONSOLE_FLAG_CRASH?
Thanks, Michal
Hi, Since the discussion is already going on in TF-A list extending it to TF-A Tests list as well.
Does TF-A Tests have test cases for console testing in TF-A? more specifically to test the CRASH console?
Regards, Akshay Belsare
-----Original Message----- From: Michal Simek via TF-A tf-a@lists.trustedfirmware.org Sent: Monday, September 18, 2023 2:17 PM To: tf-a@lists.trustedfirmware.org Subject: [TF-A] Console scope flags/ console switch state - understanding
Hi,
I am looking at how console flags are used and setup. In porting guide I see
Function : bl31_plat_runtime_setup() [optional]
:: Argument : void Return : void The purpose of this function is allow the platform to perform any BL31 runtime setup just prior to BL31 exit during cold boot. The default weak implementation of this function will invoke ``console_switch_state()`` to switch console output to consoles marked for use in the ``runtime`` state. Some platform are calling it but some of them not (like our Xilinx one). Tegra has in tegra_pwr_domain_power_down_wfi() console_flush(); console_switch_state(0); which is what none other has. Should console_flush() be called by default all the time when console is switched and also disabled when system goes down? Why console_switch_state(CONSOLE_FLAG_RUNTIME) is not called from bl31_main() when before bl31_plat_runtime_setup() is called we have console_flush() already? The second part of this how console scope is setup. Implementation is clear and set. void console_set_scope(console_t *console, unsigned int scope) { assert(console != NULL); console->flags = (console->flags & ~CONSOLE_FLAG_SCOPE_MASK) | scope; } The commit cc5859ca19ff ("Multi-console: Deprecate the `finish_console_register` macro") when finish_console_register is called (DCC is exception here) is setting up CONSOLE_FLAG_BOOT and CONSOLE_FLAG_CRASH by default. And most of platforms is calling console registration with calling console_set_scope() where new flags are recorded BOOT only, BOOT/RUNTIME, RUNTIME only or BOOT/RUNTIME/CRASH. I would like to understand what should be the right behavior. Why are platforms removing CRASH flag after registration? (I see that a lot of platforms are having private plat_crash_console_init() but pretty much crash console is the same with regular console). Why runtime console is setup directly in bl31_early_platform_setup2 when guidance is saying that it should be done much later? Also commit 63c52d0071ef ("plat/common/crash_console_helpers.S: Fix MULTI_CONSOLE_API support") removed CONSOLE_FLAG_CRASH from plat_crash_console_init but only from 64bit version. In 32bit version there is still there. It suggest that any C code should be called. Do we really need CONSOLE_FLAG_CRASH? Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP/Versal ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal/Versal NET SoCs TF-A maintainer - Xilinx ZynqMP/Versal/Versal NET SoCs -- TF-A mailing list -- tf-a@lists.trustedfirmware.org To unsubscribe send an email to tf-a-leave@lists.trustedfirmware.org
Hi Akshay
Yes, there is a test [1] to exercise crash console in TF-A. The test is performed with the help tf-a-ci-scripts, wherein a patch is applied in bl31_main() which induces a panic(). Look at CI run [2] artefacts, where UART1 log captures the console log for crash
[1] https://review.trustedfirmware.org/c/ci/tf-a-ci-scripts/+/19026 [2] https://ci.trustedfirmware.org/job/tf-a-builder/2538538/artifact/
Thanks Manish Pandey ________________________________ From: Belsare, Akshay via TF-A tf-a@lists.trustedfirmware.org Sent: 20 September 2023 12:21 To: Michal Simek monstr@monstr.eu; tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org; tf-a-tests@lists.trustedfirmware.org tf-a-tests@lists.trustedfirmware.org Subject: [TF-A] Re: Console scope flags/ console switch state - understanding
Hi, Since the discussion is already going on in TF-A list extending it to TF-A Tests list as well.
Does TF-A Tests have test cases for console testing in TF-A? more specifically to test the CRASH console?
Regards, Akshay Belsare
-----Original Message----- From: Michal Simek via TF-A tf-a@lists.trustedfirmware.org Sent: Monday, September 18, 2023 2:17 PM To: tf-a@lists.trustedfirmware.org Subject: [TF-A] Console scope flags/ console switch state - understanding
Hi,
I am looking at how console flags are used and setup. In porting guide I see
Function : bl31_plat_runtime_setup() [optional]
:: Argument : void Return : void The purpose of this function is allow the platform to perform any BL31 runtime setup just prior to BL31 exit during cold boot. The default weak implementation of this function will invoke ``console_switch_state()`` to switch console output to consoles marked for use in the ``runtime`` state. Some platform are calling it but some of them not (like our Xilinx one). Tegra has in tegra_pwr_domain_power_down_wfi() console_flush(); console_switch_state(0); which is what none other has. Should console_flush() be called by default all the time when console is switched and also disabled when system goes down? Why console_switch_state(CONSOLE_FLAG_RUNTIME) is not called from bl31_main() when before bl31_plat_runtime_setup() is called we have console_flush() already? The second part of this how console scope is setup. Implementation is clear and set. void console_set_scope(console_t *console, unsigned int scope) { assert(console != NULL); console->flags = (console->flags & ~CONSOLE_FLAG_SCOPE_MASK) | scope; } The commit cc5859ca19ff ("Multi-console: Deprecate the `finish_console_register` macro") when finish_console_register is called (DCC is exception here) is setting up CONSOLE_FLAG_BOOT and CONSOLE_FLAG_CRASH by default. And most of platforms is calling console registration with calling console_set_scope() where new flags are recorded BOOT only, BOOT/RUNTIME, RUNTIME only or BOOT/RUNTIME/CRASH. I would like to understand what should be the right behavior. Why are platforms removing CRASH flag after registration? (I see that a lot of platforms are having private plat_crash_console_init() but pretty much crash console is the same with regular console). Why runtime console is setup directly in bl31_early_platform_setup2 when guidance is saying that it should be done much later? Also commit 63c52d0071ef ("plat/common/crash_console_helpers.S: Fix MULTI_CONSOLE_API support") removed CONSOLE_FLAG_CRASH from plat_crash_console_init but only from 64bit version. In 32bit version there is still there. It suggest that any C code should be called. Do we really need CONSOLE_FLAG_CRASH? Thanks, Michal -- Michal Simek, Ing. (M.Eng), OpenPGP -> KeyID: FE3D1F91 w: www.monstr.eu<http://www.monstr.eu> p: +42-0-721842854 Maintainer of Linux kernel - Xilinx Microblaze Maintainer of Linux kernel - Xilinx Zynq ARM and ZynqMP/Versal ARM64 SoCs U-Boot custodian - Xilinx Microblaze/Zynq/ZynqMP/Versal/Versal NET SoCs TF-A maintainer - Xilinx ZynqMP/Versal/Versal NET SoCs -- TF-A mailing list -- tf-a@lists.trustedfirmware.org To unsubscribe send an email to tf-a-leave@lists.trustedfirmware.org
-- TF-A mailing list -- tf-a@lists.trustedfirmware.org To unsubscribe send an email to tf-a-leave@lists.trustedfirmware.org
Hi Manish,
On 9/20/23 13:51, Manish Pandey2 wrote:
Hi Akshay
Yes, there is a test [1] to exercise crash console in TF-A. The test is performed with the help tf-a-ci-scripts, wherein a patch is applied in bl31_main() which induces a panic(). Look at CI run [2] artefacts, where UART1 log captures the console log for crash
[1] https://review.trustedfirmware.org/c/ci/tf-a-ci-scripts/+/19026 [2] https://ci.trustedfirmware.org/job/tf-a-builder/2538538/artifact/
That's the panic from C and definitely nice to see it. Shouldn't be there also a test from asm?
Thanks, Michal
I don't see any tests which are exercise crash from assembly (will wait for others to comment) However, i did a quick experiment and can see the crash console is usable past crash stack is initialized in el3_arch_init_common() through tpidr_el3 register.
Its good to have feature, i can add it in our backlog.
Thanks Manish ________________________________ From: Michal Simek monstr@monstr.eu Sent: 20 September 2023 13:25 To: Manish Pandey2 Manish.Pandey2@arm.com; tf-a@lists.trustedfirmware.org tf-a@lists.trustedfirmware.org; tf-a-tests@lists.trustedfirmware.org tf-a-tests@lists.trustedfirmware.org; Belsare, Akshay akshay.belsare@amd.com Subject: Re: [TF-A] Re: Console scope flags/ console switch state - understanding
Hi Manish,
On 9/20/23 13:51, Manish Pandey2 wrote:
Hi Akshay
Yes, there is a test [1] to exercise crash console in TF-A. The test is performed with the help tf-a-ci-scripts, wherein a patch is applied in bl31_main() which induces a panic(). Look at CI run [2] artefacts, where UART1 log captures the console log for crash
[1] https://review.trustedfirmware.org/c/ci/tf-a-ci-scripts/+/19026 [2] https://ci.trustedfirmware.org/job/tf-a-builder/2538538/artifact/
That's the panic from C and definitely nice to see it. Shouldn't be there also a test from asm?
Thanks, Michal
Tegra has in tegra_pwr_domain_power_down_wfi() console_flush(); console_switch_state(0);
which is what none other has. Should console_flush() be called by default all the time when console is switched and also disabled when system goes down?
It seems that the Tegra platform intentionally wants to stop all console output when going into suspend and then turns it back on after resume. Not sure why they do that but there may be some platform-specific reason (e.g. maybe some code on resume would try to make it print to the UART before a clock for it is reenabled and that would make it hang or something).
If you care about seeing all console output during suspend, then generally you should explicitly call console_flush() somewhere at the end, yes. The suspend code is always platform-specific though so this is something every platform needs to implement independently. I assume most forget to do it because in practice, UART output tends to come out either way even without an explicit flush call.
Why console_switch_state(CONSOLE_FLAG_RUNTIME) is not called from bl31_main() when before bl31_plat_runtime_setup() is called we have console_flush() already?
I believe this may be a holdover from back when MULTI_CONSOLE_API was an optional feature that a platform had to enable explicitly. Now that it is the default, moving that call to bl31_main() would probably make sense.
I would like to understand what should be the right behavior. Why are platforms removing CRASH flag after registration? (I see that a lot of platforms are having private plat_crash_console_init() but pretty much crash console is the same with regular console). Why runtime console is setup directly in bl31_early_platform_setup2 when guidance is saying that it should be done much later?
I don't know about each platform and why they are overriding these flags, they may all have their own reasons. The default (BOOT | CRASH) should be right for most cases. If a console additionally sets the RUNTIME flag that just means TF-A will continue to print to it after the main operating system is booted (e.g. these might be platforms where the main operating system and TF-A use different UART ports so that they don't clash and there's no need to disable the TF-A output).
Many of the platforms that override the plat_crash_console functions may just be older platforms that implemented this manually (usually after the same standard pattern) before the default implementation was added. They could probably be fixed up to use the default instead but I assume nobody cares enough to put in that effort. It technically doesn't matter whether they disable CONSOLE_FLAG_CRASH (because when you override the plat_crash_console macros there's nothing looking at that flag anymore), but it may have still been done for clarity.
Also commit 63c52d0071ef ("plat/common/crash_console_helpers.S: Fix MULTI_CONSOLE_API support") removed CONSOLE_FLAG_CRASH from plat_crash_console_init but only from 64bit version. In 32bit version there is still there. It suggest that any C code should be called. Do we really need CONSOLE_FLAG_CRASH?
The 64bit version still respects the CRASH flag, it is just checked by plat_crash_console_putc() directly. Basically, before that patch, the crash console worked by just calling console_set_scope(CRASH) and then having plat_crash_console_putc() and plat_crash_console_flush() chain into the normal console_putc() and console_flush() functions. This worked back when the entire console framework was still written in assembly. Then somebody rewrote it in C, but that broke this crash console implementation because now console_putc() and console_flush() didn't work without a valid stack anymore. So the solution with that patch was to basically reimplement what the main console framework does (loop through all consoles and call the individual function pointers for them) in assembly. Those functions just check for CONSOLE_FLAG_CRASH directly so there was no reason to update console_set_scope() anymore.
The 32bit version is honestly just in disrepair and doesn't work. I only fixed up the 64bit version because I don't have a 32bit device to test with. In theory it shouldn't be hard to implement the equivalent to what the current 64bit version does there as well, but somebody else wrote the 32bit version and I guess they're no longer around to maintain it.
tf-a@lists.trustedfirmware.org