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

Make explicit the priorities of synch. exceptions of H extension #711

Merged
merged 2 commits into from Aug 17, 2021
Merged

Make explicit the priorities of synch. exceptions of H extension #711

merged 2 commits into from Aug 17, 2021

Conversation

jhauser-us
Copy link
Collaborator

No description provided.

& 13 & Load page fault \\
\hline
& 23 & Store/AMO guest-page fault \\
& 21 & Load page guest-fault \\
Copy link
Collaborator

Choose a reason for hiding this comment

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

Typo: "Load page guest-fault" => "Load guest-page fault"

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Fixed.

@pdonahue-ventana
Copy link
Contributor

I think that guest page fault is only sometimes lower priority than page fault. Consider this example: We have two-stage translation enabled and we're fetching an instruction. We follow vsatp to fetch the first level of VS-stage page table but in doing so we must do the G-stage translation for the GPA that vsatp points to. We might get a guest page fault on that translation and that would be the highest-priority translation-related exception.

It can be argued that we can't get even get to the point of determining that there's a regular page fault until we've done the G-stage translations for the VS-stage table walk so therefore this priority is obvious. But it should also be obvious that we can't get an illegal instruction exception until after we've translated the PC to be able to fetch the instruction to start with. Yet what is obvious to you or me is not obvious to everyone so we (and other architectures) still have the exception priority table to make this clear. For instance, ARM seems to have 29 different items in their exception prioritization for a single stage of translation (not counting non-translation-related exceptions).

This problem is a bit larger than just the hypervisor chapter. The supervisor chapter should say that access faults on page table walks are higher priority than page faults.

@jhauser-us
Copy link
Collaborator Author

Paul Donahue wrote:

This problem is a bit larger than just the hypervisor chapter. The supervisor chapter should say that access faults on page table walks are higher priority than page faults.

Assuming there is a problem, then I agree; it isn't just the hypervisor chapter.

@gfavor
Copy link
Collaborator

gfavor commented Aug 17, 2021

I don't quite see where the problem is? During a tablewalk there are many opportunities to raise Access and Page faults. There is not and cannot be an absolute priority between AF's and PF's, e.g. a PF discovered later in a tablewalk should not take priority over reporting an AF discovered earlier. (For that matter, once a fault is discovered, the tablewalk terminates and there is no opportunity to carry on, do further fault checks, and discover another fault.)

Put differently, the priority order of reporting faults during a tablewalk is based on the exception check order spelled out in the tablewalk pseudocode in the Priv spec. There are only two things that are implicit in this pseudocode - at each point where a page table PTE read occurs:

  • Each VS-stage PTE read, in a nested manner, is subject to G-stage translation faults being detected during the associated G-stage tablewalk. Followed by PMA and PMP checks on the SPA (Supervisor PA) that is produced and before doing the actual PTE read.

  • Each G-stage PTE read is subject to PMA and PMP checks on the SPA before doing the actual PTE read.

In both cases these checks and any resultant faults occur at the point or step in the tablewalk pseudocode where that read is done.

Where the priority order between PF's and G-PF's needs to be stated is where both can arise on a normal load/store address translation in an architecturally unordered manner. But I don't think any such cases exist, i.e. all causes of PF's and G-PF's are covered by the tablewalk pseudocode's ordering of fault checks. Or what case(s) am I missing?

@jhauser-us
Copy link
Collaborator Author

Greg, if there is a problem, it's that the sections of the document that discuss exception priority don't spell out exactly what you wrote, that "the priority order of reporting faults during a tablewalk is based on the exception check order spelled out in the tablewalk pseudocode in the Priv spec." The question is whether the document should have to say that explicitly, or is this obvious enough already.

@gfavor
Copy link
Collaborator

gfavor commented Aug 17, 2021

@aswaterman Wrt the last posts by me and then John, do you think it is suitably understood that the sequence of steps in the tablewalk pseudocode in the Priv spec defines the priority order in which faults - due to checks performed at each step - should be recognized? Or does this need to be explicitly stated in the spec?

I have to admit that it's hard for me to imagine that the pseudocode sequence is not a normative statement of the order in which all the various described actions are to architecturally occur. Certainly during virt-mem discussions last year about updating that pseudocode in Priv 1.12, that was the underlying unstated and understood presumption.

@allenjbaum
Copy link

allenjbaum commented Aug 17, 2021 via email

@gfavor
Copy link
Collaborator

gfavor commented Aug 17, 2021

After some further discussion with John, I'll note that the explicit prioritization between PF's and G-PF's is meaningful when one considers how VS-stage and G-stage tablewalks interweave with each other and prevents any misreading of what is allowed. (Keeping in mind that a "PF over G-PF" priority is only relevant when there is a possibility of discovering both a PF and a G-PF at the same point or step in the overall sequence of steps to do a two-stage translation of an address.)

At the same time, the question remains (for Andrew to comment on) as to whether the tablewalk pseudocode should be clarified as to it specifying the order of fault reconition.

These are two related but distinct issues. This PR looks good and should go into the spec. Separately we'll resolve with Andrew whether some text is added to the Supervisor chapter of the Priv spec.

@pdonahue-ventana
Copy link
Contributor

For comparison, ARM basically blows the whole thing out for a single stage. For RISC-V it would be something like:

  • access fault on the implicit access to the first level PTE
  • page fault detected on the first level PTE
  • access fault on the implicit access to the second level PTE
  • page fault detected on the second level PTE
  • etc. as appropriate for Sv32/39/48/57 all the way down to the leaf
  • access fault when accessing the PA of the final translated address (the explicit access)

They have 29 priorities in their table which only handles a single stage. By my quick estimate, there would be an additional 80 items in their list if they had both stages listed. So in a full priority table (which they don't have) there would be 109 items for instruction fetch and another 109 items for load/store plus all the decode-related exceptions, etc. Their priority list would be about 12 pages long.

I definitely think that the algorithmic approach is better, though I think that it should be pointed out that the table (both in hypervisor and supervisor) is not complete with respect to implicit accesses and the access fault listed in the existing priority table should say that it's specifically the access fault for the explicit access.

@jhauser-us
Copy link
Collaborator Author

See #715.

@aswaterman
Copy link
Member

aswaterman commented Aug 17, 2021

@gfavor without a doubt, the pseudocode is normative. Nevertheless, I support the clarification. (With a single English statement, not by flattening the priorities into a lengthy list.)

@aswaterman aswaterman merged commit 3bf0816 into riscv:master Aug 17, 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.

None yet

5 participants