| Message ID | 20231010033701.385725-3-gaosong@loongson.cn |
|---|---|
| State | New |
| Headers | show |
| Series | linux-user/loongarch64: Add LSX/LASX sigcontext | expand |
On 10/9/23 20:36, Song Gao wrote: > Signed-off-by: Song Gao <gaosong@loongson.cn> > --- > target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++ > target/loongarch/internals.h | 2 ++ > 2 files changed, 14 insertions(+) > > diff --git a/target/loongarch/insn_trans/trans_vec.c.inc b/target/loongarch/insn_trans/trans_vec.c.inc > index 98f856bb29..aef16ef44a 100644 > --- a/target/loongarch/insn_trans/trans_vec.c.inc > +++ b/target/loongarch/insn_trans/trans_vec.c.inc > @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, uint32_t oprsz) > > #else > > +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz) > +{ > + if (oprsz == 16) { > + ctx->extctx_flags |= EXTCTX_FLAGS_LSX; > + } > + > + if (oprsz == 32) { > + ctx->extctx_flags |= EXTCTX_FLAGS_LASX; > + } > +} > + > static bool check_vec(DisasContext *ctx, uint32_t oprsz) > { > + set_vec_extctx(ctx, oprsz); > return true; > } This doesn't do anything. Nothing copies the changed value back to env. Anyway, I think this is the wrong way to go about it. If you want to track what the program is using, you should do it exactly like the real kernel: disable the execution unit, have the program trap, and the enable the execution unit when the trap occurs. At this point, CSR_EUEN enable bits contain exactly which units have been used by the program. r~
在 2023/10/29 上午5:40, Richard Henderson 写道: > On 10/9/23 20:36, Song Gao wrote: >> Signed-off-by: Song Gao <gaosong@loongson.cn> >> --- >> target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++ >> target/loongarch/internals.h | 2 ++ >> 2 files changed, 14 insertions(+) >> >> diff --git a/target/loongarch/insn_trans/trans_vec.c.inc >> b/target/loongarch/insn_trans/trans_vec.c.inc >> index 98f856bb29..aef16ef44a 100644 >> --- a/target/loongarch/insn_trans/trans_vec.c.inc >> +++ b/target/loongarch/insn_trans/trans_vec.c.inc >> @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, uint32_t >> oprsz) >> #else >> +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz) >> +{ >> + if (oprsz == 16) { >> + ctx->extctx_flags |= EXTCTX_FLAGS_LSX; >> + } >> + >> + if (oprsz == 32) { >> + ctx->extctx_flags |= EXTCTX_FLAGS_LASX; >> + } >> +} >> + >> static bool check_vec(DisasContext *ctx, uint32_t oprsz) >> { >> + set_vec_extctx(ctx, oprsz); >> return true; >> } > > This doesn't do anything. Nothing copies the changed value back to env. > Anyway, I think this is the wrong way to go about it. > Oh, It is on patch1. @@ -294,6 +296,7 @@ static void loongarch_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) generate_exception(ctx, EXCCODE_INE); } + env->extctx_flags |= ctx->extctx_flags; ctx->base.pc_next += 4; Thanks. Song Gao > If you want to track what the program is using, you should do it > exactly like the real kernel: disable the execution unit, have the > program trap, and the enable the execution unit when the trap occurs. > At this point, CSR_EUEN enable bits contain exactly which units have > been used by the program. > > > r~
On 10/29/23 20:28, gaosong wrote: > 在 2023/10/29 上午5:40, Richard Henderson 写道: >> On 10/9/23 20:36, Song Gao wrote: >>> Signed-off-by: Song Gao <gaosong@loongson.cn> >>> --- >>> target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++ >>> target/loongarch/internals.h | 2 ++ >>> 2 files changed, 14 insertions(+) >>> >>> diff --git a/target/loongarch/insn_trans/trans_vec.c.inc >>> b/target/loongarch/insn_trans/trans_vec.c.inc >>> index 98f856bb29..aef16ef44a 100644 >>> --- a/target/loongarch/insn_trans/trans_vec.c.inc >>> +++ b/target/loongarch/insn_trans/trans_vec.c.inc >>> @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, uint32_t oprsz) >>> #else >>> +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz) >>> +{ >>> + if (oprsz == 16) { >>> + ctx->extctx_flags |= EXTCTX_FLAGS_LSX; >>> + } >>> + >>> + if (oprsz == 32) { >>> + ctx->extctx_flags |= EXTCTX_FLAGS_LASX; >>> + } >>> +} >>> + >>> static bool check_vec(DisasContext *ctx, uint32_t oprsz) >>> { >>> + set_vec_extctx(ctx, oprsz); >>> return true; >>> } >> >> This doesn't do anything. Nothing copies the changed value back to env. >> Anyway, I think this is the wrong way to go about it. >> > Oh, It is on patch1. > > @@ -294,6 +296,7 @@ static void loongarch_tr_translate_insn(DisasContextBase *dcbase, > CPUState *cs) > generate_exception(ctx, EXCCODE_INE); > } > > + env->extctx_flags |= ctx->extctx_flags; Ah, well, this is also incorrect. This copy only happens at translation time, not at execution time. Anyway, I think my previous suggestion is better: >> If you want to track what the program is using, you should do it exactly like the real >> kernel: disable the execution unit, have the program trap, and the enable the execution >> unit when the trap occurs. At this point, CSR_EUEN enable bits contain exactly which >> units have been used by the program. r~
在 2023/10/30 下午11:30, Richard Henderson 写道: > On 10/29/23 20:28, gaosong wrote: >> 在 2023/10/29 上午5:40, Richard Henderson 写道: >>> On 10/9/23 20:36, Song Gao wrote: >>>> Signed-off-by: Song Gao <gaosong@loongson.cn> >>>> --- >>>> target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++ >>>> target/loongarch/internals.h | 2 ++ >>>> 2 files changed, 14 insertions(+) >>>> >>>> diff --git a/target/loongarch/insn_trans/trans_vec.c.inc >>>> b/target/loongarch/insn_trans/trans_vec.c.inc >>>> index 98f856bb29..aef16ef44a 100644 >>>> --- a/target/loongarch/insn_trans/trans_vec.c.inc >>>> +++ b/target/loongarch/insn_trans/trans_vec.c.inc >>>> @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, >>>> uint32_t oprsz) >>>> #else >>>> +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz) >>>> +{ >>>> + if (oprsz == 16) { >>>> + ctx->extctx_flags |= EXTCTX_FLAGS_LSX; >>>> + } >>>> + >>>> + if (oprsz == 32) { >>>> + ctx->extctx_flags |= EXTCTX_FLAGS_LASX; >>>> + } >>>> +} >>>> + >>>> static bool check_vec(DisasContext *ctx, uint32_t oprsz) >>>> { >>>> + set_vec_extctx(ctx, oprsz); >>>> return true; >>>> } >>> >>> This doesn't do anything. Nothing copies the changed value back to >>> env. >>> Anyway, I think this is the wrong way to go about it. >>> >> Oh, It is on patch1. >> >> @@ -294,6 +296,7 @@ static void >> loongarch_tr_translate_insn(DisasContextBase *dcbase, CPUState *cs) >> generate_exception(ctx, EXCCODE_INE); >> } >> >> + env->extctx_flags |= ctx->extctx_flags; > > Ah, well, this is also incorrect. > > This copy only happens at translation time, not at execution time. > > Anyway, I think my previous suggestion is better: > Oh, Could you show more details? I think I didn't get you point. >>> If you want to track what the program is using, you should do it >>> exactly like the real kernel: disable the execution unit, have the >>> program trap, and the enable the execution unit when the trap >>> occurs. At this point, CSR_EUEN enable bits contain exactly which >>> units have been used by the program. > we always enabled LSX/LASX exception, This is mean that we always use target_lasx_context. Thanks. Song Gao > > r~
On 10/30/23 23:16, gaosong wrote: >> Anyway, I think my previous suggestion is better: >> > Oh, Could you show more details? I think I didn't get you point. > >>>> If you want to track what the program is using, you should do it exactly like the real >>>> kernel: disable the execution unit, have the program trap, and the enable the >>>> execution unit when the trap occurs. At this point, CSR_EUEN enable bits contain Untested, but something like this. r~
在 2023/10/31 下午10:50, Richard Henderson 写道: > On 10/30/23 23:16, gaosong wrote: >>> Anyway, I think my previous suggestion is better: >>> >> Oh, Could you show more details? I think I didn't get you point. >> >>>>> If you want to track what the program is using, you should do it >>>>> exactly like the real kernel: disable the execution unit, have the >>>>> program trap, and the enable the execution unit when the trap >>>>> occurs. At this point, CSR_EUEN enable bits contain > > Untested, but something like this. > > Got it . Thank you very much. Thanks. Song Gao > r~ >
diff --git a/target/loongarch/insn_trans/trans_vec.c.inc b/target/loongarch/insn_trans/trans_vec.c.inc index 98f856bb29..aef16ef44a 100644 --- a/target/loongarch/insn_trans/trans_vec.c.inc +++ b/target/loongarch/insn_trans/trans_vec.c.inc @@ -23,8 +23,20 @@ static bool check_vec(DisasContext *ctx, uint32_t oprsz) #else +static void set_vec_extctx(DisasContext *ctx, uint32_t oprsz) +{ + if (oprsz == 16) { + ctx->extctx_flags |= EXTCTX_FLAGS_LSX; + } + + if (oprsz == 32) { + ctx->extctx_flags |= EXTCTX_FLAGS_LASX; + } +} + static bool check_vec(DisasContext *ctx, uint32_t oprsz) { + set_vec_extctx(ctx, oprsz); return true; } diff --git a/target/loongarch/internals.h b/target/loongarch/internals.h index 01d98ac2fc..2efba9b859 100644 --- a/target/loongarch/internals.h +++ b/target/loongarch/internals.h @@ -22,6 +22,8 @@ #define LOONGARCH_HGLOBAL_SHIFT 12 #define EXTCTX_FLAGS_FPU 0b01 +#define EXTCTX_FLAGS_LSX 0b10 +#define EXTCTX_FLAGS_LASX 0b100 void loongarch_translate_init(void);
Signed-off-by: Song Gao <gaosong@loongson.cn> --- target/loongarch/insn_trans/trans_vec.c.inc | 12 ++++++++++++ target/loongarch/internals.h | 2 ++ 2 files changed, 14 insertions(+)