From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:36063) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwCeN-0001iW-0J for qemu-devel@nongnu.org; Wed, 24 Aug 2011 08:33:43 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1QwCeL-0000ex-Ly for qemu-devel@nongnu.org; Wed, 24 Aug 2011 08:33:42 -0400 Received: from mx1.redhat.com ([209.132.183.28]:1026) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1QwCeL-0000es-AV for qemu-devel@nongnu.org; Wed, 24 Aug 2011 08:33:41 -0400 Date: Wed, 24 Aug 2011 15:34:30 +0300 From: "Michael S. Tsirkin" Message-ID: <20110824123430.GA18717@redhat.com> References: <4E53E328.90601@siemens.com> <20110824100439.GA17255@redhat.com> <4E54CE18.1080508@siemens.com> <20110824115816.GA18393@redhat.com> <4E54EEB0.2070803@siemens.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <4E54EEB0.2070803@siemens.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH] pci: Error on PCI capability collisions List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jan Kiszka Cc: Alex Williamson , qemu-devel On Wed, Aug 24, 2011 at 02:29:36PM +0200, Jan Kiszka wrote: > On 2011-08-24 13:58, Michael S. Tsirkin wrote: > > On Wed, Aug 24, 2011 at 12:10:32PM +0200, Jan Kiszka wrote: > >> On 2011-08-24 12:04, 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 > >>> > >>> This doesn't build for me: > >>> > >>> /scm/qemu/hw/pci.c: In function =E2=80=98pci_add_capability=E2=80=99= : > >>> /scm/qemu/hw/pci.c:1970:45: error: =E2=80=98PCIDevice=E2=80=99 has = no member named =E2=80=98config_map=E2=80=99 > >> > >> Yeah, sorry, forgot to refresh the commit before posting. > >> > >>> > >>> I think that what that includes is the capability including each gi= ven > >>> offset, right? It would be easy to write some code scanning the > >>> capability list to figure this value out. > >>> Something along the lines of (untested): > >>> > >>> static > >>> uint8_t pci_find_capability_at_offset(PCIDevice *pdev, uint8_t offs= et) > >>> { =20 > >>> uint8_t next, prev, found =3D 0; > >>> > >>> if (!(pdev->config[PCI_STATUS] & PCI_STATUS_CAP_LIST)) > >>> return 0; > >>> > >>> for (prev =3D PCI_CAPABILITY_LIST; (next =3D pdev->config[prev]= ); > >>> prev =3D next + PCI_CAP_LIST_NEXT) > >>> if (next <=3D offset && next > found) > >>> found =3D next; > >>> > >>> return found; > >>> } > >> > >> Sounds useful, will enhance the patch. > >> > >> (Originally, I just wanted to reduce the qemu-kvm delta... :) ) > >> > >> Jan > >=20 > > Also, let's add a comment documenting the > > reason for this check: device assignment > > depends on this check to verify that the device > > is not broken. >=20 > Based on the previous discussion, I don't think this is accurate as it > will also validate emulated devices. >=20 > Jan Something like the below is accurate, right? /* Device assignment depends on this check to verify that the device is not broken. Should never trigger for emulated devices, but it's helpful for debugging these. */ > --=20 > Siemens AG, Corporate Technology, CT T DE IT 1 > Corporate Competence Center Embedded Linux