diff mbox series

[v2,2/5,RISCV_PM] Support CSRs required for RISC-V PM extension except for ones in hypervisor mode

Message ID 20201015152139.28903-2-space.monkey.delivers@gmail.com
State New
Headers show
Series [v2,1/5,RISCV_PM] Add J-extension into RISC-V | expand

Commit Message

Alexey Baturo Oct. 15, 2020, 3:21 p.m. UTC
Signed-off-by: Alexey Baturo <space.monkey.delivers@gmail.com>
---
 target/riscv/cpu.c      |   1 +
 target/riscv/cpu.h      |  11 ++
 target/riscv/cpu_bits.h |  66 ++++++++++
 target/riscv/csr.c      | 277 ++++++++++++++++++++++++++++++++++++++++
 4 files changed, 355 insertions(+)

Comments

Richard Henderson Oct. 15, 2020, 4:48 p.m. UTC | #1
On 10/15/20 8:21 AM, Alexey Baturo wrote:
> +/* Functions to access Pointer Masking feature registers 
> + * We have to check if current priv lvl could modify
> + * csr in given mode
> + */
> +static int check_pm_current_disabled(CPURISCVState *env, int csrno)
> +{
> +    /* m-mode is always allowed to modify registers, so allow */
> +    if (env->priv == PRV_M) {
> +        return 0;
> +    }
> +    int csr_priv = get_field(csrno, 0xC00);
> +    /* If priv lvls differ that means we're accessing csr from higher priv lvl, so allow */
> +    if (env->priv != csr_priv) {
> +        return 0;
> +    }
> +    int cur_bit_pos = (env->priv == PRV_U) ? U_PM_CURRENT :
> +                      (env->priv == PRV_S) ? S_PM_CURRENT : -1;
> +    assert(cur_bit_pos != -1);

This is a bit clumsy.  I suggest

    int cur_bit_pos;
    switch (env->priv) {
    case PRV_M:
        return 0;
    case PRV_S:
        cur_bit_pos = S_PM_CURRENT;
        break;
    case PRV_U:
        cur_bit_pos = U_PM_CURRENT;
        break;
    default:
        g_assert_not_reached();
    }

> +static int read_mmte(CPURISCVState *env, int csrno, target_ulong *val)
> +{
> +#if !defined(CONFIG_USER_ONLY)
> +    if (!riscv_has_ext(env, RVJ)) {
> +        *val = 0;
> +        return 0;
> +    }
> +#endif

This ifdef is wrong.  CONFIG_USER_ONLY doesn't have anything to do with what
features the cpu supports.  Nor can it be correct.  If you try to read this on
current hardware, without J, then you get an exception not zero.

I would have expected this check would actually go into the csr predicate function.

> +    *val = env->mmte & MMTE_MASK;

Surely you don't need MMTE_MASK here because you used it when writing.

That said, don't you also need to mask vs the current priv level?  There's
language in your doc that says "XS bits are available in..." and "PM bits are
available in...".

I'll also note that it says "by default only higher priv code can set the value
for PM bits", while surely it's a security bug for lower priv code to read
anything related to a higher priv.


> +    target_ulong wpri_val = val & SMTE_MASK;
> +    assert(val == wpri_val);

Incorrect assert.  This value is under guest control.  Do not crash qemu
because the guest is doing silly things.  Raise an exception if you like, and
perhaps qemu_log_mask with LOG_GUEST_ERROR.

> +    if (check_pm_current_disabled(env, csrno))
> +        return 0;

Missing braces.


r~
Alexey Baturo Oct. 15, 2020, 5:28 p.m. UTC | #2
Richard, again thanks for the review!

> This is a bit clumsy.  I suggest
Sure, will fix.

> If you try to read this on current hardware, without J, then you get an
exception not zero.
If I get the spec right, the idea is to read 0 from PM CSRs even if this
extension is not present.

>I would have expected this check would actually go into the csr predicate
function.
If I understand your proposal correctly, in this case I'd have to introduce
3 new functions for S/M/U PM CSRs as a predicate. I'm okay with that if you
suggest so

>Surely you don't need MMTE_MASK here because you used it when writing.
>That said, don't you also need to mask vs the current priv level?
I suppose that applying these masks helps to prepare the correct value for
the given priv level. So if I understand correctly if you're in *M-mode*
and try to read *UMTE*, you'll get only *U.Enable* and *U.Current*, while
if you'd try to read *SMTE*, you'll also get *XS* bits, *S.Enable* and
*S.Current*.

> it's a security bug for lower priv code to read anything related to a
higher priv.
Could you please elaborate a bit more here? I don't see how this scenario
is valid: in *U-mode *you're supposed to be able to read only *U* *registers,
while *M*/*S *mode could allow *U-mode *to write to its *U** registers.
As for *S-mode*, I believe it's allowed to write to any *U** registers and
it's available to write to *S** registers if *S.Current* is set.

> Do not crash qemu because the guest is doing silly things
Sure, you're right. Will fix.

>Raise an exception if you like, and perhaps qemu_log_mask with
LOG_GUEST_ERROR
I think raising exception here might be too much, but logging the error is
a great idea, thanks!

Thanks!

чт, 15 окт. 2020 г. в 19:48, Richard Henderson <richard.henderson@linaro.org
>:

> On 10/15/20 8:21 AM, Alexey Baturo wrote:
> > +/* Functions to access Pointer Masking feature registers
> > + * We have to check if current priv lvl could modify
> > + * csr in given mode
> > + */
> > +static int check_pm_current_disabled(CPURISCVState *env, int csrno)
> > +{
> > +    /* m-mode is always allowed to modify registers, so allow */
> > +    if (env->priv == PRV_M) {
> > +        return 0;
> > +    }
> > +    int csr_priv = get_field(csrno, 0xC00);
> > +    /* If priv lvls differ that means we're accessing csr from higher
> priv lvl, so allow */
> > +    if (env->priv != csr_priv) {
> > +        return 0;
> > +    }
> > +    int cur_bit_pos = (env->priv == PRV_U) ? U_PM_CURRENT :
> > +                      (env->priv == PRV_S) ? S_PM_CURRENT : -1;
> > +    assert(cur_bit_pos != -1);
>
> This is a bit clumsy.  I suggest
>
>     int cur_bit_pos;
>     switch (env->priv) {
>     case PRV_M:
>         return 0;
>     case PRV_S:
>         cur_bit_pos = S_PM_CURRENT;
>         break;
>     case PRV_U:
>         cur_bit_pos = U_PM_CURRENT;
>         break;
>     default:
>         g_assert_not_reached();
>     }
>
> > +static int read_mmte(CPURISCVState *env, int csrno, target_ulong *val)
> > +{
> > +#if !defined(CONFIG_USER_ONLY)
> > +    if (!riscv_has_ext(env, RVJ)) {
> > +        *val = 0;
> > +        return 0;
> > +    }
> > +#endif
>
> This ifdef is wrong.  CONFIG_USER_ONLY doesn't have anything to do with
> what
> features the cpu supports.  Nor can it be correct.  If you try to read
> this on
> current hardware, without J, then you get an exception not zero.
>
> I would have expected this check would actually go into the csr predicate
> function.
>
> > +    *val = env->mmte & MMTE_MASK;
>
> Surely you don't need MMTE_MASK here because you used it when writing.
>
> That said, don't you also need to mask vs the current priv level?  There's
> language in your doc that says "XS bits are available in..." and "PM bits
> are
> available in...".
>
> I'll also note that it says "by default only higher priv code can set the
> value
> for PM bits", while surely it's a security bug for lower priv code to read
> anything related to a higher priv.
>
>
> > +    target_ulong wpri_val = val & SMTE_MASK;
> > +    assert(val == wpri_val);
>
> Incorrect assert.  This value is under guest control.  Do not crash qemu
> because the guest is doing silly things.  Raise an exception if you like,
> and
> perhaps qemu_log_mask with LOG_GUEST_ERROR.
>
> > +    if (check_pm_current_disabled(env, csrno))
> > +        return 0;
>
> Missing braces.
>
>
> r~
>
Alexey Baturo Oct. 15, 2020, 6:05 p.m. UTC | #3
> Surely you don't need MMTE_MASK here because you used it when writing.
> That said, don't you also need to mask vs the current priv level?
> while surely it's a security bug for lower priv code to read anything
related to a higher priv
Now I think I get what you've meant:
The spec states that for lower priv level you get WPRI fields that contain
bits for higher priv levels.
Applying those masks while reading CSRs on the one hand solves this
security breach, but on the other - violates the spec. Let me raise this
question at the upcoming J WG meeting.
Meanwhile, do you think applying **MTE *masks while reading CSR values is a
good solution for now?

Thanks again!

чт, 15 окт. 2020 г. в 20:28, Alexey Baturo <baturo.alexey@gmail.com>:

> Richard, again thanks for the review!
>
> > This is a bit clumsy.  I suggest
> Sure, will fix.
>
> > If you try to read this on current hardware, without J, then you get an
> exception not zero.
> If I get the spec right, the idea is to read 0 from PM CSRs even if this
> extension is not present.
>
> >I would have expected this check would actually go into the csr predicate
> function.
> If I understand your proposal correctly, in this case I'd have to
> introduce 3 new functions for S/M/U PM CSRs as a predicate. I'm okay with
> that if you suggest so
>
> >Surely you don't need MMTE_MASK here because you used it when writing.
> >That said, don't you also need to mask vs the current priv level?
> I suppose that applying these masks helps to prepare the correct value for
> the given priv level. So if I understand correctly if you're in *M-mode*
> and try to read *UMTE*, you'll get only *U.Enable* and *U.Current*, while
> if you'd try to read *SMTE*, you'll also get *XS* bits, *S.Enable* and
> *S.Current*.
>
> > it's a security bug for lower priv code to read anything related to a
> higher priv.
> Could you please elaborate a bit more here? I don't see how this scenario
> is valid: in *U-mode *you're supposed to be able to read only *U* *registers,
> while *M*/*S *mode could allow *U-mode *to write to its *U** registers.
> As for *S-mode*, I believe it's allowed to write to any *U** registers
> and it's available to write to *S** registers if *S.Current* is set.
>
> > Do not crash qemu because the guest is doing silly things
> Sure, you're right. Will fix.
>
> >Raise an exception if you like, and perhaps qemu_log_mask with
> LOG_GUEST_ERROR
> I think raising exception here might be too much, but logging the error is
> a great idea, thanks!
>
> Thanks!
>
> чт, 15 окт. 2020 г. в 19:48, Richard Henderson <
> richard.henderson@linaro.org>:
>
>> On 10/15/20 8:21 AM, Alexey Baturo wrote:
>> > +/* Functions to access Pointer Masking feature registers
>> > + * We have to check if current priv lvl could modify
>> > + * csr in given mode
>> > + */
>> > +static int check_pm_current_disabled(CPURISCVState *env, int csrno)
>> > +{
>> > +    /* m-mode is always allowed to modify registers, so allow */
>> > +    if (env->priv == PRV_M) {
>> > +        return 0;
>> > +    }
>> > +    int csr_priv = get_field(csrno, 0xC00);
>> > +    /* If priv lvls differ that means we're accessing csr from higher
>> priv lvl, so allow */
>> > +    if (env->priv != csr_priv) {
>> > +        return 0;
>> > +    }
>> > +    int cur_bit_pos = (env->priv == PRV_U) ? U_PM_CURRENT :
>> > +                      (env->priv == PRV_S) ? S_PM_CURRENT : -1;
>> > +    assert(cur_bit_pos != -1);
>>
>> This is a bit clumsy.  I suggest
>>
>>     int cur_bit_pos;
>>     switch (env->priv) {
>>     case PRV_M:
>>         return 0;
>>     case PRV_S:
>>         cur_bit_pos = S_PM_CURRENT;
>>         break;
>>     case PRV_U:
>>         cur_bit_pos = U_PM_CURRENT;
>>         break;
>>     default:
>>         g_assert_not_reached();
>>     }
>>
>> > +static int read_mmte(CPURISCVState *env, int csrno, target_ulong *val)
>> > +{
>> > +#if !defined(CONFIG_USER_ONLY)
>> > +    if (!riscv_has_ext(env, RVJ)) {
>> > +        *val = 0;
>> > +        return 0;
>> > +    }
>> > +#endif
>>
>> This ifdef is wrong.  CONFIG_USER_ONLY doesn't have anything to do with
>> what
>> features the cpu supports.  Nor can it be correct.  If you try to read
>> this on
>> current hardware, without J, then you get an exception not zero.
>>
>> I would have expected this check would actually go into the csr predicate
>> function.
>>
>> > +    *val = env->mmte & MMTE_MASK;
>>
>> Surely you don't need MMTE_MASK here because you used it when writing.
>>
>> That said, don't you also need to mask vs the current priv level?  There's
>> language in your doc that says "XS bits are available in..." and "PM bits
>> are
>> available in...".
>>
>> I'll also note that it says "by default only higher priv code can set the
>> value
>> for PM bits", while surely it's a security bug for lower priv code to read
>> anything related to a higher priv.
>>
>>
>> > +    target_ulong wpri_val = val & SMTE_MASK;
>> > +    assert(val == wpri_val);
>>
>> Incorrect assert.  This value is under guest control.  Do not crash qemu
>> because the guest is doing silly things.  Raise an exception if you like,
>> and
>> perhaps qemu_log_mask with LOG_GUEST_ERROR.
>>
>> > +    if (check_pm_current_disabled(env, csrno))
>> > +        return 0;
>>
>> Missing braces.
>>
>>
>> r~
>>
>
Richard Henderson Oct. 16, 2020, 5:16 p.m. UTC | #4
On 10/15/20 11:05 AM, Alexey Baturo wrote:
> Meanwhile, do you think applying **MTE *masks while reading CSR values is a
> good solution for now?

Yes.


r~
diff mbox series

Patch

diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c
index fe6bab4a52..d63031eb08 100644
--- a/target/riscv/cpu.c
+++ b/target/riscv/cpu.c
@@ -440,6 +440,7 @@  static void riscv_cpu_realize(DeviceState *dev, Error **errp)
         }
         if (cpu->cfg.ext_j) {
             target_misa |= RVJ;
+            env->mmte |= PM_EXT_INITIAL;
         }
         if (cpu->cfg.ext_v) {
             target_misa |= RVV;
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index eca611a367..21e47b8283 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -226,6 +226,17 @@  struct CPURISCVState {
 
     /* True if in debugger mode.  */
     bool debugger;
+
+    /* CSRs for PM
+     * TODO: move these csr to appropriate groups
+     */
+    target_ulong mmte;
+    target_ulong mpmmask;
+    target_ulong mpmbase;
+    target_ulong spmmask;
+    target_ulong spmbase;
+    target_ulong upmmask;
+    target_ulong upmbase;
 #endif
 
     float_status fp_status;
diff --git a/target/riscv/cpu_bits.h b/target/riscv/cpu_bits.h
index bd36062877..84c93c77ae 100644
--- a/target/riscv/cpu_bits.h
+++ b/target/riscv/cpu_bits.h
@@ -354,6 +354,21 @@ 
 #define CSR_MHPMCOUNTER30H  0xb9e
 #define CSR_MHPMCOUNTER31H  0xb9f
 
+/* Custom user register */
+#define CSR_UMTE            0x8c0
+#define CSR_UPMMASK         0x8c1
+#define CSR_UPMBASE         0x8c2
+
+/* Custom machine register */
+#define CSR_MMTE            0x7c0
+#define CSR_MPMMASK         0x7c1
+#define CSR_MPMBASE         0x7c2
+
+/* Custom supervisor register */
+#define CSR_SMTE            0x9c0
+#define CSR_SPMMASK         0x9c1
+#define CSR_SPMBASE         0x9c2
+
 /* Legacy Machine Protection and Translation (priv v1.9.1) */
 #define CSR_MBASE           0x380
 #define CSR_MBOUND          0x381
@@ -604,4 +619,55 @@ 
 #define MIE_UTIE                           (1 << IRQ_U_TIMER)
 #define MIE_SSIE                           (1 << IRQ_S_SOFT)
 #define MIE_USIE                           (1 << IRQ_U_SOFT)
+
+/* general mte CSR bits*/
+#define PM_ENABLE       0x00000001ULL
+#define PM_CURRENT      0x00000002ULL
+#define PM_XS_MASK      0x00000003ULL
+
+/* PM XS bits values */
+#define PM_EXT_DISABLE  0x00000000ULL
+#define PM_EXT_INITIAL  0x00000001ULL
+#define PM_EXT_CLEAN    0x00000002ULL
+#define PM_EXT_DIRTY    0x00000003ULL
+
+/* offsets for every pair of control bits per each priv level */
+#define XS_OFFSET    0ULL
+#define U_OFFSET     2ULL
+#define S_OFFSET     4ULL
+#define M_OFFSET     6ULL
+
+#define PM_XS_BITS   (PM_XS_MASK << XS_OFFSET)
+#define U_PM_ENABLE  (PM_ENABLE  << U_OFFSET)
+#define U_PM_CURRENT (PM_CURRENT << U_OFFSET)
+#define S_PM_ENABLE  (PM_ENABLE  << S_OFFSET)
+#define S_PM_CURRENT (PM_CURRENT << S_OFFSET)
+#define M_PM_ENABLE  (PM_ENABLE  << M_OFFSET)
+
+/* mmte CSR bits */
+#define MMTE_PM_XS_BITS     PM_XS_BITS
+#define MMTE_U_PM_ENABLE    U_PM_ENABLE
+#define MMTE_U_PM_CURRENT   U_PM_CURRENT
+#define MMTE_S_PM_ENABLE    S_PM_ENABLE
+#define MMTE_S_PM_CURRENT   S_PM_CURRENT
+#define MMTE_M_PM_ENABLE    M_PM_ENABLE
+#define MMTE_MASK           (MMTE_U_PM_ENABLE | MMTE_U_PM_CURRENT | \
+                             MMTE_S_PM_ENABLE | MMTE_S_PM_CURRENT | \
+                             MMTE_M_PM_ENABLE | MMTE_PM_XS_BITS)
+
+/* smte CSR bits */
+#define SMTE_PM_XS_BITS     PM_XS_BITS
+#define SMTE_U_PM_ENABLE    U_PM_ENABLE
+#define SMTE_U_PM_CURRENT   U_PM_CURRENT
+#define SMTE_S_PM_ENABLE    S_PM_ENABLE
+#define SMTE_S_PM_CURRENT   S_PM_CURRENT
+#define SMTE_MASK           (SMTE_U_PM_ENABLE | SMTE_U_PM_CURRENT | \
+                             SMTE_S_PM_ENABLE | SMTE_S_PM_CURRENT | \
+                             SMTE_PM_XS_BITS)
+
+/* umte CSR bits */
+#define UMTE_U_PM_ENABLE    U_PM_ENABLE
+#define UMTE_U_PM_CURRENT   U_PM_CURRENT
+#define UMTE_MASK           (UMTE_U_PM_ENABLE | MMTE_U_PM_CURRENT)
+
 #endif
diff --git a/target/riscv/csr.c b/target/riscv/csr.c
index aaef6c6f20..f48f4c4070 100644
--- a/target/riscv/csr.c
+++ b/target/riscv/csr.c
@@ -140,6 +140,11 @@  static int any(CPURISCVState *env, int csrno)
     return 0;
 }
 
+static int umode(CPURISCVState *env, int csrno)
+{
+    return -!riscv_has_ext(env, RVU);
+}
+
 static int smode(CPURISCVState *env, int csrno)
 {
     return -!riscv_has_ext(env, RVS);
@@ -1250,6 +1255,263 @@  static int write_pmpaddr(CPURISCVState *env, int csrno, target_ulong val)
     return 0;
 }
 
+/* Functions to access Pointer Masking feature registers 
+ * We have to check if current priv lvl could modify
+ * csr in given mode
+ */
+static int check_pm_current_disabled(CPURISCVState *env, int csrno)
+{
+    /* m-mode is always allowed to modify registers, so allow */
+    if (env->priv == PRV_M) {
+        return 0;
+    }
+    int csr_priv = get_field(csrno, 0xC00);
+    /* If priv lvls differ that means we're accessing csr from higher priv lvl, so allow */
+    if (env->priv != csr_priv) {
+        return 0;
+    }
+    int cur_bit_pos = (env->priv == PRV_U) ? U_PM_CURRENT :
+                      (env->priv == PRV_S) ? S_PM_CURRENT : -1;
+    assert(cur_bit_pos != -1);
+    int pm_current = get_field(env->mmte, cur_bit_pos);
+    /* We're in same priv lvl, so we allow to modify csr only if pm_current==1 */
+    return !pm_current;
+}
+
+static int read_mmte(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        *val = 0;
+        return 0;
+    }
+#endif
+    *val = env->mmte & MMTE_MASK;
+    return 0;
+}
+
+static int write_mmte(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    target_ulong wpri_val = val & MMTE_MASK;
+    assert(val == wpri_val);
+    env->mmte = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
+
+static int read_smte(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        *val = 0;
+        return 0;
+    }
+#endif
+    *val = env->mmte & SMTE_MASK;
+    return 0;
+}
+
+static int write_smte(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    target_ulong wpri_val = val & SMTE_MASK;
+    assert(val == wpri_val);
+    if (check_pm_current_disabled(env, csrno))
+        return 0;
+    target_ulong new_val = val | (env->mmte & ~SMTE_MASK);
+    write_mmte(env, csrno, new_val);
+    return 0;
+}
+
+static int read_umte(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        *val = 0;
+        return 0;
+    }
+#endif
+    *val = env->mmte & UMTE_MASK;
+    return 0;
+}
+
+static int write_umte(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    target_ulong wpri_val = val & UMTE_MASK;
+    assert(val == wpri_val);
+    if (check_pm_current_disabled(env, csrno))
+        return 0;
+    target_ulong new_val = val | (env->mmte & ~UMTE_MASK);
+    write_mmte(env, csrno, new_val);
+    return 0;
+}
+
+static int read_mpmmask(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    *val = env->mpmmask;
+    return 0;
+}
+
+static int write_mpmmask(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    env->mpmmask = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
+
+static int read_spmmask(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    *val = env->spmmask;
+    return 0;
+}
+
+static int write_spmmask(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    if (check_pm_current_disabled(env, csrno))
+        return 0;
+    env->spmmask = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
+
+static int read_upmmask(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    *val = env->upmmask;
+    return 0;
+}
+
+static int write_upmmask(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    if (check_pm_current_disabled(env, csrno))
+        return 0;
+    env->upmmask = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
+
+static int read_mpmbase(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    *val = env->mpmbase;
+    return 0;
+}
+
+static int write_mpmbase(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    env->mpmbase = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
+
+static int read_spmbase(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    *val = env->spmbase;
+    return 0;
+}
+
+static int write_spmbase(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    if (check_pm_current_disabled(env, csrno))
+        return 0;
+    env->spmbase = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
+
+static int read_upmbase(CPURISCVState *env, int csrno, target_ulong *val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    *val = env->upmbase;
+    return 0;
+}
+
+static int write_upmbase(CPURISCVState *env, int csrno, target_ulong val)
+{
+#if !defined(CONFIG_USER_ONLY)
+    if (!riscv_has_ext(env, RVJ)) {
+        return -RISCV_EXCP_ILLEGAL_INST;
+    }
+#endif
+    if (check_pm_current_disabled(env, csrno))
+        return 0;
+    env->upmbase = val;
+    env->mstatus |= MSTATUS_XS | MSTATUS_SD;
+    env->mmte |= PM_EXT_DIRTY;
+    return 0;
+}
 #endif
 
 /*
@@ -1471,6 +1733,21 @@  static riscv_csr_operations csr_ops[CSR_TABLE_SIZE] = {
     [CSR_PMPCFG0  ... CSR_PMPCFG3]   = { pmp,   read_pmpcfg,  write_pmpcfg   },
     [CSR_PMPADDR0 ... CSR_PMPADDR15] = { pmp,   read_pmpaddr, write_pmpaddr  },
 
+    /* User Pointer Masking */
+    [CSR_UMTE] =                { umode,  read_umte,        write_umte        },
+    [CSR_UPMMASK] =             { umode,  read_upmmask,     write_upmmask     },
+    [CSR_UPMBASE] =             { umode,  read_upmbase,     write_upmbase     },
+
+    /* Machine Pointer Masking */
+    [CSR_MMTE] =                { any,  read_mmte,        write_mmte        },
+    [CSR_MPMMASK] =             { any,  read_mpmmask,     write_mpmmask     },
+    [CSR_MPMBASE] =             { any,  read_mpmbase,     write_mpmbase     },
+
+    /* Supervisor Pointer Masking */
+    [CSR_SMTE] =                { smode, read_smte,        write_smte        },
+    [CSR_SPMMASK] =             { smode, read_spmmask,     write_spmmask     },
+    [CSR_SPMBASE] =             { smode, read_spmbase,     write_spmbase     },
+
     /* Performance Counters */
     [CSR_HPMCOUNTER3   ... CSR_HPMCOUNTER31] =    { ctr,  read_zero          },
     [CSR_MHPMCOUNTER3  ... CSR_MHPMCOUNTER31] =   { any,  read_zero          },