Currently, the fdt_parse_aclint_node() does not handle non-contiguous
hartid correctly and returns incorrect first_hartid and hart_count.
This is because the for-loop in fdt_parse_aclint_node() skips a hartid
for which hartindex is not available (aka corresponding CPU DT node
is disabled).
For example, on a platform with 4 HARTs (hartid 0, 1, 2, and 3) where
CPU DT nodes with hartid 0 and 2 are disabled, the fdt_parse_aclint_node()
returns first_hartid = 1 and hart_count = 3 which is incorrect.
To address the above issue, drop the sbi_hartid_to_hartindex() check
from the for-loop of fdt_parse_aclint_node().
Fixes: 5e90e54a1a ("lib: utils:Check that hartid is valid")
Reported-by: Maria Mbaye <MameMaria.Mbaye@microchip.com>
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Link: https://lore.kernel.org/r/20250606055810.237441-1-apatel@ventanamicro.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Using hsm stop in hsm wait loop causes secondary harts to be stuck
forever in OpenSBI on RISC-V platforms where HSM hart hotplug is
available and all harts come-up at the same time during system
power-on.
For example, lets say we have two harts A and B on a RISC-V platform
with HSM hart hotplug which come-up at the same time during system
power-on. The hart A enters OpenSBI before hart B hence it becomes
the primary (or cold-boot) hart whereas hart B becomes the secondary
(or warm-boot) hart. The hart A follows the OpenSBI cold-boot path
and registers hsm device before hart B enters OpenSBI. The hart B
eventually enters OpenSBI and follows the OpenSBI warm-boot path
so it will increment it's own entry_count before entering hsm wait
loop where it sees hsm device and stops itself. Later as part of
the Linux boot-up sequence, hart A issues SBI HSM start call to
bring-up hart B but OpenSBI sees entry_count != init_count for
hart B in sbi_hsm_hart_start() hence hsm_device_hart_start() is
not called for hart B resulting in hart B stuck forever in OpenSBI.
To fix the above issue, revert entry_count before doing hsm stop
in hsm wait loop.
Fixes: d844deadec ("lib: sbi: Use hsm stop for hsm wait")
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Reviewed-by: Nick Hu <nick.hu@sifive.com>
Link: https://lore.kernel.org/r/20250527124821.2113467-1-apatel@ventanamicro.com
Signed-off-by: Anup Patel <anup@brainfault.org>
OpenSBI only parses MSI information of the first next level subdomain
for now, which makes the root domain misconfigured in some case:
1. the msi is not enabled on the first subdomain of the root domain,
but other subdomains enable MSI.
2. the root domain is set as direct mode, but its subdomains enable MSI.
So it is needed to parse all child of the root domain, Otherwise, the
some non-root domains are broken. As the specification says, it is
safe to parse the MSI information of all its subdomain and write the
msiaddrcfg register of the non root domain as they are read only.
Parse the aplic MSI information recursively for all aplic device.
Reviewed-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Inochi Amaoto <inochiama@gmail.com>
Link: https://lore.kernel.org/r/20250523085348.1690368-1-inochiama@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Currently, when configuring a matching programmable HPM counter with
Sscofpmf being present, cidx_base > 2, and cidx_mask == 0 to monitor
either the CPU_CYCLES or INSTRUCTIONS hardware event,
sbi_pmu_ctr_cfg_match will succeed but it will configure the
corresponding fixed counter instead of the counter specified by the
cidx_base parameter.
During counter configuration, the following issues may arise:
- If the SKIP_MATCH flag is set, an out-of-bounds memory read of the
phs->active_events array would occur, which could lead to undefined
behavior.
- If the CLEAR_VALUE flag is set, the corresponding fixed counter will
be reset, which could be considered unexpected behavior.
- If the AUTO_START flag is set, pmu_ctr_start_hw will silently start
the fixed counter, even though it has already started. From the
supervisor's perspective, nothing has changed, which could be confusing.
The supervisor will not see the SBI_ERR_ALREADY_STARTED error code since
sbi_pmu_ctr_cfg_match does not return the error code of
pmu_ctr_start_hw.
The only way to detect these issues is to check the ctr_idx return value
of sbi_pmu_ctr_cfg_match and compare it with cidx_base.
Fix these issues by returning the SBI_ERR_INVALID_PARAM error code if
the cidx_mask parameter value being passed in is 0 since an invalid
parameter should not lead to a successful sbi_pmu_ctr_cfg_match but with
unexpected side effects.
Following a similar rationale, add the validation check to
sbi_pmu_ctr_start and sbi_pmu_ctr_stop as well since sbi_fls is
undefined when the mask is 0.
This also aligns OpenSBI's behavior with KVM's.
Signed-off-by: James Raphael Tiovalen <jamestiotio@gmail.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
Reviewed-by: Andrew Jones <ajones@ventanamicro.com>
Link: https://lore.kernel.org/r/20250520132533.30974-1-jamestiotio@gmail.com
Signed-off-by: Anup Patel <anup@brainfault.org>
OpenSBI is capable of emulating time CSR through an external timer
for HARTs that don't implement a full Zicntr extension. Let's add
Zicntr extension in the FDT if CSR emulation is active.
This avoids hardcoding the extension in the devicetree, which may
confuse pre-SBI bootloaders.
Signed-off-by: Yao Zi <ziyao@disroot.org>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250516133352.36617-4-ziyao@disroot.org
Signed-off-by: Anup Patel <anup@brainfault.org>
Zicntr extension specifies three read-only CSRs, time, cycle and
instret. It isn't sufficient to report Zicntr is fully supported with
only time CSR detected.
This patch introduces a bitmap to sbi_hart_features to record
availability of these CSRs, which are detected using traps. Zicntr is
reported as present if and only if three CSRs are all available on the
HARTs.
Sites originally depending on SBI_HART_EXT_ZICNTR for detecting
existence of time CSR are switched to detect SBI_HART_CSR_TIME instead.
Suggested-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Yao Zi <ziyao@disroot.org>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250516133352.36617-3-ziyao@disroot.org
Signed-off-by: Anup Patel <anup@brainfault.org>
It seems that current implementation doesn't fail on fdt_mpxy_init(),
because platforms might not have any MPXY devices. In fact, if there are
no MPXY devices, fdt_driver_init_all() will return SBI_OK.
More importantly, if there is any MPXY device which fails the
initialization, OpenSBI must check the error code and stop the booting.
Thus, this commit adds the return value for fdt_mpxy_init().
Signed-off-by: Alvin Chang <alvinga@andestech.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250430091007.3768180-1-alvinga@andestech.com
Signed-off-by: Anup Patel <anup@brainfault.org>
The P2A doorbell system MSI index is expected to be discovered from
device tree instead of RPMI system MSI service group attribute. This
is based on ARC feedback before RPMI spec was frozen.
Let's parse P2A doorbell system MSI index from device tree and also
expose it as rpmi channel attribute to RPMI client drivers.
Signed-off-by: Anup Patel <apatel@ventanamicro.com>
Link: https://lore.kernel.org/r/20250512083827.804151-3-apatel@ventanamicro.com
Signed-off-by: Anup Patel <anup@brainfault.org>
When OpenSBI is built with a relatively new compiler (gcc-13 and greater)
I observed that GDB is unable to produce proper backtraces and some
variable values appear corrupted (even if the associated DWARF
location descriptor is correct).
Turns out that to properly work with debug information, debuggers often
need to unwind the stack. They generally rely on Call Frame Information
(CFI) records provided by the compiler to facilitate this task.
Currently, the GCC compiler offers two mechanisms:
- `.debug_frame` section (as described in the DWARF specification).
- `.eh_frame` sections (as described in LSB documents).
The latter (`.eh_frame`) supports stack unwinding at runtime, providing
a framework for C++ exceptions or enabling backtrace generation using
libraries like libunwind. However, the downside of this approach is that
these sections should be part of loadable segments.
The former (`.debug_frame`) is simply an ordinary debug section.
Starting from GCC 13, Linux targets enable the `-fasynchronous-unwind-tables`
and `-funwind-tables` flags by default. Relevant commit:
https://github.com/gcc-mirror/gcc/commit/3cd08f7168
When these flags are active, the compiler generates `.eh_frame` sections
instead of `.debug_frame`. Since OpenSBI is built using the **Linux
toolchain**, this behavior applies to OpenSBI as well.
The problem arises because the SBI build system uses `-Wl,--gc-sections`,
which discards the `.eh_frame` section.
Possible Fixes:
1. Enforce `.debug_frame` generation – Modify compiler flags to generate
`.debug_frame` instead of `.eh_frame`.
2. Preserve `.eh_frame` in the linker script – Add `KEEP(*(.eh_frame))`
to ensure the section is not discarded.
I chose Option 1 because it avoids any runtime overhead.
Signed-off-by: Parshintsev Anatoly <anatoly.parshintsev@syntacore.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250421124729.36364-1-anatoly.parshintsev@syntacore.com
Signed-off-by: Anup Patel <anup@brainfault.org>
If we hotplug a core and then perform a suspend-to-RAM operation on a
multi-core platform, the hotplugged CPU may be woken up along with the rest
of the system, particularly on platforms that wake all cores from the
deepest sleep state. When this happens, the hotplugged CPU enters the
sbi_hsm_wait WFI wait loop instead of transitioning into a
platform-specific low-power state. To address this, we add a HSM stop call
within the wait loop. This allows platforms that support HSM stop to enter
a low-power state when the CPU is unexpectedly woken up.
Signed-off-by: Nick Hu <nick.hu@sifive.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250418064506.15771-1-nick.hu@sifive.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Most CSRs are XLEN bits wide, but some are 64 bit, so rv32 needs two
accesses, plaguing the code with ifdefs.
Add new helpers that split 64 bit operation into two operations on rv32.
The helpers don't use "csr + 0x10", but append "H" at the end of the csr
name to get a compile-time error when accessing a non 64 bit register.
This has the downside that you have to use the name when accessing them.
e.g. csr_read64(0x1234) or csr_read64(CSR_SATP) won't compile and the
error messages you get for these bugs are not straightforward.
Reviewed-by: Anup Patel <anup@brainfault.org>
Signed-off-by: Radim Krčmář <rkrcmar@ventanamicro.com>
Link: https://lore.kernel.org/r/20250429142549.3673976-3-rkrcmar@ventanamicro.com
Signed-off-by: Anup Patel <anup@brainfault.org>
In current implementation, the length of hartindex_to_context_table[]
array is fixed as SBI_HARTMASK_MAX_BITS. However, the number of harts
supported by the platform might not be SBI_HARTMASK_MAX_BITS and is
usually smaller than SBI_HARTMASK_MAX_BITS. This means it is unnecessary
to allocate such fixed-length array here.
Precisely, current implementation always allocates 1024 bytes for
hartindex_to_context_table[128] on RV64 platform. However, a platform
supports two harts only needs hartindex_to_context_table[2], which only
needs 16 bytes.
This commit calculates needed size of hartindex_to_context_table[]
according to supported number of harts on the platform when registering
per-domain data, so that memory usage of per-domain context data can be
reduced.
Signed-off-by: Alvin Chang <alvinga@andestech.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250326062051.3763530-1-alvinga@andestech.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Switch all existing platform overrides to use the helper pattern instead
of the platform hooks. After this commit, only the .match_table and
.init members of struct platform_override are used.
There are two minor behavioral differences:
- For Allwinner D1, fdt_add_cpu_idle_states() is now called before the
body of generic_final_init(). This should have no functional impact.
- For StarFive JH7110, if the /chosen/starfive,boot-hart-id property is
missing, the code now falls back to using generic_coldboot_harts,
instead of accepting any hart.
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250325234342.711447-7-samuel.holland@sifive.com
Signed-off-by: Anup Patel <anup@brainfault.org>
Currently the generic platform follows the middleware pattern: it
implements the sbi_platform hooks, while providing its own set of hooks
for further customization. This has a few disadvantages: each location
where customization is needed requires a separate platform_override
hook, including places where the generic function does nothing except
forward to a platform_override hook, and the extra layer of function
pointers adds runtime overhead.
Let's restructure the generic platform to follow the helper pattern.
Allow platform overrides to treat the generic platform as a template,
adding or replacing the sbi_platform_operations as needed. Export the
generic implementations, so they can be called as helpers from inside
the override functions. With this pattern, the platform_override
function pointers are replaced by direct calls, and the forwarding
functions can be removed.
The forwarding functions are not exported, since there is no reason for
an override to call them. generic_vendor_ext_check() must be rewritten,
since now there is a new way to override vendor_ext_provider.
Signed-off-by: Samuel Holland <samuel.holland@sifive.com>
Reviewed-by: Anup Patel <anup@brainfault.org>
Link: https://lore.kernel.org/r/20250325234342.711447-6-samuel.holland@sifive.com
Signed-off-by: Anup Patel <anup@brainfault.org>
OpenSBI supports multiple supervisor domains run on same platform. When
these supervisor domains want to communicate with OpenSBI through MPXY
channels, they will allocate MPXY shared memory from their own memory
regions. Therefore, the MPXY state data structure must be per-domain and
per-hart data structure.
This commit registers per-domain MPXY state data in sbi_mpxy_init(). The
original MPXY state allocated in scratch region is also removed. We also
replace sbi_scratch_thishart_offset_ptr() macro as new
sbi_domain_mpxy_state_thishart_ptr() macro which gets MPXY state from
per-domain data.
Signed-off-by: Alvin Chang <alvinga@andestech.com>
Reviewed-by: Yu-Chien Peter Lin <peter.lin@sifive.com>
Reviewed-by: Samuel Holland <samuel.holland@sifive.com>
Link: https://lore.kernel.org/r/20250325071314.3113941-1-alvinga@andestech.com
Signed-off-by: Anup Patel <anup@brainfault.org>
