From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from [140.186.70.92] (port=41645 helo=eggs.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1PXdDf-0000DK-Mu for qemu-devel@nongnu.org; Tue, 28 Dec 2010 12:20:21 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1PXdDd-0000v5-TY for qemu-devel@nongnu.org; Tue, 28 Dec 2010 12:20:19 -0500 Received: from mx1.redhat.com ([209.132.183.28]:31050) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1PXdDd-0000u6-M6 for qemu-devel@nongnu.org; Tue, 28 Dec 2010 12:20:17 -0500 Date: Tue, 28 Dec 2010 19:19:55 +0200 From: "Michael S. Tsirkin" Message-ID: <20101228171955.GA2850@redhat.com> References: <1292879881$2997@local> <20101227140136.GA27055@redhat.com> <5C6885C942B1469F90662F3DC1C2D8A1@FSCPC> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <5C6885C942B1469F90662F3DC1C2D8A1@FSCPC> Subject: [Qemu-devel] Re: [PATCH] piix: use pci_config_set_prog_interface() List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Sebastian Herbszt Cc: qemu-devel@nongnu.org On Tue, Dec 28, 2010 at 05:24:06PM +0100, Sebastian Herbszt wrote: > Michael S. Tsirkin wrote: > >On Mon, Dec 20, 2010 at 10:18:01PM +0100, Sebastian Herbszt wrote: > >>Use pci_config_set_prog_interface(). > >> > >>Signed-off-by: Sebastian Herbszt > > > >Since I was asked explicitly - I don't have a problem > >with these changes: both class and prog interface. > >However, they aren't all that useful in themselves. > > > >For class, what I would like to see is a system where > >the device class is put in the qdev info table, > >and where -device ? > >(and hopefully the legacy -help, -nic etc as well) > >use this information. > > I am not sure if you mean something like the patch below. Not exactly - I'd like to keep the pci_config_set_class in the devices, just make it do an assert. - Nics already have DEFINE_NIC_PROPERTIES - can this be used somehow? Same for other devices ... - This is still just moving bits around. The main point would be to report device type in -device ? and friends. > >pci_config_set_prog_interface can then have an assert to > >verify that value. > >Maybe we can even make e.g. -device nic work. > > > >In a similar way, pci_config_set_prog_interface > >would really become useful if we put the handling > >for the legacy classes in a central place > >(e.g. pci_class.c) > > Which are "legacy classes"? Some class/programming interface combinations let devices claim IO addresses not requested using a BAR. This is currently hard-coded in each such device. > >Any chance of doing something like the above? > >I'd be happy to apply such patches. > > Sebastian > > diff --git a/hw/e1000.c b/hw/e1000.c > index af101bd..88ace8f 100644 > --- a/hw/e1000.c > +++ b/hw/e1000.c > @@ -1117,7 +1117,6 @@ static int pci_e1000_init(PCIDevice *pci_dev) > /* TODO: we have no capabilities, so why is this bit set? */ > pci_set_word(pci_conf + PCI_STATUS, PCI_STATUS_CAP_LIST); > pci_conf[PCI_REVISION_ID] = 0x03; > - pci_config_set_class(pci_conf, PCI_CLASS_NETWORK_ETHERNET); > /* TODO: RST# value should be 0, PCI spec 6.2.4 */ > pci_conf[PCI_CACHE_LINE_SIZE] = 0x10; > > @@ -1169,6 +1168,7 @@ static PCIDeviceInfo e1000_info = { > .init = pci_e1000_init, > .exit = pci_e1000_uninit, > .romfile = "pxe-e1000.bin", > + .class = PCI_CLASS_NETWORK_ETHERNET, > .qdev.props = (Property[]) { > DEFINE_NIC_PROPERTIES(E1000State, conf), > DEFINE_PROP_END_OF_LIST(), > diff --git a/hw/pci.c b/hw/pci.c > index ef00d20..de0038c 100644 > --- a/hw/pci.c > +++ b/hw/pci.c > @@ -1620,6 +1620,11 @@ static int pci_qdev_init(DeviceState *qdev, DeviceInfo *base) > info->is_bridge); > if (pci_dev == NULL) > return -1; > + > + if (info->class) { > + pci_config_set_class(pci_dev->config, info->class); > + } > + > rc = info->init(pci_dev); > if (rc != 0) { > do_pci_unregister_device(pci_dev); > diff --git a/hw/pci.h b/hw/pci.h > index 17744dc..29b9280 100644 > --- a/hw/pci.h > +++ b/hw/pci.h > @@ -436,6 +436,9 @@ typedef struct { > > /* rom bar */ > const char *romfile; > + > + /* class */ > + uint16_t class; > } PCIDeviceInfo; > > void pci_qdev_register(PCIDeviceInfo *info);