Cédric Le Goater's on March 26, 2020 2:39 am: > On 3/25/20 3:41 PM, Nicholas Piggin wrote: >> This implements mce injection for pnv. > > This would be the command to use ? > > (qemu) mce 0 0x100000 0x80 0xdeadbeef 1 > >> Signed-off-by: Nicholas Piggin <npig...@gmail.com> >> --- >> hw/ppc/pnv.c | 55 ++++++++++++++++++++++++++++++++++++++++ >> target/ppc/cpu.h | 1 + >> target/ppc/excp_helper.c | 12 +++++++++ >> 3 files changed, 68 insertions(+) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index 671535ebe6..9c515bfeed 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -38,6 +38,7 @@ >> #include "hw/nmi.h" >> #include "exec/address-spaces.h" >> #include "qapi/visitor.h" >> +#include "qapi/qmp/qdict.h" >> #include "monitor/monitor.h" >> #include "hw/intc/intc.h" >> #include "hw/ipmi/ipmi.h" >> @@ -1981,11 +1982,63 @@ static void pnv_nmi(NMIState *n, int cpu_index, >> Error **errp) >> } >> } >> >> +typedef struct MCEInjectionParams { >> + uint64_t srr1_mask; >> + uint32_t dsisr; >> + uint64_t dar; >> + bool recovered; >> +} MCEInjectionParams; >> + >> +static void pnv_do_mce_on_cpu(CPUState *cs, run_on_cpu_data data) >> +{ >> + MCEInjectionParams *params = data.host_ptr; >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + CPUPPCState *env = &cpu->env; >> + uint64_t srr1_mce_bits = PPC_BITMASK(42, 45) | PPC_BIT(36); >> + >> + cpu_synchronize_state(cs); > > I think this call is superfluous as we don't support any accelerators > on this machine (but we might one day).
Okay. I can remove it, or do you think it's fine to stay? >> + ppc_cpu_do_machine_check(cs); >> + >> + env->spr[SPR_SRR1] |= (params->srr1_mask & srr1_mce_bits); > > Don't we need to clear the previous settings like on spapr ? Yes, good catch. >> diff --git a/target/ppc/excp_helper.c b/target/ppc/excp_helper.c >> index 7f2b5899d3..81dd8b6f8e 100644 >> --- a/target/ppc/excp_helper.c >> +++ b/target/ppc/excp_helper.c >> @@ -279,6 +279,10 @@ static inline void powerpc_excp(PowerPCCPU *cpu, int >> excp_model, int excp) >> cs->halted = 1; >> cpu_interrupt_exittb(cs); >> } >> + if (msr_pow) { >> + /* indicate that we resumed from power save mode */ >> + msr |= 0x10000; > > #define ? It matches system reset, but yes it should be put in a define (or PPC_BIT should be okay I guess because it's architecture). As discussed in the nmi patch, we have to make some adjustments for pnv so I'll tidy that up too. Thanks, Nick