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 : PMPCFG.A field #2496

Open
AyoubJalali opened this issue Jul 5, 2024 · 10 comments
Open

Bug : PMPCFG.A field #2496

AyoubJalali opened this issue Jul 5, 2024 · 10 comments
Labels

Comments

@AyoubJalali
Copy link
Contributor

Hello, according to the CV32A65X Spec, the pmpcfg[x].A support only 2 modes disable/TOR, so the only legal value for pmpcfg.A is 2'b01 | 2'b00.
but spike support all A flied modes, so this need some mask based on a config, the bug is in solo & tandem mode.

@MikeOpenHWGroup
Copy link
Member

See also CVA6 #2024.

@zchamski
Copy link
Contributor

zchamski commented Jan 8, 2025

We need to specify in detail the behavior of pmpcfg.A writes: when the high bit of the assigned value is set, should the write be discarded altogether (no state change), or should it be masked and only affect the low bit of pmpcfg.A?

@MikeOpenHWGroup
Copy link
Member

pmpcfg.A is a WARL field and Section 2.3 CSR Field Specifications of the privileged spec says:

Implementations will not raise an exception on writes of unsupported values to a WARL field.
Implementations can return any legal value on the read of a WARL field when the last write was of
an illegal value, but the legal value returned should deterministically depend on the illegal written
value and the architectural state of the hart.

So it all depends on the interpretation of the legal value returned should deterministically depend on the illegal written value. My interpretation of this is that a write of 2'b1X should be ignored, no exception should be raised and the value of pmpcfg.A should not change.

Consider the scenario where the current value of pmpcfg.A is 2'b00 (OFF) and software writes 2'b11 (NAPOT).

In this situation, the only deterministic outcomes are that pmpcfg.A should be either NAPOT or OFF. In our case it is OFF, since NAPOT is not supported. If the write is "masked and only affect the low bit", then the value is updated to TOR, which software would not be expecting.

@AyoubJalali
Copy link
Contributor Author

Normally we should igonre changing the pmpcfg.A when trying to write an illegal value, so the last legal value should be keeped

@zchamski
Copy link
Contributor

zchamski commented Jan 9, 2025

@JeanRochCoulon: This issue does not apply to CV32A65X ==> please add the corresponding label.

@AyoubJalali
Copy link
Contributor Author

it's because we're disabling PMP ?

@JeanRochCoulon: This issue does not apply to CV32A65X ==> please add the corresponding label.

@JeanRochCoulon
Copy link
Contributor

Yes @AyoubJalali the PMP (will) be disable for 60x and 65x. That's why PMPCFG.A (will) by read only zero. If it is (will) not be the case, another issue could be open.

@AyoubJalali
Copy link
Contributor Author

@JeanRochCoulon Before doing that we should update the specification, then I can open an issue related to that

@zchamski
Copy link
Contributor

zchamski commented Jan 9, 2025

Normally we should igonre changing the pmpcfg.A when trying to write an illegal value, so the last legal value should be keeped

Handling such cases requies an enhancement to Spike configuration mechanism: currently there is no way to specify sets of legal values other than via AND+OR masks. Specifically, we need to represent arbitrary lists of legal values (example : RWX combinations in pmpNcfg. This is not a priority for CV32A65X (no PMP altogether), but should be kept in the back of our heads.

BTW, riscv-config supports arbitrary lists of legal values, so the limitation is only on the Spike side.

@MikeOpenHWGroup
Copy link
Member

BTW, riscv-config supports arbitrary lists of legal values

Indeed. Check out the WARL Node Definition section of RISC-V Config. Specifically the part about wr_illegal. It seems that RISC-V Config assumes that developers have a lot flexibility when it comes to dealing with illegal writes to WARL fields. I could not find anything in the ISA to contradict that.

The bottom-line here:

  1. @AyoubJalali's position is valid: we should ignore changing the pmpcfg.A when trying to write an illegal value, keeping the last legal value.
  2. The specification needs to be updated to specify the above behaviour. @AyoubJalali has volunteered to open an issue for that.
  3. @zchamski is right that handling such cases requires an enhancement to Spike configuration mechanism.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants