Re: [Qemu-devel] virtio-gpu doesn't build if you do a linux-headers update from kvm/next

[Laszlo Ersek]

On 11/05/15 19:13, Cornelia Huck wrote:
> On Thu, 5 Nov 2015 18:15:04 +0100
> Laszlo Ersek <address@hidden> wrote:
> 
>> On 11/05/15 16:11, Peter Maydell wrote:
>>> On 5 November 2015 at 14:58, Gerd Hoffmann <address@hidden> wrote:
>>>> On Do, 2015-11-05 at 14:45 +0000, Peter Maydell wrote:
>>>>> On 5 November 2015 at 14:42, Gerd Hoffmann <address@hidden> wrote:
>>>>>> Chicken & egg issue in that case because airlied (linux kernel drm
>>>>>> maintainer) asked to have the qemu changes merged before taking the
>>>>>> virtio-gpu pull request.  So I had no other chance than creating the
>>>>>> patches with not-yet upstream virtio header changes ...
>>>>>
>>>>> Hmm. If I'd realised that at the time I'd have pushed back on it.
>>>>> We should never take code that relies on upstream kernel
>>>>> ABI that hasn't been accepted by the maintainer yet.
>>>>
>>>> The reason airlied asked for qemu being upstream first is to avoid
>>>> having code in the kernel tree not accepted by qemu yet ...
>>>>
>>>> So, one of the two has to go first ;)
>>>
>>> Right, but this isn't a symmetrical arrangement. If on the
>>> kernel side the ABI is changed before it's finally accepted
>>> into mainline, that means that any QEMU that got released with
>>> changes made on the basis of not-yet-upstream kernel patches
>>> will be broken. But if the kernel accepts code which needs a
>>> not-yet-in-qemu feature to be useful, that doesn't break the
>>> kernel, because the kernel isn't actually dependent on the
>>> QEMU changes. That's why I think the kernel side of the ABI
>>> always needs to go first (the kernel provides the ABI, QEMU
>>> uses it, never the other way around).
>>
>> I could be confused about what ABI means, but in case of *guest* kernel
>> drivers (for virtual devices provided by QEMU), the dependency seems to
>> be the opposite.
>>
>> Alex mentioned "kernel uapi includes" up-thread. To be honest, I don't
>> know what "uapi" means here. Are those headers there to be relied upon
>> by userspace processes that need services from the kernel? That would
>> match your above argument.
>>
>> However, I found the following files there:
>>
>> include/uapi/linux/virtio_console.h
>> include/uapi/linux/virtio_input.h
>> include/uapi/linux/virtio_net.h
>> include/uapi/linux/virtio_types.h
>> include/uapi/linux/virtio_9p.h
>> include/uapi/linux/virtio_balloon.h
>> include/uapi/linux/virtio_scsi.h
>> include/uapi/linux/virtio_rng.h
>> include/uapi/linux/virtio_ids.h
>> include/uapi/linux/virtio_ring.h
>> include/uapi/linux/virtio_pci.h
>> include/uapi/linux/virtio_gpu.h
>> include/uapi/linux/virtio_config.h
>> include/uapi/linux/virtio_blk.h
>>
>> Which kinda confuses me. I cannot imagine that a userspace process
>> should depend on these, for the purpose of consuming kernel services.
>> Instead, QEMU should *definitely* dictate in this case, because it
>> provides the service (the hardware), and Linux has drivers that consume
>> that service.
>>
>> Consider: there are virtio drivers in SeaBIOS, OVMF, and Windows too.
>> Should QEMU include their header files as well? I think not.
>>
>> In other words, I agree with your argument, and the "QEMU depends on the
>> kernel" statement, as far as QEMU plays the role of a userspace process
>> that consumes the *host* kernel's services (VFIO, KVM, ...).
>>
>> However, when QEMU provides the *hardware* (which is hopefully described
>> by an industry standard, or at least by some kind of independent
>> documentation), then the *guest* kernel plays the consumer role (as do
>> other non-Linux-kernel guests), and in that case the QEMU changes should
>> go in first (hopefully after testing them with the under-development
>> guest code, of course).
>>
>> Hence, I'm leaning to think that the above virtio header files should
>> not be under uapi *at all*. They should be under "include/virtio", and
>> should be private to the guest drivers.
> 
> The virtio case is more of a convenience issue.
> 
> The *canonical* definitions for virtio implementations are neither in
> the kernel nor in qemu, but only in the standard. We can't conveniently
> pull headers (for Linux and qemu) from a third place, so we make sure
> that the Linux headers implement the standard and have qemu pull them

My argument is that this convenience is a bad thing, because it
introduces cross-project dependencies (and misleading uses of uapi per
se) that are not warranted for by the roles the projects play.

The convenience the current workflow promises is actually a pain. These
(non-)dependency arguments surface every time something new is
implemented that requires both host and guest support.

QEMU and the Linux kernel (and all other guests) should maintain their
own independent sets of definitions.

> (this is probably also a corollary of mostly the same people working on
> the Linux and qemu implementations).

The convenience sync also allows contributors to conveniently forget
about other guest code beside Linux. "It's in Linux and in QEMU, I'm done."

That is alright, unless the contributor's primary guest code actually
has a (not so obvious) dependency on that other guest code.

> IOW, I think in the virtio case *both* Linux and qemu are consumers.

In the actual service consumption sense, I don't think so; but as far as
"supporting the virtio standard" is concerned, I agree fully.

The point is, cross-project dependencies should only be dictated by the
former kind -- actual service consumption.

Given that QEMU and Linux are separate, independent "consumers" of the
virtio spec (in the latter sense), each should have an independent set
of definitions for compliance with the spec.

> The setup is as-is as it seems to be most convenient for the developers
> involved.

I agree. Until Peter asks if the header changes are in Linux yet, and
the contributor responds that the kernel maintainer wanted to see the
changes in QEMU first.

> (There has been discussion on this before, which I'm too lazy to dig up
> right now.)
> 

I apologize for dredging it up then. I've not been aware of the earlier
discussion. But, it does show that the problem remains unsolved, and a pain.

... I guess my main reason for obsessing about this is because I
manually code up all industry standard-related macros and types in edk2,
from the revelant specs directly. It gives me incredible freedom; the
code conforms to the coding style; the definitions are as minimal as
possible; and I regularly gain insight from typing them and laying them
out. Sometimes they even help me find undocumented restrictions /
limitations in the interfacing project.

Yes, with the separate definitions, new features require more typing.
That's a good thing.

Anyway, thanks for replying; I guess I've annoyed most people following
this thread quite a bit by now. I shall shut up.

Cheers
Laszlo