Merge pull request #77 from NanaHigh/nanahigh/test

fix(pmu): add missing `pmu_snapshot_set_shmem` function.

CHANGELOG.md

@@ -7,6 +7,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## Unreleased
 
+### Added
+- pmu: add missing `snapshot_set_shmem` function and testcases in `Pmu` trait.
+
 ### Added
 
 - pmu: add missing `snapshot_set_shmem` function in `Pmu` trait, impl for `&T` and `Option<T>` and `Forward` structure

sbi-rt/CHANGELOG.md

@@ -7,6 +7,9 @@ to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
 ## [Unreleased]
 
+### Added
+- pmu: add missing `pmu_snapshot_set_shmem` function.
+
 ### Added
 
 - `pmu_snapshot_set_shmem` function signature, TODO: documents and implementation

sbi-rt/src/pmu.rs

@@ -6,7 +6,7 @@ use sbi_spec::{
     binary::{SbiRet, SharedPtr},
     pmu::{
         shmem_size::SIZE, COUNTER_CONFIG_MATCHING, COUNTER_FW_READ, COUNTER_FW_READ_HI,
-        COUNTER_GET_INFO, COUNTER_START, COUNTER_STOP, EID_PMU, NUM_COUNTERS,
+        COUNTER_GET_INFO, COUNTER_START, COUNTER_STOP, EID_PMU, NUM_COUNTERS, SNAPSHOT_SET_SHMEM,
     },
 };
 
