Re: Fast-track extension proposal V3 for "Sv32 Svpbmt"

Greg Favor

On Wed, Aug 17, 2022 at 5:19 AM Guo Ren <guoren@...> wrote:
Here is the third version of the proposal.

It seems like you have dropped a bunch of text from the prior versions - which is probably part of what leads to some of my questions below.
 =======================  Supervisor Extension Additions  ========================
\subsection{``Svpbmt32'' Extension for Page-Based Memory Types}

Svpbmt32 support is being added to allow the two highest bits of a PTE
to be used as PBMT instead of PA[33:32] for Sv32.

Maybe better to say "The Svpbmt32 extension allows the two ..."
The S-mode and G-stage address translation under this extension are

"are" -> "is"
controlled by the
menvcfg.PBMTE. The VS-stage address translation under this extension
is controlled by henvcfg.PBMTE and indirectly by menvcfg.PBMTE.

This needs to specify what "controlled" means.  There also is no explanation (here or below) of how menvcfg.PBMTE indirectly controls VS-stage address translation.

If this is all duplication of some of the currently defined *envcfg PBMTE functionality, then it is better to make clear that this is just reiterating a portion of the exact same functionality as currently exists for the *envcfg.PBMTE bits (and to refer to those current definitions for the details). Otherwise, by default, there is no clear understanding as to whether or not this extension specifies the exact same *envcfg.PBMTE behaviors as for the Svpbmt extension.
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.

This is being presented as non-normative text, but this sounds like it is or needs to be normative text?  Or is this "non-normatively" just repeating existing defined functionality - in which case what extra value is this text providing past repeating existing arch definitions?

Sv32 virtual address:
| 31  22 | 21  12 | 11        0 |
  VPN[1]   VPN[0]   page offset
10 10 12
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: 
| 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

There is no statement anywhere in this spec that says what exactly is the PBMT field.  I know you intend for it to have the exact same definition as the currently defined PBMT field, but this should be stated explicitly (to remove any possible ambiguity around whether PBMT encodings and/or meanings may be a little different in RV32 versus RV64).

Join { to automatically receive all group messages.