From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1NrsUW-0000Yu-ND for qemu-devel@nongnu.org; Wed, 17 Mar 2010 08:36:52 -0400 Received: from [199.232.76.173] (port=56928 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1NrsUW-0000Yb-6J for qemu-devel@nongnu.org; Wed, 17 Mar 2010 08:36:52 -0400 Received: from Debian-exim by monty-python.gnu.org with spam-scanned (Exim 4.60) (envelope-from ) id 1NrsUU-0006k7-Jr for qemu-devel@nongnu.org; Wed, 17 Mar 2010 08:36:51 -0400 Received: from mx1.redhat.com ([209.132.183.28]:15128) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1NrsUU-0006k1-6n for qemu-devel@nongnu.org; Wed, 17 Mar 2010 08:36:50 -0400 Date: Wed, 17 Mar 2010 14:33:20 +0200 From: "Michael S. Tsirkin" Subject: Re: [Qemu-devel] [PATCH] pcnet: make subsystem vendor id match hardware Message-ID: <20100317123320.GA17973@redhat.com> References: <20100315133650.GA31573@redhat.com> <4B9FF847.8070502@mail.berlios.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4B9FF847.8070502@mail.berlios.de> List-Id: qemu-devel.nongnu.org List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Weil Cc: qemu-devel@nongnu.org On Tue, Mar 16, 2010 at 10:29:43PM +0100, Stefan Weil wrote: > Michael S. Tsirkin schrieb: > > Real pcnet device (AT2450) apparently has subsystem > > device and vendor id set to 0, this is out of spec > > (which requires that vendor id is obtained from PCI SIG) > > but windows xp driver seems to need this in order > > to associate. > > > > qemu sets pci subsystem id to qumranet/qemu > > since d350d97d196a632b6c7493acf07a061017fc6f7d, > > debian does not yet have this patch. > > > > https://bugzilla.redhat.com/show_bug.cgi?id=521247 > > > > Signed-off-by: Michael S. Tsirkin > > Cc: Gerd Hoffmann > > Cc: Anthony Liguori > > --- > > hw/pcnet.c | 3 +++ > > 1 files changed, 3 insertions(+), 0 deletions(-) > > > > diff --git a/hw/pcnet.c b/hw/pcnet.c > > index 44b5b31..12260be 100644 > > --- a/hw/pcnet.c > > +++ b/hw/pcnet.c > > @@ -1997,6 +1997,9 @@ static int pci_pcnet_init(PCIDevice *pci_dev) > > pci_set_long(pci_conf + PCI_BASE_ADDRESS_0 + 4, > > PCI_BASE_ADDRESS_SPACE_MEMORY); > > > > + pci_set_word(pci_conf + PCI_SUBSYSTEM_VENDOR_ID, 0x0); > > + pci_set_word(pci_conf + PCI_SUBSYSTEM_ID, 0x0); > > + > > /* TODO: value must be 0 at RST# */ > > pci_conf[PCI_INTERRUPT_PIN] = 1; // interrupt pin 0 > > pci_conf[PCI_MIN_GNT] = 0x06; > > > > Don't you think that a comment in the code > (with the same explanation which you wrote above) > would be helpful in this special case? > Of course one can always call git blame, but > a comment is easier to read. > > Regards > Stefan Yea, ok. Send a patch. One thing that I'm still converned about is what pci.c should do with subsystem id, since setting it to redhat/qumranet by default is not really the best thing to do: it clearly is unlikely to match any real hardware, and the heuristic it uses to detect when to set subsystem id is also kind of off: the spec ties it with class not with header type. Maybe I'll just add a TODO for now :) -- MST