[Qemu-devel] Re: [PATCH 07/14] msi: implemented msi.

["Michael S. Tsirkin" <mst@redhat.com>]

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