refactor(console): disable getc() by default

The ability to read a character from the console constitutes an attack
vector into TF-A, as it gives attackers a means to inject arbitrary
data into TF-A

refactor(console): disable getc() by default

The ability to read a character from the console constitutes an attack
vector into TF-A, as it gives attackers a means to inject arbitrary
data into TF-A. It is dangerous to keep that feature enabled if not
strictly necessary, especially in production firmware builds.

Thus, we need a way to disable this feature. Moreover, when it is
disabled, all related code should be eliminated from the firmware
binaries, such that no remnant/dead getc() code remains in memory,
which could otherwise be used as a gadget as part of a bigger security
attack.

This patch disables getc() feature by default. For legitimate getc()
use cases [1], it can be explicitly enabled by building TF-A with
ENABLE_CONSOLE_GETC=1.

The following changes are introduced when getc() is disabled:

- The multi-console framework no longer provides the console_getc()
function.

- If the console driver selected by the platform attempts to register
a getc() callback into the multi-console framework then TF-A will
now fail to build.

If registered through the assembly function finish_console_register():
- On AArch64, you'll get:
Error: undefined symbol CONSOLE_T_GETC used as an immediate value.
- On AArch32, you'll get:
Error: internal_relocation (type: OFFSET_IMM) not fixed up

If registered through the C function console_register(), this requires
populating a struct console with a getc field, which will trigger:
error: 'console_t' {aka 'struct console'} has no member named 'getc'

- All console drivers which previously registered a getc() callback
have been modified to do so only when ENABLE_CONSOLE_GETC=1.

[1] Example of such use cases would be:
- Firmware recovery: retrieving a golden BL2 image over the console in
order to repair a broken firmware on a bricked board.
- Factory CLI tool: Drive some soak tests through the console.

Discussed on TF-A mailing list here:
https://lists.trustedfirmware.org/archives/list/tf-a@lists.trustedfirmware.org/thread/YS7F6RCNTWBTEOBLAXIRTXWIOYINVRW7/

Change-Id: Icb412304cd23dbdd7662df7cf8992267b7975cc5
Signed-off-by: Sandrine Bailleux <sandrine.bailleux@arm.com>
Acked-by: Baruch Siach <baruch@tkos.co.il>

show more ...

fix(plat/marvell/a3720/uart): do external reset during initialization

Sometimes when changing UART clock from TBG to XTAL, UART HW enters into
some broken state. It does not transit characters from

fix(plat/marvell/a3720/uart): do external reset during initialization

Sometimes when changing UART clock from TBG to XTAL, UART HW enters into
some broken state. It does not transit characters from TX FIFO anymore
and TX FIFO stays always empty. TX FIFO reset does not recover UART HW
from this broken state.

Experiments show that external reset can fix UART HW from this broken
state.

TF-A fatal error handler calls console_a3700_core_init() function to
initialize UART HW. This handler may be called anytime during CPU
runtime, also when kernel is running.

U-Boot or Linux kernel may change UART clock to TBG to achieve higher
baudrates. During initialization, console_a3700_core_init() resets UART
configuration to default settings, which means that it also changes
UART clock from TBG to XTAL.

Do an external reset of UART via North Bridge Peripheral reset register
to prevent this UART hangup.

Signed-off-by: Pali Rohár <pali@kernel.org>
Change-Id: I8990bce24d1a6fd8ccc47a2cd0a5ff932fcfcf14

show more ...

fix(plat/marvell/a3720/uart): configure UART after TX FIFO reset

If TX FIFO is not empty, do not touch UART settings and let UART HW
transmit remaining bytes from TX FIFO. New UART settings are then

fix(plat/marvell/a3720/uart): configure UART after TX FIFO reset

If TX FIFO is not empty, do not touch UART settings and let UART HW
transmit remaining bytes from TX FIFO. New UART settings are then set
only after TX FIFO is reset.

Signed-off-by: Pali Rohár <pali@kernel.org>
Change-Id: I2976c0a4fbb841d3a79d42ef67c06e70174afc3b

show more ...

feat(plat/marvell/a3720/uart): preserve x1/x2 regs in console_a3700_core_init()

Followup changes will need function arguments in registers x0, x1 and
x2. Do not modify x1 and x2 registers and instea

feat(plat/marvell/a3720/uart): preserve x1/x2 regs in console_a3700_core_init()

Followup changes will need function arguments in registers x0, x1 and
x2. Do not modify x1 and x2 registers and instead use scratch x3 and x4
registers for storing local variables.

Signed-off-by: Pali Rohár <pali@kernel.org>
Change-Id: I8551a802995f39128d2f4a8f8076b5bf463d0db0

show more ...

fix(plat/marvell/a3720/uart): fix configuring UART clock

When configuring the UART_BAUD_REG register, the function
console_a3700_core_init() currently only changes the baud divisor field,
leaving ot

fix(plat/marvell/a3720/uart): fix configuring UART clock

When configuring the UART_BAUD_REG register, the function
console_a3700_core_init() currently only changes the baud divisor field,
leaving other fields to their previous value.

This is incorrect, because the baud divisor is computed with the
assumption that the parent clock rate is 25 MHz, and since the other
fields in this register configure the parent clock, which could have
been changed by U-Boot or Linux.

Fix this function to also configure the other fields so that the UART
parent clock is selected to be the xtal clock.

For example without this change TF-A prints only

ERROR: a3700_system_off needs to be implemented

followed by garbage after plat_crash_console_init() is called.

After applying this change instead of garbage it also print crash info:

PANIC at PC : 0x0000000004023800

Signed-off-by: Pali Rohár <pali@kernel.org>
Change-Id: I72f338355cc60d939b8bb978d9c7fdd576416b81

show more ...

fix(plat/marvell/a3720/uart): fix UART clock rate value and divisor calculation

UART parent clock is by default the platform's xtal clock, which is
25 MHz.

The value defined in the driver, though,

fix(plat/marvell/a3720/uart): fix UART clock rate value and divisor calculation

UART parent clock is by default the platform's xtal clock, which is
25 MHz.

The value defined in the driver, though, is 25.8048 MHz. This is a hack
for the suboptimal divisor calculation
Divisor = UART clock / (16 * baudrate)
which does not use rounding division, resulting in a suboptimal value
for divisor if the correct parent clock rate was used.

Change the code for divisor calculation to
Divisor = Round(UART clock / (16 * baudrate))
and change the parent clock rate value to 25 MHz.

The final UART divisor for default baudrate 115200 is not affected by
this change.

(Note that the parent clock rate should not be defined via a macro,
since the xtal clock can also be 40 MHz. This is outside of the scope of
this fix, though.)

Signed-off-by: Pali Rohár <pali@kernel.org>
Change-Id: Iaa401173df87aec94f2dd1b38a90fb6ed0bf0ec6

show more ...