| Message ID | 20240321055415.2441812-1-npiggin@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | target/ppc: Do not clear MSR[ME] on MCE interrupts to supervisor | expand |
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;
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;
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> --- target/ppc/excp_helper.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)