| Message ID | 20190307205844.20236-1-pbonzini@redhat.com |
|---|---|
| State | New |
| Headers | show |
On Thu, 7 Mar 2019 at 21:00, Paolo Bonzini <pbonzini@redhat.com> wrote: > > The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: > > Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' into staging (2019-03-07 16:16:02 +0000) > > are available in the Git repository at: > > git://github.com/bonzini/qemu.git tags/for-upstream-kconfig > > for you to fetch changes up to 576c3f2f16e7392e28cc9fe10d9a920d67d3645b: > > kconfig: add documentation (2019-03-07 21:54:22 +0100) > > ---------------------------------------------------------------- > Initial Kconfig work, excluding ARM and MIPS > > ---------------------------------------------------------------- > > v2->v3: fix UTF-8, adjust documentation patch for Sphinx > Applied, thanks. Please update the changelog at https://wiki.qemu.org/ChangeLog/4.0 for any user-visible changes. -- PMM
Hi Paolo, (+libvir-list) I'd like to report a regression introduced by this patch set: On 03/07/19 21:58, Paolo Bonzini wrote: > The following changes since commit > 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: > > Merge remote-tracking branch > 'remotes/cleber/tags/python-next-pull-request' into staging > (2019-03-07 16:16:02 +0000) > > are available in the Git repository at: > > git://github.com/bonzini/qemu.git tags/for-upstream-kconfig > > for you to fetch changes up to > 576c3f2f16e7392e28cc9fe10d9a920d67d3645b: > > kconfig: add documentation (2019-03-07 21:54:22 +0100) > > ---------------------------------------------------------------- > Initial Kconfig work, excluding ARM and MIPS > > ---------------------------------------------------------------- I use "libvirt-daemon-4.5.0-10.el7.x86_64" at the moment. Here are the symptoms: (1) the output of the following command changes across this series: virsh domcapabilities qemu /opt/qemu-installed-optimized/bin/qemu-system-aarch64 aarch64 virt-4.0 as shown by the diff > @@ -92,6 +92,8 @@ > <video supported='yes'> > <enum name='modelType'> > <value>vga</value> > + <value>cirrus</value> > + <value>vmvga</value> > <value>virtio</value> > </enum> > </video> > @@ -111,10 +113,7 @@ > <value>scsi</value> > </enum> > <enum name='capsType'/> > - <enum name='pciBackend'> > - <value>default</value> > - <value>vfio</value> > - </enum> > + <enum name='pciBackend'/> > </hostdev> > </devices> > <features> (2) given the following domain XML snippet, in a qemu (not KVM) aarch64 guest that I have, > <video> > <model type='virtio' heads='1' primary='yes'/> > <address type='pci' domain='0x0000' bus='0x04' slot='0x00' function='0x0'/> > </video> the QEMU command line generated by libvirt changes: > - -device virtio-gpu-pci,id=video0,max_outputs=1,bus=pci.4,addr=0x0 \ > + -device virtio-vga,id=video0,max_outputs=1,bus=pci.4,addr=0x0 \ Perhaps this series causes (1), which in turn causes (2), or else this series causes both (1) and (2) independently of each other. I'm not sure about that. Either way, (2) is a problem, because virtio-vga is inappropriate for aarch64 guests (and the ArmVirtQemu platform firmware in edk2 won't even try to drive it -- a blank screen is the result). Now, I can't pinpoint an exact commit, because (after a *brutal* bisection) I get: > There are only 'skip'ped commits left to test. > The first bad commit could be any of: > a02c0edb35662de38d400f42b68845540669ca3d > 03b348bdcbd1eda4ce097b2b84527dec774d80c4 > d6e9c470fc91f75db1785f17a9d3567d5a27953d > a7e23159074c9d774fb1e88357b778994a0c9365 > bcb129b3154ba743f8e52c21c331a0dfcaee7c38 > 02017ee385ef574133c4a978d368640772978178 > 7c28b925b7e176b4e44ed05d23cf883561000546 > 1550b0e6bfe3ab6985e5ad75df1c528a0ca39468 > e9947d18df97e6c6584f020cf9cc995404cc8061 > 8f01b41e1098d8cb9491fa3ea7bd59cf187a5bd7 > 9533dcdd416530a0d72140c122bf90517b6c81eb > 32690c8bed5762518272876dcb6dd39a54f54fd1 > c3a98aa54c734dcb7a36d193c6330d8f04d4bf8e > ccf222a816d59af1318a7efb59c6b9c5578d1abf > f349474920d80838ecea3d421531fdb0660b8740 > 2ac041c2c3d89691cda1657981c41fe4bc20244b > e0e312f3525ad6ac18ba6633af29190dd9620cbc > b42075bb77672616127c9452c0f836e757e9ce1a > We cannot bisect more! Here's how I built each step of the bisection: - "git clean -ffdx" and "git submodule deinit --all --force" on the source directory - create a new, pristine build directory - configure QEMU for out-of-tree build, in that directory - select the i386, x86_64, arm, and aarch64 targets (all softmmu) - report "skip" to git-bisect iff the build breaks, otherwise test the aarch64 emulator through libvirt, and report good/bad dependent on the test result. The bisection log is attached -- "git bisect replay" and "git bisect visualize" should tell the story better than the above dump of commit hashes. Basically: - The tree builds up to and including commit 82f5181777eb ("kconfig: introduce kconfig files", 2019-03-07), and that commit works fine as well. - Then the build breaks (starting with commit e0e312f3525a, "build: switch to Kconfig", 2019-03-07). - The tree becomes buildable again at commit b42075bb7767 ("virtio: express virtio dependencies with Kconfig", 2019-03-07), but at that point, the functionality has been regressed. Thanks, Laszlo git bisect start # good: [1c5d9d8f111b9cfc0722c7edcdc3b090736972e5] Merge remote-tracking branch 'remotes/cohuck/tags/s390x-20190218' into staging git bisect good 1c5d9d8f111b9cfc0722c7edcdc3b090736972e5 # bad: [62a172e6a77d9072bb1a18f295ce0fcf4b90a4f2] Update version for v4.0.0-rc0 release git bisect bad 62a172e6a77d9072bb1a18f295ce0fcf4b90a4f2 # good: [469bb49b3e131b5f641939d4fa6a4b09e6da47f8] qos-test: megasas test node git bisect good 469bb49b3e131b5f641939d4fa6a4b09e6da47f8 # bad: [eda1df0345f5a1e337e30367124dcb0e802bdfde] Merge remote-tracking branch 'remotes/armbru/tags/pull-pflash-2019-03-11' into staging git bisect bad eda1df0345f5a1e337e30367124dcb0e802bdfde # bad: [79d8b1dc5b44d806d9700f2e8a8028075a8ff2b2] Merge remote-tracking branch 'remotes/kraxel/tags/ui-20190311-v2-pull-request' into staging git bisect bad 79d8b1dc5b44d806d9700f2e8a8028075a8ff2b2 # bad: [e56d931a9ddee7230f647a25ada33ee19d26aa6d] Merge remote-tracking branch 'remotes/bonzini/tags/for-upstream-kconfig' into staging git bisect bad e56d931a9ddee7230f647a25ada33ee19d26aa6d # skip: [32690c8bed5762518272876dcb6dd39a54f54fd1] display: express dependencies with kconfig git bisect skip 32690c8bed5762518272876dcb6dd39a54f54fd1 # bad: [82a230d5a3031d19ca522f52251046ba30242828] riscv-softmmu.mak: replace CONFIG_* with Kconfig "select" directives git bisect bad 82a230d5a3031d19ca522f52251046ba30242828 # skip: [7c28b925b7e176b4e44ed05d23cf883561000546] build: convert pci.mak to Kconfig git bisect skip 7c28b925b7e176b4e44ed05d23cf883561000546 # skip: [9533dcdd416530a0d72140c122bf90517b6c81eb] ptimer: express dependencies with Kconfig git bisect skip 9533dcdd416530a0d72140c122bf90517b6c81eb # bad: [a22b2ce579bf20063cf1908cf3f951c4165be449] microblaze-softmmu.mak: express dependencies with Kconfig git bisect bad a22b2ce579bf20063cf1908cf3f951c4165be449 # skip: [8f01b41e1098d8cb9491fa3ea7bd59cf187a5bd7] ide: express dependencies with Kconfig git bisect skip 8f01b41e1098d8cb9491fa3ea7bd59cf187a5bd7 # skip: [03b348bdcbd1eda4ce097b2b84527dec774d80c4] scsi: express dependencies with Kconfig git bisect skip 03b348bdcbd1eda4ce097b2b84527dec774d80c4 # bad: [98bd1db99f072316129ed824048fcb81d0b98e41] ppc: Express dependencies of the embedded machines with kconfig git bisect bad 98bd1db99f072316129ed824048fcb81d0b98e41 # good: [82f5181777ebe04b550fd94a1d04c49dd3f012dc] kconfig: introduce kconfig files git bisect good 82f5181777ebe04b550fd94a1d04c49dd3f012dc # skip: [a7e23159074c9d774fb1e88357b778994a0c9365] isa: express dependencies with kconfig git bisect skip a7e23159074c9d774fb1e88357b778994a0c9365 # bad: [12bb3a90088ed427ac5e6a974c5f93a3106fc4cf] ppc: Express dependencies of the 'prep' and '40p' machines with kconfig git bisect bad 12bb3a90088ed427ac5e6a974c5f93a3106fc4cf # skip: [1550b0e6bfe3ab6985e5ad75df1c528a0ca39468] i2c: express dependencies with Kconfig git bisect skip 1550b0e6bfe3ab6985e5ad75df1c528a0ca39468 # skip: [c3a98aa54c734dcb7a36d193c6330d8f04d4bf8e] kconfig: introduce CONFIG_TEST_DEVICES git bisect skip c3a98aa54c734dcb7a36d193c6330d8f04d4bf8e # skip: [ccf222a816d59af1318a7efb59c6b9c5578d1abf] hyperv: express dependencies with kconfig git bisect skip ccf222a816d59af1318a7efb59c6b9c5578d1abf # bad: [464399a9b4ae81d7dad33a4acb30dfb76c7b1bd2] sd: express dependencies with kconfig git bisect bad 464399a9b4ae81d7dad33a4acb30dfb76c7b1bd2 # bad: [b42075bb77672616127c9452c0f836e757e9ce1a] virtio: express virtio dependencies with Kconfig git bisect bad b42075bb77672616127c9452c0f836e757e9ce1a # skip: [a02c0edb35662de38d400f42b68845540669ca3d] block: fix recursion in hw/block/dataplane git bisect skip a02c0edb35662de38d400f42b68845540669ca3d # skip: [d6e9c470fc91f75db1785f17a9d3567d5a27953d] build: convert usb.mak to Kconfig git bisect skip d6e9c470fc91f75db1785f17a9d3567d5a27953d # skip: [f349474920d80838ecea3d421531fdb0660b8740] minikconfig: implement allnoconfig and defconfig modes git bisect skip f349474920d80838ecea3d421531fdb0660b8740 # skip: [bcb129b3154ba743f8e52c21c331a0dfcaee7c38] build: convert sound.mak to Kconfig git bisect skip bcb129b3154ba743f8e52c21c331a0dfcaee7c38 # skip: [e9947d18df97e6c6584f020cf9cc995404cc8061] hw/pci/Makefile.objs: make pcie configurable git bisect skip e9947d18df97e6c6584f020cf9cc995404cc8061 # skip: [02017ee385ef574133c4a978d368640772978178] i386: express dependencies with Kconfig git bisect skip 02017ee385ef574133c4a978d368640772978178 # skip: [2ac041c2c3d89691cda1657981c41fe4bc20244b] vfio: express vfio dependencies with Kconfig git bisect skip 2ac041c2c3d89691cda1657981c41fe4bc20244b # skip: [e0e312f3525ad6ac18ba6633af29190dd9620cbc] build: switch to Kconfig git bisect skip e0e312f3525ad6ac18ba6633af29190dd9620cbc # only skipped commits left to test # possible first bad commit: [b42075bb77672616127c9452c0f836e757e9ce1a] virtio: express virtio dependencies with Kconfig # possible first bad commit: [2ac041c2c3d89691cda1657981c41fe4bc20244b] vfio: express vfio dependencies with Kconfig # possible first bad commit: [ccf222a816d59af1318a7efb59c6b9c5578d1abf] hyperv: express dependencies with kconfig # possible first bad commit: [32690c8bed5762518272876dcb6dd39a54f54fd1] display: express dependencies with kconfig # possible first bad commit: [9533dcdd416530a0d72140c122bf90517b6c81eb] ptimer: express dependencies with Kconfig # possible first bad commit: [1550b0e6bfe3ab6985e5ad75df1c528a0ca39468] i2c: express dependencies with Kconfig # possible first bad commit: [02017ee385ef574133c4a978d368640772978178] i386: express dependencies with Kconfig # possible first bad commit: [a7e23159074c9d774fb1e88357b778994a0c9365] isa: express dependencies with kconfig # possible first bad commit: [03b348bdcbd1eda4ce097b2b84527dec774d80c4] scsi: express dependencies with Kconfig # possible first bad commit: [a02c0edb35662de38d400f42b68845540669ca3d] block: fix recursion in hw/block/dataplane # possible first bad commit: [d6e9c470fc91f75db1785f17a9d3567d5a27953d] build: convert usb.mak to Kconfig # possible first bad commit: [bcb129b3154ba743f8e52c21c331a0dfcaee7c38] build: convert sound.mak to Kconfig # possible first bad commit: [7c28b925b7e176b4e44ed05d23cf883561000546] build: convert pci.mak to Kconfig # possible first bad commit: [e9947d18df97e6c6584f020cf9cc995404cc8061] hw/pci/Makefile.objs: make pcie configurable # possible first bad commit: [8f01b41e1098d8cb9491fa3ea7bd59cf187a5bd7] ide: express dependencies with Kconfig # possible first bad commit: [c3a98aa54c734dcb7a36d193c6330d8f04d4bf8e] kconfig: introduce CONFIG_TEST_DEVICES # possible first bad commit: [f349474920d80838ecea3d421531fdb0660b8740] minikconfig: implement allnoconfig and defconfig modes # possible first bad commit: [e0e312f3525ad6ac18ba6633af29190dd9620cbc] build: switch to Kconfig
On 03/21/19 01:53, Laszlo Ersek wrote: > Hi Paolo, > > (+libvir-list) > > I'd like to report a regression introduced by this patch set: > > On 03/07/19 21:58, Paolo Bonzini wrote: >> The following changes since commit >> 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: >> >> Merge remote-tracking branch >> 'remotes/cleber/tags/python-next-pull-request' into staging >> (2019-03-07 16:16:02 +0000) >> >> are available in the Git repository at: >> >> git://github.com/bonzini/qemu.git tags/for-upstream-kconfig >> >> for you to fetch changes up to >> 576c3f2f16e7392e28cc9fe10d9a920d67d3645b: >> >> kconfig: add documentation (2019-03-07 21:54:22 +0100) >> >> ---------------------------------------------------------------- >> Initial Kconfig work, excluding ARM and MIPS >> >> ---------------------------------------------------------------- > In brief, the regression is that the aarch64 system emulator now lists the "virtio-vga" device for the "virt" machine type: $ qemu-system-aarch64 -M virt -device \? | grep virtio-vga name "virtio-vga", bus PCI Here's where I think the issue was introduced: (1) In commit 82f5181777eb ("kconfig: introduce kconfig files", 2019-03-07), VIRTIO_VGA was introduced simply as a bool (with no other clauses) in "hw/display/Kconfig". (2) In commit 7c28b925b7e1 ("build: convert pci.mak to Kconfig", 2019-03-07), VIRTIO_VGA received the following clauses: default y if PCI_DEVICES depends on VIRTIO_PCI select VGA This is too lax, because it permits virtio-vga for aarch64, while that shouldn't happen (and it doesn't matches the previous state of the tree). (3) In commit b42075bb7767 ("virtio: express virtio dependencies with Kconfig", 2019-03-07), the determination of virtio-vga was switched to the Kconfig system. Importantly, in this patch, the line CONFIG_VIRTIO_VGA=y was removed *only* from the following file: default-configs/i386-softmmu.mak (4) Both of the remaining instances of CONFIG_VIRTIO_VGA=y were then removed in commit 9483cf27dd36 ("hppa-softmmu.mak: express dependencies with Kconfig", 2019-03-07) and commit 87f9108bad0c ("ppc64: Express dependencies of 'pseries' and 'powernv' machines with kconfig", 2019-03-07), from files default-configs/hppa-softmmu.mak default-configs/ppc64-softmmu.mak respectively. Therefore I think commit 7c28b925b7e1 has the bug. The VIRTIO_VGA restriction depends on VIRTIO_PCI should now be replaced with depends on VIRTIO_PCI && (PC || DINO || PSERIES) Thanks Laszlo
On Thu, 21 Mar 2019 at 12:40, Laszlo Ersek <lersek@redhat.com> wrote: > In brief, the regression is that the aarch64 system emulator now lists > the "virtio-vga" device for the "virt" machine type: > > $ qemu-system-aarch64 -M virt -device \? | grep virtio-vga > name "virtio-vga", bus PCI > > Here's where I think the issue was introduced: > > (1) In commit 82f5181777eb ("kconfig: introduce kconfig files", > 2019-03-07), VIRTIO_VGA was introduced simply as a bool (with no other > clauses) in "hw/display/Kconfig". > > (2) In commit 7c28b925b7e1 ("build: convert pci.mak to Kconfig", > 2019-03-07), VIRTIO_VGA received the following clauses: > > default y if PCI_DEVICES > depends on VIRTIO_PCI > select VGA > > This is too lax, because it permits virtio-vga for aarch64, while that > shouldn't happen (and it doesn't matches the previous state of the tree). > > (3) In commit b42075bb7767 ("virtio: express virtio dependencies with > Kconfig", 2019-03-07), the determination of virtio-vga was switched to > the Kconfig system. Importantly, in this patch, the line > > CONFIG_VIRTIO_VGA=y > > was removed *only* from the following file: > > default-configs/i386-softmmu.mak > > (4) Both of the remaining instances of > > CONFIG_VIRTIO_VGA=y > > were then removed in commit 9483cf27dd36 ("hppa-softmmu.mak: express > dependencies with Kconfig", 2019-03-07) and commit 87f9108bad0c ("ppc64: > Express dependencies of 'pseries' and 'powernv' machines with kconfig", > 2019-03-07), from files > > default-configs/hppa-softmmu.mak > default-configs/ppc64-softmmu.mak > > respectively. > > > Therefore I think commit 7c28b925b7e1 has the bug. The VIRTIO_VGA > restriction > > depends on VIRTIO_PCI > > should now be replaced with > > depends on VIRTIO_PCI && (PC || DINO || PSERIES) Should it? What makes the virtio-vga device only suitable for those three machines? thanks -- PMM
Le jeu. 21 mars 2019 13:39, Laszlo Ersek <lersek@redhat.com> a écrit : > On 03/21/19 01:53, Laszlo Ersek wrote: > > Hi Paolo, > > > > (+libvir-list) > > > > I'd like to report a regression introduced by this patch set: > > > > On 03/07/19 21:58, Paolo Bonzini wrote: > >> The following changes since commit > >> 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: > >> > >> Merge remote-tracking branch > >> 'remotes/cleber/tags/python-next-pull-request' into staging > >> (2019-03-07 16:16:02 +0000) > >> > >> are available in the Git repository at: > >> > >> git://github.com/bonzini/qemu.git tags/for-upstream-kconfig > >> > >> for you to fetch changes up to > >> 576c3f2f16e7392e28cc9fe10d9a920d67d3645b: > >> > >> kconfig: add documentation (2019-03-07 21:54:22 +0100) > >> > >> ---------------------------------------------------------------- > >> Initial Kconfig work, excluding ARM and MIPS > >> > >> ---------------------------------------------------------------- > > > > In brief, the regression is that the aarch64 system emulator now lists > the "virtio-vga" device for the "virt" machine type: > > $ qemu-system-aarch64 -M virt -device \? | grep virtio-vga > name "virtio-vga", bus PCI > > Here's where I think the issue was introduced: > > (1) In commit 82f5181777eb ("kconfig: introduce kconfig files", > 2019-03-07), VIRTIO_VGA was introduced simply as a bool (with no other > clauses) in "hw/display/Kconfig". > > (2) In commit 7c28b925b7e1 ("build: convert pci.mak to Kconfig", > 2019-03-07), VIRTIO_VGA received the following clauses: > > default y if PCI_DEVICES > depends on VIRTIO_PCI > select VGA > > This is too lax, because it permits virtio-vga for aarch64, while that > shouldn't happen (and it doesn't matches the previous state of the tree). > Similar case I noticed in another thread. The Virt machine uses a chipset that provides PCI. Does this machine expose PCI slots? If yes, this config is correct, the machine can get any PCI daughter card. Else we shouldn't use PCI_DEVICES that way. Why Virt machine now default to virtio-vga? > (3) In commit b42075bb7767 ("virtio: express virtio dependencies with > Kconfig", 2019-03-07), the determination of virtio-vga was switched to > the Kconfig system. Importantly, in this patch, the line > > CONFIG_VIRTIO_VGA=y > > was removed *only* from the following file: > > default-configs/i386-softmmu.mak > > (4) Both of the remaining instances of > > CONFIG_VIRTIO_VGA=y > > were then removed in commit 9483cf27dd36 ("hppa-softmmu.mak: express > dependencies with Kconfig", 2019-03-07) and commit 87f9108bad0c ("ppc64: > Express dependencies of 'pseries' and 'powernv' machines with kconfig", > 2019-03-07), from files > > default-configs/hppa-softmmu.mak > default-configs/ppc64-softmmu.mak > > respectively. > > > Therefore I think commit 7c28b925b7e1 has the bug. The VIRTIO_VGA > restriction > > depends on VIRTIO_PCI > > should now be replaced with > > depends on VIRTIO_PCI && (PC || DINO || PSERIES) > > Thanks > Laszlo > >
On 21/03/19 14:04, Peter Maydell wrote: >> >> were then removed in commit 9483cf27dd36 ("hppa-softmmu.mak: express >> dependencies with Kconfig", 2019-03-07) and commit 87f9108bad0c ("ppc64: >> Express dependencies of 'pseries' and 'powernv' machines with kconfig", >> 2019-03-07), from files >> >> default-configs/hppa-softmmu.mak >> default-configs/ppc64-softmmu.mak >> >> respectively. >> >> >> Therefore I think commit 7c28b925b7e1 has the bug. The VIRTIO_VGA >> restriction >> >> depends on VIRTIO_PCI >> >> should now be replaced with >> >> depends on VIRTIO_PCI && (PC || DINO || PSERIES) > Should it? What makes the virtio-vga device only > suitable for those three machines? The idea was that to only support virtio-vga for those machines where the firmware doesn't know about virtio-gpu and supported VGA legacy hardware before virtio-{gpu,vga} were introduced. I think this is a bug in libvirt, but at least for 4.0 it should be fixed/worked around in QEMU. Paolo
On 03/21/19 14:04, Peter Maydell wrote: > On Thu, 21 Mar 2019 at 12:40, Laszlo Ersek <lersek@redhat.com> wrote: >> In brief, the regression is that the aarch64 system emulator now lists >> the "virtio-vga" device for the "virt" machine type: >> >> $ qemu-system-aarch64 -M virt -device \? | grep virtio-vga >> name "virtio-vga", bus PCI >> >> Here's where I think the issue was introduced: >> >> (1) In commit 82f5181777eb ("kconfig: introduce kconfig files", >> 2019-03-07), VIRTIO_VGA was introduced simply as a bool (with no other >> clauses) in "hw/display/Kconfig". >> >> (2) In commit 7c28b925b7e1 ("build: convert pci.mak to Kconfig", >> 2019-03-07), VIRTIO_VGA received the following clauses: >> >> default y if PCI_DEVICES >> depends on VIRTIO_PCI >> select VGA >> >> This is too lax, because it permits virtio-vga for aarch64, while that >> shouldn't happen (and it doesn't matches the previous state of the tree). >> >> (3) In commit b42075bb7767 ("virtio: express virtio dependencies with >> Kconfig", 2019-03-07), the determination of virtio-vga was switched to >> the Kconfig system. Importantly, in this patch, the line >> >> CONFIG_VIRTIO_VGA=y >> >> was removed *only* from the following file: >> >> default-configs/i386-softmmu.mak >> >> (4) Both of the remaining instances of >> >> CONFIG_VIRTIO_VGA=y >> >> were then removed in commit 9483cf27dd36 ("hppa-softmmu.mak: express >> dependencies with Kconfig", 2019-03-07) and commit 87f9108bad0c ("ppc64: >> Express dependencies of 'pseries' and 'powernv' machines with kconfig", >> 2019-03-07), from files >> >> default-configs/hppa-softmmu.mak >> default-configs/ppc64-softmmu.mak >> >> respectively. >> >> >> Therefore I think commit 7c28b925b7e1 has the bug. The VIRTIO_VGA >> restriction >> >> depends on VIRTIO_PCI >> >> should now be replaced with >> >> depends on VIRTIO_PCI && (PC || DINO || PSERIES) > > Should it? What makes the virtio-vga device only > suitable for those three machines? It simply keeps the status quo from before the patchset. The specific emulation targets that virtio-vga should not be enabled for (regardless of other targets / machine types) are arm/aarch64. IOW, in the longer term, it's not that virtio-vga is suitable only for PC || DINO || PSERIES, but that it's *not* suitable for arm/aarch64. My original libvirt commit explains it: https://libvirt.org/git/?p=libvirt.git;a=commit;h=706b5b627719e95a33606c463bc83c841c7b5b0e Regarding the guest firmware: in edk2 there are two relevant drivers, QemuVideoDxe and VirtioGpuDxe. The former is framebuffer-based, the latter is a genuine virtio drier (no framebuffer). The former doesn't work on arm/aarch64 KVM, the latter does. The "virtio-vga" device presents a conflict for these drivers because it could be bound by both drivers, at the same time. (The device presents both interfaces.) Arbitration between UEFI drivers is very tricky. To keep things platform independent and sane in both drivers, VirtioGpuDxe never binds virtio-vga, only virtio-gpu-pci. Therefore, in OVMF, where both drivers are included, virtio-vga is deterministically bound by QemuVideoDxe, a framebuffer is exposed, and Windows is happy. You can still use virtio-gpu-pci, and that will be bound by VirtioGpuDxe (windows won't be happy, but you asked for it). Whereas in ArmVirtQemu, only VirtioGpuDxe is included, as QemuVideoDxe has no chance of working (on KVM). If you then boot with virtio-vga, VirtioGpuDxe will still yield... but there won't be another driver to bind the device instead. So, there you *must* use virtio-gpu-pci, for VirtioGpuDxe to like the device. Again, the goal is to keep VirtioGpuDxe and QemuVideoDxe platform-independent; i.e. refrain from aarch64-specific and x64-specific quirks in either. They bind or yield purely based on the device model they see. So, if you allow libvirt to see virtio-vga on aarch64, it will pick virtio-vga for the guest, and then VirtioGpuDxe will not bind it, and there's nothing else to bind it. Thanks, Laszlo
On 03/21/19 15:21, Philippe Mathieu-Daudé wrote: > Le jeu. 21 mars 2019 13:39, Laszlo Ersek <lersek@redhat.com> a écrit : > >> On 03/21/19 01:53, Laszlo Ersek wrote: >>> Hi Paolo, >>> >>> (+libvir-list) >>> >>> I'd like to report a regression introduced by this patch set: >>> >>> On 03/07/19 21:58, Paolo Bonzini wrote: >>>> The following changes since commit >>>> 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: >>>> >>>> Merge remote-tracking branch >>>> 'remotes/cleber/tags/python-next-pull-request' into staging >>>> (2019-03-07 16:16:02 +0000) >>>> >>>> are available in the Git repository at: >>>> >>>> git://github.com/bonzini/qemu.git tags/for-upstream-kconfig >>>> >>>> for you to fetch changes up to >>>> 576c3f2f16e7392e28cc9fe10d9a920d67d3645b: >>>> >>>> kconfig: add documentation (2019-03-07 21:54:22 +0100) >>>> >>>> ---------------------------------------------------------------- >>>> Initial Kconfig work, excluding ARM and MIPS >>>> >>>> ---------------------------------------------------------------- >>> >> >> In brief, the regression is that the aarch64 system emulator now lists >> the "virtio-vga" device for the "virt" machine type: >> >> $ qemu-system-aarch64 -M virt -device \? | grep virtio-vga >> name "virtio-vga", bus PCI >> >> Here's where I think the issue was introduced: >> >> (1) In commit 82f5181777eb ("kconfig: introduce kconfig files", >> 2019-03-07), VIRTIO_VGA was introduced simply as a bool (with no other >> clauses) in "hw/display/Kconfig". >> >> (2) In commit 7c28b925b7e1 ("build: convert pci.mak to Kconfig", >> 2019-03-07), VIRTIO_VGA received the following clauses: >> >> default y if PCI_DEVICES >> depends on VIRTIO_PCI >> select VGA >> >> This is too lax, because it permits virtio-vga for aarch64, while that >> shouldn't happen (and it doesn't matches the previous state of the tree). >> > > Similar case I noticed in another thread. The Virt machine uses a chipset > that provides PCI. Does this machine expose PCI slots? It provides a PCI Express hierarchy, so "yes". (If you mean traditional PCI, that is also available through the pcie-pci bridge device model, which you can plug into the PCIE hierarchy.) > If yes, this config > is correct, the machine can get any PCI daughter card. Haha, right, in theory :) On the same note, the ARMv8 architecture should also, "in theory", have allowed the host side page mappings unconditionally override the guest side mappings, like it works on x86. Because in that case, QemuVideoDxe / the framebuffer would have just worked in aarch64 guests too, I wouldn't have had to write VirtioGpuDxe in the first place, and everybody would have been happy with just virtio-vga. But ARMv8 is not like that, so things don't work like that. > Else we shouldn't > use PCI_DEVICES that way. > > Why Virt machine now default to virtio-vga? See above, plus my answer to Peter. Thanks Laszlo >> (3) In commit b42075bb7767 ("virtio: express virtio dependencies with >> Kconfig", 2019-03-07), the determination of virtio-vga was switched to >> the Kconfig system. Importantly, in this patch, the line >> >> CONFIG_VIRTIO_VGA=y >> >> was removed *only* from the following file: >> >> default-configs/i386-softmmu.mak >> >> (4) Both of the remaining instances of >> >> CONFIG_VIRTIO_VGA=y >> >> were then removed in commit 9483cf27dd36 ("hppa-softmmu.mak: express >> dependencies with Kconfig", 2019-03-07) and commit 87f9108bad0c ("ppc64: >> Express dependencies of 'pseries' and 'powernv' machines with kconfig", >> 2019-03-07), from files >> >> default-configs/hppa-softmmu.mak >> default-configs/ppc64-softmmu.mak >> >> respectively. >> >> >> Therefore I think commit 7c28b925b7e1 has the bug. The VIRTIO_VGA >> restriction >> >> depends on VIRTIO_PCI >> >> should now be replaced with >> >> depends on VIRTIO_PCI && (PC || DINO || PSERIES) >> >> Thanks >> Laszlo >> >> >
On Thu, 21 Mar 2019 at 17:35, Laszlo Ersek <lersek@redhat.com> wrote: > It simply keeps the status quo from before the patchset. > > The specific emulation targets that virtio-vga should not be enabled for > (regardless of other targets / machine types) are arm/aarch64. IOW, in > the longer term, it's not that virtio-vga is suitable only for PC || > DINO || PSERIES, but that it's *not* suitable for arm/aarch64. Relying on the device not being present for aarch64 seems fragile to me. It will break if we add a different aarch64 machine other than "virt" and want that other machine to have virtio-vga. It will also break if we ever manage to get full heterogenous-CPU support and end up with a single qemu-system binary that can emulate both AArch64 virt and x86-64 pc machines :-) If what you want is "don't use virtio-vga when using KVM on aarch64" then why not have logic that implements that? For 4.0 we should definitely just retain what we had before the introduction of KConfig, but as a general thing we should look to not have to have this odd wrinkle. (AFAIK virtio-vga is no different in this regard to the other vga PCI devices, which also won't work in KVM setups.) thanks -- PMM
On 03/21/19 18:58, Peter Maydell wrote: > On Thu, 21 Mar 2019 at 17:35, Laszlo Ersek <lersek@redhat.com> wrote: >> It simply keeps the status quo from before the patchset. >> >> The specific emulation targets that virtio-vga should not be enabled for >> (regardless of other targets / machine types) are arm/aarch64. IOW, in >> the longer term, it's not that virtio-vga is suitable only for PC || >> DINO || PSERIES, but that it's *not* suitable for arm/aarch64. > > Relying on the device not being present for aarch64 seems > fragile to me. It will break if we add a different > aarch64 machine other than "virt" and want that other > machine to have virtio-vga. It will also break if we > ever manage to get full heterogenous-CPU support and > end up with a single qemu-system binary that can emulate > both AArch64 virt and x86-64 pc machines :-) > > If what you want is "don't use virtio-vga when using KVM > on aarch64" then why not have logic that implements that? That's actually the logic that I implemented originally (or something very close to it -- <https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=706b5b627719e95a33606c463bc83c841c7b5b0e>), but since then, this code has been modified in libvirt multiple times: * [libvirt] [PATCH v2 00/17] cleanups and improvements for video device code https://www.redhat.com/archives/libvir-list/2016-October/msg00533.html https://libvirt.org/git/?p=libvirt.git;a=shortlog;hp=a62655f9c33a8f7c6257779ddbdbf1352d53a526;h=fb8f3b1c22c8f272bb9a47e8f8915acc3cfb47f1 * [libvirt] [PATCH v3 0/6] Add support for video and input devices on S390 https://www.redhat.com/archives/libvir-list/2018-March/msg01519.html https://libvirt.org/git/?p=libvirt.git;a=commitdiff;h=4bbf7f8cb5bf979ea2f5c247b2ddb884a07ac33f I'm not implying that those patches must have introduced a libvirt regression, I just agree that the code deserves investigation. This is why I added libvir-list to the CC list when reporting the problem. > For 4.0 we should definitely just retain what we had > before the introduction of KConfig, but as a general > thing we should look to not have to have this odd > wrinkle. > > (AFAIK virtio-vga is no different in this regard to the other > vga PCI devices, which also won't work in KVM setups.) I agree -- please refer to the "domcapabilities" part (1) of my report: https://lists.gnu.org/archive/html/qemu-devel/2019-03/msg06060.html > @@ -92,6 +92,8 @@ > <video supported='yes'> > <enum name='modelType'> > <value>vga</value> > + <value>cirrus</value> > + <value>vmvga</value> > <value>virtio</value> > </enum> > </video> "cirrus" and "vmvga" look wrong for aarch64. Thanks Laszlo
The following changes since commit 6cb4f6db4f4367faa33da85b15f75bbbd2bed2a6: Merge remote-tracking branch 'remotes/cleber/tags/python-next-pull-request' into staging (2019-03-07 16:16:02 +0000) are available in the Git repository at: git://github.com/bonzini/qemu.git tags/for-upstream-kconfig for you to fetch changes up to 576c3f2f16e7392e28cc9fe10d9a920d67d3645b: kconfig: add documentation (2019-03-07 21:54:22 +0100) ---------------------------------------------------------------- Initial Kconfig work, excluding ARM and MIPS ---------------------------------------------------------------- v2->v3: fix UTF-8, adjust documentation patch for Sphinx Paolo Bonzini (42): 9pfs: remove unnecessary conditionals xtensa: rename CONFIG_XTENSA_FPGA to CONFIG_XTENSA_XTFPGA minikconfig: add parser skeleton minikconfig: add AST minikconfig: add semantic analysis kconfig: introduce kconfig files build: switch to Kconfig minikconfig: implement allnoconfig and defconfig modes kconfig: introduce CONFIG_TEST_DEVICES ide: express dependencies with Kconfig build: convert pci.mak to Kconfig build: convert sound.mak to Kconfig build: convert usb.mak to Kconfig block: fix recursion in hw/block/dataplane scsi: express dependencies with Kconfig isa: express dependencies with kconfig i386: express dependencies with Kconfig i2c: express dependencies with Kconfig ptimer: express dependencies with Kconfig vfio: express vfio dependencies with Kconfig tpm: express dependencies with Kconfig isa: express SuperIO dependencies with Kconfig ssi: express dependencies with kconfig sd: express dependencies with kconfig ipmi: express dependencies with kconfig alpha-softmmu.mak: express dependencies with Kconfig cris-softmmu.mak: express dependencies with Kconfig hppa-softmmu.mak: express dependencies with Kconfig lm32-softmmu.mak: express dependencies with Kconfig m68k-softmmu.mak: express dependencies with Kconfig microblaze-softmmu.mak: express dependencies with Kconfig moxie-softmmu.mak: express dependencies with Kconfig nios2-softmmu.mak: express dependencies with Kconfig or1k-softmmu.mak: express dependencies with Kconfig riscv-softmmu.mak: replace CONFIG_* with Kconfig "select" directives sh4-softmmu.mak: express dependencies with Kconfig sparc-softmmu.mak: express dependencies with Kconfig sparc64-softmmu.mak: express dependencies with Kconfig unicore32-softmmu.mak: express dependencies with Kconfig xtensa-softmmu.mak: express dependencies with Kconfig .travis.yml: test that no-default-device builds do not regress kconfig: add documentation Thomas Huth (6): ppc64: Express dependencies of 'pseries' and 'powernv' machines with kconfig ppc: Express dependencies of the 'prep' and '40p' machines with kconfig ppc: Express dependencies of the Mac machines with kconfig ppc: Express dependencies of the Sam460EX machines with kconfig ppc: Express dependencies of the embedded machines with kconfig s390x: express dependencies with Kconfig Yang Zhong (6): hw/display: make edid configurable hw/pci/Makefile.objs: make pcie configurable display: express dependencies with kconfig hyperv: express dependencies with kconfig virtio: express virtio dependencies with Kconfig i386-softmmu.mak: remove all CONFIG_* except boards definitions .travis.yml | 6 + Kconfig.host | 33 ++ Makefile | 27 +- Makefile.target | 7 +- configure | 42 +- default-configs/alpha-softmmu.mak | 26 +- default-configs/arm-softmmu.mak | 18 +- default-configs/cris-softmmu.mak | 6 +- default-configs/hppa-softmmu.mak | 20 +- default-configs/hyperv.mak | 2 - default-configs/i386-softmmu.mak | 93 ++--- default-configs/lm32-softmmu.mak | 12 +- default-configs/m68k-softmmu.mak | 4 +- default-configs/microblaze-softmmu.mak | 12 +- default-configs/mips-softmmu-common.mak | 10 +- default-configs/mips64el-softmmu.mak | 2 + default-configs/moxie-softmmu.mak | 7 +- default-configs/nios2-softmmu.mak | 6 +- default-configs/or1k-softmmu.mak | 5 +- default-configs/pci.mak | 51 --- default-configs/ppc-softmmu.mak | 60 --- default-configs/ppc64-softmmu.mak | 13 - default-configs/riscv32-softmmu.mak | 21 +- default-configs/riscv64-softmmu.mak | 22 +- default-configs/s390x-softmmu.mak | 23 +- default-configs/sh4-softmmu.mak | 28 +- default-configs/sh4eb-softmmu.mak | 22 +- default-configs/sound.mak | 4 - default-configs/sparc-softmmu.mak | 24 +- default-configs/sparc64-softmmu.mak | 25 +- default-configs/unicore32-softmmu.mak | 6 +- default-configs/usb.mak | 11 - default-configs/virtio.mak | 15 - default-configs/xtensa-softmmu.mak | 8 +- default-configs/xtensaeb-softmmu.mak | 7 +- docs/devel/build-system.txt | 1 - docs/devel/index.rst | 1 + docs/devel/kconfig.rst | 306 ++++++++++++++ hw/9pfs/Kconfig | 4 + hw/9pfs/Makefile.objs | 2 - hw/Kconfig | 73 ++++ hw/Makefile.objs | 4 +- hw/acpi/Kconfig | 29 ++ hw/adc/Kconfig | 2 + hw/alpha/Kconfig | 12 + hw/arm/Kconfig | 124 ++++++ hw/audio/Kconfig | 52 +++ hw/block/Kconfig | 39 ++ hw/block/Makefile.objs | 3 +- hw/block/dataplane/Makefile.objs | 2 +- hw/bt/Kconfig | 2 + hw/char/Kconfig | 42 ++ hw/core/Kconfig | 11 + hw/cpu/Kconfig | 8 + hw/cris/Kconfig | 9 + hw/display/Kconfig | 108 +++++ hw/display/Makefile.objs | 4 +- hw/dma/Kconfig | 21 + hw/gpio/Kconfig | 9 + hw/hppa/Kconfig | 10 + hw/hyperv/Kconfig | 8 + hw/i2c/Kconfig | 27 ++ hw/i2c/Makefile.objs | 2 +- hw/i386/Kconfig | 99 +++++ hw/i386/Makefile.objs | 5 +- hw/ide/Kconfig | 54 +++ hw/input/Kconfig | 33 ++ hw/intc/Kconfig | 57 +++ hw/ipack/Kconfig | 4 + hw/ipmi/Kconfig | 22 + hw/isa/Kconfig | 53 +++ hw/lm32/Kconfig | 13 + hw/m68k/Kconfig | 9 + hw/mem/Kconfig | 11 + hw/microblaze/Kconfig | 20 + hw/mips/Kconfig | 21 + hw/misc/Kconfig | 118 ++++++ hw/misc/macio/Kconfig | 11 + hw/moxie/Kconfig | 3 + hw/net/Kconfig | 125 ++++++ hw/nios2/Kconfig | 8 + hw/nvram/Kconfig | 9 + hw/openrisc/Kconfig | 5 + hw/pci-bridge/Kconfig | 29 ++ hw/pci-host/Kconfig | 51 +++ hw/pci/Kconfig | 9 + hw/pci/Makefile.objs | 9 +- hw/pcmcia/Kconfig | 2 + hw/ppc/Kconfig | 121 ++++++ hw/riscv/Kconfig | 33 ++ hw/s390x/Kconfig | 11 + hw/s390x/Makefile.objs | 4 +- hw/scsi/Kconfig | 54 +++ hw/scsi/Makefile.objs | 2 +- hw/sd/Kconfig | 17 + hw/sh4/Kconfig | 23 ++ hw/smbios/Kconfig | 2 + hw/sparc/Kconfig | 26 ++ hw/sparc64/Kconfig | 19 + hw/ssi/Kconfig | 18 + hw/timer/Kconfig | 63 +++ hw/tpm/Kconfig | 24 ++ hw/tricore/Kconfig | 2 + hw/unicore32/Kconfig | 5 + hw/usb/Kconfig | 91 ++++ hw/usb/Makefile.objs | 2 +- hw/vfio/Kconfig | 36 ++ hw/virtio/Kconfig | 31 ++ hw/virtio/Makefile.objs | 2 + hw/watchdog/Kconfig | 16 + hw/xtensa/Kconfig | 8 + hw/xtensa/Makefile.objs | 2 +- rules.mak | 2 +- scripts/make_device_config.sh | 30 -- scripts/minikconf.py | 708 ++++++++++++++++++++++++++++++++ 115 files changed, 3211 insertions(+), 514 deletions(-) create mode 100644 Kconfig.host delete mode 100644 default-configs/hyperv.mak delete mode 100644 default-configs/pci.mak delete mode 100644 default-configs/sound.mak delete mode 100644 default-configs/usb.mak delete mode 100644 default-configs/virtio.mak create mode 100644 docs/devel/kconfig.rst create mode 100644 hw/9pfs/Kconfig create mode 100644 hw/Kconfig create mode 100644 hw/acpi/Kconfig create mode 100644 hw/adc/Kconfig create mode 100644 hw/alpha/Kconfig create mode 100644 hw/arm/Kconfig create mode 100644 hw/audio/Kconfig create mode 100644 hw/block/Kconfig create mode 100644 hw/bt/Kconfig create mode 100644 hw/char/Kconfig create mode 100644 hw/core/Kconfig create mode 100644 hw/cpu/Kconfig create mode 100644 hw/cris/Kconfig create mode 100644 hw/display/Kconfig create mode 100644 hw/dma/Kconfig create mode 100644 hw/gpio/Kconfig create mode 100644 hw/hppa/Kconfig create mode 100644 hw/hyperv/Kconfig create mode 100644 hw/i2c/Kconfig create mode 100644 hw/i386/Kconfig create mode 100644 hw/ide/Kconfig create mode 100644 hw/input/Kconfig create mode 100644 hw/intc/Kconfig create mode 100644 hw/ipack/Kconfig create mode 100644 hw/ipmi/Kconfig create mode 100644 hw/isa/Kconfig create mode 100644 hw/lm32/Kconfig create mode 100644 hw/m68k/Kconfig create mode 100644 hw/mem/Kconfig create mode 100644 hw/microblaze/Kconfig create mode 100644 hw/mips/Kconfig create mode 100644 hw/misc/Kconfig create mode 100644 hw/misc/macio/Kconfig create mode 100644 hw/moxie/Kconfig create mode 100644 hw/net/Kconfig create mode 100644 hw/nios2/Kconfig create mode 100644 hw/nvram/Kconfig create mode 100644 hw/openrisc/Kconfig create mode 100644 hw/pci-bridge/Kconfig create mode 100644 hw/pci-host/Kconfig create mode 100644 hw/pci/Kconfig create mode 100644 hw/pcmcia/Kconfig create mode 100644 hw/ppc/Kconfig create mode 100644 hw/riscv/Kconfig create mode 100644 hw/s390x/Kconfig create mode 100644 hw/scsi/Kconfig create mode 100644 hw/sd/Kconfig create mode 100644 hw/sh4/Kconfig create mode 100644 hw/smbios/Kconfig create mode 100644 hw/sparc/Kconfig create mode 100644 hw/sparc64/Kconfig create mode 100644 hw/ssi/Kconfig create mode 100644 hw/timer/Kconfig create mode 100644 hw/tpm/Kconfig create mode 100644 hw/tricore/Kconfig create mode 100644 hw/unicore32/Kconfig create mode 100644 hw/usb/Kconfig create mode 100644 hw/vfio/Kconfig create mode 100644 hw/virtio/Kconfig create mode 100644 hw/watchdog/Kconfig create mode 100644 hw/xtensa/Kconfig delete mode 100644 scripts/make_device_config.sh create mode 100644 scripts/minikconf.py