Re: [Xen-devel] [PATCH V8 RESEND 4/8] pci.c: Add pci_check_bar_overlap

[Anthony PERARD <anthony.perard@xxxxxxxxxx>]

On 19/03/12 13:15, Michael S. Tsirkin wrote:
> On Fri, Mar 16, 2012 at 04:54:18PM +0000, Anthony PERARD wrote:
> > From: Yuji Shimada<shimada-yxb@xxxxxxxxxxxxxxx>
> >
> > This function helps Xen PCI Passthrough device to check for overlap.
> >
> > Signed-off-by: Yuji Shimada<shimada-yxb@xxxxxxxxxxxxxxx>
> > Signed-off-by: Anthony PERARD<anthony.perard@xxxxxxxxxx>
> It seems that what's called for here really is
> using the new memory region infrastructure.
> That handles overlap etc nicely.
>
> That said, I don't mind, but would prefer to
> keep this mess outside the pci core. See below.
>
> > ---
> >   hw/pci.c |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
> >   hw/pci.h |    5 +++++
> >   2 files changed, 55 insertions(+), 0 deletions(-)
> >
> > diff --git a/hw/pci.c b/hw/pci.c
> > index 38e1de5..f950b4e 100644
> > --- a/hw/pci.c
> > +++ b/hw/pci.c
> > @@ -1992,6 +1992,56 @@ MemoryRegion *pci_address_space_io(PCIDevice *dev)
> >       return dev->bus->address_space_io;
> >   }
> >
> > +/* This function
> Comment blocks start with /* */
>
> > checks if an io_region overlap an io_region from another
> overlaps
>
> > + * device.  The io_region to check is provide with (addr, size and type)
> provided
>
> > + * A callback can be provide and will be called for every region that is
> provided
>
> > + * overlapped.
> > + * The return value indicate if the region is overlappsed */
> indicates
>
>
> > +bool pci_check_bar_overlap(PCIDevice *device,
> > +                           pcibus_t addr, pcibus_t size, uint8_t type,
> > +                           void (*c)(void *o, const PCIDevice *d, int index),
> > +                           void *opaque)
> IMO this is inlikely to be needed by anyone except Xen.
> How about a generic pci_foreach_device and let Xen
> implement the hacks internally.
Ok, this should be better, I'll work on that.

Thanks,

> > +{
> > +    PCIBus *bus = device->bus;
> > +    int i, j;
> > +    bool rc = false;
> > +
> > +    for (i = 0; i<  ARRAY_SIZE(bus->devices); i++) {
> > +        PCIDevice *d = bus->devices[i];
> > +        if (!d) {
> > +            continue;
> > +        }
> > +
> > +        if (d->devfn == device->devfn) {
> > +            continue;
> > +        }
> > +
> > +        /* xxx: This ignores bridges. */
> > +        for (j = 0; j<  PCI_NUM_REGIONS; j++) {
> > +            PCIIORegion *r =&d->io_regions[j];
> > +
> > +            if (!r->size) {
> > +                continue;
> > +            }
> > +            if ((type&  PCI_BASE_ADDRESS_SPACE_IO)
> > +                != (r->type&  PCI_BASE_ADDRESS_SPACE_IO)) {
> > +                continue;
> > +            }
> > +
> > +            if (ranges_overlap(addr, size, r->addr, r->size)) {
> > +                if (c) {
> > +                    c(opaque, d, j);
> > +                    rc = true;
> > +                } else {
> > +                    return true;
> > +                }
> > +            }
> > +        }
> > +    }
> > +
> > +    return rc;
> > +}
> > +
> >   static void pci_device_class_init(ObjectClass *klass, void *data)
> >   {
> >       DeviceClass *k = DEVICE_CLASS(klass);
> > diff --git a/hw/pci.h b/hw/pci.h
> > index 4f19fdb..cbd04e1 100644
> > --- a/hw/pci.h
> > +++ b/hw/pci.h
> > @@ -628,4 +628,9 @@ extern const VMStateDescription vmstate_pci_device;
> >       .offset     = vmstate_offset_pointer(_state, _field, PCIDevice), \
> >   }
> >
> > +bool pci_check_bar_overlap(PCIDevice *dev,
> > +                           pcibus_t addr, pcibus_t size, uint8_t type,
> > +                           void (*c)(void *o, const PCIDevice *d, int index),
> > +                           void *opaque);
> > +
> >   #endif
> > --
> > Anthony PERARD

--
Anthony PERARD

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel