[Qemu-devel] Re: [PATCH RFC] msi: remove range checks

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

Hmm, does this simply check that write is within the capability?
Then I think I misunderstood what this does.
Please replace hard-coded length logic with existing function call
that gets the lengths, the range check can stay then.


On Mon, Oct 25, 2010 at 04:07:13PM +0900, Isaku Yamahata wrote:
> Seems good.
> For consistency, the range check in msix_write_config() should
> be also removed.
> 
> On Mon, Oct 25, 2010 at 08:07:25AM +0200, Michael S. Tsirkin wrote:
> > config write handlers should be idempotent.
> > So no need for range checks.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  hw/msi.c |   29 -----------------------------
> >  1 files changed, 0 insertions(+), 29 deletions(-)
> > 
> > diff --git a/hw/msi.c b/hw/msi.c
> > index a949d82..af8ea44 100644
> > --- a/hw/msi.c
> > +++ b/hw/msi.c
> > @@ -257,35 +257,6 @@ void msi_write_config(PCIDevice *dev, uint32_t addr, uint32_t val, int len)
> >      uint32_t pending;
> >      int i;
> >  
> > -#ifdef MSI_DEBUG
> > -    if (ranges_overlap(addr, len, dev->msi_cap, msi_cap_sizeof(flags))) {
> > -        MSI_DEV_PRINTF(dev, "addr 0x%"PRIx32" val 0x%"PRIx32" len %d\n",
> > -                       addr, val, len);
> > -        MSI_DEV_PRINTF(dev, "ctrl: 0x%"PRIx16" address: 0x%"PRIx32,
> > -                       flags,
> > -                       pci_get_long(dev->config + msi_address_lo_off(dev)));
> > -        if (msi64bit) {
> > -            fprintf(stderr, " addrss-hi: 0x%"PRIx32,
> > -                    pci_get_long(dev->config + msi_address_hi_off(dev)));
> > -        }
> > -        fprintf(stderr, " data: 0x%"PRIx16,
> > -                pci_get_word(dev->config + msi_data_off(dev, msi64bit)));
> > -        if (flags & PCI_MSI_FLAGS_MASKBIT) {
> > -            fprintf(stderr, " mask 0x%"PRIx32" pending 0x%"PRIx32,
> > -                    pci_get_long(dev->config + msi_mask_off(dev, msi64bit)),
> > -                    pci_get_long(dev->config + msi_pending_off(dev, msi64bit)));
> > -        }
> > -        fprintf(stderr, "\n");
> > -    }
> > -#endif
> > -
> > -    /* Are we modified? */
> > -    if (!(ranges_overlap(addr, len, msi_flags_off(dev), 2) ||
> > -          (msi_per_vector_mask &&
> > -           ranges_overlap(addr, len, msi_mask_off(dev, msi64bit), 4)))) {
> > -        return;
> > -    }
> > -
> >      if (!(flags & PCI_MSI_FLAGS_ENABLE)) {
> >          return;
> >      }
> > -- 
> > 1.7.3-rc1
> > 
> 
> -- 
> yamahata