BUG and BUG_ON are not informative and are rather lazy interfaces, only
telling the user that something went wrong in a given function, but not
what, requiring the user to find the sources corresponding to their
firmware (which may not be available) and figure out how that BUG(_ON)
was hit. Even SBI_ASSERT in its current form, which does include the
condition that triggered it in the output, isn't necessarily very
informative. In some cases, the error may be fixable by the user, but
they need to know the problem in order to have any hope of fixing it.
It's also a nuisance for developers, whose development trees may have
changed significantly since the release in question being used, and so
line numbers can make it harder for them to understand which error case
a user has hit.
This patch introduces a new sbi_panic function which is printf-like,
allowing detailed error messages to be printed to the console. BUG and
BUG_ON are removed, since the former is just a worse form of sbi_panic
and the latter is a worse version of SBI_ASSERT. Finally, SBI_ASSERT is
augmented to take a set of arguments to pass to sbi_panic on failure,
used like so (sbi_boot_print_hart's current error case, which currently
manually calls sbi_printf and sbi_hart_hang):
SBI_ASSERT(xlen >= 1, ("Error %d getting MISA XLEN\n", xlen));
The existing users of BUG are replaced with calls to sbi_panic along
with informative error messages. BUG_ON and SBI_ASSERT were unused (and,
in the case of SBI_ASSERT, remain unused).
Many existing users of sbi_hart_hang should be converted to use either
sbi_panic or SBI_ASSERT after this commit.
Signed-off-by: Jessica Clarke <jrtc27@jrtc27.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
Reviewed-by: Atish Patra <atishp@rivosinc.com>
A recent commit (b28f0700) turns to read from CSR_MCOUNTINHIBIT after
checking the SBI_HART_HAS_MCOUNTINHIBIT HART feature, which leaves the
mctr_inhbt uninitialized in pmu_ctr_find_hw().
As a result, compiler will complain:
error: 'mctr_inhbt' may be used uninitialized in this function.
This commit resolves the issue by assigning an initial value to mctr_inhbt.
Signed-off-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Xiang W <wxjstz@126.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
We should clear A bits in prot variable before enabling A_NA4 or A_NAPOT.
Signed-off-by: Xiang W <wxjstz@126.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
We should ensure that pmpcfg0.pmp0cfg is set to zero before using
pmpaddr0 CSR for detecting implemented PMP address bits.
Fixes: bf21632860 ("lib: sbi: Detect PMP granularity and number
of address bits")
Signed-off-by: Vasan VS <vasan.vs@gmail.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Some platforms such as hifive unmatched doesn't implement mcountinhibit
csr. However, it has hardware events that can be monitored using 2
hpmcounter it has (i.e. mhpmcounter3 & mhpmcounter4).
Currently, PMU extension disabled if mcountinhibit is absent. That's not
really necessary as long as the supervisor OS keeps track of the delta
value of the counters. Without mcountinhibit, the delta value won't be
entirely accurate because the counters are freely running. However, that
should be fine to produce an approximate counter value which can help
performance analysis. Perf sampling won't work though as sscof extension
is not present in hifive unmatched.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
The mhpm bits represent the number of bits available in mhpmcounter
while counter width describes a zero indexed value. Fix the counter width
calculation.
Fixes: 13d40f21d5 ("lib: sbi: Add PMU support")
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
The hardware solely relies on the event selector value in mhpmevent
to figure out what event to monitor using that counter. It should be
reset when counter reset happens.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
A platform may use programmable counters for cycle/instret events.
The priv spec allows that provided that cycle/instret also report those
events in addition to the programmable counters. We should allow that
functionality in OpenSBI.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
OpenSBI doesn't use any counters for its own usage. Thus, all the counters
can be made accessible for lower privilege mode always. However, the
mcountinhibit must be set so that the counter doesn't increment.
As a result, we don't have to enable/disable mcounteren at every start/stop.
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
This patch adds sscofpmf extension in pmu module which includes
following things.
1. Enable overflow irq when starting a counter.
2. Setting the correct event filters passed from supervisor.
3. Delegating the overflow interrupt to the supervisor.
4. Add RV32 support for sscofpmf.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Sscofpmf ISA extension introduces PMU counter overflow and filtering support.
It introduces a read only `scountovf` csr that can be used to detect if
a hart supports this extension at runtime. However, this feature is only
useful if the hart already supports mcounteren and mcountinhibit.
Add a dynamic detection mechanism and boot time print message if sscofpmf
is present.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
The Sscofpmf extension introduces mhpmevent[h] csrs to handle filtering
/overflow bits in RV32. There is no way to read/write mcountinhibit
using mcountinhibit csr using a variable.
Updated the support to read/write mhpmevent[h] and mcountinhibit csr.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Moving the sbi_boot_print_general() call after the
sbi_platform_final_init() call allows to print devices initialized in the
latter.
To keep the overall print sequence the same also move
sbi_boot_print_domains().
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
The parameter passed to HFENCE.GVMA instruction in rs1 register
is guest physical address right shifted by 2 (i.e. divided by 4).
Unfortunately, we overlooked the semantics of rs1 registers for
HFENCE.GVMA instruction and never right shifted guest physical
address by 2. This issue did not manifest for hypervisors till
now because all H-extension implementations (such as QEMU, Spike,
Rocket Core FPGA, etc) we tried till now were conservatively
flushing everything upon any HFENCE.GVMA instruction.
This patch fixes GPA passed to __sbi_hfence_gvma_vmid_gpa()
and __sbi_hfence_gvma_gpa() functions.
Fixes: 331ff6a162 ("lib: Support stage1 and stage2 tlb flushing")
Reported-by: Ian Huang <ihuang@ventanamicro.com>
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
Reviewed-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Although we have PRILX to help us print unsigned long without
considering the 32bit/64bit differences, there are still some
places using 08lx and 016lx manually --- leading to redundant code.
This commit fixes the issue by using PRILX all the time.
Signed-off-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
When we are doing opensbi development, we want to know the build time
and compiler info for debug purpose.
To enable this message, please add "BUILD_INFO=y", like:
```
make BUILD_INFO=y
```
NOTE: Using `BUILD_INFO=y` without specifying SOURCE_DATE_EPOCH will
violate "reproducible builds". So it's ONLY for development and debug
purpose, and should NOT be used in a product which follows "reproducible
builds".
Signed-off-by: Wei Fu <wefu@redhat.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
The SBI specification requires that sbi_system_reset() returns
SBI_ERR_INVALID_PARAM if reset_type or reset_reason are not valid.
Signed-off-by: Heinrich Schuchardt <heinrich.schuchardt@canonical.com>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
Instead of saving context only for default non-retentive suspend,
we should save context for all non-retentive suspend types.
Fixes: 74756891cc ("lib: sbi: Implement SBI HSM suspend function")
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Xiang W <wxjstz@126.com>
Let's make system_reset_check returning priority instead of only
true/false. In that case 0 - means not supported, and anything above
means priority that makes existing reset handlers being used in first
place, unless it is decided to lower their priority.
The handler with the most priority wins.
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
To support different handlers for different types of resets, we are
adding a sbi_list of restart handlers.
Instead of sbi_system_reset_set_device we use
sbi_system_reset_add_device to reflect the actual meaning.
Signed-off-by: Nikita Shubin <n.shubin@yadro.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
We now have frequency of the timer device provided by the platform
support so we can emulate desired delay using a loop where the number
loop iterations are based on timer frequency.
This patch provides sbi_timer_delay_loop() for above purpose.
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Xiang W <wxjstz@126.com>
We now have frequency in timer device instance provided by platform
so let's print timer frequency as part of the boot prints.
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Xiang W <wxjstz@126.com>
We use BUG() macro in csr_read_num(), csr_write_num(), and
misa_string() functions for unhandled cases.
Signed-off-by: Xiang W <wxjstz@126.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
The original version of ctz will cause an endless loop, if the parameter
passed in is 0. This commit fixes this bug.
Signed-off-by: Xiang W <wxjstz@126.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
sbi_platform_get_features_str() uses sbi_snprintf() to construct the
features_str. However, it passes the wrong length value (i.e., the nfstr),
which should be (nfstr-offset) as the starting point of str (i.e.,
features_str + offset) changes.
This commit also checks the return value of snprintf, and handles the
corner case that the string buffer is full.
Signed-off-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
As other exported fifo functions, we should check whether the fifo is
valid in sbi_fifo_is_empty and sbi_fifo_is_full. To this end, this patch
changes the retval from bool to int, and the two functions will return
SBI_EINVAL in the case the fifo is invalid.
Signed-off-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Avoid getting messages from multiple harts (using dprintf and printf)
concurrently with a spinlock serializaing calls to sbi_dprintf(),
sbi_printf() and sbi_puts()
Signed-off-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Xiang W <wxjstz@126.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
commit 764a17d852 ("lib: sbi: Implement firmware counters") added
switch cases for CAUSE_LOAD_ACCESS and CAUSE_STORE_ACCESS. This caused
them to stop being redirected to U or S mode, as that is handled in the
default switch case. As a result, an error in userspace could cause the
system to hang. Fix this by allowing the acces fault case to fall
through to the default case.
Fixes: 764a17d852 ("lib: sbi: Implement firmware counters")
Signed-off-by: Samuel Holland <samuel@sholland.org>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
In sbi_ecall_rfence_handler, it will compare the funcid with
REMOTE_HFENCE_GVMA and REMOTE_HFENCE_VVMA_ASID. Later it check
whether the misa includes H-extension. This checking is incomplete
which misses REMOTE_HFENCE_GVMA_VMID and REMOTE_HFENCE_VVMA.
Fix the issue by updating the checking range.
Signed-off-by: Dong Du <Dd_nirvana@sjtu.edu.cn>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
The sbi_pmu_exit() crashes on systems not having MCOUNTINHIBIT csr
so to fix this we check SBI_HART_HAS_MCOUNTINHIBIT feature in
sbi_pmu_exit() and do nothing if it is not available.
Fixes: 13d40f21d5 ("lib: sbi: Add PMU support")
Signed-off-by: Anup Patel <anup.patel@wdc.com>
Follow the standard conventon for static function names:
All global functions should be start with sbi_<module name>_
All static functions should be start with <module name>_
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
RISC-V SBI v0.3 specification defines a set of firmware events that can
provide additional information about the current firmware context. All
of the firmware event monitoring are enabled now. The firmware
events must be defined as raw perf event with MSB set as specified in the
specification.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
RISC-V SBI specfication 0.3 defines a PMU extension that allows supervisor
mode to start/stop/configure pmu related events. This patch implements
all of the functionality defined in the specification.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
RISC-V SBI v0.3 specification defined a PMU extension to configure/start/stop
the hardware/firmware pmu events.
Implement PMU support in OpenSBI library. The implementation is agnostic of
event to counter mapping & mhpmevent value configuration. That means, it
expects platform hooks will be used to set up the mapping and provide
the mhpmevent value at runtime.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Currently, csr_read/write_num functions are used to read/write PMP related
CSRs where CSR value is decided at runtime. Expand this function to include
PMU related CSRs as well.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Currently, all bits in mcountern are enabled unconditionally at boot time.
With SBI PMU extension, all the programmable counters should enabled only
during performance monitoring for a particular event. However, this is done
only if mcountinhibit is implemented because the supervisor mode can not
start/stop any event without mcountinhibit.
Similarly, supervisor should take care enabling scounteren which allows
U-mode to access programmable pmu counters. All the non-programmable ones
(CY, TM, IR) should be enabled in M-mode because some userspace may rely on
builtins such as __builtin_readcyclecounter. Supervisor OS can still disable
them during initial configuration.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
RISC-V privilege specification allows the implementation to have less
than 64 bits.
Add a function to detect the number of implemented bits in mhpmcounter
dynamically at runtime.
Reviewed-by: Xiang W <wxjstz@126.com>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
RISC-V ISA specification v1.11 defined mcountinhibit CSR that allows
software to stop any counter from incrementing. The SBI PMU extension
depends on this CSR support in hardware.
Define mcountinhibit as a hart specific feature and detect it at runtime.
Reviewed-by: Anup Patel <anup.patel@wdc.com>
Signed-off-by: Atish Patra <atish.patra@wdc.com>
Don't assign an unused value to variable index.
Use operator '-=' where applicable.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
Compiling wake_coldboot_harts() with GCC 11 and -Wextra yields:
lib/sbi/sbi_init.c:208:27:
error: comparison of integer expressions of different signedness:
‘int’ and ‘u32’ {aka ‘unsigned int’} [-Werror=sign-compare]
208 | for (int i = 0; i <= sbi_scratch_last_hartid(); i++) {
| ^~
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
Compiling sbi_tlb_entry_process() with GCC 11 and -Wextra yields:
lib/sbi/sbi_tlb.c: In function ‘sbi_tlb_process_count’:
lib/sbi/sbi_tlb.c:206:31:
error: comparison of integer expressions of different signedness:
‘u32’ {aka ‘unsigned int’} and ‘int’ [-Werror=sign-compare]
206 | if (deq_count > count)
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Atish Patra <atish.patra@wdc.com>
Reviewed-by: Xiang W <wxjstz@126.com>
In sbi_scratch_init() we determine the last hart. The index of the last
hart cannot exceed SBI_HARTMASK_MAX_BITS - 1. We should not initialize
last_hartid_having_scratch to a higher number to avoid buffer overflows
when using this value before calling sbi_scratch_init().
When allocating scratch memory in sbi_scratch_alloc_offset() we zero out
the allocated memory for all harts except for the last one. We should not
skip the last hart.
Signed-off-by: Heinrich Schuchardt <xypron.glpk@gmx.de>
Reviewed-by: Anup Patel <anup.patel@wdc.com>
