* [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug @ 2021-10-20 12:57 Lucas Mateus Castro (alqotel) 2021-10-20 12:57 ` [PATCH 1/2] target/ppc: Fixed call to deferred exception Lucas Mateus Castro (alqotel) ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: Lucas Mateus Castro (alqotel) @ 2021-10-20 12:57 UTC (permalink / raw) To: qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> The instructions mtfsf, mtfsfi and mtfsb, when called, fail to set the FI bit (bit 46 in the FPSCR) and can set to 1 the reserved bit 52 of the FPSCR, as reported in https://gitlab.com/qemu-project/qemu/-/issues/266 (although the bug report is only for mtfsf, the bug applies to mtfsfi and mtfsb1 as well). These instructions also fail to throw an exception when the exception and enabling bits are set, this can be tested by adding 'prctl(PR_SET_FPEXC, PR_FP_EXC_PRECISE);' before the __builtin_mtfsf call in the test case of the bug report. These patches aim to fix these issues. Lucas Mateus Castro (alqotel) (2): target/ppc: Fixed call to deferred exception target/ppc: ppc_store_fpscr doesn't update bit 52 target/ppc/cpu.c | 2 +- target/ppc/cpu.h | 3 +++ target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ target/ppc/helper.h | 1 + target/ppc/translate/fp-impl.c.inc | 6 ++--- 5 files changed, 49 insertions(+), 4 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-10-20 12:57 [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Lucas Mateus Castro (alqotel) @ 2021-10-20 12:57 ` Lucas Mateus Castro (alqotel) 2021-11-09 16:37 ` Daniel Henrique Barboza 2021-11-10 8:19 ` Mark Cave-Ayland 2021-10-20 12:57 ` [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 Lucas Mateus Castro (alqotel) 2021-10-27 11:49 ` [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Matheus K. Ferst 2 siblings, 2 replies; 14+ messages in thread From: Lucas Mateus Castro (alqotel) @ 2021-10-20 12:57 UTC (permalink / raw) To: qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status after updating the value of FPSCR, but helper_float_check_status checks fp_status and fp_status isn't updated based on FPSCR and since the value of fp_status is reset earlier in the instruction, it's always 0. Because of this helper_float_check_status would change the FI bit to 0 as this bit checks if the last operation was inexact and float_flag_inexact is always 0. These instructions also don't throw exceptions correctly since helper_float_check_status throw exceptions based on fp_status. This commit created a new helper, helper_fpscr_check_status that checks FPSCR value instead of fp_status and checks for a larger variety of exceptions than do_float_check_status. The hardware used to compare QEMU's behavior to, was a Power9. Resolves: https://gitlab.com/qemu-project/qemu/-/issues/266 Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.castro@eldorado.org.br> --- target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ target/ppc/helper.h | 1 + target/ppc/translate/fp-impl.c.inc | 6 ++--- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c index c4896cecc8..f086cb503f 100644 --- a/target/ppc/fpu_helper.c +++ b/target/ppc/fpu_helper.c @@ -414,6 +414,47 @@ void helper_store_fpscr(CPUPPCState *env, uint64_t val, uint32_t nibbles) ppc_store_fpscr(env, val); } +void helper_fpscr_check_status(CPUPPCState *env) +{ + CPUState *cs = env_cpu(env); + target_ulong fpscr = env->fpscr; + int error = 0; + + if ((fpscr & FP_VXSOFT) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXSOFT; + } else if ((fpscr & FP_OX) && (fpscr & FP_OE)) { + error = POWERPC_EXCP_FP_OX; + } else if ((fpscr & FP_UX) && (fpscr & FP_UE)) { + error = POWERPC_EXCP_FP_UX; + } else if ((fpscr & FP_XX) && (fpscr & FP_XE)) { + error = POWERPC_EXCP_FP_XX; + } else if ((fpscr & FP_ZX) && (fpscr & FP_ZE)) { + error = POWERPC_EXCP_FP_ZX; + } else if ((fpscr & FP_VXSNAN) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXSNAN; + } else if ((fpscr & FP_VXISI) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXISI; + } else if ((fpscr & FP_VXIDI) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXIDI; + } else if ((fpscr & FP_VXZDZ) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXZDZ; + } else if ((fpscr & FP_VXIMZ) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXIMZ; + } else if ((fpscr & FP_VXVC) && (fpscr_ve != 0)) { + error = POWERPC_EXCP_FP_VXVC; + } + + if (error) { + cs->exception_index = POWERPC_EXCP_PROGRAM; + env->error_code = error | POWERPC_EXCP_FP; + /* Deferred floating-point exception after target FPSCR update */ + if (fp_exceptions_enabled(env)) { + raise_exception_err_ra(env, cs->exception_index, + env->error_code, GETPC()); + } + } +} + static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) { CPUState *cs = env_cpu(env); diff --git a/target/ppc/helper.h b/target/ppc/helper.h index 4076aa281e..baa3715e73 100644 --- a/target/ppc/helper.h +++ b/target/ppc/helper.h @@ -61,6 +61,7 @@ DEF_HELPER_FLAGS_1(cntlzw32, TCG_CALL_NO_RWG_SE, i32, i32) DEF_HELPER_FLAGS_2(brinc, TCG_CALL_NO_RWG_SE, tl, tl, tl) DEF_HELPER_1(float_check_status, void, env) +DEF_HELPER_1(fpscr_check_status, void, env) DEF_HELPER_1(reset_fpstatus, void, env) DEF_HELPER_2(compute_fprf_float64, void, env, i64) DEF_HELPER_3(store_fpscr, void, env, i64, i32) diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc index 9f7868ee28..0a9b1ecc60 100644 --- a/target/ppc/translate/fp-impl.c.inc +++ b/target/ppc/translate/fp-impl.c.inc @@ -782,7 +782,7 @@ static void gen_mtfsb1(DisasContext *ctx) tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); } /* We can raise a deferred exception */ - gen_helper_float_check_status(cpu_env); + gen_helper_fpscr_check_status(cpu_env); } /* mtfsf */ @@ -818,7 +818,7 @@ static void gen_mtfsf(DisasContext *ctx) tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); } /* We can raise a deferred exception */ - gen_helper_float_check_status(cpu_env); + gen_helper_fpscr_check_status(cpu_env); tcg_temp_free_i64(t1); } @@ -851,7 +851,7 @@ static void gen_mtfsfi(DisasContext *ctx) tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); } /* We can raise a deferred exception */ - gen_helper_float_check_status(cpu_env); + gen_helper_fpscr_check_status(cpu_env); } /*** Floating-point load ***/ -- 2.31.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-10-20 12:57 ` [PATCH 1/2] target/ppc: Fixed call to deferred exception Lucas Mateus Castro (alqotel) @ 2021-11-09 16:37 ` Daniel Henrique Barboza 2021-11-10 6:56 ` Cédric Le Goater 2021-11-10 8:19 ` Mark Cave-Ayland 1 sibling, 1 reply; 14+ messages in thread From: Daniel Henrique Barboza @ 2021-11-09 16:37 UTC (permalink / raw) To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david On 10/20/21 09:57, Lucas Mateus Castro (alqotel) wrote: > From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> > > mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status > after updating the value of FPSCR, but helper_float_check_status > checks fp_status and fp_status isn't updated based on FPSCR and > since the value of fp_status is reset earlier in the instruction, > it's always 0. > > Because of this helper_float_check_status would change the FI bit to 0 > as this bit checks if the last operation was inexact and > float_flag_inexact is always 0. > > These instructions also don't throw exceptions correctly since > helper_float_check_status throw exceptions based on fp_status. > > This commit created a new helper, helper_fpscr_check_status that checks > FPSCR value instead of fp_status and checks for a larger variety of > exceptions than do_float_check_status. > > The hardware used to compare QEMU's behavior to, was a Power9. Extra comma before "was a Power9". Aside from that, LGTM. Thanks, Daniel > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/266 > Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.castro@eldorado.org.br> > --- > target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ > target/ppc/helper.h | 1 + > target/ppc/translate/fp-impl.c.inc | 6 ++--- > 3 files changed, 45 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index c4896cecc8..f086cb503f 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -414,6 +414,47 @@ void helper_store_fpscr(CPUPPCState *env, uint64_t val, uint32_t nibbles) > ppc_store_fpscr(env, val); > } > > +void helper_fpscr_check_status(CPUPPCState *env) > +{ > + CPUState *cs = env_cpu(env); > + target_ulong fpscr = env->fpscr; > + int error = 0; > + > + if ((fpscr & FP_VXSOFT) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXSOFT; > + } else if ((fpscr & FP_OX) && (fpscr & FP_OE)) { > + error = POWERPC_EXCP_FP_OX; > + } else if ((fpscr & FP_UX) && (fpscr & FP_UE)) { > + error = POWERPC_EXCP_FP_UX; > + } else if ((fpscr & FP_XX) && (fpscr & FP_XE)) { > + error = POWERPC_EXCP_FP_XX; > + } else if ((fpscr & FP_ZX) && (fpscr & FP_ZE)) { > + error = POWERPC_EXCP_FP_ZX; > + } else if ((fpscr & FP_VXSNAN) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXSNAN; > + } else if ((fpscr & FP_VXISI) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXISI; > + } else if ((fpscr & FP_VXIDI) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXIDI; > + } else if ((fpscr & FP_VXZDZ) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXZDZ; > + } else if ((fpscr & FP_VXIMZ) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXIMZ; > + } else if ((fpscr & FP_VXVC) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXVC; > + } > + > + if (error) { > + cs->exception_index = POWERPC_EXCP_PROGRAM; > + env->error_code = error | POWERPC_EXCP_FP; > + /* Deferred floating-point exception after target FPSCR update */ > + if (fp_exceptions_enabled(env)) { > + raise_exception_err_ra(env, cs->exception_index, > + env->error_code, GETPC()); > + } > + } > +} > + > static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) > { > CPUState *cs = env_cpu(env); > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > index 4076aa281e..baa3715e73 100644 > --- a/target/ppc/helper.h > +++ b/target/ppc/helper.h > @@ -61,6 +61,7 @@ DEF_HELPER_FLAGS_1(cntlzw32, TCG_CALL_NO_RWG_SE, i32, i32) > DEF_HELPER_FLAGS_2(brinc, TCG_CALL_NO_RWG_SE, tl, tl, tl) > > DEF_HELPER_1(float_check_status, void, env) > +DEF_HELPER_1(fpscr_check_status, void, env) > DEF_HELPER_1(reset_fpstatus, void, env) > DEF_HELPER_2(compute_fprf_float64, void, env, i64) > DEF_HELPER_3(store_fpscr, void, env, i64, i32) > diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc > index 9f7868ee28..0a9b1ecc60 100644 > --- a/target/ppc/translate/fp-impl.c.inc > +++ b/target/ppc/translate/fp-impl.c.inc > @@ -782,7 +782,7 @@ static void gen_mtfsb1(DisasContext *ctx) > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > /* We can raise a deferred exception */ > - gen_helper_float_check_status(cpu_env); > + gen_helper_fpscr_check_status(cpu_env); > } > > /* mtfsf */ > @@ -818,7 +818,7 @@ static void gen_mtfsf(DisasContext *ctx) > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > /* We can raise a deferred exception */ > - gen_helper_float_check_status(cpu_env); > + gen_helper_fpscr_check_status(cpu_env); > tcg_temp_free_i64(t1); > } > > @@ -851,7 +851,7 @@ static void gen_mtfsfi(DisasContext *ctx) > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > /* We can raise a deferred exception */ > - gen_helper_float_check_status(cpu_env); > + gen_helper_fpscr_check_status(cpu_env); > } > > /*** Floating-point load ***/ > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-11-09 16:37 ` Daniel Henrique Barboza @ 2021-11-10 6:56 ` Cédric Le Goater 2021-11-10 17:29 ` Lucas Mateus Martins Araujo e Castro 0 siblings, 1 reply; 14+ messages in thread From: Cédric Le Goater @ 2021-11-10 6:56 UTC (permalink / raw) To: Daniel Henrique Barboza, Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david On 11/9/21 17:37, Daniel Henrique Barboza wrote: > > > On 10/20/21 09:57, Lucas Mateus Castro (alqotel) wrote: >> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> >> >> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status >> after updating the value of FPSCR, but helper_float_check_status >> checks fp_status and fp_status isn't updated based on FPSCR and >> since the value of fp_status is reset earlier in the instruction, >> it's always 0. >> >> Because of this helper_float_check_status would change the FI bit to 0 >> as this bit checks if the last operation was inexact and >> float_flag_inexact is always 0. >> >> These instructions also don't throw exceptions correctly since >> helper_float_check_status throw exceptions based on fp_status. >> >> This commit created a new helper, helper_fpscr_check_status that checks >> FPSCR value instead of fp_status and checks for a larger variety of >> exceptions than do_float_check_status. >> >> The hardware used to compare QEMU's behavior to, was a Power9. Do you have a test case for this ? If so, are you collecting them on some repo ? Thanks, C. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-11-10 6:56 ` Cédric Le Goater @ 2021-11-10 17:29 ` Lucas Mateus Martins Araujo e Castro 2021-11-16 14:43 ` Cédric Le Goater 0 siblings, 1 reply; 14+ messages in thread From: Lucas Mateus Martins Araujo e Castro @ 2021-11-10 17:29 UTC (permalink / raw) To: Cédric Le Goater, Daniel Henrique Barboza, qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david [-- Attachment #1: Type: text/plain, Size: 1796 bytes --] On 10/11/2021 03:56, Cédric Le Goater wrote: > On 11/9/21 17:37, Daniel Henrique Barboza wrote: >> >> >> On 10/20/21 09:57, Lucas Mateus Castro (alqotel) wrote: >>> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> >>> >>> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status >>> after updating the value of FPSCR, but helper_float_check_status >>> checks fp_status and fp_status isn't updated based on FPSCR and >>> since the value of fp_status is reset earlier in the instruction, >>> it's always 0. >>> >>> Because of this helper_float_check_status would change the FI bit to 0 >>> as this bit checks if the last operation was inexact and >>> float_flag_inexact is always 0. >>> >>> These instructions also don't throw exceptions correctly since >>> helper_float_check_status throw exceptions based on fp_status. >>> >>> This commit created a new helper, helper_fpscr_check_status that checks >>> FPSCR value instead of fp_status and checks for a larger variety of >>> exceptions than do_float_check_status. >>> >>> The hardware used to compare QEMU's behavior to, was a Power9. > > Do you have a test case for this ? If so, are you collecting them > on some repo ? > > Thanks, > > C. Just created a test, currently on the branch https://github.com/PPC64/qemu/tree/alqotel_bug_mtfsf commit c8a852bcdf7bdc239711679f00af2450c51d57c6 This test if FI is being set correctly and if the deferred exception is being called correctly (by enabling VE and VXSOFT bits) -- Lucas Mateus M. Araujo e Castro Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> Departamento Computação Embarcada Estagiario Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html> [-- Attachment #2: Type: text/html, Size: 3029 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-11-10 17:29 ` Lucas Mateus Martins Araujo e Castro @ 2021-11-16 14:43 ` Cédric Le Goater 0 siblings, 0 replies; 14+ messages in thread From: Cédric Le Goater @ 2021-11-16 14:43 UTC (permalink / raw) To: Lucas Mateus Martins Araujo e Castro, Daniel Henrique Barboza, qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david On 11/10/21 18:29, Lucas Mateus Martins Araujo e Castro wrote: > > On 10/11/2021 03:56, Cédric Le Goater wrote: >> On 11/9/21 17:37, Daniel Henrique Barboza wrote: >>> >>> >>> On 10/20/21 09:57, Lucas Mateus Castro (alqotel) wrote: >>>> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> >>>> >>>> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status >>>> after updating the value of FPSCR, but helper_float_check_status >>>> checks fp_status and fp_status isn't updated based on FPSCR and >>>> since the value of fp_status is reset earlier in the instruction, >>>> it's always 0. >>>> >>>> Because of this helper_float_check_status would change the FI bit to 0 >>>> as this bit checks if the last operation was inexact and >>>> float_flag_inexact is always 0. >>>> >>>> These instructions also don't throw exceptions correctly since >>>> helper_float_check_status throw exceptions based on fp_status. >>>> >>>> This commit created a new helper, helper_fpscr_check_status that checks >>>> FPSCR value instead of fp_status and checks for a larger variety of >>>> exceptions than do_float_check_status. >>>> >>>> The hardware used to compare QEMU's behavior to, was a Power9. >> >> Do you have a test case for this ? If so, are you collecting them >> on some repo ? >> >> Thanks, >> >> C. > > Just created a test, currently on the branch https://github.com/PPC64/qemu/tree/alqotel_bug_mtfsf commit c8a852bcdf7bdc239711679f00af2450c51d57c6 > > This test if FI is being set correctly and if the deferred exception is being called correctly (by enabling VE and VXSOFT bits) Nice ! May be include in the v2 ? Thanks, C. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-10-20 12:57 ` [PATCH 1/2] target/ppc: Fixed call to deferred exception Lucas Mateus Castro (alqotel) 2021-11-09 16:37 ` Daniel Henrique Barboza @ 2021-11-10 8:19 ` Mark Cave-Ayland 2021-11-10 18:58 ` Lucas Mateus Martins Araujo e Castro 1 sibling, 1 reply; 14+ messages in thread From: Mark Cave-Ayland @ 2021-11-10 8:19 UTC (permalink / raw) To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), danielhb413, richard.henderson, david On 20/10/2021 13:57, Lucas Mateus Castro (alqotel) wrote: > From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> > > mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status > after updating the value of FPSCR, but helper_float_check_status > checks fp_status and fp_status isn't updated based on FPSCR and > since the value of fp_status is reset earlier in the instruction, > it's always 0. > > Because of this helper_float_check_status would change the FI bit to 0 > as this bit checks if the last operation was inexact and > float_flag_inexact is always 0. > > These instructions also don't throw exceptions correctly since > helper_float_check_status throw exceptions based on fp_status. > > This commit created a new helper, helper_fpscr_check_status that checks > FPSCR value instead of fp_status and checks for a larger variety of > exceptions than do_float_check_status. > > The hardware used to compare QEMU's behavior to, was a Power9. > > Resolves: https://gitlab.com/qemu-project/qemu/-/issues/266 > Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.castro@eldorado.org.br> > --- > target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ > target/ppc/helper.h | 1 + > target/ppc/translate/fp-impl.c.inc | 6 ++--- > 3 files changed, 45 insertions(+), 3 deletions(-) > > diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > index c4896cecc8..f086cb503f 100644 > --- a/target/ppc/fpu_helper.c > +++ b/target/ppc/fpu_helper.c > @@ -414,6 +414,47 @@ void helper_store_fpscr(CPUPPCState *env, uint64_t val, uint32_t nibbles) > ppc_store_fpscr(env, val); > } > > +void helper_fpscr_check_status(CPUPPCState *env) > +{ > + CPUState *cs = env_cpu(env); > + target_ulong fpscr = env->fpscr; > + int error = 0; > + > + if ((fpscr & FP_VXSOFT) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXSOFT; > + } else if ((fpscr & FP_OX) && (fpscr & FP_OE)) { > + error = POWERPC_EXCP_FP_OX; > + } else if ((fpscr & FP_UX) && (fpscr & FP_UE)) { > + error = POWERPC_EXCP_FP_UX; > + } else if ((fpscr & FP_XX) && (fpscr & FP_XE)) { > + error = POWERPC_EXCP_FP_XX; > + } else if ((fpscr & FP_ZX) && (fpscr & FP_ZE)) { > + error = POWERPC_EXCP_FP_ZX; > + } else if ((fpscr & FP_VXSNAN) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXSNAN; > + } else if ((fpscr & FP_VXISI) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXISI; > + } else if ((fpscr & FP_VXIDI) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXIDI; > + } else if ((fpscr & FP_VXZDZ) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXZDZ; > + } else if ((fpscr & FP_VXIMZ) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXIMZ; > + } else if ((fpscr & FP_VXVC) && (fpscr_ve != 0)) { > + error = POWERPC_EXCP_FP_VXVC; > + } > + > + if (error) { > + cs->exception_index = POWERPC_EXCP_PROGRAM; > + env->error_code = error | POWERPC_EXCP_FP; > + /* Deferred floating-point exception after target FPSCR update */ > + if (fp_exceptions_enabled(env)) { > + raise_exception_err_ra(env, cs->exception_index, > + env->error_code, GETPC()); > + } > + } > +} > + > static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) > { > CPUState *cs = env_cpu(env); > diff --git a/target/ppc/helper.h b/target/ppc/helper.h > index 4076aa281e..baa3715e73 100644 > --- a/target/ppc/helper.h > +++ b/target/ppc/helper.h > @@ -61,6 +61,7 @@ DEF_HELPER_FLAGS_1(cntlzw32, TCG_CALL_NO_RWG_SE, i32, i32) > DEF_HELPER_FLAGS_2(brinc, TCG_CALL_NO_RWG_SE, tl, tl, tl) > > DEF_HELPER_1(float_check_status, void, env) > +DEF_HELPER_1(fpscr_check_status, void, env) > DEF_HELPER_1(reset_fpstatus, void, env) > DEF_HELPER_2(compute_fprf_float64, void, env, i64) > DEF_HELPER_3(store_fpscr, void, env, i64, i32) > diff --git a/target/ppc/translate/fp-impl.c.inc b/target/ppc/translate/fp-impl.c.inc > index 9f7868ee28..0a9b1ecc60 100644 > --- a/target/ppc/translate/fp-impl.c.inc > +++ b/target/ppc/translate/fp-impl.c.inc > @@ -782,7 +782,7 @@ static void gen_mtfsb1(DisasContext *ctx) > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > /* We can raise a deferred exception */ > - gen_helper_float_check_status(cpu_env); > + gen_helper_fpscr_check_status(cpu_env); > } > > /* mtfsf */ > @@ -818,7 +818,7 @@ static void gen_mtfsf(DisasContext *ctx) > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > /* We can raise a deferred exception */ > - gen_helper_float_check_status(cpu_env); > + gen_helper_fpscr_check_status(cpu_env); > tcg_temp_free_i64(t1); > } > > @@ -851,7 +851,7 @@ static void gen_mtfsfi(DisasContext *ctx) > tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > } > /* We can raise a deferred exception */ > - gen_helper_float_check_status(cpu_env); > + gen_helper_fpscr_check_status(cpu_env); > } > > /*** Floating-point load ***/ FWIW the real issue here is that gen_helper_reset_fpstatus() even exists at all: see the comments around enabling hardfloat in the PPC target by Emilio and Richard at https://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg04974.html and https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg00064.html. I have tried a few informal experiments on my MacOS images by completely removing all calls to gen_reset_fpstatus(), and whilst there were a few odd behaviours I was surprised to find that the basic OS was usable. The main issue I had was trying to come up with suitable test cases for the various instructions when my only available hardware is a G4 Mac Mini. So yes this patch fixes one particular use case, but the real issue is that the PPC target floating point flags need a bit of work: however once this is done it should be possible for hardfloat to be enabled via a CPU option on suitable hosts which will bring a noticeable improvement in floating point performance. ATB, Mark. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-11-10 8:19 ` Mark Cave-Ayland @ 2021-11-10 18:58 ` Lucas Mateus Martins Araujo e Castro 2021-11-10 20:40 ` BALATON Zoltan 0 siblings, 1 reply; 14+ messages in thread From: Lucas Mateus Martins Araujo e Castro @ 2021-11-10 18:58 UTC (permalink / raw) To: Mark Cave-Ayland, qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), danielhb413, richard.henderson, david [-- Attachment #1: Type: text/plain, Size: 9230 bytes --] On 10/11/2021 05:19, Mark Cave-Ayland wrote: > > On 20/10/2021 13:57, Lucas Mateus Castro (alqotel) wrote: > >> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> >> >> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status >> after updating the value of FPSCR, but helper_float_check_status >> checks fp_status and fp_status isn't updated based on FPSCR and >> since the value of fp_status is reset earlier in the instruction, >> it's always 0. >> >> Because of this helper_float_check_status would change the FI bit to 0 >> as this bit checks if the last operation was inexact and >> float_flag_inexact is always 0. >> >> These instructions also don't throw exceptions correctly since >> helper_float_check_status throw exceptions based on fp_status. >> >> This commit created a new helper, helper_fpscr_check_status that checks >> FPSCR value instead of fp_status and checks for a larger variety of >> exceptions than do_float_check_status. >> >> The hardware used to compare QEMU's behavior to, was a Power9. >> >> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/266 >> Signed-off-by: Lucas Mateus Castro (alqotel) >> <lucas.castro@eldorado.org.br> >> --- >> target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ >> target/ppc/helper.h | 1 + >> target/ppc/translate/fp-impl.c.inc | 6 ++--- >> 3 files changed, 45 insertions(+), 3 deletions(-) >> >> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c >> index c4896cecc8..f086cb503f 100644 >> --- a/target/ppc/fpu_helper.c >> +++ b/target/ppc/fpu_helper.c >> @@ -414,6 +414,47 @@ void helper_store_fpscr(CPUPPCState *env, >> uint64_t val, uint32_t nibbles) >> ppc_store_fpscr(env, val); >> } >> >> +void helper_fpscr_check_status(CPUPPCState *env) >> +{ >> + CPUState *cs = env_cpu(env); >> + target_ulong fpscr = env->fpscr; >> + int error = 0; >> + >> + if ((fpscr & FP_VXSOFT) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXSOFT; >> + } else if ((fpscr & FP_OX) && (fpscr & FP_OE)) { >> + error = POWERPC_EXCP_FP_OX; >> + } else if ((fpscr & FP_UX) && (fpscr & FP_UE)) { >> + error = POWERPC_EXCP_FP_UX; >> + } else if ((fpscr & FP_XX) && (fpscr & FP_XE)) { >> + error = POWERPC_EXCP_FP_XX; >> + } else if ((fpscr & FP_ZX) && (fpscr & FP_ZE)) { >> + error = POWERPC_EXCP_FP_ZX; >> + } else if ((fpscr & FP_VXSNAN) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXSNAN; >> + } else if ((fpscr & FP_VXISI) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXISI; >> + } else if ((fpscr & FP_VXIDI) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXIDI; >> + } else if ((fpscr & FP_VXZDZ) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXZDZ; >> + } else if ((fpscr & FP_VXIMZ) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXIMZ; >> + } else if ((fpscr & FP_VXVC) && (fpscr_ve != 0)) { >> + error = POWERPC_EXCP_FP_VXVC; >> + } >> + >> + if (error) { >> + cs->exception_index = POWERPC_EXCP_PROGRAM; >> + env->error_code = error | POWERPC_EXCP_FP; >> + /* Deferred floating-point exception after target FPSCR >> update */ >> + if (fp_exceptions_enabled(env)) { >> + raise_exception_err_ra(env, cs->exception_index, >> + env->error_code, GETPC()); >> + } >> + } >> +} >> + >> static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) >> { >> CPUState *cs = env_cpu(env); >> diff --git a/target/ppc/helper.h b/target/ppc/helper.h >> index 4076aa281e..baa3715e73 100644 >> --- a/target/ppc/helper.h >> +++ b/target/ppc/helper.h >> @@ -61,6 +61,7 @@ DEF_HELPER_FLAGS_1(cntlzw32, TCG_CALL_NO_RWG_SE, >> i32, i32) >> DEF_HELPER_FLAGS_2(brinc, TCG_CALL_NO_RWG_SE, tl, tl, tl) >> >> DEF_HELPER_1(float_check_status, void, env) >> +DEF_HELPER_1(fpscr_check_status, void, env) >> DEF_HELPER_1(reset_fpstatus, void, env) >> DEF_HELPER_2(compute_fprf_float64, void, env, i64) >> DEF_HELPER_3(store_fpscr, void, env, i64, i32) >> diff --git a/target/ppc/translate/fp-impl.c.inc >> b/target/ppc/translate/fp-impl.c.inc >> index 9f7868ee28..0a9b1ecc60 100644 >> --- a/target/ppc/translate/fp-impl.c.inc >> +++ b/target/ppc/translate/fp-impl.c.inc >> @@ -782,7 +782,7 @@ static void gen_mtfsb1(DisasContext *ctx) >> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); >> } >> /* We can raise a deferred exception */ >> - gen_helper_float_check_status(cpu_env); >> + gen_helper_fpscr_check_status(cpu_env); >> } >> >> /* mtfsf */ >> @@ -818,7 +818,7 @@ static void gen_mtfsf(DisasContext *ctx) >> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); >> } >> /* We can raise a deferred exception */ >> - gen_helper_float_check_status(cpu_env); >> + gen_helper_fpscr_check_status(cpu_env); >> tcg_temp_free_i64(t1); >> } >> >> @@ -851,7 +851,7 @@ static void gen_mtfsfi(DisasContext *ctx) >> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); >> } >> /* We can raise a deferred exception */ >> - gen_helper_float_check_status(cpu_env); >> + gen_helper_fpscr_check_status(cpu_env); >> } >> >> /*** Floating-point >> load ***/ > > FWIW the real issue here is that gen_helper_reset_fpstatus() even > exists at all: see > the comments around enabling hardfloat in the PPC target by Emilio and > Richard at > https://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg04974.html > and > https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg00064.html. > > I have tried a few informal experiments on my MacOS images by > completely removing all > calls to gen_reset_fpstatus(), and whilst there were a few odd > behaviours I was > surprised to find that the basic OS was usable. The main issue I had > was trying to > come up with suitable test cases for the various instructions when my > only available > hardware is a G4 Mac Mini. > > So yes this patch fixes one particular use case, but the real issue is > that the PPC > target floating point flags need a bit of work: however once this is > done it should > be possible for hardfloat to be enabled via a CPU option on suitable > hosts which will > bring a noticeable improvement in floating point performance. > In this case I don't think gen_helper_reset_fpstatus() is the problem, fp_status is not updated in the instruction but its value is used in helper_float_check_status(), so if the values have not been reset since the last instruction it'll contain last instruction's information and if it has (either by calling gen_helper_reset_fpstatus(), by automatically doing it every instruction or by having every instruction reset it in the end) it'll have 0. So there are 3 alternatives to solve this that I can think of: * Update FPSCR directly, then update fp_status based on FPSCR, for this you would either have to call a new helper to do this or update helper_store_fpscr to do this, and then expand do_float_check_status to throw more exceptions (or create a new helper to do this if expanding do_float_check_status could cause problems), * Just don't use fp_status, update FPSCR directly and do the deferred exception using only information from FPSCR (the one I used this patch), * Update only fp_status directly and call either a modified do_float_check_status or a new helper that would update FPSCR and throw the correct exception based on fp_status, this one I don't see how it would feasible in the current implementation as FPSCR has many bits without an equivalent in fp_status. So with this I can see how to implement the 1st and 2nd option, I chose not to use the 1st one as do_float_check_status updates the FPSCR then throw the exception, which seemed unnecessary. Also looking back I should've removed gen_reset_fpstatus() as in the way it ended implemented these instructions don't interact with fp_status anywhere else, so I'll remove it in the next version. And looking at the suggestions the current implementation could be changed to take advantage of the optimization suggested in the discussion you linked, specially the parts about checking when exception bits aren't set (but in this case it would've to be the MSR exception bits) and the part about skipping calculating a flag when marked to 1. Thanks for the links. > > ATB, > > Mark. -- Lucas Mateus M. Araujo e Castro Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> Departamento Computação Embarcada Estagiario Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html> [-- Attachment #2: Type: text/html, Size: 13355 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-11-10 18:58 ` Lucas Mateus Martins Araujo e Castro @ 2021-11-10 20:40 ` BALATON Zoltan 2021-11-12 2:31 ` 罗勇刚(Yonggang Luo) 0 siblings, 1 reply; 14+ messages in thread From: BALATON Zoltan @ 2021-11-10 20:40 UTC (permalink / raw) To: Lucas Mateus Martins Araujo e Castro Cc: Lucas Mateus Castro (alqotel), Mark Cave-Ayland, danielhb413, richard.henderson, qemu-devel, qemu-ppc, david [-- Attachment #1: Type: text/plain, Size: 10223 bytes --] On Wed, 10 Nov 2021, Lucas Mateus Martins Araujo e Castro wrote: > On 10/11/2021 05:19, Mark Cave-Ayland wrote: >> On 20/10/2021 13:57, Lucas Mateus Castro (alqotel) wrote: >>> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> >>> >>> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status >>> after updating the value of FPSCR, but helper_float_check_status >>> checks fp_status and fp_status isn't updated based on FPSCR and >>> since the value of fp_status is reset earlier in the instruction, >>> it's always 0. >>> >>> Because of this helper_float_check_status would change the FI bit to 0 >>> as this bit checks if the last operation was inexact and >>> float_flag_inexact is always 0. >>> >>> These instructions also don't throw exceptions correctly since >>> helper_float_check_status throw exceptions based on fp_status. >>> >>> This commit created a new helper, helper_fpscr_check_status that checks >>> FPSCR value instead of fp_status and checks for a larger variety of >>> exceptions than do_float_check_status. >>> >>> The hardware used to compare QEMU's behavior to, was a Power9. >>> >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/266 >>> Signed-off-by: Lucas Mateus Castro (alqotel) >>> <lucas.castro@eldorado.org.br> >>> --- >>> target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ >>> target/ppc/helper.h | 1 + >>> target/ppc/translate/fp-impl.c.inc | 6 ++--- >>> 3 files changed, 45 insertions(+), 3 deletions(-) >>> >>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c >>> index c4896cecc8..f086cb503f 100644 >>> --- a/target/ppc/fpu_helper.c >>> +++ b/target/ppc/fpu_helper.c >>> @@ -414,6 +414,47 @@ void helper_store_fpscr(CPUPPCState *env, uint64_t >>> val, uint32_t nibbles) >>> ppc_store_fpscr(env, val); >>> } >>> >>> +void helper_fpscr_check_status(CPUPPCState *env) >>> +{ >>> + CPUState *cs = env_cpu(env); >>> + target_ulong fpscr = env->fpscr; >>> + int error = 0; >>> + >>> + if ((fpscr & FP_VXSOFT) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXSOFT; >>> + } else if ((fpscr & FP_OX) && (fpscr & FP_OE)) { >>> + error = POWERPC_EXCP_FP_OX; >>> + } else if ((fpscr & FP_UX) && (fpscr & FP_UE)) { >>> + error = POWERPC_EXCP_FP_UX; >>> + } else if ((fpscr & FP_XX) && (fpscr & FP_XE)) { >>> + error = POWERPC_EXCP_FP_XX; >>> + } else if ((fpscr & FP_ZX) && (fpscr & FP_ZE)) { >>> + error = POWERPC_EXCP_FP_ZX; >>> + } else if ((fpscr & FP_VXSNAN) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXSNAN; >>> + } else if ((fpscr & FP_VXISI) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXISI; >>> + } else if ((fpscr & FP_VXIDI) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXIDI; >>> + } else if ((fpscr & FP_VXZDZ) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXZDZ; >>> + } else if ((fpscr & FP_VXIMZ) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXIMZ; >>> + } else if ((fpscr & FP_VXVC) && (fpscr_ve != 0)) { >>> + error = POWERPC_EXCP_FP_VXVC; >>> + } >>> + >>> + if (error) { >>> + cs->exception_index = POWERPC_EXCP_PROGRAM; >>> + env->error_code = error | POWERPC_EXCP_FP; >>> + /* Deferred floating-point exception after target FPSCR update */ >>> + if (fp_exceptions_enabled(env)) { >>> + raise_exception_err_ra(env, cs->exception_index, >>> + env->error_code, GETPC()); >>> + } >>> + } >>> +} >>> + >>> static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) >>> { >>> CPUState *cs = env_cpu(env); >>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h >>> index 4076aa281e..baa3715e73 100644 >>> --- a/target/ppc/helper.h >>> +++ b/target/ppc/helper.h >>> @@ -61,6 +61,7 @@ DEF_HELPER_FLAGS_1(cntlzw32, TCG_CALL_NO_RWG_SE, i32, >>> i32) >>> DEF_HELPER_FLAGS_2(brinc, TCG_CALL_NO_RWG_SE, tl, tl, tl) >>> >>> DEF_HELPER_1(float_check_status, void, env) >>> +DEF_HELPER_1(fpscr_check_status, void, env) >>> DEF_HELPER_1(reset_fpstatus, void, env) >>> DEF_HELPER_2(compute_fprf_float64, void, env, i64) >>> DEF_HELPER_3(store_fpscr, void, env, i64, i32) >>> diff --git a/target/ppc/translate/fp-impl.c.inc >>> b/target/ppc/translate/fp-impl.c.inc >>> index 9f7868ee28..0a9b1ecc60 100644 >>> --- a/target/ppc/translate/fp-impl.c.inc >>> +++ b/target/ppc/translate/fp-impl.c.inc >>> @@ -782,7 +782,7 @@ static void gen_mtfsb1(DisasContext *ctx) >>> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); >>> } >>> /* We can raise a deferred exception */ >>> - gen_helper_float_check_status(cpu_env); >>> + gen_helper_fpscr_check_status(cpu_env); >>> } >>> >>> /* mtfsf */ >>> @@ -818,7 +818,7 @@ static void gen_mtfsf(DisasContext *ctx) >>> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); >>> } >>> /* We can raise a deferred exception */ >>> - gen_helper_float_check_status(cpu_env); >>> + gen_helper_fpscr_check_status(cpu_env); >>> tcg_temp_free_i64(t1); >>> } >>> >>> @@ -851,7 +851,7 @@ static void gen_mtfsfi(DisasContext *ctx) >>> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); >>> } >>> /* We can raise a deferred exception */ >>> - gen_helper_float_check_status(cpu_env); >>> + gen_helper_fpscr_check_status(cpu_env); >>> } >>> >>> /*** Floating-point >>> load ***/ >> >> FWIW the real issue here is that gen_helper_reset_fpstatus() even exists at >> all: see >> the comments around enabling hardfloat in the PPC target by Emilio and >> Richard at >> https://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg04974.html and >> https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg00064.html. >> >> I have tried a few informal experiments on my MacOS images by completely >> removing all >> calls to gen_reset_fpstatus(), and whilst there were a few odd behaviours I >> was >> surprised to find that the basic OS was usable. The main issue I had was >> trying to >> come up with suitable test cases for the various instructions when my only >> available >> hardware is a G4 Mac Mini. >> >> So yes this patch fixes one particular use case, but the real issue is that >> the PPC >> target floating point flags need a bit of work: however once this is done >> it should >> be possible for hardfloat to be enabled via a CPU option on suitable hosts >> which will >> bring a noticeable improvement in floating point performance. >> > In this case I don't think gen_helper_reset_fpstatus() is the problem, > fp_status is not updated in the instruction but its value is used in > helper_float_check_status(), so if the values have not been reset since the > last instruction it'll contain last instruction's information and if it has > (either by calling gen_helper_reset_fpstatus(), by automatically doing it > every instruction or by having every instruction reset it in the end) it'll > have 0. So there are 3 alternatives to solve this that I can think of: > > * Update FPSCR directly, then update fp_status based on FPSCR, for this > you would either have to call a new helper to do this or update > helper_store_fpscr to do this, and then expand do_float_check_status to throw > more exceptions (or create a new helper to do this if expanding > do_float_check_status could cause problems), > > * Just don't use fp_status, update FPSCR directly and do the deferred > exception using only information from FPSCR (the one I used this patch), > > * Update only fp_status directly and call either a modified > do_float_check_status or a new helper that would update FPSCR and throw the > correct exception based on fp_status, this one I don't see how it would > feasible in the current implementation as FPSCR has many bits without an > equivalent in fp_status. > > So with this I can see how to implement the 1st and 2nd option, I chose not > to use the 1st one as do_float_check_status updates the FPSCR then throw the > exception, which seemed unnecessary. Also looking back I should've removed > gen_reset_fpstatus() as in the way it ended implemented these instructions > don't interact with fp_status anywhere else, so I'll remove it in the next > version. > > And looking at the suggestions the current implementation could be changed to > take advantage of the optimization suggested in the discussion you linked, > specially the parts about checking when exception bits aren't set (but in > this case it would've to be the MSR exception bits) and the part about > skipping calculating a flag when marked to 1. I haven't followed the discussion but here's another message with some links I've collected when FPU came up that may be relevant to the topic: https://lists.nongnu.org/archive/html/qemu-ppc/2020-04/msg00387.html among those is a long thread on patchwork that has some info on the current situation. As far as I remember the oddity in handling FPU exceptions is partly because of two bits FI and FR in FPSCR that should reflect the result of the previous FPU op so has to be updated after every op which makes it hard to emulate as other CPUs usually don't do this. (We could easily improve it if we did not emulate those bits, most guest code don't use them anyway, but QEMU prefers accuracy so that way was ruled out.) Other than that the current code maybe also can be simplified and maybe optimised via some other ways which were discussed in those threads but nobody implemented any of the ideas so far. May worth reading through what was said before as there might be sume useful ideas in there. Regards, BALATON Zoltan ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] target/ppc: Fixed call to deferred exception 2021-11-10 20:40 ` BALATON Zoltan @ 2021-11-12 2:31 ` 罗勇刚(Yonggang Luo) 0 siblings, 0 replies; 14+ messages in thread From: 罗勇刚(Yonggang Luo) @ 2021-11-12 2:31 UTC (permalink / raw) To: BALATON Zoltan Cc: Lucas Mateus Castro (alqotel), Mark Cave-Ayland, danielhb413, Richard Henderson, qemu-level, Lucas Mateus Martins Araujo e Castro, qemu-ppc, David Gibson [-- Attachment #1: Type: text/plain, Size: 10701 bytes --] On Thu, Nov 11, 2021 at 4:42 AM BALATON Zoltan <balaton@eik.bme.hu> wrote: > > On Wed, 10 Nov 2021, Lucas Mateus Martins Araujo e Castro wrote: > > On 10/11/2021 05:19, Mark Cave-Ayland wrote: > >> On 20/10/2021 13:57, Lucas Mateus Castro (alqotel) wrote: > >>> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> > >>> > >>> mtfsf, mtfsfi and mtfsb1 instructions call helper_float_check_status > >>> after updating the value of FPSCR, but helper_float_check_status > >>> checks fp_status and fp_status isn't updated based on FPSCR and > >>> since the value of fp_status is reset earlier in the instruction, > >>> it's always 0. > >>> > >>> Because of this helper_float_check_status would change the FI bit to 0 > >>> as this bit checks if the last operation was inexact and > >>> float_flag_inexact is always 0. > >>> > >>> These instructions also don't throw exceptions correctly since > >>> helper_float_check_status throw exceptions based on fp_status. > >>> > >>> This commit created a new helper, helper_fpscr_check_status that checks > >>> FPSCR value instead of fp_status and checks for a larger variety of > >>> exceptions than do_float_check_status. > >>> > >>> The hardware used to compare QEMU's behavior to, was a Power9. > >>> > >>> Resolves: https://gitlab.com/qemu-project/qemu/-/issues/266 > >>> Signed-off-by: Lucas Mateus Castro (alqotel) > >>> <lucas.castro@eldorado.org.br> > >>> --- > >>> target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ > >>> target/ppc/helper.h | 1 + > >>> target/ppc/translate/fp-impl.c.inc | 6 ++--- > >>> 3 files changed, 45 insertions(+), 3 deletions(-) > >>> > >>> diff --git a/target/ppc/fpu_helper.c b/target/ppc/fpu_helper.c > >>> index c4896cecc8..f086cb503f 100644 > >>> --- a/target/ppc/fpu_helper.c > >>> +++ b/target/ppc/fpu_helper.c > >>> @@ -414,6 +414,47 @@ void helper_store_fpscr(CPUPPCState *env, uint64_t > >>> val, uint32_t nibbles) > >>> ppc_store_fpscr(env, val); > >>> } > >>> > >>> +void helper_fpscr_check_status(CPUPPCState *env) > >>> +{ > >>> + CPUState *cs = env_cpu(env); > >>> + target_ulong fpscr = env->fpscr; > >>> + int error = 0; > >>> + > >>> + if ((fpscr & FP_VXSOFT) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXSOFT; > >>> + } else if ((fpscr & FP_OX) && (fpscr & FP_OE)) { > >>> + error = POWERPC_EXCP_FP_OX; > >>> + } else if ((fpscr & FP_UX) && (fpscr & FP_UE)) { > >>> + error = POWERPC_EXCP_FP_UX; > >>> + } else if ((fpscr & FP_XX) && (fpscr & FP_XE)) { > >>> + error = POWERPC_EXCP_FP_XX; > >>> + } else if ((fpscr & FP_ZX) && (fpscr & FP_ZE)) { > >>> + error = POWERPC_EXCP_FP_ZX; > >>> + } else if ((fpscr & FP_VXSNAN) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXSNAN; > >>> + } else if ((fpscr & FP_VXISI) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXISI; > >>> + } else if ((fpscr & FP_VXIDI) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXIDI; > >>> + } else if ((fpscr & FP_VXZDZ) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXZDZ; > >>> + } else if ((fpscr & FP_VXIMZ) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXIMZ; > >>> + } else if ((fpscr & FP_VXVC) && (fpscr_ve != 0)) { > >>> + error = POWERPC_EXCP_FP_VXVC; > >>> + } > >>> + > >>> + if (error) { > >>> + cs->exception_index = POWERPC_EXCP_PROGRAM; > >>> + env->error_code = error | POWERPC_EXCP_FP; > >>> + /* Deferred floating-point exception after target FPSCR update */ > >>> + if (fp_exceptions_enabled(env)) { > >>> + raise_exception_err_ra(env, cs->exception_index, > >>> + env->error_code, GETPC()); > >>> + } > >>> + } > >>> +} > >>> + > >>> static void do_float_check_status(CPUPPCState *env, uintptr_t raddr) > >>> { > >>> CPUState *cs = env_cpu(env); > >>> diff --git a/target/ppc/helper.h b/target/ppc/helper.h > >>> index 4076aa281e..baa3715e73 100644 > >>> --- a/target/ppc/helper.h > >>> +++ b/target/ppc/helper.h > >>> @@ -61,6 +61,7 @@ DEF_HELPER_FLAGS_1(cntlzw32, TCG_CALL_NO_RWG_SE, i32, > >>> i32) > >>> DEF_HELPER_FLAGS_2(brinc, TCG_CALL_NO_RWG_SE, tl, tl, tl) > >>> > >>> DEF_HELPER_1(float_check_status, void, env) > >>> +DEF_HELPER_1(fpscr_check_status, void, env) > >>> DEF_HELPER_1(reset_fpstatus, void, env) > >>> DEF_HELPER_2(compute_fprf_float64, void, env, i64) > >>> DEF_HELPER_3(store_fpscr, void, env, i64, i32) > >>> diff --git a/target/ppc/translate/fp-impl.c.inc > >>> b/target/ppc/translate/fp-impl.c.inc > >>> index 9f7868ee28..0a9b1ecc60 100644 > >>> --- a/target/ppc/translate/fp-impl.c.inc > >>> +++ b/target/ppc/translate/fp-impl.c.inc > >>> @@ -782,7 +782,7 @@ static void gen_mtfsb1(DisasContext *ctx) > >>> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > >>> } > >>> /* We can raise a deferred exception */ > >>> - gen_helper_float_check_status(cpu_env); > >>> + gen_helper_fpscr_check_status(cpu_env); > >>> } > >>> > >>> /* mtfsf */ > >>> @@ -818,7 +818,7 @@ static void gen_mtfsf(DisasContext *ctx) > >>> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > >>> } > >>> /* We can raise a deferred exception */ > >>> - gen_helper_float_check_status(cpu_env); > >>> + gen_helper_fpscr_check_status(cpu_env); > >>> tcg_temp_free_i64(t1); > >>> } > >>> > >>> @@ -851,7 +851,7 @@ static void gen_mtfsfi(DisasContext *ctx) > >>> tcg_gen_shri_i32(cpu_crf[1], cpu_crf[1], FPSCR_OX); > >>> } > >>> /* We can raise a deferred exception */ > >>> - gen_helper_float_check_status(cpu_env); > >>> + gen_helper_fpscr_check_status(cpu_env); > >>> } > >>> > >>> /*** Floating-point > >>> load ***/ > >> > >> FWIW the real issue here is that gen_helper_reset_fpstatus() even exists at > >> all: see > >> the comments around enabling hardfloat in the PPC target by Emilio and > >> Richard at > >> https://lists.nongnu.org/archive/html/qemu-devel/2018-11/msg04974.html and > >> https://lists.gnu.org/archive/html/qemu-devel/2020-05/msg00064.html. > >> > >> I have tried a few informal experiments on my MacOS images by completely > >> removing all > >> calls to gen_reset_fpstatus(), and whilst there were a few odd behaviours I > >> was > >> surprised to find that the basic OS was usable. The main issue I had was > >> trying to > >> come up with suitable test cases for the various instructions when my only > >> available > >> hardware is a G4 Mac Mini. > >> > >> So yes this patch fixes one particular use case, but the real issue is that > >> the PPC > >> target floating point flags need a bit of work: however once this is done > >> it should > >> be possible for hardfloat to be enabled via a CPU option on suitable hosts > >> which will > >> bring a noticeable improvement in floating point performance. > >> > > In this case I don't think gen_helper_reset_fpstatus() is the problem, > > fp_status is not updated in the instruction but its value is used in > > helper_float_check_status(), so if the values have not been reset since the > > last instruction it'll contain last instruction's information and if it has > > (either by calling gen_helper_reset_fpstatus(), by automatically doing it > > every instruction or by having every instruction reset it in the end) it'll > > have 0. So there are 3 alternatives to solve this that I can think of: > > > > * Update FPSCR directly, then update fp_status based on FPSCR, for this > > you would either have to call a new helper to do this or update > > helper_store_fpscr to do this, and then expand do_float_check_status to throw > > more exceptions (or create a new helper to do this if expanding > > do_float_check_status could cause problems), > > > > * Just don't use fp_status, update FPSCR directly and do the deferred > > exception using only information from FPSCR (the one I used this patch), > > > > * Update only fp_status directly and call either a modified > > do_float_check_status or a new helper that would update FPSCR and throw the > > correct exception based on fp_status, this one I don't see how it would > > feasible in the current implementation as FPSCR has many bits without an > > equivalent in fp_status. > > > > So with this I can see how to implement the 1st and 2nd option, I chose not > > to use the 1st one as do_float_check_status updates the FPSCR then throw the > > exception, which seemed unnecessary. Also looking back I should've removed > > gen_reset_fpstatus() as in the way it ended implemented these instructions > > don't interact with fp_status anywhere else, so I'll remove it in the next > > version. > > > > And looking at the suggestions the current implementation could be changed to > > take advantage of the optimization suggested in the discussion you linked, > > specially the parts about checking when exception bits aren't set (but in > > this case it would've to be the MSR exception bits) and the part about > > skipping calculating a flag when marked to 1. > > I haven't followed the discussion but here's another message with some > links I've collected when FPU came up that may be relevant to the topic: > > https://lists.nongnu.org/archive/html/qemu-ppc/2020-04/msg00387.html > > among those is a long thread on patchwork that has some info on the > current situation. As far as I remember the oddity in handling FPU > exceptions is partly because of two bits FI and FR in FPSCR that should That's correct, the most hard part is to simulate FI and FR bits in FPSCR as that was not provided on other CPU. > reflect the result of the previous FPU op so has to be updated after every > op which makes it hard to emulate as other CPUs usually don't do this. (We > could easily improve it if we did not emulate those bits, most guest code > don't use them anyway, but QEMU prefers accuracy so that way was ruled > out.) Other than that the current code maybe also can be simplified and > maybe optimised via some other ways which were discussed in those threads > but nobody implemented any of the ideas so far. May worth reading through > what was said before as there might be sume useful ideas in there. > > Regards, > BALATON Zoltan -- 此致 礼 罗勇刚 Yours sincerely, Yonggang Luo [-- Attachment #2: Type: text/html, Size: 14236 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 2021-10-20 12:57 [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Lucas Mateus Castro (alqotel) 2021-10-20 12:57 ` [PATCH 1/2] target/ppc: Fixed call to deferred exception Lucas Mateus Castro (alqotel) @ 2021-10-20 12:57 ` Lucas Mateus Castro (alqotel) 2021-11-09 16:44 ` Daniel Henrique Barboza 2021-10-27 11:49 ` [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Matheus K. Ferst 2 siblings, 1 reply; 14+ messages in thread From: Lucas Mateus Castro (alqotel) @ 2021-10-20 12:57 UTC (permalink / raw) To: qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> This commit fixes the difference reported in the bug in the reserved bit 52, it does this by adding this bit to the mask of bits to not be directly altered in the ppc_store_fpscr function (the hardware used to compare to QEMU was a Power9). Although this is a difference reported in the bug, since it's a reserved bit it may be a "don't care" case, as put in the bug report. Looking at the ISA it doesn't explicitly mentions this bit can't be set, like it does for FEX and VX, so I'm unsure if this is necessary. Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.castro@eldorado.org.br> --- target/ppc/cpu.c | 2 +- target/ppc/cpu.h | 3 +++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c index 7ad9bd6044..5c411b32ff 100644 --- a/target/ppc/cpu.c +++ b/target/ppc/cpu.c @@ -112,7 +112,7 @@ static inline void fpscr_set_rounding_mode(CPUPPCState *env) void ppc_store_fpscr(CPUPPCState *env, target_ulong val) { - val &= ~(FP_VX | FP_FEX); + val &= FPSCR_MTFS_MASK; if (val & FPSCR_IX) { val |= FP_VX; } diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h index baa4e7c34d..4b42b281ed 100644 --- a/target/ppc/cpu.h +++ b/target/ppc/cpu.h @@ -736,6 +736,9 @@ enum { FP_VXZDZ | FP_VXIMZ | FP_VXVC | FP_VXSOFT | \ FP_VXSQRT | FP_VXCVI) +/* FPSCR bits that can be set by mtfsf, mtfsfi and mtfsb1 */ +#define FPSCR_MTFS_MASK ~((1ull << 11) | FP_VX | FP_FEX) + /*****************************************************************************/ /* Vector status and control register */ #define VSCR_NJ 16 /* Vector non-java */ -- 2.31.1 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 2021-10-20 12:57 ` [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 Lucas Mateus Castro (alqotel) @ 2021-11-09 16:44 ` Daniel Henrique Barboza 2021-11-10 19:07 ` Lucas Mateus Martins Araujo e Castro 0 siblings, 1 reply; 14+ messages in thread From: Daniel Henrique Barboza @ 2021-11-09 16:44 UTC (permalink / raw) To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david On 10/20/21 09:57, Lucas Mateus Castro (alqotel) wrote: > From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> > > This commit fixes the difference reported in the bug in the reserved > bit 52, it does this by adding this bit to the mask of bits to not be > directly altered in the ppc_store_fpscr function (the hardware used to > compare to QEMU was a Power9). IIUC, "bug" here is related to https://gitlab.com/qemu-project/qemu/-/issues/266, the bug mentioned in the commit msg of the first patch. In that case, you should mention it again in this commit message explicitly. In fact, I also believe that the "Resolves:" tag from the first patch should be moved to this patch instead, given that the bug is only fully fixed after both patches are applied. > > Although this is a difference reported in the bug, since it's a reserved > bit it may be a "don't care" case, as put in the bug report. Looking at > the ISA it doesn't explicitly mentions this bit can't be set, like it > does for FEX and VX, so I'm unsure if this is necessary. > > Signed-off-by: Lucas Mateus Castro (alqotel) <lucas.castro@eldorado.org.br> > --- > target/ppc/cpu.c | 2 +- > target/ppc/cpu.h | 3 +++ > 2 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c > index 7ad9bd6044..5c411b32ff 100644 > --- a/target/ppc/cpu.c > +++ b/target/ppc/cpu.c > @@ -112,7 +112,7 @@ static inline void fpscr_set_rounding_mode(CPUPPCState *env) > > void ppc_store_fpscr(CPUPPCState *env, target_ulong val) > { > - val &= ~(FP_VX | FP_FEX); > + val &= FPSCR_MTFS_MASK; > if (val & FPSCR_IX) { > val |= FP_VX; > } > diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h > index baa4e7c34d..4b42b281ed 100644 > --- a/target/ppc/cpu.h > +++ b/target/ppc/cpu.h > @@ -736,6 +736,9 @@ enum { > FP_VXZDZ | FP_VXIMZ | FP_VXVC | FP_VXSOFT | \ > FP_VXSQRT | FP_VXCVI) > > +/* FPSCR bits that can be set by mtfsf, mtfsfi and mtfsb1 */ > +#define FPSCR_MTFS_MASK ~((1ull << 11) | FP_VX | FP_FEX) ./scripts/checkpatch.pl is not happy about this line: ERROR: Macros with complex values should be enclosed in parenthesis #44: FILE: target/ppc/cpu.h:763: +#define FPSCR_MTFS_MASK ~((1ull << 11) | FP_VX | FP_FEX) total: 1 errors, 0 warnings, 17 lines checked Thanks, Daniel > + > /*****************************************************************************/ > /* Vector status and control register */ > #define VSCR_NJ 16 /* Vector non-java */ > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 2021-11-09 16:44 ` Daniel Henrique Barboza @ 2021-11-10 19:07 ` Lucas Mateus Martins Araujo e Castro 0 siblings, 0 replies; 14+ messages in thread From: Lucas Mateus Martins Araujo e Castro @ 2021-11-10 19:07 UTC (permalink / raw) To: Daniel Henrique Barboza, qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, david [-- Attachment #1: Type: text/plain, Size: 3372 bytes --] On 09/11/2021 13:44, Daniel Henrique Barboza wrote: > > On 10/20/21 09:57, Lucas Mateus Castro (alqotel) wrote: >> From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> >> >> This commit fixes the difference reported in the bug in the reserved >> bit 52, it does this by adding this bit to the mask of bits to not be >> directly altered in the ppc_store_fpscr function (the hardware used to >> compare to QEMU was a Power9). > > IIUC, "bug" here is related to > https://gitlab.com/qemu-project/qemu/-/issues/266, > the bug mentioned in the commit msg of the first patch. In that case, you > should mention it again in this commit message explicitly. > > In fact, I also believe that the "Resolves:" tag from the first patch > should > be moved to this patch instead, given that the bug is only fully fixed > after > both patches are applied. > Ok I'll change that, originally I put it in the first patch as that patch resolved the part of the bug that could cause problem. > >> >> Although this is a difference reported in the bug, since it's a reserved >> bit it may be a "don't care" case, as put in the bug report. Looking at >> the ISA it doesn't explicitly mentions this bit can't be set, like it >> does for FEX and VX, so I'm unsure if this is necessary. >> >> Signed-off-by: Lucas Mateus Castro (alqotel) >> <lucas.castro@eldorado.org.br> >> --- >> target/ppc/cpu.c | 2 +- >> target/ppc/cpu.h | 3 +++ >> 2 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/target/ppc/cpu.c b/target/ppc/cpu.c >> index 7ad9bd6044..5c411b32ff 100644 >> --- a/target/ppc/cpu.c >> +++ b/target/ppc/cpu.c >> @@ -112,7 +112,7 @@ static inline void >> fpscr_set_rounding_mode(CPUPPCState *env) >> >> void ppc_store_fpscr(CPUPPCState *env, target_ulong val) >> { >> - val &= ~(FP_VX | FP_FEX); >> + val &= FPSCR_MTFS_MASK; >> if (val & FPSCR_IX) { >> val |= FP_VX; >> } >> diff --git a/target/ppc/cpu.h b/target/ppc/cpu.h >> index baa4e7c34d..4b42b281ed 100644 >> --- a/target/ppc/cpu.h >> +++ b/target/ppc/cpu.h >> @@ -736,6 +736,9 @@ enum { >> FP_VXZDZ | FP_VXIMZ | FP_VXVC | >> FP_VXSOFT | \ >> FP_VXSQRT | FP_VXCVI) >> >> +/* FPSCR bits that can be set by mtfsf, mtfsfi and mtfsb1 */ >> +#define FPSCR_MTFS_MASK ~((1ull << 11) | FP_VX | FP_FEX) > > > ./scripts/checkpatch.pl is not happy about this line: > > > ERROR: Macros with complex values should be enclosed in parenthesis > #44: FILE: target/ppc/cpu.h:763: > +#define FPSCR_MTFS_MASK ~((1ull << 11) | FP_VX | FP_FEX) > > total: 1 errors, 0 warnings, 17 lines checked > Ok I'll put it in parenthesis and send a next version with this and the gen_helper_reset_fpstatus removed. > > > > Thanks, > > > > Daniel > > >> + >> /*****************************************************************************/ >> /* Vector status and control register */ >> #define VSCR_NJ 16 /* Vector non-java */ >> -- Lucas Mateus M. Araujo e Castro Instituto de Pesquisas ELDORADO <https://www.eldorado.org.br/?utm_campaign=assinatura_de_e-mail&utm_medium=email&utm_source=RD+Station> Departamento Computação Embarcada Estagiario Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html> [-- Attachment #2: Type: text/html, Size: 5714 bytes --] ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug 2021-10-20 12:57 [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Lucas Mateus Castro (alqotel) 2021-10-20 12:57 ` [PATCH 1/2] target/ppc: Fixed call to deferred exception Lucas Mateus Castro (alqotel) 2021-10-20 12:57 ` [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 Lucas Mateus Castro (alqotel) @ 2021-10-27 11:49 ` Matheus K. Ferst 2 siblings, 0 replies; 14+ messages in thread From: Matheus K. Ferst @ 2021-10-27 11:49 UTC (permalink / raw) To: Lucas Mateus Castro (alqotel), qemu-devel, qemu-ppc Cc: Lucas Mateus Castro (alqotel), richard.henderson, pc, david CC'ing the reporter On 20/10/2021 09:57, Lucas Mateus Castro (alqotel) wrote: > From: "Lucas Mateus Castro (alqotel)" <lucas.castro@eldorado.org.br> > > The instructions mtfsf, mtfsfi and mtfsb, when called, fail to set the FI > bit (bit 46 in the FPSCR) and can set to 1 the reserved bit 52 of the > FPSCR, as reported in https://gitlab.com/qemu-project/qemu/-/issues/266 > (although the bug report is only for mtfsf, the bug applies to mtfsfi and > mtfsb1 as well). > > These instructions also fail to throw an exception when the exception > and enabling bits are set, this can be tested by adding > 'prctl(PR_SET_FPEXC, PR_FP_EXC_PRECISE);' before the __builtin_mtfsf > call in the test case of the bug report. > > These patches aim to fix these issues. > > Lucas Mateus Castro (alqotel) (2): > target/ppc: Fixed call to deferred exception > target/ppc: ppc_store_fpscr doesn't update bit 52 > > target/ppc/cpu.c | 2 +- > target/ppc/cpu.h | 3 +++ > target/ppc/fpu_helper.c | 41 ++++++++++++++++++++++++++++++ > target/ppc/helper.h | 1 + > target/ppc/translate/fp-impl.c.inc | 6 ++--- > 5 files changed, 49 insertions(+), 4 deletions(-) > > -- > 2.31.1 > > -- Matheus K. Ferst Instituto de Pesquisas ELDORADO <http://www.eldorado.org.br/> Analista de Software Aviso Legal - Disclaimer <https://www.eldorado.org.br/disclaimer.html> ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2021-11-16 14:45 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-20 12:57 [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Lucas Mateus Castro (alqotel) 2021-10-20 12:57 ` [PATCH 1/2] target/ppc: Fixed call to deferred exception Lucas Mateus Castro (alqotel) 2021-11-09 16:37 ` Daniel Henrique Barboza 2021-11-10 6:56 ` Cédric Le Goater 2021-11-10 17:29 ` Lucas Mateus Martins Araujo e Castro 2021-11-16 14:43 ` Cédric Le Goater 2021-11-10 8:19 ` Mark Cave-Ayland 2021-11-10 18:58 ` Lucas Mateus Martins Araujo e Castro 2021-11-10 20:40 ` BALATON Zoltan 2021-11-12 2:31 ` 罗勇刚(Yonggang Luo) 2021-10-20 12:57 ` [PATCH 2/2] target/ppc: ppc_store_fpscr doesn't update bit 52 Lucas Mateus Castro (alqotel) 2021-11-09 16:44 ` Daniel Henrique Barboza 2021-11-10 19:07 ` Lucas Mateus Martins Araujo e Castro 2021-10-27 11:49 ` [PATCH 0/2] Fix mtfsf, mtfsfi and mtfsb1 bug Matheus K. Ferst
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).