From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NeY4a-0006uv-Nv for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:11:00 -0500 Received: from [199.232.76.173] (port=37955 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NeY4a-0006u8-50 for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:11:00 -0500 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NeY4S-0001kq-9O for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:10:59 -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 1NeY4P-0001bA-P4 for qemu-devel@nongnu.org; Mon, 08 Feb 2010 13:10:50 -0500 Received: from mx1.redhat.com ([209.132.183.28]) by mx20.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NeXfN-0003ke-7T for qemu-devel@nongnu.org; Mon, 08 Feb 2010 12:44:57 -0500 Message-ID: <4B704D57.3030509@redhat.com> Date: Mon, 08 Feb 2010 18:43:51 +0100 From: Gerd Hoffmann MIME-Version: 1.0 Subject: Re: [Qemu-devel] Re: [PATCH] pci: initialize header type register. 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> In-Reply-To: <20100208173741.GB10716@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Michael S. Tsirkin" Cc: Blue Swirl , Isaku Yamahata , qemu-devel@nongnu.org On 02/08/10 18:37, 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? >>>>>>> From qdev perspective, it is probably cleaner to make >>>>>>> multifunction bit a separate qdev property though, right? >>>>>> >>>>>> From a qdev perspective it would make *alot* of sense to move a bunch of >>>>>> pci config stuff (including, but not limited to header type) into >>>>>> PCIDeviceInfo. >>>>>> >>>>>> cheers, >>>>>> Gerd >>>>> >>>>> Actually - won't this make it possible to create broken configurations >>>>> by tweaking properties from command-line? >>>> >>>> Not as property, as struct element in PCIDeviceInfo. i.e. >>>> >>>> static PCIDeviceInfo e1000_info = { >>>> [ stuff which is here right now ] >>>> .vendor_id = PCI_VENDOR_ID_INTEL, >>>> .device_id = E1000_DEVID, >>>> .class = PCI_CLASS_NETWORK_ETHERNET, >>>> [ 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, >>>> Gerd >>> >>> We still end up with class, vendor etc duplicated in 2 places. >> >> No. The info should be *only* in PCIDeviceInfo then. > > That would put a lot of code in pci config cycle path. A single array > mirroring the whole config space is much cleaner. > >>> Why do >>> we want stuff like vendor id in PCIDeviceInfo at all? Why 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. Hmm, do you mix up PCIDevice and PCIDeviceInfo structs? cheers, Gerd