On Wed, Sep 08, 2010 at 04:43:27PM +0900, Isaku Yamahata wrote: > Thank you for through review. > > On Mon, Sep 06, 2010 at 12:44:16PM +0300, Michael S. Tsirkin wrote: > > > + uint32_t pending = > > > + pci_get_long(dev->config + msi_pending_reg(dev, is64bit)); > > > + uint8_t vector; > > > + > > > + /* deliver pending interrupts which are unmasked */ > > > + for (vector = 0; vector < nr_vector; ++vector) { > > > + if (msi_is_masked(dev, vector) || !msi_test_bit(pending, > > > vector)) { > > > > I am confused. This is called after mask is updated, right? > > So msi_is_masked means vector was masked, not unmasked? > > I think the logic is reversed here. > > I suppose you were missing the following continue. > > > > > + continue; > ^^^^^^^^^ Here
I see. You are right. The block is small enough to maybe make it worthwhile to revert the logic and avoid continue. It's up to you though, there's no bug here. > > > + } > > > + > > > + msi_clear_bit(&pending, vector); > > > + pci_set_long(dev->config + msi_pending_reg(dev, is64bit), > > > pending); > > > + msi_notify(dev, vector); > > > + } > > > + } > > > +} > > > + > > > +uint8_t msi_nr_allocated_vector(const PCIDevice *dev) > > -- > yamahata