diff mbox series

[v4,3/6] target/riscv: Add helper functions to calculate current number of masked bits for pointer masking

Message ID 20240109102930.405323-4-me@deliversmonkey.space
State New
Headers show
Series Pointer Masking update for Zjpm v0.8 | expand

Commit Message

Alexey Baturo Jan. 9, 2024, 10:29 a.m. UTC 
From: Alexey Baturo <baturo.alexey@gmail.com>

Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
---
 target/riscv/cpu.h        |  4 +++
 target/riscv/cpu_helper.c | 58 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 62 insertions(+)

Comments

Deepak Gupta Jan. 18, 2024, 5:21 p.m. UTC | #1 
On Tue, Jan 9, 2024 at 2:31 AM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> From: Alexey Baturo <baturo.alexey@gmail.com>
>
> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> ---

> +
> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> +{
> +    bool virt_mem_en = false;
> +#ifndef CONFIG_USER_ONLY
> +    int satp_mode = 0;
> +    int priv_mode = cpu_address_mode(env);
> +    /* Get current PMM field */
> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> +        satp_mode = get_field(env->satp, SATP32_MODE);
> +    } else {
> +        satp_mode = get_field(env->satp, SATP64_MODE);
> +    }
> +    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> +#endif
> +    return virt_mem_en;

Obsessing a little bit on how to test PM enabled binaries with qemu-user.
If we return false above then we're not allowed to test binaries with
pointer masking enabled with qemu-user.
That use case is not required?

> +}
> +
> +int riscv_pm_get_pmlen(RISCVPmPmm pmm)
> +{
> +    switch (pmm) {
> +    case PMM_FIELD_DISABLED:
> +        return 0;
> +    case PMM_FIELD_PMLEN7:
> +        return 7;
> +    case PMM_FIELD_PMLEN16:
> +        return 16;
> +    default:
> +        g_assert_not_reached();
> +    }
> +    return -1;
> +}
> +
>  #ifndef CONFIG_USER_ONLY
>
>  /*
> --
> 2.34.1
>
>
Richard Henderson Jan. 18, 2024, 8:50 p.m. UTC | #2 
On 1/19/24 04:21, Deepak Gupta wrote:
> On Tue, Jan 9, 2024 at 2:31 AM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>>
>> From: Alexey Baturo <baturo.alexey@gmail.com>
>>
>> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
>> ---
> 
>> +
>> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
>> +{
>> +    bool virt_mem_en = false;
>> +#ifndef CONFIG_USER_ONLY
>> +    int satp_mode = 0;
>> +    int priv_mode = cpu_address_mode(env);
>> +    /* Get current PMM field */
>> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
>> +        satp_mode = get_field(env->satp, SATP32_MODE);
>> +    } else {
>> +        satp_mode = get_field(env->satp, SATP64_MODE);
>> +    }
>> +    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
>> +#endif
>> +    return virt_mem_en;
> 
> Obsessing a little bit on how to test PM enabled binaries with qemu-user.
> If we return false above then we're not allowed to test binaries with
> pointer masking enabled with qemu-user.
> That use case is not required?

In a previous round I suggested that the ifdefs are not necessary.
But for now it will always be off for qemu-user.

At some point pointer masking will be in hardware, and the kernel will gain support for 
it, and there will likely be a prctl() added for it.  At the point the kernel finalizes 
the API, you will be able to enable pointer masking for qemu-user.


