Hi Greg,
Thx for the review, here is my reply.
On Tue, Aug 2, 2022 at 1:40 AM Greg Favor <gfavor@...> wrote:
Below are my comments ...
On Mon, Aug 1, 2022 at 3:34 AM Guo Ren <guoren@...> wrote:
First note that a ratified extension will probably appear in the Priv spec as a separate chapter (as was done with the three Sv* extensions ratified last year), or it might be included as part of the Svpbmt chapter as a separate section. Tbd.
Agree, I would put it in the Svpbmt chapter as a separate section.
The Sv32 Svpbmt extension adds Svpbmt (Chapter~\ref{svpbmt}) support to Sv32
implementations by reducing the physical address space from 34-bit to 32-bit,
when {\tt menvcfg}.PBMTE (for V=0) or
The OR is incorrect. For satp only menvcfg matters.
My meaning here is satp with V=0 or V=1, but I agree using vsatp is clearer.
For vsatp both menvcfg AND henvcfg matter - as is the case in general for features in VS/VU modes that are controlled by the *envcfg CSRs.
vsatp depends on henvcfg.PBMTE, and henvcfg.PBMTE depends on
menvcfg.PBMTE. Here is the rewritten sentence:
The Sv32 Svpbmt extension adds Svpbmt (Chapter~\ref{svpbmt}) support
to Sv32 implementations by reducing the physical address space from
34-bit to 32-bit and leaving the highest 2 bits of the leaf PTE as
PBMT properties for two-level translation. The satp and hgatp depend
on henvcfg.PBMTE, and the vsatp depends on henvcfg.PBMTE. When
menvcfg.PBMTE not set, henvcfg.PBMTE must be zero.
{\tt henvcfg}.PBMTE (for V=1)
... is ...
set. Then
the 20-bit VPN is translated into a 20-bit physical page number (PPN),
This is only true for 4 KiB pages. What about larger page sizes? This probably needs to be expressed in a manner that is page size agnostic.
I think the sentence could be removed. Look at the below commentary part.
and
the highest 2 bits
... of the leaf PTE ...
Yes, I've done.
are PBMT properties.
\begin{commentary}
For example, consider an RV32 system supporting Svpbmt and Hypervisor Extension
(Chapter~\ref{hypervisor}). When {\tt vsatp}.MODE is Sv32x4 and
the {\tt henvcfg}.PBMTE set, a 32-bit physical address is produced with PBMT
attribute when in VS-mode and VU-mode. When the value of {\tt satp}.MODE is
Sv32x4
Satp does not support this translation mode. And satp is not dependent on menvcfg. (Conversely vsatp is dependent on both memvcfg AND henvcfg.)
More generally this whole paragraph seems to either have a number of errors or is poorly worded. Also the intro - that says to consider a system supporting the H extension - would seem to focus in on vstap, but then the paragraph starts talking about satp. It probably would be good to have separate paragraphs for the satp functionality and the vsatp functionality (especially since this extension is not limited to use only in virtualized environments).
How about:
\begin{commentary}
For example, consider an RV32 system supporting Svpbmt and Hypervisor
Extension (Chapter~\ref{hypervisor}). When satp.mode is Sv32x4 and
menvcfg.PBMTE set, 32-bit physical address with PBMT is translated for
V=0. When vsatp.mode is Sv32x4 and henvcfg.PBMTE set, 32-bit physical
address with PBMT is translated for V=1.
\end{commentary}
and the {\tt menvcfg}.PBMTE set, a 32-bit physical address is produced
with PBMT attribute from 32-bit ({\tt henvcfg}.PBMTE = 1) or 34-bit
({\tt henvcfg}.PBMTE = 0) guest physical address.
\end{commentary}
Sv32 virtual address with Svpbmt:
| 31 22 | 21 12 | 11 0 |
VPN[1] VPN[0] page offset
10 10 12
Sv32 physical address with Svpbmt:
Isn't this for without Svpbmt?
| 31 22 | 21 12 | 11 0 |
PPN[1] PPN[0] page offset
10 10 12
Sv32 page table entry with Svpbmt:
| 31 30 | 29 10 | 9 8 | 7 | 6 | 5 | 4 | 3 | 2 | 1 | 0
MT[2] PFN reserved for SW D A G U X W R V
By merging PPN[1] and PPN[0], this seems to only apply to 4K leaf PTEs? Is that intended or why the merging?
I separate them in latex diff; the style is from the Linux comment.
| 31 30 | 29 20 | 19 10 | 9 8 | 7 | 6 | 5 |
4 | 3 | 2 | 1 | 0
PBMT PFN[1] PFN[0] reserved for SW D A G U X W R V
Why the '[2]' suffix to 'MT'? PFN doesn't have such a suffix. Conversely the above PPN's have a suffix to distinguish between the two PPN fields.
This means 2 bits for PBMT, it's from the Linux comment. I agree with
using the spec style here.
Greg
--
Best Regards
Guo Ren
