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 < dbarb...@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.ch...@sifive.com> > > Signed-off-by: Fea.Wang <fea.w...@sifive.com> > > Reviewed-by: Jim Shu <jim....@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 >
