On Mon, May 8, 2023 at 3:12 PM Cornelia Huck <coh...@redhat.com> wrote:
> On Wed, May 03 2023, Albert Esteve <aest...@redhat.com> wrote:
>
> > This API manages objects (in this iteration,
> > dmabuf fds) that can be shared along different
> > virtio devices.
> >
> > The API allows the different devices to add,
> > remove and/or retrieve the objects by simply
> > invoking the public functions that reside in the
> > virtio-dmabuf file.
> >
> > Signed-off-by: Albert Esteve <aest...@redhat.com>
> > ---
> > hw/display/meson.build | 1 +
> > hw/display/virtio-dmabuf.c | 88 +++++++++++++++++++++++
> > include/hw/virtio/virtio-dmabuf.h | 58 ++++++++++++++++
> > tests/unit/meson.build | 1 +
> > tests/unit/test-virtio-dmabuf.c | 112 ++++++++++++++++++++++++++++++
> > 5 files changed, 260 insertions(+)
> > create mode 100644 hw/display/virtio-dmabuf.c
> > create mode 100644 include/hw/virtio/virtio-dmabuf.h
> > create mode 100644 tests/unit/test-virtio-dmabuf.c
> >
> > diff --git a/hw/display/meson.build b/hw/display/meson.build
> > index 17165bd536..62a27395c0 100644
> > --- a/hw/display/meson.build
> > +++ b/hw/display/meson.build
> > @@ -37,6 +37,7 @@ softmmu_ss.add(when: 'CONFIG_MACFB', if_true:
> files('macfb.c'))
> > softmmu_ss.add(when: 'CONFIG_NEXTCUBE', if_true: files('next-fb.c'))
> >
> > softmmu_ss.add(when: 'CONFIG_VGA', if_true: files('vga.c'))
> > +softmmu_ss.add(when: 'CONFIG_VIRTIO', if_true: files('virtio-dmabuf.c'))
> >
> > if (config_all_devices.has_key('CONFIG_VGA_CIRRUS') or
> > config_all_devices.has_key('CONFIG_VGA_PCI') or
> > diff --git a/hw/display/virtio-dmabuf.c b/hw/display/virtio-dmabuf.c
> > new file mode 100644
>
> General comment: new files should be covered in MAINTAINERS; not sure if
> there is any generic section that could match it, or if this should go
> into a new section.
>
You are right. I thought the entire folder would have an owner already, but
I see
it is split by features. I guess this would make sense under a new section
then.
Thanks!
>
> > index 0000000000..3db939a2e3
> > --- /dev/null
> > +++ b/hw/display/virtio-dmabuf.c
>
> Is virtio-dmabuf only useful for stuff under display/, or could it go
> into a more generic section?
>
>
I hesitated myself and I wouldn't be against changing the location.
In this first version of the infrastructure, it is introduced with dma-buf
sharing in mind, and virtio-gpu -> virtio-video as the main usecase.
Both these devices are/will be at display/, hence I ended up adding
the infrastructure in the same folder, close from where it is going to be
used.
However, in the future other devices may want to use the shared table
for other object types, the virtio specs seem to leave that door open.
In that case, it may be more interesting in another folder.
I had ui/ or hw/virtio/ as candidates myself. Depends on whether
we want to plan ahead for future uses or keep it closer to where it
is being to be used now.
