Commit 5ed19506 authored by Nicholas Piggin's avatar Nicholas Piggin Committed by David Gibson
Browse files

target/ppc: Fix mtmsr(d) L=1 variant that loses interrupts 

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: default avatarAnton Blanchard <anton@ozlabs.org>
Reported-by: default avatarNathan Chancellor <natechancellor@gmail.com>
Tested-by: default avatarNathan Chancellor <natechancellor@gmail.com>
Signed-off-by: default avatarNicholas Piggin <npiggin@gmail.com>
Message-Id: <20200414111131.465560-1-npiggin@gmail.com>
Reviewed-by: default avatarCédric Le Goater <clg@kaod.org>
Tested-by: default avatarCédric Le Goater <clg@kaod.org>
Signed-off-by: default avatarDavid Gibson <david@gibson.dropbear.id.au>
parent 211a7784

target/ppc/translate.c

+27 −19
Original line number Diff line number Diff line
@@ -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);

    }

#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 (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 {

        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);

    }

#endif

}