| Message ID | 20160705184740-mutt-send-email-mst@redhat.com |
|---|---|
| State | New |
| Headers | show |
On (Tue) 05 Jul 2016 [18:47:40], Michael S. Tsirkin wrote: > From: Cao jin <caoj.fnst@cn.fujitsu.com> > > internal flag msi_in_use in unnecessary, msi_uninit() could be called > directly, and msi_enabled() is enough to check device msi state. > > cc: Markus Armbruster <armbru@redhat.com> > cc: Marcel Apfelbaum <marcel@redhat.com> > cc: Paolo Bonzini <pbonzini@redhat.com> > cc: Michael S. Tsirkin <mst@redhat.com> > > Reviewed-by: Markus Armbruster <armbru@redhat.com> > Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> > Reviewed-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> [...] > static void mptsas_reset(DeviceState *dev) > @@ -1375,7 +1370,6 @@ static const VMStateDescription vmstate_mptsas = { > .post_load = mptsas_post_load, > .fields = (VMStateField[]) { > VMSTATE_PCI_DEVICE(dev, MPTSASState), > - VMSTATE_BOOL(msi_in_use, MPTSASState), This removes vmstate -- please use 'unused' instead of removing this value. Flagged by the static checker. Amit
On 07/26/2016 01:01 PM, Amit Shah wrote: > On (Tue) 05 Jul 2016 [18:47:40], Michael S. Tsirkin wrote: >> From: Cao jin <caoj.fnst@cn.fujitsu.com> >> >> internal flag msi_in_use in unnecessary, msi_uninit() could be called >> directly, and msi_enabled() is enough to check device msi state. >> >> cc: Markus Armbruster <armbru@redhat.com> >> cc: Marcel Apfelbaum <marcel@redhat.com> >> cc: Paolo Bonzini <pbonzini@redhat.com> >> cc: Michael S. Tsirkin <mst@redhat.com> >> >> Reviewed-by: Markus Armbruster <armbru@redhat.com> >> Signed-off-by: Cao jin <caoj.fnst@cn.fujitsu.com> >> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> >> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > > [...] > >> static void mptsas_reset(DeviceState *dev) >> @@ -1375,7 +1370,6 @@ static const VMStateDescription vmstate_mptsas = { >> .post_load = mptsas_post_load, >> .fields = (VMStateField[]) { >> VMSTATE_PCI_DEVICE(dev, MPTSASState), >> - VMSTATE_BOOL(msi_in_use, MPTSASState), > > This removes vmstate -- please use 'unused' instead of removing this > value. > > Flagged by the static checker. > > Hi Amit I will take care of this. BTW, did't see it in coverity scan outstanding defects, Do I missed or it is checked by other static check tools?
On (Tue) 26 Jul 2016 [15:29:36], Cao jin wrote: > Hi Amit > > I will take care of this. > BTW, did't see it in coverity scan outstanding defects, Do I missed or it is > checked by other static check tools? This is checked with the vmstate static checker -- scripts/vmstate-static-checker.py. The -dump-vmstate cmdline option to qemu gives a json file that the static checker uses as input. Get a 'before' and 'after' version of the json files, and pass those on to the checker with '-s' and '-d' arguments respectively. Thanks, Amit
On Tue, Jul 26, 2016 at 04:48:06PM +0530, Amit Shah wrote: > On (Tue) 26 Jul 2016 [15:29:36], Cao jin wrote: > > Hi Amit > > > > I will take care of this. > > BTW, did't see it in coverity scan outstanding defects, Do I missed or it is > > checked by other static check tools? > > This is checked with the vmstate static checker -- > scripts/vmstate-static-checker.py. > > The -dump-vmstate cmdline option to qemu gives a json file that the > static checker uses as input. Get a 'before' and 'after' version of > the json files, and pass those on to the checker with '-s' and '-d' > arguments respectively. > > Thanks, > > Amit How about adding this to make check? You can run this with a given machine type to avoid too much churn.
diff --git a/hw/scsi/mptsas.h b/hw/scsi/mptsas.h index 0436a33..da014a3 100644 --- a/hw/scsi/mptsas.h +++ b/hw/scsi/mptsas.h @@ -31,8 +31,6 @@ struct MPTSASState { OnOffAuto msi; uint64_t sas_addr; - bool msi_in_use; - /* Doorbell register */ uint32_t state; uint8_t who_init; diff --git a/hw/scsi/mptsas.c b/hw/scsi/mptsas.c index bb056c4..1ae32fb 100644 --- a/hw/scsi/mptsas.c +++ b/hw/scsi/mptsas.c @@ -63,7 +63,7 @@ static void mptsas_update_interrupt(MPTSASState *s) PCIDevice *pci = (PCIDevice *) s; uint32_t state = s->intr_status & ~(s->intr_mask | MPI_HIS_IOP_DOORBELL_STATUS); - if (s->msi_in_use && msi_enabled(pci)) { + if (msi_enabled(pci)) { if (state) { trace_mptsas_irq_msi(s); msi_notify(pci, 0); @@ -1289,15 +1289,12 @@ static void mptsas_scsi_init(PCIDevice *dev, Error **errp) error_append_hint(&err, "You have to use msi=auto (default) or " "msi=off with this machine type.\n"); error_propagate(errp, err); - s->msi_in_use = false; return; - } else if (ret) { - /* With msi=auto, we fall back to MSI off silently */ - error_free(err); - s->msi_in_use = false; - } else { - s->msi_in_use = true; } + assert(!err || s->msi == ON_OFF_AUTO_AUTO); + /* With msi=auto, we fall back to MSI off silently */ + error_free(err); + } memory_region_init_io(&s->mmio_io, OBJECT(s), &mptsas_mmio_ops, s, @@ -1337,9 +1334,7 @@ static void mptsas_scsi_uninit(PCIDevice *dev) MPTSASState *s = MPT_SAS(dev); qemu_bh_delete(s->request_bh); - if (s->msi_in_use) { - msi_uninit(dev); - } + msi_uninit(dev); } static void mptsas_reset(DeviceState *dev) @@ -1375,7 +1370,6 @@ static const VMStateDescription vmstate_mptsas = { .post_load = mptsas_post_load, .fields = (VMStateField[]) { VMSTATE_PCI_DEVICE(dev, MPTSASState), - VMSTATE_BOOL(msi_in_use, MPTSASState), VMSTATE_UINT32(state, MPTSASState), VMSTATE_UINT8(who_init, MPTSASState),