Follow the Svukte spec, do the memory access address checking
1. Include instruction fetches or explicit memory accesses
2. System run in effective privilege U or VU
3. Check senvcfg[UKTE] being set, or hstatus[HUKTE] being set if
instruction is HLV, HLVX, HSV and excute from U mode to VU mode
4. Depend on Sv39 and check virtual addresses bit[SXLEN-1]
5. Raises a page-fault exception corresponding to the original access
type.
Ref: https://github.com/riscv/riscv-isa-manual/pull/1564/files
Signed-off-by: Frank Chang <frank.chang@sifive.com>
Signed-off-by: Fea.Wang <fea.wang@sifive.com>
Reviewed-by: Jim Shu <jim.shu@sifive.com>
---
target/riscv/cpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++
1 file changed, 57 insertions(+)
diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
index 0a3ead69ea..725c049f7a 100644
--- a/target/riscv/cpu_helper.c
+++ b/target/riscv/cpu_helper.c
@@ -857,6 +857,57 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot, hwaddr addr,
return TRANSLATE_SUCCESS;
}
+/*
+ * Return 'true' means no need to do svukte check, or need to do svukte and the
+ * address is valid. Return 'false' means need to do svukte check but address
+ * is invalid.
+ */
+static bool check_svukte_valid(CPURISCVState *env, vaddr addr,
+ int mode, bool virt)
+{
+ /*
+ * Check hstatus.HUKTE if the effective mode is switched to VU-mode by
+ * executing HLV/HLVX/HSV in U-mode.
+ * For other cases, check senvcfg.UKTE.
+ */
+ bool ukte;
+ if (env->priv == PRV_U && !env->virt_enabled && virt) {
+ ukte = !!(env->hstatus & HSTATUS_HUKTE);
+ } else {
+ ukte = !!(env->senvcfg & SENVCFG_UKTE);
+ }
+
+ if (VM_1_10_SV39 != get_field(env->satp, SATP64_MODE)) {
+ /* Svukte extension depends on Sv39. */
+ return true;
+ }
+
+ /*
+ * Svukte extension is qualified only in U or VU-mode.
+ *
+ * Effective mode can be switched to U or VU-mode by:
+ * - M-mode + mstatus.MPRV=1 + mstatus.MPP=U-mode.
+ * - Execute HLV/HLVX/HSV from HS-mode + hstatus.SPVP=0.
+ * - U-mode.
+ * - VU-mode.
+ * - Execute HLV/HLVX/HSV from U-mode + hstatus.HU=1.
+ */
+ if (mode != PRV_U) {
+ return true;
+ }
+
+ if (!ukte) {
+ return true;
+ }
+
+ uint32_t sxl = riscv_cpu_sxl(env);
+ sxl = (sxl == 0) ? MXL_RV32 : sxl;
+ uint32_t sxlen = 32 * sxl;
+ uint64_t high_bit = addr & (1UL << (sxlen - 1));
+
+ return !high_bit;
+}
+
/*
* get_physical_address - get the physical address for this virtual address
*
@@ -894,6 +945,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
MemTxResult res;
MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
int mode = mmuidx_priv(mmu_idx);
+ bool virt = mmuidx_2stage(mmu_idx);
bool use_background = false;
hwaddr ppn;
int napot_bits = 0;
@@ -901,6 +953,11 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
bool sstack_page = false;
+ if (env_archcpu(env)->cfg.ext_svukte && first_stage &&
+ !check_svukte_valid(env, addr, mode, virt)) {
+ return TRANSLATE_FAIL;
+ }
+
/*
* Check if we should use the background registers for the two
* stage translation. We don't need to check if we actually need
--
2.34.1
On 11/8/24 5:52 AM, Fea.Wang wrote:
> Follow the Svukte spec, do the memory access address checking
>
> 1. Include instruction fetches or explicit memory accesses
> 2. System run in effective privilege U or VU
> 3. Check senvcfg[UKTE] being set, or hstatus[HUKTE] being set if
> instruction is HLV, HLVX, HSV and excute from U mode to VU mode
typo: s/excute/execute
> 4. Depend on Sv39 and check virtual addresses bit[SXLEN-1]
> 5. Raises a page-fault exception corresponding to the original access
> type.
>
> Ref: https://github.com/riscv/riscv-isa-manual/pull/1564/files
>
> Signed-off-by: Frank Chang <frank.chang@sifive.com>
> Signed-off-by: Fea.Wang <fea.wang@sifive.com>
> Reviewed-by: Jim Shu <jim.shu@sifive.com>
> ---
> target/riscv/cpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 57 insertions(+)
>
> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> index 0a3ead69ea..725c049f7a 100644
> --- a/target/riscv/cpu_helper.c
> +++ b/target/riscv/cpu_helper.c
> @@ -857,6 +857,57 @@ static int get_physical_address_pmp(CPURISCVState *env, int *prot, hwaddr addr,
> return TRANSLATE_SUCCESS;
> }
>
> +/*
> + * Return 'true' means no need to do svukte check, or need to do svukte and the
> + * address is valid. Return 'false' means need to do svukte check but address
> + * is invalid.
> + */
I think this function is doing a bit too much while not doing everything we need.
For instance, we're still doing some checks in get_physical_address() prior to
calling it.
I would put all the svukte logic in two functions:
- 'do_svukte_check' would check all dependencies for the svukte check per se
- 'check_svukte_addr' would do the addr check if we need to do a svukte check
An example:
/* Returns 'true' if a svukte address check is needed */
static bool do_svukte_check(CPURISCVState *env, bool first_stage,
int mode, bool virt)
{
bool ukte;
/* Svukte extension depends on Sv39. */
if (!(env_archcpu(env)->cfg.ext_svukte &&
first_stage &&
VM_1_10_SV39 == get_field(env->satp, SATP64_MODE))) {
return false;
}
/*
* Check hstatus.HUKTE if the effective mode is switched to VU-mode by
* executing HLV/HLVX/HSV in U-mode.
* For other cases, check senvcfg.UKTE.
*/
if (env->priv == PRV_U && !env->virt_enabled && virt) {
ukte = !!(env->hstatus & HSTATUS_HUKTE);
} else {
ukte = !!(env->senvcfg & SENVCFG_UKTE);
}
if (!ukte) {
return false;
}
/*
* Svukte extension is qualified only in U or VU-mode.
*
* Effective mode can be switched to U or VU-mode by:
* - M-mode + mstatus.MPRV=1 + mstatus.MPP=U-mode.
* - Execute HLV/HLVX/HSV from HS-mode + hstatus.SPVP=0.
* - U-mode.
* - VU-mode.
* - Execute HLV/HLVX/HSV from U-mode + hstatus.HU=1.
*/
if (mode != PRV_U) {
return false;
}
return true;
}
static bool check_svukte_addr(CPURISCVState *env, vaddr addr)
{
uint32_t sxl = riscv_cpu_sxl(env);
sxl = (sxl == 0) ? MXL_RV32 : sxl;
uint32_t sxlen = 32 * sxl;
uint64_t high_bit = addr & (1UL << (sxlen - 1));
return !high_bit;
}
And then on get_physical_addr():
if (do_svukte_check(env, first_stage, mode, virt) &&
!check_svukte_addr(env, addr)) {
return TRANSLATE_FAIL;
}
Feel free to use these functions (compiled tested only) if you want. Thanks,
Daniel
> +static bool check_svukte_valid(CPURISCVState *env, vaddr addr,
> + int mode, bool virt)
> +{
> + /*
> + * Check hstatus.HUKTE if the effective mode is switched to VU-mode by
> + * executing HLV/HLVX/HSV in U-mode.
> + * For other cases, check senvcfg.UKTE.
> + */
> + bool ukte;
> + if (env->priv == PRV_U && !env->virt_enabled && virt) {
> + ukte = !!(env->hstatus & HSTATUS_HUKTE);
> + } else {
> + ukte = !!(env->senvcfg & SENVCFG_UKTE);
> + }
> +
> + if (VM_1_10_SV39 != get_field(env->satp, SATP64_MODE)) {
> + /* Svukte extension depends on Sv39. */
> + return true;
> + }> +
> + /*
> + * Svukte extension is qualified only in U or VU-mode.
> + *
> + * Effective mode can be switched to U or VU-mode by:
> + * - M-mode + mstatus.MPRV=1 + mstatus.MPP=U-mode.
> + * - Execute HLV/HLVX/HSV from HS-mode + hstatus.SPVP=0.
> + * - U-mode.
> + * - VU-mode.
> + * - Execute HLV/HLVX/HSV from U-mode + hstatus.HU=1.
> + */
> + if (mode != PRV_U) {
> + return true;
> + }
> +
> + if (!ukte) {
> + return true;
> + }
> +
> + uint32_t sxl = riscv_cpu_sxl(env);
> + sxl = (sxl == 0) ? MXL_RV32 : sxl;
> + uint32_t sxlen = 32 * sxl;
> + uint64_t high_bit = addr & (1UL << (sxlen - 1));
> +
> + return !high_bit;
> +}
> +
> /*
> * get_physical_address - get the physical address for this virtual address
> *
> @@ -894,6 +945,7 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> MemTxResult res;
> MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> int mode = mmuidx_priv(mmu_idx);
> + bool virt = mmuidx_2stage(mmu_idx);
> bool use_background = false;
> hwaddr ppn;
> int napot_bits = 0;
> @@ -901,6 +953,11 @@ static int get_physical_address(CPURISCVState *env, hwaddr *physical,
> bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) == MMU_IDX_SS_WRITE);
> bool sstack_page = false;
>
> + if (env_archcpu(env)->cfg.ext_svukte && first_stage &&
> + !check_svukte_valid(env, addr, mode, virt)) {
> + return TRANSLATE_FAIL;
> + }
> +
> /*
> * Check if we should use the background registers for the two
> * stage translation. We don't need to check if we actually need
Thank you for refining it.
I fixed some parts and will put them in the V3 patches.
Sincerely,
Fea
On Tue, Nov 12, 2024 at 2:32 AM Daniel Henrique Barboza <
dbarboza@ventanamicro.com> wrote:
>
>
> On 11/8/24 5:52 AM, Fea.Wang wrote:
> > Follow the Svukte spec, do the memory access address checking
> >
> > 1. Include instruction fetches or explicit memory accesses
> > 2. System run in effective privilege U or VU
> > 3. Check senvcfg[UKTE] being set, or hstatus[HUKTE] being set if
> > instruction is HLV, HLVX, HSV and excute from U mode to VU mode
>
> typo: s/excute/execute
>
> > 4. Depend on Sv39 and check virtual addresses bit[SXLEN-1]
> > 5. Raises a page-fault exception corresponding to the original access
> > type.
> >
> > Ref: https://github.com/riscv/riscv-isa-manual/pull/1564/files
> >
> > Signed-off-by: Frank Chang <frank.chang@sifive.com>
> > Signed-off-by: Fea.Wang <fea.wang@sifive.com>
> > Reviewed-by: Jim Shu <jim.shu@sifive.com>
> > ---
> > target/riscv/cpu_helper.c | 57 +++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 57 insertions(+)
> >
> > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c
> > index 0a3ead69ea..725c049f7a 100644
> > --- a/target/riscv/cpu_helper.c
> > +++ b/target/riscv/cpu_helper.c
> > @@ -857,6 +857,57 @@ static int get_physical_address_pmp(CPURISCVState
> *env, int *prot, hwaddr addr,
> > return TRANSLATE_SUCCESS;
> > }
> >
> > +/*
> > + * Return 'true' means no need to do svukte check, or need to do svukte
> and the
> > + * address is valid. Return 'false' means need to do svukte check but
> address
> > + * is invalid.
> > + */
>
> I think this function is doing a bit too much while not doing everything
> we need.
> For instance, we're still doing some checks in get_physical_address()
> prior to
> calling it.
>
> I would put all the svukte logic in two functions:
>
> - 'do_svukte_check' would check all dependencies for the svukte check per
> se
> - 'check_svukte_addr' would do the addr check if we need to do a svukte
> check
>
>
> An example:
>
>
>
> /* Returns 'true' if a svukte address check is needed */
> static bool do_svukte_check(CPURISCVState *env, bool first_stage,
> int mode, bool virt)
> {
> bool ukte;
>
> /* Svukte extension depends on Sv39. */
> if (!(env_archcpu(env)->cfg.ext_svukte &&
> first_stage &&
> VM_1_10_SV39 == get_field(env->satp, SATP64_MODE))) {
> return false;
> }
>
> /*
> * Check hstatus.HUKTE if the effective mode is switched to VU-mode by
> * executing HLV/HLVX/HSV in U-mode.
> * For other cases, check senvcfg.UKTE.
> */
> if (env->priv == PRV_U && !env->virt_enabled && virt) {
> ukte = !!(env->hstatus & HSTATUS_HUKTE);
> } else {
> ukte = !!(env->senvcfg & SENVCFG_UKTE);
> }
>
> if (!ukte) {
> return false;
> }
>
> /*
> * Svukte extension is qualified only in U or VU-mode.
> *
> * Effective mode can be switched to U or VU-mode by:
> * - M-mode + mstatus.MPRV=1 + mstatus.MPP=U-mode.
> * - Execute HLV/HLVX/HSV from HS-mode + hstatus.SPVP=0.
> * - U-mode.
> * - VU-mode.
> * - Execute HLV/HLVX/HSV from U-mode + hstatus.HU=1.
> */
> if (mode != PRV_U) {
> return false;
> }
>
> return true;
> }
>
>
> static bool check_svukte_addr(CPURISCVState *env, vaddr addr)
> {
> uint32_t sxl = riscv_cpu_sxl(env);
> sxl = (sxl == 0) ? MXL_RV32 : sxl;
> uint32_t sxlen = 32 * sxl;
> uint64_t high_bit = addr & (1UL << (sxlen - 1));
>
> return !high_bit;
> }
>
>
> And then on get_physical_addr():
>
>
> if (do_svukte_check(env, first_stage, mode, virt) &&
> !check_svukte_addr(env, addr)) {
> return TRANSLATE_FAIL;
> }
>
>
> Feel free to use these functions (compiled tested only) if you want.
> Thanks,
>
> Daniel
>
>
> > +static bool check_svukte_valid(CPURISCVState *env, vaddr addr,
> > + int mode, bool virt)
> > +{
> > + /*
> > + * Check hstatus.HUKTE if the effective mode is switched to VU-mode
> by
> > + * executing HLV/HLVX/HSV in U-mode.
> > + * For other cases, check senvcfg.UKTE.
> > + */
> > + bool ukte;
> > + if (env->priv == PRV_U && !env->virt_enabled && virt) {
> > + ukte = !!(env->hstatus & HSTATUS_HUKTE);
> > + } else {
> > + ukte = !!(env->senvcfg & SENVCFG_UKTE);
> > + }
> > +
> > + if (VM_1_10_SV39 != get_field(env->satp, SATP64_MODE)) {
> > + /* Svukte extension depends on Sv39. */
> > + return true;
> > + }> +
> > + /*
> > + * Svukte extension is qualified only in U or VU-mode.
> > + *
> > + * Effective mode can be switched to U or VU-mode by:
> > + * - M-mode + mstatus.MPRV=1 + mstatus.MPP=U-mode.
> > + * - Execute HLV/HLVX/HSV from HS-mode + hstatus.SPVP=0.
> > + * - U-mode.
> > + * - VU-mode.
> > + * - Execute HLV/HLVX/HSV from U-mode + hstatus.HU=1.
> > + */
> > + if (mode != PRV_U) {
> > + return true;
> > + }
> > +
> > + if (!ukte) {
> > + return true;
> > + }
> > +
> > + uint32_t sxl = riscv_cpu_sxl(env);
> > + sxl = (sxl == 0) ? MXL_RV32 : sxl;
> > + uint32_t sxlen = 32 * sxl;
> > + uint64_t high_bit = addr & (1UL << (sxlen - 1));
> > +
> > + return !high_bit;
> > +}
> > +
> > /*
> > * get_physical_address - get the physical address for this virtual
> address
> > *
> > @@ -894,6 +945,7 @@ static int get_physical_address(CPURISCVState *env,
> hwaddr *physical,
> > MemTxResult res;
> > MemTxAttrs attrs = MEMTXATTRS_UNSPECIFIED;
> > int mode = mmuidx_priv(mmu_idx);
> > + bool virt = mmuidx_2stage(mmu_idx);
> > bool use_background = false;
> > hwaddr ppn;
> > int napot_bits = 0;
> > @@ -901,6 +953,11 @@ static int get_physical_address(CPURISCVState *env,
> hwaddr *physical,
> > bool is_sstack_idx = ((mmu_idx & MMU_IDX_SS_WRITE) ==
> MMU_IDX_SS_WRITE);
> > bool sstack_page = false;
> >
> > + if (env_archcpu(env)->cfg.ext_svukte && first_stage &&
> > + !check_svukte_valid(env, addr, mode, virt)) {
> > + return TRANSLATE_FAIL;
> > + }
> > +
> > /*
> > * Check if we should use the background registers for the two
> > * stage translation. We don't need to check if we actually need
>
© 2016 - 2024 Red Hat, Inc.