Open kdockser opened 1 year ago
Alternatively, we could add a normative description that explicitly calls out of these differences in two-stage translations.
The section 9.5.1 does call out these differences:
hgatp
substitutes for the usual satp
;U
bit, the current privilege mode is always taken to be U-mode; andD=1
as noted in section 5.3.2). However, any exception is always reported for the original access type (instruction, load, or store/AMO). G
bit in all G-stage PTEs is reserved for future standard use.We agree that section 9.5.1 calls out those differences. However, applying those differences to the single-stage step-by-step virtual address translation process does not yield a complete two-stage step-by-step description. There is no mention in these steps, for example, of when to set the D bit in a G-stage PTE as a result of the setting of an A bit in a VS-stage PTE.
Since people seem to be relying on these step-by-step instructions when creating models or RTL implementations, it would be helpful to create a version that explicitly and completely covers two-stage translation.
Probably would be useful for writing the complete ACT tests as well.
On Tue, Sep 5, 2023 at 12:09 PM Ken Dockser @.***> wrote:
We agree that section 9.5.1 calls out those differences. However, applying those differences to the single-stage step-by-step virtual address translation process does not yield a complete two-stage step-by-step description. There is no mention in these steps, for example, of when to set the D bit in a G-stage PTE as a result of the setting of an A bit in a VS-stage PTE.
Since people seem to be relying on these step-by-step instructions when creating models or RTL implementations, it would be helpful to create a version that explicitly and completely covers two-stage translation.
— Reply to this email directly, view it on GitHub https://github.com/riscv/riscv-isa-manual/issues/1103#issuecomment-1707175122, or unsubscribe https://github.com/notifications/unsubscribe-auth/AHPXVJSQXZJGYILHZVBX4A3XY52HDANCNFSM6AAAAAA4DJNDPI . You are receiving this because you are subscribed to this thread.Message ID: @.***>
Or (to Ken's and Ved's last posts) just add one more bullet to the section 9.5.1 bullet list that explicitly makes clear when to set the D bit in a G-stage PTE as a result of the setting of an A bit in a VS-stage PTE.
Section 9.5.1 states:
For a memory access made to support VS-stage address translation (such as to read/write a VS-level page table), permissions are checked as though for a load or store, not for the original access type.
For added clarity, the phrase "permissions are checked as though for a load or store" could be expanded to "permissions, A-bit, and the D-bit are checked as though for an implicit load or store".
"permissions, A-bit, and the D-bit are checked" reads in a confusing way. I assume this is trying to refer to setting of G-stage A&D bits as a function of the VS-level PTE read or write access (and not of the original access type)?
If so, then this might read better as:
For a memory access made to support VS-stage address translation (such as to read/write a VS-level page table), permissions and the need to set A and/or D bits at the G-stage level are checked as though for a load or store, not for the original access type.
Also, consider expanding "for a load or store" to explicitly state "for an implicit load or store." The distinction between implicit and explicit guides the decision on the value reported in [h|m]tinst
when a guest-page fault trap occurs.
As follows: For a memory access made to support VS-stage address translation (such as to read/write a VS-level page table), permissions and the need to set A and/or D bits at the G-stage level are checked as though for an implicit load or store, not for the original access type.
Thanks Ved and Greg. That is much better. While it makes sense to me, I think it still might be confusing to someone who doesn't already know the correct behavior. I think Greg's suggestion of "just add one more bullet" will make this more concrete:
- For a memory access made to support VS-stage address translation for a read or write (e.g., to a VS-level page table), permissions are checked as though for an implicit load or store (respectively), not for the original access type.
- For a memory access made to support G-stage address translation, step 7 shall be performed as if the original access type were a load when pointing to the page of a non-leaf VS-stage PTE, as if the original access type were a store for a leaf VS-stage PTE, otherwise it should be performed according to the original access type
Please clarify what the second bullet implies by "to support G-stage address translation". For example, in the scenario where the VS-stage has Sv48 active and the G-stage has Sv48x4 active, the translation steps would unfold as follows: vsatp
-> (G-1) -> VS-level-4 -> (G-2) -> VS-level-3 -> (G-3) -> VS-level-2 -> (G-4) -> VS-level-1 -{(G-5) -> VS-level-1}-> (G-6); where each G-x involves: G-level-4->G-level-3->G-level-2->G-level-1 to determine the SPA. For the invocation of the G-stage for VS-stage address translation—specifically at steps G-1, G-2, G-3, G-4, and optional G-5 —the bullet we discussed is applicable and are all implicit load to read a VS-stage PTE except G-5 which is a implicit store and is done optionally if a A or D bit needs to be set i.e. to write to VS-stage PTE. When translating the final GPA (i.e., the GPA computed by step 8 using the ppn from the VS-stage L1 PTE) that serves as the input for G6, the original access type applies. This is because G6 is not invoked for a memory access to support VS-stage address translation; rather, it is invoked for the original memory access type.
Created PR #1141 with the change discussed in this issue. @gfavor @kdockser
In the latest pdf build (5/23/23) of the Privileged Architecture, the step-by-step description in 5.3.2 Virtual Address Translation Process is clear and concise for single stage translation. However, when we move to two-stage translation, the differences specified in 9.5.1 Guest Physical Address Translation (e.g., hgatp substitutes for the usual satp) seem to be incomplete.
For example, applying step 7 in 5.3.2 (as updated by 9.5.1) "if the original memory access is a store, also set pte.d to 1" is a bit confusing when trying to apply it to G-stage translations. The intent is probably for all traversed G-stage PTEs to have A=1 and, if the operation that kicked off the original translation was a STORE, the final G-stage PTE (i.e. pointing to the SPA of the page of the STORE) needs D=1. We should have a clarification of some sort for this.
Furthermore, the updated steps in 5.3.2 do not provide any guidance on how to update the G-stage PTE that points to the VS-stage leaf cell. Presumably, if the VS-stage leaf cell is updated, this G-stage that points to it must have D=1.
It appears to me that the various models (SPIKE, RISC-V, and QEMU) are missing this last step (actually, SPIKE is currently being corrected based this issue being raised in Are G-stage D bits set correctly as a result of setting a VS-stage D bit? #1447 with the changes in A/D updates in G-stage PTE #1448)
I suggest that we add clarifications to the Privileged Architecture in the form of a step-by-step Two-stage Virtual Address Translation Process in 9.5.1 Guest Physical Address Translation. Alternatively, we could add a normative description that explicitly calls out of these differences in two-stage translations.