Re: [PATCH v2 1/1] target/riscv: log guest errors when reserved bits are set in PTEs

[Alistair Francis]

On Mon, Feb 3, 2025 at 4:20 PM julia <midnight@trainwit.ch> wrote:
>
> For instance, QEMUs newer than b6ecc63c569bb88c0fcadf79fb92bf4b88aefea8
> would silently treat this akin to an unmapped page (as required by the
> RISC-V spec, admittedly). However, not all hardware platforms do (e.g.
> CVA6) which leads to an apparent QEMU bug.
>
> Instead, log a guest error so that in future, incorrectly set up page
> tables can be debugged without bisecting QEMU.
>
> Signed-off-by: julia <midnight@trainwit.ch>

Thanks!

Applied to riscv-to-apply.next

Alistair

> ---
>  target/riscv/cpu_helper.c | 27 ++++++++++++++++++++++++++-
>  1 file changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index e1dfc4ecbf..3dd8e06044 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1226,14 +1226,27 @@ restart:
>              ppn = pte >> PTE_PPN_SHIFT;
>          } else {
>              if (pte & PTE_RESERVED) {
> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: reserved bits set in 
> PTE: "
> +                              "addr: 0x%" HWADDR_PRIx " pte: 0x" 
> TARGET_FMT_lx "\n",
> +                              __func__, pte_addr, pte);
>                  return TRANSLATE_FAIL;
>              }
>
>              if (!pbmte && (pte & PTE_PBMT)) {
> +                /* Reserved without Svpbmt. */
> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: PBMT bits set in PTE, "
> +                              "and Svpbmt extension is disabled: "
> +                              "addr: 0x%" HWADDR_PRIx " pte: 0x" 
> TARGET_FMT_lx "\n",
> +                              __func__, pte_addr, pte);
>                  return TRANSLATE_FAIL;
>              }
>
>              if (!riscv_cpu_cfg(env)->ext_svnapot && (pte & PTE_N)) {
> +                /* Reserved without Svnapot extension */
> +                qemu_log_mask(LOG_GUEST_ERROR, "%s: N bit set in PTE, "
> +                              "and Svnapot extension is disabled: "
> +                              "addr: 0x%" HWADDR_PRIx " pte: 0x" 
> TARGET_FMT_lx "\n",
> +                              __func__, pte_addr, pte);
>                  return TRANSLATE_FAIL;
>              }
>
> @@ -1244,14 +1257,19 @@ restart:
>              /* Invalid PTE */
>              return TRANSLATE_FAIL;
>          }
> +
>          if (pte & (PTE_R | PTE_W | PTE_X)) {
>              goto leaf;
>          }
>
> -        /* Inner PTE, continue walking */
>          if (pte & (PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> +            /* D, A, and U bits are reserved in non-leaf/inner PTEs */
> +            qemu_log_mask(LOG_GUEST_ERROR, "%s: D, A, or U bits set in 
> non-leaf PTE: "
> +                          "addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx 
> "\n",
> +                          __func__, pte_addr, pte);
>              return TRANSLATE_FAIL;
>          }
> +        /* Inner PTE, continue walking */
>          base = ppn << PGSHIFT;
>      }
>
> @@ -1261,10 +1279,17 @@ restart:
>   leaf:
>      if (ppn & ((1ULL << ptshift) - 1)) {
>          /* Misaligned PPN */
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: PPN bits in PTE is misaligned: "
> +                      "addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx "\n",
> +                      __func__, pte_addr, pte);
>          return TRANSLATE_FAIL;
>      }
>      if (!pbmte && (pte & PTE_PBMT)) {
>          /* Reserved without Svpbmt. */
> +        qemu_log_mask(LOG_GUEST_ERROR, "%s: PBMT bits set in PTE, "
> +                      "and Svpbmt extension is disabled: "
> +                      "addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx "\n",
> +                      __func__, pte_addr, pte);
>          return TRANSLATE_FAIL;
>      }
>
> --
> 2.47.0
>
>