From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony PERARD Subject: Re: [PATCH V8 RESEND 4/8] pci.c: Add pci_check_bar_overlap Date: Mon, 19 Mar 2012 17:22:16 +0000 Message-ID: <4F676B48.6010704@citrix.com> References: <1331916862-20504-1-git-send-email-anthony.perard@citrix.com> <1331916862-20504-5-git-send-email-anthony.perard@citrix.com> <20120319131529.GB4591@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset="ISO-8859-1"; format=flowed Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <20120319131529.GB4591@redhat.com> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org Sender: qemu-devel-bounces+gceq-qemu-devel=gmane.org@nongnu.org To: "Michael S. Tsirkin" Cc: Yuji Shimada , Xen Devel , QEMU-devel , Stefano Stabellini List-Id: xen-devel@lists.xenproject.org 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 >> >> This function helps Xen PCI Passthrough device to check for overlap. >> >> Signed-off-by: Yuji Shimada >> Signed-off-by: Anthony PERARD > > 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