Re: [PATCH] target: riscv: Add Svrsw60t59b extension support

[Alexandre Ghiti <alexghiti@rivosinc.com>]

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.
>