Re: [virtio-dev] Re: [RFC PATCH v6] virtio-video: Add virtio video device specification

[Alexandre Courbot <acourbot@chromium.org>]

On Fri, Apr 28, 2023 at 12:12âAM Alexander Gordeev
<alexander.gordeev@opensynergy.com> wrote:
>
> On 27.04.23 15:23, Alexandre Courbot wrote:
> > On Thu, Apr 27, 2023 at 12:52âAM Alexander Gordeev
> > <alexander.gordeev@opensynergy.com> wrote:
> >>
> >> On 21.04.23 06:02, Alexandre Courbot wrote:
> >>> On Wed, Apr 19, 2023 at 4:39âPM Alexander Gordeev
> >>> <alexander.gordeev@opensynergy.com> wrote:
> >>>>
> >>>> On 17.04.23 16:43, Cornelia Huck wrote:
> >>>>> On Mon, Apr 17 2023, Alexander Gordeev <alexander.gordeev@opensynergy.com> wrote:
> >>>>>
> >>>>>> OpenSynergy, the company that I work for, develops a proprietary
> >>>>>> hypervisor called COQOS mainly for automotive and aerospace domains. We
> >>>>>> have our proprietary device implementations, but overall our goal is to
> >>>>>> bring open standards into these quite closed domains and we're betting
> >>>>>> big on virtio. The idea is to run safety-critical functions like cockpit
> >>>>>> controller alongside with multimedia stuff in different VMs on the same
> >>>>>> physical board. Right now they have it on separate physical devices. So
> >>>>>> they already have maximum isolation. And we're trying to make this
> >>>>>> equally safe on a single board. The benefit is the reduced costs and
> >>>>>> some additional features. Of course, we also need features here, but at
> >>>>>> the same time security and ease of certification are among the top of
> >>>>>> our priorities. Nobody wants cars or planes to have security problems,
> >>>>>> right? Also nobody really needs DVB and even more exotic devices in cars
> >>>>>> and planes AFAIK.
> >>>>>>
> >>>>>> For the above mentioned reasons our COQOS hypervisor is running on bare
> >>>>>> metal. Also memory management for the guests is mostly static. It is
> >>>>>> possible to make a shared memory region between a device and a driver
> >>>>>> managed by device in advance. But definitely no mapping of random host
> >>>>>> pages on the fly is supported.
> >>>>>>
> >>>>>> AFAIU crosvm is about making Chrome OS more secure by putting every app
> >>>>>> in its own virtualized environment, right? Both the host and guest are
> >>>>>> linux. In this case I totally understand why V4L2 UAPI pass-through
> >>>>>> feels like a right move. I guess, you'd like to make the switch to
> >>>>>> virtualized apps as seemless as possible for your users. If they can't
> >>>>>> use their DVBs anymore, they complain. And adding the virtualization
> >>>>>> makes the whole thing more secure anyway. So I understand the desire to
> >>>>>> have the range of supported devices as broad as possible. It is also
> >>>>>> understandable that priorities are different with desktop
> >>>>>> virtualization. Also I'm not trying to diminish the great work, that you
> >>>>>> have done. It is just that from my perspective this looks like a step in
> >>>>>> the wrong direction because of the mentioned concerns. So I'm going to
> >>>>>> continue being a skeptic here, sorry.
> >>>>>>
> >>>>>> Of course, I don't expect that you continue working on the old approach
> >>>>>> now as you have put that many efforts into the V4L2 UAPI pass-through.
> >>>>>> So I think it is best to do the evolutionary changes in scope of virtio
> >>>>>> video device specification, and create a new device specification
> >>>>>> (virtio-v4l2 ?) for the revolutionary changes. Then I'd be glad to
> >>>>>> continue the virtio-video development. In fact I already started making
> >>>>>> draft v7 of the spec according to the comments. I hope it will be ready
> >>>>>> for review soon.
> >>>>>>
> >>>>>> I hope this approach will also help fix issues with virtio-video spec
> >>>>>> and driver development misalignment as well as V4L2 compliance issues
> >>>>>> with the driver. I believe the problems were caused partly by poor
> >>>>>> communication between us and by misalignment of our development cycles,
> >>>>>> not by the driver complexity.
> >>>>>>
> >>>>>> So in my opinion it is OK to have different specs with overlapping
> >>>>>> functionality for some time. My only concern is if this would be
> >>>>>> accepted by the community and the committee. How the things usually go
> >>>>>> here: preferring features and tolerating possible security issues or the
> >>>>>> other way around? Also how acceptable is having linux-specific protocols
> >>>>>> at all?
> >>>>> My main question is: What would be something that we can merge as a
> >>>>> spec, that would either cover the different use cases already, or that
> >>>>> could be easily extended to cover the use cases it does not handle
> >>>>> initially?
> >>>>>
> >>>>> For example, can some of the features that would be useful in crosvm be
> >>>>> tucked behind some feature bit(s), so that the more restricted COQOS
> >>>>> hypervisor would simply not offer them? (Two feature bits covering two
> >>>>> different mechanisms, like the current approach and the v4l2 approach,
> >>>>> would also be good, as long as there's enough common ground between the
> >>>>> two.)
> >>>>>
> >>>>> If a staged approach (adding features controled by feature bits) would
> >>>>> be possible, that would be my preferred way to do it.
> >>>>
> >>>> Hmm, I see several ways how we can use the feature flags:
> >>>> 1. Basically making two feature flags: one for the current video spec
> >>>> and one for the V4L2 UAPI pass through. Kind of the same as having two
> >>>> different specs, but within one device. Not sure which way is better.
> >>>> Probably having two separate devices would be easier to review and merge.
> >>>
> >>> Having two different devices with their own IDs would indeed be less
> >>> confusing than using feature bits.
> >>>
> >>> That being said, the whole point of proposing virtio-v4l2 is to end up
> >>> with *less* specification, not more. Having two concurrent and largely
> >>> overlapping approaches will result in fragmentation and duplicated
> >>> work, so my suggestion would be to decide on one or the other and
> >>> stick to it.
> >>
> >> Hmm, Enrico pointed out, that having virtio-v4l2 would also be good
> >> because of much better compatibility with Android right now. I don't
> >> think the specification length should be our ultimate goal. Cornelia
> >> said, that her ultimate goal is to have a spec everyone is happy with,
> >> regardless on how we arrive there. Well, I can only say, that I also
> >> think this should be our goal.
> >
> > Try to put yourself into the shoes of someone who needs to write a new
> > video device using virtio. Oh, there are two ways to do it. This guest
> > OS only supports virtio-video. But this guest OS only supports
> > virtio-v4l2. Great, now you need to support two interfaces with your
> > device, or write two different devices.
>
> I think this is a hypothetical issue.

