On 8/6/24 5:46 AM, Andrew Jones wrote:
On Tue, Jul 23, 2024 at 04:30:10PM GMT, Atish Patra wrote:
Counter delegation/configuration extension requires the following
extensions to be enabled.

1. Smcdeleg - To enable counter delegation from M to S
2. S[m|s]csrind - To enable indirect access CSRs
3. Smstateen - Indirect CSR extensions depend on it.
4. Sscofpmf - To enable counter overflow feature
5. S[m|s]aia - To enable counter overflow feature in virtualization
6. Smcntrpmf - To enable privilege mode filtering for cycle/instret

While first 3 are mandatory to enable the counter delegation,
next 3 set of extension are preferred to enable all the PMU related
features.

Just my 2 cents, but I think for the first three we can apply the concept
of extension bundles, which we need for other extensions as well. In those
cases we just auto enable all the dependencies. For the three preferred
extensions I think we can just leave them off for 'base', but we should
enable them by default for 'max' along with Ssccfg.

I like this idea. I would throw in all these 6 extensions in a 
'pmu_advanced_ops'
(or any other better fitting name for the bundle) flag and then 
'pmu_advanced_ops=true'
would enable all of those. 'pmu_advanced_ops=true,smcntrpmf=false' enables all 
but
'smcntrpmf' and so on.

As long as we document what the flag is enabling I don't see any problems with 
it.
This is how profiles are implemented after all.

With this bundle we can also use implied rule only if an extension really needs
(i.e. it breaks without) a dependency being enabled, instead of overloading it
with extensions that 'would be nice to have together' like it seems to be the
case for the last 3 extensions in that list.

I believe users would benefit more from a single flag to enable everything and
be done with it.


Thanks,

Daniel




Thanks,
drew

That's why, enable all of these if Ssccfg extension is
enabled from the commandline.

Signed-off-by: Atish Patra <ati...@rivosinc.com>
---
  target/riscv/cpu.c | 14 +++++++++++++-
  1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 22ba43c7ff2a..abebfcc46dea 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -2665,8 +2665,20 @@ RISCVCPUImpliedExtsRule *riscv_multi_ext_implied_rules[] 
= {
      NULL
  };
+static RISCVCPUPreferredExtsRule SSCCFG_PREFERRED = {
+    .ext = CPU_CFG_OFFSET(ext_ssccfg),
+    .preferred_multi_exts = {
+        CPU_CFG_OFFSET(ext_smcsrind), CPU_CFG_OFFSET(ext_sscsrind),
+        CPU_CFG_OFFSET(ext_ssaia), CPU_CFG_OFFSET(ext_smaia),
+        CPU_CFG_OFFSET(ext_smstateen), CPU_CFG_OFFSET(ext_sscofpmf),
+        CPU_CFG_OFFSET(ext_smcntrpmf), CPU_CFG_OFFSET(ext_smcdeleg),
+
+        RISCV_PREFRRED_EXTS_RULE_END
+    },
+};
+
  RISCVCPUPreferredExtsRule *riscv_multi_ext_preferred_rules[] = {
-    NULL
+    &SSCCFG_PREFERRED, NULL
  };
static Property riscv_cpu_properties[] = {

--
2.34.1



Reply via email to