| Age | Commit message (Collapse) | Author |
|---|
|
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 <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
sbi_pmu_ctr_cfg_match() only acts on SBI_PMU_CFG_FLAG_CLEAR_VALUE and
SBI_PMU_CFG_FLAG_AUTO_START when the event type is SBI_PMU_EVENT_TYPE_HW.
However, pmu_ctr_find_hw() allocates a hardware counter from the same
hw_event_map for SBI_PMU_EVENT_TYPE_HW_CACHE, SBI_PMU_EVENT_TYPE_HW_RAW,
and SBI_PMU_EVENT_TYPE_HW_RAW_V2 as well, and the start/clear helpers
(pmu_ctr_start_hw, pmu_ctr_write_hw) operate on the counter index alone
and are agnostic to the event type. As a result, when a supervisor
configures a HW_CACHE/HW_RAW/HW_RAW_V2 event with these flags, the
counter is programmed and recorded in active_events[] but is never
cleared or started, requiring an extra SBI call to make it count.
Extend the check to cover all hardware-counter event types so that
the configuration flags take effect for HW_CACHE and raw events too.
Deliberately avoiding using "not FW" logic to be explicit about
HW-backed events only.
Fixes: 13d40f21 ("lib: sbi: Add PMU support")
Signed-off-by: David E. Garcia Porras <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Currently when searching for a hardware counter for an event, if no
programmable counter is available, the code falls back to using a fixed
counter (mcycle/minstret) if one matches the event.
However the fallback is incorrect when sscofpmf is present but
smcntrpmf is not. That's because with sscofpmf, programmable counters
support mode filtering, but the fixed counters do not (without
smcntrpmf). Even if the caller didn't configure mode filtering, by
default programmable counters don't count M mode when sscofpmf is
present, whereas mcycle/minstret do.
Fix the logic to not fallback to a fixed counter if sscofpmf is present
but smcntrpmf is not.
Fixes: 0c304b661965 ("lib: sbi: Allow programmable counters to monitor cycle/instret events")
Signed-off-by: Michael Ellerman <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
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: 51951d9e9af8 ("lib: sbi_pmu: Implement sbi_pmu_counter_fw_read_hi")
Signed-off-by: James Raphael Tiovalen <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
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: 57d3aa3b0dbd ("lib: sbi_pmu: Introduce fw_counter_write_value API")
Signed-off-by: James Raphael Tiovalen <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Currently, OpenSBI returns SBI_ERR_ALREADY_STARTED when attempting to
start a HW counter that is already started and SBI_ERR_ALREADY_STOPPED
when attempting to stop a HW counter that is already stopped. However,
this is not yet implemented for FW counters.
Add the necessary checks to return the same error codes when attempting
the same actions on FW counters.
Signed-off-by: James Raphael Tiovalen <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
The sbi_hart_pmp_xyz() and sbi_hart_map/unmap_addr() functions can
now be replaced by various sbi_hart_protection_xyz() functions.
Signed-off-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Before this patch sbi_pmu_ctr_start() ignores flags received in
sbi_pmu_ctr_cfg_match() including inhibit ones. To prevent it,
save flags together with event_data and use them both in
sbi_pmu_ctr_start().
Fixes: 1db95da2997b ("lib: sbi: sbi_pmu: fixed hw counters start for hart")
Signed-off-by: Shifrin Dmitry <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Previously, in sbi_pmu_ctr_cfg_match() function, ctr_idx was used immediately
after pmu_ctr_find_fw() or pmu_ctr_find_hw() calls. In first case, array index
was (ctr_idx - num_hw_ctrs), in second - ctr_idx. But pmu_ctr_find_fw() and
pmu_ctr_find_hw() functions can return negative value, in which case writing
in arrays with such indexes would corrupt sbi_pmu_hart_state structure.
To avoid this situation, direct ctr_idx value check added.
Signed-off-by: Alexander Chuprunov <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Deleted spaces before brace in pmu_ctr_start_fw() for correct alignment.
Signed-off-by: Alexander Chuprunov <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Generally, hardware performance counters can only be started, stopped,
or configured from machine-mode using mcountinhibit and mhpmeventX CSRs.
Also, in opensbi only sbi_pmu_ctr_cfg_match() managed mhpmeventX. But
in generic Linux driver, when perf starts, Linux calls both
sbi_pmu_ctr_cfg_match() and sbi_pmu_ctr_start(), while after hart suspend
only sbi_pmu_ctr_start() command called through SBI interface. This doesn't
work properly in case when suspend state resets HPM registers. In order
to keep counter integrity, sbi_pmu_ctr_start() modified. First, we're saving
hw_counters_data, and after hart suspend this value is restored if
event is currently active.
Signed-off-by: Alexander Chuprunov <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
We do not need to iterate over all values in the loop,
we can break the loop when we found a valid counter
that is not started yet.
Signed-off-by: Manuel Hernández Méndez <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Clearing MIP at that point means that we can probably lose a pending
interrupt. This should not happen, remove MIP clearing from there.
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
As soon as the SSE event is registered, there is no reason not to
delegate the interrupt. Split the PMU SSE enable/disable()
callbacks by moving MIDELEG setting to register/unregister().
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
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 <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
Andes PMU
Referencing commit 0c304b661965
("lib: sbi: Allow programmable counters to monitor cycle/instret events")
to support this functionality for Andes PMU.
Signed-off-by: Leo Yu-Chi Liang <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Anup Patel <[email protected]>
|
|
The latest specification added new high priority RAS events and renamed
the PMU to PMU_OVERFLOW.
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
|
|
Add the PMU SSE event only if an overflow irq bit is present.
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
In order to allow events to be dynamically added, remove the existing
static array of events and use a simply linked list of supported events.
This allows us to move the cb_ops into this list and associated it with
an event_id. Drivers can now register cb_ops before bringing up the sse
core to handle additional events (platform ones for instance).
sbi_sse_init() now allocates as many events as present in the linked
list. Events can now be added with sbi_sse_add_event() which allows to
add new supported events with some callback operations if any. If an
event is not to be supported, then sbi_sse_add_event() should not be
called. This approach currently consider that local events are to be
supported on all harts (ie, they all support the same ISA or
dependencies). If per-hart event availability needs to be supported,
then, an is_supported() callback could be added later and called for
each hart.
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
|
|
While sbi_pmu_irq_bit() was used to delegate irq to S-mode, LCOFIP usage
was still hardcoded in various places. This led to change the returned
value of sbi_pmu_irq_bit() to be a bit number rather than a bit mask
since it returns an 'int' and we need to obtain the bit number itself to
handle it in the IRQs handlers. Add a similar function to return the
irq mask which can also be used where the mask is required rather than
the bit itself.
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
The raw events have same event_idx. Thus, the event info sanity check
only relies on the select mask. The raw event check should be done first
so that regular hardware events can be verified using event idx range
check.
Fixes: e4345842168b ("lib: sbi_pmu: Implement SBI PMU event info function")
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
The hardware event map function invoked from platform code should
exclude any raw events as there is a separate function for the raw
events.
Fixes: d8a483fc7fd0 ("lib: sbi_pmu: PMU raw event v2 support")
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
Allow the supervisor software to query about the event using the
new function. This supports both firmware and hardware events.
The hardware event presence is verified hw_event_map which is populated
via PMU device tree node. The firmware event presence is checked through
event validation function which should take care of both standard and
platform firmware events.
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
The pmu_validate_event already has the same check for fw events.
The validate function is called prior to the current function
to find a counter for firmware. That's why, the redudant
check can be removed from the find counter function.
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
As per the updated ISA specification and SBI PMU v3.0, lower 56
bits are available for the platform to implement mhpmeventX
encoding. Implement the PMU raw event V2 support defined in SBI
v3.0 which allows more bits for platforms to encode the raw events.
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
Add SSE callbacks registration to PMU driver in order to disable
interrupt delegation for PMU interrupts. When interrupts are
undelegated send the PMU SSE event upon LCOFIP IRQ.
Signed-off-by: Clément Léger <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
If trap earlier before sbi_pmu_init done, some path would call
sbi_pmu_ctr_incr_fw, then it would go wrong:
1. if phs_ptr_offset is zero, then it get a wrong pmu state ptr
2. if phs_ptr_offset is ok, but we didn't call pmu_set_hart_state_ptr
it would be NULL POINT
Of course, the above situation will not occur at present, but it is
reasonable to check before using.
Signed-off-by: yang.zhang <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
The counter info should only return valid hardware counters for the ones
set in the counter mask. Otherwise, it will report incorrect number of
hardware counters to the supervisor if the platform has discontiguous
counters.
Fixes: c744ed77b18c ("lib: sbi_pmu: Enable noncontigous hpm event and counters")
Signed-off-by: Atish Patra <[email protected]>
Reviewed-by: Xiang W <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
OpenSBI doesn't support SBI PMU snapshot yet as there is not much benefit
unless the multiple counters overflow at the same time.
Just add the definition and return not supported error at this moment. The
default returned error is also not supported. Thus, no functional change
intended.
Reviewed-by: Anup Patel <[email protected]>
Signed-off-by: Atish Patra <[email protected]>
|
|
Add support for custom PMU extensions to set inhibit bits
on custom CSRs by introducing the PMU device callback
hw_counter_filter_mode(). This allows the perf tool to
restrict event counting under a specified privileged
mode by appending a modifier, e.g. perf record -e event:k
to count events only happening in kernel mode.
Signed-off-by: Yu Chien Peter Lin <[email protected]>
Reviewed-by: Leo Yu-Chi Liang <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
This patch makes the following changes:
- As sbi_platform_pmu_init() returns a negative error code on
failure, let sbi_pmu_init() print out the error code with
sbi_dprintf().
- In order to distinguish the SBI_EFAIL error returned by
sbi_pmu_add_*_counter_map(), return SBI_ENOENT to indicate
that fdt_pmu_setup() failed to locate "riscv,pmu" node, and
generic_pmu_init() ignores such case.
Signed-off-by: Yu Chien Peter Lin <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Reviewed-by: Lad Prabhakar <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
total_ctrs is bounded by
SBI_PMU_FW_CTR_MAX + SBI_PMU_HW_CTR_MAX) == 48
which exceeds BITS_PER_LONG on 32 bit systems.
Iterating over the bits of &cmask results in a buffer overflow when looking
for a bit >= BITS_PER_LONG.
Adjust the iterators in sbi_pmu_ctr_start() and sbi_pmu_ctr_stop()
accordingly.
Signed-off-by: Heinrich Schuchardt <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
sbi_pmu_ctr_cfg_match() receives data from a lower privilege level mode.
We must catch maliciously wrong values.
We already check against total_ctrs. But we do not check that total_ctrs is
less than SBI_PMU_HW_CTR_MAX + SBI_PMU_FW_CTR_MAX.
Check that the number of hardware counters is in the valid range.
Addresses-Coverity-ID: 1566114 Out-of-bounds write
Signed-off-by: Heinrich Schuchardt <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
After the hardware hpm counter is stopped, it should not raise any new
interrupt as it is already stopped. So add the hw_counter_disable_irq
callback to allow the custom pmu device to control this behavior.
Signed-off-by: Inochi Amaoto <[email protected]>
Reviewed-by: Guo Ren <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Tested-by: Samuel Holland <[email protected]>
|
|
When detecting features of PMU, the hpm counter may be written to some
value, this will cause some unexpected behavior in some cases. So ensure
the hpm counter is updated before starting the counter and the related
interrupt.
Signed-off-by: Haijiao Liu <[email protected]>
Co-authored-by: Inochi Amaoto <[email protected]>
Signed-off-by: Inochi Amaoto <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Tested-by: Samuel Holland <[email protected]>
|
|
This adds the support for ISA extension smcntrpmf. When some inhibit flags
are set by a lower privilege mode for new CSRs added by smcntrpmf, OpenSBI
sets the appropriate values correspondingly.
Signed-off-by: Kaiwen Xue <[email protected]>
Signed-off-by: Kaiwen Xue <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
Cycle and instructions are hardware events instead of firmware ones. Fix
the typo in the name of this function.
Signed-off-by: Kaiwen Xue <[email protected]>
Signed-off-by: Kaiwen Xue <[email protected]>
Reviewed-by: Anup patel <[email protected]>
|
|
Platforms may implement hpm events/counters non contiguously but the current
implementation assumes them to be always contigous. Add a bitmap that
captures the hpm events/counters as implemented in the hardware and use
it to set the max limit of hardware counters visible to the OS. Counters
not implemented in the hardware can't be used by the OS because those
wont be described in the DT.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
On a misconfigured system we could access phs->active_events[] out of
bounds. Check that num_hw_ctrs is less or equal SBI_PMU_HW_CTR_MAX.
Addresses-Coverity-ID: 1566113 ("Out-of-bounds read")
Addresses-Coverity-ID: 1566114 ("Out-of-bounds write")
Signed-off-by: Heinrich Schuchardt <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
Instead of using a global array for per-HART PMU state, we should
use heap to on-demand allocate per-HART PMU state when the HART
is initialized in cold boot or warm boot path.
Signed-off-by: Anup Patel <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
|
|
As per the SBI specification, we should "unconditionally select the first
counter from the set of counters specified by the counter_idx_base and
counter_idx_mask", so implement this behaviour.
Suggested-by: Atish Patra <[email protected]>
Signed-off-by: Alexandre Ghiti <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
Those events are enabled by default and should not be reset afterwards
since when using SBI_PMU_CFG_FLAG_SKIP_MATCH, it leads to unaccessible
counters after the first use.
Signed-off-by: Alexandre Ghiti <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
|
|
The bits encoded in event_idx[19:16] indicate the event type, with
an offset of 16 instead of 20.
Fixes: 13d40f21d588 ("lib: sbi: Add PMU support")
Signed-off-by: Yu Chien Peter Lin <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
Reviewed-by: Xiang W <[email protected]>
|
|
Platform specific firmware event handler may leverage the hartid to program
per hart specific registers for a given counter.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
Add fw_counter_write_value API for platform specific firmware events
which separates setting the counter's initial value from starting the
counter. This is required so that the fw_event_data array can be reused
to save the event data received.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
For all platform specific firmware event operations use the dedicated
event code (0xFFFF) when matching against the input firmware event.
Furthermore save the real platform specific firmware event code received as
the event data for future use.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
Update fw_event_validate_code, fw_counter_match_code and fw_counter_start
ops which used a 32 bit event code to use the 64 bit event data instead.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
Rename and reuse fw_counter_value array to save both the counter values
for the SBI firmware events and event data for the SBI platform specific
firmware events.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Reviewed-by: Andrew Jones <[email protected]>
|
|
This patch adds a callback to fetch the number of bits implemented for a
custom firmware counter. If the callback fails or is not implemented then
width defaults to 63.
Signed-off-by: Mayuresh Chitale <[email protected]>
Reviewed-by: Atish Patra <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
|
|
C language standard uses true/false for the boolean type.
Let's switch to that for better language compatibility.
Signed-off-by: Bin Meng <[email protected]>
Reviewed-by: Anup Patel <[email protected]>
Reviewed-by: Samuel Holland <[email protected]>
Tested-by: Samuel Holland <[email protected]>
|
