[PATCH v1 0/9] gfxstream + rutabaga_gfx

Gurchetan Singh posted 9 patches 10 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20230711025649.708-1-gurchetansingh@chromium.org
Maintainers: Gerd Hoffmann <kraxel@redhat.com>, "Michael S. Tsirkin" <mst@redhat.com>, Paolo Bonzini <pbonzini@redhat.com>, "Marc-André Lureau" <marcandre.lureau@redhat.com>, "Daniel P. Berrangé" <berrange@redhat.com>, Thomas Huth <thuth@redhat.com>, "Philippe Mathieu-Daudé" <philmd@linaro.org>, Eduardo Habkost <eduardo@habkost.net>
There is a newer version of this series
docs/system/device-emulation.rst     |    1 +
docs/system/devices/virtio-gpu.rst   |   80 ++
hw/display/meson.build               |   22 +
hw/display/virtio-gpu-base.c         |    6 +-
hw/display/virtio-gpu-pci-rutabaga.c |   48 ++
hw/display/virtio-gpu-pci.c          |   14 +
hw/display/virtio-gpu-rutabaga.c     | 1088 ++++++++++++++++++++++++++
hw/display/virtio-gpu.c              |   17 +-
hw/display/virtio-vga-rutabaga.c     |   52 ++
hw/display/virtio-vga.c              |   33 +-
hw/virtio/virtio-pci.c               |   18 +
include/hw/virtio/virtio-gpu-bswap.h |   18 +
include/hw/virtio/virtio-gpu.h       |   34 +
include/hw/virtio/virtio-pci.h       |    4 +
meson.build                          |    7 +
meson_options.txt                    |    2 +
scripts/meson-buildoptions.sh        |    3 +
softmmu/qdev-monitor.c               |    3 +
softmmu/vl.c                         |    1 +
19 files changed, 1431 insertions(+), 20 deletions(-)
create mode 100644 docs/system/devices/virtio-gpu.rst
create mode 100644 hw/display/virtio-gpu-pci-rutabaga.c
create mode 100644 hw/display/virtio-gpu-rutabaga.c
create mode 100644 hw/display/virtio-vga-rutabaga.c
[PATCH v1 0/9] gfxstream + rutabaga_gfx
Posted by Gurchetan Singh 10 months ago
From: Gurchetan Singh <gurchetansingh@google.com>

Latest iteration of rutabaga_gfx + gfxstream patches.  Previous version
and more background available here:

https://patchew.org/QEMU/20230421011223.718-1-gurchetansingh@chromium.org/

Changes since RFC:
- All important memory tests pass
- Went with separate virtio-gpu-rutabaga device as suggested by Bernard
  Berschow
- Incorporated review feedback, mostly from Akihiko Odaki
- gfxstream has new unified guest/host repo + build system improvements
- added documentation on virtio-gpu
- new instructions on how to build available in the tracking bug [a]

In terms of API stability/versioning/packaging, once this series is
reviewed, the plan is to cut a "gfxstream upstream release branch".  We
will have the same API guarantees as any other QEMU project then, i.e no
breaking API changes for 5 years.

The Android Emulator will build both gfxstream (to get bug fixes fast)
and QEMU8.0+ (due to regulatory requirements) from sources.  So we haven't
created a gfxstream Debian/Ubuntu package since we actually don't need it.
Though, we plan to upload our QEMU8.0+ gfxstream enabled builds somewhere
on AOSP when it's ready.

It's more important for us to be in-tree to reduce technical debt given
this.  Let us know if there are any strong opinions on packaging.

Otherwise, feedback + reviews welcome!

[a] https://gitlab.com/qemu-project/qemu/-/issues/1611

Antonio Caggiano (2):
  virtio-gpu: CONTEXT_INIT feature
  virtio-gpu: blob prep

Dr. David Alan Gilbert (1):
  virtio: Add shared memory capability

Gerd Hoffmann (1):
  virtio-gpu: hostmem

