Thanks for the advice. I will fix them in the next patch version. Sincerely, Fea
On Tue, Nov 19, 2024 at 11:33 AM Alistair Francis <alistai...@gmail.com> wrote: > On Tue, Nov 12, 2024 at 7:13 PM Fea.Wang <fea.w...@sifive.com> 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 execute 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.ch...@sifive.com> > > Signed-off-by: Fea.Wang <fea.w...@sifive.com> > > Signed-off-by: Daniel Henrique Barboza <dbarb...@ventanamicro.com> > > Reviewed-by: Jim Shu <jim....@sifive.com> > > --- > > target/riscv/cpu_helper.c | 61 +++++++++++++++++++++++++++++++++++++++ > > 1 file changed, 61 insertions(+) > > > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > index 0a3ead69ea..5b29344c4f 100644 > > --- a/target/riscv/cpu_helper.c > > +++ b/target/riscv/cpu_helper.c > > @@ -857,6 +857,61 @@ static int get_physical_address_pmp(CPURISCVState > *env, int *prot, hwaddr addr, > > return TRANSLATE_SUCCESS; > > } > > > > +/* 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); > > You should just be able to use get_field() here > > > + } else { > > + ukte = !!(env->senvcfg & SENVCFG_UKTE); > > + } > > + > > + if (!ukte) { > > + return false; > > and it's probably simpler to remove the ukte variable and just return > based on the result of get_field() > > > + } > > + > > + /* > > + * 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; > > I don't think riscv_cpu_sxl() can return 0, do we actually need this check? > > Also this extension isn't defined for RV32 > > Alistair > > > + 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 +949,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 +957,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 (do_svukte_check(env, first_stage, mode, virt) && > > + !check_svukte_addr(env, addr)) { > > + 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 > > > > >
