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

add check for pa width during MTT walk #88

Merged
merged 4 commits into from
Oct 8, 2024
Merged

add check for pa width during MTT walk #88

merged 4 commits into from
Oct 8, 2024

Conversation

rsahita
Copy link
Collaborator

@rsahita rsahita commented Oct 3, 2024

address issue #62 and other related

Signed-off-by: Ravi Sahita <ravi@rivosinc.com>
chapter4.adoc Outdated Show resolved Hide resolved
chapter4.adoc Outdated Show resolved Hide resolved
chapter4.adoc Outdated Show resolved Hide resolved
Co-authored-by: Ved Shanbhogue <91900059+ved-rivos@users.noreply.github.com>
Signed-off-by: Ravi Sahita <rsahita@yahoo.com>
chapter4.adoc Show resolved Hide resolved
chapter4.adoc Outdated Show resolved Hide resolved
chapter4.adoc Outdated Show resolved Hide resolved
chapter4.adoc Outdated Show resolved Hide resolved
chapter4.adoc Outdated
of using the maximum _accessible_ physical address space versus the maximum
_implemented_ physical address width also allows for the case where an Smmtt
mode is used that is lower width than what is implemented by the platform,
for example, Smmtt46 used with a platform maximum PA of say 56 bits.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Including in this paragraph access restrictions defined by this standard is circular logic: accesses to addresses above 2^46 are disallowed with Smmtt46 because they are not "accessible", but they are only inaccessible because they are disallowed by this same paragraph. (It's not circular for some platform-specific hardware limit, since those accesses would raise an access fault anyway without Smmtt.)

Also, being vague about what makes an address "accessible" creates ambiguity: addresses blocked by PMPs aren't accessible, so can I use a smaller MTT if I restrict S-mode to a smaller portion of the address space with PMPs?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree with the confusing "accessible" term - updating that to use "maximum-addressable" instead.

Re. the implementation note - so PMP are the last thing to be checked (and hence MTT walks are subject to PMPs), but both MTT and PMP have to allow the access for an address to become accessible - so if the PMP settings on a platform blocks access to a smaller region of memory than available on a platform, then yes, the MTT would only need to cover that region, since any access that violates that would be blocked by PMPs.

will update the implementation note as well to use "addressable" vs "accessible" - and the second part is probably confusing and not needed - so only keeping the software optimization of memory use for MTT structures in the note.

Copy link
Collaborator

@SiFiveHolland SiFiveHolland Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The two statements

so PMP are the last thing to be checked (and hence MTT walks are subject to PMPs)

and

yes, the MTT would only need to cover that region, since any access that violates that would be blocked by PMPs.

seem to be contradictory to me. If the PMPs are checked last, then the hardware could have started the MTT table walk, and thus performed an out-of-bounds read (possibly chasing a wild pointer from an out-of-bounds MTTL2 entry to an arbitrary "MTTL1" address that could be I/O with side effects) before the PMP rejects the original access.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardware is not allowed to bypass PMP/PMA for MTT walks. MTT accesses are themselves subject to PMA/PMP rules. Whether an implementation checks PMP for the original access in parallel with doing the MTT walk or does it serially (either before or after MTT walk) is implementation choice - the result should be the same i.e. original memory access is allowed only if PMP/PMA/MTT allow the access.

Copy link
Collaborator

@SiFiveHolland SiFiveHolland Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardware is not allowed to bypass PMP/PMA for MTT walks. MTT accesses are themselves subject to PMA/PMP rules.

"MTT structure accesses are to be treated as implicit M-mode accesses", so they use a different set of PMP permissions than the original S/U-mode access. And the MTT structure accesses are at a totally different address than the original access.

Whether an implementation checks PMP for the original access in parallel with doing the MTT walk or does it serially (either before or after MTT walk) is implementation choice - the result should be the same i.e. original memory access is allowed only if PMP/PMA/MTT allow the access.

Here's an example of what I mean:

Say I have a 32-bit system where the physical address space contains 1G of MMIO (0x00000000-0x40000000) followed by 2G of RAM (0x40000000-0xc0000000). So the platform's PAW is 32 bits. I then configure the PMP rules to restrict S-mode to the address range 0x00000000-0x80000000. You guys are saying that because of the PMP rule, I only need an MTTL2 with 64 entries, large enough to cover 31 bits of physical address space. I allocate the MTTL2 at, say, address 0x40100000. The memory immediately following the MTTL2 table (at 0x40100200) happens to contain other arbitrary data, with the first 4 bytes coincidentally having the value 0x01010000.

Now my S-mode software tries to access the address 0x80000004. My hardware does the MTT walk before the PMP check. The MTT walk accesses the out-of-bounds MTTL2 entry at 0x40100200 (because the address passes the PAW bounds check). This implicit access passes its own PMP check, because 1) the address 0x40100200 is in the allowed PMP range, and 2) the MTT walk operates with M-mode permissions anyway. This is a valid MTTL2 entry, pointing to an MTTL1 at physical address 0x10000000.