Gurchetan Singh (5):
  gfxstream + rutabaga prep: added need defintions, fields, and options
  gfxstream + rutabaga: add initial support for gfxstream
  gfxstream + rutabaga: meson support
  gfxstream + rutabaga: enable rutabaga
  docs/system: add basic virtio-gpu documentation

 docs/system/device-emulation.rst     |    1 +
 docs/system/devices/virtio-gpu.rst   |   80 ++
 hw/display/meson.build               |   22 +
 hw/display/virtio-gpu-base.c         |    6 +-
 hw/display/virtio-gpu-pci-rutabaga.c |   48 ++
 hw/display/virtio-gpu-pci.c          |   14 +
 hw/display/virtio-gpu-rutabaga.c     | 1088 ++++++++++++++++++++++++++
 hw/display/virtio-gpu.c              |   17 +-
 hw/display/virtio-vga-rutabaga.c     |   52 ++
 hw/display/virtio-vga.c              |   33 +-
 hw/virtio/virtio-pci.c               |   18 +
 include/hw/virtio/virtio-gpu-bswap.h |   18 +
 include/hw/virtio/virtio-gpu.h       |   34 +
 include/hw/virtio/virtio-pci.h       |    4 +
 meson.build                          |    7 +
 meson_options.txt                    |    2 +
 scripts/meson-buildoptions.sh        |    3 +
 softmmu/qdev-monitor.c               |    3 +
 softmmu/vl.c                         |    1 +
 19 files changed, 1431 insertions(+), 20 deletions(-)
 create mode 100644 docs/system/devices/virtio-gpu.rst
 create mode 100644 hw/display/virtio-gpu-pci-rutabaga.c
 create mode 100644 hw/display/virtio-gpu-rutabaga.c
 create mode 100644 hw/display/virtio-vga-rutabaga.c

-- 
2.41.0.255.g8b1d071c50-goog
Re: [PATCH v1 0/9] gfxstream + rutabaga_gfx
Posted by Alyssa Ross 9 months, 2 weeks ago
Gurchetan Singh <gurchetansingh@chromium.org> writes:

> In terms of API stability/versioning/packaging, once this series is
> reviewed, the plan is to cut a "gfxstream upstream release branch".  We
> will have the same API guarantees as any other QEMU project then, i.e no
> breaking API changes for 5 years.

What about Rutabaga?
Re: [PATCH v1 0/9] gfxstream + rutabaga_gfx
Posted by Gurchetan Singh 9 months, 2 weeks ago
On Mon, Jul 24, 2023 at 2:56 AM Alyssa Ross <hi@alyssa.is> wrote:
>
> Gurchetan Singh <gurchetansingh@chromium.org> writes:
>
> > In terms of API stability/versioning/packaging, once this series is
> > reviewed, the plan is to cut a "gfxstream upstream release branch".  We
> > will have the same API guarantees as any other QEMU project then, i.e no
> > breaking API changes for 5 years.
>
> What about Rutabaga?

Yes, rutabaga + gfxstream will both be versioned and maintain API
backwards compatibility in line with QEMU guidelines.
Rutabaga backwards compatibility
Posted by Alyssa Ross 9 months, 1 week ago
Gurchetan Singh <gurchetansingh@chromium.org> writes:

> On Mon, Jul 24, 2023 at 2:56 AM Alyssa Ross <hi@alyssa.is> wrote:
>>
>> Gurchetan Singh <gurchetansingh@chromium.org> writes:
>>
>> > In terms of API stability/versioning/packaging, once this series is
>> > reviewed, the plan is to cut a "gfxstream upstream release branch".  We
>> > will have the same API guarantees as any other QEMU project then, i.e no
>> > breaking API changes for 5 years.
>>
>> What about Rutabaga?
>
> Yes, rutabaga + gfxstream will both be versioned and maintain API
> backwards compatibility in line with QEMU guidelines.

