* [PATCH] target: riscv: Add Svrsw60t59b extension support
@ 2025-06-05 14:21 Alexandre Ghiti
2025-06-06 16:34 ` Deepak Gupta
2025-06-07 17:54 ` Daniel Henrique Barboza
0 siblings, 2 replies; 5+ messages in thread
From: Alexandre Ghiti @ 2025-06-05 14:21 UTC (permalink / raw)
To: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, debug, qemu-riscv,
qemu-devel
Cc: Alexandre Ghiti
The Svrsw60t59b extension allows to free the PTE reserved bits 60 and 59
for software to use.
Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
---
Changes in v2:
- Add support for IOMMU
- Make svrsw60t59b depend on sv39 (deepak)
Open question: svrsw60t59b in IOMMU should also depend on 64bit, but I
did not find an easy to way in riscv_iommu_realize() to detect that, how
should I do?
hw/riscv/riscv-iommu-bits.h | 1 +
hw/riscv/riscv-iommu.c | 3 ++-
target/riscv/cpu.c | 2 ++
target/riscv/cpu_bits.h | 3 ++-
target/riscv/cpu_cfg_fields.h.inc | 1 +
target/riscv/cpu_helper.c | 3 ++-
target/riscv/tcg/tcg-cpu.c | 6 ++++++
7 files changed, 16 insertions(+), 3 deletions(-)
diff --git a/hw/riscv/riscv-iommu-bits.h b/hw/riscv/riscv-iommu-bits.h
index 1017d73fc6..47fe01bee5 100644
--- a/hw/riscv/riscv-iommu-bits.h
+++ b/hw/riscv/riscv-iommu-bits.h
@@ -79,6 +79,7 @@ struct riscv_iommu_pq_record {
#define RISCV_IOMMU_CAP_SV39 BIT_ULL(9)
#define RISCV_IOMMU_CAP_SV48 BIT_ULL(10)
#define RISCV_IOMMU_CAP_SV57 BIT_ULL(11)
+#define RISCV_IOMMU_CAP_SVRSW60T59B BIT_ULL(14)
#define RISCV_IOMMU_CAP_SV32X4 BIT_ULL(16)
#define RISCV_IOMMU_CAP_SV39X4 BIT_ULL(17)
#define RISCV_IOMMU_CAP_SV48X4 BIT_ULL(18)
diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
index a877e5da84..36eda95a1c 100644
--- a/hw/riscv/riscv-iommu.c
+++ b/hw/riscv/riscv-iommu.c
@@ -2355,7 +2355,8 @@ static void riscv_iommu_realize(DeviceState *dev, Error **errp)
}
if (s->enable_g_stage) {
s->cap |= RISCV_IOMMU_CAP_SV32X4 | RISCV_IOMMU_CAP_SV39X4 |
- RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4;
+ RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4 |
+ RISCV_IOMMU_CAP_SVRSW60T59B;
}
if (s->hpm_cntrs > 0) {
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index 629ac37501..13f1f56d95 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -228,6 +228,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
+ ISA_EXT_DATA_ENTRY(svrsw60t59b, PRIV_VERSION_1_13_0, ext_svrsw60t59b),
ISA_EXT_DATA_ENTRY(svukte, PRIV_VERSION_1_13_0, ext_svukte),
ISA_EXT_DATA_ENTRY(svvptc, PRIV_VERSION_1_13_0, ext_svvptc),
ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
@@ -1282,6 +1283,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
+ MULTI_EXT_CFG_BOOL("svrsw60t59b", ext_svrsw60t59b, false),
MULTI_EXT_CFG_BOOL("svvptc", ext_svvptc, true),
MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index a30317c617..51eb7114da 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -675,7 +675,8 @@ typedef enum {
#define PTE_SOFT 0x300 /* Reserved for Software */
#define PTE_PBMT 0x6000000000000000ULL /* Page-based memory types */
#define PTE_N 0x8000000000000000ULL /* NAPOT translation */
-#define PTE_RESERVED 0x1FC0000000000000ULL /* Reserved bits */
+#define PTE_RESERVED(svrsw60t59b) \
+ (svrsw60t59b ? 0x07C0000000000000ULL : 0x1FC0000000000000ULL) /* Reserved bits */
#define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
/* Page table PPN shift amount */
diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc
index 59f134a419..ab61c1ccf2 100644
--- a/target/riscv/cpu_cfg_fields.h.inc
+++ b/target/riscv/cpu_cfg_fields.h.inc
@@ -57,6 +57,7 @@ BOOL_FIELD(ext_svadu)
BOOL_FIELD(ext_svinval)
BOOL_FIELD(ext_svnapot)
BOOL_FIELD(ext_svpbmt)
+BOOL_FIELD(ext_svrsw60t59b)
BOOL_FIELD(ext_svvptc)
BOOL_FIELD(ext_svukte)
BOOL_FIELD(ext_zdinx)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 2ed69d7c2d..3479a62cc7 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -1309,6 +1309,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
bool svade = riscv_cpu_cfg(env)->ext_svade;
bool svadu = riscv_cpu_cfg(env)->ext_svadu;
bool adue = svadu ? env->menvcfg & MENVCFG_ADUE : !svade;
+ bool svrsw60t59b = riscv_cpu_cfg(env)->ext_svrsw60t59b;
if (first_stage && two_stage && env->virt_enabled) {
pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
@@ -1376,7 +1377,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
if (riscv_cpu_sxl(env) == MXL_RV32) {
ppn = pte >> PTE_PPN_SHIFT;
} else {
- if (pte & PTE_RESERVED) {
+ if (pte & PTE_RESERVED(svrsw60t59b)) {
qemu_log_mask(LOG_GUEST_ERROR, "%s: reserved bits set in PTE: "
"addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx "\n",
__func__, pte_addr, pte);
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index 305912b8dd..886006abc3 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -804,6 +804,12 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
cpu->cfg.ext_ssctr = false;
}
+ if (cpu->cfg.ext_svrsw60t59b &&
+ (!cpu->cfg.mmu || mcc->def->misa_mxl_max == MXL_RV32)) {
+ error_setg(errp, "svrsw60t59b is not supported on RV32 and MMU-less platforms");
+ return;
+ }
+
/*
* Disable isa extensions based on priv spec after we
* validated and set everything we need.
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] target: riscv: Add Svrsw60t59b extension support
2025-06-05 14:21 [PATCH] target: riscv: Add Svrsw60t59b extension support Alexandre Ghiti
@ 2025-06-06 16:34 ` Deepak Gupta
2025-06-07 17:54 ` Daniel Henrique Barboza
1 sibling, 0 replies; 5+ messages in thread
From: Deepak Gupta @ 2025-06-06 16:34 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li,
Daniel Henrique Barboza, Liu Zhiwei, qemu-riscv, qemu-devel
On Thu, Jun 5, 2025 at 7:21 AM Alexandre Ghiti <alexghiti@rivosinc.com> wrote:
>
> The Svrsw60t59b extension allows to free the PTE reserved bits 60 and 59
> for software to use.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
Reviewed-by: Deepak Gupta <debug@rivosinc.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] target: riscv: Add Svrsw60t59b extension support
2025-06-05 14:21 [PATCH] target: riscv: Add Svrsw60t59b extension support Alexandre Ghiti
2025-06-06 16:34 ` Deepak Gupta
@ 2025-06-07 17:54 ` Daniel Henrique Barboza
2025-06-25 7:36 ` Alexandre Ghiti
1 sibling, 1 reply; 5+ messages in thread
From: Daniel Henrique Barboza @ 2025-06-07 17:54 UTC (permalink / raw)
To: Alexandre Ghiti, Palmer Dabbelt, Alistair Francis, Bin Meng,
Weiwei Li, Liu Zhiwei, debug, qemu-riscv, qemu-devel
On 6/5/25 11:21 AM, Alexandre Ghiti wrote:
> The Svrsw60t59b extension allows to free the PTE reserved bits 60 and 59
> for software to use.
>
> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> ---
>
> Changes in v2:
> - Add support for IOMMU
> - Make svrsw60t59b depend on sv39 (deepak)
>
> Open question: svrsw60t59b in IOMMU should also depend on 64bit, but I
> did not find an easy to way in riscv_iommu_realize() to detect that, how
> should I do?
What controls the IOMMU behavior is the set of IOMMU capabilities that the driver
chooses to use. Other than that the device should be oblivious to the CPU word
size.
From what I see in this patch you did the right thing: you added a new capability
to be advertised to software and that's it. It's up to software to decide whether
it's going to use it or not. You can advertise a 64 bit only IOMMU capability running
in a 32 bit CPU and it's up to the OS to not use/ignore it. In fact we already do
that: satp_mode related caps (e.g. RISCV_IOMMU_CAP_SV32X4) are 32/64 bits exclusive
but are always advertised.
Now, Svrsw60t59b being a 32 bit only extension requires special handling in
riscv_init_max_cpu_extensions() because the 'max' CPU has a 32 bit variant and
enabled everything by default. You can use this diff:
diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
index f93cd53f37..96201e15c6 100644
--- a/target/riscv/tcg/tcg-cpu.c
+++ b/target/riscv/tcg/tcg-cpu.c
@@ -1612,6 +1612,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
if (env->misa_mxl != MXL_RV32) {
isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_zcf), false);
+ } else {
+ isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_svrsw60t59b), false);
}
/*
To fix this test break in 'make check-functional':
Command: /home/danielhb/work/qemu/build/qemu-system-riscv32 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -machine virt -chardev socket,id=console,fd=10 -serial chardev:console -cpu max -kernel /home/danielhb/.cache/qemu/download/872bc8f8e0d4661825d5f47f7bec64988e9d0a8bd5db8917d57e16f66d83b329 -append printk.time=0 root=/dev/vda console=ttyS0 -blockdev driver=raw,file.driver=file,file.filename=/home/danielhb/work/qemu/build/tests/functional/riscv32/test_riscv32_tuxrun.TuxRunRiscV32Test.test_riscv32_maxcpu/scratch/511ad34e63222db08d6c1da16fad224970de36517a784110956ba6a24a0ee5f6,node-name=hd0 -device virtio-blk-device,drive=hd0
Output: qemu-system-riscv32: svrsw60t59b is not supported on RV32 and MMU-less platforms
Thanks,
Daniel
>
> hw/riscv/riscv-iommu-bits.h | 1 +
> hw/riscv/riscv-iommu.c | 3 ++-
> target/riscv/cpu.c | 2 ++
> target/riscv/cpu_bits.h | 3 ++-
> target/riscv/cpu_cfg_fields.h.inc | 1 +
> target/riscv/cpu_helper.c | 3 ++-
> target/riscv/tcg/tcg-cpu.c | 6 ++++++
> 7 files changed, 16 insertions(+), 3 deletions(-)
>
> diff --git a/hw/riscv/riscv-iommu-bits.h b/hw/riscv/riscv-iommu-bits.h
> index 1017d73fc6..47fe01bee5 100644
> --- a/hw/riscv/riscv-iommu-bits.h
> +++ b/hw/riscv/riscv-iommu-bits.h
> @@ -79,6 +79,7 @@ struct riscv_iommu_pq_record {
> #define RISCV_IOMMU_CAP_SV39 BIT_ULL(9)
> #define RISCV_IOMMU_CAP_SV48 BIT_ULL(10)
> #define RISCV_IOMMU_CAP_SV57 BIT_ULL(11)
> +#define RISCV_IOMMU_CAP_SVRSW60T59B BIT_ULL(14)
> #define RISCV_IOMMU_CAP_SV32X4 BIT_ULL(16)
> #define RISCV_IOMMU_CAP_SV39X4 BIT_ULL(17)
> #define RISCV_IOMMU_CAP_SV48X4 BIT_ULL(18)
> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> index a877e5da84..36eda95a1c 100644
> --- a/hw/riscv/riscv-iommu.c
> +++ b/hw/riscv/riscv-iommu.c
> @@ -2355,7 +2355,8 @@ static void riscv_iommu_realize(DeviceState *dev, Error **errp)
> }
> if (s->enable_g_stage) {
> s->cap |= RISCV_IOMMU_CAP_SV32X4 | RISCV_IOMMU_CAP_SV39X4 |
> - RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4;
> + RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4 |
> + RISCV_IOMMU_CAP_SVRSW60T59B;
> }
>
> if (s->hpm_cntrs > 0) {
> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> index 629ac37501..13f1f56d95 100644
> --- a/target/riscv/cpu.c
> +++ b/target/riscv/cpu.c
> @@ -228,6 +228,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
> ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
> ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
> + ISA_EXT_DATA_ENTRY(svrsw60t59b, PRIV_VERSION_1_13_0, ext_svrsw60t59b),
> ISA_EXT_DATA_ENTRY(svukte, PRIV_VERSION_1_13_0, ext_svukte),
> ISA_EXT_DATA_ENTRY(svvptc, PRIV_VERSION_1_13_0, ext_svvptc),
> ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
> @@ -1282,6 +1283,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
> MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
> MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
> + MULTI_EXT_CFG_BOOL("svrsw60t59b", ext_svrsw60t59b, false),
> MULTI_EXT_CFG_BOOL("svvptc", ext_svvptc, true),
>
> MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> index a30317c617..51eb7114da 100644
> --- a/target/riscv/cpu_bits.h
> +++ b/target/riscv/cpu_bits.h
> @@ -675,7 +675,8 @@ typedef enum {
> #define PTE_SOFT 0x300 /* Reserved for Software */
> #define PTE_PBMT 0x6000000000000000ULL /* Page-based memory types */
> #define PTE_N 0x8000000000000000ULL /* NAPOT translation */
> -#define PTE_RESERVED 0x1FC0000000000000ULL /* Reserved bits */
> +#define PTE_RESERVED(svrsw60t59b) \
> + (svrsw60t59b ? 0x07C0000000000000ULL : 0x1FC0000000000000ULL) /* Reserved bits */
> #define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>
> /* Page table PPN shift amount */
> diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc
> index 59f134a419..ab61c1ccf2 100644
> --- a/target/riscv/cpu_cfg_fields.h.inc
> +++ b/target/riscv/cpu_cfg_fields.h.inc
> @@ -57,6 +57,7 @@ BOOL_FIELD(ext_svadu)
> BOOL_FIELD(ext_svinval)
> BOOL_FIELD(ext_svnapot)
> BOOL_FIELD(ext_svpbmt)
> +BOOL_FIELD(ext_svrsw60t59b)
> BOOL_FIELD(ext_svvptc)
> BOOL_FIELD(ext_svukte)
> BOOL_FIELD(ext_zdinx)
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 2ed69d7c2d..3479a62cc7 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -1309,6 +1309,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> bool svade = riscv_cpu_cfg(env)->ext_svade;
> bool svadu = riscv_cpu_cfg(env)->ext_svadu;
> bool adue = svadu ? env->menvcfg & MENVCFG_ADUE : !svade;
> + bool svrsw60t59b = riscv_cpu_cfg(env)->ext_svrsw60t59b;
>
> if (first_stage && two_stage && env->virt_enabled) {
> pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
> @@ -1376,7 +1377,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> if (riscv_cpu_sxl(env) == MXL_RV32) {
> ppn = pte >> PTE_PPN_SHIFT;
> } else {
> - if (pte & PTE_RESERVED) {
> + if (pte & PTE_RESERVED(svrsw60t59b)) {
> qemu_log_mask(LOG_GUEST_ERROR, "%s: reserved bits set in PTE: "
> "addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx "\n",
> __func__, pte_addr, pte);
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index 305912b8dd..886006abc3 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -804,6 +804,12 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> cpu->cfg.ext_ssctr = false;
> }
>
> + if (cpu->cfg.ext_svrsw60t59b &&
> + (!cpu->cfg.mmu || mcc->def->misa_mxl_max == MXL_RV32)) {
> + error_setg(errp, "svrsw60t59b is not supported on RV32 and MMU-less platforms");
> + return;
> + }
> +
> /*
> * Disable isa extensions based on priv spec after we
> * validated and set everything we need.
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] target: riscv: Add Svrsw60t59b extension support
2025-06-07 17:54 ` Daniel Henrique Barboza
@ 2025-06-25 7:36 ` Alexandre Ghiti
2025-06-29 9:14 ` Daniel Henrique Barboza
0 siblings, 1 reply; 5+ messages in thread
From: Alexandre Ghiti @ 2025-06-25 7:36 UTC (permalink / raw)
To: Daniel Henrique Barboza
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei,
debug, qemu-riscv, qemu-devel
Hi Daniel,
On Sat, Jun 7, 2025 at 7:54 PM Daniel Henrique Barboza
<dbarboza@ventanamicro.com> wrote:
>
>
>
> On 6/5/25 11:21 AM, Alexandre Ghiti wrote:
> > The Svrsw60t59b extension allows to free the PTE reserved bits 60 and 59
> > for software to use.
> >
> > Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
> > ---
> >
> > Changes in v2:
> > - Add support for IOMMU
> > - Make svrsw60t59b depend on sv39 (deepak)
> >
> > Open question: svrsw60t59b in IOMMU should also depend on 64bit, but I
> > did not find an easy to way in riscv_iommu_realize() to detect that, how
> > should I do?
>
>
> What controls the IOMMU behavior is the set of IOMMU capabilities that the driver
> chooses to use. Other than that the device should be oblivious to the CPU word
> size.
>
> From what I see in this patch you did the right thing: you added a new capability
> to be advertised to software and that's it. It's up to software to decide whether
> it's going to use it or not. You can advertise a 64 bit only IOMMU capability running
> in a 32 bit CPU and it's up to the OS to not use/ignore it. In fact we already do
> that: satp_mode related caps (e.g. RISCV_IOMMU_CAP_SV32X4) are 32/64 bits exclusive
> but are always advertised.
>
>
>
> Now, Svrsw60t59b being a 32 bit only extension requires special handling in
> riscv_init_max_cpu_extensions() because the 'max' CPU has a 32 bit variant and
> enabled everything by default. You can use this diff:
>
>
> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> index f93cd53f37..96201e15c6 100644
> --- a/target/riscv/tcg/tcg-cpu.c
> +++ b/target/riscv/tcg/tcg-cpu.c
> @@ -1612,6 +1612,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>
> if (env->misa_mxl != MXL_RV32) {
> isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_zcf), false);
> + } else {
> + isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_svrsw60t59b), false);
> }
>
> /*
>
>
> To fix this test break in 'make check-functional':
>
> Command: /home/danielhb/work/qemu/build/qemu-system-riscv32 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -machine virt -chardev socket,id=console,fd=10 -serial chardev:console -cpu max -kernel /home/danielhb/.cache/qemu/download/872bc8f8e0d4661825d5f47f7bec64988e9d0a8bd5db8917d57e16f66d83b329 -append printk.time=0 root=/dev/vda console=ttyS0 -blockdev driver=raw,file.driver=file,file.filename=/home/danielhb/work/qemu/build/tests/functional/riscv32/test_riscv32_tuxrun.TuxRunRiscV32Test.test_riscv32_maxcpu/scratch/511ad34e63222db08d6c1da16fad224970de36517a784110956ba6a24a0ee5f6,node-name=hd0 -device virtio-blk-device,drive=hd0
> Output: qemu-system-riscv32: svrsw60t59b is not supported on RV32 and MMU-less platforms
Sorry for the late answer and thanks for the fix ^! I have just sent the v2.
Thanks,
Alex
>
>
> Thanks,
>
> Daniel
>
>
> >
> > hw/riscv/riscv-iommu-bits.h | 1 +
> > hw/riscv/riscv-iommu.c | 3 ++-
> > target/riscv/cpu.c | 2 ++
> > target/riscv/cpu_bits.h | 3 ++-
> > target/riscv/cpu_cfg_fields.h.inc | 1 +
> > target/riscv/cpu_helper.c | 3 ++-
> > target/riscv/tcg/tcg-cpu.c | 6 ++++++
> > 7 files changed, 16 insertions(+), 3 deletions(-)
> >
> > diff --git a/hw/riscv/riscv-iommu-bits.h b/hw/riscv/riscv-iommu-bits.h
> > index 1017d73fc6..47fe01bee5 100644
> > --- a/hw/riscv/riscv-iommu-bits.h
> > +++ b/hw/riscv/riscv-iommu-bits.h
> > @@ -79,6 +79,7 @@ struct riscv_iommu_pq_record {
> > #define RISCV_IOMMU_CAP_SV39 BIT_ULL(9)
> > #define RISCV_IOMMU_CAP_SV48 BIT_ULL(10)
> > #define RISCV_IOMMU_CAP_SV57 BIT_ULL(11)
> > +#define RISCV_IOMMU_CAP_SVRSW60T59B BIT_ULL(14)
> > #define RISCV_IOMMU_CAP_SV32X4 BIT_ULL(16)
> > #define RISCV_IOMMU_CAP_SV39X4 BIT_ULL(17)
> > #define RISCV_IOMMU_CAP_SV48X4 BIT_ULL(18)
> > diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
> > index a877e5da84..36eda95a1c 100644
> > --- a/hw/riscv/riscv-iommu.c
> > +++ b/hw/riscv/riscv-iommu.c
> > @@ -2355,7 +2355,8 @@ static void riscv_iommu_realize(DeviceState *dev, Error **errp)
> > }
> > if (s->enable_g_stage) {
> > s->cap |= RISCV_IOMMU_CAP_SV32X4 | RISCV_IOMMU_CAP_SV39X4 |
> > - RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4;
> > + RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4 |
> > + RISCV_IOMMU_CAP_SVRSW60T59B;
> > }
> >
> > if (s->hpm_cntrs > 0) {
> > diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
> > index 629ac37501..13f1f56d95 100644
> > --- a/target/riscv/cpu.c
> > +++ b/target/riscv/cpu.c
> > @@ -228,6 +228,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
> > ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
> > ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
> > ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
> > + ISA_EXT_DATA_ENTRY(svrsw60t59b, PRIV_VERSION_1_13_0, ext_svrsw60t59b),
> > ISA_EXT_DATA_ENTRY(svukte, PRIV_VERSION_1_13_0, ext_svukte),
> > ISA_EXT_DATA_ENTRY(svvptc, PRIV_VERSION_1_13_0, ext_svvptc),
> > ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
> > @@ -1282,6 +1283,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
> > MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
> > MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
> > MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
> > + MULTI_EXT_CFG_BOOL("svrsw60t59b", ext_svrsw60t59b, false),
> > MULTI_EXT_CFG_BOOL("svvptc", ext_svvptc, true),
> >
> > MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
> > diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
> > index a30317c617..51eb7114da 100644
> > --- a/target/riscv/cpu_bits.h
> > +++ b/target/riscv/cpu_bits.h
> > @@ -675,7 +675,8 @@ typedef enum {
> > #define PTE_SOFT 0x300 /* Reserved for Software */
> > #define PTE_PBMT 0x6000000000000000ULL /* Page-based memory types */
> > #define PTE_N 0x8000000000000000ULL /* NAPOT translation */
> > -#define PTE_RESERVED 0x1FC0000000000000ULL /* Reserved bits */
> > +#define PTE_RESERVED(svrsw60t59b) \
> > + (svrsw60t59b ? 0x07C0000000000000ULL : 0x1FC0000000000000ULL) /* Reserved bits */
> > #define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
> >
> > /* Page table PPN shift amount */
> > diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc
> > index 59f134a419..ab61c1ccf2 100644
> > --- a/target/riscv/cpu_cfg_fields.h.inc
> > +++ b/target/riscv/cpu_cfg_fields.h.inc
> > @@ -57,6 +57,7 @@ BOOL_FIELD(ext_svadu)
> > BOOL_FIELD(ext_svinval)
> > BOOL_FIELD(ext_svnapot)
> > BOOL_FIELD(ext_svpbmt)
> > +BOOL_FIELD(ext_svrsw60t59b)
> > BOOL_FIELD(ext_svvptc)
> > BOOL_FIELD(ext_svukte)
> > BOOL_FIELD(ext_zdinx)
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 2ed69d7c2d..3479a62cc7 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -1309,6 +1309,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> > bool svade = riscv_cpu_cfg(env)->ext_svade;
> > bool svadu = riscv_cpu_cfg(env)->ext_svadu;
> > bool adue = svadu ? env->menvcfg & MENVCFG_ADUE : !svade;
> > + bool svrsw60t59b = riscv_cpu_cfg(env)->ext_svrsw60t59b;
> >
> > if (first_stage && two_stage && env->virt_enabled) {
> > pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
> > @@ -1376,7 +1377,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> > if (riscv_cpu_sxl(env) == MXL_RV32) {
> > ppn = pte >> PTE_PPN_SHIFT;
> > } else {
> > - if (pte & PTE_RESERVED) {
> > + if (pte & PTE_RESERVED(svrsw60t59b)) {
> > qemu_log_mask(LOG_GUEST_ERROR, "%s: reserved bits set in PTE: "
> > "addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx "\n",
> > __func__, pte_addr, pte);
> > diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
> > index 305912b8dd..886006abc3 100644
> > --- a/target/riscv/tcg/tcg-cpu.c
> > +++ b/target/riscv/tcg/tcg-cpu.c
> > @@ -804,6 +804,12 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
> > cpu->cfg.ext_ssctr = false;
> > }
> >
> > + if (cpu->cfg.ext_svrsw60t59b &&
> > + (!cpu->cfg.mmu || mcc->def->misa_mxl_max == MXL_RV32)) {
> > + error_setg(errp, "svrsw60t59b is not supported on RV32 and MMU-less platforms");
> > + return;
> > + }
> > +
> > /*
> > * Disable isa extensions based on priv spec after we
> > * validated and set everything we need.
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] target: riscv: Add Svrsw60t59b extension support
2025-06-25 7:36 ` Alexandre Ghiti
@ 2025-06-29 9:14 ` Daniel Henrique Barboza
0 siblings, 0 replies; 5+ messages in thread
From: Daniel Henrique Barboza @ 2025-06-29 9:14 UTC (permalink / raw)
To: Alexandre Ghiti
Cc: Palmer Dabbelt, Alistair Francis, Bin Meng, Weiwei Li, Liu Zhiwei,
debug, qemu-riscv, qemu-devel
On 6/25/25 4:36 AM, Alexandre Ghiti wrote:
> Hi Daniel,
>
> On Sat, Jun 7, 2025 at 7:54 PM Daniel Henrique Barboza
> <dbarboza@ventanamicro.com> wrote:
>>
>>
>>
>> On 6/5/25 11:21 AM, Alexandre Ghiti wrote:
>>> The Svrsw60t59b extension allows to free the PTE reserved bits 60 and 59
>>> for software to use.
>>>
>>> Signed-off-by: Alexandre Ghiti <alexghiti@rivosinc.com>
>>> ---
>>>
>>> Changes in v2:
>>> - Add support for IOMMU
>>> - Make svrsw60t59b depend on sv39 (deepak)
>>>
>>> Open question: svrsw60t59b in IOMMU should also depend on 64bit, but I
>>> did not find an easy to way in riscv_iommu_realize() to detect that, how
>>> should I do?
>>
>>
>> What controls the IOMMU behavior is the set of IOMMU capabilities that the driver
>> chooses to use. Other than that the device should be oblivious to the CPU word
>> size.
>>
>> From what I see in this patch you did the right thing: you added a new capability
>> to be advertised to software and that's it. It's up to software to decide whether
>> it's going to use it or not. You can advertise a 64 bit only IOMMU capability running
>> in a 32 bit CPU and it's up to the OS to not use/ignore it. In fact we already do
>> that: satp_mode related caps (e.g. RISCV_IOMMU_CAP_SV32X4) are 32/64 bits exclusive
>> but are always advertised.
>>
>>
>>
>> Now, Svrsw60t59b being a 32 bit only extension requires special handling in
>> riscv_init_max_cpu_extensions() because the 'max' CPU has a 32 bit variant and
>> enabled everything by default. You can use this diff:
>>
>>
>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>> index f93cd53f37..96201e15c6 100644
>> --- a/target/riscv/tcg/tcg-cpu.c
>> +++ b/target/riscv/tcg/tcg-cpu.c
>> @@ -1612,6 +1612,8 @@ static void riscv_init_max_cpu_extensions(Object *obj)
>>
>> if (env->misa_mxl != MXL_RV32) {
>> isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_zcf), false);
>> + } else {
>> + isa_ext_update_enabled(cpu, CPU_CFG_OFFSET(ext_svrsw60t59b), false);
>> }
>>
>> /*
>>
>>
>> To fix this test break in 'make check-functional':
>>
>> Command: /home/danielhb/work/qemu/build/qemu-system-riscv32 -display none -vga none -chardev socket,id=mon,fd=5 -mon chardev=mon,mode=control -machine virt -chardev socket,id=console,fd=10 -serial chardev:console -cpu max -kernel /home/danielhb/.cache/qemu/download/872bc8f8e0d4661825d5f47f7bec64988e9d0a8bd5db8917d57e16f66d83b329 -append printk.time=0 root=/dev/vda console=ttyS0 -blockdev driver=raw,file.driver=file,file.filename=/home/danielhb/work/qemu/build/tests/functional/riscv32/test_riscv32_tuxrun.TuxRunRiscV32Test.test_riscv32_maxcpu/scratch/511ad34e63222db08d6c1da16fad224970de36517a784110956ba6a24a0ee5f6,node-name=hd0 -device virtio-blk-device,drive=hd0
>> Output: qemu-system-riscv32: svrsw60t59b is not supported on RV32 and MMU-less platforms
>
> Sorry for the late answer and thanks for the fix ^! I have just sent the v2.
I'm not seeing it in my box and in qemu-devel and in qemu-riscv. Perhaps a
re-sending might be in order. Thanks,
Daniel
>
> Thanks,
>
> Alex
>
>>
>>
>> Thanks,
>>
>> Daniel
>>
>>
>>>
>>> hw/riscv/riscv-iommu-bits.h | 1 +
>>> hw/riscv/riscv-iommu.c | 3 ++-
>>> target/riscv/cpu.c | 2 ++
>>> target/riscv/cpu_bits.h | 3 ++-
>>> target/riscv/cpu_cfg_fields.h.inc | 1 +
>>> target/riscv/cpu_helper.c | 3 ++-
>>> target/riscv/tcg/tcg-cpu.c | 6 ++++++
>>> 7 files changed, 16 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/hw/riscv/riscv-iommu-bits.h b/hw/riscv/riscv-iommu-bits.h
>>> index 1017d73fc6..47fe01bee5 100644
>>> --- a/hw/riscv/riscv-iommu-bits.h
>>> +++ b/hw/riscv/riscv-iommu-bits.h
>>> @@ -79,6 +79,7 @@ struct riscv_iommu_pq_record {
>>> #define RISCV_IOMMU_CAP_SV39 BIT_ULL(9)
>>> #define RISCV_IOMMU_CAP_SV48 BIT_ULL(10)
>>> #define RISCV_IOMMU_CAP_SV57 BIT_ULL(11)
>>> +#define RISCV_IOMMU_CAP_SVRSW60T59B BIT_ULL(14)
>>> #define RISCV_IOMMU_CAP_SV32X4 BIT_ULL(16)
>>> #define RISCV_IOMMU_CAP_SV39X4 BIT_ULL(17)
>>> #define RISCV_IOMMU_CAP_SV48X4 BIT_ULL(18)
>>> diff --git a/hw/riscv/riscv-iommu.c b/hw/riscv/riscv-iommu.c
>>> index a877e5da84..36eda95a1c 100644
>>> --- a/hw/riscv/riscv-iommu.c
>>> +++ b/hw/riscv/riscv-iommu.c
>>> @@ -2355,7 +2355,8 @@ static void riscv_iommu_realize(DeviceState *dev, Error **errp)
>>> }
>>> if (s->enable_g_stage) {
>>> s->cap |= RISCV_IOMMU_CAP_SV32X4 | RISCV_IOMMU_CAP_SV39X4 |
>>> - RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4;
>>> + RISCV_IOMMU_CAP_SV48X4 | RISCV_IOMMU_CAP_SV57X4 |
>>> + RISCV_IOMMU_CAP_SVRSW60T59B;
>>> }
>>>
>>> if (s->hpm_cntrs > 0) {
>>> diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
>>> index 629ac37501..13f1f56d95 100644
>>> --- a/target/riscv/cpu.c
>>> +++ b/target/riscv/cpu.c
>>> @@ -228,6 +228,7 @@ const RISCVIsaExtData isa_edata_arr[] = {
>>> ISA_EXT_DATA_ENTRY(svinval, PRIV_VERSION_1_12_0, ext_svinval),
>>> ISA_EXT_DATA_ENTRY(svnapot, PRIV_VERSION_1_12_0, ext_svnapot),
>>> ISA_EXT_DATA_ENTRY(svpbmt, PRIV_VERSION_1_12_0, ext_svpbmt),
>>> + ISA_EXT_DATA_ENTRY(svrsw60t59b, PRIV_VERSION_1_13_0, ext_svrsw60t59b),
>>> ISA_EXT_DATA_ENTRY(svukte, PRIV_VERSION_1_13_0, ext_svukte),
>>> ISA_EXT_DATA_ENTRY(svvptc, PRIV_VERSION_1_13_0, ext_svvptc),
>>> ISA_EXT_DATA_ENTRY(xtheadba, PRIV_VERSION_1_11_0, ext_xtheadba),
>>> @@ -1282,6 +1283,7 @@ const RISCVCPUMultiExtConfig riscv_cpu_extensions[] = {
>>> MULTI_EXT_CFG_BOOL("svinval", ext_svinval, false),
>>> MULTI_EXT_CFG_BOOL("svnapot", ext_svnapot, false),
>>> MULTI_EXT_CFG_BOOL("svpbmt", ext_svpbmt, false),
>>> + MULTI_EXT_CFG_BOOL("svrsw60t59b", ext_svrsw60t59b, false),
>>> MULTI_EXT_CFG_BOOL("svvptc", ext_svvptc, true),
>>>
>>> MULTI_EXT_CFG_BOOL("zicntr", ext_zicntr, true),
>>> diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
>>> index a30317c617..51eb7114da 100644
>>> --- a/target/riscv/cpu_bits.h
>>> +++ b/target/riscv/cpu_bits.h
>>> @@ -675,7 +675,8 @@ typedef enum {
>>> #define PTE_SOFT 0x300 /* Reserved for Software */
>>> #define PTE_PBMT 0x6000000000000000ULL /* Page-based memory types */
>>> #define PTE_N 0x8000000000000000ULL /* NAPOT translation */
>>> -#define PTE_RESERVED 0x1FC0000000000000ULL /* Reserved bits */
>>> +#define PTE_RESERVED(svrsw60t59b) \
>>> + (svrsw60t59b ? 0x07C0000000000000ULL : 0x1FC0000000000000ULL) /* Reserved bits */
>>> #define PTE_ATTR (PTE_N | PTE_PBMT) /* All attributes bits */
>>>
>>> /* Page table PPN shift amount */
>>> diff --git a/target/riscv/cpu_cfg_fields.h.inc b/target/riscv/cpu_cfg_fields.h.inc
>>> index 59f134a419..ab61c1ccf2 100644
>>> --- a/target/riscv/cpu_cfg_fields.h.inc
>>> +++ b/target/riscv/cpu_cfg_fields.h.inc
>>> @@ -57,6 +57,7 @@ BOOL_FIELD(ext_svadu)
>>> BOOL_FIELD(ext_svinval)
>>> BOOL_FIELD(ext_svnapot)
>>> BOOL_FIELD(ext_svpbmt)
>>> +BOOL_FIELD(ext_svrsw60t59b)
>>> BOOL_FIELD(ext_svvptc)
>>> BOOL_FIELD(ext_svukte)
>>> BOOL_FIELD(ext_zdinx)
>>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
>>> index 2ed69d7c2d..3479a62cc7 100644
>>> --- a/target/riscv/cpu_helper.c
>>> +++ b/target/riscv/cpu_helper.c
>>> @@ -1309,6 +1309,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>>> bool svade = riscv_cpu_cfg(env)->ext_svade;
>>> bool svadu = riscv_cpu_cfg(env)->ext_svadu;
>>> bool adue = svadu ? env->menvcfg & MENVCFG_ADUE : !svade;
>>> + bool svrsw60t59b = riscv_cpu_cfg(env)->ext_svrsw60t59b;
>>>
>>> if (first_stage && two_stage && env->virt_enabled) {
>>> pbmte = pbmte && (env->henvcfg & HENVCFG_PBMTE);
>>> @@ -1376,7 +1377,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
>>> if (riscv_cpu_sxl(env) == MXL_RV32) {
>>> ppn = pte >> PTE_PPN_SHIFT;
>>> } else {
>>> - if (pte & PTE_RESERVED) {
>>> + if (pte & PTE_RESERVED(svrsw60t59b)) {
>>> qemu_log_mask(LOG_GUEST_ERROR, "%s: reserved bits set in PTE: "
>>> "addr: 0x%" HWADDR_PRIx " pte: 0x" TARGET_FMT_lx "\n",
>>> __func__, pte_addr, pte);
>>> diff --git a/target/riscv/tcg/tcg-cpu.c b/target/riscv/tcg/tcg-cpu.c
>>> index 305912b8dd..886006abc3 100644
>>> --- a/target/riscv/tcg/tcg-cpu.c
>>> +++ b/target/riscv/tcg/tcg-cpu.c
>>> @@ -804,6 +804,12 @@ void riscv_cpu_validate_set_extensions(RISCVCPU *cpu, Error **errp)
>>> cpu->cfg.ext_ssctr = false;
>>> }
>>>
>>> + if (cpu->cfg.ext_svrsw60t59b &&
>>> + (!cpu->cfg.mmu || mcc->def->misa_mxl_max == MXL_RV32)) {
>>> + error_setg(errp, "svrsw60t59b is not supported on RV32 and MMU-less platforms");
>>> + return;
>>> + }
>>> +
>>> /*
>>> * Disable isa extensions based on priv spec after we
>>> * validated and set everything we need.
>>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-06-29 9:15 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-06-05 14:21 [PATCH] target: riscv: Add Svrsw60t59b extension support Alexandre Ghiti
2025-06-06 16:34 ` Deepak Gupta
2025-06-07 17:54 ` Daniel Henrique Barboza
2025-06-25 7:36 ` Alexandre Ghiti
2025-06-29 9:14 ` Daniel Henrique Barboza
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).