Sg204x v1.9 rva23 - #26
Open
wxjstz wants to merge 143 commits into
Open
Conversation
Fix "sbi_scratch" to "HART id" to better reflect its purpose. Signed-off-by: Leo Yu-Chi Liang <ycliang@andestech.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260107032602.1143819-1-ycliang@andestech.com Signed-off-by: Anup Patel <anup@brainfault.org>
The commit 42139bb ("lib: sbi: Replace sbi_hart_pmp_xyz() and sbi_hart_map/unmap_addr()") changed logic by calling sbi_hart_protection_configure(). But when protection doesn't exist the function is returning SBI_EINVAL. But on systems without protection this is correct configuration that's why do not hang when system don't have any HART protection. Fixes: 42139bb ("lib: sbi: Replace sbi_hart_pmp_xyz() and sbi_hart_map/unmap_addr()") Signed-off-by: Michal Simek <michal.simek@amd.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/bb8641e5f82654e3989537cea85f165f67a7044e.1767801896.git.michal.simek@amd.com Signed-off-by: Anup Patel <anup@brainfault.org>
Refactor ATCSMU (System Management Unit) support by moving it from a system utility into a dedicated FDT-based HSM driver. Key changes include: - Moving the functions in lib/utils/sys/atcsmu.c into the new HSM driver - Moving hart start and stop operations on AE350 platform into the new HSM driver - Converting the assembly-based functions in sleep.S to C code for the readability - Updating the ATCWDT200 driver Signed-off-by: Ben Zong-You Xie <ben717@andestech.com> Signed-off-by: Leo Yu-Chi Liang <ycliang@andestech.com> Link: https://lore.kernel.org/r/20251229071914.1451587-2-ben717@andestech.com Signed-off-by: Anup Patel <anup@brainfault.org>
… platform Implement a save and restore mechanism for Andes-specific CSRs to support hardware power-saving modes, such as CPU hotplug or suspend to RAM. Signed-off-by: Ben Zong-You Xie <ben717@andestech.com> Signed-off-by: Leo Yu-Chi Liang <ycliang@andestech.com> Link: https://lore.kernel.org/r/20251229071914.1451587-3-ben717@andestech.com Signed-off-by: Anup Patel <anup@brainfault.org>
Add functions to enable/disable the cache. Signed-off-by: Ben Zong-You Xie <ben717@andestech.com> Link: https://lore.kernel.org/r/20251229071914.1451587-4-ben717@andestech.com Signed-off-by: Anup Patel <anup@brainfault.org>
Introduce a FDT-based driver for the Andes Last Level Cache (LLC) controller to support cache maintenance operations. Signed-off-by: Ben Zong-You Xie <ben717@andestech.com> Link: https://lore.kernel.org/r/20251229071914.1451587-5-ben717@andestech.com Signed-off-by: Anup Patel <anup@brainfault.org>
Implement a system-wide suspend driver for the Andes AE350 platform. This driver supports Andes-specific deep sleep (suspend to RAM) and light sleep (suspend to standby) functionalities via the ATCSMU. The major differences between deep sleep and light sleep are: - Power Domain and Resume Path: Deep sleep powers down the core domain. Consequently, harts waking from deep sleep resume from the reset vector. Light sleep utilizes clock gating to the core domain; harts maintain state and resume execution at the instruction immediately following the WFI instruction. - Primary Hart Wakeup: In both modes, the primary hart is woken by UART or RTC alarm interrupts. In deep sleep, the primary hart is additionally responsible for re-enabling the Last Level Cache (LLC) and restoring Andes-specific CSRs. - Secondary Hart Wakeup: In light sleep, secondary harts are woken by an IPI sent from the primary hart. In deep sleep, they are woken by an ATCSMU hardware wake-up command. Furthermore, secondary harts must restore Andes-specific CSRs when returning from deep sleep. Signed-off-by: Ben Zong-You Xie <ben717@andestech.com> Signed-off-by: Leo Yu-Chi Liang <ycliang@andestech.com> Link: https://lore.kernel.org/r/20251229071914.1451587-6-ben717@andestech.com Signed-off-by: Anup Patel <anup@brainfault.org>
Try to initialize stack protection guard via the zkr extension. Signed-off-by: Xiang W <wxjstz@126.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260104051812.128496-1-wxjstz@126.com Signed-off-by: Anup Patel <anup@brainfault.org>
Altera provides a JTAG UART core that provides virtual UART over JTAG and can coexist with their virtual JTAG. [1] This core has already been supported by Linux and the programming interface has always been stable. Add support for it to OpenSBI to ease JTAG prototype bringing up. The driver follows the device tree binding in mainline Linux. [2] [1] https://docs.altera.com/r/docs/683130/25.3/embedded-peripherals-ip-user-guide/jtag-uart-core [2] https://github.com/torvalds/linux/blob/v6.19-rc1/Documentation/devicetree/bindings/serial/altr%2Cjuart-1.0.yaml Signed-off-by: Icenowy Zheng <zhengxingda@iscas.ac.cn> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260104065506.70182-1-zhengxingda@iscas.ac.cn Signed-off-by: Anup Patel <anup@brainfault.org>
…U device Currently, we immediately return the result of `fw_counter_start` if the event code is 0xFFFF. However, this skips setting the bit in the `fw_counters_started` bitmap even if the platform-specific call succeeds. Restore the original behavior of returning early only on an error so that we still set the bit in the bitmap. This prevents multiple starts of the same FW counter. This also aligns the expectations of `pmu_ctr_start_fw` with `pmu_ctr_stop_fw` since we cannot assume that the platform-specific functions to start and stop FW counters will modify the bitmap state. Fixes: 57d3aa3 ("lib: sbi_pmu: Introduce fw_counter_write_value API") Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260116165304.180441-1-jamestiotio@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
…on RV64 Currently, when we attempt to read the upper 32 bits of a firmware counter on RV64 or higher, we just set `sbiret.value` to 0 without validating the counter index. The SBI specification requires us to set `sbiret.error` to `SBI_ERR_INVALID_PARAM` if the counter index points to a hardware counter or an invalid counter. Add a validation check to ensure compliance with the specification on RV64 or higher. Fixes: 51951d9 ("lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi") Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260125090643.190748-1-jamestiotio@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
Hartid is better represented by hex number since it is likely a combination of bits representing various elements in the platform hierarchy Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-1-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Disable PMP entries not configured in domain. These entries may contain values configured by the boot loader; disabling it to be certain PMP configuration is exactly as configured by the openSBI Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-2-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
use function call like format; debug print returning value of the "read" accessor Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-3-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Reserve memory upfront in large well aligned chunks, to avoid problem with PMP granularity that is 64Kbytes for the p8700 CPU Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-4-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
While powering up cluster, only indication is bit in cluster power control. It used to wait for CORE0 in that cluster reach U5 state (non-coherent execution), this won't happen when only CM powered up without booting any core Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-5-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
When powering up cores, wait for power up to complete using tight loop. This saves 10ms delay observed for every core Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-6-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
In the multi-cluster system each cluster has its own CM (Coherency Manager). Every CM has its "global" memory address where it is accessible from any bus master. Initially, all CMs accessible from the local cluster using same "local" address. Transactions by local address are not routed through system bus and thus are faster. Remap CM in every cluster to the local address matching its global address. Then, every CM is always accessed using same address, but when transaction initiated from the local cluster it is routed internally. This removes need for 2 PMP regions covering local address access. CM accessor functions simplified because there's no need to detect whether transaction is local or global Access timer always in cluster 0 Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-7-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Introduce a structure p8700_cm_info holding the number of coherence managers and their base addresses found in a particular SoC. Declare a global pointer to the structure that is set in mips_p8700_platform_init(), based on the match data of the platform compatible. For the match data of the MIPS P8700, a single coherence manager with a base address of 0x16100000 is declared, identical to what is found in mips/board.h. For now, access to the coherence manager register is still based on the hard-coded values defined in mips/board.h. Signed-off-by: Benoît Monin <benoit.monin@bootlin.com> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-8-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Modify the coherence manager register accessors to use the global variable p8700_cm_info instead of the statically declared GLOBAL_CM_BASE array. Also use p8700_cm_info to get the number of coherence managers and their base addresses in mips_p8700_early_init() and mips_p8700_nascent_init(). Clean up the hard-coded values in mips/board.h, access to the coherence manager is now fully based on information provided by platform compatible from the device tree. Signed-off-by: Benoît Monin <benoit.monin@bootlin.com> Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-9-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Refactor MIPS P8700 support, convert P8700 into a "CPU" and add 2 platforms using this CPU: - "boston" - FPGA platform developed by MIPS - "eyeq7h" - automotive platform by Mobileye Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-10-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
In the eyeq7h platform, there's extra power control for the CPU clusters. To enable cluster, it should be powered up using this OLB registers prior to accessing any cluster management registers Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-11-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
In the design, accelerator clusters ACC[01] and XNN[01] presence indicated by the OLB_WEST register OLB_WEST_TSTCSR. In the simulation environments, part (or all) accelerators may be not instantiated Disable clusters not present in the model, updating the DTB Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-12-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
On the EyeQ7H board, there's cluster level resets found in the accelerator OLBs. These resets should be deasserted once on boot and never used after Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-13-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
P8700 has a read-only cache configuration registers. Provide a CPU specific function to extract cache information. Use this information in the eyeq7h board for informational message Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-14-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Reserve memory regions for CPU clusters according to P8700 cluster memory layout. There's a set of components in the CPU cluster according to [1] [1] https://mips.com/wp-content/uploads/2025/11/P8700-F_Programmers_Reference_Manual-TM.pdf Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-15-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Switch to GENMASK, EXTRACT_BITFIELD, INSERT_BITFIELD Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-16-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
There's high-resolution (1GHz) timer found in the p8700 cluster. This timer used for precise time measurement by platform specific software. Synchronize this proprietary timers to reference in cluster 0. Procedure borrowed from the aclint mtimer. Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-17-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
Use eyeq7 specific method to synchronously restart architectural mtimer and eyeq7h specific high-resolution timer with common hardware trigger. This ensures all timers are precisely in sync Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-18-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
…ers 1..2 CPU clusters 1 and 2 cannot access NCORE registers through AUX ports. AUX ports of clusters 1 and 2 are connected to NCORE through east port. East port has no access to NCORE registers address space. Re-route NCORE registers range to MEM port by re-configuring MMIO regions in the GCR. REsulting map is as below. Mind a gap between regions [1] and [2]; this gap covering NCORE registers now routed to the default MEM port Cluster 0: 4 MMIO regions [0] : 0x0000000000000000-0x000000001fff0000 AUX0 UC|UCA [1] : 0x0000000020000000-0x00000000677f0000 AUX0 ANY [2] : 0x0000000080000000-0x0000001fffff0000 AUX0 UC|UCA [3] : --disabled-- Cluster 1: 4 MMIO regions [0] : 0x0000000000000000-0x000000001fff0000 AUX0 UC|UCA [1] : 0x0000000020000000-0x00000000677f0000 AUX0 ANY [2] : 0x0000000080000000-0x0000001fffff0000 AUX0 UC|UCA [3] : --disabled-- Cluster 2: 4 MMIO regions [0] : 0x0000000000000000-0x000000001fff0000 AUX0 UC|UCA [1] : 0x0000000020000000-0x00000000677f0000 AUX0 ANY [2] : 0x0000000080000000-0x0000001fffff0000 AUX0 UC|UCA [3] : --disabled-- Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260223-for-upstream-eyeq7h-v3-19-621d004d1a21@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
During warm reset, my EIC770X/Hifive Premier P550 can sometimes
encounter memory corruption issue crashing Linux boot. Currently the
issue is mitigated by having a sbi_printf before writing to the reset
register. I analyzed the issue further since then. From the SoC
datasheet[1], it's recommended to implement power-down flow as:
a. Designate a primary core, and let it broadcast requests to other
cores to execute a CEASE insn. Primary core also notifies an
"Externel Agent" to start monitoring.
b. Primary core waits for other cores to CEASE before it CEASEs.
c. "External Agent" waits for primary core to CEASE before resets
the Core Complex.
It's possible that EIC770X can trigger undefined behavior if the core
complex is reset while the harts are actively running. The sbi_printf
in the reset handler effectively hides the problem by delaying the
reset -- by the time sbi_printf finishes, all other harts will have
already landed in the loop in sbi_hsm_hart_wait(), which parks the hart.
Without the sbi_printf, I confirmed that other harts haven't reached
sbi_hsm_hart_wait yet before current hart resets the SoC. (by debugging)
To safely reset, and inspired by the datasheet, the warm reset logic
in eic770x.c now use the current hart as both primary core and the
"External Agent", and other harts as secondary cores. It leverages
the HSM framework and a new eic770x_hsm device to CEASE other harts,
and wait for them to CEASE before resets the SoC. with the sbi_printf
before reset removed, and this logic in place, stress test shows that
the memory corruption issue no longer occurs.
The new eic770x_hsm device is only used for the reset-CEASE logic at
the moment, and may be extended to a fully functional HSM device in
the future.
[1] https://github.com/eswincomputing/EIC7700X-SoC-Technical-Reference-Manual
Fixes: e5797e0 ("platform: generic: eswin: add EIC7700")
Signed-off-by: Bo Gan <ganboing@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260605075708.96-3-ganboing@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>
- Add MXL encoding for calculating XLEN. - Add instruction encoding for c.lbu/c.sb, and imm encoding for multiple RVC insn. Signed-off-by: Bo Gan <ganboing@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Link: https://lore.kernel.org/r/20260605113214.242-2-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
sbi_regs_prev_xlen reports the xlen of previous mode by decoding from multiple CSRs including mstatus/hstatus/vsstatus Signed-off-by: Bo Gan <ganboing@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Link: https://lore.kernel.org/r/20260605113214.242-3-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
These macros can be used to decode rd' and set rd' in RVC instructions Signed-off-by: Bo Gan <ganboing@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Link: https://lore.kernel.org/r/20260605113214.242-4-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
According to Privileged ISA 19.2.11: Modifying the floating-point state when V=1 causes both fields (vsstatus.FS and the HS-level sstatus.FS) to be set to 3 (Dirty) Fixes: 130e65d ("lib: sbi: Implement SET_FS_DIRTY() to make sure the mstatus FS dirty is set") Signed-off-by: Bo Gan <ganboing@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Link: https://lore.kernel.org/r/20260605113214.242-5-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
It's wrong to override the emulator callback in sbi_trap_emulate_load/ store. The function must respect the callback function passed in the parameter. Hence, let the misaligned emulator callback decide when to use sbi_misaligned_v_ld/st_emulator. To clean up things, also make the following changes: - Add the `insn` parameter to the callback. The trapping insn has been fetched by the caller already, whether transformed or directly loaded, thus saving the trouble in the callback. Note that you must not rely on the length of the `insn`, as it can be a transformed one from tinst - Also the `tcntx` is added, providing the callback with register values to handle vector insn or other customized insns. - Clarify that the read/write length (rlen/wlen) can be 0, in which case it could be a vector load/store or some customized instruction. The callback is responsible to handle it accordingly. Also fixed issues in the sbi_misaligned_v_ld/st_emulator: a. Redirect the trap when OPENSBI_CC_SUPPORT_VECTOR is not available. b. Ensure the return code is >0 when no faults are redirected. Fixes: c2acc5e ("lib: sbi_misaligned_ldst: Add handling of vector load/store") Signed-off-by: Bo Gan <ganboing@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Link: https://lore.kernel.org/r/20260605113214.242-6-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Bo Gan <ganboing@gmail.com> Reviewed-by: Anup Patel <anup@brainfault.org> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Link: https://lore.kernel.org/r/20260605113214.242-7-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
Rehaul instruction decoding to fix the following issues:
- We assume the XLEN of previous mode is the same as MXLEN. However,
RVC instructions decodes differently in RV32 and RV64, so shouldn't
have assumed that.
- We assume it's a misaligned fault and the load/store offset is 0,
i.e., base address == fault address, but access faults can have
non-0 offset (on HW supporting misaligned accesses), so platform
specific load/store fault handler gets the wrong base address.
- No checking of [63:32] of tinst in RV64, which is explicitly
required by Privileged ISA 19.6.3. Must reject tinst with non-0
high 32 bits.
Thus, fix all the above. For misaligned load/store fault, the address
offset should be 0, but we'll validate that on a DEBUG build. On an
optmized build, we kill the use of base address, and use trap address
instead (same as before), which lets the compiler optimize out imm
parsing and other calculations.
I also analyzed the behavior of misaligned fault handler before fix.
With the following conditions met, it can trigger data corruption:
- HW doesn't transform instruction into tinst.
- HW doesn't support misaligned load/store, and OS doesn't enable
misaligned delegation, thus OpenSBI handler is in effect
- HW supports mixed XLEN, and M mode is running RV64, and the trapping
mode (U/VS/VU) is running RV32.
- The trapping instruction is c.f{l|s}w(sp).
Due to the incorrect insn decoding, the trapping instruction would
mistakenly be decoded as c.{l|s}d(sp). With this fix, c.f{l|s}w(sp)
in RV32 is now emulated correctly.
Validation:
The patch is validated to have fixed the issue with test cases running
on a modified version of QEMU that exposes misaligned faults [1], and
a further modified version that removes tinst transformation [2]. The
S-mode OS is a local build of Debian Trixie 6.12 kernel that enables
COMPAT (RV32), and the U-mode test application exercises all integer
and floating-point load/store (RVIFD64/32+RVC64/32) instructions with
all possible imm values. The patch is also tested on real HW (Sifive
P550/ESWIN EIC7700), which only supports RV64. On P550, the same test
was validated both in U mode and VU mode, where the host runs a 6.12
ESWIN vendor kernel that has some ESWIN SoC device driver patches [3]
applied, and the guest runs the exact same Debian Trixie 6.12 kernel
mentioned above.
[1] https://github.com/ganboing/qemu/tree/ganboing-misalign
[2] https://github.com/ganboing/qemu/tree/ganboing-misalign-no-tinst
[3] https://github.com/sifiveinc/riscv-linux/tree/rel/kernel-6.12/hifive-premier-p550
Fixes: 7219477 ("lib: Use MTINST CSR in misaligned load/store emulation")
Fixes: b5ae8e8 ("lib: Add misaligned load/store trap handling")
Fixes: 4c11265 ("lib: sbi: abstract out insn decoding to unify mem fault handlers")
Signed-off-by: Bo Gan <ganboing@gmail.com>
Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260605113214.242-8-ganboing@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>
The per-HART stack size for exception/interrupt handling is currently hardcoded to 8192 bytes in SBI_PLATFORM_DEFAULT_HART_STACK_SIZE. This may not be sufficient for platforms with deeper call stacks (e.g. those enabling additional SBI extensions) or may be wasteful for minimal platforms. Introduce a HART_STACK_SIZE Kconfig option in lib/sbi/Kconfig with a valid range of 8192 to 1048576 bytes and a default of 8192 bytes to preserve existing behavior. The SBI_PLATFORM_DEFAULT_HART_STACK_SIZE macro now resolves to CONFIG_HART_STACK_SIZE, allowing all platforms to benefit from a single configuration knob without any source changes. Signed-off-by: Oriol Catalan <oriol.catalan@openchip.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/VI0P192MB3062735A6194BB6DA72083499E002@VI0P192MB3062.EURP192.PROD.OUTLOOK.COM Signed-off-by: Anup Patel <anup@brainfault.org>
The current SBI DBTR extension implementation accesses tdata2 and tdata3
without first checking whether either register is implemented on the
underlying hart. This produces an illegal instruction exception on
otherwise spec-compliant cores that legitimately omit one or both
registers.
Per the RISC-V Debug Specification, Chapter 5 (Sdtrig ISA Extension)
and Section 5.7 (Trigger Module Registers):
Section 5 (Sdtrig introduction):
"If Sdtrig is implemented, the Trigger Module must support at least
one trigger. Accessing trigger CSRs that are not used by any of the
implemented triggers must result in an illegal instruction
exception. M-Mode and Debug Mode accesses to trigger CSRs that are
used by any of the implemented triggers must succeed, regardless of
the current type of the currently selected trigger."
Section 5.7 (Trigger Module Registers):
"Attempts to access an unimplemented Trigger Module Register raise
an illegal instruction exception."
Per-register optionality is also explicit:
Section 5.7.3 (Trigger Data 2, at 0x7a2):
"Trigger-specific data. It is optional if no implemented triggers
use it."
Section 5.7.4 (Trigger Data 3, at 0x7a3):
"Trigger-specific data. It is optional if no implemented triggers
use it."
Section 5.7.17 (Trigger Extra (RV32), at 0x7a3), which also applies
via textra64 on RV64:
"All functionality in this register is optional. Any number of
upper bits of mhvalue and svalue may be tied to 0. mhselect and
sselect may only support 0 (ignore)."
Unconditionally accessing tdata2/tdata3 in the install/update/read/
uninstall paths causes SBI calls to fail with an illegal instruction
exception on hardware that does not implement one or both CSRs, even
if the supervisor-supplied trigger configuration does not require the
missing CSR(s).
This patch:
1. Introduces tdata_read_safe() / tdata_write_safe() helpers that
wrap csr_read_allowed / csr_write_allowed so that an illegal-
instruction trap raised by an unimplemented CSR is caught locally
rather than propagated. On the read path, a trapped read yields
zero; on the write path, the trap is silently absorbed (writes to
an unimplemented CSR are no-ops by definition). Every tdata2/tdata3
read and write in the install/update/read/uninstall paths is
converted to these helpers.
2. On the install and update paths, rejects requests that program
a non-zero trig_tdata2 or trig_tdata3 into an unimplemented CSR
with SBI_ERR_NOT_SUPPORTED, matching the SBI spec
wording in sections 19.4 / 19.5:
"One of the trigger configuration can't be programmed due to
unimplemented optional bits in tdata1, tdata2, or tdata3
CSRs."
Implementation status is probed once per call via the
tdata_implemented() helper. This only catches the "whole CSR
unimplemented" case; tied-off WARL bits inside an otherwise-
implemented CSR are not caught here and would require programming
the trigger and reading the value back for comparison, which can
be addressed separately.
3. Enable tdata3 configuration in the debug trigger install path.
References:
- RISC-V Debug Specification, Chapter 5 (Sdtrig), sections 5, 5.7,
5.7.3, 5.7.4, 5.7.17.
- RISC-V SBI Specification v3.0, Chapter 19 (Debug Triggers
Extension), sections 19.4, 19.5.
Fixes: 97f234f ("lib: sbi: Introduce the SBI debug triggers extension support")
Suggested-by: Nicholas Piggin <npiggin@gmail.com>
Suggested-by: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>
Reviewed-By: Himanshu Chauhan <himanshu.chauhan@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260616170118.3515676-1-david.garcia@aheadcomputing.com
Signed-off-by: Anup Patel <anup@brainfault.org>
In preparation for subsequent patches. Signed-off-by: Bo Gan <ganboing@gmail.com> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260609060024.706-2-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
The load/store address offset between the uptrap and the orig_trap can be derived by orig_trap->tval - uptrap->tval, thus refactor the function prototype for simplicity. For vector load, sbi_misaligned_v_tinst_fixup is introduced. There's no transformed instruction for vector load/store, so null out tinst if the fault is not a guest-page fault. Signed-off-by: Bo Gan <ganboing@gmail.com> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260609060024.706-3-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
sbi_load/store_loop read/write variable-length buffer unprivileged. Both function use the widest aligned 8/4/2/1 byte load/stores in each loop to reduce the total number of iterations. Also switch the scalar/vector misaligned handlers to make use of such functions to simplify code. Miscellaneous: remove the unnecessary [taddr] in inline assembly Signed-off-by: Bo Gan <ganboing@gmail.com> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260609060024.706-4-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
Fix the following issues with misaligned vector load/store: a. Stack overflow: the mask[VLEN_MAX / 8] variable consumes 8K stack space, given VLEN_MAX=65536, overflowing the default-sized stack. There's no need to fetch the whole mask in one go, instead, make it on-demand. Use a 128-byte mask as local buffer to hold the sliding window of mask. For rvv load, this is allowed -- from the spec: "The destination vector register group for a masked vector instruction cannot overlap the source mask register (v0), unless the destination vector register is being written with a mask value (e.g., compares) or the scalar result of a reduction" We don't need to worry about the mask getting overwritten. b. Maintain the value of vstart upon abort (uptrap) to avoid duplicate work. After fault resolution, the instruction can restart from the faulting vstart. For Fault-Only-First loads, reset vstart to 0, as previously done so, to conform to spec. c. Explicitly set VS dirty in VSSTATUS with SET_VS_DIRTY() if faulting from V=1, and if any vector register, including vstart/vl/vtype, gets changed in the handler. It can add 1 unnecessary op to set VS dirty in M/SSTATUS (not VSSTATUS), where the HW already did, but for code simplicity, do it anyway. The overhead should be negligible. Signed-off-by: Bo Gan <ganboing@gmail.com> Tested-by: Anirudh Srinivasan <asrinivasan@oss.tenstorrent.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260609060024.706-5-ganboing@gmail.com Signed-off-by: Anup Patel <anup@brainfault.org>
When JTAG is connected, internal logic leads to the bit MIPS_CTL0_DBG_RST_DASRT (for the debug unit) stay high and this prevents normal cluster power-up. Force proper power-on reset value prior to power-up sequence. Hold this value for about 10 usec Signed-off-by: Vladimir Kondratiev <vladimir.kondratiev@mobileye.com> Link: https://lore.kernel.org/r/20260618103713.2588984-1-vladimir.kondratiev@mobileye.com Signed-off-by: Anup Patel <anup@brainfault.org>
PMU_AP_CORE2_IDLE_CFG and PMU_AP_CORE3_IDLE_CFG are not continuous with PMU_AP_CORE0_IDLE_CFG and PMU_AP_CORE1_IDLE_CFG. They are at PMU AP base + 0x160 and + 0x164, matching the vendor OpenSBI definitions. After fixing these addresses, the intermediate cluster offset macros are redundant now, so define the wakeup and idle registers directly as PMU_AP_BASE offsets. This makes the actual register addresses easier to inspect and compare against the vendor code. C1_RVBADDR_HI_ADDR is also corrected according to the vendor OpenSBI definition. This was tested by writing an invalid value to the corrected address, which prevents cluster1 CPUs from coming online, while doing the same with the old address does not affect SMP boot. Fixes: 1f84ec2 ("platform: generic: spacemit: add K1") Signed-off-by: Junhui Liu <junhui.liu@pigmoral.tech> Link: https://lore.kernel.org/r/20260623-k1-fix-addr-v1-1-3dbde8c03bd6@pigmoral.tech Signed-off-by: Anup Patel <anup@brainfault.org>
pmu_ctr_idx_validate() checks whether counter indices are in range using cbase + sbi_fls(cmask) < total_ctrs. Both operands are unsigned long, so a crafted cbase close to ULONG_MAX causes the addition to wrap around to a small value that passes the comparison. Once validation is bypassed, sbi_pmu_ctr_cfg_match() with the SKIP_MATCH flag uses the overflowed index directly as an array subscript into phs->active_events[], producing an out-of-bounds read in M-mode. Through the firmware-event code path, the same overflowed index reaches fw_counters_data[] and fw_counters_started, giving an attacker OOB write-zero and OOB bit-set primitives in M-mode memory. Fix pmu_ctr_idx_validate() by checking for unsigned overflow before the comparison, and add a secondary bounds check on cidx_first in the SKIP_MATCH path so that even if validation is somehow bypassed in the future, the array access remains bounded. Signed-off-by: liutong <liutong@iscas.ac.cn> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260624035049.1753003-1-liutong@iscas.ac.cn Signed-off-by: Anup Patel <anup@brainfault.org>
After relocation is done, the code is running from the load address, not the link address. Fix the comment to correctly reflect this. Signed-off-by: Zong Li <zong.li@sifive.com> Reviewed-by: Anup Patel <anup@brainfault.org> Link: https://lore.kernel.org/r/20260626022121.1885209-1-zong.li@sifive.com Signed-off-by: Anup Patel <anup@brainfault.org>
Update the OpenSBI version to 1.9 as part of release preparation. Signed-off-by: Anup Patel <anup@brainfault.org>
Add RPMI MPXY proxy driver for LOGGING service group so that S-mode can leverage LOGGING service group implemented by the platform microcontroller. Reviewed-by: Rahul Pathak <rahul.pathak@oss.qualcomm.com> Signed-off-by: Subrahmanya Lingappa <subrahmanya.lingappa@oss.qualcomm.com> Link: https://lore.kernel.org/r/20260619103424.990954-1-subrahmanya.lingappa@oss.qualcomm.com Signed-off-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
According to the RISCVuzz paper by Thomas et al., the T-Head/XuanTie C906 and C910 cores fail to ignore reserved fields in the "fence" and "fence.i" encodings and trigger illegal instruction traps if these fields are non-zero, so address that in the illegal instruction trap handler. Signed-off-by: Benedikt Freisen <b.freisen@gmx.net> Link: https://lore.kernel.org/r/20251227121802.15703-2-b.freisen@gmx.net Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn> Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Add trap-based emulation code or compatibility stubs for the following ISA extensions: Zicbom, Zicboz, Zicond, Zimop, Zawrs, Zfa, Zfhmin, Zcb, Zcmop, Zba, Zbb, Zbc, Zbs, Supm Signed-off-by: Benedikt Freisen <b.freisen@gmx.net> Link: https://lore.kernel.org/r/20251227121802.15703-3-b.freisen@gmx.net Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Add trap-based emulation code for the ISA extension Zvbb based on RVV 1.0 and Zbb. Signed-off-by: Benedikt Freisen <b.freisen@gmx.net> Link: https://lore.kernel.org/r/20251227121802.15703-4-b.freisen@gmx.net Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Signed-off-by: Xiang W <wxjstz@126.com> Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Since Zvbb operations may modify vector register state, set vector state as dirty after emulating. [ Vivian: open-coded SET_VS_DIRTY() macro; after 1.9, modify to use the one defined in <sbi/sbi_vector.h>. ] Signed-off-by: Vivian Wang <wangruikang@iscas.ac.cn> Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Sorry @wxjstz, your pull request is larger than the review limit of 150000 diff characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.