In that case, I should draw your attention to
<https://crrev.com/c/4584252>, which I've just realised while testing v2
of your series here breaks the build of the rutabaga ffi, and which will
require the addition of a "prot" field to struct rutabaga_handle (a
breaking change).  I'll push a new version of that CL to fix rutabaga
ffi in the next few days.

Since this is already coming up, before the release has even been made,
is it worth exploring how to limit the rutabaga API to avoid more
breaking changes after the release?  Could there be more use of opaque
structs, for example?

(CCing the crosvm list)
Re: Rutabaga backwards compatibility
Posted by Gurchetan Singh 9 months ago
On Tue, Aug 1, 2023 at 8:18 AM Alyssa Ross <hi@alyssa.is> wrote:

> Gurchetan Singh <gurchetansingh@chromium.org> writes:
>
> > On Mon, Jul 24, 2023 at 2:56 AM Alyssa Ross <hi@alyssa.is> wrote:
> >>
> >> Gurchetan Singh <gurchetansingh@chromium.org> writes:
> >>
> >> > In terms of API stability/versioning/packaging, once this series is
> >> > reviewed, the plan is to cut a "gfxstream upstream release branch".
> We
> >> > will have the same API guarantees as any other QEMU project then, i.e
> no
> >> > breaking API changes for 5 years.
> >>
> >> What about Rutabaga?
> >
> > Yes, rutabaga + gfxstream will both be versioned and maintain API
> > backwards compatibility in line with QEMU guidelines.
>
> In that case, I should draw your attention to
> <https://crrev.com/c/4584252>, which I've just realised while testing v2
> of your series here breaks the build of the rutabaga ffi, and which will
> require the addition of a "prot" field to struct rutabaga_handle (a
> breaking change).  I'll push a new version of that CL to fix rutabaga
> ffi in the next few days.
>

Sorry, I didn't see this until now.  At first glance, do we need to modify
the rutabaga_handle?  Can't we do fcntl(.., GET_FL) to get the access flags
when needed?

Since this is already coming up, before the release has even been made,
> is it worth exploring how to limit the rutabaga API to avoid more
> breaking changes after the release?  Could there be more use of opaque
> structs, for example?
>
> (CCing the crosvm list)
>
Re: Rutabaga backwards compatibility
Posted by Alyssa Ross 9 months ago
Gurchetan Singh <gurchetansingh@chromium.org> writes:

> On Tue, Aug 1, 2023 at 8:18 AM Alyssa Ross <hi@alyssa.is> wrote:
>
>> Gurchetan Singh <gurchetansingh@chromium.org> writes:
>>
>> > On Mon, Jul 24, 2023 at 2:56 AM Alyssa Ross <hi@alyssa.is> wrote:
>> >>
>> >> Gurchetan Singh <gurchetansingh@chromium.org> writes:
>> >>
>> >> > In terms of API stability/versioning/packaging, once this series is
>> >> > reviewed, the plan is to cut a "gfxstream upstream release branch".  We
>> >> > will have the same API guarantees as any other QEMU project then, i.e no
>> >> > breaking API changes for 5 years.
>> >>
>> >> What about Rutabaga?
>> >
>> > Yes, rutabaga + gfxstream will both be versioned and maintain API
>> > backwards compatibility in line with QEMU guidelines.
>>
>> In that case, I should draw your attention to
>> <https://crrev.com/c/4584252>, which I've just realised while testing v2
>> of your series here breaks the build of the rutabaga ffi, and which will
>> require the addition of a "prot" field to struct rutabaga_handle (a
>> breaking change).  I'll push a new version of that CL to fix rutabaga
>> ffi in the next few days.
>
> Sorry, I didn't see this until now.  At first glance, do we need to modify
> the rutabaga_handle?  Can't we do fcntl(.., GET_FL) to get the access flags
> when needed?

That was my original approach[1], but it was difficult to make work on
Windows and not popular.

[1]: https://crrev.com/c/4543310