* [PATCH v5 0/5] support subsets of virtual memory extension
@ 2022-01-18 1:17 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, Weiwei Li, lazyparser
This patchset implements virtual memory related RISC-V extensions: Svnapot version 1.0, Svinval vesion 1.0, Svpbmt version 1.0.
Specification:
https://github.com/riscv/virtual-memory/tree/main/specs
The port is available here:
https://github.com/plctlab/plct-qemu/tree/plct-virtmem-upstream-v5
To test this implementation, specify cpu argument with 'svinval=true,svnapot=true,svpbmt=true'.
This implementation can pass the riscv-tests for rv64ssvnapot.
v5:
* merge patch https://lore.kernel.org/qemu-devel/1569456861-8502-1-git-send-email-guoren@kernel.org/
* fix type compatibility in RV32
v4:
* fix encodings for hinval_vvma and hinval_gvma
* partition inner PTE check into several steps
* improve commit messages to describe changes
v3:
* drop "x-" in exposed properties
v2:
* add extension check for svnapot and svpbmt
Guo Ren (1):
target/riscv: Ignore reserved bits in PTE for RV64
Weiwei Li (4):
target/riscv: add PTE_A/PTE_D/PTE_U bits check for inner PTE
target/riscv: add support for svnapot extension
target/riscv: add support for svinval extension
target/riscv: add support for svpbmt extension
target/riscv/cpu.c | 4 ++
target/riscv/cpu.h | 3 +
target/riscv/cpu_bits.h | 10 +++
target/riscv/cpu_helper.c | 23 ++++++-
target/riscv/insn32.decode | 7 ++
target/riscv/insn_trans/trans_svinval.c.inc | 75 +++++++++++++++++++++
target/riscv/translate.c | 1 +
7 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 target/riscv/insn_trans/trans_svinval.c.inc
--
2.17.1
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v5 0/5] support subsets of virtual memory extension
@ 2022-01-18 1:17 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
This patchset implements virtual memory related RISC-V extensions: Svnapot version 1.0, Svinval vesion 1.0, Svpbmt version 1.0.
Specification:
https://github.com/riscv/virtual-memory/tree/main/specs
The port is available here:
https://github.com/plctlab/plct-qemu/tree/plct-virtmem-upstream-v5
To test this implementation, specify cpu argument with 'svinval=true,svnapot=true,svpbmt=true'.
This implementation can pass the riscv-tests for rv64ssvnapot.
v5:
* merge patch https://lore.kernel.org/qemu-devel/1569456861-8502-1-git-send-email-guoren@kernel.org/
* fix type compatibility in RV32
v4:
* fix encodings for hinval_vvma and hinval_gvma
* partition inner PTE check into several steps
* improve commit messages to describe changes
v3:
* drop "x-" in exposed properties
v2:
* add extension check for svnapot and svpbmt
Guo Ren (1):
target/riscv: Ignore reserved bits in PTE for RV64
Weiwei Li (4):
target/riscv: add PTE_A/PTE_D/PTE_U bits check for inner PTE
target/riscv: add support for svnapot extension
target/riscv: add support for svinval extension
target/riscv: add support for svpbmt extension
target/riscv/cpu.c | 4 ++
target/riscv/cpu.h | 3 +
target/riscv/cpu_bits.h | 10 +++
target/riscv/cpu_helper.c | 23 ++++++-
target/riscv/insn32.decode | 7 ++
target/riscv/insn_trans/trans_svinval.c.inc | 75 +++++++++++++++++++++
target/riscv/translate.c | 1 +
7 files changed, 120 insertions(+), 3 deletions(-)
create mode 100644 target/riscv/insn_trans/trans_svinval.c.inc
--
2.17.1
^ permalink raw reply [flat|nested] 52+ messages in thread
* [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 1:17 ` Weiwei Li
(?)
@ 2022-01-18 1:17 ` Weiwei Li
2022-01-18 3:30 ` Anup Patel
-1 siblings, 1 reply; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Guo Ren
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>
---
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 5a6d49aa64..282cd8eecd 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -490,6 +490,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 434a83e66a..26608ddf1c 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.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 2/5] target/riscv: add PTE_A/PTE_D/PTE_U bits check for inner PTE
2022-01-18 1:17 ` Weiwei Li
@ 2022-01-18 1:17 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, Weiwei Li, lazyparser
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 26608ddf1c..1820188f41 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -626,6 +626,9 @@ restart:
return TRANSLATE_FAIL;
} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
/* Inner PTE, continue walking */
+ if (pte & (PTE_D | PTE_A | PTE_U)) {
+ return TRANSLATE_FAIL;
+ }
base = ppn << PGSHIFT;
} else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
/* Reserved leaf PTE flags: PTE_W */
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 2/5] target/riscv: add PTE_A/PTE_D/PTE_U bits check for inner PTE
@ 2022-01-18 1:17 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu_helper.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 26608ddf1c..1820188f41 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -626,6 +626,9 @@ restart:
return TRANSLATE_FAIL;
} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
/* Inner PTE, continue walking */
+ if (pte & (PTE_D | PTE_A | PTE_U)) {
+ return TRANSLATE_FAIL;
+ }
base = ppn << PGSHIFT;
} else if ((pte & (PTE_R | PTE_W | PTE_X)) == PTE_W) {
/* Reserved leaf PTE flags: PTE_W */
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 3/5] target/riscv: add support for svnapot extension
2022-01-18 1:17 ` Weiwei Li
@ 2022-01-18 1:17 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, Weiwei Li, lazyparser
- add PTE_N bit
- add PTE_N bit check for inner PTE
- update address translation to support 64KiB continuous region (napot_bits = 4)
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Cc: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 1 +
target/riscv/cpu_helper.c | 18 +++++++++++++++---
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9bc25d3055..ff6c86c85b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -668,6 +668,8 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+ DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
+
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4d63086765..d3d17cde82 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -327,6 +327,7 @@ struct RISCVCPU {
bool ext_counters;
bool ext_ifencei;
bool ext_icsr;
+ bool ext_svnapot;
bool ext_zfh;
bool ext_zfhmin;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 282cd8eecd..5501e9698b 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -486,6 +486,7 @@ typedef enum {
#define PTE_A 0x040 /* Accessed */
#define PTE_D 0x080 /* Dirty */
#define PTE_SOFT 0x300 /* Reserved for Software */
+#define PTE_N 0x8000000000000000 /* NAPOT translation */
/* Page table PPN shift amount */
#define PTE_PPN_SHIFT 10
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1820188f41..c276760c7f 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -621,12 +621,13 @@ restart:
hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ RISCVCPU *cpu = env_archcpu(env);
if (!(pte & PTE_V)) {
/* Invalid PTE */
return TRANSLATE_FAIL;
} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
/* Inner PTE, continue walking */
- if (pte & (PTE_D | PTE_A | PTE_U)) {
+ if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
return TRANSLATE_FAIL;
}
base = ppn << PGSHIFT;
@@ -702,8 +703,19 @@ restart:
/* for superpage mappings, make a fake leaf PTE for the TLB's
benefit. */
target_ulong vpn = addr >> PGSHIFT;
- *physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
- (addr & ~TARGET_PAGE_MASK);
+
+ int napot_bits = 0;
+ if (cpu->cfg.ext_svnapot && (pte & (target_ulong)PTE_N)) {
+ napot_bits = ctzl(ppn) + 1;
+ if ((i != (levels - 1)) || (napot_bits != 4)) {
+ return TRANSLATE_FAIL;
+ }
+ }
+
+ *physical = (((ppn & ~(((target_ulong)1 << napot_bits) - 1)) |
+ (vpn & (((target_ulong)1 << napot_bits) - 1)) |
+ (vpn & (((target_ulong)1 << ptshift) - 1))
+ ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
/* set permissions on the TLB entry */
if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 3/5] target/riscv: add support for svnapot extension
@ 2022-01-18 1:17 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
- add PTE_N bit
- add PTE_N bit check for inner PTE
- update address translation to support 64KiB continuous region (napot_bits = 4)
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Cc: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu.c | 2 ++
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 1 +
target/riscv/cpu_helper.c | 18 +++++++++++++++---
4 files changed, 19 insertions(+), 3 deletions(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 9bc25d3055..ff6c86c85b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -668,6 +668,8 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+ DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
+
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index 4d63086765..d3d17cde82 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -327,6 +327,7 @@ struct RISCVCPU {
bool ext_counters;
bool ext_ifencei;
bool ext_icsr;
+ bool ext_svnapot;
bool ext_zfh;
bool ext_zfhmin;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 282cd8eecd..5501e9698b 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -486,6 +486,7 @@ typedef enum {
#define PTE_A 0x040 /* Accessed */
#define PTE_D 0x080 /* Dirty */
#define PTE_SOFT 0x300 /* Reserved for Software */
+#define PTE_N 0x8000000000000000 /* NAPOT translation */
/* Page table PPN shift amount */
#define PTE_PPN_SHIFT 10
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 1820188f41..c276760c7f 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -621,12 +621,13 @@ restart:
hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ RISCVCPU *cpu = env_archcpu(env);
if (!(pte & PTE_V)) {
/* Invalid PTE */
return TRANSLATE_FAIL;
} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
/* Inner PTE, continue walking */
- if (pte & (PTE_D | PTE_A | PTE_U)) {
+ if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
return TRANSLATE_FAIL;
}
base = ppn << PGSHIFT;
@@ -702,8 +703,19 @@ restart:
/* for superpage mappings, make a fake leaf PTE for the TLB's
benefit. */
target_ulong vpn = addr >> PGSHIFT;
- *physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
- (addr & ~TARGET_PAGE_MASK);
+
+ int napot_bits = 0;
+ if (cpu->cfg.ext_svnapot && (pte & (target_ulong)PTE_N)) {
+ napot_bits = ctzl(ppn) + 1;
+ if ((i != (levels - 1)) || (napot_bits != 4)) {
+ return TRANSLATE_FAIL;
+ }
+ }
+
+ *physical = (((ppn & ~(((target_ulong)1 << napot_bits) - 1)) |
+ (vpn & (((target_ulong)1 << napot_bits) - 1)) |
+ (vpn & (((target_ulong)1 << ptshift) - 1))
+ ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
/* set permissions on the TLB entry */
if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 4/5] target/riscv: add support for svinval extension
2022-01-18 1:17 ` Weiwei Li
@ 2022-01-18 1:17 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, Weiwei Li, lazyparser
- sinval.vma, hinval.vvma and hinval.gvma do the same as sfence.vma, hfence.vvma and hfence.gvma except extension check
- do nothing other than extension check for sfence.w.inval and sfence.inval.ir
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 1 +
target/riscv/insn32.decode | 7 ++
target/riscv/insn_trans/trans_svinval.c.inc | 75 +++++++++++++++++++++
target/riscv/translate.c | 1 +
5 files changed, 85 insertions(+)
create mode 100644 target/riscv/insn_trans/trans_svinval.c.inc
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ff6c86c85b..45ac98e06b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -668,6 +668,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+ DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d3d17cde82..c3d1845ca1 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -327,6 +327,7 @@ struct RISCVCPU {
bool ext_counters;
bool ext_ifencei;
bool ext_icsr;
+ bool ext_svinval;
bool ext_svnapot;
bool ext_zfh;
bool ext_zfhmin;
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 5bbedc254c..1d3ff1efe1 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -809,3 +809,10 @@ fcvt_l_h 1100010 00010 ..... ... ..... 1010011 @r2_rm
fcvt_lu_h 1100010 00011 ..... ... ..... 1010011 @r2_rm
fcvt_h_l 1101010 00010 ..... ... ..... 1010011 @r2_rm
fcvt_h_lu 1101010 00011 ..... ... ..... 1010011 @r2_rm
+
+# *** Svinval Standard Extension ***
+sinval_vma 0001011 ..... ..... 000 00000 1110011 @sfence_vma
+sfence_w_inval 0001100 00000 00000 000 00000 1110011
+sfence_inval_ir 0001100 00001 00000 000 00000 1110011
+hinval_vvma 0010011 ..... ..... 000 00000 1110011 @hfence_vvma
+hinval_gvma 0110011 ..... ..... 000 00000 1110011 @hfence_gvma
diff --git a/target/riscv/insn_trans/trans_svinval.c.inc b/target/riscv/insn_trans/trans_svinval.c.inc
new file mode 100644
index 0000000000..1dde665661
--- /dev/null
+++ b/target/riscv/insn_trans/trans_svinval.c.inc
@@ -0,0 +1,75 @@
+/*
+ * RISC-V translation routines for the Svinval Standard Instruction Set.
+ *
+ * Copyright (c) 2020-2021 PLCT lab
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define REQUIRE_SVINVAL(ctx) do { \
+ if (!RISCV_CPU(ctx->cs)->cfg.ext_svinval) { \
+ return false; \
+ } \
+} while (0)
+
+static bool trans_sinval_vma(DisasContext *ctx, arg_sinval_vma *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ /* Do the same as sfence.vma currently */
+ REQUIRE_EXT(ctx, RVS);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_tlb_flush(cpu_env);
+ return true;
+#endif
+ return false;
+}
+
+static bool trans_sfence_w_inval(DisasContext *ctx, arg_sfence_w_inval *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ REQUIRE_EXT(ctx, RVS);
+ /* Do nothing currently */
+ return true;
+}
+
+static bool trans_sfence_inval_ir(DisasContext *ctx, arg_sfence_inval_ir *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ REQUIRE_EXT(ctx, RVS);
+ /* Do nothing currently */
+ return true;
+}
+
+static bool trans_hinval_vvma(DisasContext *ctx, arg_hinval_vvma *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ /* Do the same as hfence.vvma currently */
+ REQUIRE_EXT(ctx, RVH);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_hyp_tlb_flush(cpu_env);
+ return true;
+#endif
+ return false;
+}
+
+static bool trans_hinval_gvma(DisasContext *ctx, arg_hinval_gvma *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ /* Do the same as hfence.gvma currently */
+ REQUIRE_EXT(ctx, RVH);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_hyp_gvma_tlb_flush(cpu_env);
+ return true;
+#endif
+ return false;
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 615048ec87..4e5a9660a4 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -838,6 +838,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
#include "insn_trans/trans_rvb.c.inc"
#include "insn_trans/trans_rvzfh.c.inc"
#include "insn_trans/trans_privileged.c.inc"
+#include "insn_trans/trans_svinval.c.inc"
/* Include the auto-generated decoder for 16 bit insn */
#include "decode-insn16.c.inc"
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 4/5] target/riscv: add support for svinval extension
@ 2022-01-18 1:17 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li
- sinval.vma, hinval.vvma and hinval.gvma do the same as sfence.vma, hfence.vvma and hfence.gvma except extension check
- do nothing other than extension check for sfence.w.inval and sfence.inval.ir
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Reviewed-by: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 1 +
target/riscv/insn32.decode | 7 ++
target/riscv/insn_trans/trans_svinval.c.inc | 75 +++++++++++++++++++++
target/riscv/translate.c | 1 +
5 files changed, 85 insertions(+)
create mode 100644 target/riscv/insn_trans/trans_svinval.c.inc
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index ff6c86c85b..45ac98e06b 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -668,6 +668,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
+ DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index d3d17cde82..c3d1845ca1 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -327,6 +327,7 @@ struct RISCVCPU {
bool ext_counters;
bool ext_ifencei;
bool ext_icsr;
+ bool ext_svinval;
bool ext_svnapot;
bool ext_zfh;
bool ext_zfhmin;
diff --git a/target/riscv/insn32.decode b/target/riscv/insn32.decode
index 5bbedc254c..1d3ff1efe1 100644
--- a/target/riscv/insn32.decode
+++ b/target/riscv/insn32.decode
@@ -809,3 +809,10 @@ fcvt_l_h 1100010 00010 ..... ... ..... 1010011 @r2_rm
fcvt_lu_h 1100010 00011 ..... ... ..... 1010011 @r2_rm
fcvt_h_l 1101010 00010 ..... ... ..... 1010011 @r2_rm
fcvt_h_lu 1101010 00011 ..... ... ..... 1010011 @r2_rm
+
+# *** Svinval Standard Extension ***
+sinval_vma 0001011 ..... ..... 000 00000 1110011 @sfence_vma
+sfence_w_inval 0001100 00000 00000 000 00000 1110011
+sfence_inval_ir 0001100 00001 00000 000 00000 1110011
+hinval_vvma 0010011 ..... ..... 000 00000 1110011 @hfence_vvma
+hinval_gvma 0110011 ..... ..... 000 00000 1110011 @hfence_gvma
diff --git a/target/riscv/insn_trans/trans_svinval.c.inc b/target/riscv/insn_trans/trans_svinval.c.inc
new file mode 100644
index 0000000000..1dde665661
--- /dev/null
+++ b/target/riscv/insn_trans/trans_svinval.c.inc
@@ -0,0 +1,75 @@
+/*
+ * RISC-V translation routines for the Svinval Standard Instruction Set.
+ *
+ * Copyright (c) 2020-2021 PLCT lab
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2 or later, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ *
+ * You should have received a copy of the GNU General Public License along with
+ * this program. If not, see <http://www.gnu.org/licenses/>.
+ */
+
+#define REQUIRE_SVINVAL(ctx) do { \
+ if (!RISCV_CPU(ctx->cs)->cfg.ext_svinval) { \
+ return false; \
+ } \
+} while (0)
+
+static bool trans_sinval_vma(DisasContext *ctx, arg_sinval_vma *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ /* Do the same as sfence.vma currently */
+ REQUIRE_EXT(ctx, RVS);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_tlb_flush(cpu_env);
+ return true;
+#endif
+ return false;
+}
+
+static bool trans_sfence_w_inval(DisasContext *ctx, arg_sfence_w_inval *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ REQUIRE_EXT(ctx, RVS);
+ /* Do nothing currently */
+ return true;
+}
+
+static bool trans_sfence_inval_ir(DisasContext *ctx, arg_sfence_inval_ir *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ REQUIRE_EXT(ctx, RVS);
+ /* Do nothing currently */
+ return true;
+}
+
+static bool trans_hinval_vvma(DisasContext *ctx, arg_hinval_vvma *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ /* Do the same as hfence.vvma currently */
+ REQUIRE_EXT(ctx, RVH);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_hyp_tlb_flush(cpu_env);
+ return true;
+#endif
+ return false;
+}
+
+static bool trans_hinval_gvma(DisasContext *ctx, arg_hinval_gvma *a)
+{
+ REQUIRE_SVINVAL(ctx);
+ /* Do the same as hfence.gvma currently */
+ REQUIRE_EXT(ctx, RVH);
+#ifndef CONFIG_USER_ONLY
+ gen_helper_hyp_gvma_tlb_flush(cpu_env);
+ return true;
+#endif
+ return false;
+}
diff --git a/target/riscv/translate.c b/target/riscv/translate.c
index 615048ec87..4e5a9660a4 100644
--- a/target/riscv/translate.c
+++ b/target/riscv/translate.c
@@ -838,6 +838,7 @@ static uint32_t opcode_at(DisasContextBase *dcbase, target_ulong pc)
#include "insn_trans/trans_rvb.c.inc"
#include "insn_trans/trans_rvzfh.c.inc"
#include "insn_trans/trans_privileged.c.inc"
+#include "insn_trans/trans_svinval.c.inc"
/* Include the auto-generated decoder for 16 bit insn */
#include "decode-insn16.c.inc"
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 5/5] target/riscv: add support for svpbmt extension
2022-01-18 1:17 ` Weiwei Li
@ 2022-01-18 1:17 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, Weiwei Li, lazyparser, Heiko Stuebner
- add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
- add PTE_PBMT bit check for inner PTE
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 2 ++
target/riscv/cpu_helper.c | 4 +++-
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 45ac98e06b..4f82bd00a3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
+ DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c3d1845ca1..53f314c752 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -329,6 +329,7 @@ struct RISCVCPU {
bool ext_icsr;
bool ext_svinval;
bool ext_svnapot;
+ bool ext_svpbmt;
bool ext_zfh;
bool ext_zfhmin;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5501e9698b..24b7eb2b1f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -486,7 +486,9 @@ typedef enum {
#define PTE_A 0x040 /* Accessed */
#define PTE_D 0x080 /* Dirty */
#define PTE_SOFT 0x300 /* Reserved for Software */
+#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
#define PTE_N 0x8000000000000000 /* NAPOT translation */
+#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
/* Page table PPN shift amount */
#define PTE_PPN_SHIFT 10
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index c276760c7f..9fffaccffb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -625,9 +625,11 @@ restart:
if (!(pte & PTE_V)) {
/* Invalid PTE */
return TRANSLATE_FAIL;
+ } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
+ return TRANSLATE_FAIL;
} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
/* Inner PTE, continue walking */
- if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
+ if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
return TRANSLATE_FAIL;
}
base = ppn << PGSHIFT;
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* [PATCH v5 5/5] target/riscv: add support for svpbmt extension
@ 2022-01-18 1:17 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 1:17 UTC (permalink / raw)
To: anup, palmer, alistair.francis, bin.meng, qemu-riscv, qemu-devel
Cc: wangjunqiang, lazyparser, Weiwei Li, Heiko Stuebner
- add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
- add PTE_PBMT bit check for inner PTE
Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
Cc: Heiko Stuebner <heiko@sntech.de>
Cc: Anup Patel <anup@brainfault.org>
---
target/riscv/cpu.c | 1 +
target/riscv/cpu.h | 1 +
target/riscv/cpu_bits.h | 2 ++
target/riscv/cpu_helper.c | 4 +++-
4 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 45ac98e06b..4f82bd00a3 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
+ DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c3d1845ca1..53f314c752 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -329,6 +329,7 @@ struct RISCVCPU {
bool ext_icsr;
bool ext_svinval;
bool ext_svnapot;
+ bool ext_svpbmt;
bool ext_zfh;
bool ext_zfhmin;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index 5501e9698b..24b7eb2b1f 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -486,7 +486,9 @@ typedef enum {
#define PTE_A 0x040 /* Accessed */
#define PTE_D 0x080 /* Dirty */
#define PTE_SOFT 0x300 /* Reserved for Software */
+#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
#define PTE_N 0x8000000000000000 /* NAPOT translation */
+#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
/* Page table PPN shift amount */
#define PTE_PPN_SHIFT 10
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index c276760c7f..9fffaccffb 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -625,9 +625,11 @@ restart:
if (!(pte & PTE_V)) {
/* Invalid PTE */
return TRANSLATE_FAIL;
+ } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
+ return TRANSLATE_FAIL;
} else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
/* Inner PTE, continue walking */
- if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
+ if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
return TRANSLATE_FAIL;
}
base = ppn << PGSHIFT;
--
2.17.1
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 1:17 ` [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64 Weiwei Li
@ 2022-01-18 3:30 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 3:30 UTC (permalink / raw)
To: Weiwei Li
Cc: Wei Wu (吴伟),
open list:RISC-V, wangjunqiang, Bin Meng, QEMU Developers,
Alistair Francis, Guo Ren, Palmer Dabbelt
On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> ---
> 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 5a6d49aa64..282cd8eecd 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -490,6 +490,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
> +
Going forward we should avoid using target specific "#if"
so that we can use the same qemu-system-riscv64 for both
RV32 and RV64.
> /* Leaf page shift amount */
> #define PGSHIFT 12
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 434a83e66a..26608ddf1c 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;
Rather than using "#if", please use "xlen" comparison to extract
PPN correctly from PTE.
Regards,
Anup
>
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 3:30 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 3:30 UTC (permalink / raw)
To: Weiwei Li
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
QEMU Developers, wangjunqiang, Wei Wu (吴伟),
Guo Ren
On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> ---
> 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 5a6d49aa64..282cd8eecd 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -490,6 +490,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
> +
Going forward we should avoid using target specific "#if"
so that we can use the same qemu-system-riscv64 for both
RV32 and RV64.
> /* Leaf page shift amount */
> #define PGSHIFT 12
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 434a83e66a..26608ddf1c 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;
Rather than using "#if", please use "xlen" comparison to extract
PPN correctly from PTE.
Regards,
Anup
>
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 3/5] target/riscv: add support for svnapot extension
2022-01-18 1:17 ` Weiwei Li
@ 2022-01-18 3:32 ` Anup Patel
-1 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 3:32 UTC (permalink / raw)
To: Weiwei Li
Cc: Wei Wu (吴伟),
open list:RISC-V, wangjunqiang, Bin Meng, QEMU Developers,
Alistair Francis, Palmer Dabbelt
On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> - add PTE_N bit
> - add PTE_N bit check for inner PTE
> - update address translation to support 64KiB continuous region (napot_bits = 4)
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Cc: Anup Patel <anup@brainfault.org>
I did review this patch previously.
In any case, this looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 1 +
> target/riscv/cpu_helper.c | 18 +++++++++++++++---
> 4 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9bc25d3055..ff6c86c85b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -668,6 +668,8 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> + DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> +
> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4d63086765..d3d17cde82 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -327,6 +327,7 @@ struct RISCVCPU {
> bool ext_counters;
> bool ext_ifencei;
> bool ext_icsr;
> + bool ext_svnapot;
> bool ext_zfh;
> bool ext_zfhmin;
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 282cd8eecd..5501e9698b 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -486,6 +486,7 @@ typedef enum {
> #define PTE_A 0x040 /* Accessed */
> #define PTE_D 0x080 /* Dirty */
> #define PTE_SOFT 0x300 /* Reserved for Software */
> +#define PTE_N 0x8000000000000000 /* NAPOT translation */
>
> /* Page table PPN shift amount */
> #define PTE_PPN_SHIFT 10
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 1820188f41..c276760c7f 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -621,12 +621,13 @@ restart:
>
> hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> + RISCVCPU *cpu = env_archcpu(env);
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> return TRANSLATE_FAIL;
> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> /* Inner PTE, continue walking */
> - if (pte & (PTE_D | PTE_A | PTE_U)) {
> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
> return TRANSLATE_FAIL;
> }
> base = ppn << PGSHIFT;
> @@ -702,8 +703,19 @@ restart:
> /* for superpage mappings, make a fake leaf PTE for the TLB's
> benefit. */
> target_ulong vpn = addr >> PGSHIFT;
> - *physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
> - (addr & ~TARGET_PAGE_MASK);
> +
> + int napot_bits = 0;
> + if (cpu->cfg.ext_svnapot && (pte & (target_ulong)PTE_N)) {
> + napot_bits = ctzl(ppn) + 1;
> + if ((i != (levels - 1)) || (napot_bits != 4)) {
> + return TRANSLATE_FAIL;
> + }
> + }
> +
> + *physical = (((ppn & ~(((target_ulong)1 << napot_bits) - 1)) |
> + (vpn & (((target_ulong)1 << napot_bits) - 1)) |
> + (vpn & (((target_ulong)1 << ptshift) - 1))
> + ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
>
> /* set permissions on the TLB entry */
> if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 3/5] target/riscv: add support for svnapot extension
@ 2022-01-18 3:32 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 3:32 UTC (permalink / raw)
To: Weiwei Li
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
QEMU Developers, wangjunqiang, Wei Wu (吴伟)
On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> - add PTE_N bit
> - add PTE_N bit check for inner PTE
> - update address translation to support 64KiB continuous region (napot_bits = 4)
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Cc: Anup Patel <anup@brainfault.org>
I did review this patch previously.
In any case, this looks good to me.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
> ---
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 1 +
> target/riscv/cpu_helper.c | 18 +++++++++++++++---
> 4 files changed, 19 insertions(+), 3 deletions(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 9bc25d3055..ff6c86c85b 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -668,6 +668,8 @@ static Property riscv_cpu_properties[] = {
> DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
> DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>
> + DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> +
> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index 4d63086765..d3d17cde82 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -327,6 +327,7 @@ struct RISCVCPU {
> bool ext_counters;
> bool ext_ifencei;
> bool ext_icsr;
> + bool ext_svnapot;
> bool ext_zfh;
> bool ext_zfhmin;
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 282cd8eecd..5501e9698b 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -486,6 +486,7 @@ typedef enum {
> #define PTE_A 0x040 /* Accessed */
> #define PTE_D 0x080 /* Dirty */
> #define PTE_SOFT 0x300 /* Reserved for Software */
> +#define PTE_N 0x8000000000000000 /* NAPOT translation */
>
> /* Page table PPN shift amount */
> #define PTE_PPN_SHIFT 10
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 1820188f41..c276760c7f 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -621,12 +621,13 @@ restart:
>
> hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> + RISCVCPU *cpu = env_archcpu(env);
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> return TRANSLATE_FAIL;
> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> /* Inner PTE, continue walking */
> - if (pte & (PTE_D | PTE_A | PTE_U)) {
> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
> return TRANSLATE_FAIL;
> }
> base = ppn << PGSHIFT;
> @@ -702,8 +703,19 @@ restart:
> /* for superpage mappings, make a fake leaf PTE for the TLB's
> benefit. */
> target_ulong vpn = addr >> PGSHIFT;
> - *physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
> - (addr & ~TARGET_PAGE_MASK);
> +
> + int napot_bits = 0;
> + if (cpu->cfg.ext_svnapot && (pte & (target_ulong)PTE_N)) {
> + napot_bits = ctzl(ppn) + 1;
> + if ((i != (levels - 1)) || (napot_bits != 4)) {
> + return TRANSLATE_FAIL;
> + }
> + }
> +
> + *physical = (((ppn & ~(((target_ulong)1 << napot_bits) - 1)) |
> + (vpn & (((target_ulong)1 << napot_bits) - 1)) |
> + (vpn & (((target_ulong)1 << ptshift) - 1))
> + ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
>
> /* set permissions on the TLB entry */
> if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
> --
> 2.17.1
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
2022-01-18 1:17 ` Weiwei Li
@ 2022-01-18 3:35 ` Anup Patel
-1 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 3:35 UTC (permalink / raw)
To: Weiwei Li
Cc: Wei Wu (吴伟),
open list:RISC-V, Heiko Stuebner, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Palmer Dabbelt
On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
> - add PTE_PBMT bit check for inner PTE
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Anup Patel <anup@brainfault.org>
> ---
> target/riscv/cpu.c | 1 +
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 2 ++
> target/riscv/cpu_helper.c | 4 +++-
> 4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 45ac98e06b..4f82bd00a3 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>
> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>
> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c3d1845ca1..53f314c752 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -329,6 +329,7 @@ struct RISCVCPU {
> bool ext_icsr;
> bool ext_svinval;
> bool ext_svnapot;
> + bool ext_svpbmt;
> bool ext_zfh;
> bool ext_zfhmin;
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 5501e9698b..24b7eb2b1f 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -486,7 +486,9 @@ typedef enum {
> #define PTE_A 0x040 /* Accessed */
> #define PTE_D 0x080 /* Dirty */
> #define PTE_SOFT 0x300 /* Reserved for Software */
> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
> #define PTE_N 0x8000000000000000 /* NAPOT translation */
> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>
> /* Page table PPN shift amount */
> #define PTE_PPN_SHIFT 10
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index c276760c7f..9fffaccffb 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -625,9 +625,11 @@ restart:
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> return TRANSLATE_FAIL;
> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
Rather than, type-casting defines here you can simply define
ULL constants. E.g.
#define PTE_PBMT 0x6000000000000000ULL
> + return TRANSLATE_FAIL;
> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> /* Inner PTE, continue walking */
> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> return TRANSLATE_FAIL;
> }
> base = ppn << PGSHIFT;
> --
> 2.17.1
>
Regards,
Anup
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
@ 2022-01-18 3:35 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 3:35 UTC (permalink / raw)
To: Weiwei Li
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
QEMU Developers, wangjunqiang, Wei Wu (吴伟),
Heiko Stuebner
On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
> - add PTE_PBMT bit check for inner PTE
>
> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Anup Patel <anup@brainfault.org>
> ---
> target/riscv/cpu.c | 1 +
> target/riscv/cpu.h | 1 +
> target/riscv/cpu_bits.h | 2 ++
> target/riscv/cpu_helper.c | 4 +++-
> 4 files changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 45ac98e06b..4f82bd00a3 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>
> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>
> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> index c3d1845ca1..53f314c752 100644
> --- a/target/riscv/cpu.h
> +++ b/target/riscv/cpu.h
> @@ -329,6 +329,7 @@ struct RISCVCPU {
> bool ext_icsr;
> bool ext_svinval;
> bool ext_svnapot;
> + bool ext_svpbmt;
> bool ext_zfh;
> bool ext_zfhmin;
>
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index 5501e9698b..24b7eb2b1f 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -486,7 +486,9 @@ typedef enum {
> #define PTE_A 0x040 /* Accessed */
> #define PTE_D 0x080 /* Dirty */
> #define PTE_SOFT 0x300 /* Reserved for Software */
> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
> #define PTE_N 0x8000000000000000 /* NAPOT translation */
> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>
> /* Page table PPN shift amount */
> #define PTE_PPN_SHIFT 10
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index c276760c7f..9fffaccffb 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -625,9 +625,11 @@ restart:
> if (!(pte & PTE_V)) {
> /* Invalid PTE */
> return TRANSLATE_FAIL;
> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
Rather than, type-casting defines here you can simply define
ULL constants. E.g.
#define PTE_PBMT 0x6000000000000000ULL
> + return TRANSLATE_FAIL;
> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> /* Inner PTE, continue walking */
> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> return TRANSLATE_FAIL;
> }
> base = ppn << PGSHIFT;
> --
> 2.17.1
>
Regards,
Anup
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 3:30 ` Anup Patel
@ 2022-01-18 4:51 ` Alistair Francis
-1 siblings, 0 replies; 52+ messages in thread
From: Alistair Francis @ 2022-01-18 4:51 UTC (permalink / raw)
To: Anup Patel
Cc: Weiwei Li, open list:RISC-V, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > ---
> > 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 5a6d49aa64..282cd8eecd 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -490,6 +490,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
> > +
>
> Going forward we should avoid using target specific "#if"
> so that we can use the same qemu-system-riscv64 for both
> RV32 and RV64.
>
> > /* Leaf page shift amount */
> > #define PGSHIFT 12
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 434a83e66a..26608ddf1c 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;
>
> Rather than using "#if", please use "xlen" comparison to extract
> PPN correctly from PTE.
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
>
> Regards,
> Anup
>
> >
> > if (!(pte & PTE_V)) {
> > /* Invalid PTE */
> > --
> > 2.17.1
> >
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 4:51 ` Alistair Francis
0 siblings, 0 replies; 52+ messages in thread
From: Alistair Francis @ 2022-01-18 4:51 UTC (permalink / raw)
To: Anup Patel
Cc: Weiwei Li, Wei Wu (吴伟),
open list:RISC-V, wangjunqiang, Bin Meng, QEMU Developers,
Alistair Francis, Guo Ren, Palmer Dabbelt
On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > ---
> > 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 5a6d49aa64..282cd8eecd 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -490,6 +490,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
> > +
>
> Going forward we should avoid using target specific "#if"
> so that we can use the same qemu-system-riscv64 for both
> RV32 and RV64.
>
> > /* Leaf page shift amount */
> > #define PGSHIFT 12
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 434a83e66a..26608ddf1c 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;
>
> Rather than using "#if", please use "xlen" comparison to extract
> PPN correctly from PTE.
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
>
> Regards,
> Anup
>
> >
> > if (!(pte & PTE_V)) {
> > /* Invalid PTE */
> > --
> > 2.17.1
> >
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 3/5] target/riscv: add support for svnapot extension
2022-01-18 3:32 ` Anup Patel
@ 2022-01-18 8:32 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 8:32 UTC (permalink / raw)
To: Anup Patel
Cc: Wei Wu (吴伟),
open list:RISC-V, wangjunqiang, Bin Meng, QEMU Developers,
Alistair Francis, Palmer Dabbelt
在 2022/1/18 上午11:32, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> - add PTE_N bit
>> - add PTE_N bit check for inner PTE
>> - update address translation to support 64KiB continuous region (napot_bits = 4)
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Cc: Anup Patel <anup@brainfault.org>
> I did review this patch previously.
>
> In any case, this looks good to me.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
Thanks a lot. Sorry for your repeated work.
Regards.
Weiwei Li
>> ---
>> target/riscv/cpu.c | 2 ++
>> target/riscv/cpu.h | 1 +
>> target/riscv/cpu_bits.h | 1 +
>> target/riscv/cpu_helper.c | 18 +++++++++++++++---
>> 4 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 9bc25d3055..ff6c86c85b 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -668,6 +668,8 @@ static Property riscv_cpu_properties[] = {
>> DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>> DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>>
>> + DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> +
>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>> DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 4d63086765..d3d17cde82 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -327,6 +327,7 @@ struct RISCVCPU {
>> bool ext_counters;
>> bool ext_ifencei;
>> bool ext_icsr;
>> + bool ext_svnapot;
>> bool ext_zfh;
>> bool ext_zfhmin;
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 282cd8eecd..5501e9698b 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -486,6 +486,7 @@ typedef enum {
>> #define PTE_A 0x040 /* Accessed */
>> #define PTE_D 0x080 /* Dirty */
>> #define PTE_SOFT 0x300 /* Reserved for Software */
>> +#define PTE_N 0x8000000000000000 /* NAPOT translation */
>>
>> /* Page table PPN shift amount */
>> #define PTE_PPN_SHIFT 10
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 1820188f41..c276760c7f 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -621,12 +621,13 @@ restart:
>>
>> hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>
>> + RISCVCPU *cpu = env_archcpu(env);
>> if (!(pte & PTE_V)) {
>> /* Invalid PTE */
>> return TRANSLATE_FAIL;
>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>> /* Inner PTE, continue walking */
>> - if (pte & (PTE_D | PTE_A | PTE_U)) {
>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>> return TRANSLATE_FAIL;
>> }
>> base = ppn << PGSHIFT;
>> @@ -702,8 +703,19 @@ restart:
>> /* for superpage mappings, make a fake leaf PTE for the TLB's
>> benefit. */
>> target_ulong vpn = addr >> PGSHIFT;
>> - *physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
>> - (addr & ~TARGET_PAGE_MASK);
>> +
>> + int napot_bits = 0;
>> + if (cpu->cfg.ext_svnapot && (pte & (target_ulong)PTE_N)) {
>> + napot_bits = ctzl(ppn) + 1;
>> + if ((i != (levels - 1)) || (napot_bits != 4)) {
>> + return TRANSLATE_FAIL;
>> + }
>> + }
>> +
>> + *physical = (((ppn & ~(((target_ulong)1 << napot_bits) - 1)) |
>> + (vpn & (((target_ulong)1 << napot_bits) - 1)) |
>> + (vpn & (((target_ulong)1 << ptshift) - 1))
>> + ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
>>
>> /* set permissions on the TLB entry */
>> if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 3/5] target/riscv: add support for svnapot extension
@ 2022-01-18 8:32 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 8:32 UTC (permalink / raw)
To: Anup Patel
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
QEMU Developers, wangjunqiang, Wei Wu (吴伟)
在 2022/1/18 上午11:32, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> - add PTE_N bit
>> - add PTE_N bit check for inner PTE
>> - update address translation to support 64KiB continuous region (napot_bits = 4)
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Cc: Anup Patel <anup@brainfault.org>
> I did review this patch previously.
>
> In any case, this looks good to me.
>
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
Thanks a lot. Sorry for your repeated work.
Regards.
Weiwei Li
>> ---
>> target/riscv/cpu.c | 2 ++
>> target/riscv/cpu.h | 1 +
>> target/riscv/cpu_bits.h | 1 +
>> target/riscv/cpu_helper.c | 18 +++++++++++++++---
>> 4 files changed, 19 insertions(+), 3 deletions(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 9bc25d3055..ff6c86c85b 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -668,6 +668,8 @@ static Property riscv_cpu_properties[] = {
>> DEFINE_PROP_UINT16("vlen", RISCVCPU, cfg.vlen, 128),
>> DEFINE_PROP_UINT16("elen", RISCVCPU, cfg.elen, 64),
>>
>> + DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> +
>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>> DEFINE_PROP_BOOL("zbc", RISCVCPU, cfg.ext_zbc, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index 4d63086765..d3d17cde82 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -327,6 +327,7 @@ struct RISCVCPU {
>> bool ext_counters;
>> bool ext_ifencei;
>> bool ext_icsr;
>> + bool ext_svnapot;
>> bool ext_zfh;
>> bool ext_zfhmin;
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 282cd8eecd..5501e9698b 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -486,6 +486,7 @@ typedef enum {
>> #define PTE_A 0x040 /* Accessed */
>> #define PTE_D 0x080 /* Dirty */
>> #define PTE_SOFT 0x300 /* Reserved for Software */
>> +#define PTE_N 0x8000000000000000 /* NAPOT translation */
>>
>> /* Page table PPN shift amount */
>> #define PTE_PPN_SHIFT 10
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index 1820188f41..c276760c7f 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -621,12 +621,13 @@ restart:
>>
>> hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>
>> + RISCVCPU *cpu = env_archcpu(env);
>> if (!(pte & PTE_V)) {
>> /* Invalid PTE */
>> return TRANSLATE_FAIL;
>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>> /* Inner PTE, continue walking */
>> - if (pte & (PTE_D | PTE_A | PTE_U)) {
>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>> return TRANSLATE_FAIL;
>> }
>> base = ppn << PGSHIFT;
>> @@ -702,8 +703,19 @@ restart:
>> /* for superpage mappings, make a fake leaf PTE for the TLB's
>> benefit. */
>> target_ulong vpn = addr >> PGSHIFT;
>> - *physical = ((ppn | (vpn & ((1L << ptshift) - 1))) << PGSHIFT) |
>> - (addr & ~TARGET_PAGE_MASK);
>> +
>> + int napot_bits = 0;
>> + if (cpu->cfg.ext_svnapot && (pte & (target_ulong)PTE_N)) {
>> + napot_bits = ctzl(ppn) + 1;
>> + if ((i != (levels - 1)) || (napot_bits != 4)) {
>> + return TRANSLATE_FAIL;
>> + }
>> + }
>> +
>> + *physical = (((ppn & ~(((target_ulong)1 << napot_bits) - 1)) |
>> + (vpn & (((target_ulong)1 << napot_bits) - 1)) |
>> + (vpn & (((target_ulong)1 << ptshift) - 1))
>> + ) << PGSHIFT) | (addr & ~TARGET_PAGE_MASK);
>>
>> /* set permissions on the TLB entry */
>> if ((pte & PTE_R) || ((pte & PTE_X) && mxr)) {
>> --
>> 2.17.1
>>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
2022-01-18 3:35 ` Anup Patel
@ 2022-01-18 8:33 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 8:33 UTC (permalink / raw)
To: Anup Patel
Cc: Wei Wu (吴伟),
open list:RISC-V, Heiko Stuebner, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Palmer Dabbelt
在 2022/1/18 上午11:35, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
>> - add PTE_PBMT bit check for inner PTE
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Anup Patel <anup@brainfault.org>
>> ---
>> target/riscv/cpu.c | 1 +
>> target/riscv/cpu.h | 1 +
>> target/riscv/cpu_bits.h | 2 ++
>> target/riscv/cpu_helper.c | 4 +++-
>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 45ac98e06b..4f82bd00a3 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>>
>> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>
>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index c3d1845ca1..53f314c752 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -329,6 +329,7 @@ struct RISCVCPU {
>> bool ext_icsr;
>> bool ext_svinval;
>> bool ext_svnapot;
>> + bool ext_svpbmt;
>> bool ext_zfh;
>> bool ext_zfhmin;
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 5501e9698b..24b7eb2b1f 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -486,7 +486,9 @@ typedef enum {
>> #define PTE_A 0x040 /* Accessed */
>> #define PTE_D 0x080 /* Dirty */
>> #define PTE_SOFT 0x300 /* Reserved for Software */
>> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
>> #define PTE_N 0x8000000000000000 /* NAPOT translation */
>> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>
>> /* Page table PPN shift amount */
>> #define PTE_PPN_SHIFT 10
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index c276760c7f..9fffaccffb 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -625,9 +625,11 @@ restart:
>> if (!(pte & PTE_V)) {
>> /* Invalid PTE */
>> return TRANSLATE_FAIL;
>> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
> Rather than, type-casting defines here you can simply define
> ULL constants. E.g.
> #define PTE_PBMT 0x6000000000000000ULL
OK. I'll update this.
Regards,
Weiwei Li
>> + return TRANSLATE_FAIL;
>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>> /* Inner PTE, continue walking */
>> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
>> return TRANSLATE_FAIL;
>> }
>> base = ppn << PGSHIFT;
>> --
>> 2.17.1
>>
> Regards,
> Anup
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
@ 2022-01-18 8:33 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 8:33 UTC (permalink / raw)
To: Anup Patel
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
QEMU Developers, wangjunqiang, Wei Wu (吴伟),
Heiko Stuebner
在 2022/1/18 上午11:35, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
>> - add PTE_PBMT bit check for inner PTE
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Anup Patel <anup@brainfault.org>
>> ---
>> target/riscv/cpu.c | 1 +
>> target/riscv/cpu.h | 1 +
>> target/riscv/cpu_bits.h | 2 ++
>> target/riscv/cpu_helper.c | 4 +++-
>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 45ac98e06b..4f82bd00a3 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>>
>> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>
>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index c3d1845ca1..53f314c752 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -329,6 +329,7 @@ struct RISCVCPU {
>> bool ext_icsr;
>> bool ext_svinval;
>> bool ext_svnapot;
>> + bool ext_svpbmt;
>> bool ext_zfh;
>> bool ext_zfhmin;
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 5501e9698b..24b7eb2b1f 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -486,7 +486,9 @@ typedef enum {
>> #define PTE_A 0x040 /* Accessed */
>> #define PTE_D 0x080 /* Dirty */
>> #define PTE_SOFT 0x300 /* Reserved for Software */
>> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
>> #define PTE_N 0x8000000000000000 /* NAPOT translation */
>> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>
>> /* Page table PPN shift amount */
>> #define PTE_PPN_SHIFT 10
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index c276760c7f..9fffaccffb 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -625,9 +625,11 @@ restart:
>> if (!(pte & PTE_V)) {
>> /* Invalid PTE */
>> return TRANSLATE_FAIL;
>> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
> Rather than, type-casting defines here you can simply define
> ULL constants. E.g.
> #define PTE_PBMT 0x6000000000000000ULL
OK. I'll update this.
Regards,
Weiwei Li
>> + return TRANSLATE_FAIL;
>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>> /* Inner PTE, continue walking */
>> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
>> return TRANSLATE_FAIL;
>> }
>> base = ppn << PGSHIFT;
>> --
>> 2.17.1
>>
> Regards,
> Anup
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 3:30 ` Anup Patel
@ 2022-01-18 8:33 ` Guo Ren
-1 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 8:33 UTC (permalink / raw)
To: Anup Patel
Cc: Weiwei Li, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > ---
> > 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 5a6d49aa64..282cd8eecd 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -490,6 +490,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
> > +
>
> Going forward we should avoid using target specific "#if"
> so that we can use the same qemu-system-riscv64 for both
> RV32 and RV64.
>
> > /* Leaf page shift amount */
> > #define PGSHIFT 12
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 434a83e66a..26608ddf1c 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;
>
> Rather than using "#if", please use "xlen" comparison to extract
> PPN correctly from PTE.
Do you mean?
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9fffaccffb..071b7ea4cf 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -619,7 +619,11 @@ restart:
return TRANSLATE_FAIL;
}
- hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ if (riscv_cpu_mxl(env) == MXL_RV32) {
+ hwaddr ppn = pte >> PTE_PPN_SHIFT;
+ } else {
+ hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
+ }
RISCVCPU *cpu = env_archcpu(env);
if (!(pte & PTE_V)) {
>
> Regards,
> Anup
>
> >
> > if (!(pte & PTE_V)) {
> > /* Invalid PTE */
> > --
> > 2.17.1
> >
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 8:33 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 8:33 UTC (permalink / raw)
To: Anup Patel
Cc: Weiwei Li, Wei Wu (吴伟),
open list:RISC-V, Wang Junqiang, Bin Meng, QEMU Developers,
Alistair Francis, Guo Ren, Palmer Dabbelt
On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > ---
> > 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 5a6d49aa64..282cd8eecd 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -490,6 +490,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
> > +
>
> Going forward we should avoid using target specific "#if"
> so that we can use the same qemu-system-riscv64 for both
> RV32 and RV64.
>
> > /* Leaf page shift amount */
> > #define PGSHIFT 12
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 434a83e66a..26608ddf1c 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;
>
> Rather than using "#if", please use "xlen" comparison to extract
> PPN correctly from PTE.
Do you mean?
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 9fffaccffb..071b7ea4cf 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -619,7 +619,11 @@ restart:
return TRANSLATE_FAIL;
}
- hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ if (riscv_cpu_mxl(env) == MXL_RV32) {
+ hwaddr ppn = pte >> PTE_PPN_SHIFT;
+ } else {
+ hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
+ }
RISCVCPU *cpu = env_archcpu(env);
if (!(pte & PTE_V)) {
>
> Regards,
> Anup
>
> >
> > if (!(pte & PTE_V)) {
> > /* Invalid PTE */
> > --
> > 2.17.1
> >
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply related [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 8:33 ` Guo Ren
@ 2022-01-18 8:51 ` Anup Patel
-1 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 8:51 UTC (permalink / raw)
To: Guo Ren
Cc: Weiwei Li, open list:RISC-V, Anup Patel, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > ---
> > > 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 5a6d49aa64..282cd8eecd 100644
> > > --- a/target/riscv/cpu_bits.h
> > > +++ b/target/riscv/cpu_bits.h
> > > @@ -490,6 +490,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
> > > +
> >
> > Going forward we should avoid using target specific "#if"
> > so that we can use the same qemu-system-riscv64 for both
> > RV32 and RV64.
> >
> > > /* Leaf page shift amount */
> > > #define PGSHIFT 12
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 434a83e66a..26608ddf1c 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;
> >
> > Rather than using "#if", please use "xlen" comparison to extract
> > PPN correctly from PTE.
> Do you mean?
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9fffaccffb..071b7ea4cf 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -619,7 +619,11 @@ restart:
> return TRANSLATE_FAIL;
> }
>
> - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> + if (riscv_cpu_mxl(env) == MXL_RV32) {
> + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> + } else {
> + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> + }
Yes, something like this but use a define for 0x3fffffffffffffULL
Regards,
Anup
>
> RISCVCPU *cpu = env_archcpu(env);
> if (!(pte & PTE_V)) {
>
> >
> > Regards,
> > Anup
> >
> > >
> > > if (!(pte & PTE_V)) {
> > > /* Invalid PTE */
> > > --
> > > 2.17.1
> > >
> >
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 8:51 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 8:51 UTC (permalink / raw)
To: Guo Ren
Cc: Anup Patel, Weiwei Li, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > ---
> > > 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 5a6d49aa64..282cd8eecd 100644
> > > --- a/target/riscv/cpu_bits.h
> > > +++ b/target/riscv/cpu_bits.h
> > > @@ -490,6 +490,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
> > > +
> >
> > Going forward we should avoid using target specific "#if"
> > so that we can use the same qemu-system-riscv64 for both
> > RV32 and RV64.
> >
> > > /* Leaf page shift amount */
> > > #define PGSHIFT 12
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 434a83e66a..26608ddf1c 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;
> >
> > Rather than using "#if", please use "xlen" comparison to extract
> > PPN correctly from PTE.
> Do you mean?
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 9fffaccffb..071b7ea4cf 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -619,7 +619,11 @@ restart:
> return TRANSLATE_FAIL;
> }
>
> - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> + if (riscv_cpu_mxl(env) == MXL_RV32) {
> + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> + } else {
> + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> + }
Yes, something like this but use a define for 0x3fffffffffffffULL
Regards,
Anup
>
> RISCVCPU *cpu = env_archcpu(env);
> if (!(pte & PTE_V)) {
>
> >
> > Regards,
> > Anup
> >
> > >
> > > if (!(pte & PTE_V)) {
> > > /* Invalid PTE */
> > > --
> > > 2.17.1
> > >
> >
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
2022-01-18 3:35 ` Anup Patel
@ 2022-01-18 9:09 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 9:09 UTC (permalink / raw)
To: Anup Patel
Cc: Wei Wu (吴伟),
open list:RISC-V, Heiko Stuebner, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Palmer Dabbelt
在 2022/1/18 上午11:35, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
>> - add PTE_PBMT bit check for inner PTE
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Anup Patel <anup@brainfault.org>
>> ---
>> target/riscv/cpu.c | 1 +
>> target/riscv/cpu.h | 1 +
>> target/riscv/cpu_bits.h | 2 ++
>> target/riscv/cpu_helper.c | 4 +++-
>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 45ac98e06b..4f82bd00a3 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>>
>> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>
>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index c3d1845ca1..53f314c752 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -329,6 +329,7 @@ struct RISCVCPU {
>> bool ext_icsr;
>> bool ext_svinval;
>> bool ext_svnapot;
>> + bool ext_svpbmt;
>> bool ext_zfh;
>> bool ext_zfhmin;
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 5501e9698b..24b7eb2b1f 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -486,7 +486,9 @@ typedef enum {
>> #define PTE_A 0x040 /* Accessed */
>> #define PTE_D 0x080 /* Dirty */
>> #define PTE_SOFT 0x300 /* Reserved for Software */
>> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
>> #define PTE_N 0x8000000000000000 /* NAPOT translation */
>> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>
>> /* Page table PPN shift amount */
>> #define PTE_PPN_SHIFT 10
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index c276760c7f..9fffaccffb 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -625,9 +625,11 @@ restart:
>> if (!(pte & PTE_V)) {
>> /* Invalid PTE */
>> return TRANSLATE_FAIL;
>> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
> Rather than, type-casting defines here you can simply define
> ULL constants. E.g.
> #define PTE_PBMT 0x6000000000000000ULL
Sorry, I'm wonder why add ULL can replace the function of type-casting.
The type-casting here is to compatible with RV32 for possible strict
type check warnings since pte is 32 bits and PTE_PBMT is 64 bits in RV32.
If I add ULL in PTE_PBMT, it seems have no change to PTE_PBMT. It's
still 64 bits in RV32.
Regards,
Weiwei Li
>
>> + return TRANSLATE_FAIL;
>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>> /* Inner PTE, continue walking */
>> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
>> return TRANSLATE_FAIL;
>> }
>> base = ppn << PGSHIFT;
>> --
>> 2.17.1
>>
> Regards,
> Anup
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
@ 2022-01-18 9:09 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 9:09 UTC (permalink / raw)
To: Anup Patel
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, open list:RISC-V,
QEMU Developers, wangjunqiang, Wei Wu (吴伟),
Heiko Stuebner
在 2022/1/18 上午11:35, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
>> - add PTE_PBMT bit check for inner PTE
>>
>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>> Cc: Heiko Stuebner <heiko@sntech.de>
>> Cc: Anup Patel <anup@brainfault.org>
>> ---
>> target/riscv/cpu.c | 1 +
>> target/riscv/cpu.h | 1 +
>> target/riscv/cpu_bits.h | 2 ++
>> target/riscv/cpu_helper.c | 4 +++-
>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>> index 45ac98e06b..4f82bd00a3 100644
>> --- a/target/riscv/cpu.c
>> +++ b/target/riscv/cpu.c
>> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>>
>> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>
>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>> index c3d1845ca1..53f314c752 100644
>> --- a/target/riscv/cpu.h
>> +++ b/target/riscv/cpu.h
>> @@ -329,6 +329,7 @@ struct RISCVCPU {
>> bool ext_icsr;
>> bool ext_svinval;
>> bool ext_svnapot;
>> + bool ext_svpbmt;
>> bool ext_zfh;
>> bool ext_zfhmin;
>>
>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>> index 5501e9698b..24b7eb2b1f 100644
>> --- a/target/riscv/cpu_bits.h
>> +++ b/target/riscv/cpu_bits.h
>> @@ -486,7 +486,9 @@ typedef enum {
>> #define PTE_A 0x040 /* Accessed */
>> #define PTE_D 0x080 /* Dirty */
>> #define PTE_SOFT 0x300 /* Reserved for Software */
>> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
>> #define PTE_N 0x8000000000000000 /* NAPOT translation */
>> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>
>> /* Page table PPN shift amount */
>> #define PTE_PPN_SHIFT 10
>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>> index c276760c7f..9fffaccffb 100644
>> --- a/target/riscv/cpu_helper.c
>> +++ b/target/riscv/cpu_helper.c
>> @@ -625,9 +625,11 @@ restart:
>> if (!(pte & PTE_V)) {
>> /* Invalid PTE */
>> return TRANSLATE_FAIL;
>> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
> Rather than, type-casting defines here you can simply define
> ULL constants. E.g.
> #define PTE_PBMT 0x6000000000000000ULL
Sorry, I'm wonder why add ULL can replace the function of type-casting.
The type-casting here is to compatible with RV32 for possible strict
type check warnings since pte is 32 bits and PTE_PBMT is 64 bits in RV32.
If I add ULL in PTE_PBMT, it seems have no change to PTE_PBMT. It's
still 64 bits in RV32.
Regards,
Weiwei Li
>
>> + return TRANSLATE_FAIL;
>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>> /* Inner PTE, continue walking */
>> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
>> return TRANSLATE_FAIL;
>> }
>> base = ppn << PGSHIFT;
>> --
>> 2.17.1
>>
> Regards,
> Anup
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
2022-01-18 9:09 ` Weiwei Li
@ 2022-01-18 11:04 ` Anup Patel
-1 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 11:04 UTC (permalink / raw)
To: Weiwei Li
Cc: Wei Wu (吴伟),
Heiko Stuebner, Anup Patel, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, open list:RISC-V,
Palmer Dabbelt
On Tue, Jan 18, 2022 at 2:40 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> 在 2022/1/18 上午11:35, Anup Patel 写道:
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> >> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
> >> - add PTE_PBMT bit check for inner PTE
> >>
> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> >> Cc: Heiko Stuebner <heiko@sntech.de>
> >> Cc: Anup Patel <anup@brainfault.org>
> >> ---
> >> target/riscv/cpu.c | 1 +
> >> target/riscv/cpu.h | 1 +
> >> target/riscv/cpu_bits.h | 2 ++
> >> target/riscv/cpu_helper.c | 4 +++-
> >> 4 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index 45ac98e06b..4f82bd00a3 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
> >>
> >> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> >> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> >> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> >>
> >> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> >> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> >> index c3d1845ca1..53f314c752 100644
> >> --- a/target/riscv/cpu.h
> >> +++ b/target/riscv/cpu.h
> >> @@ -329,6 +329,7 @@ struct RISCVCPU {
> >> bool ext_icsr;
> >> bool ext_svinval;
> >> bool ext_svnapot;
> >> + bool ext_svpbmt;
> >> bool ext_zfh;
> >> bool ext_zfhmin;
> >>
> >> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> >> index 5501e9698b..24b7eb2b1f 100644
> >> --- a/target/riscv/cpu_bits.h
> >> +++ b/target/riscv/cpu_bits.h
> >> @@ -486,7 +486,9 @@ typedef enum {
> >> #define PTE_A 0x040 /* Accessed */
> >> #define PTE_D 0x080 /* Dirty */
> >> #define PTE_SOFT 0x300 /* Reserved for Software */
> >> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
> >> #define PTE_N 0x8000000000000000 /* NAPOT translation */
> >> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
> >>
> >> /* Page table PPN shift amount */
> >> #define PTE_PPN_SHIFT 10
> >> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> index c276760c7f..9fffaccffb 100644
> >> --- a/target/riscv/cpu_helper.c
> >> +++ b/target/riscv/cpu_helper.c
> >> @@ -625,9 +625,11 @@ restart:
> >> if (!(pte & PTE_V)) {
> >> /* Invalid PTE */
> >> return TRANSLATE_FAIL;
> >> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
> > Rather than, type-casting defines here you can simply define
> > ULL constants. E.g.
> > #define PTE_PBMT 0x6000000000000000ULL
>
> Sorry, I'm wonder why add ULL can replace the function of type-casting.
>
> The type-casting here is to compatible with RV32 for possible strict
> type check warnings since pte is 32 bits and PTE_PBMT is 64 bits in RV32.
If adding ULL does not help for RV32 target then no need to change.
>
> If I add ULL in PTE_PBMT, it seems have no change to PTE_PBMT. It's
> still 64 bits in RV32.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
>
> Regards,
>
> Weiwei Li
>
> >
> >> + return TRANSLATE_FAIL;
> >> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> >> /* Inner PTE, continue walking */
> >> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
> >> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> >> return TRANSLATE_FAIL;
> >> }
> >> base = ppn << PGSHIFT;
> >> --
> >> 2.17.1
> >>
> > Regards,
> > Anup
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
@ 2022-01-18 11:04 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 11:04 UTC (permalink / raw)
To: Weiwei Li
Cc: Anup Patel, Wei Wu (吴伟),
open list:RISC-V, Heiko Stuebner, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Palmer Dabbelt
On Tue, Jan 18, 2022 at 2:40 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>
>
> 在 2022/1/18 上午11:35, Anup Patel 写道:
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
> >> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
> >> - add PTE_PBMT bit check for inner PTE
> >>
> >> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
> >> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
> >> Cc: Heiko Stuebner <heiko@sntech.de>
> >> Cc: Anup Patel <anup@brainfault.org>
> >> ---
> >> target/riscv/cpu.c | 1 +
> >> target/riscv/cpu.h | 1 +
> >> target/riscv/cpu_bits.h | 2 ++
> >> target/riscv/cpu_helper.c | 4 +++-
> >> 4 files changed, 7 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> >> index 45ac98e06b..4f82bd00a3 100644
> >> --- a/target/riscv/cpu.c
> >> +++ b/target/riscv/cpu.c
> >> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
> >>
> >> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
> >> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
> >> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
> >>
> >> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
> >> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
> >> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
> >> index c3d1845ca1..53f314c752 100644
> >> --- a/target/riscv/cpu.h
> >> +++ b/target/riscv/cpu.h
> >> @@ -329,6 +329,7 @@ struct RISCVCPU {
> >> bool ext_icsr;
> >> bool ext_svinval;
> >> bool ext_svnapot;
> >> + bool ext_svpbmt;
> >> bool ext_zfh;
> >> bool ext_zfhmin;
> >>
> >> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> >> index 5501e9698b..24b7eb2b1f 100644
> >> --- a/target/riscv/cpu_bits.h
> >> +++ b/target/riscv/cpu_bits.h
> >> @@ -486,7 +486,9 @@ typedef enum {
> >> #define PTE_A 0x040 /* Accessed */
> >> #define PTE_D 0x080 /* Dirty */
> >> #define PTE_SOFT 0x300 /* Reserved for Software */
> >> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
> >> #define PTE_N 0x8000000000000000 /* NAPOT translation */
> >> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
> >>
> >> /* Page table PPN shift amount */
> >> #define PTE_PPN_SHIFT 10
> >> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >> index c276760c7f..9fffaccffb 100644
> >> --- a/target/riscv/cpu_helper.c
> >> +++ b/target/riscv/cpu_helper.c
> >> @@ -625,9 +625,11 @@ restart:
> >> if (!(pte & PTE_V)) {
> >> /* Invalid PTE */
> >> return TRANSLATE_FAIL;
> >> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
> > Rather than, type-casting defines here you can simply define
> > ULL constants. E.g.
> > #define PTE_PBMT 0x6000000000000000ULL
>
> Sorry, I'm wonder why add ULL can replace the function of type-casting.
>
> The type-casting here is to compatible with RV32 for possible strict
> type check warnings since pte is 32 bits and PTE_PBMT is 64 bits in RV32.
If adding ULL does not help for RV32 target then no need to change.
>
> If I add ULL in PTE_PBMT, it seems have no change to PTE_PBMT. It's
> still 64 bits in RV32.
Reviewed-by: Anup Patel <anup@brainfault.org>
Regards,
Anup
>
> Regards,
>
> Weiwei Li
>
> >
> >> + return TRANSLATE_FAIL;
> >> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
> >> /* Inner PTE, continue walking */
> >> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
> >> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
> >> return TRANSLATE_FAIL;
> >> }
> >> base = ppn << PGSHIFT;
> >> --
> >> 2.17.1
> >>
> > Regards,
> > Anup
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 8:51 ` Anup Patel
@ 2022-01-18 11:15 ` Guo Ren
-1 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 11:15 UTC (permalink / raw)
To: Anup Patel
Cc: Weiwei Li, open list:RISC-V, Anup Patel, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > ---
> > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > --- a/target/riscv/cpu_bits.h
> > > > +++ b/target/riscv/cpu_bits.h
> > > > @@ -490,6 +490,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
> > > > +
> > >
> > > Going forward we should avoid using target specific "#if"
> > > so that we can use the same qemu-system-riscv64 for both
> > > RV32 and RV64.
> > >
> > > > /* Leaf page shift amount */
> > > > #define PGSHIFT 12
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index 434a83e66a..26608ddf1c 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;
> > >
> > > Rather than using "#if", please use "xlen" comparison to extract
> > > PPN correctly from PTE.
> > Do you mean?
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 9fffaccffb..071b7ea4cf 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -619,7 +619,11 @@ restart:
> > return TRANSLATE_FAIL;
> > }
> >
> > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > + } else {
> > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > + }
>
> Yes, something like this but use a define for 0x3fffffffffffffULL
Just as Alistair said: This will need to be dynamic based on get_xl()
I still prefer:
+#if defined(TARGET_RISCV32)
+#define PTE_PPN_MASK 0xffffffffUL
+#elif defined(TARGET_RISCV64)
+#define PTE_PPN_MASK 0x3fffffffffffffULL
+#endif
+ hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> Regards,
> Anup
>
> >
> > RISCVCPU *cpu = env_archcpu(env);
> > if (!(pte & PTE_V)) {
> >
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > if (!(pte & PTE_V)) {
> > > > /* Invalid PTE */
> > > > --
> > > > 2.17.1
> > > >
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 11:15 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 11:15 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Weiwei Li, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
>
> On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > ---
> > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > --- a/target/riscv/cpu_bits.h
> > > > +++ b/target/riscv/cpu_bits.h
> > > > @@ -490,6 +490,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
> > > > +
> > >
> > > Going forward we should avoid using target specific "#if"
> > > so that we can use the same qemu-system-riscv64 for both
> > > RV32 and RV64.
> > >
> > > > /* Leaf page shift amount */
> > > > #define PGSHIFT 12
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index 434a83e66a..26608ddf1c 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;
> > >
> > > Rather than using "#if", please use "xlen" comparison to extract
> > > PPN correctly from PTE.
> > Do you mean?
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 9fffaccffb..071b7ea4cf 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -619,7 +619,11 @@ restart:
> > return TRANSLATE_FAIL;
> > }
> >
> > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > + } else {
> > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > + }
>
> Yes, something like this but use a define for 0x3fffffffffffffULL
Just as Alistair said: This will need to be dynamic based on get_xl()
I still prefer:
+#if defined(TARGET_RISCV32)
+#define PTE_PPN_MASK 0xffffffffUL
+#elif defined(TARGET_RISCV64)
+#define PTE_PPN_MASK 0x3fffffffffffffULL
+#endif
+ hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> Regards,
> Anup
>
> >
> > RISCVCPU *cpu = env_archcpu(env);
> > if (!(pte & PTE_V)) {
> >
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > if (!(pte & PTE_V)) {
> > > > /* Invalid PTE */
> > > > --
> > > > 2.17.1
> > > >
> > >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
2022-01-18 11:04 ` Anup Patel
@ 2022-01-18 11:21 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 11:21 UTC (permalink / raw)
To: Anup Patel
Cc: Wei Wu (吴伟),
Heiko Stuebner, Anup Patel, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, open list:RISC-V,
Palmer Dabbelt
在 2022/1/18 下午7:04, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 2:40 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>
>> 在 2022/1/18 上午11:35, Anup Patel 写道:
>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>>> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
>>>> - add PTE_PBMT bit check for inner PTE
>>>>
>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>> Cc: Anup Patel <anup@brainfault.org>
>>>> ---
>>>> target/riscv/cpu.c | 1 +
>>>> target/riscv/cpu.h | 1 +
>>>> target/riscv/cpu_bits.h | 2 ++
>>>> target/riscv/cpu_helper.c | 4 +++-
>>>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index 45ac98e06b..4f82bd00a3 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>>>>
>>>> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>>>> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>>>> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>>>
>>>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>>>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>>>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>>>> index c3d1845ca1..53f314c752 100644
>>>> --- a/target/riscv/cpu.h
>>>> +++ b/target/riscv/cpu.h
>>>> @@ -329,6 +329,7 @@ struct RISCVCPU {
>>>> bool ext_icsr;
>>>> bool ext_svinval;
>>>> bool ext_svnapot;
>>>> + bool ext_svpbmt;
>>>> bool ext_zfh;
>>>> bool ext_zfhmin;
>>>>
>>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>>>> index 5501e9698b..24b7eb2b1f 100644
>>>> --- a/target/riscv/cpu_bits.h
>>>> +++ b/target/riscv/cpu_bits.h
>>>> @@ -486,7 +486,9 @@ typedef enum {
>>>> #define PTE_A 0x040 /* Accessed */
>>>> #define PTE_D 0x080 /* Dirty */
>>>> #define PTE_SOFT 0x300 /* Reserved for Software */
>>>> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
>>>> #define PTE_N 0x8000000000000000 /* NAPOT translation */
>>>> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>>>
>>>> /* Page table PPN shift amount */
>>>> #define PTE_PPN_SHIFT 10
>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>> index c276760c7f..9fffaccffb 100644
>>>> --- a/target/riscv/cpu_helper.c
>>>> +++ b/target/riscv/cpu_helper.c
>>>> @@ -625,9 +625,11 @@ restart:
>>>> if (!(pte & PTE_V)) {
>>>> /* Invalid PTE */
>>>> return TRANSLATE_FAIL;
>>>> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
>>> Rather than, type-casting defines here you can simply define
>>> ULL constants. E.g.
>>> #define PTE_PBMT 0x6000000000000000ULL
>> Sorry, I'm wonder why add ULL can replace the function of type-casting.
>>
>> The type-casting here is to compatible with RV32 for possible strict
>> type check warnings since pte is 32 bits and PTE_PBMT is 64 bits in RV32.
> If adding ULL does not help for RV32 target then no need to change.
OK. Thanks a lot.
Regards,
Weiwei Li
>> If I add ULL in PTE_PBMT, it seems have no change to PTE_PBMT. It's
>> still 64 bits in RV32.
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
>
>> Regards,
>>
>> Weiwei Li
>>
>>>> + return TRANSLATE_FAIL;
>>>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>>>> /* Inner PTE, continue walking */
>>>> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>>>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
>>>> return TRANSLATE_FAIL;
>>>> }
>>>> base = ppn << PGSHIFT;
>>>> --
>>>> 2.17.1
>>>>
>>> Regards,
>>> Anup
>>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 5/5] target/riscv: add support for svpbmt extension
@ 2022-01-18 11:21 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 11:21 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Wei Wu (吴伟),
open list:RISC-V, Heiko Stuebner, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Palmer Dabbelt
在 2022/1/18 下午7:04, Anup Patel 写道:
> On Tue, Jan 18, 2022 at 2:40 PM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>
>> 在 2022/1/18 上午11:35, Anup Patel 写道:
>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> wrote:
>>>> - add PTE_PBMT bits: It uses two PTE bits, but otherwise has no effect on QEMU, since QEMU is sequentially consistent and doesn't model PMAs currently
>>>> - add PTE_PBMT bit check for inner PTE
>>>>
>>>> Signed-off-by: Weiwei Li <liweiwei@iscas.ac.cn>
>>>> Signed-off-by: Junqiang Wang <wangjunqiang@iscas.ac.cn>
>>>> Cc: Heiko Stuebner <heiko@sntech.de>
>>>> Cc: Anup Patel <anup@brainfault.org>
>>>> ---
>>>> target/riscv/cpu.c | 1 +
>>>> target/riscv/cpu.h | 1 +
>>>> target/riscv/cpu_bits.h | 2 ++
>>>> target/riscv/cpu_helper.c | 4 +++-
>>>> 4 files changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>>> index 45ac98e06b..4f82bd00a3 100644
>>>> --- a/target/riscv/cpu.c
>>>> +++ b/target/riscv/cpu.c
>>>> @@ -670,6 +670,7 @@ static Property riscv_cpu_properties[] = {
>>>>
>>>> DEFINE_PROP_BOOL("svinval", RISCVCPU, cfg.ext_svinval, false),
>>>> DEFINE_PROP_BOOL("svnapot", RISCVCPU, cfg.ext_svnapot, false),
>>>> + DEFINE_PROP_BOOL("svpbmt", RISCVCPU, cfg.ext_svpbmt, false),
>>>>
>>>> DEFINE_PROP_BOOL("zba", RISCVCPU, cfg.ext_zba, true),
>>>> DEFINE_PROP_BOOL("zbb", RISCVCPU, cfg.ext_zbb, true),
>>>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
>>>> index c3d1845ca1..53f314c752 100644
>>>> --- a/target/riscv/cpu.h
>>>> +++ b/target/riscv/cpu.h
>>>> @@ -329,6 +329,7 @@ struct RISCVCPU {
>>>> bool ext_icsr;
>>>> bool ext_svinval;
>>>> bool ext_svnapot;
>>>> + bool ext_svpbmt;
>>>> bool ext_zfh;
>>>> bool ext_zfhmin;
>>>>
>>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>>>> index 5501e9698b..24b7eb2b1f 100644
>>>> --- a/target/riscv/cpu_bits.h
>>>> +++ b/target/riscv/cpu_bits.h
>>>> @@ -486,7 +486,9 @@ typedef enum {
>>>> #define PTE_A 0x040 /* Accessed */
>>>> #define PTE_D 0x080 /* Dirty */
>>>> #define PTE_SOFT 0x300 /* Reserved for Software */
>>>> +#define PTE_PBMT 0x6000000000000000 /* Page-based memory types */
>>>> #define PTE_N 0x8000000000000000 /* NAPOT translation */
>>>> +#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>>>
>>>> /* Page table PPN shift amount */
>>>> #define PTE_PPN_SHIFT 10
>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>> index c276760c7f..9fffaccffb 100644
>>>> --- a/target/riscv/cpu_helper.c
>>>> +++ b/target/riscv/cpu_helper.c
>>>> @@ -625,9 +625,11 @@ restart:
>>>> if (!(pte & PTE_V)) {
>>>> /* Invalid PTE */
>>>> return TRANSLATE_FAIL;
>>>> + } else if (!cpu->cfg.ext_svpbmt && (pte & (target_ulong)PTE_PBMT)) {
>>> Rather than, type-casting defines here you can simply define
>>> ULL constants. E.g.
>>> #define PTE_PBMT 0x6000000000000000ULL
>> Sorry, I'm wonder why add ULL can replace the function of type-casting.
>>
>> The type-casting here is to compatible with RV32 for possible strict
>> type check warnings since pte is 32 bits and PTE_PBMT is 64 bits in RV32.
> If adding ULL does not help for RV32 target then no need to change.
OK. Thanks a lot.
Regards,
Weiwei Li
>> If I add ULL in PTE_PBMT, it seems have no change to PTE_PBMT. It's
>> still 64 bits in RV32.
> Reviewed-by: Anup Patel <anup@brainfault.org>
>
> Regards,
> Anup
>
>> Regards,
>>
>> Weiwei Li
>>
>>>> + return TRANSLATE_FAIL;
>>>> } else if (!(pte & (PTE_R | PTE_W | PTE_X))) {
>>>> /* Inner PTE, continue walking */
>>>> - if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_N)) {
>>>> + if (pte & (target_ulong)(PTE_D | PTE_A | PTE_U | PTE_ATTR)) {
>>>> return TRANSLATE_FAIL;
>>>> }
>>>> base = ppn << PGSHIFT;
>>>> --
>>>> 2.17.1
>>>>
>>> Regards,
>>> Anup
>>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 11:15 ` Guo Ren
@ 2022-01-18 11:25 ` Anup Patel
-1 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 11:25 UTC (permalink / raw)
To: Guo Ren
Cc: Anup Patel, Bin Meng, open list:RISC-V, Wang Junqiang, Weiwei Li,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 4:45 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > > ---
> > > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > > --- a/target/riscv/cpu_bits.h
> > > > > +++ b/target/riscv/cpu_bits.h
> > > > > @@ -490,6 +490,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
> > > > > +
> > > >
> > > > Going forward we should avoid using target specific "#if"
> > > > so that we can use the same qemu-system-riscv64 for both
> > > > RV32 and RV64.
> > > >
> > > > > /* Leaf page shift amount */
> > > > > #define PGSHIFT 12
> > > > >
> > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > > index 434a83e66a..26608ddf1c 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;
> > > >
> > > > Rather than using "#if", please use "xlen" comparison to extract
> > > > PPN correctly from PTE.
> > > Do you mean?
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 9fffaccffb..071b7ea4cf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -619,7 +619,11 @@ restart:
> > > return TRANSLATE_FAIL;
> > > }
> > >
> > > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > + } else {
> > > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > > + }
> >
> > Yes, something like this but use a define for 0x3fffffffffffffULL
> Just as Alistair said: This will need to be dynamic based on get_xl()
By get_xl() Alistair meant riscv_cpu_mxl()
>
> I still prefer:
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK 0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK 0x3fffffffffffffULL
> +#endif
>
> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
This approach does not allow us to use same
qemu-system-riscv64 for RV32 emulation.
Regards,
Anup
>
> >
> > Regards,
> > Anup
> >
> > >
> > > RISCVCPU *cpu = env_archcpu(env);
> > > if (!(pte & PTE_V)) {
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > >
> > > > > if (!(pte & PTE_V)) {
> > > > > /* Invalid PTE */
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> > >
> > > ML: https://lore.kernel.org/linux-csky/
> > >
>
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 11:25 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 11:25 UTC (permalink / raw)
To: Guo Ren
Cc: Anup Patel, Weiwei Li, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 4:45 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > > ---
> > > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > > --- a/target/riscv/cpu_bits.h
> > > > > +++ b/target/riscv/cpu_bits.h
> > > > > @@ -490,6 +490,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
> > > > > +
> > > >
> > > > Going forward we should avoid using target specific "#if"
> > > > so that we can use the same qemu-system-riscv64 for both
> > > > RV32 and RV64.
> > > >
> > > > > /* Leaf page shift amount */
> > > > > #define PGSHIFT 12
> > > > >
> > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > > index 434a83e66a..26608ddf1c 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;
> > > >
> > > > Rather than using "#if", please use "xlen" comparison to extract
> > > > PPN correctly from PTE.
> > > Do you mean?
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 9fffaccffb..071b7ea4cf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -619,7 +619,11 @@ restart:
> > > return TRANSLATE_FAIL;
> > > }
> > >
> > > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > + } else {
> > > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > > + }
> >
> > Yes, something like this but use a define for 0x3fffffffffffffULL
> Just as Alistair said: This will need to be dynamic based on get_xl()
By get_xl() Alistair meant riscv_cpu_mxl()
>
> I still prefer:
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK 0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK 0x3fffffffffffffULL
> +#endif
>
> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
This approach does not allow us to use same
qemu-system-riscv64 for RV32 emulation.
Regards,
Anup
>
> >
> > Regards,
> > Anup
> >
> > >
> > > RISCVCPU *cpu = env_archcpu(env);
> > > if (!(pte & PTE_V)) {
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > >
> > > > > if (!(pte & PTE_V)) {
> > > > > /* Invalid PTE */
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> > >
> > > ML: https://lore.kernel.org/linux-csky/
> > >
>
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 11:15 ` Guo Ren
@ 2022-01-18 11:28 ` Anup Patel
-1 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 11:28 UTC (permalink / raw)
To: Guo Ren
Cc: Anup Patel, Bin Meng, open list:RISC-V, Wang Junqiang, Weiwei Li,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 4:45 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > > ---
> > > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > > --- a/target/riscv/cpu_bits.h
> > > > > +++ b/target/riscv/cpu_bits.h
> > > > > @@ -490,6 +490,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
> > > > > +
> > > >
> > > > Going forward we should avoid using target specific "#if"
> > > > so that we can use the same qemu-system-riscv64 for both
> > > > RV32 and RV64.
> > > >
> > > > > /* Leaf page shift amount */
> > > > > #define PGSHIFT 12
> > > > >
> > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > > index 434a83e66a..26608ddf1c 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;
> > > >
> > > > Rather than using "#if", please use "xlen" comparison to extract
> > > > PPN correctly from PTE.
> > > Do you mean?
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 9fffaccffb..071b7ea4cf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -619,7 +619,11 @@ restart:
> > > return TRANSLATE_FAIL;
> > > }
> > >
> > > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > + } else {
> > > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > > + }
> >
> > Yes, something like this but use a define for 0x3fffffffffffffULL
> Just as Alistair said: This will need to be dynamic based on get_xl()
>
> I still prefer:
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK 0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK 0x3fffffffffffffULL
> +#endif
>
> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
Actually, using cpu_get_xl() is even better because it allows
having lower privilege mode running at different XLEN.
Regards,
Anup
>
> >
> > Regards,
> > Anup
> >
> > >
> > > RISCVCPU *cpu = env_archcpu(env);
> > > if (!(pte & PTE_V)) {
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > >
> > > > > if (!(pte & PTE_V)) {
> > > > > /* Invalid PTE */
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> > >
> > > ML: https://lore.kernel.org/linux-csky/
> > >
>
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 11:28 ` Anup Patel
0 siblings, 0 replies; 52+ messages in thread
From: Anup Patel @ 2022-01-18 11:28 UTC (permalink / raw)
To: Guo Ren
Cc: Anup Patel, Weiwei Li, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 4:45 PM Guo Ren <guoren@kernel.org> wrote:
>
> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
> >
> > On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > > ---
> > > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > > --- a/target/riscv/cpu_bits.h
> > > > > +++ b/target/riscv/cpu_bits.h
> > > > > @@ -490,6 +490,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
> > > > > +
> > > >
> > > > Going forward we should avoid using target specific "#if"
> > > > so that we can use the same qemu-system-riscv64 for both
> > > > RV32 and RV64.
> > > >
> > > > > /* Leaf page shift amount */
> > > > > #define PGSHIFT 12
> > > > >
> > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > > index 434a83e66a..26608ddf1c 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;
> > > >
> > > > Rather than using "#if", please use "xlen" comparison to extract
> > > > PPN correctly from PTE.
> > > Do you mean?
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 9fffaccffb..071b7ea4cf 100644
> > > --- a/target/riscv/cpu_helper.c
> > > +++ b/target/riscv/cpu_helper.c
> > > @@ -619,7 +619,11 @@ restart:
> > > return TRANSLATE_FAIL;
> > > }
> > >
> > > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > + } else {
> > > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > > + }
> >
> > Yes, something like this but use a define for 0x3fffffffffffffULL
> Just as Alistair said: This will need to be dynamic based on get_xl()
>
> I still prefer:
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK 0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK 0x3fffffffffffffULL
> +#endif
>
> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
Actually, using cpu_get_xl() is even better because it allows
having lower privilege mode running at different XLEN.
Regards,
Anup
>
> >
> > Regards,
> > Anup
> >
> > >
> > > RISCVCPU *cpu = env_archcpu(env);
> > > if (!(pte & PTE_V)) {
> > >
> > > >
> > > > Regards,
> > > > Anup
> > > >
> > > > >
> > > > > if (!(pte & PTE_V)) {
> > > > > /* Invalid PTE */
> > > > > --
> > > > > 2.17.1
> > > > >
> > > >
> > >
> > >
> > > --
> > > Best Regards
> > > Guo Ren
> > >
> > > ML: https://lore.kernel.org/linux-csky/
> > >
>
>
>
> --
> Best Regards
> Guo Ren
>
> ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 11:15 ` Guo Ren
@ 2022-01-18 11:29 ` Weiwei Li
-1 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 11:29 UTC (permalink / raw)
To: Guo Ren, Anup Patel
Cc: open list:RISC-V, Anup Patel, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
在 2022/1/18 下午7:15, Guo Ren 写道:
> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
>> On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
>>> On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
>>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
>>>>> ---
>>>>> 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 5a6d49aa64..282cd8eecd 100644
>>>>> --- a/target/riscv/cpu_bits.h
>>>>> +++ b/target/riscv/cpu_bits.h
>>>>> @@ -490,6 +490,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
>>>>> +
>>>> Going forward we should avoid using target specific "#if"
>>>> so that we can use the same qemu-system-riscv64 for both
>>>> RV32 and RV64.
>>>>
>>>>> /* Leaf page shift amount */
>>>>> #define PGSHIFT 12
>>>>>
>>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>>> index 434a83e66a..26608ddf1c 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;
>>>> Rather than using "#if", please use "xlen" comparison to extract
>>>> PPN correctly from PTE.
>>> Do you mean?
>>>
>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>> index 9fffaccffb..071b7ea4cf 100644
>>> --- a/target/riscv/cpu_helper.c
>>> +++ b/target/riscv/cpu_helper.c
>>> @@ -619,7 +619,11 @@ restart:
>>> return TRANSLATE_FAIL;
>>> }
>>>
>>> - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>> + if (riscv_cpu_mxl(env) == MXL_RV32) {
>>> + hwaddr ppn = pte >> PTE_PPN_SHIFT;
>>> + } else {
>>> + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
>>> + }
>> Yes, something like this but use a define for 0x3fffffffffffffULL
> Just as Alistair said: This will need to be dynamic based on get_xl()
>
> I still prefer:
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK 0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK 0x3fffffffffffffULL
> +#endif
>
> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
I think the difference may be xl = MXL_RV32 in RV64.
Or we may define PTE_PPN_MASK as 0x3fffffffffffffULL, and use type
contrast
+ hwaddr ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
Regards,
Weiwei Li
>> Regards,
>> Anup
>>
>>> RISCVCPU *cpu = env_archcpu(env);
>>> if (!(pte & PTE_V)) {
>>>
>>>> Regards,
>>>> Anup
>>>>
>>>>> if (!(pte & PTE_V)) {
>>>>> /* Invalid PTE */
>>>>> --
>>>>> 2.17.1
>>>>>
>>>
>>> --
>>> Best Regards
>>> Guo Ren
>>>
>>> ML: https://lore.kernel.org/linux-csky/
>>>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 11:29 ` Weiwei Li
0 siblings, 0 replies; 52+ messages in thread
From: Weiwei Li @ 2022-01-18 11:29 UTC (permalink / raw)
To: Guo Ren, Anup Patel
Cc: Anup Patel, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
在 2022/1/18 下午7:15, Guo Ren 写道:
> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
>> On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
>>> On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
>>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
>>>>> ---
>>>>> 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 5a6d49aa64..282cd8eecd 100644
>>>>> --- a/target/riscv/cpu_bits.h
>>>>> +++ b/target/riscv/cpu_bits.h
>>>>> @@ -490,6 +490,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
>>>>> +
>>>> Going forward we should avoid using target specific "#if"
>>>> so that we can use the same qemu-system-riscv64 for both
>>>> RV32 and RV64.
>>>>
>>>>> /* Leaf page shift amount */
>>>>> #define PGSHIFT 12
>>>>>
>>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>>> index 434a83e66a..26608ddf1c 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;
>>>> Rather than using "#if", please use "xlen" comparison to extract
>>>> PPN correctly from PTE.
>>> Do you mean?
>>>
>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>> index 9fffaccffb..071b7ea4cf 100644
>>> --- a/target/riscv/cpu_helper.c
>>> +++ b/target/riscv/cpu_helper.c
>>> @@ -619,7 +619,11 @@ restart:
>>> return TRANSLATE_FAIL;
>>> }
>>>
>>> - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>> + if (riscv_cpu_mxl(env) == MXL_RV32) {
>>> + hwaddr ppn = pte >> PTE_PPN_SHIFT;
>>> + } else {
>>> + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
>>> + }
>> Yes, something like this but use a define for 0x3fffffffffffffULL
> Just as Alistair said: This will need to be dynamic based on get_xl()
>
> I still prefer:
> +#if defined(TARGET_RISCV32)
> +#define PTE_PPN_MASK 0xffffffffUL
> +#elif defined(TARGET_RISCV64)
> +#define PTE_PPN_MASK 0x3fffffffffffffULL
> +#endif
>
> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
I think the difference may be xl = MXL_RV32 in RV64.
Or we may define PTE_PPN_MASK as 0x3fffffffffffffULL, and use type
contrast
+ hwaddr ppn = (pte & (target_ulong)PTE_PPN_MASK) >> PTE_PPN_SHIFT;
Regards,
Weiwei Li
>> Regards,
>> Anup
>>
>>> RISCVCPU *cpu = env_archcpu(env);
>>> if (!(pte & PTE_V)) {
>>>
>>>> Regards,
>>>> Anup
>>>>
>>>>> if (!(pte & PTE_V)) {
>>>>> /* Invalid PTE */
>>>>> --
>>>>> 2.17.1
>>>>>
>>>
>>> --
>>> Best Regards
>>> Guo Ren
>>>
>>> ML: https://lore.kernel.org/linux-csky/
>>>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 4:51 ` Alistair Francis
@ 2022-01-18 11:54 ` Guo Ren
-1 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 11:54 UTC (permalink / raw)
To: Alistair Francis
Cc: Weiwei Li, open list:RISC-V, Anup Patel, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > ---
> > > 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 5a6d49aa64..282cd8eecd 100644
> > > --- a/target/riscv/cpu_bits.h
> > > +++ b/target/riscv/cpu_bits.h
> > > @@ -490,6 +490,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
> > > +
> >
> > Going forward we should avoid using target specific "#if"
> > so that we can use the same qemu-system-riscv64 for both
> > RV32 and RV64.
> >
> > > /* Leaf page shift amount */
> > > #define PGSHIFT 12
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 434a83e66a..26608ddf1c 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;
> >
> > Rather than using "#if", please use "xlen" comparison to extract
> > PPN correctly from PTE.
>
> 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."
Okay, we would add some checker, such as:
+ if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot)
+ hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ else
+ hwaddr ppn = pte >> PTE_PPN_SHIFT;
>
> Alistair
>
> >
> > Regards,
> > Anup
> >
> > >
> > > if (!(pte & PTE_V)) {
> > > /* Invalid PTE */
> > > --
> > > 2.17.1
> > >
> >
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 11:54 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 11:54 UTC (permalink / raw)
To: Alistair Francis
Cc: Anup Patel, Weiwei Li, open list:RISC-V, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > ---
> > > 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 5a6d49aa64..282cd8eecd 100644
> > > --- a/target/riscv/cpu_bits.h
> > > +++ b/target/riscv/cpu_bits.h
> > > @@ -490,6 +490,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
> > > +
> >
> > Going forward we should avoid using target specific "#if"
> > so that we can use the same qemu-system-riscv64 for both
> > RV32 and RV64.
> >
> > > /* Leaf page shift amount */
> > > #define PGSHIFT 12
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 434a83e66a..26608ddf1c 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;
> >
> > Rather than using "#if", please use "xlen" comparison to extract
> > PPN correctly from PTE.
>
> 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."
Okay, we would add some checker, such as:
+ if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot)
+ hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ else
+ hwaddr ppn = pte >> PTE_PPN_SHIFT;
>
> Alistair
>
> >
> > Regards,
> > Anup
> >
> > >
> > > if (!(pte & PTE_V)) {
> > > /* Invalid PTE */
> > > --
> > > 2.17.1
> > >
> >
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 11:28 ` Anup Patel
@ 2022-01-18 11:57 ` Guo Ren
-1 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 11:57 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Bin Meng, open list:RISC-V, Wang Junqiang, Weiwei Li,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 7:28 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 4:45 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > > > ---
> > > > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > > > --- a/target/riscv/cpu_bits.h
> > > > > > +++ b/target/riscv/cpu_bits.h
> > > > > > @@ -490,6 +490,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
> > > > > > +
> > > > >
> > > > > Going forward we should avoid using target specific "#if"
> > > > > so that we can use the same qemu-system-riscv64 for both
> > > > > RV32 and RV64.
> > > > >
> > > > > > /* Leaf page shift amount */
> > > > > > #define PGSHIFT 12
> > > > > >
> > > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > > > index 434a83e66a..26608ddf1c 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;
> > > > >
> > > > > Rather than using "#if", please use "xlen" comparison to extract
> > > > > PPN correctly from PTE.
> > > > Do you mean?
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index 9fffaccffb..071b7ea4cf 100644
> > > > --- a/target/riscv/cpu_helper.c
> > > > +++ b/target/riscv/cpu_helper.c
> > > > @@ -619,7 +619,11 @@ restart:
> > > > return TRANSLATE_FAIL;
> > > > }
> > > >
> > > > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > > > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > > > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > > + } else {
> > > > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > > > + }
> > >
> > > Yes, something like this but use a define for 0x3fffffffffffffULL
> > Just as Alistair said: This will need to be dynamic based on get_xl()
> >
> > I still prefer:
> > +#if defined(TARGET_RISCV32)
> > +#define PTE_PPN_MASK 0xffffffffUL
> > +#elif defined(TARGET_RISCV64)
> > +#define PTE_PPN_MASK 0x3fffffffffffffULL
> > +#endif
> >
> > + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> Actually, using cpu_get_xl() is even better because it allows
> having lower privilege mode running at different XLEN.
Good point. You have convinced me.
>
> Regards,
> Anup
>
> >
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > RISCVCPU *cpu = env_archcpu(env);
> > > > if (!(pte & PTE_V)) {
> > > >
> > > > >
> > > > > Regards,
> > > > > Anup
> > > > >
> > > > > >
> > > > > > if (!(pte & PTE_V)) {
> > > > > > /* Invalid PTE */
> > > > > > --
> > > > > > 2.17.1
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> > > >
> > > > ML: https://lore.kernel.org/linux-csky/
> > > >
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-18 11:57 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-18 11:57 UTC (permalink / raw)
To: Anup Patel
Cc: Anup Patel, Weiwei Li, open list:RISC-V, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On Tue, Jan 18, 2022 at 7:28 PM Anup Patel <anup@brainfault.org> wrote:
>
> On Tue, Jan 18, 2022 at 4:45 PM Guo Ren <guoren@kernel.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com> wrote:
> > >
> > > On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
> > > >
> > > > On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org> wrote:
> > > > >
> > > > > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > > > > ---
> > > > > > 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 5a6d49aa64..282cd8eecd 100644
> > > > > > --- a/target/riscv/cpu_bits.h
> > > > > > +++ b/target/riscv/cpu_bits.h
> > > > > > @@ -490,6 +490,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
> > > > > > +
> > > > >
> > > > > Going forward we should avoid using target specific "#if"
> > > > > so that we can use the same qemu-system-riscv64 for both
> > > > > RV32 and RV64.
> > > > >
> > > > > > /* Leaf page shift amount */
> > > > > > #define PGSHIFT 12
> > > > > >
> > > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > > > index 434a83e66a..26608ddf1c 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;
> > > > >
> > > > > Rather than using "#if", please use "xlen" comparison to extract
> > > > > PPN correctly from PTE.
> > > > Do you mean?
> > > >
> > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > > index 9fffaccffb..071b7ea4cf 100644
> > > > --- a/target/riscv/cpu_helper.c
> > > > +++ b/target/riscv/cpu_helper.c
> > > > @@ -619,7 +619,11 @@ restart:
> > > > return TRANSLATE_FAIL;
> > > > }
> > > >
> > > > - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > > > + if (riscv_cpu_mxl(env) == MXL_RV32) {
> > > > + hwaddr ppn = pte >> PTE_PPN_SHIFT;
> > > > + } else {
> > > > + hwaddr ppn = (pte & 0x3fffffffffffffULL) >> PTE_PPN_SHIFT;
> > > > + }
> > >
> > > Yes, something like this but use a define for 0x3fffffffffffffULL
> > Just as Alistair said: This will need to be dynamic based on get_xl()
> >
> > I still prefer:
> > +#if defined(TARGET_RISCV32)
> > +#define PTE_PPN_MASK 0xffffffffUL
> > +#elif defined(TARGET_RISCV64)
> > +#define PTE_PPN_MASK 0x3fffffffffffffULL
> > +#endif
> >
> > + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> Actually, using cpu_get_xl() is even better because it allows
> having lower privilege mode running at different XLEN.
Good point. You have convinced me.
>
> Regards,
> Anup
>
> >
> > >
> > > Regards,
> > > Anup
> > >
> > > >
> > > > RISCVCPU *cpu = env_archcpu(env);
> > > > if (!(pte & PTE_V)) {
> > > >
> > > > >
> > > > > Regards,
> > > > > Anup
> > > > >
> > > > > >
> > > > > > if (!(pte & PTE_V)) {
> > > > > > /* Invalid PTE */
> > > > > > --
> > > > > > 2.17.1
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > Best Regards
> > > > Guo Ren
> > > >
> > > > ML: https://lore.kernel.org/linux-csky/
> > > >
> >
> >
> >
> > --
> > Best Regards
> > Guo Ren
> >
> > ML: https://lore.kernel.org/linux-csky/
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 11:29 ` Weiwei Li
(?)
@ 2022-01-19 3:14 ` LIU Zhiwei
-1 siblings, 0 replies; 52+ messages in thread
From: LIU Zhiwei @ 2022-01-19 3:14 UTC (permalink / raw)
To: Weiwei Li, Guo Ren, Anup Patel
Cc: open list:RISC-V, Anup Patel, Wang Junqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On 2022/1/18 下午7:29, Weiwei Li wrote:
>
> 在 2022/1/18 下午7:15, Guo Ren 写道:
>> On Tue, Jan 18, 2022 at 4:51 PM Anup Patel <apatel@ventanamicro.com>
>> wrote:
>>> On Tue, Jan 18, 2022 at 2:16 PM Guo Ren <guoren@kernel.org> wrote:
>>>> On Tue, Jan 18, 2022 at 11:32 AM Anup Patel <anup@brainfault.org>
>>>> wrote:
>>>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn>
>>>>> 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>
>>>>>> ---
>>>>>> 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 5a6d49aa64..282cd8eecd 100644
>>>>>> --- a/target/riscv/cpu_bits.h
>>>>>> +++ b/target/riscv/cpu_bits.h
>>>>>> @@ -490,6 +490,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
>>>>>> +
>>>>> Going forward we should avoid using target specific "#if"
>>>>> so that we can use the same qemu-system-riscv64 for both
>>>>> RV32 and RV64.
>>>>>
>>>>>> /* Leaf page shift amount */
>>>>>> #define PGSHIFT 12
>>>>>>
>>>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>>>> index 434a83e66a..26608ddf1c 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;
>>>>> Rather than using "#if", please use "xlen" comparison to extract
>>>>> PPN correctly from PTE.
>>>> Do you mean?
>>>>
>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>> index 9fffaccffb..071b7ea4cf 100644
>>>> --- a/target/riscv/cpu_helper.c
>>>> +++ b/target/riscv/cpu_helper.c
>>>> @@ -619,7 +619,11 @@ restart:
>>>> return TRANSLATE_FAIL;
>>>> }
>>>>
>>>> - hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>>> + if (riscv_cpu_mxl(env) == MXL_RV32) {
>>>> + hwaddr ppn = pte >> PTE_PPN_SHIFT;
>>>> + } else {
>>>> + hwaddr ppn = (pte & 0x3fffffffffffffULL) >>
>>>> PTE_PPN_SHIFT;
>>>> + }
>>> Yes, something like this but use a define for 0x3fffffffffffffULL
>> Just as Alistair said: This will need to be dynamic based on get_xl()
>>
>> I still prefer:
>> +#if defined(TARGET_RISCV32)
>> +#define PTE_PPN_MASK 0xffffffffUL
>> +#elif defined(TARGET_RISCV64)
>> +#define PTE_PPN_MASK 0x3fffffffffffffULL
>> +#endif
>>
>> + hwaddr ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>
> I think the difference may be xl = MXL_RV32 in RV64.
Agree.
>
> Or we may define PTE_PPN_MASK as 0x3fffffffffffffULL, and use type
> contrast
>
> + hwaddr ppn = (pte & (target_ulong)PTE_PPN_MASK) >>
> PTE_PPN_SHIFT;
It is absolutely not right to use target_ulong here.
Although we don't support dynamic SXLEN currently, we should avoid
assuming that SXLEN is always 64 on qemu-system-riscv64. Get_cpu_xl is
the right way.
I have moved xl to CPURISCVState in my patch set v6 for UXL. I think
it will be much convenient for this situation and you can just use
env->xl to get right XLEN.
But I don't know when it will be merged.
Thanks,
Zhiwei
>
> Regards,
> Weiwei Li
>
>>> Regards,
>>> Anup
>>>
>>>> RISCVCPU *cpu = env_archcpu(env);
>>>> if (!(pte & PTE_V)) {
>>>>
>>>>> Regards,
>>>>> Anup
>>>>>
>>>>>> if (!(pte & PTE_V)) {
>>>>>> /* Invalid PTE */
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>>>>
>>>> --
>>>> Best Regards
>>>> Guo Ren
>>>>
>>>> ML: https://lore.kernel.org/linux-csky/
>>>>
>>
>>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-18 4:51 ` Alistair Francis
(?)
(?)
@ 2022-01-20 13:47 ` Guo Ren
2022-01-20 22:28 ` LIU Zhiwei
-1 siblings, 1 reply; 52+ messages in thread
From: Guo Ren @ 2022-01-20 13:47 UTC (permalink / raw)
To: Alistair Francis, Anup Patel
Cc: Weiwei Li, open list:RISC-V, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
Hi Alistair and Anup,
On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
>
> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
> >
> > On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> > > ---
> > > 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 5a6d49aa64..282cd8eecd 100644
> > > --- a/target/riscv/cpu_bits.h
> > > +++ b/target/riscv/cpu_bits.h
> > > @@ -490,6 +490,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
> > > +
> >
> > Going forward we should avoid using target specific "#if"
> > so that we can use the same qemu-system-riscv64 for both
> > RV32 and RV64.
> >
> > > /* Leaf page shift amount */
> > > #define PGSHIFT 12
> > >
> > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > > index 434a83e66a..26608ddf1c 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;
> >
> > Rather than using "#if", please use "xlen" comparison to extract
> > PPN correctly from PTE.
>
> 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."
How about:
+ RISCVCPU *cpu = env_archcpu(env);
+ hwaddr ppn;
+
+ if (get_field(env->mstatus, MSTATUS64_SXL) == MXL_RV32) {
+ ppn = pte >> PTE_PPN_SHIFT;
+ } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
+ ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
+ } else {
+ ppn = pte >> PTE_PPN_SHIFT;
+ if ((pte & ~PTE_PPN_MASK) >> PTE_PPN_SHIFT)
+ return TRANSLATE_FAIL;
+ }
>
> Alistair
>
> >
> > Regards,
> > Anup
> >
> > >
> > > if (!(pte & PTE_V)) {
> > > /* Invalid PTE */
> > > --
> > > 2.17.1
> > >
> >
>
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-20 13:47 ` Guo Ren
@ 2022-01-20 22:28 ` LIU Zhiwei
2022-01-21 1:50 ` Guo Ren
0 siblings, 1 reply; 52+ messages in thread
From: LIU Zhiwei @ 2022-01-20 22:28 UTC (permalink / raw)
To: Guo Ren, Alistair Francis, Anup Patel
Cc: Weiwei Li, open list:RISC-V, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren,
Wei Wu (吴伟),
Palmer Dabbelt
On 2022/1/20 下午9:47, Guo Ren wrote:
> Hi Alistair and Anup,
>
> On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
>> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
>>>> ---
>>>> 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 5a6d49aa64..282cd8eecd 100644
>>>> --- a/target/riscv/cpu_bits.h
>>>> +++ b/target/riscv/cpu_bits.h
>>>> @@ -490,6 +490,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
>>>> +
>>> Going forward we should avoid using target specific "#if"
>>> so that we can use the same qemu-system-riscv64 for both
>>> RV32 and RV64.
>>>
>>>> /* Leaf page shift amount */
>>>> #define PGSHIFT 12
>>>>
>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>> index 434a83e66a..26608ddf1c 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;
>>> Rather than using "#if", please use "xlen" comparison to extract
>>> PPN correctly from PTE.
>> 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."
> How about:
>
> + RISCVCPU *cpu = env_archcpu(env);
> + hwaddr ppn;
> +
> + if (get_field(env->mstatus, MSTATUS64_SXL) == MXL_RV32) {
Use riscv_cpu_mxl currently. Or define a new function riscv_cpu_sxl in cpu.h
> + ppn = pte >> PTE_PPN_SHIFT;
> + } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
> + ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> + } else {
> + ppn = pte >> PTE_PPN_SHIFT;
> + if ((pte & ~PTE_PPN_MASK) >> PTE_PPN_SHIFT)
Just if (pte & ~PTE_PPN_MASK)
> + return TRANSLATE_FAIL;
> + }
Otherwise looks good to me.
Thanks,
Zhiwei
>> Alistair
>>
>>> Regards,
>>> Anup
>>>
>>>> if (!(pte & PTE_V)) {
>>>> /* Invalid PTE */
>>>> --
>>>> 2.17.1
>>>>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-20 22:28 ` LIU Zhiwei
@ 2022-01-21 1:50 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-21 1:50 UTC (permalink / raw)
To: LIU Zhiwei
Cc: Wei Wu (吴伟),
Weiwei Li, open list:RISC-V, Anup Patel, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren, Alistair Francis,
Palmer Dabbelt
On Fri, Jan 21, 2022 at 6:48 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
>
> On 2022/1/20 下午9:47, Guo Ren wrote:
> > Hi Alistair and Anup,
> >
> > On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
> >> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
> >>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> >>>> ---
> >>>> 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 5a6d49aa64..282cd8eecd 100644
> >>>> --- a/target/riscv/cpu_bits.h
> >>>> +++ b/target/riscv/cpu_bits.h
> >>>> @@ -490,6 +490,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
> >>>> +
> >>> Going forward we should avoid using target specific "#if"
> >>> so that we can use the same qemu-system-riscv64 for both
> >>> RV32 and RV64.
> >>>
> >>>> /* Leaf page shift amount */
> >>>> #define PGSHIFT 12
> >>>>
> >>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >>>> index 434a83e66a..26608ddf1c 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;
> >>> Rather than using "#if", please use "xlen" comparison to extract
> >>> PPN correctly from PTE.
> >> 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."
> > How about:
> >
> > + RISCVCPU *cpu = env_archcpu(env);
> > + hwaddr ppn;
> > +
> > + if (get_field(env->mstatus, MSTATUS64_SXL) == MXL_RV32) {
> Use riscv_cpu_mxl currently. Or define a new function riscv_cpu_sxl in cpu.h
I perfer riscv_cpu_sxl.
> > + ppn = pte >> PTE_PPN_SHIFT;
> > + } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
> > + ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > + } else {
> > + ppn = pte >> PTE_PPN_SHIFT;
> > + if ((pte & ~PTE_PPN_MASK) >> PTE_PPN_SHIFT)
> Just if (pte & ~PTE_PPN_MASK)
Why? low bits in pte is correct. R W X A D
> > + return TRANSLATE_FAIL;
> > + }
>
> Otherwise looks good to me.
>
> Thanks,
> Zhiwei
>
> >> Alistair
> >>
> >>> Regards,
> >>> Anup
> >>>
> >>>> if (!(pte & PTE_V)) {
> >>>> /* Invalid PTE */
> >>>> --
> >>>> 2.17.1
> >>>>
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-21 1:50 ` Guo Ren
0 siblings, 0 replies; 52+ messages in thread
From: Guo Ren @ 2022-01-21 1:50 UTC (permalink / raw)
To: LIU Zhiwei
Cc: Alistair Francis, Anup Patel, Weiwei Li, open list:RISC-V,
wangjunqiang, Bin Meng, QEMU Developers, Alistair Francis,
Guo Ren, Wei Wu (吴伟),
Palmer Dabbelt
On Fri, Jan 21, 2022 at 6:48 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>
>
> On 2022/1/20 下午9:47, Guo Ren wrote:
> > Hi Alistair and Anup,
> >
> > On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
> >> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
> >>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
> >>>> ---
> >>>> 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 5a6d49aa64..282cd8eecd 100644
> >>>> --- a/target/riscv/cpu_bits.h
> >>>> +++ b/target/riscv/cpu_bits.h
> >>>> @@ -490,6 +490,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
> >>>> +
> >>> Going forward we should avoid using target specific "#if"
> >>> so that we can use the same qemu-system-riscv64 for both
> >>> RV32 and RV64.
> >>>
> >>>> /* Leaf page shift amount */
> >>>> #define PGSHIFT 12
> >>>>
> >>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> >>>> index 434a83e66a..26608ddf1c 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;
> >>> Rather than using "#if", please use "xlen" comparison to extract
> >>> PPN correctly from PTE.
> >> 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."
> > How about:
> >
> > + RISCVCPU *cpu = env_archcpu(env);
> > + hwaddr ppn;
> > +
> > + if (get_field(env->mstatus, MSTATUS64_SXL) == MXL_RV32) {
> Use riscv_cpu_mxl currently. Or define a new function riscv_cpu_sxl in cpu.h
I perfer riscv_cpu_sxl.
> > + ppn = pte >> PTE_PPN_SHIFT;
> > + } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
> > + ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
> > + } else {
> > + ppn = pte >> PTE_PPN_SHIFT;
> > + if ((pte & ~PTE_PPN_MASK) >> PTE_PPN_SHIFT)
> Just if (pte & ~PTE_PPN_MASK)
Why? low bits in pte is correct. R W X A D
> > + return TRANSLATE_FAIL;
> > + }
>
> Otherwise looks good to me.
>
> Thanks,
> Zhiwei
>
> >> Alistair
> >>
> >>> Regards,
> >>> Anup
> >>>
> >>>> if (!(pte & PTE_V)) {
> >>>> /* Invalid PTE */
> >>>> --
> >>>> 2.17.1
> >>>>
> >
--
Best Regards
Guo Ren
ML: https://lore.kernel.org/linux-csky/
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
2022-01-21 1:50 ` Guo Ren
@ 2022-01-21 2:08 ` LIU Zhiwei
-1 siblings, 0 replies; 52+ messages in thread
From: LIU Zhiwei @ 2022-01-21 2:08 UTC (permalink / raw)
To: Guo Ren
Cc: Wei Wu (吴伟),
Weiwei Li, open list:RISC-V, Anup Patel, wangjunqiang, Bin Meng,
QEMU Developers, Alistair Francis, Guo Ren, Alistair Francis,
Palmer Dabbelt
On 2022/1/21 上午9:50, Guo Ren wrote:
> On Fri, Jan 21, 2022 at 6:48 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>>
>> On 2022/1/20 下午9:47, Guo Ren wrote:
>>> Hi Alistair and Anup,
>>>
>>> On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
>>>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
>>>>>> ---
>>>>>> 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 5a6d49aa64..282cd8eecd 100644
>>>>>> --- a/target/riscv/cpu_bits.h
>>>>>> +++ b/target/riscv/cpu_bits.h
>>>>>> @@ -490,6 +490,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
>>>>>> +
>>>>> Going forward we should avoid using target specific "#if"
>>>>> so that we can use the same qemu-system-riscv64 for both
>>>>> RV32 and RV64.
>>>>>
>>>>>> /* Leaf page shift amount */
>>>>>> #define PGSHIFT 12
>>>>>>
>>>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>>>> index 434a83e66a..26608ddf1c 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;
>>>>> Rather than using "#if", please use "xlen" comparison to extract
>>>>> PPN correctly from PTE.
>>>> 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."
>>> How about:
>>>
>>> + RISCVCPU *cpu = env_archcpu(env);
>>> + hwaddr ppn;
>>> +
>>> + if (get_field(env->mstatus, MSTATUS64_SXL) == MXL_RV32) {
>> Use riscv_cpu_mxl currently. Or define a new function riscv_cpu_sxl in cpu.h
> I perfer riscv_cpu_sxl.
That's better. Thanks.
>
>>> + ppn = pte >> PTE_PPN_SHIFT;
>>> + } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
>>> + ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>> + } else {
>>> + ppn = pte >> PTE_PPN_SHIFT;
>>> + if ((pte & ~PTE_PPN_MASK) >> PTE_PPN_SHIFT)
>> Just if (pte & ~PTE_PPN_MASK)
> Why? low bits in pte is correct. R W X A D
Oops, I forget about the lower bits.
Zhiwei
>>> + return TRANSLATE_FAIL;
>>> + }
>> Otherwise looks good to me.
>>
>> Thanks,
>> Zhiwei
>>
>>>> Alistair
>>>>
>>>>> Regards,
>>>>> Anup
>>>>>
>>>>>> if (!(pte & PTE_V)) {
>>>>>> /* Invalid PTE */
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
* Re: [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64
@ 2022-01-21 2:08 ` LIU Zhiwei
0 siblings, 0 replies; 52+ messages in thread
From: LIU Zhiwei @ 2022-01-21 2:08 UTC (permalink / raw)
To: Guo Ren
Cc: Alistair Francis, Anup Patel, Weiwei Li, open list:RISC-V,
wangjunqiang, Bin Meng, QEMU Developers, Alistair Francis,
Guo Ren, Wei Wu (吴伟),
Palmer Dabbelt
On 2022/1/21 上午9:50, Guo Ren wrote:
> On Fri, Jan 21, 2022 at 6:48 AM LIU Zhiwei <zhiwei_liu@c-sky.com> wrote:
>>
>> On 2022/1/20 下午9:47, Guo Ren wrote:
>>> Hi Alistair and Anup,
>>>
>>> On Tue, Jan 18, 2022 at 12:56 PM Alistair Francis <alistair23@gmail.com> wrote:
>>>> On Tue, Jan 18, 2022 at 1:31 PM Anup Patel <anup@brainfault.org> wrote:
>>>>> On Tue, Jan 18, 2022 at 6:47 AM Weiwei Li <liweiwei@iscas.ac.cn> 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>
>>>>>> ---
>>>>>> 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 5a6d49aa64..282cd8eecd 100644
>>>>>> --- a/target/riscv/cpu_bits.h
>>>>>> +++ b/target/riscv/cpu_bits.h
>>>>>> @@ -490,6 +490,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
>>>>>> +
>>>>> Going forward we should avoid using target specific "#if"
>>>>> so that we can use the same qemu-system-riscv64 for both
>>>>> RV32 and RV64.
>>>>>
>>>>>> /* Leaf page shift amount */
>>>>>> #define PGSHIFT 12
>>>>>>
>>>>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>>>>> index 434a83e66a..26608ddf1c 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;
>>>>> Rather than using "#if", please use "xlen" comparison to extract
>>>>> PPN correctly from PTE.
>>>> 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."
>>> How about:
>>>
>>> + RISCVCPU *cpu = env_archcpu(env);
>>> + hwaddr ppn;
>>> +
>>> + if (get_field(env->mstatus, MSTATUS64_SXL) == MXL_RV32) {
>> Use riscv_cpu_mxl currently. Or define a new function riscv_cpu_sxl in cpu.h
> I perfer riscv_cpu_sxl.
That's better. Thanks.
>
>>> + ppn = pte >> PTE_PPN_SHIFT;
>>> + } else if (cpu->cfg.ext_svpbmt || cpu->cfg.ext_svnapot) {
>>> + ppn = (pte & PTE_PPN_MASK) >> PTE_PPN_SHIFT;
>>> + } else {
>>> + ppn = pte >> PTE_PPN_SHIFT;
>>> + if ((pte & ~PTE_PPN_MASK) >> PTE_PPN_SHIFT)
>> Just if (pte & ~PTE_PPN_MASK)
> Why? low bits in pte is correct. R W X A D
Oops, I forget about the lower bits.
Zhiwei
>>> + return TRANSLATE_FAIL;
>>> + }
>> Otherwise looks good to me.
>>
>> Thanks,
>> Zhiwei
>>
>>>> Alistair
>>>>
>>>>> Regards,
>>>>> Anup
>>>>>
>>>>>> if (!(pte & PTE_V)) {
>>>>>> /* Invalid PTE */
>>>>>> --
>>>>>> 2.17.1
>>>>>>
>
>
^ permalink raw reply [flat|nested] 52+ messages in thread
end of thread, other threads:[~2022-01-21 2:14 UTC | newest]
Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-18 1:17 [PATCH v5 0/5] support subsets of virtual memory extension Weiwei Li
2022-01-18 1:17 ` Weiwei Li
2022-01-18 1:17 ` [PATCH v5 1/5] target/riscv: Ignore reserved bits in PTE for RV64 Weiwei Li
2022-01-18 3:30 ` Anup Patel
2022-01-18 3:30 ` Anup Patel
2022-01-18 4:51 ` Alistair Francis
2022-01-18 4:51 ` Alistair Francis
2022-01-18 11:54 ` Guo Ren
2022-01-18 11:54 ` Guo Ren
2022-01-20 13:47 ` Guo Ren
2022-01-20 22:28 ` LIU Zhiwei
2022-01-21 1:50 ` Guo Ren
2022-01-21 1:50 ` Guo Ren
2022-01-21 2:08 ` LIU Zhiwei
2022-01-21 2:08 ` LIU Zhiwei
2022-01-18 8:33 ` Guo Ren
2022-01-18 8:33 ` Guo Ren
2022-01-18 8:51 ` Anup Patel
2022-01-18 8:51 ` Anup Patel
2022-01-18 11:15 ` Guo Ren
2022-01-18 11:15 ` Guo Ren
2022-01-18 11:25 ` Anup Patel
2022-01-18 11:25 ` Anup Patel
2022-01-18 11:28 ` Anup Patel
2022-01-18 11:28 ` Anup Patel
2022-01-18 11:57 ` Guo Ren
2022-01-18 11:57 ` Guo Ren
2022-01-18 11:29 ` Weiwei Li
2022-01-18 11:29 ` Weiwei Li
2022-01-19 3:14 ` LIU Zhiwei
2022-01-18 1:17 ` [PATCH v5 2/5] target/riscv: add PTE_A/PTE_D/PTE_U bits check for inner PTE Weiwei Li
2022-01-18 1:17 ` Weiwei Li
2022-01-18 1:17 ` [PATCH v5 3/5] target/riscv: add support for svnapot extension Weiwei Li
2022-01-18 1:17 ` Weiwei Li
2022-01-18 3:32 ` Anup Patel
2022-01-18 3:32 ` Anup Patel
2022-01-18 8:32 ` Weiwei Li
2022-01-18 8:32 ` Weiwei Li
2022-01-18 1:17 ` [PATCH v5 4/5] target/riscv: add support for svinval extension Weiwei Li
2022-01-18 1:17 ` Weiwei Li
2022-01-18 1:17 ` [PATCH v5 5/5] target/riscv: add support for svpbmt extension Weiwei Li
2022-01-18 1:17 ` Weiwei Li
2022-01-18 3:35 ` Anup Patel
2022-01-18 3:35 ` Anup Patel
2022-01-18 8:33 ` Weiwei Li
2022-01-18 8:33 ` Weiwei Li
2022-01-18 9:09 ` Weiwei Li
2022-01-18 9:09 ` Weiwei Li
2022-01-18 11:04 ` Anup Patel
2022-01-18 11:04 ` Anup Patel
2022-01-18 11:21 ` Weiwei Li
2022-01-18 11:21 ` Weiwei Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.