Re: [PATCH] target/ppc: Do not clear MSR[ME] on MCE interrupts to supervisor

[Harsh Prateek Bora <harshpb@linux.ibm.com>]

On 3/21/24 11:24, Nicholas Piggin wrote:
> Hardware clears the MSR[ME] bit when delivering a machine check
> interrupt, so that is what QEMU does.
> 
> The spapr environment runs in supervisor mode though, and receives
> machine check interrupts after they are processed by the hypervisor,
> and MSR[ME] must always be enabled in supervisor mode (otherwise it
> could checkstop the system). So MSR[ME] must not be cleared when
> delivering machine checks to the supervisor.
> 
> The fix to prevent supervisor mode from modifying MSR[ME] also
> prevented it from re-enabling the incorrectly cleared MSR[ME] bit
> when returning from handling the interrupt. Before that fix, the
> problem was not very noticable with well-behaved code. So the
> Fixes tag is not strictly correct, but practically they go together.
> 
> Found by kvm-unit-tests machine check tests (not yet upstream).
> 
> Fixes: 678b6f1af75ef ("target/ppc: Prevent supervisor from modifying MSR[ME]")
> Signed-off-by: Nicholas Piggin <npiggin@gmail.com>

Reviewed-by: Harsh Prateek Bora <harshpb@linux.ibm.com>

> ---
>   target/ppc/excp_helper.c | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c
> index 80f584f933..674c05a2ce 100644
> --- a/target/ppc/excp_helper.c
> +++ b/target/ppc/excp_helper.c
> @@ -1345,9 +1345,10 @@ static void powerpc_excp_books(PowerPCCPU *cpu, int excp)
>                * clear (e.g., see FWNMI in PAPR).
>                */
>               new_msr |= (target_ulong)MSR_HVB;
> +
> +            /* HV machine check exceptions don't have ME set */
> +            new_msr &= ~((target_ulong)1 << MSR_ME);
>           }
> -        /* machine check exceptions don't have ME set */
> -        new_msr &= ~((target_ulong)1 << MSR_ME);
>   
>           msr |= env->error_code;
>           break;