From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([208.118.235.92]:44763) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPKzj-0002vx-JA for qemu-devel@nongnu.org; Tue, 01 May 2012 17:52:29 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1SPKzh-0007vT-Is for qemu-devel@nongnu.org; Tue, 01 May 2012 17:52:27 -0400 Received: from cantor2.suse.de ([195.135.220.15]:42414 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1SPKzh-0007v6-9s for qemu-devel@nongnu.org; Tue, 01 May 2012 17:52:25 -0400 Message-ID: <4FA05B16.4040806@suse.de> Date: Tue, 01 May 2012 23:52:22 +0200 From: =?ISO-8859-15?Q?Andreas_F=E4rber?= MIME-Version: 1.0 References: <1335558083-26196-1-git-send-email-mdroth@linux.vnet.ibm.com> <1335558083-26196-7-git-send-email-mdroth@linux.vnet.ibm.com> In-Reply-To: <1335558083-26196-7-git-send-email-mdroth@linux.vnet.ibm.com> Content-Type: text/plain; charset=ISO-8859-15 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 6/7] qdev: use int32_t container for devfn property List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Michael Roth Cc: pbonzini@redhat.com, aliguori@us.ibm.com, qemu-devel@nongnu.org, "Michael S. Tsirkin" Am 27.04.2012 22:21, schrieb Michael Roth: > Valid range for devfn is -1 to 255 (-1 for automatic assignment). We do > not currently validate this due to devfn being stored as a uint32_t. > This can lead to segfaults and other strange behavior. >=20 > We could technically just cast it to int32_t to implement the checking, > but this will not work for visitor-based setting where we may do additi= onal > bounds-checking based on target container type, which is int32_t for th= is > case. >=20 > Signed-off-by: Michael Roth Reviewed-by: Andreas F=E4rber Upper limit matches my limited PCI knowledge; cc'ing mst. /-F > --- > hw/pci.c | 2 +- > hw/pci.h | 2 +- > hw/qdev-properties.c | 11 ++++------- > hw/qdev.h | 2 +- > 4 files changed, 7 insertions(+), 10 deletions(-) >=20 > diff --git a/hw/pci.c b/hw/pci.c > index b706e69..7818c9b 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1538,7 +1538,7 @@ PCIDevice *pci_create_multifunction(PCIBus *bus, = int devfn, bool multifunction, > DeviceState *dev; > =20 > dev =3D qdev_create(&bus->qbus, name); > - qdev_prop_set_uint32(dev, "addr", devfn); > + qdev_prop_set_int32(dev, "addr", devfn); > qdev_prop_set_bit(dev, "multifunction", multifunction); > return PCI_DEVICE(dev); > } > diff --git a/hw/pci.h b/hw/pci.h > index 8d0aa49..3bc9218 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -193,7 +193,7 @@ struct PCIDevice { > =20 > /* the following fields are read only */ > PCIBus *bus; > - uint32_t devfn; > + int32_t devfn; > char name[64]; > PCIIORegion io_regions[PCI_NUM_REGIONS]; > =20 > diff --git a/hw/qdev-properties.c b/hw/qdev-properties.c > index 98dd06a..36d0aa0 100644 > --- a/hw/qdev-properties.c > +++ b/hw/qdev-properties.c > @@ -822,7 +822,7 @@ static void set_pci_devfn(Object *obj, Visitor *v, = void *opaque, > { > DeviceState *dev =3D DEVICE(obj); > Property *prop =3D opaque; > - uint32_t *ptr =3D qdev_get_prop_ptr(dev, prop); > + int32_t *ptr =3D qdev_get_prop_ptr(dev, prop); > unsigned int slot, fn, n; > Error *local_err =3D NULL; > char *str =3D (char *)""; > @@ -855,7 +855,7 @@ invalid: > =20 > static int print_pci_devfn(DeviceState *dev, Property *prop, char *des= t, size_t len) > { > - uint32_t *ptr =3D qdev_get_prop_ptr(dev, prop); > + int32_t *ptr =3D qdev_get_prop_ptr(dev, prop); > =20 > if (*ptr =3D=3D -1) { > return snprintf(dest, len, ""); > @@ -870,11 +870,8 @@ PropertyInfo qdev_prop_pci_devfn =3D { > .print =3D print_pci_devfn, > .get =3D get_int32, > .set =3D set_pci_devfn, > - /* FIXME: this should be -1...255, but the address is stored > - * into an uint32_t rather than int32_t. > - */ > - .min =3D 0, > - .max =3D 0xFFFFFFFFULL, > + .min =3D -1, > + .max =3D 255, > }; > =20 > /* --- blocksize --- */ > diff --git a/hw/qdev.h b/hw/qdev.h > index 4e90119..d07da45 100644 > --- a/hw/qdev.h > +++ b/hw/qdev.h > @@ -267,7 +267,7 @@ extern PropertyInfo qdev_prop_blocksize; > #define DEFINE_PROP_HEX64(_n, _s, _f, _d) \ > DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_hex64, uint64_t) > #define DEFINE_PROP_PCI_DEVFN(_n, _s, _f, _d) \ > - DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, uint32_t) > + DEFINE_PROP_DEFAULT(_n, _s, _f, _d, qdev_prop_pci_devfn, int32_t) > =20 > #define DEFINE_PROP_PTR(_n, _s, _f) \ > DEFINE_PROP(_n, _s, _f, qdev_prop_ptr, void*) --=20 SUSE LINUX Products GmbH, Maxfeldstr. 5, 90409 N=FCrnberg, Germany GF: Jeff Hawn, Jennifer Guild, Felix Imend=F6rffer; HRB 16746 AG N=FCrnbe= rg