[PATCH] target/riscv: Ignore reserved bits in PTE for RV64

guoren@kernel.org posted 1 patch 2 years, 3 months ago
Test checkpatch passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20220117071439.1841844-1-guoren@kernel.org
Maintainers: Alistair Francis <alistair.francis@wdc.com>, Bin Meng <bin.meng@windriver.com>, Palmer Dabbelt <palmer@dabbelt.com>
There is a newer version of this series
target/riscv/cpu_bits.h   | 7 +++++++
target/riscv/cpu_helper.c | 2 +-
2 files changed, 8 insertions(+), 1 deletion(-)
[PATCH] target/riscv: Ignore reserved bits in PTE for RV64
Posted by guoren@kernel.org 2 years, 3 months ago
From: Guo Ren <ren_guo@c-sky.com>

Highest bits of PTE has been used for svpbmt, ref: [1], [2], so we
need to ignore them. They cannot be a part of ppn.

1: The RISC-V Instruction Set Manual, Volume II: Privileged Architecture
   4.4 Sv39: Page-Based 39-bit Virtual-Memory System
   4.5 Sv48: Page-Based 48-bit Virtual-Memory System

2: https://github.com/riscv/virtual-memory/blob/main/specs/663-Svpbmt-diff.pdf

Signed-off-by: Guo Ren <ren_guo@c-sky.com>
Tested-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Liu Zhiwei <zhiwei_liu@c-sky.com>
Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Cc: Weiwei Li <liweiwei@iscas.ac.cn>
Cc: Junqiang Wang <wangjunqiang@iscas.ac.cn>

---
 Changelog V7:
  - Update svpbmt reason in commit log

 Changelog V6:
  - Add Reviewer: Alistair Francis

 Changelog V5:
  - Add Reviewer and Tester: Bin Meng

 Changelog V4:
  - Change title to Ignore not Bugfix
  - Use PTE_PPN_MASK for RV32 and RV64

 Changelog V3:
  - Use UUL define for PTE_RESERVED
  - Keep ppn >> PTE_PPN_SHIFT

 Changelog V2:
  - Bugfix pte destroyed cause boot fail
  - Change to AND with a mask instead of shifting both directions
---
 target/riscv/cpu_bits.h   | 7 +++++++
 target/riscv/cpu_helper.c | 2 +-
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5106f0e769..fa22839d54 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -479,6 +479,13 @@ typedef enum {
 /* Page table PPN shift amount */
 #define PTE_PPN_SHIFT       10
 
+/* Page table PPN mask */
+#if defined(TARGET_RISCV32)
+#define PTE_PPN_MASK        0xffffffffUL
+#elif defined(TARGET_RISCV64)
+#define PTE_PPN_MASK        0x3fffffffffffffULL
+#endif
+
 /* Leaf page shift amount */
 #define PGSHIFT             12
 
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 7945f687b4..06c5403fdd 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -619,7 +619,7 @@ restart:
             return TRANSLATE_FAIL;
         }
 
-        hwaddr ppn = pte >> PTE_PPN_SHIFT;
+        hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
 
         if (!(pte & PTE_V)) {
             /* Invalid PTE */
-- 
2.25.1


Re: [PATCH] target/riscv: Ignore reserved bits in PTE for RV64
Posted by Alistair Francis 2 years, 3 months ago
On Mon, Jan 17, 2022 at 5:18 PM <guoren@kernel.org> wrote:
>
> From: Guo Ren <ren_guo@c-sky.com>
>
> Highest bits of PTE has been used for svpbmt, ref: [1], [2], so we
> need to ignore them. They cannot be a part of ppn.
>
> 1: The RISC-V Instruction Set Manual, Volume II: Privileged Architecture
>    4.4 Sv39: Page-Based 39-bit Virtual-Memory System
>    4.5 Sv48: Page-Based 48-bit Virtual-Memory System
>
> 2: https://github.com/riscv/virtual-memory/blob/main/specs/663-Svpbmt-diff.pdf
>
> Signed-off-by: Guo Ren <ren_guo@c-sky.com>
> Tested-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Liu Zhiwei <zhiwei_liu@c-sky.com>
> Reviewed-by: Bin Meng <bmeng.cn@gmail.com>
> Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
> Cc: Weiwei Li <liweiwei@iscas.ac.cn>
> Cc: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>
> ---
>  Changelog V7:
>   - Update svpbmt reason in commit log
>
>  Changelog V6:
>   - Add Reviewer: Alistair Francis
>
>  Changelog V5:
>   - Add Reviewer and Tester: Bin Meng
>
>  Changelog V4:
>   - Change title to Ignore not Bugfix
>   - Use PTE_PPN_MASK for RV32 and RV64
>
>  Changelog V3:
>   - Use UUL define for PTE_RESERVED
>   - Keep ppn >> PTE_PPN_SHIFT
>
>  Changelog V2:
>   - Bugfix pte destroyed cause boot fail
>   - Change to AND with a mask instead of shifting both directions
> ---
>  target/riscv/cpu_bits.h   | 7 +++++++
>  target/riscv/cpu_helper.c | 2 +-
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 5106f0e769..fa22839d54 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -479,6 +479,13 @@ typedef enum {
>  /* Page table PPN shift amount */
>  #define PTE_PPN_SHIFT       10
>
> +/* Page table PPN mask */
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK        0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK        0x3fffffffffffffULL
> +#endif
> +
>  /* Leaf page shift amount */
>  #define PGSHIFT             12
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 7945f687b4..06c5403fdd 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -619,7 +619,7 @@ restart:
>              return TRANSLATE_FAIL;
>          }
>
> -        hwaddr ppn = pte >> PTE_PPN_SHIFT;
> +        hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;

This will need to be dynamic based on get_xl()

It does look like we should check the existence of the extensions though:

"Bit 63 is reserved for use by the Svnapot extension in Chapter 5. If
Svnapot is not implemented, bit 63 remains reserved and must be zeroed
by software for forward compatibility, or else a page-fault exception
is raised. Bits 62–61 are reserved for use by the Svpbmt extension in
Chapter 6. If Svpbmt is not implemented, bits 62–61 remain reserved
and must be zeroed by software for forward compatibility, or else a
page-fault exception is raised."

Alistair

>
>          if (!(pte & PTE_V)) {
>              /* Invalid PTE */
> --
> 2.25.1
>
>