@@ -4361,30 +4361,34 @@ static void gen_mtmsrd(DisasContext *ctx)
CHK_SV;
#if !defined(CONFIG_USER_ONLY)
+ if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+ gen_io_start();
+ }
if (ctx->opcode & 0x00010000) {
- /* Special form that does not need any synchronisation */
+ /* L=1 form only updates EE and RI */
TCGv t0 = tcg_temp_new();
+ TCGv t1 = tcg_temp_new();
tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
(1 << MSR_RI) | (1 << MSR_EE));
- tcg_gen_andi_tl(cpu_msr, cpu_msr,
+ tcg_gen_andi_tl(t1, cpu_msr,
~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
- tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
+ tcg_gen_or_tl(t1, t1, t0);
+
+ gen_helper_store_msr(cpu_env, t1);
tcg_temp_free(t0);
+ tcg_temp_free(t1);
+
} else {
/*
* XXX: we need to update nip before the store if we enter
* power saving mode, we will exit the loop directly from
* ppc_store_msr
*/
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
gen_update_nip(ctx, ctx->base.pc_next);
gen_helper_store_msr(cpu_env, cpu_gpr[rS(ctx->opcode)]);
- /* Must stop the translation as machine state (may have) changed */
- /* Note that mtmsr is not always defined as context-synchronizing */
- gen_stop_exception(ctx);
}
+ /* Must stop the translation as machine state (may have) changed */
+ gen_stop_exception(ctx);
#endif /* !defined(CONFIG_USER_ONLY) */
}
#endif /* defined(TARGET_PPC64) */
@@ -4394,15 +4398,23 @@ static void gen_mtmsr(DisasContext *ctx)
CHK_SV;
#if !defined(CONFIG_USER_ONLY)
- if (ctx->opcode & 0x00010000) {
- /* Special form that does not need any synchronisation */
+ if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
+ gen_io_start();
+ }
+ if (ctx->opcode & 0x00010000) {
+ /* L=1 form only updates EE and RI */
TCGv t0 = tcg_temp_new();
+ TCGv t1 = tcg_temp_new();
tcg_gen_andi_tl(t0, cpu_gpr[rS(ctx->opcode)],
(1 << MSR_RI) | (1 << MSR_EE));
- tcg_gen_andi_tl(cpu_msr, cpu_msr,
+ tcg_gen_andi_tl(t1, cpu_msr,
~(target_ulong)((1 << MSR_RI) | (1 << MSR_EE)));
- tcg_gen_or_tl(cpu_msr, cpu_msr, t0);
+ tcg_gen_or_tl(t1, t1, t0);
+
+ gen_helper_store_msr(cpu_env, t1);
tcg_temp_free(t0);
+ tcg_temp_free(t1);
+
} else {
TCGv msr = tcg_temp_new();
@@ -4411,9 +4423,6 @@ static void gen_mtmsr(DisasContext *ctx)
* power saving mode, we will exit the loop directly from
* ppc_store_msr
*/
- if (tb_cflags(ctx->base.tb) & CF_USE_ICOUNT) {
- gen_io_start();
- }
gen_update_nip(ctx, ctx->base.pc_next);
#if defined(TARGET_PPC64)
tcg_gen_deposit_tl(msr, cpu_msr, cpu_gpr[rS(ctx->opcode)], 0, 32);
@@ -4422,10 +4431,9 @@ static void gen_mtmsr(DisasContext *ctx)
#endif
gen_helper_store_msr(cpu_env, msr);
tcg_temp_free(msr);
- /* Must stop the translation as machine state (may have) changed */
- /* Note that mtmsr is not always defined as context-synchronizing */
- gen_stop_exception(ctx);
}
+ /* Must stop the translation as machine state (may have) changed */
+ gen_stop_exception(ctx);
#endif
}
If mtmsr L=1 sets MSR[EE] while there is a maskable exception pending, it does not cause an interrupt. This causes the test case to hang: https://lists.gnu.org/archive/html/qemu-ppc/2019-10/msg00826.html More recently, Linux reduced the occurance of operations (e.g., rfi) which stop translation and allow pending interrupts to be processed. This started causing hangs in Linux boot in long-running kernel tests, running with '-d int' shows the decrementer stops firing despite DEC wrapping and MSR[EE]=1. https://lists.ozlabs.org/pipermail/linuxppc-dev/2020-April/208301.html The cause is the broken mtmsr L=1 behaviour, which is contrary to the architecture. From Power ISA v3.0B, p.977, Move To Machine State Register, Programming Note states: If MSR[EE]=0 and an External, Decrementer, or Performance Monitor exception is pending, executing an mtmsrd instruction that sets MSR[EE] to 1 will cause the interrupt to occur before the next instruction is executed, if no higher priority exception exists Fix this by handling L=1 exactly the same way as L=0, modulo the MSR bits altered. The confusion arises from L=0 being "context synchronizing" whereas L=1 is "execution synchronizing", which is a weaker semantic. However this is not a relaxation of the requirement that these exceptions cause interrupts when MSR[EE]=1 (e.g., when mtmsr executes to completion as TCG is doing here), rather it specifies how a pipelined processor can have multiple instructions in flight where one may influence how another behaves. Cc: qemu-stable@nongnu.org Reported-by: Anton Blanchard <anton@ozlabs.org> Reported-by: Nathan Chancellor <natechancellor@gmail.com> Tested-by: Nathan Chancellor <natechancellor@gmail.com> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- Thanks very much to Nathan for reporting and testing it, I added his Tested-by tag despite a more polished patch, as the the basics are still the same (and still fixes his test case here). This bug possibly goes back to early v2.04 / mtmsrd L=1 support around 2007, and the code has been changed several times since then so may require some backporting. 32-bit / mtmsr untested at the moment, I don't have an environment handy. target/ppc/translate.c | 46 +++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 19 deletions(-)