| Message ID | 20200325144147.221875-3-npiggin@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | ppc: sreset and machine check injection | expand |
[ Please use clg@kaod.org ! ] On 3/25/20 3:41 PM, Nicholas Piggin wrote: > This implements the NMI interface for the PNV machine, similarly to > commit 3431648272d ("spapr: Add support for new NMI interface") for > SPAPR. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> one minor comment, Reviewed-by: Cédric Le Goater <clg@kaod.org> > --- > hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index b75ad06390..671535ebe6 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -27,6 +27,7 @@ > #include "sysemu/runstate.h" > #include "sysemu/cpus.h" > #include "sysemu/device_tree.h" > +#include "sysemu/hw_accel.h" > #include "target/ppc/cpu.h" > #include "qemu/log.h" > #include "hw/ppc/fdt.h" > @@ -34,6 +35,7 @@ > #include "hw/ppc/pnv.h" > #include "hw/ppc/pnv_core.h" > #include "hw/loader.h" > +#include "hw/nmi.h" > #include "exec/address-spaces.h" > #include "qapi/visitor.h" > #include "monitor/monitor.h" > @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) > } > } > > +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + cpu_synchronize_state(cs); > + ppc_cpu_do_system_reset(cs); > + /* > + * SRR1[42:45] is set to 0100 which the ISA defines as implementation I see 'System Reset' in ISA 3.0 > + * dependent. POWER processors use this for xscom triggered interrupts, > + * which come from the BMC or NMI IPIs. > + */ > + env->spr[SPR_SRR1] |= PPC_BIT(43); So we could have used the skiboot SPR_SRR1_PM_WAKE_SRESET define ? > +} > + > +static void pnv_nmi(NMIState *n, int cpu_index, Error **errp) > +{ > + CPUState *cs; > + > + CPU_FOREACH(cs) { > + async_run_on_cpu(cs, pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_NULL); > + } > +} > + > static void pnv_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc); > + NMIClass *nc = NMI_CLASS(oc); > > mc->desc = "IBM PowerNV (Non-Virtualized)"; > mc->init = pnv_init; > @@ -1975,6 +2002,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) > mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; > mc->default_ram_id = "pnv.ram"; > ispc->print_info = pnv_pic_print_info; > + nc->nmi_monitor_handler = pnv_nmi; > > object_class_property_add_bool(oc, "hb-mode", > pnv_machine_get_hb, pnv_machine_set_hb, > @@ -2038,7 +2066,7 @@ static const TypeInfo types[] = { > .class_size = sizeof(PnvMachineClass), > .interfaces = (InterfaceInfo[]) { > { TYPE_INTERRUPT_STATS_PROVIDER }, > - { }, > + { TYPE_NMI }, > }, > }, > { >
On Thu, Mar 26, 2020 at 12:41:44AM +1000, Nicholas Piggin wrote: > This implements the NMI interface for the PNV machine, similarly to > commit 3431648272d ("spapr: Add support for new NMI interface") for > SPAPR. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> Applied to ppc-for-5.1. > --- > hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index b75ad06390..671535ebe6 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -27,6 +27,7 @@ > #include "sysemu/runstate.h" > #include "sysemu/cpus.h" > #include "sysemu/device_tree.h" > +#include "sysemu/hw_accel.h" > #include "target/ppc/cpu.h" > #include "qemu/log.h" > #include "hw/ppc/fdt.h" > @@ -34,6 +35,7 @@ > #include "hw/ppc/pnv.h" > #include "hw/ppc/pnv_core.h" > #include "hw/loader.h" > +#include "hw/nmi.h" > #include "exec/address-spaces.h" > #include "qapi/visitor.h" > #include "monitor/monitor.h" > @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) > } > } > > +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + cpu_synchronize_state(cs); > + ppc_cpu_do_system_reset(cs); > + /* > + * SRR1[42:45] is set to 0100 which the ISA defines as implementation > + * dependent. POWER processors use this for xscom triggered interrupts, > + * which come from the BMC or NMI IPIs. > + */ > + env->spr[SPR_SRR1] |= PPC_BIT(43); > +} > + > +static void pnv_nmi(NMIState *n, int cpu_index, Error **errp) > +{ > + CPUState *cs; > + > + CPU_FOREACH(cs) { > + async_run_on_cpu(cs, pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_NULL); > + } > +} > + > static void pnv_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc); > + NMIClass *nc = NMI_CLASS(oc); > > mc->desc = "IBM PowerNV (Non-Virtualized)"; > mc->init = pnv_init; > @@ -1975,6 +2002,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) > mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; > mc->default_ram_id = "pnv.ram"; > ispc->print_info = pnv_pic_print_info; > + nc->nmi_monitor_handler = pnv_nmi; > > object_class_property_add_bool(oc, "hb-mode", > pnv_machine_get_hb, pnv_machine_set_hb, > @@ -2038,7 +2066,7 @@ static const TypeInfo types[] = { > .class_size = sizeof(PnvMachineClass), > .interfaces = (InterfaceInfo[]) { > { TYPE_INTERRUPT_STATS_PROVIDER }, > - { }, > + { TYPE_NMI }, > }, > }, > {
On 26/03/2020 01:41, Nicholas Piggin wrote: > This implements the NMI interface for the PNV machine, similarly to > commit 3431648272d ("spapr: Add support for new NMI interface") for > SPAPR. > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > --- > hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- > 1 file changed, 29 insertions(+), 1 deletion(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index b75ad06390..671535ebe6 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -27,6 +27,7 @@ > #include "sysemu/runstate.h" > #include "sysemu/cpus.h" > #include "sysemu/device_tree.h" > +#include "sysemu/hw_accel.h" > #include "target/ppc/cpu.h" > #include "qemu/log.h" > #include "hw/ppc/fdt.h" > @@ -34,6 +35,7 @@ > #include "hw/ppc/pnv.h" > #include "hw/ppc/pnv_core.h" > #include "hw/loader.h" > +#include "hw/nmi.h" > #include "exec/address-spaces.h" > #include "qapi/visitor.h" > #include "monitor/monitor.h" > @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) > } > } > > +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) > +{ > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + cpu_synchronize_state(cs); > + ppc_cpu_do_system_reset(cs); > + /* > + * SRR1[42:45] is set to 0100 which the ISA defines as implementation > + * dependent. POWER processors use this for xscom triggered interrupts, > + * which come from the BMC or NMI IPIs. > + */ > + env->spr[SPR_SRR1] |= PPC_BIT(43); > +} > + > +static void pnv_nmi(NMIState *n, int cpu_index, Error **errp) > +{ > + CPUState *cs; > + > + CPU_FOREACH(cs) { > + async_run_on_cpu(cs, pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_NULL); > + } > +} > + > static void pnv_machine_class_init(ObjectClass *oc, void *data) > { > MachineClass *mc = MACHINE_CLASS(oc); > InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc); > + NMIClass *nc = NMI_CLASS(oc); > > mc->desc = "IBM PowerNV (Non-Virtualized)"; > mc->init = pnv_init; > @@ -1975,6 +2002,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) > mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; > mc->default_ram_id = "pnv.ram"; > ispc->print_info = pnv_pic_print_info; > + nc->nmi_monitor_handler = pnv_nmi; > > object_class_property_add_bool(oc, "hb-mode", > pnv_machine_get_hb, pnv_machine_set_hb, > @@ -2038,7 +2066,7 @@ static const TypeInfo types[] = { > .class_size = sizeof(PnvMachineClass), > .interfaces = (InterfaceInfo[]) { > { TYPE_INTERRUPT_STATS_PROVIDER }, > - { }, > + { TYPE_NMI }, The interface list must end with {}, otherwise QEMU crashes very early. Thanks, > }, > }, > { >
On Tue, Mar 31, 2020 at 02:07:42PM +1100, Alexey Kardashevskiy wrote: > > > On 26/03/2020 01:41, Nicholas Piggin wrote: > > This implements the NMI interface for the PNV machine, similarly to > > commit 3431648272d ("spapr: Add support for new NMI interface") for > > SPAPR. > > > > Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > --- > > hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- > > 1 file changed, 29 insertions(+), 1 deletion(-) > > > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > > index b75ad06390..671535ebe6 100644 > > --- a/hw/ppc/pnv.c > > +++ b/hw/ppc/pnv.c > > @@ -27,6 +27,7 @@ > > #include "sysemu/runstate.h" > > #include "sysemu/cpus.h" > > #include "sysemu/device_tree.h" > > +#include "sysemu/hw_accel.h" > > #include "target/ppc/cpu.h" > > #include "qemu/log.h" > > #include "hw/ppc/fdt.h" > > @@ -34,6 +35,7 @@ > > #include "hw/ppc/pnv.h" > > #include "hw/ppc/pnv_core.h" > > #include "hw/loader.h" > > +#include "hw/nmi.h" > > #include "exec/address-spaces.h" > > #include "qapi/visitor.h" > > #include "monitor/monitor.h" > > @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) > > } > > } > > > > +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) > > +{ > > + PowerPCCPU *cpu = POWERPC_CPU(cs); > > + CPUPPCState *env = &cpu->env; > > + > > + cpu_synchronize_state(cs); > > + ppc_cpu_do_system_reset(cs); > > + /* > > + * SRR1[42:45] is set to 0100 which the ISA defines as implementation > > + * dependent. POWER processors use this for xscom triggered interrupts, > > + * which come from the BMC or NMI IPIs. > > + */ > > + env->spr[SPR_SRR1] |= PPC_BIT(43); > > +} > > + > > +static void pnv_nmi(NMIState *n, int cpu_index, Error **errp) > > +{ > > + CPUState *cs; > > + > > + CPU_FOREACH(cs) { > > + async_run_on_cpu(cs, pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_NULL); > > + } > > +} > > + > > static void pnv_machine_class_init(ObjectClass *oc, void *data) > > { > > MachineClass *mc = MACHINE_CLASS(oc); > > InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc); > > + NMIClass *nc = NMI_CLASS(oc); > > > > mc->desc = "IBM PowerNV (Non-Virtualized)"; > > mc->init = pnv_init; > > @@ -1975,6 +2002,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) > > mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; > > mc->default_ram_id = "pnv.ram"; > > ispc->print_info = pnv_pic_print_info; > > + nc->nmi_monitor_handler = pnv_nmi; > > > > object_class_property_add_bool(oc, "hb-mode", > > pnv_machine_get_hb, pnv_machine_set_hb, > > @@ -2038,7 +2066,7 @@ static const TypeInfo types[] = { > > .class_size = sizeof(PnvMachineClass), > > .interfaces = (InterfaceInfo[]) { > > { TYPE_INTERRUPT_STATS_PROVIDER }, > > - { }, > > + { TYPE_NMI }, > > > The interface list must end with {}, otherwise QEMU crashes very early. > Thanks, I've fixed that inline now. > > > > }, > > }, > > { > > >
Cédric Le Goater's on March 26, 2020 2:38 am: > [ Please use clg@kaod.org ! ] > > On 3/25/20 3:41 PM, Nicholas Piggin wrote: >> This implements the NMI interface for the PNV machine, similarly to >> commit 3431648272d ("spapr: Add support for new NMI interface") for >> SPAPR. >> >> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> > > one minor comment, > > Reviewed-by: Cédric Le Goater <clg@kaod.org> > >> --- >> hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- >> 1 file changed, 29 insertions(+), 1 deletion(-) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index b75ad06390..671535ebe6 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -27,6 +27,7 @@ >> #include "sysemu/runstate.h" >> #include "sysemu/cpus.h" >> #include "sysemu/device_tree.h" >> +#include "sysemu/hw_accel.h" >> #include "target/ppc/cpu.h" >> #include "qemu/log.h" >> #include "hw/ppc/fdt.h" >> @@ -34,6 +35,7 @@ >> #include "hw/ppc/pnv.h" >> #include "hw/ppc/pnv_core.h" >> #include "hw/loader.h" >> +#include "hw/nmi.h" >> #include "exec/address-spaces.h" >> #include "qapi/visitor.h" >> #include "monitor/monitor.h" >> @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) >> } >> } >> >> +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) >> +{ >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + CPUPPCState *env = &cpu->env; >> + >> + cpu_synchronize_state(cs); >> + ppc_cpu_do_system_reset(cs); >> + /* >> + * SRR1[42:45] is set to 0100 which the ISA defines as implementation > > I see 'System Reset' in ISA 3.0 >> + * dependent. POWER processors use this for xscom triggered interrupts, >> + * which come from the BMC or NMI IPIs. >> + */ >> + env->spr[SPR_SRR1] |= PPC_BIT(43); > > So we could have used the skiboot SPR_SRR1_PM_WAKE_SRESET define ? Ah, that's only for power-saving wakeup. But you got me to dig further and I think I've got a few things wrong here. The architectural power save wakeup due to sreset bit 43 needs to be set, probably in excp_helper.c if (msr_pow) test. case POWERPC_EXCP_RESET: /* System reset exception */ /* A power-saving exception sets ME, otherwise it is unchanged */ if (msr_pow) { /* indicate that we resumed from power save mode */ msr |= 0x10000; new_msr |= ((target_ulong)1 << MSR_ME); } For non-power save wakeup, it's all implementation defined. POWER9 UM has the table, but I got the damn bit wrong, I was probably looking in the ISA table by mistake. It's bit 44 for that case. Linux doesn't tend to care about that case, but it does care about the power save wakeup case, so this patch seems to generally "work", but I'll have to fix it. Thanks, Nick
Nicholas Piggin's on April 3, 2020 5:57 pm: > Cédric Le Goater's on March 26, 2020 2:38 am: >> [ Please use clg@kaod.org ! ] >> >> On 3/25/20 3:41 PM, Nicholas Piggin wrote: >>> This implements the NMI interface for the PNV machine, similarly to >>> commit 3431648272d ("spapr: Add support for new NMI interface") for >>> SPAPR. >>> >>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >> >> one minor comment, >> >> Reviewed-by: Cédric Le Goater <clg@kaod.org> >> >>> --- >>> hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- >>> 1 file changed, 29 insertions(+), 1 deletion(-) >>> >>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >>> index b75ad06390..671535ebe6 100644 >>> --- a/hw/ppc/pnv.c >>> +++ b/hw/ppc/pnv.c >>> @@ -27,6 +27,7 @@ >>> #include "sysemu/runstate.h" >>> #include "sysemu/cpus.h" >>> #include "sysemu/device_tree.h" >>> +#include "sysemu/hw_accel.h" >>> #include "target/ppc/cpu.h" >>> #include "qemu/log.h" >>> #include "hw/ppc/fdt.h" >>> @@ -34,6 +35,7 @@ >>> #include "hw/ppc/pnv.h" >>> #include "hw/ppc/pnv_core.h" >>> #include "hw/loader.h" >>> +#include "hw/nmi.h" >>> #include "exec/address-spaces.h" >>> #include "qapi/visitor.h" >>> #include "monitor/monitor.h" >>> @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) >>> } >>> } >>> >>> +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) >>> +{ >>> + PowerPCCPU *cpu = POWERPC_CPU(cs); >>> + CPUPPCState *env = &cpu->env; >>> + >>> + cpu_synchronize_state(cs); >>> + ppc_cpu_do_system_reset(cs); >>> + /* >>> + * SRR1[42:45] is set to 0100 which the ISA defines as implementation >> >> I see 'System Reset' in ISA 3.0 >>> + * dependent. POWER processors use this for xscom triggered interrupts, >>> + * which come from the BMC or NMI IPIs. >>> + */ >>> + env->spr[SPR_SRR1] |= PPC_BIT(43); >> >> So we could have used the skiboot SPR_SRR1_PM_WAKE_SRESET define ? > > Ah, that's only for power-saving wakeup. But you got me to dig further > and I think I've got a few things wrong here. > > The architectural power save wakeup due to sreset bit 43 needs to be > set, probably in excp_helper.c if (msr_pow) test. > > case POWERPC_EXCP_RESET: /* System reset exception */ > /* A power-saving exception sets ME, otherwise it is unchanged */ > if (msr_pow) { > /* indicate that we resumed from power save mode */ > msr |= 0x10000; > new_msr |= ((target_ulong)1 << MSR_ME); > } Sorry I'm wrong, that's the old MSR_POW thing I guess G5 had it. 'stop' state wakeup is powerpc_reset_wakeup of course, which seems to do the right thing with SRR1. Something like this patch should fix this issue in the ppc-5.1 branch. This appears to do the right thing with SRR1 in testing with Linux. --- hw/ppc/pnv.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index ac83b8698b..596ccfd99e 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -1964,12 +1964,21 @@ static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) cpu_synchronize_state(cs); ppc_cpu_do_system_reset(cs); - /* - * SRR1[42:45] is set to 0100 which the ISA defines as implementation - * dependent. POWER processors use this for xscom triggered interrupts, - * which come from the BMC or NMI IPIs. - */ - env->spr[SPR_SRR1] |= PPC_BIT(43); + if (env->spr[SPR_SRR1] & PPC_BITMASK(46, 47)) { + /* system reset caused wake from power saving state */ + if (!(env->spr[SPR_SRR1] & PPC_BIT(43))) { + warn_report("ppc_cpu_do_system_reset does not set system reset wakeup reason"); + env->spr[SPR_SRR1] |= PPC_BIT(43); + } + } else { + /* + * For non-powersave wakeup system resets, SRR1[42:45] are defined to + * be implementation dependent. Set to 0b0010, which POWER9 UM defines + * to be interrupt caused by SCOM, which can come from the BMC or NMI + * IPIs. + */ + env->spr[SPR_SRR1] |= PPC_BIT(44); + } } static void pnv_nmi(NMIState *n, int cpu_index, Error **errp)
On 4/3/20 3:12 PM, Nicholas Piggin wrote: > Nicholas Piggin's on April 3, 2020 5:57 pm: >> Cédric Le Goater's on March 26, 2020 2:38 am: >>> [ Please use clg@kaod.org ! ] >>> >>> On 3/25/20 3:41 PM, Nicholas Piggin wrote: >>>> This implements the NMI interface for the PNV machine, similarly to >>>> commit 3431648272d ("spapr: Add support for new NMI interface") for >>>> SPAPR. >>>> >>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>> >>> one minor comment, >>> >>> Reviewed-by: Cédric Le Goater <clg@kaod.org> >>> >>>> --- >>>> hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- >>>> 1 file changed, 29 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >>>> index b75ad06390..671535ebe6 100644 >>>> --- a/hw/ppc/pnv.c >>>> +++ b/hw/ppc/pnv.c >>>> @@ -27,6 +27,7 @@ >>>> #include "sysemu/runstate.h" >>>> #include "sysemu/cpus.h" >>>> #include "sysemu/device_tree.h" >>>> +#include "sysemu/hw_accel.h" >>>> #include "target/ppc/cpu.h" >>>> #include "qemu/log.h" >>>> #include "hw/ppc/fdt.h" >>>> @@ -34,6 +35,7 @@ >>>> #include "hw/ppc/pnv.h" >>>> #include "hw/ppc/pnv_core.h" >>>> #include "hw/loader.h" >>>> +#include "hw/nmi.h" >>>> #include "exec/address-spaces.h" >>>> #include "qapi/visitor.h" >>>> #include "monitor/monitor.h" >>>> @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) >>>> } >>>> } >>>> >>>> +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) >>>> +{ >>>> + PowerPCCPU *cpu = POWERPC_CPU(cs); >>>> + CPUPPCState *env = &cpu->env; >>>> + >>>> + cpu_synchronize_state(cs); >>>> + ppc_cpu_do_system_reset(cs); >>>> + /* >>>> + * SRR1[42:45] is set to 0100 which the ISA defines as implementation >>> >>> I see 'System Reset' in ISA 3.0 >>>> + * dependent. POWER processors use this for xscom triggered interrupts, >>>> + * which come from the BMC or NMI IPIs. >>>> + */ >>>> + env->spr[SPR_SRR1] |= PPC_BIT(43); >>> >>> So we could have used the skiboot SPR_SRR1_PM_WAKE_SRESET define ? >> >> Ah, that's only for power-saving wakeup. But you got me to dig further >> and I think I've got a few things wrong here. >> >> The architectural power save wakeup due to sreset bit 43 needs to be >> set, probably in excp_helper.c if (msr_pow) test. >> >> case POWERPC_EXCP_RESET: /* System reset exception */ >> /* A power-saving exception sets ME, otherwise it is unchanged */ >> if (msr_pow) { >> /* indicate that we resumed from power save mode */ >> msr |= 0x10000; >> new_msr |= ((target_ulong)1 << MSR_ME); >> } > > Sorry I'm wrong, that's the old MSR_POW thing I guess G5 had it. > > 'stop' state wakeup is powerpc_reset_wakeup of course, which seems to > do the right thing with SRR1. > > Something like this patch should fix this issue in the ppc-5.1 branch. > This appears to do the right thing with SRR1 in testing with Linux. > > --- > hw/ppc/pnv.c | 21 +++++++++++++++------ > 1 file changed, 15 insertions(+), 6 deletions(-) > > diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c > index ac83b8698b..596ccfd99e 100644 > --- a/hw/ppc/pnv.c > +++ b/hw/ppc/pnv.c > @@ -1964,12 +1964,21 @@ static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) > > cpu_synchronize_state(cs); > ppc_cpu_do_system_reset(cs); > - /* > - * SRR1[42:45] is set to 0100 which the ISA defines as implementation > - * dependent. POWER processors use this for xscom triggered interrupts, > - * which come from the BMC or NMI IPIs. > - */ > - env->spr[SPR_SRR1] |= PPC_BIT(43); > + if (env->spr[SPR_SRR1] & PPC_BITMASK(46, 47)) { > + /* system reset caused wake from power saving state */ > + if (!(env->spr[SPR_SRR1] & PPC_BIT(43))) { > + warn_report("ppc_cpu_do_system_reset does not set system reset wakeup reason"); > + env->spr[SPR_SRR1] |= PPC_BIT(43); > + } > + } else { > + /* > + * For non-powersave wakeup system resets, SRR1[42:45] are defined to > + * be implementation dependent. Set to 0b0010, which POWER9 UM defines > + * to be interrupt caused by SCOM, which can come from the BMC or NMI > + * IPIs. > + */ > + env->spr[SPR_SRR1] |= PPC_BIT(44); > + } > } That looks correct according to the UM. Do we care about the other non-powersave wakeup system resets ? 0011 Interrupt caused by hypervisor door bell. 0101 Interrupt caused by privileged door bell. The reason is that I sometime see CPU lockups under load, or with a KVM guest, and I haven't found why yet. Thanks, C. [ 259.069436] virbr0: port 2(tap0) entered forwarding state [ 259.069523] virbr0: topology change detected, propagating [ 384.427337] watchdog: CPU 1 detected hard LOCKUP on other CPUs 0 [ 384.428566] watchdog: CPU 1 TB:255514422948, last SMP heartbeat TB:246648941120 (17315ms ago) [ 384.528958] watchdog: CPU 0 Hard LOCKUP [ 384.529380] watchdog: CPU 0 TB:255566522003, last heartbeat TB:246648941120 (17417ms ago) [ 384.529879] Modules linked in: vhost_net vhost xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_reject_ipv4 xt_tcpudp ip6table_mangle ip6table_nat iptable_mangle iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 libcrc32c nf_defrag_ipv4 nf_tables nfnetlink ip6table_filter ip6_tables iptable_filter bpfilter bridge stp llc vmx_crypto crct10dif_vpmsum crc32c_vpmsum uio_pdrv_genirq uio kvm_hv kvm sch_fq_codel ip_tables x_tables autofs4 ipr e1000e ahci libahci [ 384.530400] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5+ #31 [ 384.530473] NIP: c00000000000a93c LR: c00000000001b944 CTR: 00007094cb3d9680 [ 384.530522] REGS: c000000001a47a40 TRAP: 0e81 Not tainted (5.6.0-rc5+) [ 384.530532] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 48000242 XER: 00000000 [ 384.530649] CFAR: 00000ac5e544b424 IRQMASK: 0 GPR00: c000000000c03620 c000000001a47cd0 c000000001a48f00 0000000028000000 GPR04: 00000000682a0000 0000003b80f29024 0000003ea9382e20 c000000001cf0000 GPR08: 0000000200000000 00000ac5e55200a0 0000000000000000 00000ac61eb65648 GPR12: 0000000044022444 00007094cb5ddcc0 [ 384.531164] NIP [c00000000000a93c] replay_interrupt_return+0x0/0x4 [ 384.531203] LR [c00000000001b944] arch_local_irq_restore.part.0+0x54/0x70 [ 384.531305] Call Trace: [ 384.531360] [c000000001a47cd0] [c000000001a47d50] init_stack+0x3d50/0x4000 (unreliable) [ 384.531450] [c000000001a47cf0] [c000000000c03620] cpuidle_enter_state+0xf0/0x590 [ 384.531471] [c000000001a47d70] [c000000000c03b5c] cpuidle_enter+0x4c/0x70 [ 384.531503] [c000000001a47db0] [c00000000018c7cc] call_cpuidle+0x4c/0x90 [ 384.531514] [c000000001a47dd0] [c00000000018cde8] do_idle+0x308/0x420 [ 384.531523] [c000000001a47e70] [c00000000018d148] cpu_startup_entry+0x38/0x40 [ 384.531531] [c000000001a47ea0] [c000000000010800] rest_init+0xe0/0xf8 [ 384.531543] [c000000001a47ed0] [c00000000130445c] start_kernel+0x7c4/0x810 [ 384.531553] [c000000001a47f90] [c00000000000accc] start_here_common+0x1c/0x3e8 [ 384.531610] Instruction dump: [ 384.531782] 7d200026 618c8000 2c030900 4182e778 2c030500 4182f3b0 2c030f00 4182f4c8 [ 384.531821] 2c030a00 4182ffbc 2c030e60 4182f158 <4e800020> 7c781b78 48000329 48000341 [ 388.127242] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 388.128093] rcu: 0-...!: (0 ticks this GP) idle=de0/0/0x0 softirq=40880/40880 fqs=0 [ 388.128760] (detected by 1, t=5252 jiffies, g=61401, q=9509) [ 388.128967] Sending NMI from CPU 1 to CPUs 0: [ 388.124766] NMI backtrace for cpu 0 [ 388.124789] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5+ #31 [ 388.124799] NIP: c00000000000a93c LR: c00000000001b944 CTR: 00007094cb3d9680 [ 388.124807] REGS: c000000001a47a40 TRAP: 0e81 Not tainted (5.6.0-rc5+) [ 388.124811] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 48000242 XER: 00000000 [ 388.124839] CFAR: 00000ac5e544b424 IRQMASK: 0 GPR00: c000000000c03620 c000000001a47cd0 c000000001a48f00 0000000028000000 GPR04: 00000000682a0000 0000003beeaf96c3 0000003ea9382e20 c000000001cf0000 GPR08: 0000000200000000 00000ac5e55200a0 0000000000000000 00000ac61eb65648 GPR12: 0000000044022444 00007094cb5ddcc0 [ 388.124894] NIP [c00000000000a93c] replay_interrupt_return+0x0/0x4 [ 388.124904] LR [c00000000001b944] arch_local_irq_restore.part.0+0x54/0x70 [ 388.124909] Call Trace: [ 388.124920] [c000000001a47cd0] [c000000001a47d50] init_stack+0x3d50/0x4000 (unreliable) [ 388.124935] [c000000001a47cf0] [c000000000c03620] cpuidle_enter_state+0xf0/0x590 [ 388.124946] [c000000001a47d70] [c000000000c03b5c] cpuidle_enter+0x4c/0x70 [ 388.124959] [c000000001a47db0] [c00000000018c7cc] call_cpuidle+0x4c/0x90 [ 388.124970] [c000000001a47dd0] [c00000000018cde8] do_idle+0x308/0x420 [ 388.124981] [c000000001a47e70] [c00000000018d148] cpu_startup_entry+0x38/0x40 [ 388.124991] [c000000001a47ea0] [c000000000010800] rest_init+0xe0/0xf8 [ 388.125002] [c000000001a47ed0] [c00000000130445c] start_kernel+0x7c4/0x810 [ 388.125013] [c000000001a47f90] [c00000000000accc] start_here_common+0x1c/0x3e8 [ 388.125020] Instruction dump: [ 388.125031] 7d200026 618c8000 2c030900 4182e778 2c030500 4182f3b0 2c030f00 4182f4c8 [ 388.125057] 2c030a00 4182ffbc 2c030e60 4182f158 <4e800020> 7c781b78 48000329 48000341 [ 388.129812] rcu: rcu_sched kthread starved for 5252 jiffies! g61401 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402 ->cpu=0 [ 388.130357] rcu: RCU grace-period kthread stack dump: [ 388.130752] rcu_sched I 0 10 2 0x00000808 [ 388.130829] Call Trace: [ 388.131100] [c000000068487890] [c000000068487940] 0xc000000068487940 (unreliable) [ 388.131135] [c000000068487a70] [c000000000021c6c] __switch_to+0x2dc/0x450 [ 388.131151] [c000000068487ae0] [c000000000e98a14] __schedule+0x2d4/0x920 [ 388.131162] [c000000068487bb0] [c000000000e990d4] schedule+0x74/0x140 [ 388.131173] [c000000068487be0] [c000000000e9ed24] schedule_timeout+0x1a4/0x3e0 [ 388.131186] [c000000068487cc0] [c0000000001eac88] rcu_gp_kthread+0x788/0xd00 [ 388.131197] [c000000068487db0] [c000000000170744] kthread+0x1a4/0x1b0 [ 388.131210] [c000000068487e20] [c00000000000b648] ret_from_kernel_thread+0x5c/0x74 [ 451.146090] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 451.147979] rcu: 0-...!: (0 ticks this GP) idle=e30/0/0x0 softirq=40880/40880 fqs=1 [ 451.150009] (detected by 1, t=21007 jiffies, g=61401, q=9531) [ 451.150097] Sending NMI from CPU 1 to CPUs 0: [ 451.145933] NMI backtrace for cpu 0 [ 451.146028] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5+ #31 [ 451.146095] NIP: c00000000000a93c LR: c00000000001b944 CTR: c0000000000b9780 [ 451.146126] REGS: c000000001a47a40 TRAP: 0e81 Not tainted (5.6.0-rc5+) [ 451.146139] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 48000242 XER: 00000000 [ 451.146233] CFAR: c0000000000b89a0 IRQMASK: 0 GPR00: c000000000c03620 c000000001a47cd0 c000000001a48f00 0000000028000000 GPR04: 00000000682a0000 0000004371f05548 0000004659916004 c000000001cf0000 GPR08: 0000000200000000 c00a001080060031 0000000000000000 c000000001cb8f00 GPR12: 0000000000008000 c000000001cf0000 [ 451.146433] NIP [c00000000000a93c] replay_interrupt_return+0x0/0x4 [ 451.146471] LR [c00000000001b944] arch_local_irq_restore.part.0+0x54/0x70 [ 451.146492] Call Trace: [ 451.146540] [c000000001a47cd0] [c000000001a47d50] init_stack+0x3d50/0x4000 (unreliable) [ 451.146603] [c000000001a47cf0] [c000000000c03620] cpuidle_enter_state+0xf0/0x590 [ 451.146636] [c000000001a47d70] [c000000000c03b5c] cpuidle_enter+0x4c/0x70 [ 451.146686] [c000000001a47db0] [c00000000018c7cc] call_cpuidle+0x4c/0x90 [ 451.146722] [c000000001a47dd0] [c00000000018cde8] do_idle+0x308/0x420 [ 451.146757] [c000000001a47e70] [c00000000018d14c] cpu_startup_entry+0x3c/0x40 [ 451.146786] [c000000001a47ea0] [c000000000010800] rest_init+0xe0/0xf8 [ 451.146834] [c000000001a47ed0] [c00000000130445c] start_kernel+0x7c4/0x810 [ 451.146871] [c000000001a47f90] [c00000000000accc] start_here_common+0x1c/0x3e8 [ 451.146894] Instruction dump: [ 451.146933] 7d200026 618c8000 2c030900 4182e778 2c030500 4182f3b0 2c030f00 4182f4c8 [ 451.146998] 2c030a00 4182ffbc 2c030e60 4182f158 <4e800020> 7c781b78 48000329 48000341 [ 451.151652] rcu: rcu_sched kthread starved for 15754 jiffies! g61401 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402 ->cpu=0 [ 451.154297] rcu: RCU grace-period kthread stack dump: [ 451.155712] rcu_sched I 0 10 2 0x00000808 [ 451.155782] Call Trace: [ 451.155861] [c000000068487890] [c000000068487940] 0xc000000068487940 (unreliable) [ 451.155950] [c000000068487a70] [c000000000021c6c] __switch_to+0x2dc/0x450 [ 451.155999] [c000000068487ae0] [c000000000e98a14] __schedule+0x2d4/0x920 [ 451.156029] [c000000068487bb0] [c000000000e990d4] schedule+0x74/0x140 [ 451.156061] [c000000068487be0] [c000000000e9ed24] schedule_timeout+0x1a4/0x3e0 [ 451.156103] [c000000068487cc0] [c0000000001eac88] rcu_gp_kthread+0x788/0xd00 [ 451.156135] [c000000068487db0] [c000000000170744] kthread+0x1a4/0x1b0 [ 451.156177] [c000000068487e20] [c00000000000b648] ret_from_kernel_thread+0x5c/0x74 [ 496.617376] rcu: INFO: rcu_sched detected stalls on CPUs/tasks: [ 496.617727] rcu: 0-...!: (0 ticks this GP) idle=e80/0/0x0 softirq=40880/40880 fqs=0 [ 496.618063] (detected by 1, t=5252 jiffies, g=61437, q=7149) [ 496.618085] Sending NMI from CPU 1 to CPUs 0: [ 496.613805] NMI backtrace for cpu 0 [ 496.613833] CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.6.0-rc5+ #31 [ 496.613844] NIP: c00000000000a93c LR: c00000000001b944 CTR: c0000000005028c0 [ 496.613852] REGS: c000000001a47a40 TRAP: 0e81 Not tainted (5.6.0-rc5+) [ 496.613856] MSR: 9000000000009033 <SF,HV,EE,ME,IR,DR,RI,LE> CR: 48000242 XER: 00000000 [ 496.613884] CFAR: c000000000ea0310 IRQMASK: 0 GPR00: c000000000c03620 c000000001a47cd0 c000000001a48f00 0000000028000000 GPR04: 00000000682a0000 00000048dd82a1dd 0000004a31bd97db c000000001cf0000 GPR08: 0000000200000000 0000000000000000 00007fffef5c14a0 0800000042546c02 GPR12: 0000000000002000 c000000001cf0000 [ 496.613941] NIP [c00000000000a93c] replay_interrupt_return+0x0/0x4 [ 496.613951] LR [c00000000001b944] arch_local_irq_restore.part.0+0x54/0x70 [ 496.613956] Call Trace: [ 496.613970] [c000000001a47cd0] [c000000001a47d50] init_stack+0x3d50/0x4000 (unreliable) [ 496.613988] [c000000001a47cf0] [c000000000c03620] cpuidle_enter_state+0xf0/0x590 [ 496.613998] [c000000001a47d70] [c000000000c03b5c] cpuidle_enter+0x4c/0x70 [ 496.614011] [c000000001a47db0] [c00000000018c7cc] call_cpuidle+0x4c/0x90 [ 496.614021] [c000000001a47dd0] [c00000000018cde8] do_idle+0x308/0x420 [ 496.614030] [c000000001a47e70] [c00000000018d148] cpu_startup_entry+0x38/0x40 [ 496.614038] [c000000001a47ea0] [c000000000010800] rest_init+0xe0/0xf8 [ 496.614060] [c000000001a47ed0] [c00000000130445c] start_kernel+0x7c4/0x810 [ 496.614071] [c000000001a47f90] [c00000000000accc] start_here_common+0x1c/0x3e8 [ 496.614078] Instruction dump: [ 496.614091] 7d200026 618c8000 2c030900 4182e778 2c030500 4182f3b0 2c030f00 4182f4c8 [ 496.614109] 2c030a00 4182ffbc 2c030e60 4182f158 <4e800020> 7c781b78 48000329 48000341 [ 496.618540] rcu: rcu_sched kthread starved for 5252 jiffies! g61437 f0x0 RCU_GP_WAIT_FQS(5) ->state=0x402 ->cpu=0 [ 496.618949] rcu: RCU grace-period kthread stack dump: [ 496.619169] rcu_sched I 0 10 2 0x00000808 [ 496.619184] Call Trace: [ 496.619207] [c000000068487890] [c0000000684878c0] 0xc0000000684878c0 (unreliable) [ 496.619231] [c000000068487a70] [c000000000021c6c] __switch_to+0x2dc/0x450 [ 496.619244] [c000000068487ae0] [c000000000e98a14] __schedule+0x2d4/0x920 [ 496.619252] [c000000068487bb0] [c000000000e990d4] schedule+0x74/0x140 [ 496.619260] [c000000068487be0] [c000000000e9ed24] schedule_timeout+0x1a4/0x3e0 [ 496.619271] [c000000068487cc0] [c0000000001eac88] rcu_gp_kthread+0x788/0xd00 [ 496.619280] [c000000068487db0] [c000000000170744] kthread+0x1a4/0x1b0 [ 496.619291] [c000000068487e20] [c00000000000b648] ret_from_kernel_thread+0x5c/0x74 (qemu) info registers NIP c00000000003c95c LR c0000000000c40c4 CTR c000000000c083f0 XER 0000000000000000 CPU#0 MSR 9000000002803033 HID0 0880000000000000 HF 9000000002802031 iidx 5 didx 5 TB 00000079 342952940705 DECR 466647440485 GPR00 c0000000000c4560 c000000001a47b80 c000000001a48f00 0000000000300331 GPR04 c0000000000c40c4 0000000088000242 0000000000000008 c0000000019c8c00 GPR08 c000000001a88f00 0000000000300331 0000000000000000 ffffffffffffffff GPR12 c000000000c083f0 c000000001cf0000 0000000000000001 c000000000000000 GPR16 0000000000000001 c000000001af03b0 c000000001a81ff8 0000000000000000 GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR24 0000000000000000 0000000000000000 0000000000000000 4000000000000000 GPR28 ffffffffffffffff 0000000000000000 0000000080000000 0000000000000000 CR 88000242 [ L L - - - E G E ] RES ffffffffffffffff FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR08 0000000000000000 0000000000000000 0000000000000000 736f6d6570736575 FPR12 6727fe175ed234df 0000000000000000 0000000000000000 0000000000000000 FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPSCR 0000000000000000 SRR0 00007094cb412eac SRR1 900000000280f033 PVR 00000000004e1200 VRSAVE ffffffffffffffff SPRG0 0000000000000000 SPRG1 c000000001cf0000 SPRG2 c000000001cf0000 SPRG3 0000000000000000 SPRG4 0000000000000000 SPRG5 0000000000000000 SPRG6 0000000000000000 SPRG7 0000000000000000 HSRR0 c0000000007f2efc HSRR1 9000000000009033 CFAR c0000000000c40c0 LPCR 0040400001d2f012 PTCR 00000000f7fe0004 DAR 00000ac61eb32280 DSISR 000000000a000000 (qemu) cpu 1 (qemu) info registers NIP c00000000003c95c LR c0000000000c40c4 CTR c000000000c083f0 XER 0000000000000000 CPU#1 MSR 9000000000001033 HID0 0880000000000000 HF 9000000000000031 iidx 5 didx 5 TB 00000081 348632348722 DECR 18446744073709522216 GPR00 c0000000000c4560 c0000000684bbbe0 c000000001a48f00 0000000000300331 GPR04 c0000000000c40c4 0000000088004222 0000000000000808 c000000068479000 GPR08 c000000001a88f00 0000000000300331 0000000000000000 ffffffffffffffff GPR12 c000000000c083f0 c0000000fffff300 0000000000000001 c000000000000000 GPR16 0000000000000001 c000000001af03b0 c000000001a81ff8 0000000000000000 GPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000 GPR24 0000000000000000 0000000000000000 0000000000000000 4000000000000000 GPR28 ffffffffffffffff 0000000000000000 0000000080000000 0000000000000000 CR 88004222 [ L L - - G E E E ] RES ffffffffffffffff FPR00 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR04 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR08 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR12 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR16 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR20 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR24 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPR28 0000000000000000 0000000000000000 0000000000000000 0000000000000000 FPSCR 0000000000000000 SRR0 c00000000000a93c SRR1 9000000000009033 PVR 00000000004e1200 VRSAVE 0000000000000000 SPRG0 0000000000000000 SPRG1 c000000001cf0000 SPRG2 c000000001cf0000 SPRG3 0000000000000001 SPRG4 0000000000000000 SPRG5 0000000000000000 SPRG6 0000000000000000 SPRG7 0000000000000000 HSRR0 c0000000000c1168 HSRR1 9000000000001033 CFAR c0000000000c40c0 LPCR 0040400001d2f012 PTCR 00000000f7fe0004 DAR 00007be52be83320 DSISR 000000000a000000
Cédric Le Goater's on April 4, 2020 1:47 am: > On 4/3/20 3:12 PM, Nicholas Piggin wrote: >> Nicholas Piggin's on April 3, 2020 5:57 pm: >>> Cédric Le Goater's on March 26, 2020 2:38 am: >>>> [ Please use clg@kaod.org ! ] >>>> >>>> On 3/25/20 3:41 PM, Nicholas Piggin wrote: >>>>> This implements the NMI interface for the PNV machine, similarly to >>>>> commit 3431648272d ("spapr: Add support for new NMI interface") for >>>>> SPAPR. >>>>> >>>>> Signed-off-by: Nicholas Piggin <npiggin@gmail.com> >>>> >>>> one minor comment, >>>> >>>> Reviewed-by: Cédric Le Goater <clg@kaod.org> >>>> >>>>> --- >>>>> hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- >>>>> 1 file changed, 29 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >>>>> index b75ad06390..671535ebe6 100644 >>>>> --- a/hw/ppc/pnv.c >>>>> +++ b/hw/ppc/pnv.c >>>>> @@ -27,6 +27,7 @@ >>>>> #include "sysemu/runstate.h" >>>>> #include "sysemu/cpus.h" >>>>> #include "sysemu/device_tree.h" >>>>> +#include "sysemu/hw_accel.h" >>>>> #include "target/ppc/cpu.h" >>>>> #include "qemu/log.h" >>>>> #include "hw/ppc/fdt.h" >>>>> @@ -34,6 +35,7 @@ >>>>> #include "hw/ppc/pnv.h" >>>>> #include "hw/ppc/pnv_core.h" >>>>> #include "hw/loader.h" >>>>> +#include "hw/nmi.h" >>>>> #include "exec/address-spaces.h" >>>>> #include "qapi/visitor.h" >>>>> #include "monitor/monitor.h" >>>>> @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) >>>>> } >>>>> } >>>>> >>>>> +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) >>>>> +{ >>>>> + PowerPCCPU *cpu = POWERPC_CPU(cs); >>>>> + CPUPPCState *env = &cpu->env; >>>>> + >>>>> + cpu_synchronize_state(cs); >>>>> + ppc_cpu_do_system_reset(cs); >>>>> + /* >>>>> + * SRR1[42:45] is set to 0100 which the ISA defines as implementation >>>> >>>> I see 'System Reset' in ISA 3.0 >>>>> + * dependent. POWER processors use this for xscom triggered interrupts, >>>>> + * which come from the BMC or NMI IPIs. >>>>> + */ >>>>> + env->spr[SPR_SRR1] |= PPC_BIT(43); >>>> >>>> So we could have used the skiboot SPR_SRR1_PM_WAKE_SRESET define ? >>> >>> Ah, that's only for power-saving wakeup. But you got me to dig further >>> and I think I've got a few things wrong here. >>> >>> The architectural power save wakeup due to sreset bit 43 needs to be >>> set, probably in excp_helper.c if (msr_pow) test. >>> >>> case POWERPC_EXCP_RESET: /* System reset exception */ >>> /* A power-saving exception sets ME, otherwise it is unchanged */ >>> if (msr_pow) { >>> /* indicate that we resumed from power save mode */ >>> msr |= 0x10000; >>> new_msr |= ((target_ulong)1 << MSR_ME); >>> } >> >> Sorry I'm wrong, that's the old MSR_POW thing I guess G5 had it. >> >> 'stop' state wakeup is powerpc_reset_wakeup of course, which seems to >> do the right thing with SRR1. >> >> Something like this patch should fix this issue in the ppc-5.1 branch. >> This appears to do the right thing with SRR1 in testing with Linux. >> >> --- >> hw/ppc/pnv.c | 21 +++++++++++++++------ >> 1 file changed, 15 insertions(+), 6 deletions(-) >> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c >> index ac83b8698b..596ccfd99e 100644 >> --- a/hw/ppc/pnv.c >> +++ b/hw/ppc/pnv.c >> @@ -1964,12 +1964,21 @@ static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) >> >> cpu_synchronize_state(cs); >> ppc_cpu_do_system_reset(cs); >> - /* >> - * SRR1[42:45] is set to 0100 which the ISA defines as implementation >> - * dependent. POWER processors use this for xscom triggered interrupts, >> - * which come from the BMC or NMI IPIs. >> - */ >> - env->spr[SPR_SRR1] |= PPC_BIT(43); >> + if (env->spr[SPR_SRR1] & PPC_BITMASK(46, 47)) { >> + /* system reset caused wake from power saving state */ >> + if (!(env->spr[SPR_SRR1] & PPC_BIT(43))) { >> + warn_report("ppc_cpu_do_system_reset does not set system reset wakeup reason"); >> + env->spr[SPR_SRR1] |= PPC_BIT(43); >> + } >> + } else { >> + /* >> + * For non-powersave wakeup system resets, SRR1[42:45] are defined to >> + * be implementation dependent. Set to 0b0010, which POWER9 UM defines >> + * to be interrupt caused by SCOM, which can come from the BMC or NMI >> + * IPIs. >> + */ >> + env->spr[SPR_SRR1] |= PPC_BIT(44); >> + } >> } > > That looks correct according to the UM. > > Do we care about the other non-powersave wakeup system resets ? > > 0011 Interrupt caused by hypervisor door bell. > 0101 Interrupt caused by privileged door bell. I think that's a typo in the UM, and those are powersave ones. > > The reason is that I sometime see CPU lockups under load, or with a KVM guest, > and I haven't found why yet. I can't tell what's going on there, does it keep taking e80 interrupts and never clear the doorbell message? Linux wakeup replay code has always been solid. Thanks, Nick
diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c index b75ad06390..671535ebe6 100644 --- a/hw/ppc/pnv.c +++ b/hw/ppc/pnv.c @@ -27,6 +27,7 @@ #include "sysemu/runstate.h" #include "sysemu/cpus.h" #include "sysemu/device_tree.h" +#include "sysemu/hw_accel.h" #include "target/ppc/cpu.h" #include "qemu/log.h" #include "hw/ppc/fdt.h" @@ -34,6 +35,7 @@ #include "hw/ppc/pnv.h" #include "hw/ppc/pnv_core.h" #include "hw/loader.h" +#include "hw/nmi.h" #include "exec/address-spaces.h" #include "qapi/visitor.h" #include "monitor/monitor.h" @@ -1955,10 +1957,35 @@ static void pnv_machine_set_hb(Object *obj, bool value, Error **errp) } } +static void pnv_cpu_do_nmi_on_cpu(CPUState *cs, run_on_cpu_data arg) +{ + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + + cpu_synchronize_state(cs); + ppc_cpu_do_system_reset(cs); + /* + * SRR1[42:45] is set to 0100 which the ISA defines as implementation + * dependent. POWER processors use this for xscom triggered interrupts, + * which come from the BMC or NMI IPIs. + */ + env->spr[SPR_SRR1] |= PPC_BIT(43); +} + +static void pnv_nmi(NMIState *n, int cpu_index, Error **errp) +{ + CPUState *cs; + + CPU_FOREACH(cs) { + async_run_on_cpu(cs, pnv_cpu_do_nmi_on_cpu, RUN_ON_CPU_NULL); + } +} + static void pnv_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); InterruptStatsProviderClass *ispc = INTERRUPT_STATS_PROVIDER_CLASS(oc); + NMIClass *nc = NMI_CLASS(oc); mc->desc = "IBM PowerNV (Non-Virtualized)"; mc->init = pnv_init; @@ -1975,6 +2002,7 @@ static void pnv_machine_class_init(ObjectClass *oc, void *data) mc->default_ram_size = INITRD_LOAD_ADDR + INITRD_MAX_SIZE; mc->default_ram_id = "pnv.ram"; ispc->print_info = pnv_pic_print_info; + nc->nmi_monitor_handler = pnv_nmi; object_class_property_add_bool(oc, "hb-mode", pnv_machine_get_hb, pnv_machine_set_hb, @@ -2038,7 +2066,7 @@ static const TypeInfo types[] = { .class_size = sizeof(PnvMachineClass), .interfaces = (InterfaceInfo[]) { { TYPE_INTERRUPT_STATS_PROVIDER }, - { }, + { TYPE_NMI }, }, }, {
This implements the NMI interface for the PNV machine, similarly to commit 3431648272d ("spapr: Add support for new NMI interface") for SPAPR. Signed-off-by: Nicholas Piggin <npiggin@gmail.com> --- hw/ppc/pnv.c | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-)