r~
Deepak Gupta Jan. 18, 2024, 10:40 p.m. UTC | #3 
On Thu, Jan 18, 2024 at 12:50 PM Richard Henderson
<richard.henderson@linaro.org> wrote:
>
> On 1/19/24 04:21, Deepak Gupta wrote:
> > On Tue, Jan 9, 2024 at 2:31 AM Alexey Baturo <baturo.alexey@gmail.com> wrote:
> >>
> >> From: Alexey Baturo <baturo.alexey@gmail.com>
> >>
> >> Signed-off-by: Alexey Baturo <baturo.alexey@gmail.com>
> >> ---
> >
> >> +
> >> +bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
> >> +{
> >> +    bool virt_mem_en = false;
> >> +#ifndef CONFIG_USER_ONLY
> >> +    int satp_mode = 0;
> >> +    int priv_mode = cpu_address_mode(env);
> >> +    /* Get current PMM field */
> >> +    if (riscv_cpu_mxl(env) == MXL_RV32) {
> >> +        satp_mode = get_field(env->satp, SATP32_MODE);
> >> +    } else {
> >> +        satp_mode = get_field(env->satp, SATP64_MODE);
> >> +    }
> >> +    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
> >> +#endif
> >> +    return virt_mem_en;
> >
> > Obsessing a little bit on how to test PM enabled binaries with qemu-user.
> > If we return false above then we're not allowed to test binaries with
> > pointer masking enabled with qemu-user.
> > That use case is not required?
>
> In a previous round I suggested that the ifdefs are not necessary.
> But for now it will always be off for qemu-user.
>
> At some point pointer masking will be in hardware, and the kernel will gain support for
> it, and there will likely be a prctl() added for it.  At the point the kernel finalizes
> the API, you will be able to enable pointer masking for qemu-user.

I am sure I am missing some important detail here, BUT...

How is it different from aarch64 "top byte ignore".
I think commit: 16c8497 enables top byte ignore for user pointers and
by default for qemu-user for aarch64 target.

IIRC, user <--> kernel abi is only needed for pointers that are passed
to the kernel.

And in the case of qemu-user, we are talking about the host kernel.
Since arm64 had TBI enabled for qemu-user for a while and I imagine it
works on x86 host kernel
(assuming qemu-user is sanitizing pointers)
Same should work for risc-v qemu-user, right?

>
>
> r~
Richard Henderson Jan. 20, 2024, 7:37 a.m. UTC | #4 
On 1/19/24 09:40, Deepak Gupta wrote:
> On Thu, Jan 18, 2024 at 12:50 PM Richard Henderson
> <richard.henderson@linaro.org> wrote:
>> At some point pointer masking will be in hardware, and the kernel will gain support for
>> it, and there will likely be a prctl() added for it.  At the point the kernel finalizes
>> the API, you will be able to enable pointer masking for qemu-user.
> 
> I am sure I am missing some important detail here, BUT...
> 
> How is it different from aarch64 "top byte ignore".

It is very similar, yes.

> I think commit: 16c8497 enables top byte ignore for user pointers and
> by default for qemu-user for aarch64 target.

Not quite, no.

commit 0e0c030c681730f3ec55ba3b223b608a8f3e8282
Author: Richard Henderson <richard.henderson@linaro.org>
Date:   Fri Feb 12 10:48:51 2021 -0800

     linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE

is more relevant.

> IIRC, user <--> kernel abi is only needed for pointers that are passed
> to the kernel.

It is also needed to *enable* pointer masking at all.

For aarch64, TBI has been enabled for user-space since the beginning, but that is not true 
for riscv.  Therefore there will be a need for a syscall to opt in and enable pointer masking.

> And in the case of qemu-user, we are talking about the host kernel.

No, we are not.  We are always emulating the guest kernel.


r~
Alexey Baturo Jan. 21, 2024, 7:13 a.m. UTC | #5 
Hi,

Having the feature to run binaries with pointer masking on qemu-user is
really nice, but I see this patch series as an initial support.
Obviously there'll be more patches and fixes for pointer masking as soon as
arch tests are ready.
I suggest supporting qemu-user in the next patches, but make sure we do
this before claiming 100% support for pointer masking.
@Deepak Gupta <debug@rivosinc.com> what do you think?

Thanks

сб, 20 янв. 2024 г. в 10:37, Richard Henderson <richard.henderson@linaro.org
>:

> On 1/19/24 09:40, Deepak Gupta wrote:
> > On Thu, Jan 18, 2024 at 12:50 PM Richard Henderson
> > <richard.henderson@linaro.org> wrote:
> >> At some point pointer masking will be in hardware, and the kernel will
> gain support for
> >> it, and there will likely be a prctl() added for it.  At the point the
> kernel finalizes
> >> the API, you will be able to enable pointer masking for qemu-user.
> >
> > I am sure I am missing some important detail here, BUT...
> >
> > How is it different from aarch64 "top byte ignore".
>
> It is very similar, yes.
>
> > I think commit: 16c8497 enables top byte ignore for user pointers and
> > by default for qemu-user for aarch64 target.
>
> Not quite, no.
>
> commit 0e0c030c681730f3ec55ba3b223b608a8f3e8282
> Author: Richard Henderson <richard.henderson@linaro.org>
> Date:   Fri Feb 12 10:48:51 2021 -0800
>
>      linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE
>
> is more relevant.
>
> > IIRC, user <--> kernel abi is only needed for pointers that are passed
> > to the kernel.
>
> It is also needed to *enable* pointer masking at all.
>
> For aarch64, TBI has been enabled for user-space since the beginning, but
> that is not true
> for riscv.  Therefore there will be a need for a syscall to opt in and
> enable pointer masking.
>
> > And in the case of qemu-user, we are talking about the host kernel.
>
> No, we are not.  We are always emulating the guest kernel.
>
>
> r~
>
Deepak Gupta Jan. 23, 2024, 5:44 p.m. UTC | #6 
On Sat, Jan 20, 2024 at 11:14 PM Alexey Baturo <baturo.alexey@gmail.com> wrote:
>
> Hi,
>
> Having the feature to run binaries with pointer masking on qemu-user is really nice, but I see this patch series as an initial support.
> Obviously there'll be more patches and fixes for pointer masking as soon as arch tests are ready.
> I suggest supporting qemu-user in the next patches, but make sure we do this before claiming 100% support for pointer masking.
> @Deepak Gupta what do you think?

I think that makes sense. Thanks.

>
> Thanks
>
> сб, 20 янв. 2024 г. в 10:37, Richard Henderson <richard.henderson@linaro.org>:
>>
>> On 1/19/24 09:40, Deepak Gupta wrote:
>> > On Thu, Jan 18, 2024 at 12:50 PM Richard Henderson
>> > <richard.henderson@linaro.org> wrote:
>> >> At some point pointer masking will be in hardware, and the kernel will gain support for
>> >> it, and there will likely be a prctl() added for it.  At the point the kernel finalizes
>> >> the API, you will be able to enable pointer masking for qemu-user.
>> >
>> > I am sure I am missing some important detail here, BUT...
>> >
>> > How is it different from aarch64 "top byte ignore".
>>
>> It is very similar, yes.
>>
>> > I think commit: 16c8497 enables top byte ignore for user pointers and
>> > by default for qemu-user for aarch64 target.
>>
>> Not quite, no.
>>
>> commit 0e0c030c681730f3ec55ba3b223b608a8f3e8282
>> Author: Richard Henderson <richard.henderson@linaro.org>
>> Date:   Fri Feb 12 10:48:51 2021 -0800
>>
>>      linux-user/aarch64: Implement PR_TAGGED_ADDR_ENABLE
>>
>> is more relevant.
>>
>> > IIRC, user <--> kernel abi is only needed for pointers that are passed
>> > to the kernel.
>>
>> It is also needed to *enable* pointer masking at all.
>>
>> For aarch64, TBI has been enabled for user-space since the beginning, but that is not true
>> for riscv.  Therefore there will be a need for a syscall to opt in and enable pointer masking.
>>
>> > And in the case of qemu-user, we are talking about the host kernel.
>>
>> No, we are not.  We are always emulating the guest kernel.
>>
>>
>> r~
diff mbox series

Patch

diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h
index c9bed5c9fc..1c8979c1c8 100644
--- a/target/riscv/cpu.h
+++ b/target/riscv/cpu.h
@@ -671,6 +671,10 @@  static inline uint32_t vext_get_vlmax(RISCVCPU *cpu, target_ulong vtype)
 void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
                           uint64_t *cs_base, uint32_t *pflags);
 
