Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bump to upstream spike 8faa928 (Feb 16, 2021) #24

Closed
wants to merge 335 commits into from

Conversation

zitaofang
Copy link
Member

(Required for #23)

aswaterman and others added 30 commits June 4, 2020 03:28
Previously, we unintentionally prioritized access faults and page faults.

Resolves #431
`toupper` depends on locale information, so it actually results in
a function call, preventing the comparison from being constpropped.
Fix by manually inlining the comparison.

cc @chihminchao
make library name general for multiple custom extension built in one
shared library.

Signed-off-by: Chih-Min Chao <[email protected]>
for --isa=rv64_zavmo_xmyext

1. make custom extension work with z extension and underline char
2. search libmyext.so and libcustomext.so
3. check myext in open library
4. fix custom extension disassembler initialization bug

Signed-off-by: Chih-Min Chao <[email protected]>
just check the execution privilege

Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
There are two options to specify custom extension and register it

1. --isa with x  ex: --isa=rv64gcv_xmyext

  parse, load, register in  processor_t::processor_t

2. --extension

  parse, load in main
  register later by calling processort_t::register_extension

  The patch fix the register pass in 2

Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
aswaterman and others added 3 commits February 16, 2021 22:34
Guest/VM extension status related fixes
These instructions are RV32 only. Previously, they zero-extended
their 32-bit result to 64-bits, to match the Spike implementation detail
that the X registers are always 64-bits long.

This exposed a data dependant problem when the instruction results fed
into the add and sltu instructions. The lack of sign extension on the
sha512*, combined with the presence of sign extension on the add, meant
sltu would (as it is currently implemented) produce the wrong result.

There were two potential fixes:

1) Sign extend from 32-bits to XLEN the result of the SHA512 instructions.

2) Change the SLTU implementation to truncate RS1/RS2 to be XLEN bits
   before it does the comparison.

This patch implements option 1, because I didn't want to mess with a
base ISA instruction. However, this leaves the implementation detail
open to cause problems for people in the future. Fixing this is outside
the scope of this commit.

 On branch scalar-crypto-fix
 Changes to be committed:
	modified:   riscv/insns/sha512sig0h.h
	modified:   riscv/insns/sha512sig0l.h
	modified:   riscv/insns/sha512sig1h.h
	modified:   riscv/insns/sha512sig1l.h
	modified:   riscv/insns/sha512sum0r.h
	modified:   riscv/insns/sha512sum1r.h
Historically, one could uniquely decode any RISC-V instruction based on
the instruciton to decode, plus a MATCH and MASK pair.

The scalar crypto extension adds instructions for accelerating the AES
algorithm which work very differently on RV32 and RV64. However, they
overlap in terms of opcodes. The instructions are always mutually
exclusive, and so it makes sense to overlap them this way to save
encoding space.

This exposed a problem, where previously Spike assumed the decoder
function was something like:

> decode(instr_word, MATCH, MASK)

Now it needed to be

> decode(instr_word, MATCH, MASK, current_xlen)

To get around this in the initial implementation, the instructions which
shared opcodes were implemented in the same *.h file - e.g. aesds.h
contained an implementation of aes32dsi, and aes64ds. We detected
xlen in the file, and executed the appropriate instruction logic.
This worked fine for our limited set of benchmarks.

After more extensive testing, we found that Spike has an optimisation
which changes the order in which it tries to decode instructions based
on past instructions.

Running more extensive tests exposed the fact that the decoding logic
could still not unambiguously decode the instructions. Hence, more
substantial changes were needed to tell spike that an instruction is
RV32 or RV64 only.

These changes have been implemented as part of

- riscv/encoding.h
- disasm/disasm.cc
- riscv/processor.cc/h

Basically, every instr_desc_t has an extra field which marks which
base architecture the instruction can be exectuted on. This bitfield
can be altered for particular instructions.

The changes to riscv/insns/* simply split out the previously combined
instructions into a separate header files.

 On branch scalar-crypto-fix
 Changes to be committed:
	modified:   disasm/disasm.cc
	modified:   riscv/encoding.h
	new file:   riscv/insns/aes32dsi.h
	new file:   riscv/insns/aes32dsmi.h
	new file:   riscv/insns/aes32esi.h
	new file:   riscv/insns/aes32esmi.h
	new file:   riscv/insns/aes64ds.h
	new file:   riscv/insns/aes64dsm.h
	new file:   riscv/insns/aes64es.h
	new file:   riscv/insns/aes64esm.h
	deleted:    riscv/insns/aesds.h
	deleted:    riscv/insns/aesdsm.h
	deleted:    riscv/insns/aeses.h
	deleted:    riscv/insns/aesesm.h
	modified:   riscv/processor.cc
	modified:   riscv/processor.h
	modified:   riscv/riscv.mk.in
@zitaofang
Copy link
Member Author

Seems like I messed up, I will fix the conflict locally first.

chihminchao and others added 22 commits February 23, 2021 23:49
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
Signed-off-by: Chih-Min Chao <[email protected]>
It was reading as 0, which is not a legal value.

In mstatus, UXL gets initialized by the call to set_csr(CSR_MSTATUS)
here in processor_t::reset().
It was reading as 0, which is not a legal value.
The priv spec says: "For example, the value of the HS-level sstatus.FS
[aka mstatus.FS] does not affect vsstatus.SD."
* Don't make MPRV load/store virtual if MPV=1, MPP=3

* Use PRV_M instead of the value "3"
* Simplify Boolean logic

No functional change intended.

* Apply same logic to virtualize sstatus.XS as used for VS and FS

Though this macro does not seem to be used anywhere today.

* Extract common macro to DRY up code

* Dirty both mstatus and vsstatus FP fields

Fixes riscv-software-src/riscv-isa-sim#660
Since this is not modifiable in the real sstatus, so it should not be
in the virtualized version either.
@colinschmidt
Copy link
Contributor

Are you planning on bumping esp-tools with this commit? I think we should do that so we can propagate the hwacha vsetvl name change.

@zitaofang
Copy link
Member Author

I am closing this PR and reopen another one so that the branch can be in this repo.

@zitaofang zitaofang closed this May 25, 2021
@zitaofang zitaofang mentioned this pull request May 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.