From mboxrd@z Thu Jan 1 00:00:00 1970 From: Anthony Liguori Subject: Re: [Qemu-devel] [PATCH 2/2] ac97: don't override the pci subsystem id Date: Mon, 07 Nov 2011 10:00:25 -0600 Message-ID: <4EB80099.8050304@codemonkey.ws> References: <1320679989-8274-1-git-send-email-kraxel@redhat.com> <1320679989-8274-2-git-send-email-kraxel@redhat.com> Mime-Version: 1.0 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: qemu-devel@nongnu.org, Takashi Iwai , kvm@vger.kernel.org To: Gerd Hoffmann Return-path: Received: from mail-gy0-f174.google.com ([209.85.160.174]:65420 "EHLO mail-gy0-f174.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751342Ab1KGQA3 (ORCPT ); Mon, 7 Nov 2011 11:00:29 -0500 Received: by gyc15 with SMTP id 15so4308016gyc.19 for ; Mon, 07 Nov 2011 08:00:28 -0800 (PST) In-Reply-To: <1320679989-8274-2-git-send-email-kraxel@redhat.com> Sender: kvm-owner@vger.kernel.org List-ID: On 11/07/2011 09:33 AM, Gerd Hoffmann wrote: > This patch removes the code lines which set the subsystem id for the > emulated ac97 card to 8086:0000. Due to the device id being zero the > subsystem id isn't vaild anyway. With the patch applied the sound card > gets the default qemu subsystem id (1af4:1100) instead. I don't like having a property of "use broken". Wouldn't it be better to have the subsystem vendor and device id be configurable, set the default to the qemu subsystem ids, and then set it to 8086:0000 for < 1.0? Regards, Anthony Liguori > > [ v2: old& broken id is maintained for -M pc-$oldqemuversion ] > > Cc: Takashi Iwai > Signed-off-by: Gerd Hoffmann > --- > hw/ac97.c | 16 +++++++++++----- > hw/pc_piix.c | 16 ++++++++++++++++ > 2 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/hw/ac97.c b/hw/ac97.c > index 6800af4..0dbba3b 100644 > --- a/hw/ac97.c > +++ b/hw/ac97.c > @@ -150,6 +150,7 @@ typedef struct AC97BusMasterRegs { > typedef struct AC97LinkState { > PCIDevice dev; > QEMUSoundCard card; > + uint32_t use_broken_id; > uint32_t glob_cnt; > uint32_t glob_sta; > uint32_t cas; > @@ -1305,11 +1306,12 @@ static int ac97_initfn (PCIDevice *dev) > c[PCI_BASE_ADDRESS_0 + 6] = 0x00; > c[PCI_BASE_ADDRESS_0 + 7] = 0x00; > > - c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; /* svid subsystem vendor id rwo */ > - c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; > - > - c[PCI_SUBSYSTEM_ID] = 0x00; /* sid subsystem id rwo */ > - c[PCI_SUBSYSTEM_ID + 1] = 0x00; > + if (s->use_broken_id) { > + c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; > + c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; > + c[PCI_SUBSYSTEM_ID] = 0x00; > + c[PCI_SUBSYSTEM_ID + 1] = 0x00; > + } > > c[PCI_INTERRUPT_LINE] = 0x00; /* intr_ln interrupt line rw */ > c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */ > @@ -1350,6 +1352,10 @@ static PCIDeviceInfo ac97_info = { > .device_id = PCI_DEVICE_ID_INTEL_82801AA_5, > .revision = 0x01, > .class_id = PCI_CLASS_MULTIMEDIA_AUDIO, > + .qdev.props = (Property[]) { > + DEFINE_PROP_UINT32("use_broken_id", AC97LinkState, use_broken_id, 0), > + DEFINE_PROP_END_OF_LIST(), > + } > }; > > static void ac97_register (void) > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 93e40d0..27ea570 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -347,6 +347,10 @@ static QEMUMachine pc_machine_v0_13 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > }, > @@ -390,6 +394,10 @@ static QEMUMachine pc_machine_v0_12 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > } > @@ -441,6 +449,10 @@ static QEMUMachine pc_machine_v0_11 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > } > @@ -504,6 +516,10 @@ static QEMUMachine pc_machine_v0_10 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > }, From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([140.186.70.92]:45944) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNRcq-0003VG-O9 for qemu-devel@nongnu.org; Mon, 07 Nov 2011 11:00:49 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1RNRcg-000417-Tf for qemu-devel@nongnu.org; Mon, 07 Nov 2011 11:00:44 -0500 Received: from mail-gx0-f173.google.com ([209.85.161.173]:46180) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1RNRcg-0003zZ-R8 for qemu-devel@nongnu.org; Mon, 07 Nov 2011 11:00:34 -0500 Received: by ggnp2 with SMTP id p2so1867833ggn.4 for ; Mon, 07 Nov 2011 08:00:29 -0800 (PST) Message-ID: <4EB80099.8050304@codemonkey.ws> Date: Mon, 07 Nov 2011 10:00:25 -0600 From: Anthony Liguori MIME-Version: 1.0 References: <1320679989-8274-1-git-send-email-kraxel@redhat.com> <1320679989-8274-2-git-send-email-kraxel@redhat.com> In-Reply-To: <1320679989-8274-2-git-send-email-kraxel@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 2/2] ac97: don't override the pci subsystem id List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Gerd Hoffmann Cc: Takashi Iwai , qemu-devel@nongnu.org, kvm@vger.kernel.org On 11/07/2011 09:33 AM, Gerd Hoffmann wrote: > This patch removes the code lines which set the subsystem id for the > emulated ac97 card to 8086:0000. Due to the device id being zero the > subsystem id isn't vaild anyway. With the patch applied the sound card > gets the default qemu subsystem id (1af4:1100) instead. I don't like having a property of "use broken". Wouldn't it be better to have the subsystem vendor and device id be configurable, set the default to the qemu subsystem ids, and then set it to 8086:0000 for < 1.0? Regards, Anthony Liguori > > [ v2: old& broken id is maintained for -M pc-$oldqemuversion ] > > Cc: Takashi Iwai > Signed-off-by: Gerd Hoffmann > --- > hw/ac97.c | 16 +++++++++++----- > hw/pc_piix.c | 16 ++++++++++++++++ > 2 files changed, 27 insertions(+), 5 deletions(-) > > diff --git a/hw/ac97.c b/hw/ac97.c > index 6800af4..0dbba3b 100644 > --- a/hw/ac97.c > +++ b/hw/ac97.c > @@ -150,6 +150,7 @@ typedef struct AC97BusMasterRegs { > typedef struct AC97LinkState { > PCIDevice dev; > QEMUSoundCard card; > + uint32_t use_broken_id; > uint32_t glob_cnt; > uint32_t glob_sta; > uint32_t cas; > @@ -1305,11 +1306,12 @@ static int ac97_initfn (PCIDevice *dev) > c[PCI_BASE_ADDRESS_0 + 6] = 0x00; > c[PCI_BASE_ADDRESS_0 + 7] = 0x00; > > - c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; /* svid subsystem vendor id rwo */ > - c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; > - > - c[PCI_SUBSYSTEM_ID] = 0x00; /* sid subsystem id rwo */ > - c[PCI_SUBSYSTEM_ID + 1] = 0x00; > + if (s->use_broken_id) { > + c[PCI_SUBSYSTEM_VENDOR_ID] = 0x86; > + c[PCI_SUBSYSTEM_VENDOR_ID + 1] = 0x80; > + c[PCI_SUBSYSTEM_ID] = 0x00; > + c[PCI_SUBSYSTEM_ID + 1] = 0x00; > + } > > c[PCI_INTERRUPT_LINE] = 0x00; /* intr_ln interrupt line rw */ > c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */ > @@ -1350,6 +1352,10 @@ static PCIDeviceInfo ac97_info = { > .device_id = PCI_DEVICE_ID_INTEL_82801AA_5, > .revision = 0x01, > .class_id = PCI_CLASS_MULTIMEDIA_AUDIO, > + .qdev.props = (Property[]) { > + DEFINE_PROP_UINT32("use_broken_id", AC97LinkState, use_broken_id, 0), > + DEFINE_PROP_END_OF_LIST(), > + } > }; > > static void ac97_register (void) > diff --git a/hw/pc_piix.c b/hw/pc_piix.c > index 93e40d0..27ea570 100644 > --- a/hw/pc_piix.c > +++ b/hw/pc_piix.c > @@ -347,6 +347,10 @@ static QEMUMachine pc_machine_v0_13 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > }, > @@ -390,6 +394,10 @@ static QEMUMachine pc_machine_v0_12 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > } > @@ -441,6 +449,10 @@ static QEMUMachine pc_machine_v0_11 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > } > @@ -504,6 +516,10 @@ static QEMUMachine pc_machine_v0_10 = { > .driver = "virtio-net-pci", > .property = "event_idx", > .value = "off", > + },{ > + .driver = "AC97", > + .property = "use_broken_id", > + .value = stringify(1), > }, > { /* end of list */ } > },