On Wed, Aug 3, 2022 at 12:46 AM Greg Favor <gfavor@...> wrote:

On Tue, Aug 2, 2022 at 4:48 AM Guo Ren <guoren@...> wrote:

and Hypervisor Extension (Chapter~\ref{hypervisor}). When satp.mode is Sv32x4

Satp doesn't support this mode. Neither does vsatp.

Yes, Sv32. It comes from hgatp, Sv32x4 means 34-bit virtual guest
physical address, right?

Yes.

The Svpbmt32 extension adds Svpbmt

The more proper way to say this is that PBMT support is being added (versus saying that an existing extension is being added).

Okay.

(Chapter~\ref{svpbmt}) support to
Sv32 implementations by reducing the physical address space from
34-bit to 32-bit to hold the highest 2 PBMT bits

Maybe say "... to allow use of the two highest bits instead as PBMT bits."

Okay.

. The S-mode and
G-stage address translation under this extension are controlled by the
menvcfg.PBMTE. The VS-stage address translation under this extension
is controlled by the henvcfg.PBMTE. The henvcfg.PBMTE depends on
menvcfg.PBMTE.

Maybe say "... controlled by henvcfg.PBMTE and indirectly also menvcfg.PBMTE."

Okay.

\begin{commentary}
For example, consider an RV32 system supporting Svpbmt32 and
Hypervisor Extension (Chapter~\ref{hypervisor}). When satp.mode is
Sv32 and menvcfg.PBMTE set, HS-mode virtual addresses and guest
physical addresses are translated to 32-bit physical addresses with
PBMT.

It would be better to describe this in the same way as the existing PBMTE description, i.e. "When menvcfg.PBMTE=1, Svpbmt32 is available for S-mode and G-stage address translation."

Ultimately these statements will need to be incorporated into the two existing PBMTE descriptions (for menvcfg and henvcfg), but in the meantime one should use the same wording as the existing descriptions.

Okay, I would try; see below.

When vsatp.mode is Sv32 and henvcfg.PBMTE set, guest virtual
addresses are translated to 32-bit guest physical address with PBMT.
\end{commentary}

Same comment as above.

Sv32 physical address with Svpbmt:

| 31 22 | 21 12 | 11 0 |
PPN[1] PPN[0] page offset
10 10 12

Sv32 page table entry with Svpbmt:

I assume the preceding should be 'Svpbmt32'.

Okay

| 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

What does 'PFN' stand for? But more importantly why change names from PPN to PFN (versus maintaining the same naming as in the existing spec)? Especially since PFN[0] is the same as PPN[0], and PFN[1] is just a smaller version of PPN[1].

Okay, I will fix it.


\subsection{Svpbmt32}
.\label{sec:translation}

Svpbmt32 support is being added to allow the two highest bits of PTE
used as PBMT for Sv32. The S-mode and G-stage address translation
under this extension are controlled by the menvcfg.PBMTE. The VS-stage
address translation under this extension is controlled by
henvcfg.PBMTE and menvcfg.PBMTE indirectly.

\begin{commentary}
For example, consider an RV32 system supporting Svpbmt32 and
Hypervisor Extension (Chapter~\ref{hypervisor}). When menvcfg.PBMTE=1,
Svpbmt32 is available for S-mode and G-stage address translation. When
henvcfg.PBMTE=1, Svpbmt32 is available for VS-mode address
translation.
\end{commentary}

Sv32 virtual address:
| 31 22 | 21 12 | 11 0 |
VPN[1] VPN[0] page offset
10 10 12

Sv32 physical address with Svpbmt32:
| 31 22 | 21 12 | 11 0 |
PPN[1] PPN[0] page offset
10 10 12

Sv32 page table entry with Svpbmt32:
| 31 30 | 29 20 | 19 10 | 9 8 | 7 | 6 | 5
| 4 | 3 | 2 | 1 | 0
PBMT PPN[1] PPN[0] reserved for SW D A G U X W R V

Greg

--
Best Regards
Guo Ren