+bool riscv_cpu_virt_mem_enabled(CPURISCVState *env);
+RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env);
+int riscv_pm_get_pmlen(RISCVPmPmm pmm);
+
 RISCVException riscv_csrrw(CPURISCVState *env, int csrno,
                            target_ulong *ret_value,
                            target_ulong new_value, target_ulong write_mask);
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index a3d477d226..9640e4c2c5 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -139,6 +139,64 @@  void cpu_get_tb_cpu_state(CPURISCVState *env, vaddr *pc,
     *pflags = flags;
 }
 
+RISCVPmPmm riscv_pm_get_pmm(CPURISCVState *env)
+{
+    int pmm = 0;
+#ifndef CONFIG_USER_ONLY
+    int priv_mode = cpu_address_mode(env);
+    /* Get current PMM field */
+    switch (priv_mode) {
+    case PRV_M:
+        pmm = riscv_cpu_cfg(env)->ext_smmpm ?
+                  get_field(env->mseccfg, MSECCFG_PMM) : PMM_FIELD_DISABLED;
+        break;
+    case PRV_S:
+        pmm = riscv_cpu_cfg(env)->ext_smnpm ?
+                  get_field(env->menvcfg, MENVCFG_PMM) : PMM_FIELD_DISABLED;
+        break;
+    case PRV_U:
+        pmm = riscv_cpu_cfg(env)->ext_ssnpm ?
+                  get_field(env->senvcfg, SENVCFG_PMM) : PMM_FIELD_DISABLED;
+        break;
+    default:
+        g_assert_not_reached();
+    }
+#endif
+    return pmm;
+}
+
+bool riscv_cpu_virt_mem_enabled(CPURISCVState *env)
+{
+    bool virt_mem_en = false;
+#ifndef CONFIG_USER_ONLY
+    int satp_mode = 0;
+    int priv_mode = cpu_address_mode(env);
+    /* Get current PMM field */
+    if (riscv_cpu_mxl(env) == MXL_RV32) {
+        satp_mode = get_field(env->satp, SATP32_MODE);
+    } else {
+        satp_mode = get_field(env->satp, SATP64_MODE);
+    }
+    virt_mem_en = ((satp_mode != VM_1_10_MBARE) && (priv_mode != PRV_M));
+#endif
+    return virt_mem_en;
+}
+
+int riscv_pm_get_pmlen(RISCVPmPmm pmm)
+{
+    switch (pmm) {
+    case PMM_FIELD_DISABLED:
+        return 0;
+    case PMM_FIELD_PMLEN7:
+        return 7;
+    case PMM_FIELD_PMLEN16:
+        return 16;
+    default:
+        g_assert_not_reached();
+    }
+    return -1;
+}
+
 #ifndef CONFIG_USER_ONLY
 
 /*