@@ -14,7 +14,7 @@ use sbi_spec::{
 ///
 /// This call would always succeed without returning any error.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.5.
+/// This function is defined in RISC-V SBI Specification chapter 11.6.
 #[inline]
 pub fn pmu_num_counters() -> usize {
     sbi_call_0(EID_PMU, NUM_COUNTERS).value
@@ -35,8 +35,6 @@ pub fn pmu_num_counters() -> usize {
 /// ```
 /// If `counter_info.type` == `1` then `counter_info.csr` and `counter_info.width` should be ignored.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.6.
-///
 /// # Return value
 ///
 /// Returns the `counter_info` described above in `SbiRet.value`.
@@ -48,7 +46,7 @@ pub fn pmu_num_counters() -> usize {
 /// | `SbiRet::success()`       | `counter_info` read successfully.
 /// | `SbiRet::invalid_param()` | `counter_idx` points to an invalid counter.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.6.
+/// This function is defined in RISC-V SBI Specification chapter 11.7.
 #[inline]
 pub fn pmu_counter_get_info(counter_idx: usize) -> SbiRet {
     sbi_call_1(EID_PMU, COUNTER_GET_INFO, counter_idx)
@@ -103,7 +101,7 @@ pub fn pmu_counter_get_info(counter_idx: usize) -> SbiRet {
 /// | `SbiRet::invalid_param()` | set of counters has an invalid counter.
 /// | `SbiRet::not_supported()` | none of the counters can monitor specified event.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.7.
+/// This function is defined in RISC-V SBI Specification chapter 11.8.
 #[inline]
 pub fn pmu_counter_config_matching<T>(
     counter_idx_base: usize,
@@ -167,7 +165,7 @@ where
 /// | `SbiRet::invalid_param()`   | some of the counters specified in parameters are invalid.
 /// | `SbiRet::already_started()` | some of the counters specified in parameters are already started.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.8.
+/// This function is defined in RISC-V SBI Specification chapter 11.9.
 #[inline]
 pub fn pmu_counter_start<T>(
     counter_idx_base: usize,
@@ -223,7 +221,7 @@ where
 /// | `SbiRet::invalid_param()`   | some of the counters specified in parameters are invalid.
 /// | `SbiRet::already_stopped()` | some of the counters specified in parameters are already stopped.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.9.
+/// This function is defined in RISC-V SBI Specification chapter 11.10.
 #[inline]
 pub fn pmu_counter_stop<T>(
     counter_idx_base: usize,
@@ -261,7 +259,7 @@ where
 /// | `SbiRet::success()`       | firmware counter read successfully.
 /// | `SbiRet::invalid_param()` | `counter_idx` points to a hardware counter or an invalid counter.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.10.
+/// This function is defined in RISC-V SBI Specification chapter 11.11.
 #[inline]
 pub fn pmu_counter_fw_read(counter_idx: usize) -> SbiRet {
     sbi_call_1(EID_PMU, COUNTER_FW_READ, counter_idx)
@@ -280,7 +278,7 @@ pub fn pmu_counter_fw_read(counter_idx: usize) -> SbiRet {
 /// | `SbiRet::success()`       | firmware counter read successfully.
 /// | `SbiRet::invalid_param()` | `counter_idx` points to a hardware counter or an invalid counter.
 ///
-/// This function is defined in RISC-V SBI Specification chapter 11.11.
+/// This function is defined in RISC-V SBI Specification chapter 11.12.
 #[inline]
 pub fn pmu_counter_fw_read_hi(counter_idx: usize) -> SbiRet {
     sbi_call_1(EID_PMU, COUNTER_FW_READ_HI, counter_idx)
@@ -288,15 +286,41 @@ pub fn pmu_counter_fw_read_hi(counter_idx: usize) -> SbiRet {
 
 /// Set and enable the PMU snapshot shared memory on the calling hart.
 ///
-/// TODO detailed documents from rustsbi library
+/// This function should be invoked only once per hart at boot time. Once configured, the SBI
+/// implementation has read/write access to the shared memory when `sbi_pmu_counter_stop` is
+/// invoked with the `SBI_PMU_STOP_FLAG_TAKE_SNAPSHOT` flag set.
+///
+/// # Parameters
+///
+/// If both `shmem_phys_lo` and `shmem_phys_hi` parameters are not all-ones bitwise then
+/// `shmem_phys_lo` specifies the lower XLEN bits and `shmem_phys_hi` specifies the upper XLEN bits
+/// of the snapshot shared memory physical base address. The `shmem_phys_lo` MUST be 4096 bytes
+/// (i.e. page) aligned and the size of the snapshot shared memory must be 4096 bytes.
+///
+/// The `flags` parameter is reserved for future use and must be zero.
+///
+/// # Return value
+///
+/// The possible return error codes returned in `SbiRet.error` are shown in the table below:
+///
+/// | Return code                 | Description
+/// |:----------------------------|:----------------------------------------------
+/// | `SbiRet::success()`         | Shared memory was set or cleared successfully.
+/// | `SbiRet::not_supported()`   | The SBI PMU snapshot functionality is not available in the SBI implementation.
+/// | `SbiRet::invalid_param()`   | The flags parameter is not zero or the `shmem_phys_lo` parameter is not 4096 bytes aligned.
+/// | `SbiRet::invalid_address()` | The shared memory pointed to by the `shmem_phys_lo` and `shmem_phys_hi` parameters is not writable
+/// |                             | or does not satisfy other requirements of RISC-V SBI Specification chapter 3.2.
+/// | `SbiRet::failed()`          | The request failed for unspecified or unknown other reasons.
+/// This function is defined in RISC-V SBI Specification chapter 11.13.
 #[inline]
 pub fn pmu_snapshot_set_shmem(shmem: SharedPtr<[u8; SIZE]>, flags: usize) -> SbiRet {
-    // TODO call sbi_call_3 for this argument list:
-    // struct sbiret sbi_pmu_snapshot_set_shmem(unsigned long shmem_phys_lo,
-    //     unsigned long shmem_phys_hi,
-    //     unsigned long flags)
-    let _ = (shmem, flags);
-    todo!()
+    sbi_call_3(
+        EID_PMU,
+        SNAPSHOT_SET_SHMEM,
+        shmem.phys_addr_lo(),
+        shmem.phys_addr_hi(),
+        flags,
+    )
 }
 
 /// Flags to configure performance counter.

sbi-spec/CHANGELOG.md

@@ -6,6 +6,9 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 
 ## [Unreleased]
 
+### Fixed
+- pmu: fix serial number issues in docs.
+
 ### Added
 
 - base: add Coreboot and Oreboot to `impl_id` module

sbi-spec/src/pmu.rs

@@ -8,35 +8,35 @@ pub use fid::*;
 mod fid {
     /// Function ID to get the number of counters, both hardware and firmware.
     ///
-    /// Declared in §11.5.
+    /// Declared in §11.6.
     pub const NUM_COUNTERS: usize = 0;
     /// Function ID to get details about the specified counter.
     ///
-    /// Declared in §11.6.
+    /// Declared in §11.7.
     pub const COUNTER_GET_INFO: usize = 1;
     /// Function ID to find and configure a counter from a set of counters.
     ///
-    /// Declared in §11.7.
+    /// Declared in §11.8.
     pub const COUNTER_CONFIG_MATCHING: usize = 2;
     /// Function ID to start or enable a set of counters on the calling hart with the specified initial value.
     ///
-    /// Declared in §11.8.
+    /// Declared in §11.9.
     pub const COUNTER_START: usize = 3;
     /// Function ID to stop or disable a set of counters on the calling hart.
     ///
-    /// Declared in §11.9.
+    /// Declared in §11.10.
     pub const COUNTER_STOP: usize = 4;
     /// Function ID to provide the current value of a firmware counter.
     ///
-    /// Declared in §11.10.
+    /// Declared in §11.11.
     pub const COUNTER_FW_READ: usize = 5;
     /// Function ID to provide the upper 32 bits from the value of the current firmware counter.
     ///
-    /// Declared in §11.11.
+    /// Declared in §11.12.
     pub const COUNTER_FW_READ_HI: usize = 6;
     /// Function ID to set and enable the PMU snapshot shared memory.
     ///
-    /// Declared in §11.12.
+    /// Declared in §11.13.
     pub const SNAPSHOT_SET_SHMEM: usize = 7;
 }
 

src/traits.rs

@@ -255,7 +255,9 @@ pub fn _rustsbi_pmu<T: crate::Pmu>(pmu: &T, param: [usize; 6], function: usize)
                 spec::pmu::COUNTER_STOP => pmu.counter_stop(param0, param1, param2),
                 spec::pmu::COUNTER_FW_READ => pmu.counter_fw_read(param0),
                 spec::pmu::COUNTER_FW_READ_HI => pmu.counter_fw_read_hi(param0),
-                // TODO spec::pmu::SNAPSHOT_SET_SHMEM => pmu.snapshot_set_shmem(...)
+                spec::pmu::SNAPSHOT_SET_SHMEM => {
+                    pmu.snapshot_set_shmem(SharedPtr::new(param0, param1), param2)
+                }
                 _ => SbiRet::not_supported(),
             }
         }
@@ -279,7 +281,9 @@ pub fn _rustsbi_pmu<T: crate::Pmu>(pmu: &T, param: [usize; 6], function: usize)
                 spec::pmu::COUNTER_STOP => pmu.counter_stop(param0, param1, param2),
                 spec::pmu::COUNTER_FW_READ => pmu.counter_fw_read(param0),
                 spec::pmu::COUNTER_FW_READ_HI => pmu.counter_fw_read_hi(param0),
-                // TODO spec::pmu::SNAPSHOT_SET_SHMEM => pmu.snapshot_set_shmem(...)
+                spec::pmu::SNAPSHOT_SET_SHMEM => {
+                    pmu.snapshot_set_shmem(SharedPtr::new(param0, param1), param2)
+                }
                 _ => SbiRet::not_supported(),
             }
         }

tests/build-full.rs

@@ -3,6 +3,7 @@ use rustsbi::RustSBI;
 use sbi_spec::{
     binary::{HartMask, Physical, SbiRet, SharedPtr},
     nacl::shmem_size::NATIVE,
+    pmu::shmem_size::SIZE,
 };
 
 #[derive(RustSBI)]
@@ -176,22 +177,23 @@ fn generated_extensions() {
     assert_eq!(sbi.handle_ecall(0x504D55, 4, [0; 6]), SbiRet::success(22));
     assert_eq!(sbi.handle_ecall(0x504D55, 5, [0; 6]), SbiRet::success(23));
     assert_eq!(sbi.handle_ecall(0x504D55, 6, [0; 6]), SbiRet::success(24));
+    assert_eq!(sbi.handle_ecall(0x504D55, 7, [0; 6]), SbiRet::success(25));
     // TODO eid and fid of snapshot_set_shmem
-    assert_eq!(sbi.handle_ecall(0x53525354, 0, [0; 6]), SbiRet::success(25));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 0, [0; 6]), SbiRet::success(26));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 1, [0; 6]), SbiRet::success(27));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 2, [0; 6]), SbiRet::success(28));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 3, [0; 6]), SbiRet::success(29));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 4, [0; 6]), SbiRet::success(30));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 5, [0; 6]), SbiRet::success(31));
-    assert_eq!(sbi.handle_ecall(0x52464E43, 6, [0; 6]), SbiRet::success(32));
-    assert_eq!(sbi.handle_ecall(0x535441, 0, [0; 6]), SbiRet::success(33));
-    assert_eq!(sbi.handle_ecall(0x53555350, 0, [0; 6]), SbiRet::success(34));
+    assert_eq!(sbi.handle_ecall(0x53525354, 0, [0; 6]), SbiRet::success(26));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 0, [0; 6]), SbiRet::success(27));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 1, [0; 6]), SbiRet::success(28));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 2, [0; 6]), SbiRet::success(29));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 3, [0; 6]), SbiRet::success(30));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 4, [0; 6]), SbiRet::success(31));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 5, [0; 6]), SbiRet::success(32));
+    assert_eq!(sbi.handle_ecall(0x52464E43, 6, [0; 6]), SbiRet::success(33));
+    assert_eq!(sbi.handle_ecall(0x535441, 0, [0; 6]), SbiRet::success(34));
+    assert_eq!(sbi.handle_ecall(0x53555350, 0, [0; 6]), SbiRet::success(35));
     assert!(sbi.handle_ecall(0x54494D45, 0, [0; 6]).is_ok());
-    assert_eq!(sbi.timer.0.take(), 35);
-    assert_eq!(sbi.handle_ecall(0x10, 4, [0; 6]), SbiRet::success(36));
-    assert_eq!(sbi.handle_ecall(0x10, 5, [0; 6]), SbiRet::success(37));
-    assert_eq!(sbi.handle_ecall(0x10, 6, [0; 6]), SbiRet::success(38));
+    assert_eq!(sbi.timer.0.take(), 36);
+    assert_eq!(sbi.handle_ecall(0x10, 4, [0; 6]), SbiRet::success(37));
+    assert_eq!(sbi.handle_ecall(0x10, 5, [0; 6]), SbiRet::success(38));
+    assert_eq!(sbi.handle_ecall(0x10, 6, [0; 6]), SbiRet::success(39));
 }
 
 struct DummyConsole;
@@ -311,6 +313,10 @@ impl rustsbi::Pmu for DummyPmu {
     fn counter_fw_read_hi(&self, _: usize) -> SbiRet {
         SbiRet::success(24)
     }
+
+    fn snapshot_set_shmem(&self, _: SharedPtr<[u8; SIZE]>, _: usize) -> SbiRet {
+        SbiRet::success(25)
+    }
     // TODO fn snapshot_set_shmem
 }
 
