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

[Bug Report] incorrect MISC_MEM decoder #900

Closed
Phantom1003 opened this issue Jun 3, 2022 · 2 comments
Closed

[Bug Report] incorrect MISC_MEM decoder #900

Phantom1003 opened this issue Jun 3, 2022 · 2 comments

Comments

@Phantom1003
Copy link
Contributor

Our co-simulation framework found the decoder has an incorrect behavior when execute a fence.i/fence with non-zero rd field.

As discussed before 719, fence/fence.i should ignores immediate, rs1, rd field. We found the patch 724 does not fix this bug.

The checking after the patch still throws an exception for non-zero rd and rs1:

cva6/core/decoder.sv

Lines 238 to 239 in 909d85a

if (instr.stype.rs1 != '0 || instr.stype.imm0 != '0 || instr.instr[31:28] != '0)
illegal_instr = 1'b1;

According to the The RISC-V Instruction Set Manual Volume I: Unprivileged ISA:

FENCE: The unused fields in the FENCE instructions—rs1 and rd—are reserved for finer-grain fences in future extensions. For forward compatibility, base implementations shall ignore these fields, and standard software shall zero these fields.
FENCE.I: The unused fields in the FENCE.I instruction, imm[11:0], rs1, and rd, are reserved for finer-grain fences in future extensions. For forward compatibility, base implementations shall ignore these fields, and standard software shall zero these fields.

In the following test case, there is a valid fence.i at 0x80000194, whose rd field is 1, and a fence with non-zero rd at 0x80000198. cva6 still throws exceptions for them.

[cva6] Exception @     68600, PC: 0000000080000194, Cause: Illegal Instruction,
[cva6]                                 tval: 000000000000120f
[spike] core   0: 0x0000000080000194 (0x0000120f) fence.i
[error] PC SIM 0000000080000194, DUT 0000000080000004
[error] INSN SIM 0000120f, DUT 34202f73
[CJ] Commit Failed

cva6-4.zip

@LuminaDCIX helps reproduce the problem

@zarubaf
Copy link
Contributor

zarubaf commented Jun 7, 2022

Hm, based on the current spec it seems that:

cva6/core/decoder.sv

Lines 238 to 239 in 909d85a

if (instr.stype.rs1 != '0 || instr.stype.imm0 != '0 || instr.instr[31:28] != '0)
illegal_instr = 1'b1;

Is not needed anymore to preserve forward compatibility. I can't seem to find how rd is factored into the equation. Would you mind trying to delete the above lines and re-run the testcase?

@Phantom1003
Copy link
Contributor Author

Phantom1003 commented Jun 7, 2022

I can't seem to find how rd is factored into the equation.

The fence instruction is I type, instr.stype.imm0 != '0 will check the rd field.

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

No branches or pull requests

2 participants