The MTT walk then accesses the first entry of (what it thinks is an) MTTL1 table at 0x10000000. This implicit access also passes its PMP check, for the same two reasons. But 0x10000000 is really the MMIO address of my NS16550 UART, so the MTT checker has just consumed and discarded a character from the UART's receive FIFO. The character happens to be "w" (0x77), which has the low two bits set, so the MTT checker interprets this as allowing read, write, and execute access.

Now the PMP check for the original S-mode access is performed. It denies the access, because 0x80000004 is outside the range allowed by the PMP rules, but the MTT checker has already eaten a byte of my UART input! This is clearly a different result than if the PMP rejection of the S-mode access prevented the MTT walk from occurring entirely.

Maybe I'm missing something, but this appears to be a counterexample to your statement. Either 1) the PMP denial must prevent the MTT walk from occurring, or 2) the MTT must be sized the same regardless of PMP settings.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A clarifying question - if an implementation follows the spec with the updated step 2 - where the pa should not be beyond the platform-defined maximum-addressable physical address for the hart (which should be 0x80000000 in your example), then the MTT walk should not access the out-of-bounds MTTL2 entry at 0x40100200?

Copy link
Collaborator

@SiFiveHolland SiFiveHolland Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, the platform-defined maximum-addressable physical address for the hart must be at least 0xbfffffff, because that's the end of DRAM. The addresses from 0x80000000-0xc0000000 are still currently accessible by M-mode on that hart even with the PMP configuration I described. More generally, I would consider an address range to be addressable even if access is being temporarily blocked by a permission check.

Is your interpretation that any memory access that, in the absence of Smmtt, would raise an access fault for absolutely any reason, should fail at step 2? If so, then we should just say that. I think that's reasonable. But that wasn't my existing interpretation, and I don't know if that interpretation is reasonable from a hardware design perspective.

Copy link
Collaborator

@ved-rivos ved-rivos Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You guys are saying that because of the PMP rule, I only need an MTTL2 with 64 entries, large enough to cover 31 bits of physical address space. I allocate the MTTL2 at, say, address 0x40100000.

The text is saying "addressable" and not "accessible". In a 32-bit system where 0 to 2^32-1 is addressable, the MTT must cover all of that memory. On other hand consider a 32-bit system that really only has a 24 bit physical address width i.e. maximum addressable memory is 16 MB. In such a system the MTT only need to be sized to cover 16 MiB since any address beyond 16 MiB would not even initiate a MTT walk.

So there are two gates:

  1. The incoming PA must be addressable.
  2. If it is addressable it must be accessible in PMA, PMP, and MTT

If the PA is not addressable a) because its wider than that supported by the present MTT mode or b) is wider than the platfoms maximum addressable physical address - then a MTT walk does not occur and the access gets an access fault.

The MTT must have enough valid entries for all addressable physical memory i.e. minimum of the bound defined by the MTT mode or the the platforms maximum addressable physical address.

Copy link
Collaborator Author

@rsahita rsahita Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry yes I misread - so in your example, the MTT would need to cover the 3G addressable memory space.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Got it, thanks!

rsahita and others added 2 commits October 7, 2024 15:33
Co-authored-by: Samuel Holland <samuel.holland@sifive.com>
Signed-off-by: Ravi Sahita <rsahita@yahoo.com>
Signed-off-by: Ravi Sahita <ravi@rivosinc.com>
Copy link
Collaborator

@SiFiveHolland SiFiveHolland left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@rsahita rsahita merged commit b4db5a9 into main Oct 8, 2024
1 check passed
@rsahita rsahita deleted the issue/62 branch October 8, 2024 22:22
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.

3 participants