Skip to content

mmu: enforce full PMP match across memory operations - #1889

Open
Syed-Moeed-Ali wants to merge 1 commit into
openhwfoundation:mainfrom
Syed-Moeed-Ali:fix/pmp-full-access-match
Open

Syed-Moeed-Ali wants to merge 1 commit into
openhwfoundation:mainfrom
Syed-Moeed-Ali:fix/pmp-full-access-match

Conversation

@Syed-Moeed-Ali

Copy link
Copy Markdown
Contributor

Partially addresses #1833 (PMP item 1).

Summary

The PMP checker currently determines the matching PMP entry using only the starting physical address of a memory operation.

According to the RISC-V privileged specification, the lowest-numbered PMP entry that matches any byte of an access must match all bytes of that access. If a memory operation crosses into another PMP region, the access must fail even if both regions independently allow the requested operation.

Changes

This PR:

Computes the address of the final byte of each memory operation.
Performs PMP matching on both the starting and ending addresses.
Determines the highest-priority PMP entry for both endpoints.
Raises an access fault when the first and last byte match different PMP entries.
Applies the boundary check independently of the PMP R/W/X permission bits.
Uses the cache-line access size for cache-block operations.
Why this is needed

Previously, an access beginning inside a permitted PMP entry could extend beyond that entry without checking whether the same PMP entry covered the complete operation.

For example, an access crossing between two adjacent readable PMP regions must still fault because no single PMP entry covers the entire access.

This change compares the selected PMP entry for the first and last byte instead of only checking permissions at the ending address.

Scope

This PR specifically addresses the PMP full-memory-operation matching issue described in #1833.

Other MMU issues tracked in #1833 are outside the scope of this change.

Signed-off-by: Syed-Moeed-Ali <syedmoeedali2006@gmail.com>
@davidharrishmc

Copy link
Copy Markdown
Contributor

Thank you for looking at this. I'm not certain the bug is real. PMP is subtle.

riscvassertions_wally sets minimum bounds on G when misaligned accesses are possible. They are intended to avoid any cases where the end needs to be checked. Also, note that accesses spanning cache line boundaries are split into multiple accesses, each of which should satisfy the PMP rule.

Can you construct any test cases (and configurations) where this fix is truly needed?

This PR adds a significant amount of hardware, so I'm reluctant to add it unless the problem is real. If the problem is real, let's look for the lowest-cost solution.

    assert (P.PMP_G > 0 | P.XLEN == 32 | P.PMP_ENTRIES == 0) else $fatal(1, "RV64 requires PMP_G at least 1 to avoid checking for 8-byte accesses to 4-byte region");
    assert ((P.PMP_G >= $clog2(P.DCACHE_LINELENINBITS/8)-2) | !P.ZICCLSM_SUPPORTED | P.PMP_ENTRIES == 0) else $fatal(1, "Systems that support misaligned data with PMP must have grain size of at least one cache line so accesses that span grains will also cause spills");
    assert ((P.PMP_G >= $clog2(P.ICACHE_LINELENINBITS/8)-2) | !P.ZCA_SUPPORTED | (P.PMP_ENTRIES == 0) | !P.ICACHE_SUPPORTED) else $fatal(1, "Systems that support compressed instructions with PMP must have grain size of at least one cache line so fetches that span grains will also cause spills");

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.

2 participants