From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NeYMg-0005Rp-DY for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:29:42 -0500 Received: from [199.232.76.173] (port=45691 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NeYMf-0005RN-Ub for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:29:41 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NeYMd-0007KN-PV for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:29:41 -0500 Received: from mx1.redhat.com ([209.132.183.28]:63215) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NeYMd-0007Jz-A0 for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:29:39 -0500 Date: Mon, 8 Feb 2010 20:26:24 +0200 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] Re: [PATCH] pci: initialize header type register. Message-ID: <20100208182624.GD10716@redhat.com> References: <20100208064147.GD22624@valinux.co.jp> <20100208101753.GA14662@redhat.com> <4B6FF203.6010304@redhat.com> <20100208162753.GA28230@redhat.com> <4B7048E9.6000706@redhat.com> <20100208173204.GA10716@redhat.com> <4B704BE5.9010205@redhat.com> <20100208173741.GB10716@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Blue Swirl Cc: Isaku Yamahata , Gerd Hoffmann , qemu-devel@nongnu.org On Mon, Feb 08, 2010 at 07:56:27PM +0200, Blue Swirl wrote: > On Mon, Feb 8, 2010 at 7:37 PM, Michael S. Tsirkin wro= te: > > On Mon, Feb 08, 2010 at 06:37:41PM +0100, Gerd Hoffmann wrote: > >> On 02/08/10 18:32, Michael S. Tsirkin wrote: > >>> On Mon, Feb 08, 2010 at 06:24:57PM +0100, Gerd Hoffmann wrote: > >>>> On 02/08/10 17:27, Michael S. Tsirkin wrote: > >>>>> On Mon, Feb 08, 2010 at 12:14:11PM +0100, Gerd Hoffmann wrote: > >>>>>> On 02/08/10 11:17, Michael S. Tsirkin wrote: > >>>>>>> On Mon, Feb 08, 2010 at 03:41:47PM +0900, Isaku Yamahata wrote: > >>>>>>>> initialize header type register in pci generic code. > >>>>>>>> > >>>>>>>> Cc: Blue Swirl > >>>>>>>> Cc: "Michael S. Tsirkin" > >>>>>>>> Signed-off-by: Isaku Yamahata > >>>>>>> > >>>>>>> No objections here, I am assuming this will be followed > >>>>>>> by patches removing header type init from bridges? > >>>>>>> =A0 =A0From qdev perspective, it is probably cleaner to make > >>>>>>> multifunction bit a separate qdev property though, right? > >>>>>> > >>>>>> =A0 =A0From a qdev perspective it would make *alot* of sense to = move a bunch of > >>>>>> pci config stuff (including, but not limited to header type) int= o > >>>>>> PCIDeviceInfo. > >>>>>> > >>>>>> cheers, > >>>>>> =A0 =A0 Gerd > >>>>> > >>>>> Actually - won't this make it possible to create broken configura= tions > >>>>> by tweaking properties from command-line? > >>>> > >>>> Not as property, as struct element in PCIDeviceInfo. =A0i.e. > >>>> > >>>> static PCIDeviceInfo e1000_info =3D { > >>>> =A0 =A0 =A0[ stuff which is here right now ] > >>>> =A0 =A0 =A0.vendor_id =3D PCI_VENDOR_ID_INTEL, > >>>> =A0 =A0 =A0.device_id =3D E1000_DEVID, > >>>> =A0 =A0 =A0.class =A0 =A0 =3D PCI_CLASS_NETWORK_ETHERNET, > >>>> =A0 =A0 =A0[ probably more stuff which makes sense ] > >>>> } > >>>> > >>>> Then setup this in generic pci code instead of having each driver = doing > >>>> a bunch of pci_config_set_*() calls. > >>>> > >>>> cheers, > >>>> =A0 =A0Gerd > >>> > >>> We still end up with class, vendor etc duplicated in 2 places. > >> > >> No. =A0The info should be *only* in PCIDeviceInfo then. > > > > That would put a lot of code in pci config cycle path. =A0A single ar= ray > > mirroring the whole config space is much cleaner. >=20 > I'd suppose the arrays would remain as they are now, they just would > be initialized (using the pci functions) based on PCIDeviceInfo > structure. This still means we have two copies of same data and need to maintain code that keeps them in sync, even if that is called just at init time. > This would simplify the device code a lot. Well, I think pci_set_class(pci_dev, PCI_CLASS_NETWORK_ETHERNET) is simpler than .class =3D PCI_CLASS_NETWORK_ETHERNET and some magic that copies that to pci config. > >>> =A0Why do > >>> we want stuff like vendor id in PCIDeviceInfo at all? =A0Why can't > >>> everyone just use pci_config_set/get calls? > >> > >> You can do nice stuff like printing vendor/device IDs in the '-devic= e ?' > >> list then. > > > > That should use pci functions as well. > > > >> cheers, > >> =A0 Gerd > >