fix(prototyper): fix event comparison bug and add pmu test in test-kernel

Signed-off-by: guttatus <[email protected]>

prototyper/prototyper/src/main.rs

@@ -21,11 +21,10 @@ mod sbi;
 
 use core::arch::{asm, naked_asm};
 
-use sbi::features::hart_mhpm_mask;
-
 use crate::platform::PLATFORM;
 use crate::riscv::csr::menvcfg;
 use crate::riscv::current_hartid;
+use crate::sbi::features::hart_mhpm_mask;
 use crate::sbi::features::{
     Extension, PrivilegedVersion, hart_extension_probe, hart_features_detection,
     hart_privileged_version,
@@ -122,7 +121,12 @@ extern "C" fn rust_main(_hart_id: usize, opaque: usize, nonstandard_a2: usize) {
         medeleg::clear_load_misaligned();
         medeleg::clear_store_misaligned();
         medeleg::clear_illegal_instruction();
-        if hart_privileged_version(current_hartid()) >= PrivilegedVersion::Version1_12 {
+
+        let hart_priv_version = hart_privileged_version(current_hartid());
+        if hart_priv_version >= PrivilegedVersion::Version1_11 {
+            asm!("csrw mcountinhibit, {}", in(reg) !0b10);
+        }
+        if hart_priv_version >= PrivilegedVersion::Version1_12 {
             // Configure environment features based on available extensions.
             if hart_extension_probe(current_hartid(), Extension::Sstc) {
                 menvcfg::set_bits(

prototyper/prototyper/src/sbi/pmu.rs

@@ -18,8 +18,6 @@ const PMU_HARDWARE_COUNTER_MAX: usize = 32;
 const PMU_FIRMWARE_COUNTER_MAX: usize = 16;
 /// Marker value for inactive/invalid event indices.
 const PMU_EVENT_IDX_INVALID: usize = usize::MAX;
-/// Bit mask for fixed counters (mcycle, time, minstret).
-const PMU_FIXED_COUNTER_MASK: u32 = 0x7;
 
 /// PMU state tracking hardware and firmware performance counters
 #[repr(C)]
@@ -43,9 +41,7 @@ impl PmuState {
         let mut active_event =
             [PMU_EVENT_IDX_INVALID; PMU_HARDWARE_COUNTER_MAX + PMU_FIRMWARE_COUNTER_MAX];
         // Standard mappings for fixed counters
-        active_event[0] = 0x1; // mcycle -> HW_CPU_CYCLES
         active_event[1] = 0x0; // time (memory-mapped)
-        active_event[2] = 0x2; // minstret -> HW_INSTRUCTIONS
 
         Self {
             active_event,
@@ -359,9 +355,6 @@ impl Pmu for SbiPmu {
             if counter_idx >= pmu_state.total_counters_num {
                 return SbiRet::invalid_param();
             }
-            if !is_counter_started(pmu_state, counter_idx) {
-                return SbiRet::already_stopped();
-            }
 
             let stop_result = if counter_idx >= pmu_state.get_hw_counter_num() {
                 pmu_state.stop_fw_counter(counter_idx, is_reset)
@@ -504,7 +497,7 @@ impl SbiPmu {
         }
         // mcycle, time, minstret cannot be used for other events.
         let mhpm_mask = hart_mhpm_mask(current_hartid());
-        let can_use_counter_mask = hw_counters_mask & (!PMU_FIXED_COUNTER_MASK) & mhpm_mask;
+        let can_use_counter_mask = hw_counters_mask & mhpm_mask;
 
         // Find a counter that meets the conditions from a set of counters
         for counter_idx in CounterMask::new(counter_idx_base, counter_idx_mask) {
@@ -515,11 +508,17 @@ impl SbiPmu {
             // If the counter idx corresponding to the hardware counter index cannot be used by the event,
             // or has already been used, skip this counter idx
             let mhpm_offset = get_mhpm_csr_offset(counter_idx).unwrap();
+            // Find a unused counter
             if (can_use_counter_mask >> mhpm_offset) & 0x1 == 0
                 || pmu_state.active_event[counter_idx] != PMU_EVENT_IDX_INVALID
             {
                 continue;
             }
+            // If mcycle is selected but the event is not SBI_PMU_HW_CPU_CYCLES,
+            // or minstret is selected but the event is not SBI_PMU_HW_INSTRUCTIONS, skip
+            if (mhpm_offset == 0 && event_idx != 1) || (mhpm_offset == 2 && event_idx != 2) {
+                continue;
+            }
             // If the counter idx corresponding to the hardware counter index has already started counting, skip the counter
             if hart_privileged_version(current_hartid()) >= PrivilegedVersion::Version1_11 {
                 let inhibit = riscv::register::mcountinhibit::read();
@@ -541,8 +540,13 @@ impl SbiPmu {
         event_idx: usize,
         event_data: u64,
     ) -> Result<(), SbiRet> {
+        // If the event is SBI_PMU_HW_CPU_CYCLES and mcycle is selected,
+        // or the event is SBI_PMU_HW_INSTRUCTIONS and minstret is selected, return directly
+        if (mhpm_offset == 0 && event_idx == 1) || (mhpm_offset == 2 && event_idx == 2) {
+            return Ok(());
+        }
         // Validate counter offset range (only mhpmcounter3-31 are configurable)
-        if mhpm_offset < 3 || mhpm_offset > 31 {
+        if mhpm_offset == 1 || mhpm_offset > 31 {
             return Err(SbiRet::not_supported());
         }
 
@@ -661,27 +665,6 @@ fn get_mhpm_csr_offset(counter_idx: usize) -> Option<u16> {
     None
 }
 
-/// Checks if a counter is currently started.
-///
-/// Returns `true` if the counter is active (not inhibited), `false` otherwise.
-#[inline]
-fn is_counter_started(pmu_state: &PmuState, counter_idx: usize) -> bool {
-    if counter_idx < pmu_state.hw_counters_num {
-        // Hardware counter: Check mcountinhibit CSR
-        if hart_privileged_version(current_hartid()) >= PrivilegedVersion::Version1_11 {
-            let inhibit = riscv::register::mcountinhibit::read();
-            let mhpm_offset = get_mhpm_csr_offset(counter_idx).unwrap();
-            return (inhibit.bits() & (1 << mhpm_offset)) == 0;
-        } else {
-            return pmu_state.active_event[counter_idx] != PMU_EVENT_IDX_INVALID;
-        }
-    } else {
-        // Firmware counter: Check fw_counter_state bitmask
-        let fw_idx = counter_idx - pmu_state.hw_counters_num;
-        fw_idx < PMU_FIRMWARE_COUNTER_MAX && (pmu_state.fw_counter_state & (1 << fw_idx)) != 0
-    }
-}
-
 /// Start Hardware Counter
 enum StartCounterErr {
     OffsetInvalid,
@@ -884,70 +867,82 @@ pub struct EventIdx {
 #[allow(unused)]
 impl EventIdx {
     #[inline]
-    pub const fn new(event_idx: usize) -> Self {
+    const fn new(event_idx: usize) -> Self {
         Self { inner: event_idx }
     }
 
     #[inline]
-    pub const fn event_type(&self) -> usize {
+    fn from_firmwarw_event(firmware_event: usize) -> Self {
+        Self {
+            inner: 0xf << 16 | firmware_event,
+        }
+    }
+
+    #[inline]
+    fn raw(&self) -> usize {
+        self.inner
+    }
+
+    #[inline]
+    const fn event_type(&self) -> usize {
         (self.inner >> 16) & 0xF
     }
 
     #[inline]
-    pub const fn event_code(&self) -> usize {
+    const fn event_code(&self) -> usize {
         self.inner & 0xFFFF
     }
 
     /// Extracts the cache ID for HARDWARE_CACHE events (13 bits, [15:3])
     #[inline]
-    pub const fn cache_id(&self) -> usize {
+    const fn cache_id(&self) -> usize {
         (self.inner >> 3) & 0x1FFF
     }
 
     /// Extracts the cache operation ID (2 bits, [2:1])
     #[inline]
-    pub const fn cache_op_id(&self) -> usize {
+    const fn cache_op_id(&self) -> usize {
         (self.inner >> 1) & 0x3
     }
 
     /// Extracts the cache result ID (1 bit, [0])
     #[inline]
-    pub const fn cache_result_id(&self) -> usize {
+    const fn cache_result_id(&self) -> usize {
         self.inner & 0x1
     }
 
     #[inline]
-    pub const fn is_general_event(&self) -> bool {
+    const fn is_general_event(&self) -> bool {
         self.event_type() == event_type::HARDWARE_GENERAL
     }
 
     #[inline]
-    pub const fn is_cache_event(&self) -> bool {
+    const fn is_cache_event(&self) -> bool {
         self.event_type() == event_type::HARDWARE_CACHE
     }
 
     #[inline]
-    pub const fn is_raw_event_v1(&self) -> bool {
+    const fn is_raw_event_v1(&self) -> bool {
         self.event_type() == event_type::HARDWARE_RAW
     }
 
     #[inline]
-    pub const fn is_raw_event_v2(&self) -> bool {
+    const fn is_raw_event_v2(&self) -> bool {
         self.event_type() == event_type::HARDWARE_RAW_V2
     }
 
     #[inline]
-    pub const fn is_raw_event(&self) -> bool {
+    const fn is_raw_event(&self) -> bool {
         self.is_raw_event_v1() || self.is_raw_event_v2()
     }
 
     #[inline]
-    pub const fn is_firmware_event(&self) -> bool {
+    const fn is_firmware_event(&self) -> bool {
         self.event_type() == event_type::FIRMWARE
     }
 
     #[inline]
-    pub fn check_event_type(self) -> bool {
+    fn check_event_type(self) -> bool {
         let event_type = self.event_type();
         let event_code = self.event_code();
 
@@ -965,7 +960,7 @@ impl EventIdx {
     }
 
     #[inline]
-    pub fn firmware_event_valid(self) -> bool {
+    fn firmware_event_valid(self) -> bool {
         let event_type = self.event_type();
         let event_code = self.event_code();
         if event_type != event_type::FIRMWARE {
@@ -1116,7 +1111,8 @@ pub fn pmu_firmware_counter_increment(firmware_event: usize) {
     let counter_idx_start = pmu_state.hw_counters_num;
     for counter_idx in counter_idx_start..counter_idx_start + PMU_FIRMWARE_COUNTER_MAX {
         let fw_idx = counter_idx - counter_idx_start;
-        if pmu_state.active_event[counter_idx] == firmware_event
+        if pmu_state.active_event[counter_idx]
+            == EventIdx::from_firmwarw_event(firmware_event).raw()
             && pmu_state.is_firmware_event_start(counter_idx)
         {
             pmu_state.fw_counter[fw_idx] += 1;

prototyper/test-kernel/Cargo.toml

@@ -10,6 +10,9 @@ publish = false
 
 [dependencies]
 sbi-testing = { features = ["log"], path = "../../library/sbi-testing" }
+sbi-spec = { version = "0.0.8", features = [
+    "legacy",
+], path = "../../library/sbi-spec" }
 log = "0.4"
 riscv = "0.12.1"
 spin = "0.9"

prototyper/test-kernel/src/main.rs

@@ -10,7 +10,12 @@ use core::{
     arch::{asm, naked_asm},
     ptr::null,
 };
-use sbi_testing::sbi;
+use sbi_spec::{
+    binary::{CounterMask, HartMask, SbiRet},
+    pmu::firmware_event,
+};
+use sbi_testing::sbi::{self, ConfigFlags, StartFlags, StopFlags};
+// use sbi_spec::pmu::*;
 use uart16550::Uart16550;
 
 const RISCV_HEAD_FLAGS: u64 = 0;
@@ -108,7 +113,130 @@ extern "C" fn rust_main(hartid: usize, dtb_pa: usize) -> ! {
         hart_mask_base: 0,
         delay: frequency,
     };
-    if testing.test() {
+    let test_result = testing.test();
+
+    // pmu test, only valid on qemu-system-riscv64 platform
+    let counters_num = sbi::pmu_num_counters();
+    println!("[pmu] counters number: {}", counters_num);
+    for idx in 0..counters_num {
+        let counter_info = sbi::pmu_counter_get_info(idx);
+        let counter_info = CounterInfo::new(counter_info.value);
+        if counter_info.is_firmware_counter() {
+            println!("[pmu] counter index:{:>2}, is a firmware counter", idx);
+        } else {
+            println!(
+                "[pmu] counter index:{:>2}, csr num: {:#03x}, width: {}",
+                idx,
+                counter_info.get_csr(),
+                counter_info.get_width()
+            );
+        }
+    }
+
+    // Hardware event
+    let counter_mask = CounterMask::from_mask_base(0x7ffff, 0);
+    let result = sbi::pmu_counter_config_matching(counter_mask, Flag::new(0b110), 0x1, 0);
+    assert!(result.is_ok());
+    let result = sbi::pmu_counter_config_matching(counter_mask, Flag::new(0b110), 0x2, 0);
+    assert!(result.is_ok());
+    let result = sbi::pmu_counter_config_matching(counter_mask, Flag::new(0b110), 0x10019, 0);
+    assert!(result.is_ok());
+    let result = sbi::pmu_counter_config_matching(counter_mask, Flag::new(0b110), 0x1001b, 0);
+    assert!(result.is_ok());
+    let result = sbi::pmu_counter_config_matching(counter_mask, Flag::new(0b110), 0x10021, 0);
+    assert!(result.is_ok());
+    let result = sbi::pmu_counter_config_matching(counter_mask, Flag::new(0b110), 0x3, 0);
+    assert_eq!(result, SbiRet::not_supported());
+
+    // Firmware  event
+    let counter_mask = CounterMask::from_mask_base(0x7ffffffff, 0);
+
+    // Mapping a counter to the `SBI_PMU_FW_ACCESS_LOAD` event should result in unsupported
+    let result = sbi::pmu_counter_config_matching(
+        counter_mask,
+        Flag::new(0b010),
+        EventIdx::new_firmware_event(firmware_event::ACCESS_LOAD).raw(),
+        0,
+    );
+    assert_eq!(result, SbiRet::not_supported());
+
+    // Map a counter to the `SBI_PMU_FW_IPI_SENT` event.
+    // This counter should be a firmware counter and its value should be initialized to 0.
+    let result = sbi::pmu_counter_config_matching(
+        counter_mask,
+        Flag::new(0b010),
+        EventIdx::new_firmware_event(firmware_event::IPI_SENT).raw(),
+        0,
+    );
+    assert!(result.is_ok());
+    assert!(result.value >= 19);
+    let ipi_counter_idx = result.value;
+    let ipi_num = sbi::pmu_counter_fw_read(ipi_counter_idx);
+    assert!(ipi_num.is_ok());
+    assert_eq!(ipi_num.value, 0);
+
+    // Start counting `SBI_PMU_FW_IPI_SENT` events and assign an initial value of 25 to the event counter
+    let start_result = sbi::pmu_counter_start(
+        CounterMask::from_mask_base(0x1, ipi_counter_idx),
+        Flag::new(0x1),
+        25,
+    );
+    assert!(start_result.is_ok());
+    // Read the value of the `SBI_PMU_FW_IPI_SENT` event counter, which should be 25
+    let ipi_num = sbi::pmu_counter_fw_read(ipi_counter_idx);
+    assert!(ipi_num.is_ok());
+    assert_eq!(ipi_num.value, 25);
+
+    // Send IPI to other core, and the `SBI_PMU_FW_IPI_SENT` event counter value increases by one
+    let send_ipi_result = sbi::send_ipi(HartMask::from_mask_base(0b10, 0));
+    assert_eq!(send_ipi_result, SbiRet::invalid_param());
+
+    // Read the value of the `SBI_PMU_FW_IPI_SENT` event counter, which should be 26
+    let ipi_num = sbi::pmu_counter_fw_read(ipi_counter_idx);
+    assert!(ipi_num.is_ok());
+    assert_eq!(ipi_num.value, 26);
+
+    // Stop counting `SBI_PMU_FW_IPI_SENT` events
+    let stop_result = sbi::pmu_counter_stop(
+        CounterMask::from_mask_base(0x1, ipi_counter_idx),
+        Flag::new(0x0),
+    );
+    assert!(stop_result.is_ok());
+
+    // Restop counting `SBI_PMU_FW_IPI_SENT` events, the result should be already stop
+    let stop_result = sbi::pmu_counter_stop(
+        CounterMask::from_mask_base(0x1, ipi_counter_idx),
+        Flag::new(0x0),
+    );
+    assert_eq!(stop_result, SbiRet::already_stopped());
+
+    // Send IPI to other core, `SBI_PMU_FW_IPI_SENT` event counter should not change
+    let send_ipi_result = sbi::send_ipi(HartMask::from_mask_base(0b10, 0));
+    assert_eq!(send_ipi_result, SbiRet::invalid_param());
+
+    // Read the value of the `SBI_PMU_FW_IPI_SENT` event counter, which should be 26
+    let ipi_num = sbi::pmu_counter_fw_read(ipi_counter_idx);
+    assert!(ipi_num.is_ok());
+    assert_eq!(ipi_num.value, 26);
+
+    // Restart counting `SBI_PMU_FW_IPI_SENT` events
+    let start_result = sbi::pmu_counter_start(
+        CounterMask::from_mask_base(0x1, ipi_counter_idx),
+        Flag::new(0x0),
+        0,
+    );
+    assert!(start_result.is_ok());
+
+    // Send IPI to other core, and the `SBI_PMU_FW_IPI_SENT` event counter value increases by one
+    let send_ipi_result = sbi::send_ipi(HartMask::from_mask_base(0b10, 0));
+    assert_eq!(send_ipi_result, SbiRet::invalid_param());
+
+    // Read the value of the `SBI_PMU_FW_IPI_SENT` event counter, which should be 27
+    let ipi_num = sbi::pmu_counter_fw_read(ipi_counter_idx);
+    assert!(ipi_num.is_ok());
+    assert_eq!(ipi_num.value, 27);
+
+    if test_result {
         sbi::system_reset(sbi::Shutdown, sbi::NoReason);
     } else {
         sbi::system_reset(sbi::Shutdown, sbi::SystemFailure);
@@ -211,3 +339,113 @@ impl rcore_console::Console for Console {
         unsafe { UART.get().write(s.as_bytes()) };
     }
 }
+
+struct Flag {
+    inner: usize,
+}
+
+impl ConfigFlags for Flag {
+    fn raw(&self) -> usize {
+        self.inner
+    }
+}
+
+impl StartFlags for Flag {
+    fn raw(&self) -> usize {
+        self.inner
+    }
+}
+
+impl StopFlags for Flag {
+    fn raw(&self) -> usize {
+        self.inner
+    }
+}
+
+impl Flag {
+    pub fn new(flag: usize) -> Self {
+        Self { inner: flag }
+    }
+}
+
+/// Wrap for counter info
+struct CounterInfo {
+    /// Packed representation of counter information:
+    /// - Bits [11:0]: CSR number for hardware counters
+    /// - Bits [17:12]: Counter width (typically 63 for RV64)
+    /// - MSB: Set for firmware counters, clear for hardware counters
+    inner: usize,
+}
+
+#[allow(unused)]
+impl CounterInfo {
+    const CSR_MASK: usize = 0xFFF; // Bits [11:0]
+    const WIDTH_MASK: usize = 0x3F << 12; // Bits [17:12]
+    const FIRMWARE_FLAG: usize = 1 << (size_of::<usize>() * 8 - 1); // MSB
+
+    #[inline]
+    pub const fn new(counter_info: usize) -> Self {
+        Self {
+            inner: counter_info,
+        }
+    }
+
+    #[inline]
+    pub fn set_csr(&mut self, csr_num: u16) {
+        self.inner = (self.inner & !Self::CSR_MASK) | ((csr_num as usize) & Self::CSR_MASK);
+    }
+
+    #[inline]
+    pub fn get_csr(&self) -> usize {
+        self.inner & Self::CSR_MASK
+    }
+
+    #[inline]
+    pub fn set_width(&mut self, width: u8) {
+        self.inner = (self.inner & !Self::WIDTH_MASK) | (((width as usize) & 0x3F) << 12);
+    }
+
+    #[inline]
+    pub fn get_width(&self) -> usize {
+        (self.inner & Self::WIDTH_MASK) >> 12
+    }
+
+    #[inline]
+    pub fn is_firmware_counter(&self) -> bool {
+        self.inner & Self::FIRMWARE_FLAG != 0
+    }
+
+    #[inline]
+    pub const fn with_hardware_info(csr_num: u16, width: u8) -> Self {
+        Self {
+            inner: ((csr_num as usize) & Self::CSR_MASK) | (((width as usize) & 0x3F) << 12),
+        }
+    }
+
+    #[inline]
+    pub const fn with_firmware_info() -> Self {
+        Self {
+            inner: Self::FIRMWARE_FLAG,
+        }
+    }
+
+    #[inline]
+    pub const fn inner(self) -> usize {
+        self.inner
+    }
+}
+
+struct EventIdx {
+    inner: usize,
+}
+
+impl EventIdx {
+    fn raw(&self) -> usize {
+        self.inner
+    }
+
+    fn new_firmware_event(event_code: usize) -> Self {
+        let inner = 0xf << 16 | event_code;
+        Self { inner }
+    }
+}