| Message ID | 1395886880-11021-3-git-send-email-aik@ozlabs.ru |
|---|---|
| State | New |
| Headers | show |
Il 27/03/2014 03:21, Alexey Kardashevskiy ha scritto: > This defines and makes use of an NMI interface in order to support > the "nmi" command. > > Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> > --- > hw/ppc/spapr.c | 26 ++++++++++++++++++++++++++ > 1 file changed, 26 insertions(+) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index 62ddb4d..495fa88 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -48,6 +48,7 @@ > #include "hw/pci/pci.h" > #include "hw/scsi/scsi.h" > #include "hw/virtio/virtio-scsi.h" > +#include "hw/nmi.h" > > #include "exec/address-spaces.h" > #include "hw/usb.h" > @@ -1539,13 +1540,36 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, > return NULL; > } > > +static void spapr_do_nmi(void *arg) > +{ > + CPUState *cs = arg; > + PowerPCCPU *cpu = POWERPC_CPU(cs); > + CPUPPCState *env = &cpu->env; > + > + cpu_synchronize_state(cs); > + env->spr[SPR_SRR0] = env->nip; > + env->spr[SPR_SRR1] = env->msr; > + env->nip = 0x100; > + env->msr = (1ULL << MSR_SF) | (1 << MSR_ME); > + if (env->spr[SPR_LPCR] & LPCR_ILE) { > + env->msr |= 1 << MSR_LE; > + } > +} I think an interface isn't the right tool here. You want a method in CPUClass, and you also should move the existing code for x86 and s390 to target-i386 and target-s390. Paolo
On 03/27/2014 10:58 PM, Paolo Bonzini wrote: > Il 27/03/2014 03:21, Alexey Kardashevskiy ha scritto: >> This defines and makes use of an NMI interface in order to support >> the "nmi" command. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> hw/ppc/spapr.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 62ddb4d..495fa88 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -48,6 +48,7 @@ >> #include "hw/pci/pci.h" >> #include "hw/scsi/scsi.h" >> #include "hw/virtio/virtio-scsi.h" >> +#include "hw/nmi.h" >> >> #include "exec/address-spaces.h" >> #include "hw/usb.h" >> @@ -1539,13 +1540,36 @@ static char *spapr_get_fw_dev_path(FWPathProvider >> *p, BusState *bus, >> return NULL; >> } >> >> +static void spapr_do_nmi(void *arg) >> +{ >> + CPUState *cs = arg; >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + CPUPPCState *env = &cpu->env; >> + >> + cpu_synchronize_state(cs); >> + env->spr[SPR_SRR0] = env->nip; >> + env->spr[SPR_SRR1] = env->msr; >> + env->nip = 0x100; >> + env->msr = (1ULL << MSR_SF) | (1 << MSR_ME); >> + if (env->spr[SPR_LPCR] & LPCR_ILE) { >> + env->msr |= 1 << MSR_LE; >> + } >> +} > > > I think an interface isn't the right tool here. You want a method in > CPUClass, and you also should move the existing code for x86 and s390 to > target-i386 and target-s390. I can do that, no big deal (bit afraid there will be some third approach then :) ), but 1) how many x86 CPUs/families are there to support? On ppc I'll add it for POWER7/7+/8 families and I am ok. 2) what should "nmi" really do - deliver NMI to the current CPU (as s390 does) or on all CPUs (as x86 does)?
> Am 27.03.2014 um 19:58 schrieb Paolo Bonzini <pbonzini@redhat.com>: > > Il 27/03/2014 03:21, Alexey Kardashevskiy ha scritto: >> This defines and makes use of an NMI interface in order to support >> the "nmi" command. >> >> Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> >> --- >> hw/ppc/spapr.c | 26 ++++++++++++++++++++++++++ >> 1 file changed, 26 insertions(+) >> >> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c >> index 62ddb4d..495fa88 100644 >> --- a/hw/ppc/spapr.c >> +++ b/hw/ppc/spapr.c >> @@ -48,6 +48,7 @@ >> #include "hw/pci/pci.h" >> #include "hw/scsi/scsi.h" >> #include "hw/virtio/virtio-scsi.h" >> +#include "hw/nmi.h" >> >> #include "exec/address-spaces.h" >> #include "hw/usb.h" >> @@ -1539,13 +1540,36 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, >> return NULL; >> } >> >> +static void spapr_do_nmi(void *arg) >> +{ >> + CPUState *cs = arg; >> + PowerPCCPU *cpu = POWERPC_CPU(cs); >> + CPUPPCState *env = &cpu->env; >> + >> + cpu_synchronize_state(cs); >> + env->spr[SPR_SRR0] = env->nip; >> + env->spr[SPR_SRR1] = env->msr; >> + env->nip = 0x100; >> + env->msr = (1ULL << MSR_SF) | (1 << MSR_ME); >> + if (env->spr[SPR_LPCR] & LPCR_ILE) { >> + env->msr |= 1 << MSR_LE; >> + } >> +} > > > I think an interface isn't the right tool here. You want a method in CPUClass, and you also should move the existing code for x86 and s390 to target-i386 and target-s390. Also the code above is duplicating the existing sreset pin logic. We should either just trigger the sreset itq line or - if that's too hard to get working with kvm - at least share the code. Alex > > Paolo >
Il 27/03/2014 14:14, Alexey Kardashevskiy ha scritto: > 1) how many x86 CPUs/families are there to support? On ppc I'll add it for > POWER7/7+/8 families and I am ok. Just move the existing x86 code. It works on all CPUs from 486 on. > 2) what should "nmi" really do - deliver NMI to the current CPU (as s390 > does) or on all CPUs (as x86 does)? As you prefer. Current CPU, as s390 does, was in your patch here and it's fine. I don't know why x86 delivers it to all CPUs. Paolo
diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 62ddb4d..495fa88 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -48,6 +48,7 @@ #include "hw/pci/pci.h" #include "hw/scsi/scsi.h" #include "hw/virtio/virtio-scsi.h" +#include "hw/nmi.h" #include "exec/address-spaces.h" #include "hw/usb.h" @@ -1539,13 +1540,36 @@ static char *spapr_get_fw_dev_path(FWPathProvider *p, BusState *bus, return NULL; } +static void spapr_do_nmi(void *arg) +{ + CPUState *cs = arg; + PowerPCCPU *cpu = POWERPC_CPU(cs); + CPUPPCState *env = &cpu->env; + + cpu_synchronize_state(cs); + env->spr[SPR_SRR0] = env->nip; + env->spr[SPR_SRR1] = env->msr; + env->nip = 0x100; + env->msr = (1ULL << MSR_SF) | (1 << MSR_ME); + if (env->spr[SPR_LPCR] & LPCR_ILE) { + env->msr |= 1 << MSR_LE; + } +} + +void spapr_deliver_nmi(NMIObj *p, CPUState *cs) +{ + async_run_on_cpu(cs, spapr_do_nmi, cs); +} + static void spapr_machine_class_init(ObjectClass *oc, void *data) { MachineClass *mc = MACHINE_CLASS(oc); FWPathProviderClass *fwc = FW_PATH_PROVIDER_CLASS(oc); + NMIClass *nmic = NMI_CLASS(oc); mc->qemu_machine = data; fwc->get_dev_path = spapr_get_fw_dev_path; + nmic->deliver_nmi = spapr_deliver_nmi; } static const TypeInfo spapr_machine_info = { @@ -1555,10 +1579,12 @@ static const TypeInfo spapr_machine_info = { .class_data = &spapr_machine, .interfaces = (InterfaceInfo[]) { { TYPE_FW_PATH_PROVIDER }, + { TYPE_NMI }, { } }, }; + static void spapr_machine_register_types(void) { type_register_static(&spapr_machine_info);
This defines and makes use of an NMI interface in order to support the "nmi" command. Signed-off-by: Alexey Kardashevskiy <aik@ozlabs.ru> --- hw/ppc/spapr.c | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+)