There is an issue with minigbm, virtio-gpu resources, and the zwp_linux_dmabuf_v1 wayland interface. The typical guest wayland egl application will do the following:

1) The application uses EGL to make a surface for the gpu (mesa's virtio gpu driver in this case).
2) The driver will use DRM_IOCTL_VIRTGPU_RESOURCE_CREATE to create a resource.
3) Crosvm creates that resource using minigbm on the host (i915 driver in the case of eve).
4) The wayland platform of EGL uses that resource along with associated buffer metadata in the zwp_linux_dmabuf_v1 interface to create a wl_buffer.
5) The wl_buffer is attached to a wl_surface, triggering its eventual display by Chrome compositor.
6) Chrome compositor attempts to create a GL texture (for composition) or a DRM framebuffer (for overlay) from the zwp_linux_dmabuf_v1 supplied resource and metadata.

The issue is that the buffer's metadata is calculated twice: once by the guest mesa driver using the generic constraints of virtio gpu, and once by the host minigbm driver which will use driver specific constraints. Sometimes, critical metadata does not agree, such is the stride. For example, on i915, the stride must be 64 byte aligned but there is no such restriction on virtio-gpu. This is not an issue for rendering purposes because buffers are not directly mapped in the guest, but this does cause to an issue with the zwp_linux_dmabuf_v1 protocol, which will be using the guest's incorrect metadata (stride and possibly modifier) and will most likely fail on step 6.

A potential solution would be to pass down true buffer metadata (from the point of view of the host driver) to the VIRTIO_GPU_CMD_RESOURCE_CREATE_3D call in crosvm, and then through the kernel to userspace in the DRM_IOCTL_VIRTGPU_RESOURCE_CREATE ioctl. That way the egl driver would have correct metadata for the zwp_linux_dmabuf_v1 interface. Are there any thoughts about this plan, or the problem in general?

The approach outlined in the meeting (returning the correct stride) sounded good to me.

Also, will cros_vm have composition information when it decides to allocate via minigbm?  I'm asking since we have long-standing problem that we can't ever allocate compressed formats in certain cases for VMs and containers (since they are not the compositor) -- see https://buganizer.corp.google.com/issues/70234632.

I am not sure why stride is so special, there are meta data like width/height/format etc and we don't return them to user space from kernel, why we need to return a correct stride from kernel?

