| Message ID | 20201015152139.28903-1-space.monkey.delivers@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [v2,1/5,RISCV_PM] Add J-extension into RISC-V | expand |
On 10/15/20 8:21 AM, Alexey Baturo wrote: > + switch (priv) { > + case PRV_U: > + pm_enabled = env->mmte & U_PM_ENABLE; > + break; > + case PRV_S: > + pm_enabled = env->mmte & S_PM_ENABLE; > + break; > + case PRV_M: > + pm_enabled = env->mmte & M_PM_ENABLE; > + break; > + default: > + assert(0 && "Unreachable"); g_assert_not_reached(); > + /* PointerMasking extension */ > + uint8_t pm_enabled; bool > + if (s->pm_enabled == 0) { !s->pm_enabled > + if (riscv_has_ext(env, RVJ)) { > + ctx->pm_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_ENABLED); > + int priv = cpu_mmu_index(env, false); > + ctx->pm_mask = pm_mask[priv]; > + ctx->pm_base = pm_base[priv]; > + } else { > + ctx->pm_enabled = 0; > + } Don't need the if. And should it in fact be placed outside the ifdef? This shouldn't be related to !CONFIG_USER_ONLY here and nowhere else. r~
> g_assert_not_reached(); Would fix, thanks. > bool Would fix. >!s->pm_enabled Same. > Don't need the if. Would remove, thanks. > And should it in fact be placed outside the ifdef? Sure, you're right. Richard, thank you for your time and effort reviewing these changes! чт, 15 окт. 2020 г. в 20:07, Richard Henderson <richard.henderson@linaro.org >: > On 10/15/20 8:21 AM, Alexey Baturo wrote: > > + switch (priv) { > > + case PRV_U: > > + pm_enabled = env->mmte & U_PM_ENABLE; > > + break; > > + case PRV_S: > > + pm_enabled = env->mmte & S_PM_ENABLE; > > + break; > > + case PRV_M: > > + pm_enabled = env->mmte & M_PM_ENABLE; > > + break; > > + default: > > + assert(0 && "Unreachable"); > > g_assert_not_reached(); > > > + /* PointerMasking extension */ > > + uint8_t pm_enabled; > > bool > > > + if (s->pm_enabled == 0) { > > !s->pm_enabled > > > + if (riscv_has_ext(env, RVJ)) { > > + ctx->pm_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_ENABLED); > > + int priv = cpu_mmu_index(env, false); > > + ctx->pm_mask = pm_mask[priv]; > > + ctx->pm_base = pm_base[priv]; > > + } else { > > + ctx->pm_enabled = 0; > > + } > > Don't need the if. And should it in fact be placed outside the ifdef? > This > shouldn't be related to !CONFIG_USER_ONLY here and nowhere else. > > > r~ > <div dir="ltr">> g_assert_not_reached();<div>Would fix, thanks.</div><div><br></div><div>> bool</div><div>Would fix.</div><div><br></div><div>>!s->pm_enabled</div><div>Same.</div><div><br></div><div>> Don't need the if. </div><div>Would remove, thanks.</div><div><br></div><div>> And should it in fact be placed outside the ifdef? </div><div>Sure, you're right.</div><div><br></div><div>Richard, thank you for your time and effort reviewing these changes!</div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">чт, 15 окт. 2020 г. в 20:07, Richard Henderson <<a href="mailto:richard.henderson@linaro.org">richard.henderson@linaro.org</a>>:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 10/15/20 8:21 AM, Alexey Baturo wrote:<br> > + switch (priv) {<br> > + case PRV_U:<br> > + pm_enabled = env->mmte & U_PM_ENABLE;<br> > + break;<br> > + case PRV_S:<br> > + pm_enabled = env->mmte & S_PM_ENABLE;<br> > + break;<br> > + case PRV_M:<br> > + pm_enabled = env->mmte & M_PM_ENABLE;<br> > + break;<br> > + default:<br> > + assert(0 && "Unreachable");<br> <br> g_assert_not_reached();<br> <br> > + /* PointerMasking extension */<br> > + uint8_t pm_enabled;<br> <br> bool<br> <br> > + if (s->pm_enabled == 0) {<br> <br> !s->pm_enabled<br> <br> > + if (riscv_has_ext(env, RVJ)) {<br> > + ctx->pm_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_ENABLED);<br> > + int priv = cpu_mmu_index(env, false);<br> > + ctx->pm_mask = pm_mask[priv];<br> > + ctx->pm_base = pm_base[priv];<br> > + } else {<br> > + ctx->pm_enabled = 0;<br> > + }<br> <br> Don't need the if. And should it in fact be placed outside the ifdef? This<br> shouldn't be related to !CONFIG_USER_ONLY here and nowhere else.<br> <br> <br> r~<br> </blockquote></div>
diff --git a/target/riscv/cpu.c b/target/riscv/cpu.c index 0bbfd7f457..fe6bab4a52 100644 --- a/target/riscv/cpu.c +++ b/target/riscv/cpu.c @@ -438,6 +438,9 @@ static void riscv_cpu_realize(DeviceState *dev, Error **errp) if (cpu->cfg.ext_h) { target_misa |= RVH; } + if (cpu->cfg.ext_j) { + target_misa |= RVJ; + } if (cpu->cfg.ext_v) { target_misa |= RVV; if (!is_power_of_2(cpu->cfg.vlen)) { @@ -516,6 +519,7 @@ static Property riscv_cpu_properties[] = { DEFINE_PROP_BOOL("u", RISCVCPU, cfg.ext_u, true), /* This is experimental so mark with 'x-' */ DEFINE_PROP_BOOL("x-h", RISCVCPU, cfg.ext_h, false), + DEFINE_PROP_BOOL("x-j", RISCVCPU, cfg.ext_j, false), DEFINE_PROP_BOOL("x-v", RISCVCPU, cfg.ext_v, false), DEFINE_PROP_BOOL("Counters", RISCVCPU, cfg.ext_counters, true), DEFINE_PROP_BOOL("Zifencei", RISCVCPU, cfg.ext_ifencei, true), diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index de275782e6..eca611a367 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -66,6 +66,7 @@ #define RVS RV('S') #define RVU RV('U') #define RVH RV('H') +#define RVJ RV('J') /* S extension denotes that Supervisor mode exists, however it is possible to have a core that support S mode but does not have an MMU and there @@ -277,6 +278,7 @@ struct RISCVCPU { bool ext_s; bool ext_u; bool ext_h; + bool ext_j; bool ext_v; bool ext_counters; bool ext_ifencei;
Signed-off-by: Alexey Baturo <space.monkey.delivers@gmail.com> --- target/riscv/cpu.c | 4 ++++ target/riscv/cpu.h | 2 ++ 2 files changed, 6 insertions(+)