Issues have a strange tendency to become hypothetical when it's
convenient for you.

> IMO it makes sense to use V4L2
> UAPI only on Linux host with V4L2 devices + Linux/Android guest.
> virtio-video driver is already available for Linux. We already know our
> priorities and limitations, so building a decision tree for a potential
> device developer would be very easy.
>
> >>> * Having two overlapping specifications for video is overkill and will
> >>> just fragment virtio (as tempting as it is, I won't link to XKCD). I
> >>> strongly advise against that.
> >>
> >> I think they're not going to create more problems, than virtio-blk,
> >> virtio-scsi and virtio-fs, for example.
> >
> > At least these devices work at different layers, that makes them more
> > justifiable. virtio-video and virtio-v4l2 are just going to provide
> > the same API for video devices, only with different structures and
> > commands.
>
> Hmm, I think virtio-blk and virtio-scsi work on the same level, aren't
> they? They could also say this is basically the same thing.
>
> >> The decision can be made like this:
> >> 1. You have a V4L2 device, you don't need any more processing, just want
> >> it inside a Linux/Android VM => use virtio-v4l2.
> >> 2. You don't have a V4L2 device, or your host is not Linux, or your
> >> maybe your guest is not Linux/Android, or you want some extra processing
> >> on the host (say you have a third-party proprietary library or whatever)
> >> => use virtio-video.
> >
> > That would make sense if 2. could not be done just as easily by also
> > using virtio-v4l2, which I believe it can be.
>
> I'm sorry, I think a potential developer would just look into V4L2 docs,
> see struct v4l2_buffer (AFAIU with patches in the spec for the
> host/guest/object memory types on top) and run back to the cleanliness
> and simplicity of virtio-video. That's basically my story. :)
> Iterating on formats in VIDIOC_ENUM_FMT looks quite weird to me too.

I think you will probably end up doing similarly in virtio-video due
to the dependency between input and output formats that is not easy to
represent in a single structure.