| Message ID | 20230209062404.3582018-8-debug@rivosinc.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v1,RFC,Zisslpcfi,1/9] target/riscv: adding zimops and zisslpcfi extension to RISCV cpu config | expand |
On 2023/2/9 14:24, Deepak Gupta wrote: > zisslpcfi protects forward control flow (if enabled) by enforcing all > indirect call and jmp must land on a landing pad instruction `lpcll` > short for landing pad and check lower label value. If target of an > indirect call or jmp is not `lpcll` then cpu/hart must raise an illegal > instruction exception. > > This patch implements the mechanism using TCG. Target architecture branch > instruction must define the end of a TB. Using this property, during > translation of branch instruction, TB flag = FCFI_LP_EXPECTED can be set. > Translation of target TB can check if FCFI_LP_EXPECTED flag is set and a > flag (fcfi_lp_expected) can be set in DisasContext. If `lpcll` gets > translated, fcfi_lp_expected flag in DisasContext can be cleared. Else > it'll fault. > > This patch also also adds flag for forward and backward cfi in > DisasContext. > > Signed-off-by: Deepak Gupta <debug@rivosinc.com> > Signed-off-by: Kip Walker <kip@rivosinc.com> > --- > target/riscv/cpu.h | 3 +++ > target/riscv/cpu_helper.c | 12 +++++++++ > target/riscv/translate.c | 52 +++++++++++++++++++++++++++++++++++++++ > 3 files changed, 67 insertions(+) > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > index 8803ea6426..98b272bcad 100644 > --- a/target/riscv/cpu.h > +++ b/target/riscv/cpu.h > @@ -644,6 +644,9 @@ FIELD(TB_FLAGS, VMA, 25, 1) > /* Native debug itrigger */ > FIELD(TB_FLAGS, ITRIGGER, 26, 1) > > +/* Zisslpcfi needs a TB flag to track indirect branches */ > +FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 27, 1) > + > #ifdef TARGET_RISCV32 > #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32) > #else > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > index 63377abc2f..d15918f534 100644 > --- a/target/riscv/cpu_helper.c > +++ b/target/riscv/cpu_helper.c > @@ -129,6 +129,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1); > } > > + if (cpu->cfg.ext_cfi) { > + /* > + * For Forward CFI, only the expectation of a lpcll at > + * the start of the block is tracked (which can only happen > + * when FCFI is enabled for the current processor mode). A jump > + * or call at the end of the previous TB will have updated > + * env->elp to indicate the expectation. > + */ > + flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, > + env->elp != NO_LP_EXPECTED); > + } > + > #ifdef CONFIG_USER_ONLY > flags |= TB_FLAGS_MSTATUS_FS; > flags |= TB_FLAGS_MSTATUS_VS; > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > index df38db7553..7d43d20fc3 100644 > --- a/target/riscv/translate.c > +++ b/target/riscv/translate.c > @@ -41,6 +41,7 @@ static TCGv load_val; > /* globals for PM CSRs */ > static TCGv pm_mask; > static TCGv pm_base; > +static TCGOp *cfi_lp_check; > > #include "exec/gen-icount.h" > > @@ -116,6 +117,10 @@ typedef struct DisasContext { > bool itrigger; > /* TCG of the current insn_start */ > TCGOp *insn_start; > + /* CFI extension */ > + bool bcfi_enabled; > + bool fcfi_enabled; > + bool fcfi_lp_expected; > } DisasContext; > > static inline bool has_ext(DisasContext *ctx, uint32_t ext) > @@ -1166,11 +1171,44 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED); > ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED); > ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); > + ctx->bcfi_enabled = cpu_get_bcfien(env); > + ctx->fcfi_enabled = cpu_get_fcfien(env); This is wrong. If you ctx->bcfi_enabled in the translation and don't put it in a tb flags field, the translated tb will be misused. Zhiwei > + ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED); > ctx->zero = tcg_constant_tl(0); > } > > static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) > { > + DisasContext *ctx = container_of(db, DisasContext, base); > + > + if (ctx->fcfi_lp_expected) { > + /* > + * Since we can't look ahead to confirm that the first > + * instruction is a legal landing pad instruction, emit > + * compare-and-branch sequence that will be fixed-up in > + * riscv_tr_tb_stop() to either statically hit or skip an > + * illegal instruction exception depending on whether the > + * flag was lowered by translation of a CJLP or JLP as > + * the first instruction in the block. > + */ > + TCGv_i32 immediate; > + TCGLabel *l; > + l = gen_new_label(); > + immediate = tcg_temp_local_new_i32(); > + tcg_gen_movi_i32(immediate, 0); > + cfi_lp_check = tcg_last_op(); > + tcg_gen_brcondi_i32(TCG_COND_EQ, immediate, 0, l); > + tcg_temp_free_i32(immediate); > + gen_exception_illegal(ctx); > + gen_set_label(l); > + /* > + * Despite the use of gen_exception_illegal(), the rest of > + * the TB needs to be generated. The TCG optimizer will > + * clean things up depending on which path ends up being > + * active. > + */ > + ctx->base.is_jmp = DISAS_NEXT; > + } > } > > static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) > @@ -1225,6 +1263,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) > { > DisasContext *ctx = container_of(dcbase, DisasContext, base); > + CPURISCVState *env = cpu->env_ptr; > > switch (ctx->base.is_jmp) { > case DISAS_TOO_MANY: > @@ -1235,6 +1274,19 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) > default: > g_assert_not_reached(); > } > + > + if (ctx->fcfi_lp_expected) { > + /* > + * If the "lp expected" flag is still up, the block needs to take an > + * illegal instruction exception. > + */ > + tcg_set_insn_param(cfi_lp_check, 1, tcgv_i32_arg(tcg_constant_i32(1))); > + } else { > + /* > + * LP instruction requirement was met, clear up LP expected > + */ > + env->elp = NO_LP_EXPECTED; > + } > } > > static void riscv_tr_disas_log(const DisasContextBase *dcbase,
On Wed, Feb 15, 2023 at 12:55 AM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: > > > On 2023/2/9 14:24, Deepak Gupta wrote: > > zisslpcfi protects forward control flow (if enabled) by enforcing all > > indirect call and jmp must land on a landing pad instruction `lpcll` > > short for landing pad and check lower label value. If target of an > > indirect call or jmp is not `lpcll` then cpu/hart must raise an illegal > > instruction exception. > > > > This patch implements the mechanism using TCG. Target architecture branch > > instruction must define the end of a TB. Using this property, during > > translation of branch instruction, TB flag = FCFI_LP_EXPECTED can be set. > > Translation of target TB can check if FCFI_LP_EXPECTED flag is set and a > > flag (fcfi_lp_expected) can be set in DisasContext. If `lpcll` gets > > translated, fcfi_lp_expected flag in DisasContext can be cleared. Else > > it'll fault. > > > > This patch also also adds flag for forward and backward cfi in > > DisasContext. > > > > Signed-off-by: Deepak Gupta <debug@rivosinc.com> > > Signed-off-by: Kip Walker <kip@rivosinc.com> > > --- > > target/riscv/cpu.h | 3 +++ > > target/riscv/cpu_helper.c | 12 +++++++++ > > target/riscv/translate.c | 52 +++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 67 insertions(+) > > > > diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > > index 8803ea6426..98b272bcad 100644 > > --- a/target/riscv/cpu.h > > +++ b/target/riscv/cpu.h > > @@ -644,6 +644,9 @@ FIELD(TB_FLAGS, VMA, 25, 1) > > /* Native debug itrigger */ > > FIELD(TB_FLAGS, ITRIGGER, 26, 1) > > > > +/* Zisslpcfi needs a TB flag to track indirect branches */ > > +FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 27, 1) > > + > > #ifdef TARGET_RISCV32 > > #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32) > > #else > > diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > > index 63377abc2f..d15918f534 100644 > > --- a/target/riscv/cpu_helper.c > > +++ b/target/riscv/cpu_helper.c > > @@ -129,6 +129,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > > flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1); > > } > > > > + if (cpu->cfg.ext_cfi) { > > + /* > > + * For Forward CFI, only the expectation of a lpcll at > > + * the start of the block is tracked (which can only happen > > + * when FCFI is enabled for the current processor mode). A jump > > + * or call at the end of the previous TB will have updated > > + * env->elp to indicate the expectation. > > + */ > > + flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, > > + env->elp != NO_LP_EXPECTED); > > + } > > + > > #ifdef CONFIG_USER_ONLY > > flags |= TB_FLAGS_MSTATUS_FS; > > flags |= TB_FLAGS_MSTATUS_VS; > > diff --git a/target/riscv/translate.c b/target/riscv/translate.c > > index df38db7553..7d43d20fc3 100644 > > --- a/target/riscv/translate.c > > +++ b/target/riscv/translate.c > > @@ -41,6 +41,7 @@ static TCGv load_val; > > /* globals for PM CSRs */ > > static TCGv pm_mask; > > static TCGv pm_base; > > +static TCGOp *cfi_lp_check; > > > > #include "exec/gen-icount.h" > > > > @@ -116,6 +117,10 @@ typedef struct DisasContext { > > bool itrigger; > > /* TCG of the current insn_start */ > > TCGOp *insn_start; > > + /* CFI extension */ > > + bool bcfi_enabled; > > + bool fcfi_enabled; > > + bool fcfi_lp_expected; > > } DisasContext; > > > > static inline bool has_ext(DisasContext *ctx, uint32_t ext) > > @@ -1166,11 +1171,44 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > > ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED); > > ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED); > > ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); > > + ctx->bcfi_enabled = cpu_get_bcfien(env); > > + ctx->fcfi_enabled = cpu_get_fcfien(env); > This is wrong. If you ctx->bcfi_enabled in the translation and don't > put it in a tb flags field, the translated tb will > be misused. TLB for shadow stack index is flushed on privilege transfers. All TLBs is flushed whenever enable/disable bits for shadow stack are toggled. Can you elaborate a bit more how this can be misused? > > > Zhiwei > > > + ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED); > > ctx->zero = tcg_constant_tl(0); > > } > > > > static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) > > { > > + DisasContext *ctx = container_of(db, DisasContext, base); > > + > > + if (ctx->fcfi_lp_expected) { > > + /* > > + * Since we can't look ahead to confirm that the first > > + * instruction is a legal landing pad instruction, emit > > + * compare-and-branch sequence that will be fixed-up in > > + * riscv_tr_tb_stop() to either statically hit or skip an > > + * illegal instruction exception depending on whether the > > + * flag was lowered by translation of a CJLP or JLP as > > + * the first instruction in the block. > > + */ > > + TCGv_i32 immediate; > > + TCGLabel *l; > > + l = gen_new_label(); > > + immediate = tcg_temp_local_new_i32(); > > + tcg_gen_movi_i32(immediate, 0); > > + cfi_lp_check = tcg_last_op(); > > + tcg_gen_brcondi_i32(TCG_COND_EQ, immediate, 0, l); > > + tcg_temp_free_i32(immediate); > > + gen_exception_illegal(ctx); > > + gen_set_label(l); > > + /* > > + * Despite the use of gen_exception_illegal(), the rest of > > + * the TB needs to be generated. The TCG optimizer will > > + * clean things up depending on which path ends up being > > + * active. > > + */ > > + ctx->base.is_jmp = DISAS_NEXT; > > + } > > } > > > > static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) > > @@ -1225,6 +1263,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > > static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) > > { > > DisasContext *ctx = container_of(dcbase, DisasContext, base); > > + CPURISCVState *env = cpu->env_ptr; > > > > switch (ctx->base.is_jmp) { > > case DISAS_TOO_MANY: > > @@ -1235,6 +1274,19 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) > > default: > > g_assert_not_reached(); > > } > > + > > + if (ctx->fcfi_lp_expected) { > > + /* > > + * If the "lp expected" flag is still up, the block needs to take an > > + * illegal instruction exception. > > + */ > > + tcg_set_insn_param(cfi_lp_check, 1, tcgv_i32_arg(tcg_constant_i32(1))); > > + } else { > > + /* > > + * LP instruction requirement was met, clear up LP expected > > + */ > > + env->elp = NO_LP_EXPECTED; > > + } > > } > > > > static void riscv_tr_disas_log(const DisasContextBase *dcbase,
On 2023/2/16 8:02, Deepak Gupta wrote: > On Wed, Feb 15, 2023 at 12:55 AM LIU Zhiwei > <zhiwei_liu@linux.alibaba.com> wrote: >> >> On 2023/2/9 14:24, Deepak Gupta wrote: >>> zisslpcfi protects forward control flow (if enabled) by enforcing all >>> indirect call and jmp must land on a landing pad instruction `lpcll` >>> short for landing pad and check lower label value. If target of an >>> indirect call or jmp is not `lpcll` then cpu/hart must raise an illegal >>> instruction exception. >>> >>> This patch implements the mechanism using TCG. Target architecture branch >>> instruction must define the end of a TB. Using this property, during >>> translation of branch instruction, TB flag = FCFI_LP_EXPECTED can be set. >>> Translation of target TB can check if FCFI_LP_EXPECTED flag is set and a >>> flag (fcfi_lp_expected) can be set in DisasContext. If `lpcll` gets >>> translated, fcfi_lp_expected flag in DisasContext can be cleared. Else >>> it'll fault. >>> >>> This patch also also adds flag for forward and backward cfi in >>> DisasContext. >>> >>> Signed-off-by: Deepak Gupta <debug@rivosinc.com> >>> Signed-off-by: Kip Walker <kip@rivosinc.com> >>> --- >>> target/riscv/cpu.h | 3 +++ >>> target/riscv/cpu_helper.c | 12 +++++++++ >>> target/riscv/translate.c | 52 +++++++++++++++++++++++++++++++++++++++ >>> 3 files changed, 67 insertions(+) >>> >>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h >>> index 8803ea6426..98b272bcad 100644 >>> --- a/target/riscv/cpu.h >>> +++ b/target/riscv/cpu.h >>> @@ -644,6 +644,9 @@ FIELD(TB_FLAGS, VMA, 25, 1) >>> /* Native debug itrigger */ >>> FIELD(TB_FLAGS, ITRIGGER, 26, 1) >>> >>> +/* Zisslpcfi needs a TB flag to track indirect branches */ >>> +FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 27, 1) >>> + >>> #ifdef TARGET_RISCV32 >>> #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32) >>> #else >>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c >>> index 63377abc2f..d15918f534 100644 >>> --- a/target/riscv/cpu_helper.c >>> +++ b/target/riscv/cpu_helper.c >>> @@ -129,6 +129,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, >>> flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1); >>> } >>> >>> + if (cpu->cfg.ext_cfi) { >>> + /* >>> + * For Forward CFI, only the expectation of a lpcll at >>> + * the start of the block is tracked (which can only happen >>> + * when FCFI is enabled for the current processor mode). A jump >>> + * or call at the end of the previous TB will have updated >>> + * env->elp to indicate the expectation. >>> + */ >>> + flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, >>> + env->elp != NO_LP_EXPECTED); >>> + } >>> + >>> #ifdef CONFIG_USER_ONLY >>> flags |= TB_FLAGS_MSTATUS_FS; >>> flags |= TB_FLAGS_MSTATUS_VS; >>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c >>> index df38db7553..7d43d20fc3 100644 >>> --- a/target/riscv/translate.c >>> +++ b/target/riscv/translate.c >>> @@ -41,6 +41,7 @@ static TCGv load_val; >>> /* globals for PM CSRs */ >>> static TCGv pm_mask; >>> static TCGv pm_base; >>> +static TCGOp *cfi_lp_check; >>> >>> #include "exec/gen-icount.h" >>> >>> @@ -116,6 +117,10 @@ typedef struct DisasContext { >>> bool itrigger; >>> /* TCG of the current insn_start */ >>> TCGOp *insn_start; >>> + /* CFI extension */ >>> + bool bcfi_enabled; >>> + bool fcfi_enabled; >>> + bool fcfi_lp_expected; >>> } DisasContext; >>> >>> static inline bool has_ext(DisasContext *ctx, uint32_t ext) >>> @@ -1166,11 +1171,44 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) >>> ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED); >>> ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED); >>> ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); >>> + ctx->bcfi_enabled = cpu_get_bcfien(env); >>> + ctx->fcfi_enabled = cpu_get_fcfien(env); >> This is wrong. If you ctx->bcfi_enabled in the translation and don't >> put it in a tb flags field, the translated tb will >> be misused. > TLB for shadow stack index is flushed on privilege transfers. > All TLBs is flushed whenever enable/disable bits for shadow stack are toggled. > Can you elaborate a bit more how this can be misused? As Richard has pointed out, you need put this fields into tb flags if you want to use them in translation process as constants. Nothing to do with TLB. Tb flags will always be calculated (Except the direct block chain I think, is it right? @Richard) before using the translated tb. So if your translated tb depend on some machine states, you should put it in tb flags. Otherwise, the translated tb will be misused, as its execution environment varies from its translation machine states. Zhiwei > >> >> Zhiwei >> >>> + ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED); >>> ctx->zero = tcg_constant_tl(0); >>> } >>> >>> static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) >>> { >>> + DisasContext *ctx = container_of(db, DisasContext, base); >>> + >>> + if (ctx->fcfi_lp_expected) { >>> + /* >>> + * Since we can't look ahead to confirm that the first >>> + * instruction is a legal landing pad instruction, emit >>> + * compare-and-branch sequence that will be fixed-up in >>> + * riscv_tr_tb_stop() to either statically hit or skip an >>> + * illegal instruction exception depending on whether the >>> + * flag was lowered by translation of a CJLP or JLP as >>> + * the first instruction in the block. >>> + */ >>> + TCGv_i32 immediate; >>> + TCGLabel *l; >>> + l = gen_new_label(); >>> + immediate = tcg_temp_local_new_i32(); >>> + tcg_gen_movi_i32(immediate, 0); >>> + cfi_lp_check = tcg_last_op(); >>> + tcg_gen_brcondi_i32(TCG_COND_EQ, immediate, 0, l); >>> + tcg_temp_free_i32(immediate); >>> + gen_exception_illegal(ctx); >>> + gen_set_label(l); >>> + /* >>> + * Despite the use of gen_exception_illegal(), the rest of >>> + * the TB needs to be generated. The TCG optimizer will >>> + * clean things up depending on which path ends up being >>> + * active. >>> + */ >>> + ctx->base.is_jmp = DISAS_NEXT; >>> + } >>> } >>> >>> static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) >>> @@ -1225,6 +1263,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) >>> static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) >>> { >>> DisasContext *ctx = container_of(dcbase, DisasContext, base); >>> + CPURISCVState *env = cpu->env_ptr; >>> >>> switch (ctx->base.is_jmp) { >>> case DISAS_TOO_MANY: >>> @@ -1235,6 +1274,19 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) >>> default: >>> g_assert_not_reached(); >>> } >>> + >>> + if (ctx->fcfi_lp_expected) { >>> + /* >>> + * If the "lp expected" flag is still up, the block needs to take an >>> + * illegal instruction exception. >>> + */ >>> + tcg_set_insn_param(cfi_lp_check, 1, tcgv_i32_arg(tcg_constant_i32(1))); >>> + } else { >>> + /* >>> + * LP instruction requirement was met, clear up LP expected >>> + */ >>> + env->elp = NO_LP_EXPECTED; >>> + } >>> } >>> >>> static void riscv_tr_disas_log(const DisasContextBase *dcbase,
On Wed, Feb 15, 2023 at 6:44 PM LIU Zhiwei <zhiwei_liu@linux.alibaba.com> wrote: > > > On 2023/2/16 8:02, Deepak Gupta wrote: > > On Wed, Feb 15, 2023 at 12:55 AM LIU Zhiwei > > <zhiwei_liu@linux.alibaba.com> wrote: > >> > >> On 2023/2/9 14:24, Deepak Gupta wrote: > >>> zisslpcfi protects forward control flow (if enabled) by enforcing all > >>> indirect call and jmp must land on a landing pad instruction `lpcll` > >>> short for landing pad and check lower label value. If target of an > >>> indirect call or jmp is not `lpcll` then cpu/hart must raise an illegal > >>> instruction exception. > >>> > >>> This patch implements the mechanism using TCG. Target architecture branch > >>> instruction must define the end of a TB. Using this property, during > >>> translation of branch instruction, TB flag = FCFI_LP_EXPECTED can be set. > >>> Translation of target TB can check if FCFI_LP_EXPECTED flag is set and a > >>> flag (fcfi_lp_expected) can be set in DisasContext. If `lpcll` gets > >>> translated, fcfi_lp_expected flag in DisasContext can be cleared. Else > >>> it'll fault. > >>> > >>> This patch also also adds flag for forward and backward cfi in > >>> DisasContext. > >>> > >>> Signed-off-by: Deepak Gupta <debug@rivosinc.com> > >>> Signed-off-by: Kip Walker <kip@rivosinc.com> > >>> --- > >>> target/riscv/cpu.h | 3 +++ > >>> target/riscv/cpu_helper.c | 12 +++++++++ > >>> target/riscv/translate.c | 52 +++++++++++++++++++++++++++++++++++++++ > >>> 3 files changed, 67 insertions(+) > >>> > >>> diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h > >>> index 8803ea6426..98b272bcad 100644 > >>> --- a/target/riscv/cpu.h > >>> +++ b/target/riscv/cpu.h > >>> @@ -644,6 +644,9 @@ FIELD(TB_FLAGS, VMA, 25, 1) > >>> /* Native debug itrigger */ > >>> FIELD(TB_FLAGS, ITRIGGER, 26, 1) > >>> > >>> +/* Zisslpcfi needs a TB flag to track indirect branches */ > >>> +FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 27, 1) > >>> + > >>> #ifdef TARGET_RISCV32 > >>> #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32) > >>> #else > >>> diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c > >>> index 63377abc2f..d15918f534 100644 > >>> --- a/target/riscv/cpu_helper.c > >>> +++ b/target/riscv/cpu_helper.c > >>> @@ -129,6 +129,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, > >>> flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1); > >>> } > >>> > >>> + if (cpu->cfg.ext_cfi) { > >>> + /* > >>> + * For Forward CFI, only the expectation of a lpcll at > >>> + * the start of the block is tracked (which can only happen > >>> + * when FCFI is enabled for the current processor mode). A jump > >>> + * or call at the end of the previous TB will have updated > >>> + * env->elp to indicate the expectation. > >>> + */ > >>> + flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, > >>> + env->elp != NO_LP_EXPECTED); > >>> + } > >>> + > >>> #ifdef CONFIG_USER_ONLY > >>> flags |= TB_FLAGS_MSTATUS_FS; > >>> flags |= TB_FLAGS_MSTATUS_VS; > >>> diff --git a/target/riscv/translate.c b/target/riscv/translate.c > >>> index df38db7553..7d43d20fc3 100644 > >>> --- a/target/riscv/translate.c > >>> +++ b/target/riscv/translate.c > >>> @@ -41,6 +41,7 @@ static TCGv load_val; > >>> /* globals for PM CSRs */ > >>> static TCGv pm_mask; > >>> static TCGv pm_base; > >>> +static TCGOp *cfi_lp_check; > >>> > >>> #include "exec/gen-icount.h" > >>> > >>> @@ -116,6 +117,10 @@ typedef struct DisasContext { > >>> bool itrigger; > >>> /* TCG of the current insn_start */ > >>> TCGOp *insn_start; > >>> + /* CFI extension */ > >>> + bool bcfi_enabled; > >>> + bool fcfi_enabled; > >>> + bool fcfi_lp_expected; > >>> } DisasContext; > >>> > >>> static inline bool has_ext(DisasContext *ctx, uint32_t ext) > >>> @@ -1166,11 +1171,44 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) > >>> ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED); > >>> ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED); > >>> ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); > >>> + ctx->bcfi_enabled = cpu_get_bcfien(env); > >>> + ctx->fcfi_enabled = cpu_get_fcfien(env); > >> This is wrong. If you ctx->bcfi_enabled in the translation and don't > >> put it in a tb flags field, the translated tb will > >> be misused. > > TLB for shadow stack index is flushed on privilege transfers. > > All TLBs is flushed whenever enable/disable bits for shadow stack are toggled. > > Can you elaborate a bit more how this can be misused? > As Richard has pointed out, you need put this fields into tb flags if > you want to use them in translation process as constants. > Nothing to do with TLB. > > Tb flags will always be calculated (Except the direct block chain I > think, is it right? @Richard) before using the translated tb. So if your > translated tb depend on some machine states, you > should put it in tb flags. Otherwise, the translated tb will be misused, > as its execution environment varies from its translation machine states. > Yes it's TIL day. Thanks a lot for the review and comments. Really appreciate it. > Zhiwei > > > > >> > >> Zhiwei > >> > >>> + ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED); > >>> ctx->zero = tcg_constant_tl(0); > >>> } > >>> > >>> static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) > >>> { > >>> + DisasContext *ctx = container_of(db, DisasContext, base); > >>> + > >>> + if (ctx->fcfi_lp_expected) { > >>> + /* > >>> + * Since we can't look ahead to confirm that the first > >>> + * instruction is a legal landing pad instruction, emit > >>> + * compare-and-branch sequence that will be fixed-up in > >>> + * riscv_tr_tb_stop() to either statically hit or skip an > >>> + * illegal instruction exception depending on whether the > >>> + * flag was lowered by translation of a CJLP or JLP as > >>> + * the first instruction in the block. > >>> + */ > >>> + TCGv_i32 immediate; > >>> + TCGLabel *l; > >>> + l = gen_new_label(); > >>> + immediate = tcg_temp_local_new_i32(); > >>> + tcg_gen_movi_i32(immediate, 0); > >>> + cfi_lp_check = tcg_last_op(); > >>> + tcg_gen_brcondi_i32(TCG_COND_EQ, immediate, 0, l); > >>> + tcg_temp_free_i32(immediate); > >>> + gen_exception_illegal(ctx); > >>> + gen_set_label(l); > >>> + /* > >>> + * Despite the use of gen_exception_illegal(), the rest of > >>> + * the TB needs to be generated. The TCG optimizer will > >>> + * clean things up depending on which path ends up being > >>> + * active. > >>> + */ > >>> + ctx->base.is_jmp = DISAS_NEXT; > >>> + } > >>> } > >>> > >>> static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) > >>> @@ -1225,6 +1263,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) > >>> static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) > >>> { > >>> DisasContext *ctx = container_of(dcbase, DisasContext, base); > >>> + CPURISCVState *env = cpu->env_ptr; > >>> > >>> switch (ctx->base.is_jmp) { > >>> case DISAS_TOO_MANY: > >>> @@ -1235,6 +1274,19 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) > >>> default: > >>> g_assert_not_reached(); > >>> } > >>> + > >>> + if (ctx->fcfi_lp_expected) { > >>> + /* > >>> + * If the "lp expected" flag is still up, the block needs to take an > >>> + * illegal instruction exception. > >>> + */ > >>> + tcg_set_insn_param(cfi_lp_check, 1, tcgv_i32_arg(tcg_constant_i32(1))); > >>> + } else { > >>> + /* > >>> + * LP instruction requirement was met, clear up LP expected > >>> + */ > >>> + env->elp = NO_LP_EXPECTED; > >>> + } > >>> } > >>> > >>> static void riscv_tr_disas_log(const DisasContextBase *dcbase,
On 2/8/23 20:24, Deepak Gupta wrote: > + if (cpu->cfg.ext_cfi) { > + /* > + * For Forward CFI, only the expectation of a lpcll at > + * the start of the block is tracked (which can only happen > + * when FCFI is enabled for the current processor mode). A jump > + * or call at the end of the previous TB will have updated > + * env->elp to indicate the expectation. > + */ > + flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, > + env->elp != NO_LP_EXPECTED); You should also check cpu_fcfien here. We can completely ignore elp if the feature is disabled. Which means that the tb flag will be set if and only if we require a landing pad. > static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) > { > + DisasContext *ctx = container_of(db, DisasContext, base); > + > + if (ctx->fcfi_lp_expected) { > + /* > + * Since we can't look ahead to confirm that the first > + * instruction is a legal landing pad instruction, emit > + * compare-and-branch sequence that will be fixed-up in > + * riscv_tr_tb_stop() to either statically hit or skip an > + * illegal instruction exception depending on whether the > + * flag was lowered by translation of a CJLP or JLP as > + * the first instruction in the block. You can "look ahead" by deferring this to riscv_tr_translate_insn. Compare target/arm/translate-a64.c, btype_destination_ok and uses thereof. Note that risc-v does not have the same "guarded page" bit that aa64 does. r~
diff --git a/target/riscv/cpu.h b/target/riscv/cpu.h index 8803ea6426..98b272bcad 100644 --- a/target/riscv/cpu.h +++ b/target/riscv/cpu.h @@ -644,6 +644,9 @@ FIELD(TB_FLAGS, VMA, 25, 1) /* Native debug itrigger */ FIELD(TB_FLAGS, ITRIGGER, 26, 1) +/* Zisslpcfi needs a TB flag to track indirect branches */ +FIELD(TB_FLAGS, FCFI_LP_EXPECTED, 27, 1) + #ifdef TARGET_RISCV32 #define riscv_cpu_mxl(env) ((void)(env), MXL_RV32) #else diff --git a/target/riscv/cpu_helper.c b/target/riscv/cpu_helper.c index 63377abc2f..d15918f534 100644 --- a/target/riscv/cpu_helper.c +++ b/target/riscv/cpu_helper.c @@ -129,6 +129,18 @@ void cpu_get_tb_cpu_state(CPURISCVState *env, target_ulong *pc, flags = FIELD_DP32(flags, TB_FLAGS, VILL, 1); } + if (cpu->cfg.ext_cfi) { + /* + * For Forward CFI, only the expectation of a lpcll at + * the start of the block is tracked (which can only happen + * when FCFI is enabled for the current processor mode). A jump + * or call at the end of the previous TB will have updated + * env->elp to indicate the expectation. + */ + flags = FIELD_DP32(flags, TB_FLAGS, FCFI_LP_EXPECTED, + env->elp != NO_LP_EXPECTED); + } + #ifdef CONFIG_USER_ONLY flags |= TB_FLAGS_MSTATUS_FS; flags |= TB_FLAGS_MSTATUS_VS; diff --git a/target/riscv/translate.c b/target/riscv/translate.c index df38db7553..7d43d20fc3 100644 --- a/target/riscv/translate.c +++ b/target/riscv/translate.c @@ -41,6 +41,7 @@ static TCGv load_val; /* globals for PM CSRs */ static TCGv pm_mask; static TCGv pm_base; +static TCGOp *cfi_lp_check; #include "exec/gen-icount.h" @@ -116,6 +117,10 @@ typedef struct DisasContext { bool itrigger; /* TCG of the current insn_start */ TCGOp *insn_start; + /* CFI extension */ + bool bcfi_enabled; + bool fcfi_enabled; + bool fcfi_lp_expected; } DisasContext; static inline bool has_ext(DisasContext *ctx, uint32_t ext) @@ -1166,11 +1171,44 @@ static void riscv_tr_init_disas_context(DisasContextBase *dcbase, CPUState *cs) ctx->pm_mask_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_MASK_ENABLED); ctx->pm_base_enabled = FIELD_EX32(tb_flags, TB_FLAGS, PM_BASE_ENABLED); ctx->itrigger = FIELD_EX32(tb_flags, TB_FLAGS, ITRIGGER); + ctx->bcfi_enabled = cpu_get_bcfien(env); + ctx->fcfi_enabled = cpu_get_fcfien(env); + ctx->fcfi_lp_expected = FIELD_EX32(tb_flags, TB_FLAGS, FCFI_LP_EXPECTED); ctx->zero = tcg_constant_tl(0); } static void riscv_tr_tb_start(DisasContextBase *db, CPUState *cpu) { + DisasContext *ctx = container_of(db, DisasContext, base); + + if (ctx->fcfi_lp_expected) { + /* + * Since we can't look ahead to confirm that the first + * instruction is a legal landing pad instruction, emit + * compare-and-branch sequence that will be fixed-up in + * riscv_tr_tb_stop() to either statically hit or skip an + * illegal instruction exception depending on whether the + * flag was lowered by translation of a CJLP or JLP as + * the first instruction in the block. + */ + TCGv_i32 immediate; + TCGLabel *l; + l = gen_new_label(); + immediate = tcg_temp_local_new_i32(); + tcg_gen_movi_i32(immediate, 0); + cfi_lp_check = tcg_last_op(); + tcg_gen_brcondi_i32(TCG_COND_EQ, immediate, 0, l); + tcg_temp_free_i32(immediate); + gen_exception_illegal(ctx); + gen_set_label(l); + /* + * Despite the use of gen_exception_illegal(), the rest of + * the TB needs to be generated. The TCG optimizer will + * clean things up depending on which path ends up being + * active. + */ + ctx->base.is_jmp = DISAS_NEXT; + } } static void riscv_tr_insn_start(DisasContextBase *dcbase, CPUState *cpu) @@ -1225,6 +1263,7 @@ static void riscv_tr_translate_insn(DisasContextBase *dcbase, CPUState *cpu) static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) { DisasContext *ctx = container_of(dcbase, DisasContext, base); + CPURISCVState *env = cpu->env_ptr; switch (ctx->base.is_jmp) { case DISAS_TOO_MANY: @@ -1235,6 +1274,19 @@ static void riscv_tr_tb_stop(DisasContextBase *dcbase, CPUState *cpu) default: g_assert_not_reached(); } + + if (ctx->fcfi_lp_expected) { + /* + * If the "lp expected" flag is still up, the block needs to take an + * illegal instruction exception. + */ + tcg_set_insn_param(cfi_lp_check, 1, tcgv_i32_arg(tcg_constant_i32(1))); + } else { + /* + * LP instruction requirement was met, clear up LP expected + */ + env->elp = NO_LP_EXPECTED; + } } static void riscv_tr_disas_log(const DisasContextBase *dcbase,