| Message ID | 1320663634-29453-1-git-send-email-kraxel@redhat.com |
|---|---|
| State | New |
| Headers | show |
On 11/07/2011 01:00 PM, 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. > > Cc: Takashi Iwai <tiwai@suse.de> > Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> > --- > hw/ac97.c | 6 ------ > 1 files changed, 0 insertions(+), 6 deletions(-) > > diff --git a/hw/ac97.c b/hw/ac97.c > index 6800af4..b43e435 100644 > --- a/hw/ac97.c > +++ b/hw/ac97.c > @@ -1305,12 +1305,6 @@ 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; > - This a guest ABI change. Do we want -M support for it?
On 11/07/11 15:17, Avi Kivity wrote: > On 11/07/2011 01:00 PM, 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. > > This a guest ABI change. Do we want -M support for it? Given that the old subsystem id isn't valid I'd say no unless someone comes up with a good reason. cheers, Gerd
On 11/07/2011 08:33 AM, Gerd Hoffmann wrote: > On 11/07/11 15:17, Avi Kivity wrote: >> On 11/07/2011 01:00 PM, 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. >> >> This a guest ABI change. Do we want -M support for it? > > Given that the old subsystem id isn't valid I'd say no unless someone > comes up with a good reason. Agreed. Regards, Anthony Liguori > > cheers, > Gerd > >
On 11/07/2011 04:33 PM, Gerd Hoffmann wrote: > On 11/07/11 15:17, Avi Kivity wrote: > > On 11/07/2011 01:00 PM, 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. > > > > This a guest ABI change. Do we want -M support for it? > > Given that the old subsystem id isn't valid I'd say no unless someone > comes up with a good reason. Do we know that Windows won't complain about it?
On 11/07/2011 08:42 AM, Avi Kivity wrote: > On 11/07/2011 04:33 PM, Gerd Hoffmann wrote: >> On 11/07/11 15:17, Avi Kivity wrote: >>> On 11/07/2011 01:00 PM, 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. >>> >>> This a guest ABI change. Do we want -M support for it? >> >> Given that the old subsystem id isn't valid I'd say no unless someone >> comes up with a good reason. > > Do we know that Windows won't complain about it? I thought the original motivation for the default subsystem ids was that some Windows test suite was explicitly complaining about having invalid subsystem ids? Regards, Anthony Liguori >
On 11/07/2011 04:44 PM, Anthony Liguori wrote: >>>> This a guest ABI change. Do we want -M support for it? >>> >>> Given that the old subsystem id isn't valid I'd say no unless someone >>> comes up with a good reason. >> >> Do we know that Windows won't complain about it? > > > I thought the original motivation for the default subsystem ids was > that some Windows test suite was explicitly complaining about having > invalid subsystem ids? I think so, but that's unrelated. The worry is that some DRM code checksums your hardware and complains if it changed too much. Nothing to do with the test suite. The sense of Gerd's comment is reversed. We should preserve the ABI unless there is a strong reason not to.
On 11/07/2011 08:50 AM, Avi Kivity wrote: > On 11/07/2011 04:44 PM, Anthony Liguori wrote: >>>>> This a guest ABI change. Do we want -M support for it? >>>> >>>> Given that the old subsystem id isn't valid I'd say no unless someone >>>> comes up with a good reason. >>> >>> Do we know that Windows won't complain about it? >> >> >> I thought the original motivation for the default subsystem ids was >> that some Windows test suite was explicitly complaining about having >> invalid subsystem ids? > > I think so, but that's unrelated. The worry is that some DRM code > checksums your hardware and complains if it changed too much. Nothing > to do with the test suite. > > The sense of Gerd's comment is reversed. We should preserve the ABI > unless there is a strong reason not to. Yes, I understand where you're coming from and I agree except when it comes to bug fixes. My view toward bug fixes is the opposite--unless we know that the bug fix breaks something, we should fix the bug. If it's a bug, we have to assume it's breaking something. Regards, Anthony Liguori
On 11/07/2011 04:53 PM, Anthony Liguori wrote: >> I think so, but that's unrelated. The worry is that some DRM code >> checksums your hardware and complains if it changed too much. Nothing >> to do with the test suite. >> >> The sense of Gerd's comment is reversed. We should preserve the ABI >> unless there is a strong reason not to. > > > Yes, I understand where you're coming from and I agree except when it > comes to bug fixes. > > My view toward bug fixes is the opposite--unless we know that the bug > fix breaks something, we should fix the bug. If it's a bug, we have to > assume it's breaking something. You're right in that not every bug fix deserves an entry in our quirk table. We don't want -M pc-0.15 to reintroduce a data corrupting bug just because it is guest visible! This is more of an edge case however, since we know that hardware tools rely on PCI IDs. For example our hypothetical ABI signature tool will certainly include lspci like functionality and detect this as a change. I now agree with both sides of the argument and can continue the discussion unaided for at least 30-40 emails.
Hi, > This is more of an edge case however, since we know that hardware tools > rely on PCI IDs. The ID is invalid, you can't do anything useful with it ... > For example our hypothetical ABI signature tool will > certainly include lspci like functionality and detect this as a change. ... except maybe recording it somewhere to notice when changes. So I guess your point is Windows guests might think they got a new sound card, record that has hardware change and may require re-activation because of that? cheers, Gerd
On 11/07/2011 05:10 PM, Gerd Hoffmann wrote: > Hi, > > > This is more of an edge case however, since we know that hardware tools > > rely on PCI IDs. > > The ID is invalid, you can't do anything useful with it ... > > > For example our hypothetical ABI signature tool will > > certainly include lspci like functionality and detect this as a change. > > ... except maybe recording it somewhere to notice when changes. > > So I guess your point is Windows guests might think they got a new sound > card, record that has hardware change and may require re-activation > because of that? Yes. Or some hardware inventory tool we know nothing about, which was constructed specifically in order to make our lives miserable.
diff --git a/hw/ac97.c b/hw/ac97.c index 6800af4..b43e435 100644 --- a/hw/ac97.c +++ b/hw/ac97.c @@ -1305,12 +1305,6 @@ 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; - c[PCI_INTERRUPT_LINE] = 0x00; /* intr_ln interrupt line rw */ c[PCI_INTERRUPT_PIN] = 0x01; /* intr_pn interrupt pin ro */
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. Cc: Takashi Iwai <tiwai@suse.de> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> --- hw/ac97.c | 6 ------ 1 files changed, 0 insertions(+), 6 deletions(-)