sbi_ecall_get_extensions_str() advanced offset by the nominal extension
name length without checking remaining capacity. When the caller buffer
was smaller than the concatenated extension list, offset could pass
exts_str_size, so (exts_str_size - offset) became negative and was
passed to sbi_snprintf() as a large u32, and the trailing NUL write
could step past the caller buffer.
The helper can write beyond a caller-provided destination when the
registered extension list exceeds the supplied capacity.
Mirror the guard already used by sbi_hart_get_extensions_str(): stop
appending when the next name would not fit. Add an SBIUNIT regression
that registers several extensions into a 16-byte buffer with a redzone
and verifies no out-of-bounds write.
Closes: https://github.com/riscv-software-src/opensbi/issues/416
Signed-off-by: Yudistira Putra <pyudistira519@gmail.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260719101125.190314-1-pyudistira519@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Commit 4813a20420 ("lib: sbi_init: Call hart init and timer init
before platform early init") reordered the cold/warm boot sequence so
that sbi_timer_init() runs before sbi_platform_early_init().
The SG2042 platform workaround that merges the 16 separate timer
regions into a single domain memregion was previously performed in
early_init(). After the reordering the MTIMER driver therefore adds
the individual regions first, defeating the purpose of the combined
region (and wasting PMP entries).
Move the addition of the combined memregion into extensions_init(),
which is invoked from sbi_hart_init() and consequently still executes
before sbi_timer_init().
Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Signed-off-by: Xiang W <wangxiang@iscas.ac.cn>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260721150844.741606-1-wangxiang@iscas.ac.cn
Signed-off-by: Anup Patel <anup@brainfault.org>
Some T-Head based processors, for example the Sophgo SG2044, have a
JTLB errata that requires special handling of certain TLB maintenance
instructions (sfence.vma).
Introduce a sbi_tlb_local_operations structure so platforms can provide
custom local TLB flush implementations. Use it to implement the JTLB
workaround on affected SoCs (currently Sophgo SG2044).
Signed-off-by: Xiang W <wangxiang@iscas.ac.cn>
Signed-off-by: Han Gao <gaohan@iscas.ac.cn>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260617112925.1144190-2-wangxiang@iscas.ac.cn
Signed-off-by: Anup Patel <anup@brainfault.org>
Currently, REFERENCE_CTR and DELIVERED_CTR are always read by sending
an RPMI message to the controller. This does not scale well since
these registers are typically polled frequently by CPU frequency
governors.
The reference counter can be simply the TIME csr, so it can be read
locally without going over RPMI. If the fast channel is supported,
the delivered counter can also be emulated locally: the fast channel
exposes the current frequency, which is multiplied by the elapsed
TIME csr ticks since the last read to calculate the delivered
performance counter.
Signed-off-by: Sunil V L <sunilvl@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260703050530.2460037-3-sunilvl@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
The MPXY framework currently stores registered channels in a global
list and exposes them to all callers. However, the intended model is
that each channel is assigned to a single supervisor domain at boot and
remains owned by that domain for its lifetime.
Introduce fixed owner-domain tracking in struct sbi_mpxy_channel and
make channel lookup and enumeration domain-aware. A channel is now
visible only when accessed from its owning domain. Also require the
owner to be set before channel registration.
This allows MPXY to support systems where the same hart may be reused by
multiple domains while keeping channel ownership and visibility fixed to
the domain that owns the service.
Signed-off-by: Oza Pawandeep <pawandeep.oza@oss.qualcomm.com>
Reviewed-by: Rahul Pathak <rahul.pathak@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260721215758.695736-1-pawandeep.oza@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Fix sbi_irqchip_register_msi() that prevented contiguous EIID allocation:
- Gap check condition was inverted: the original `h->first_hwirq -
hwirq < num_hwirq` incorrectly set `found = true` when the gap was
too small to fit num_hwirq entries. Correct to `>= num_hwirq` so
allocation only proceeds when sufficient space exists between
registered handlers.
- Tail-space check `!found && !hwirq` never triggered after iteration
Replace with `(chip->num_hwirq - hwirq) >= num_hwirq` to correctly
allocate if handler list is empty.
Fixes: 79e63bc834 ("irqchip: add sbi_irqchip_register_msi support")
Signed-off-by: Oza Pawandeep <pawandeep.oza@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260721214833.687361-7-pawandeep.oza@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Add an irq_state field to struct sbi_irqchip_hwirq_data with a single
IRQ_ENABLED flag (bit 0) to track whether a hardware interrupt has been
enabled via the irqchip framework. Set IRQ_ENABLED in
sbi_irqchip_unmask_hwirq() when the unmask callback is invoked.
Add sbi_irqchip_get_irq_state() as a private inline accessor and expose
sbi_irqchip_is_irq_enabled() as a public API for drivers to query the
enabled state of a hardware interrupt by chip pointer and hwirq number.
Refactor imsic_local_eix_update() to operate on a single interrupt ID
instead of a base+count range, simplifying the CSR bit manipulation to
a direct BIT(id) write without the inner loop. Update all call sites
accordingly.
Use sbi_irqchip_is_irq_enabled() in imsic_warm_irqchip_init() to
restore per-EIID EIE CSR state on warm boot and HSM resume based on
the saved irq_state, replacing the previous blanket disable of all
interrupts. This ensures that EIIDs enabled during hotplug/warminit
cycle are correctly re-enabled on the resuming hart without requiring
software to re-register or re-unmask each interrupt.
Signed-off-by: Oza Pawandeep <pawandeep.oza@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260721214833.687361-6-pawandeep.oza@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Rewrite imsic_process_hwirqs() to use container_of() for instance
lookup, validate ownership via imsic_current_hart_owner(), mask
IMSIC_TOPEI_ID_MASK on the raw CSR value, and dispatch non-IPI EIIDs
through sbi_irqchip_process_hwirq() instead of an open-coded switch.
Rewrite imsic_warm_irqchip_init() to resolve the imsic_data instance
via container_of() from the sbi_irqchip_device pointer, and skip
initialization silently for harts not owned by this instance.
Extend the sbi_irqchip callback set:
- hwirq_setup: registers sbi_irqchip_raw_handler_default for the EIID
- hwirq_cleanup: clears EIE and EIP bits for the EIID on the owner hart
- hwirq_eoi: no-op as claim/ack is already performed by CSR_MTOPEI
read in imsic_process_hwirqs()
- hwirq_set_affinity: reprograms the upstream MSI source via
imsic_program_msi() to retarget the interrupt to the new hart
- hwirq_mask: clears the EIE bit for the EIID on the owner hart
- hwirq_unmask: sets the EIE bit for the EIID on the owner hart,
skipping IPI (EIID 1) and non-owner harts
Add imsic_compose_msi_msg() to compute the MMIO target address for a
given hart and EIID by walking the imsic_regs array and applying file
and guest index offsets.
Add imsic_program_msi() that composes an MSI message
and delivers it to the upstream interrupt source via
sbi_irqchip_write_msi(), enabling APLIC TARGET register programming
from the IMSIC driver.
Migrate imsic_cold_irqchip_init() to the instance-based model:
preserve any pre-configured target_harts mask set by platform or FDT
code before applying the imsic_device template, falling back to
sbi_hartmask_set_all() only when no mask was provided. Register the
irqchip and reserved EIIDs on the embedded irqchip rather than the
global singleton.
Signed-off-by: Oza Pawandeep <pawandeep.oza@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260721214833.687361-5-pawandeep.oza@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Add parent_unique_id fields to struct aplic_data for
IMSIC parent linking in MSI mode. Store parent IMSIC unique_id
during FDT parsing of the APLIC node.
Add aplic_is_msi_mode() complementing aplic_is_direct_mode() to
consolidate delivery mode detection. Add APLIC_TARGET_EIID() macro
for packing the EIID field into the TARGET register.
Add parent_unique_id and parent_irq_map fields to struct aplic_data.
parent_unique_id identifies the upstream IMSIC irqchip device resolved
via sbi_irqchip_find_device() during hwirq_setup. parent_irq_map is
a per-source array allocated at cold init time to track the EIID
assigned by the IMSIC for each APLIC source.
Restore aplic_writel_msicfg() and re-introduce MSI address register
programming in aplic_init(), gated on aplic_is_msi_mode(). Set the
DOMAINCFG_DM bit to switch the hardware to MSI delivery mode when
no IDC structures are present.
Add aplic_program_msi_target() to pack hart_index, guest_index, and
EIID into the APLIC_TARGET register. Add aplic_write_msi() as the
sbi_irqchip write_msi callback that extracts EIID and hart_index from
the MSI message and calls aplic_program_msi_target(). Add
aplic_msi_callback() as the MSI receive callback that dispatches to
sbi_irqchip_process_hwirq() on the APLIC chip.
Extend aplic_hwirq_setup() with an MSI path that resolves the parent
IMSIC chip by parent_unique_id, registers an MSI route via
sbi_irqchip_register_msi(), and stores the allocated EIID in
parent_irq_map for the source being configured.
Extend aplic_hwirq_set_affinity() with an MSI path that delegates
affinity reprogramming to the parent IMSIC chip via
sbi_irqchip_set_affinity() using the stored parent_irq_map entry.
Guard warm_init, process_hwirqs, and hwirq_eoi with early returns in
MSI mode as interrupt delivery and acknowledgement are handled by the
IMSIC in that configuration.
Signed-off-by: Oza Pawandeep <pawandeep.oza@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260721214833.687361-3-pawandeep.oza@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Add inline register accessors (aplic_sourcecfg_write/read,
aplic_target_write, aplic_irq_setie/clrie/clrip, aplic_idc_base/
read/write) to consolidate all MMIO access behind typed helpers
and eliminate open-coded address arithmetic throughout the driver.
Add field-packing macros APLIC_TARGET_HART_IDX, APLIC_TARGET_IPRIO,
and APLIC_TARGET_GUEST_IDX for constructing
APLIC_TARGET register values.
Add aplic_is_direct_mode() mode helper, aplic_hwirq_flags_to_sourcecfg()
to map generic hwirq flags to APLIC source modes, aplic_hwirq_is_delegated()
to detect S-mode delegated sources, aplic_find_idc_index() to map a
hart index to its IDC slot, and aplic_first_target_hart() to select
the first available IDC hart for initial interrupt targeting.
Implement the full sbi_irqchip callback set for direct (IDC) mode:
- warm_init: enables IDC interrupt delivery and threshold per hart
- process_hwirqs: reads TOPI and dispatches via sbi_irqchip_process_hwirq,
skipping delegated sources
- hwirq_setup: programs sourcecfg from hwirq_flags and sets the TARGET
register to the first available IDC hart
- hwirq_cleanup: clears IE, IP, sourcecfg, and TARGET on teardown
- hwirq_eoi: reads CLAIMI to acknowledge the interrupt on the IDC
- hwirq_set_affinity: reprograms TARGET to the specified hart index
- hwirq_mask/unmask: clears/sets IE via CLRIENUM/SETIENUM
Register the irqchip device and populate target_harts from idc_map
only when targets_mmode is set in aplic_cold_irqchip_init().
Co-developed-by: Raymond Mao <raymond.mao@riscstar.com>
Signed-off-by Raymond Mao <raymond.mao@riscstar.com>
Signed-off-by: Oza Pawandeep <pawandeep.oza@oss.qualcomm.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260721214833.687361-2-pawandeep.oza@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Currently, the trap delegation setup is part of sbi_hart.c and
called via sbi_hart_init() whereas the per-hart trap (aka exception
and interrupt) handling is part of sbi_trap.c.
Move trap delegation init to sbi_trap.c and call it via new
sbi_trap_init() from both cold boot and warm boot path. This
way trap delegation setup is in same place as trap handling.
Signed-off-by: Anup Patel <anup.patel@oss.qualcomm.com>
Link: https://lore.kernel.org/r/20260613132129.2135128-2-anup.patel@oss.qualcomm.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Commit 37eaca4ab3 ("lib: sbi_ipi: Return error for invalid hartids")
added a weight check against the HSM-interruptible mask, which excludes
harts in STOPPED state. This causes sbi_ipi_send_many() to return
SBI_EINVAL when the kernel sends a remote fence (e.g. TLB shootdown)
whose hmask includes a hart that was stopped by CPU hotplug:
__sbi_rfence_v02_call: hbase = [0] hmask = [0x3] failed (error [-22])
Validate requested hartids against the domain's assigned hartmask
instead. This still rejects truly invalid or cross-domain hartids, but
allows HSM-STOPPED harts to be silently skipped — which is safe because
a stopped hart will do a full local TLB flush on its next HSM start
before re-entering S-mode.
Fixes: 37eaca4ab3 ("lib: sbi_ipi: Return error for invalid hartids")
Signed-off-by: Chen Pei <cp0613@linux.alibaba.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260612115208.118132-1-cp0613@linux.alibaba.com
Signed-off-by: Anup Patel <anup@brainfault.org>
When called with first == NULL, sbi_irqchip_find_device_by_caps() should
scan the device list from the start. Instead it always returns NULL: "found"
starts false and only flips true when an entry equals "first", but no entry
ever equals NULL, so every entry hits "else continue" and nothing is checked.
This breaks MSI detection in sbi_mpxy, which calls it with first == NULL:
ms->msi_avail = !!sbi_irqchip_find_device_by_caps(SBI_IRQCHIP_CAPS_MSI, NULL);
msi_avail is therefore always false, so mpxy_write_std_attr() silently drops
the MSI attributes (MSI_ADDR_LO/HI, MSI_DATA, MSI_CONTROL) while still
returning success, and MSI-based MPXY notifications are never delivered.
Initialize "found" from "first" so a NULL "first" scans from the beginning,
and always continue in the pre-match branch so a non-NULL "first" resumes
after the given device.
Fixes: 8570b93844 ("lib: sbi_irqchip: Allow irqchip drivers advertise capabilities")
Signed-off-by: David E. Garcia Porras <david.garcia@aheadcomputing.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260701181334.969877-1-david.garcia@aheadcomputing.com
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>
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: 1f84ec2a ("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>
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>
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>
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>
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: 97f234f15c ("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>
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>
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: 7219477f7b ("lib: Use MTINST CSR in misaligned load/store emulation")
Fixes: b5ae8e8a65 ("lib: Add misaligned load/store trap handling")
Fixes: 4c112650bb ("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>
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: c2acc5e5b0 ("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>
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: e5797e0688 ("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>
When SBI is built with a compiler that doesn't support vector, the
misaligned vector load/store emulation is not built in, the handlers are
just stubs.
Currently the stubs just return 0, causing sbi_trap_emulate_load() to
return without incrementing mepc, meaning the instruction will just
fault again, an infinite loop.
Fix the stubs to use sbi_trap_redirect(), which forwards the trap to the
previous mode, allowing it to be handled there.
Fixes: c2acc5e5 ("lib: sbi_misaligned_ldst: Add handling of vector load/store")
Signed-off-by: Michael Ellerman <mpe@kernel.org>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20260530-trap-redirect-v1-1-45d4d333d8c9@kernel.org
Signed-off-by: Anup Patel <anup@brainfault.org>
David E. Garcia Porras