| Message ID | 1453824953-27230-1-git-send-email-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Tue, Jan 26, 2016 at 05:15:53PM +0100, Paolo Bonzini wrote: > This is not necessary and actually causes a hang; it was probably copied > and pasted from KVM code, that is one of the very few places that run > outside iothread lock. > > Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> Makes sense. I wonder how hard would it be to have a test to catch this. > --- > hw/ipmi/ipmi.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c > index 52aba1e..fcba0ca 100644 > --- a/hw/ipmi/ipmi.c > +++ b/hw/ipmi/ipmi.c > @@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly) > if (checkonly) { > return 0; > } > - qemu_mutex_lock_iothread(); > qmp_inject_nmi(NULL); > - qemu_mutex_unlock_iothread(); > return 0; > > case IPMI_POWERCYCLE_CHASSIS: > -- > 2.5.0
On 01/26/2016 10:21 AM, Michael S. Tsirkin wrote: > On Tue, Jan 26, 2016 at 05:15:53PM +0100, Paolo Bonzini wrote: >> This is not necessary and actually causes a hang; it was probably copied >> and pasted from KVM code, that is one of the very few places that run >> outside iothread lock. >> >> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> > Makes sense. I wonder how hard would it be to have a test to catch this. The IPMI side would be easy for me, but I don't know how to handle catching the NMI. -corey >> --- >> hw/ipmi/ipmi.c | 2 -- >> 1 file changed, 2 deletions(-) >> >> diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c >> index 52aba1e..fcba0ca 100644 >> --- a/hw/ipmi/ipmi.c >> +++ b/hw/ipmi/ipmi.c >> @@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly) >> if (checkonly) { >> return 0; >> } >> - qemu_mutex_lock_iothread(); >> qmp_inject_nmi(NULL); >> - qemu_mutex_unlock_iothread(); >> return 0; >> >> case IPMI_POWERCYCLE_CHASSIS: >> -- >> 2.5.0
On 26/01/2016 18:57, Corey Minyard wrote: > On 01/26/2016 10:21 AM, Michael S. Tsirkin wrote: >> On Tue, Jan 26, 2016 at 05:15:53PM +0100, Paolo Bonzini wrote: >>> This is not necessary and actually causes a hang; it was probably copied >>> and pasted from KVM code, that is one of the very few places that run >>> outside iothread lock. >>> >>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> >> Makes sense. I wonder how hard would it be to have a test to catch this. > > The IPMI side would be easy for me, but I don't know how to handle > catching the NMI. I'm afraid there's no way to trap an NMI from qtest. Paolo
diff --git a/hw/ipmi/ipmi.c b/hw/ipmi/ipmi.c index 52aba1e..fcba0ca 100644 --- a/hw/ipmi/ipmi.c +++ b/hw/ipmi/ipmi.c @@ -50,9 +50,7 @@ static int ipmi_do_hw_op(IPMIInterface *s, enum ipmi_op op, int checkonly) if (checkonly) { return 0; } - qemu_mutex_lock_iothread(); qmp_inject_nmi(NULL); - qemu_mutex_unlock_iothread(); return 0; case IPMI_POWERCYCLE_CHASSIS:
This is not necessary and actually causes a hang; it was probably copied and pasted from KVM code, that is one of the very few places that run outside iothread lock. Signed-off-by: Paolo Bonzini <pbonzini@redhat.com> --- hw/ipmi/ipmi.c | 2 -- 1 file changed, 2 deletions(-)