Only implement one solution for native triggers.

[rtwfroody]

When S mode is present, use option 1 (disable triggers in M-Mode unless MIE is set) from the Debug Spec. When S mode is not present, use option 2 (implement mte and mpte bits in tcontrol).

See discussion in #1777.

[rtwfroody]

@YenHaoChen can you look at this?

[YenHaoChen]

I am unsure whether we should implement a read-only 0 tcontorl in Solution 1 and could not find a direct answer from the spec.

The spec may want to keep the flexibility for hardware implementation. However, software would need a straight method to understand the hardware features for development. Additionally, from the hardware perspective, implementing an optional read-only 0 CSR increases costs without providing any beneficial features, and this does not make sense.

After another reading of the spec, I am convinced the modification does not violate the words of the spec.

[aswaterman]

Ping me once the two of you are in agreement, then I'll merge the PR.

[rtwfroody]

Good point. The spec is a bit messy here, and can use some clarification. The easy way to make it unambiguous what spike is doing is for tcontrol to not exist at all when S is implemented.

[rtwfroody]

Just force pushed no change to get the checks to start again. I'm pretty sure they failed because of some network issue.

[aswaterman]

@rtwfroody Not a network problem; there's now a segfault when running a basic test on Spike. I checked this by rerunning CI locally. Here's what gdb has to say; I didn't spend any time looking into it:

$ gdb ../install/bin/spike 
GNU gdb (GDB) Red Hat Enterprise Linux 8.2-20.el8
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-redhat-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ../install/bin/spike...run done.
(gdb) run --isa=rv64gc pk hello
Starting program: /scratch/waterman/riscv-isa-sim/ci-tests/build/install/bin/spike --isa=rv64gc pk hello
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
[Detaching after fork from child process 1192447]
[Detaching after fork from child process 1192448]

Program received signal SIGSEGV, Segmentation fault.
fast_rv32i_mret (p=0x7ffff7fa3010, insn=..., pc=<optimized out>) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/processor.h:264
264	  state_t* get_state() { return &state; }
Missing separate debuginfos, use: yum debuginfo-install glibc-2.28-236.el8_9.13.x86_64 libgcc-8.5.0-20.el8.x86_64 libstdc++-8.5.0-20.el8.x86_64
(gdb) bt
#0  fast_rv32i_mret (p=0x7ffff7fa3010, insn=..., pc=<optimized out>) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/processor.h:264
#1  0x0000000000929ba4 in execute_insn_fast (fetch=..., pc=2147497700, p=0x7ffff7fa3010) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/execute.cc:306
#2  processor_t::step (this=0x7ffff7fa3010, n=908, n@entry=5000) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/execute.cc:306
#3  0x000000000054b386 in sim_t::step (this=this@entry=0x7fffffffca80, n=n@entry=5000) at /usr/include/c++/8/bits/stl_vector.h:930
#4  0x000000000054b4d5 in sim_t::idle (this=0x7fffffffca80) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/sim.cc:432
#5  sim_t::idle (this=0x7fffffffca80) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/sim.cc:424
#6  0x0000000000957985 in htif_t::run (this=this@entry=0x7fffffffca80) at /scratch/waterman/riscv-isa-sim/ci-tests/../fesvr/byteorder.h:12
#7  0x000000000054bdb4 in sim_t::run (this=this@entry=0x7fffffffca80) at /scratch/waterman/riscv-isa-sim/ci-tests/../riscv/sim.cc:277
#8  0x0000000000509d09 in main (argc=<optimized out>, argv=<optimized out>) at /scratch/waterman/riscv-isa-sim/ci-tests/../spike_main/spike.cc:556
(gdb) 

[YenHaoChen]

@rtwfroody It looks like the issue might be related to writing tcontrol in mret.h. We can access the tcontrol only when it exists, i.e., if (!proc->extension_enabled_const('S')).

[rtwfroody]

I wonder why that's not happening for me. I rebased off of master just in case. Running spike with no arguments under gdb or valgrind report no issues whatsover. Anyway, I'll figure it out.

[rtwfroody]

Got it to reproduce. Typical user error...

[YenHaoChen]

Overall LGTM. The functionality seems correct from my tests.

1. The commit is big and could be broken down a little bit. Specifically, we could have a refactoring commit, which only changes the  allow_action() function parameter.
2. I wonder whether we still need to check the tcontrol.mte in common_match().
3. Solution 2 requires the tcontrol. We could use the existence of tcontrol to decide which solution to use and guarantee that the tcontrol exists in Solution 2.

[rtwfroody]

Should the allow_action() logic really just be in common_match()?

[aswaterman]

I'm OK with this version, but I'll let @YenHaoChen sign off and click the merge button when he's ready.

[YenHaoChen]

The PR has addressed all of my comments.