From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NeY4L-0006nV-1X for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:10:45 -0500 Received: from [199.232.76.173] (port=37928 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NeY4K-0006my-6c for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:10:44 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NeY4G-0001fY-GM for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:10:42 -0500 Received: from mx20.gnu.org ([199.232.41.8]:4219) by monty-python.gnu.org with esmtps (TLS-1.0:RSA_AES_256_CBC_SHA1:32) (Exim 4.60) (envelope-from ) id 1NeY4D-0001bA-Vb for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:10:38 -0500 Received: from mail-px0-f197.google.com ([209.85.216.197]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NeXqq-00042p-S9 for qemu-devel@nongnu.org; Mon, 08 Feb 2010 12:56:49 -0500 Received: by pxi35 with SMTP id 35so2530193pxi.18 for ; Mon, 08 Feb 2010 09:56:47 -0800 (PST) MIME-Version: 1.0 In-Reply-To: <20100208173741.GB10716@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> From: Blue Swirl Date: Mon, 8 Feb 2010 19:56:27 +0200 Message-ID: Subject: Re: [Qemu-devel] Re: [PATCH] pci: initialize header type register. Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Isaku Yamahata , Gerd Hoffmann , qemu-devel@nongnu.org On Mon, Feb 8, 2010 at 7:37 PM, Michael S. Tsirkin wrote: > 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? >>>>>>> =C2=A0 =C2=A0From qdev perspective, it is probably cleaner to make >>>>>>> multifunction bit a separate qdev property though, right? >>>>>> >>>>>> =C2=A0 =C2=A0From a qdev perspective it would make *alot* of sense t= o move a bunch of >>>>>> pci config stuff (including, but not limited to header type) into >>>>>> PCIDeviceInfo. >>>>>> >>>>>> cheers, >>>>>> =C2=A0 =C2=A0 Gerd >>>>> >>>>> Actually - won't this make it possible to create broken configuration= s >>>>> by tweaking properties from command-line? >>>> >>>> Not as property, as struct element in PCIDeviceInfo. =C2=A0i.e. >>>> >>>> static PCIDeviceInfo e1000_info =3D { >>>> =C2=A0 =C2=A0 =C2=A0[ stuff which is here right now ] >>>> =C2=A0 =C2=A0 =C2=A0.vendor_id =3D PCI_VENDOR_ID_INTEL, >>>> =C2=A0 =C2=A0 =C2=A0.device_id =3D E1000_DEVID, >>>> =C2=A0 =C2=A0 =C2=A0.class =C2=A0 =C2=A0 =3D PCI_CLASS_NETWORK_ETHERNE= T, >>>> =C2=A0 =C2=A0 =C2=A0[ probably more stuff which makes sense ] >>>> } >>>> >>>> Then setup this in generic pci code instead of having each driver doin= g >>>> a bunch of pci_config_set_*() calls. >>>> >>>> cheers, >>>> =C2=A0 =C2=A0Gerd >>> >>> We still end up with class, vendor etc duplicated in 2 places. >> >> No. =C2=A0The info should be *only* in PCIDeviceInfo then. > > That would put a lot of code in pci config cycle path. =C2=A0A single arr= ay > mirroring the whole config space is much cleaner. 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 would simplify the device code a lot. >>> =C2=A0Why do >>> we want stuff like vendor id in PCIDeviceInfo at all? =C2=A0Why can't >>> everyone just use pci_config_set/get calls? >> >> You can do nice stuff like printing vendor/device IDs in the '-device ?' >> list then. > > That should use pci functions as well. > >> cheers, >> =C2=A0 Gerd >