Stride isn't special, it's just an example of something that the guest does not know which needs to be correct for this to work. As far as I can tell, minigbm will not change the width, height, or format from what was requested (it will fail if it can't match those) so the guest will have correct information about it.

Re #7, for format like yv12,  actually for each plane, it's using R8 format, and there are different height/weight for each planes. now code just get this information based on same original width/height/format and assume the actual width/height/format for each plane should be "fixed", I guess we can do same thing for stride?

#5: crosvm has very little usage information at buffer time. We employ a simple heuristic to determine if a buffer should be allocated in minigbm  or virglrenderer, but it will always have false-positives. That is, some allocations crosvm decided to make in minigbm will never end up in the compositor. For reference, this is the structure as seen on the virtio queue

struct virtio_gpu_resource_create_3d {
	struct virtio_gpu_ctrl_hdr hdr;
	__le32 resource_id;
	__le32 target;
	__le32 format;
	__le32 bind;
	__le32 width;
	__le32 height;
	__le32 depth;
	__le32 array_size;
	__le32 last_level;
	__le32 nr_samples;
	__le32 flags; // 0 or VIRGL_RESOURCE_Y_0_TOP
	__le32 padding;
};

Re #7: I'm not sure what you're proposing but I will be concrete so that the purpose of this issue is clear. The code at https://gitlab.freedesktop.org/mesa/mesa/blob/54a9622d/src/egl/drivers/dri2/platform_wayland.c#L832 needs to build up a zwp_linux_dmabuf_v1_create_params request with correct metadata. Each plane of the create_params request has an index, offset, stride, and modifier. The create_params request has a single width, height, and format. The create_params request must be accurate or composition will eventually fail. The guest knows the width, height, and format because they requested that when they made the buffer. The offset, stride, and modifier are not known because it depends on the host minigbm's driver.

Re #10, ok, actually I don't know the detail of this issue. Actually I jump in this bug because it's mentioned here:
https://lists.freedesktop.org/archives/virglrenderer-devel/2018-September/001587.html

I understand your point for passing back right stride. Just wondering, is that possible someday we also need something else and we should build something more general. Of course, it's possible that as you said, it sounds like we only need to care about offset/stide/modifier so we don't need to think about it too much now.

There is actually "something special" about stride. Width is just a number of pixels in a line that the client cares about, height is just a number of lines the client cares about. However, stride actually defines what "a line" is.

As long as all the consumers use the same stride, width <= stride and height*stride <= buffer (or plane) size, they can access image contents fine, regardless of whether they use the same widths and heights.

However, there is another "special" parameter, next to stride, which becomes meaningful only for planar formats - plane offset. For example, given a YUV420 buffer, with 3 planes (Y, Cb, Cr), width, height, stride alone is not enough to determine the location of Cb and Cr planes in the buffer, since there may be further padding needed (e.g. to full 16 lines).

I don't see a way to pass information about planes through current virtio-gpu create commands. One can indeed work around it by using R8 and allocating more lines to cover all 3 planes, e.g. height = real_height + real_height/2, but it doesn't work with hosts, which need additional alignment of planes. One could allocate 3 separate R8 buffers, but that wouldn't work with host components which require all planes in 1 buffer.

Perhaps we may need to extend the interface?

Re #12, fyi, for YUV420 on virgl, actually I have a series proof of concept CL to address this issue. As you said, I have to extend the interface. It's from here:

https://lists.freedesktop.org/archives/virglrenderer-devel/2018-August/001579.html
kernel: https://chromium-review.googlesource.com/c/chromiumos/third_party/kernel/+/1194613
mesa:  https://chromium-review.googlesource.com/c/chromiumos/third_party/mesa/+/1195011
minigbm: https://chromium-review.googlesource.com/c/chromiumos/platform/minigbm/+/1195587

I am not sure if I understand the issue correctly:

When the application in guest use DRM_IOCTL_VIRTGPU_RESOURCE_CREATE to create a resource, it will get a FD to the resource and that FD is connected with a memory in guest memory space. Virglrender will create a GL texture as backend of that guest memory and virglrender will upload guest memory to host GL texture when guest call IOCTL_TRANSFER_TO_HOST.

So here, it seems we can't just use wayland to pass that fd between host and guest since that fd is not from virtio-wayland driver. 

It seems what we need is:

At step 6, chrome compositor just directly use GL texture backed that guest memory. Since we can't create wl_buffer in host from that guest fd.

Re #16, ok, it seems there is code inside crosvm which is using custom code path to handle resource_create request.

re #16, I have already devised solutions to passing these buffers around but david riley and I are currently ironing out the kinks:

https://chromium-review.googlesource.com/q/BUG%253D875998

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/crosvm/+/aa5756669a8331420b84a22e29ddbfc13b791da5

commit aa5756669a8331420b84a22e29ddbfc13b791da5
Author: Zach Reizner <zachr@google.com>
Date: Wed Dec 12 03:33:56 2018

devices: allow virtio-wayland to use virtgpu resources

This change uses the resource bridge between virtio-gpu and virtio-cpu
to send resources over the host wayland connection that originated from
the virtio-gpu device. This will help support gpu accelerated wayland
surfaces.

BUG=chromium:875998
TEST=wayland-simple-egl

Change-Id: I3340ecef438779be5cb3643b2de8bb8c33097d75
Reviewed-on: https://chromium-review.googlesource.com/1182793
Commit-Ready: ChromeOS CL Exonerator Bot <chromiumos-cl-exonerator@appspot.gserviceaccount.com>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>

[add] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/src/virtio/resource_bridge.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/src/linux.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/src/virtio/gpu/backend.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/src/virtio/wl.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/src/lib.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/src/virtio/gpu/mod.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/Cargo.lock
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/Cargo.toml
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/devices/src/virtio/mod.rs
[modify] https://crrev.com/aa5756669a8331420b84a22e29ddbfc13b791da5/gpu_renderer/src/pipe_format_fourcc.rs

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform/crosvm/+/c5899296c41f9c3991542350a3e2e6118e1972c2

commit c5899296c41f9c3991542350a3e2e6118e1972c2
Author: Zach Reizner <zachr@google.com>
Date: Sat Dec 15 06:40:45 2018

devices: gpu: add plane info response support

In order to properly send dmabufs over the wayland protocol, accurate
buffer metadata is needed in the guest. This change plumbs information
from minigbm allocations to the guest using a virtio-gpu response.

BUG=875998
TEST=wayland-simple-egl

Change-Id: I5c80d539bc7757c302ad7adf56f5d3b011304617
Reviewed-on: https://chromium-review.googlesource.com/1227054
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: David Riley <davidriley@chromium.org>

[modify] https://crrev.com/c5899296c41f9c3991542350a3e2e6118e1972c2/devices/src/virtio/gpu/protocol.rs
[modify] https://crrev.com/c5899296c41f9c3991542350a3e2e6118e1972c2/devices/src/virtio/gpu/backend.rs

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/platform2/+/2f88b5500401d9d6c07b6673d9f7326c5dee62d8

commit 2f88b5500401d9d6c07b6673d9f7326c5dee62d8
Author: Zach Reizner <zachr@google.com>
Date: Sat Dec 15 06:40:20 2018

vm_tools: sommelier: fix stride0 on create_prime_buffer

This fixes an issue with using virtio-gpu resources over virtio-wayland
in which the guest and the host would have different values for the
stride of a framebuffer. This does not affect rendering but does confuse
the host compositor when it tries to import the real dmabuf with the
wrong stride. To solve the issue, sommelier attempts to fix the stride
being sent to the compositor over the wayland protocol.

TEST=eglgears_wayland
BUG=chromium:875998,chromium:892242

Change-Id: I31e7c221a941ebebc099b5e57f175951be2008b6
Reviewed-on: https://chromium-review.googlesource.com/1279274
Commit-Ready: Zach Reizner <zachr@chromium.org>
Tested-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: David Riley <davidriley@chromium.org>

[modify] https://crrev.com/2f88b5500401d9d6c07b6673d9f7326c5dee62d8/vm_tools/sommelier/sommelier-drm.c
[modify] https://crrev.com/2f88b5500401d9d6c07b6673d9f7326c5dee62d8/vm_tools/sommelier/sommelier.gyp

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/11a93b73abe7df1fb77b5ec4ee151676457a3cd9

commit 11a93b73abe7df1fb77b5ec4ee151676457a3cd9
Author: Zach Reizner <zachr@google.com>
Date: Tue Jan 08 03:40:27 2019

CHROMIUM: virtwl: send foreign id support

This change is needed to support sending virtualized host resources
across virtio wayland connections. Initially, only virtio gpu resources
are supported.

Ensure that the host has processed the create request before trying
to send it otherwise there is a race where the send can occur before
the create is processed.

BUG=chromium:875998, chromium:892342 
TEST=wayland-simple-egl
     eyes repeatedly does not fail

Change-Id: I4f7ceff45f048cffcb87ce065feefd455578e6e3
Signed-off-by: Zach Reizner <zachr@google.com>
Signed-off-by: David Riley <davidriley@chromium.org>
Reviewed-on: https://chromium-review.googlesource.com/1351812
Reviewed-by: Zach Reizner <zachr@chromium.org>

[modify] https://crrev.com/11a93b73abe7df1fb77b5ec4ee151676457a3cd9/drivers/virtio/virtio_wl.c
[modify] https://crrev.com/11a93b73abe7df1fb77b5ec4ee151676457a3cd9/include/uapi/linux/virtio_wl.h

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/third_party/kernel/+/bd498c054f657e996db80a39eb16486003c46f1b

commit bd498c054f657e996db80a39eb16486003c46f1b
Author: Zach Reizner <zachr@google.com>
Date: Tue Jan 08 03:40:23 2019

CHROMIUM: virtwl: store plane info per virtio_gpu_object

This change extends the drm_virtgpu_resource_info struct to include that
plane info.

BUG=chromium:875998
TEST=wayland-simple-egl

Change-Id: I9941133844b3c218c4183861b35a7f0b0e716bfa
Signed-off-by: Zach Reizner <zachr@google.com>
Reviewed-on: https://chromium-review.googlesource.com/1351813
Commit-Ready: David Riley <davidriley@chromium.org>
Tested-by: David Riley <davidriley@chromium.org>
Reviewed-by: Tomasz Figa <tfiga@chromium.org>
Reviewed-by: Zach Reizner <zachr@chromium.org>
Reviewed-by: Gurchetan Singh <gurchetansingh@chromium.org>

[modify] https://crrev.com/bd498c054f657e996db80a39eb16486003c46f1b/drivers/gpu/drm/virtio/virtgpu_vq.c
[modify] https://crrev.com/bd498c054f657e996db80a39eb16486003c46f1b/include/uapi/drm/virtgpu_drm.h
[modify] https://crrev.com/bd498c054f657e996db80a39eb16486003c46f1b/drivers/gpu/drm/virtio/virtgpu_ioctl.c
[modify] https://crrev.com/bd498c054f657e996db80a39eb16486003c46f1b/drivers/gpu/drm/virtio/virtgpu_drv.h
[modify] https://crrev.com/bd498c054f657e996db80a39eb16486003c46f1b/include/uapi/linux/virtio_gpu.h