@@ -318,7 +324,7 @@ struct DummyReset;
 
 impl rustsbi::Reset for DummyReset {
     fn system_reset(&self, _: u32, _: u32) -> SbiRet {
-        SbiRet::success(25)
+        SbiRet::success(26)
     }
 }
 
@@ -326,31 +332,31 @@ struct DummyFence;
 
 impl rustsbi::Fence for DummyFence {
     fn remote_fence_i(&self, _: HartMask) -> SbiRet {
-        SbiRet::success(26)
+        SbiRet::success(27)
     }
 
     fn remote_sfence_vma(&self, _: HartMask, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(27)
+        SbiRet::success(28)
     }
 
     fn remote_sfence_vma_asid(&self, _: HartMask, _: usize, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(28)
+        SbiRet::success(29)
     }
 
     fn remote_hfence_gvma_vmid(&self, _: HartMask, _: usize, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(29)
+        SbiRet::success(30)
     }
 
     fn remote_hfence_gvma(&self, _: HartMask, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(30)
+        SbiRet::success(31)
     }
 
     fn remote_hfence_vvma_asid(&self, _: HartMask, _: usize, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(31)
+        SbiRet::success(32)
     }
 
     fn remote_hfence_vvma(&self, _: HartMask, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(32)
+        SbiRet::success(33)
     }
 }
 
@@ -358,7 +364,7 @@ struct DummySta;
 
 impl rustsbi::Sta for DummySta {
     fn set_shmem(&self, _: SharedPtr<[u8; 64]>, _: usize) -> SbiRet {
-        SbiRet::success(33)
+        SbiRet::success(34)
     }
 }
 
@@ -366,7 +372,7 @@ struct DummySusp;
 
 impl rustsbi::Susp for DummySusp {
     fn system_suspend(&self, _: u32, _: usize, _: usize) -> SbiRet {
-        SbiRet::success(34)
+        SbiRet::success(35)
     }
 }
 
@@ -374,7 +380,7 @@ struct DummyTimer(RefCell<u64>);
 
 impl rustsbi::Timer for DummyTimer {
     fn set_timer(&self, _: u64) {
-        self.0.replace(35);
+        self.0.replace(36);
     }
 }
 
@@ -382,14 +388,14 @@ struct DummyEnvInfo;
 
 impl rustsbi::EnvInfo for DummyEnvInfo {
     fn mvendorid(&self) -> usize {
-        36
+        37
     }
 
     fn marchid(&self) -> usize {
-        37
+        38
     }
 
     fn mimpid(&self) -> usize {
-        38
+        39
     }
 }