hw/arm: rk3588: add UART3 and fix GIC NonSecure handoff for direct kernel boot - #35
Conversation
|
@codex review this PR thoroughly |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 716c4474c3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if (albifc->arm_linux_init) { | ||
| albifc->arm_linux_init(albif, false); |
There was a problem hiding this comment.
When -kernel points to a firmware ELF such as U-Boot or BL31 rather than a NonSecure Zephyr payload, arm_load_kernel() classifies the ELF as non-Linux (hw/arm/boot.c:919-921) and leaves it booting at the highest supported exception level, which is secure EL3 under TCG (hw/arm/boot.c:1260-1270). This unconditional hook nevertheless makes the GIC reset every SPI as Group 1 NonSecure, so secure firmware relying on or testing the hardware Group 0 reset state receives incorrect interrupt ownership and can lose interrupts. Gate the handoff on a mode known to boot NonSecure, such as zephyr_ram, rather than applying it to every -kernel payload.
Useful? React with 👍 / 👎.
| rk3588_create_one_uart(s, RK3588_UART3, RK3588_UART3_SPI, | ||
| s->zephyr_ram ? 2 : 1, "uart3-vendor"); |
There was a problem hiding this comment.
For direct Linux boots that omit -dtb, rk3588_get_dtb() still calls rk3588_fdt_add_uart_node(), which emits only /serial@feb50000 for UART2. Consequently Linux cannot discover or bind a driver to the new UART3 at 0xfeb60000, leaving its serial_hd(1) backend unusable despite the MMIO device being instantiated. Add the corresponding UART3 node and interrupt information to the generated DT.
Useful? React with 👍 / 👎.
The RK3588 exposes UART3 (dw-apb-uart, 16550-compatible) at 0xfeb60000 with GIC SPI 334, but the machine model only wired up UART2. Guests that need a second serial channel, such as Zephyr images using uart2 as the console and uart3 as a dedicated data link, had no device to attach. Factor the existing UART2 creation into a per-UART helper and instantiate UART3 with the same serial_mm + DesignWare vendor window layout. In zephyr-ram mode UART3 takes serial_hd(2), keeping the existing assignments (0 = DWC3 UDC CDC bridge, 1 = UART2 console); otherwise it takes serial_hd(1). Describe UART3 in the generated device tree as well (new /serial@feb60000 node plus a serial3 alias) so direct Linux boots without -dtb can discover it and bind a driver. Signed-off-by: Hongquan Li <hongquan.li@processmission.com>
Zephyr-ram direct kernel boot on the RK3588 machine starts the guest in NonSecure state with no secure firmware having run. arm/boot.c only invokes the ARMLinuxBootIf handoff (which marks a TrustZone-aware GIC's interrupts as NonSecure Group 1 at reset) for raw Linux images; ELF kernels such as Zephyr skip it. The GIC then resets with every interrupt in Group 0 and DS=0, so a NonSecure guest's writes to GICD_IGROUPR/ISENABLER are RAZ/WI and it can never enable any SPI. This was observed as UART RX interrupts (e.g. uart3, SPI 334) never being delivered to a Zephyr guest while polled TX worked fine. Invoke the same arm_linux_init(false) hook before arm_load_kernel(), but only in zephyr-ram mode, which is known to boot NonSecure. Other ELF payloads (e.g. U-Boot or BL31) boot at secure EL3 and may rely on or test the hardware Group 0 reset state, so leave the GIC untouched for them. Signed-off-by: Hongquan Li <hongquan.li@processmission.com>
|
To use Codex here, create a Codex account and connect to github. |
|
@codex review this PR thoroughly |
|
Codex Review: Didn't find any major issues. Nice work! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
2 participants
hw/arm: add UART3 to the RK3588 machine model (f85d7d6)
hw/arm: hand off GIC interrupts to NonSecure for direct kernel boot (716c447)
handoff (marking a TZ-aware GIC's interrupts NonSecure Group 1 at reset) for raw Linux images, so ELF kernels such as Zephyr
skip it
Testing
received incrementing samples at 1 Hz