Merge "refactor(console): disable getc() by default" into integration

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 ...

Merge changes I8990bce2,Iacef5e67,I2976c0a4,I8551a802 into integration

* changes:
fix(plat/marvell/a3720/uart): do external reset during initialization
feat(plat/marvell/a3k): add north and sout

Merge changes I8990bce2,Iacef5e67,I2976c0a4,I8551a802 into integration

* changes:
fix(plat/marvell/a3720/uart): do external reset during initialization
feat(plat/marvell/a3k): add north and south bridge reset registers
fix(plat/marvell/a3720/uart): configure UART after TX FIFO reset
feat(plat/marvell/a3720/uart): preserve x1/x2 regs in console_a3700_core_init()

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 ...

Merge "fix(plat/marvell/a3720/uart): fix configuring UART clock" into integration

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 ...

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

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 ...

Merge changes I8ea4ea58,I1f0b4aab,I2cccad40 into integration

* changes:
marvell: uart: a3720: Increase TX FIFO EMPTY timeout from 2ms to 3ms
marvell: uart: a3720: Update delay code to be compati

Merge changes I8ea4ea58,I1f0b4aab,I2cccad40 into integration

* changes:
marvell: uart: a3720: Increase TX FIFO EMPTY timeout from 2ms to 3ms
marvell: uart: a3720: Update delay code to be compatible with 1200 MHz CPU
marvell: uart: a3720: Fix comments in console_a3700_core_init() function

show more ...

marvell: uart: a3720: Increase TX FIFO EMPTY timeout from 2ms to 3ms

TX FIFO has space for 32 characters. With default UART baudrate 115200 it
takes more than 2ms to transmit all 32 characters, so w

marvell: uart: a3720: Increase TX FIFO EMPTY timeout from 2ms to 3ms

TX FIFO has space for 32 characters. With default UART baudrate 115200 it
takes more than 2ms to transmit all 32 characters, so wait at least 3ms
before flushing TX FIFO.

If WTMI firmware transmitted something via UART before TF-A was booted,
some characters may still wait in TX FIFO when TF-A is initializing UART
driver. So wait at least 3ms to ensure that HW has enough time to transmit
all characters waiting in TX FIFO.

This fixes an issue where sometimes characters transmitted on UART by our
custom WTMI image are lost.

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

show more ...

marvell: uart: a3720: Update delay code to be compatible with 1200 MHz CPU

Console initialization function needs to wait at least minimal specified
time. The fastest Armada 3720 CPU is 1200 MHz so i

marvell: uart: a3720: Update delay code to be compatible with 1200 MHz CPU

Console initialization function needs to wait at least minimal specified
time. The fastest Armada 3720 CPU is 1200 MHz so increase loop delay to
wait at least for 100 us on 1200 MHz variant too. The slowest Armada 3720
CPU is 600 MHz and in this case delay loop would take just 2 times more,
which is not a problem.

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

show more ...

marvell: uart: a3720: Fix comments in console_a3700_core_init() function

The delay loop executes 3 instructions. These 3 instructions are executed
in 2 processor ticks and 30000 iterations on a 600

marvell: uart: a3720: Fix comments in console_a3700_core_init() function

The delay loop executes 3 instructions. These 3 instructions are executed
in 2 processor ticks and 30000 iterations on a 600 MHz CPU should yield
approximately 100 us. This means we are waiting 2 ms, not 20 ms, for TX
FIFO to be empty.

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

show more ...

Merge changes I19e4e7f5,I226b6e33 into integration

* changes:
marvell: uart: a3720: Fix macro name for 6th bit of Status Register
marvell: uart: a3720: Implement console_a3700_core_getc

marvell: uart: a3720: Fix macro name for 6th bit of Status Register

This patch does not change code, it only updates comments and macro name
for 6th bit of Status Register. So TF-A binary stay same.

marvell: uart: a3720: Fix macro name for 6th bit of Status Register

This patch does not change code, it only updates comments and macro name
for 6th bit of Status Register. So TF-A binary stay same.

6th bit of the Status Register is named TX EMPTY and is set to 1 when both
Transmitter Holding Register (THR) or Transmitter Shift Register (TSR) are
empty. It is when all characters were already transmitted.

There is also TX FIFO EMPTY bit in the Status Register which is set to 1
only when THR is empty.

In both console_a3700_core_init() and console_a3700_core_flush() functions
we should wait until both THR and TSR are empty therefore we should check
6th bit of the Status Register.

So current code is correct, just had misleading macro names and comments.
This change fixes this "documentation" issue, fixes macro name for 6th bit
of the Status Register and also updates comments.

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

show more ...

marvell: uart: a3720: Implement console_a3700_core_getc

Implementation is simple, just check if there is a pending character in
RX FIFO via RXRDY bit of Status Register and if yes, read it from
UART

marvell: uart: a3720: Implement console_a3700_core_getc

Implementation is simple, just check if there is a pending character in
RX FIFO via RXRDY bit of Status Register and if yes, read it from
UART_RX_REG register.

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

show more ...

Merge "marvell: uart: a3720: Implement console_a3700_core_flush" into integration

marvell: uart: a3720: Implement console_a3700_core_flush

Implementation is simple, just wait for the TX FIFO to be empty.

Without this patch TF-A on A3720 truncate the last line:

NOTICE: BL31:

marvell: uart: a3720: Implement console_a3700_core_flush

Implementation is simple, just wait for the TX FIFO to be empty.

Without this patch TF-A on A3720 truncate the last line:

NOTICE: BL31: Built : 16:1

With this patch TF-A on A3720 print correctly also the last line:

NOTICE: BL31: Built : 19:03:31, Dec 23 2020

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

show more ...

Merge "Don't return error information from console_flush" into integration

Don't return error information from console_flush

And from crash_console_flush.

We ignore the error information return by console_flush in _every_
place where we call it, and casting the return typ

Don't return error information from console_flush

And from crash_console_flush.

We ignore the error information return by console_flush in _every_
place where we call it, and casting the return type to void does not
work around the MISRA violation that this causes. Instead, we collect
the error information from the driver (to avoid changing that API), and
don't return it to the caller.

Change-Id: I1e35afe01764d5c8f0efd04f8949d333ffb688c1
Signed-off-by: Jimmy Brisson <jimmy.brisson@arm.com>

show more ...

Merge changes from topic "console_t_cleanup" into integration

* changes:
marvell: Consolidate console register calls
uniphier: Use generic console_t data structure
spe: Use generic console_t d

Merge changes from topic "console_t_cleanup" into integration

* changes:
marvell: Consolidate console register calls
uniphier: Use generic console_t data structure
spe: Use generic console_t data structure
LS 16550: Use generic console_t data structure
stm32: Use generic console_t data structure
rcar: Use generic console_t data structure
a3700: Use generic console_t data structure
16550: Use generic console_t data structure
imx: Use generic console_t data structure

show more ...

a3700: Use generic console_t data structure

Since now the generic console_t structure holds the UART base address as
well, let's use that generic location and drop the UART driver specific
data stru

a3700: Use generic console_t data structure

Since now the generic console_t structure holds the UART base address as
well, let's use that generic location and drop the UART driver specific
data structure at all.

Change-Id: I89c3ab2ed85ab941d8b38ced48474feb4aaa8b7e
Signed-off-by: Andre Przywara <andre.przywara@arm.com>

show more ...

Merge pull request #1920 from ambroise-arm/av/deprecated

Remove deprecated interfaces

Console: remove deprecated finish_console_register

The old version of the macro is deprecated.

Commit cc5859ca19ff ("Multi-console: Deprecate the
`finish_console_register` macro") provides more det

Console: remove deprecated finish_console_register

The old version of the macro is deprecated.

Commit cc5859ca19ff ("Multi-console: Deprecate the
`finish_console_register` macro") provides more details.

Change-Id: I3d1cdf6496db7d8e6cfbb5804f508ff46ae7e67e
Signed-off-by: Ambroise Vincent <ambroise.vincent@arm.com>

show more ...