Re: [Qemu-devel] [PULL v3 00/54] Kconfig conversion, excluding ARM and MIPS

[Laszlo Ersek]

On 03/21/19 15:21, Philippe Mathieu-Daudé wrote:
> Le jeu. 21 mars 2019 13:39, Laszlo Ersek <address@hidden> 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
>>
>>
>