search

ARM-software / tf-issues

Issue tracking for the ARM Trusted Firmware project
37 stars 16 forks source link

console_flush() may create infinite loop on poweroff/reboot if OS disabled serial #647

Closed pbatard closed 6 years ago

pbatard commented 6 years ago

This issue is being observed with Ubuntu 18.04 LTS installed in UEFI mode on a Raspberry Pi 3.

The problem is that the default of Ubuntu is to disable the serial console that was used by ATF, but not in a manner where characters can not be enqueued.

As a result, the informational messages generated by ATF during Power Off are setting the 16550 UARTLSR_TEMT, UARTLSR_THRE flags, but since no characters are ever sent, these flags are never cleared, causing an infinite loop in 16550_console.S:

    /* Loop until the transmit FIFO is empty */
1:  ldr w1, [x0, #UARTLSR]
    and w1, w1, #(UARTLSR_TEMT | UARTLSR_THRE)
    cmp w1, #(UARTLSR_TEMT | UARTLSR_THRE)
    b.ne    1b

The end result is that users are left with a system that can not be rebooted.

Because this is an issue that only manifests itself during the power down/cycle paths, and we can envision other systems than the Raspberry Pi running into a similar issue, we believe that a new global build option, called CONSOLE_FLUSH_ON_POWEROFF (that would be enabled by default so as not to alter the current behaviour) should be added, so that it becomes possible to disable calls to console_flush() altogether during poweroff, thus alleviating this issue.

Having this option integrated into ATF is especially crucial for the work we are doing in getting an UEFI firmware for the Raspberry Pi 3 submitted to the EDK2, as, without a fix for the Ubuntu 18.04 reboot issue in ATF, we won't be in a position to provide vanilla ATF binaries as part of that firmware...

We will therefore submit a pull request with a patch proposal soon.

soby-mathew commented 6 years ago

Hmm, this seems to a platform port issue. The firmware must never share the UART with normal world, as this can a DoS attack vector into the secure world. On ARM reference platforms such as FVP, it initially uses the same UART as Linux during the cold boot phase and once cold boot is completed, it switches to a runtime UART (which is not shared with Linux).

RPi3 should also do the same. If it doesn't have a separate UART to register as runtime UART, then it shouldn't register any UART for runtime.

ghost commented 6 years ago

I'd like to have an option to print runtime messages in that UART even though it's not advisable to do that, it's still useful for debugging. Maybe the best thing to do is to have something like RPI3_RUNTIME_UART that can decide what to do (UART0, UART1, DISABLED) that defaults to DISABLED.

pbatard commented 6 years ago

Even if this is the cause of my issue, I agree with @antonio-nino-diaz-arm that I'd prefer having a build option, disabled by default, to leave the UART enabled to print runtime message.

I guess if we go with this option my pull request becomes moot, as I should then be able to use RPI3_RUNTIME_UART=DISABLED, which is fine with me.