From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:50995) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QvvfT-0007Cx-CM for qemu-devel@nongnu.org; Tue, 23 Aug 2011 14:25:44 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QvvfS-0001hq-2n for qemu-devel@nongnu.org; Tue, 23 Aug 2011 14:25:43 -0400 Received: from mx1.redhat.com ([209.132.183.28]:31036) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QvvfR-0001hf-PD for qemu-devel@nongnu.org; Tue, 23 Aug 2011 14:25:42 -0400 Date: Tue, 23 Aug 2011 21:26:32 +0300 From: "Michael S. Tsirkin" Message-ID: <20110823182632.GA6489@redhat.com> References: <4E53E328.90601@siemens.com> <20110823181751.GB6326@redhat.com> <1314123707.2859.87.camel@bling.home> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1314123707.2859.87.camel@bling.home> Subject: Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alex Williamson Cc: Jan Kiszka , qemu-devel On Tue, Aug 23, 2011 at 12:21:47PM -0600, Alex Williamson wrote: > On Tue, 2011-08-23 at 21:17 +0300, Michael S. Tsirkin wrote: > > On Tue, Aug 23, 2011 at 07:28:08PM +0200, Jan Kiszka wrote: > > > From: Alex Williamson > > > > > > Nothing good can happen when we overlap capabilities > > > > > > [ Jan: rebased over qemu, minor formatting ] > > > > > > Signed-off-by: Jan Kiszka > > > > I'll stick an assert there instead. Normal devices > > don't generate overlapping caps unless there's a bug, > > and device assignment should do it's own checks. > > > > I really have a mind to rip out the used array too. > > So you'd rather kill qemu rather than have a reasonable error return > path... great :( > > Alex Well that will make it possible to make pci_add_capability return void, less work for callers :) Dev assignment is really the only place where capability offsets need to be verified. > > > --- > > > hw/pci.c | 14 ++++++++++++++ > > > 1 files changed, 14 insertions(+), 0 deletions(-) > > > > > > diff --git a/hw/pci.c b/hw/pci.c > > > index 6124790..ff20631 100644 > > > --- a/hw/pci.c > > > +++ b/hw/pci.c > > > @@ -1952,11 +1952,25 @@ int pci_add_capability(PCIDevice *pdev, uint8_t cap_id, > > > uint8_t offset, uint8_t size) > > > { > > > uint8_t *config; > > > + int i; > > > + > > > if (!offset) { > > > offset = pci_find_space(pdev, size); > > > if (!offset) { > > > return -ENOSPC; > > > } > > > + } else { > > > + for (i = offset; i < offset + size; i++) { > > > + if (pdev->used[i]) { > > > + fprintf(stderr, "ERROR: %04x:%02x:%02x.%x " > > > + "Attempt to add PCI capability %x at offset " > > > + "%x overlaps existing capability %x at offset %x\n", > > > + pci_find_domain(pdev->bus), pci_bus_num(pdev->bus), > > > + PCI_SLOT(pdev->devfn), PCI_FUNC(pdev->devfn), > > > + cap_id, offset, pdev->config_map[i], i); > > > + return -EFAULT; > > > + } > > > + } > > > } > > > > > > config = pdev->config + offset; > > > -- > > > 1.7.3.4 > >