[virtio_magma] Add scenic token to images.
VirtioImages now provide eventpair handles to the wayland bridge,
so this change requires reverts of the image tiling workarounds.
Bug:71878
Change-Id: I2c404a7b339f5262ae31c53fa64422a4103bd860
Reviewed-on: https://fuchsia-review.googlesource.com/c/fuchsia/+/511825
Commit-Queue: Craig Stout <cstout@google.com>
Reviewed-by: Abdulla Kamar <abdulla@google.com>
Reviewed-by: David Reveman <reveman@google.com>
API-Review: Abdulla Kamar <abdulla@google.com>
diff --git a/sdk/fidl/fuchsia.virtualization.hardware/device.fidl b/sdk/fidl/fuchsia.virtualization.hardware/device.fidl
index 05ab77e5..e36c1f0 100644
--- a/sdk/fidl/fuchsia.virtualization.hardware/device.fidl
+++ b/sdk/fidl/fuchsia.virtualization.hardware/device.fidl
@@ -171,6 +171,7 @@
/// opaque image info needed by VirtioMagma.
resource struct VirtioImage {
zx.handle:VMO vmo;
+ zx.handle:EVENTPAIR? token;
bytes:VIRTIO_WAYLAND_MAX_IMAGE_INFO_SIZE info;
};
diff --git a/src/virtualization/bin/vmm/device/BUILD.gn b/src/virtualization/bin/vmm/device/BUILD.gn
index 3fd19fb..cb88ee4 100644
--- a/src/virtualization/bin/vmm/device/BUILD.gn
+++ b/src/virtualization/bin/vmm/device/BUILD.gn
@@ -182,6 +182,7 @@
"magma_image.h",
]
deps = [
+ "//sdk/fidl/fuchsia.scenic.allocation:fuchsia.scenic.allocation_llcpp",
"//sdk/fidl/fuchsia.sysmem",
"//src/graphics/lib/magma/include:magma_abi",
"//src/lib/fsl",
diff --git a/src/virtualization/bin/vmm/device/magma_image.cc b/src/virtualization/bin/vmm/device/magma_image.cc
index f0bd470..70998ac1 100644
--- a/src/virtualization/bin/vmm/device/magma_image.cc
+++ b/src/virtualization/bin/vmm/device/magma_image.cc
@@ -4,6 +4,7 @@
#include "magma_image.h"
+#include <fuchsia/scenic/allocation/llcpp/fidl.h>
#include <fuchsia/sysmem/llcpp/fidl.h>
#include <lib/async-loop/cpp/loop.h>
#include <lib/image-format-llcpp/image-format-llcpp.h>
@@ -67,6 +68,7 @@
public:
vk::Result InitVulkan(uint32_t physical_device_index);
zx_status_t InitSysmem();
+ zx_status_t InitScenic();
vk::PhysicalDeviceLimits GetPhysicalDeviceLimits();
@@ -76,18 +78,24 @@
std::vector<uint64_t>& modifiers);
zx_status_t GetImageInfo(uint32_t width, uint32_t height, zx::vmo* vmo_out,
- magma_image_info_t* image_info_out);
+ zx::eventpair* token_out, magma_image_info_t* image_info_out);
+
+ // Scenic is used if client asks for presentable images.
+ bool use_scenic() { return scenic_allocator_.client_end().is_valid(); }
private:
vk::DispatchLoaderDynamic loader_;
vk::UniqueInstance instance_;
vk::PhysicalDevice physical_device_;
vk::UniqueDevice device_;
+ fidl::WireSyncClient<fuchsia_scenic_allocation::Allocator> scenic_allocator_;
fidl::WireSyncClient<fuchsia_sysmem::Allocator> sysmem_allocator_;
fidl::WireSyncClient<fuchsia_sysmem::BufferCollectionToken> local_token_;
fidl::WireSyncClient<fuchsia_sysmem::BufferCollectionToken> vulkan_token_;
+ fidl::ClientEnd<fuchsia_sysmem::BufferCollectionToken> scenic_token_endpoint_;
std::shared_ptr<AsyncHandler> async_handler_;
fidl::Client<fuchsia_sysmem::BufferCollection> collection_;
+ fuchsia_scenic_allocation::wire::BufferCollectionImportToken scenic_import_token_;
};
vk::Result VulkanImageCreator::InitVulkan(uint32_t physical_device_index) {
@@ -175,13 +183,7 @@
zx_status_t VulkanImageCreator::InitSysmem() {
{
- auto client_end_directory = service::OpenServiceRoot();
- if (!client_end_directory.is_ok()) {
- LOG_VERBOSE("Failed to open service root: %d", client_end_directory.status_value());
- return client_end_directory.status_value();
- }
-
- auto client_end = service::ConnectAt<fuchsia_sysmem::Allocator>(*client_end_directory);
+ auto client_end = service::Connect<fuchsia_sysmem::Allocator>();
if (!client_end.is_ok()) {
LOG_VERBOSE("Failed to connect to sysmem allocator: %d", client_end.status_value());
return client_end.status_value();
@@ -210,6 +212,23 @@
fidl::WireSyncClient<fuchsia_sysmem::BufferCollectionToken>(std::move(endpoints->client));
}
+ if (use_scenic()) {
+ auto endpoints = fidl::CreateEndpoints<fuchsia_sysmem::BufferCollectionToken>();
+ if (!endpoints.is_ok()) {
+ LOG_VERBOSE("Failed to create endpoints: %d", endpoints.status_value());
+ return endpoints.status_value();
+ }
+
+ constexpr uint32_t kNoRightsAttentuation = ~0;
+ auto result = local_token_.Duplicate(kNoRightsAttentuation, std::move(endpoints->server));
+ if (!result.ok()) {
+ LOG_VERBOSE("Failed to duplicate token: %d", result.status());
+ return result.status();
+ }
+
+ scenic_token_endpoint_ = std::move(endpoints->client);
+ }
+
{
auto endpoints = fidl::CreateEndpoints<fuchsia_sysmem::BufferCollectionToken>();
if (!endpoints.is_ok()) {
@@ -229,7 +248,8 @@
}
{
- auto result = vulkan_token_.Sync();
+ // Sync the local token that was used for Duplicating
+ auto result = local_token_.Sync();
if (!result.ok()) {
LOG_VERBOSE("Failed to sync token: %d", result.status());
return result.status();
@@ -239,6 +259,19 @@
return ZX_OK;
}
+zx_status_t VulkanImageCreator::InitScenic() {
+ auto client_end = service::Connect<fuchsia_scenic_allocation::Allocator>();
+ if (!client_end.is_ok()) {
+ LOG_VERBOSE("Failed to connect to scenic allocator: %d", client_end.status_value());
+ return client_end.status_value();
+ }
+
+ scenic_allocator_ =
+ fidl::WireSyncClient<fuchsia_scenic_allocation::Allocator>(std::move(*client_end));
+
+ return ZX_OK;
+}
+
vk::PhysicalDeviceLimits VulkanImageCreator::GetPhysicalDeviceLimits() {
assert(physical_device_);
@@ -252,6 +285,29 @@
std::vector<uint64_t>& modifiers) {
assert(device_);
+ if (use_scenic()) {
+ fuchsia_scenic_allocation::wire::BufferCollectionExportToken export_token;
+
+ zx_status_t status = zx::eventpair::create(0, &export_token.value, &scenic_import_token_.value);
+ if (status != ZX_OK) {
+ LOG_VERBOSE("zx::eventpair::create failed: %d", status);
+ return vk::Result::eErrorInitializationFailed;
+ }
+
+ auto result = scenic_allocator_.RegisterBufferCollection(std::move(export_token),
+ std::move(scenic_token_endpoint_));
+ if (!result.ok()) {
+ LOG_VERBOSE("RegisterBufferCollection returned %d", result.status());
+ return vk::Result::eErrorInitializationFailed;
+ }
+
+ if (result->result.is_err()) {
+ LOG_VERBOSE("RegisterBufferCollection is_err()");
+ return vk::Result::eErrorInitializationFailed;
+ }
+ }
+
+ // Set vulkan constraints.
vk::UniqueHandle<vk::BufferCollectionFUCHSIA, vk::DispatchLoaderDynamic> vk_collection;
{
@@ -285,6 +341,7 @@
}
}
+ // Set local constraints.
async_handler_ = std::make_shared<AsyncHandler>();
{
@@ -342,6 +399,7 @@
}
zx_status_t VulkanImageCreator::GetImageInfo(uint32_t width, uint32_t height, zx::vmo* vmo_out,
+ zx::eventpair* token_out,
magma_image_info_t* image_info_out) {
auto result = collection_->WaitForBuffersAllocated_Sync();
@@ -354,6 +412,8 @@
return unbind_info->status;
}
+ collection_->Close();
+
// Run the loop to ensure local unbind completes and async_handler_ is freed
collection_.Unbind();
async_handler_->loop().RunUntilIdle();
@@ -419,6 +479,7 @@
}
*vmo_out = std::move(collection_info.buffers[0].vmo);
+ *token_out = std::move(scenic_import_token_.value);
return ZX_OK;
}
@@ -489,7 +550,8 @@
magma_status_t CreateDrmImage(uint32_t physical_device_index,
const magma_image_create_info_t* create_info,
- magma_image_info_t* image_info_out, zx::vmo* vmo_out) {
+ magma_image_info_t* image_info_out, zx::vmo* vmo_out,
+ zx::eventpair* token_out) {
assert(create_info);
assert(image_info_out);
assert(vmo_out);
@@ -557,6 +619,14 @@
}
}
+ if (create_info->flags & MAGMA_IMAGE_CREATE_FLAGS_PRESENTABLE) {
+ zx_status_t status = image_creator.InitScenic();
+ if (status != ZX_OK) {
+ LOG_VERBOSE("Failed to initialize scenic: %d", status);
+ return MAGMA_STATUS_INTERNAL_ERROR;
+ }
+ }
+
zx_status_t status = image_creator.InitSysmem();
if (status != ZX_OK) {
LOG_VERBOSE("Failed to initialize sysmem: %d", status);
@@ -585,8 +655,8 @@
return MagmaStatus(result);
}
- status =
- image_creator.GetImageInfo(create_info->width, create_info->height, vmo_out, image_info_out);
+ status = image_creator.GetImageInfo(create_info->width, create_info->height, vmo_out, token_out,
+ image_info_out);
if (status != ZX_OK) {
LOG_VERBOSE("GetImageInfo failed: %d", status);
if (status == ZX_ERR_NOT_SUPPORTED)
diff --git a/src/virtualization/bin/vmm/device/magma_image.h b/src/virtualization/bin/vmm/device/magma_image.h
index 636cb6d..faefafb 100644
--- a/src/virtualization/bin/vmm/device/magma_image.h
+++ b/src/virtualization/bin/vmm/device/magma_image.h
@@ -5,6 +5,7 @@
#ifndef SRC_VIRTUALIZATION_BIN_VMM_DEVICE_MAGMA_IMAGE_H_
#define SRC_VIRTUALIZATION_BIN_VMM_DEVICE_MAGMA_IMAGE_H_
+#include <lib/zx/eventpair.h>
#include <lib/zx/vmo.h>
#include <cstdint>
@@ -14,14 +15,12 @@
namespace magma_image {
// Creates a single buffer buffer collection for the given DRM format, and optional
-// DRM format modifiers; returns the VMO and the image parameters, including the
-// negotiated format modifier.
-// TODO(fxbug.dev/71878) - if create_info flags specifies MAGMA_IMAGE_CREATE_FLAGS_PRESENTABLE,
-// the buffer collection should be registered with scenic, and a token returned to
-// the caller.
+// DRM format modifiers; returns the VMO, scenic import token, and the image parameters,
+// including the negotiated format modifier.
magma_status_t CreateDrmImage(uint32_t physical_device_index,
const magma_image_create_info_t* create_info,
- magma_image_info_t* image_info_out, zx::vmo* vmo_out);
+ magma_image_info_t* image_info_out, zx::vmo* vmo_out,
+ zx::eventpair* token_out);
} // namespace magma_image
diff --git a/src/virtualization/bin/vmm/device/magma_image_test.cc b/src/virtualization/bin/vmm/device/magma_image_test.cc
index 65f052a..0c58513 100644
--- a/src/virtualization/bin/vmm/device/magma_image_test.cc
+++ b/src/virtualization/bin/vmm/device/magma_image_test.cc
@@ -17,6 +17,7 @@
TEST_F(MagmaImageTesting, SpecifyLinear) {
uint32_t physical_device_index = 0;
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -27,17 +28,19 @@
};
magma_image_info_t image_info = {};
- ASSERT_EQ(MAGMA_STATUS_OK,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ ASSERT_EQ(MAGMA_STATUS_OK, magma_image::CreateDrmImage(physical_device_index, &create_info,
+ &image_info, &buffer, &token));
EXPECT_EQ(DRM_FORMAT_MOD_LINEAR, image_info.drm_format_modifier);
EXPECT_EQ(kWidth * 4u, image_info.plane_strides[0]);
EXPECT_EQ(0u, image_info.plane_offsets[0]);
+ EXPECT_FALSE(token);
}
TEST_F(MagmaImageTesting, SpecifyIntelX) {
uint32_t physical_device_index = 0;
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -48,17 +51,19 @@
};
magma_image_info_t image_info = {};
- ASSERT_EQ(MAGMA_STATUS_OK,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ ASSERT_EQ(MAGMA_STATUS_OK, magma_image::CreateDrmImage(physical_device_index, &create_info,
+ &image_info, &buffer, &token));
EXPECT_EQ(I915_FORMAT_MOD_X_TILED, image_info.drm_format_modifier);
EXPECT_EQ(0u, image_info.plane_strides[0]);
EXPECT_EQ(0u, image_info.plane_offsets[0]);
+ EXPECT_FALSE(token);
}
TEST_F(MagmaImageTesting, SpecifyIntelY) {
uint32_t physical_device_index = 0;
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -69,17 +74,19 @@
};
magma_image_info_t image_info = {};
- ASSERT_EQ(MAGMA_STATUS_OK,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ ASSERT_EQ(MAGMA_STATUS_OK, magma_image::CreateDrmImage(physical_device_index, &create_info,
+ &image_info, &buffer, &token));
EXPECT_EQ(I915_FORMAT_MOD_Y_TILED, image_info.drm_format_modifier);
EXPECT_EQ(0u, image_info.plane_strides[0]);
EXPECT_EQ(0u, image_info.plane_offsets[0]);
+ EXPECT_FALSE(token);
}
TEST_F(MagmaImageTesting, SpecifyIntelYf) {
uint32_t physical_device_index = 0;
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -91,12 +98,15 @@
magma_image_info_t image_info = {};
ASSERT_EQ(MAGMA_STATUS_INVALID_ARGS,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer,
+ &token));
+ EXPECT_FALSE(token);
}
TEST_F(MagmaImageTesting, IntelMany) {
uint32_t physical_device_index = 0;
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -109,18 +119,20 @@
};
magma_image_info_t image_info = {};
- ASSERT_EQ(MAGMA_STATUS_OK,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ ASSERT_EQ(MAGMA_STATUS_OK, magma_image::CreateDrmImage(physical_device_index, &create_info,
+ &image_info, &buffer, &token));
EXPECT_EQ(I915_FORMAT_MOD_Y_TILED, image_info.drm_format_modifier);
EXPECT_EQ(0u, image_info.plane_strides[0]);
EXPECT_EQ(0u, image_info.plane_offsets[0]);
+ EXPECT_FALSE(token);
}
TEST_F(MagmaImageTesting, IntelNone) {
uint32_t physical_device_index = 0;
magma_image_info_t image_info = {};
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -130,18 +142,20 @@
.flags = 0,
};
- ASSERT_EQ(MAGMA_STATUS_OK,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ ASSERT_EQ(MAGMA_STATUS_OK, magma_image::CreateDrmImage(physical_device_index, &create_info,
+ &image_info, &buffer, &token));
EXPECT_EQ(I915_FORMAT_MOD_Y_TILED, image_info.drm_format_modifier);
EXPECT_EQ(0u, image_info.plane_strides[0]);
EXPECT_EQ(0u, image_info.plane_offsets[0]);
+ EXPECT_FALSE(token);
}
TEST_F(MagmaImageTesting, IntelNonePresentable) {
uint32_t physical_device_index = 0;
magma_image_info_t image_info = {};
zx::vmo buffer;
+ zx::eventpair token;
magma_image_create_info_t create_info = {
.drm_format = kFormat,
@@ -151,11 +165,12 @@
.flags = MAGMA_IMAGE_CREATE_FLAGS_PRESENTABLE,
};
- ASSERT_EQ(MAGMA_STATUS_OK,
- magma_image::CreateDrmImage(physical_device_index, &create_info, &image_info, &buffer));
+ ASSERT_EQ(MAGMA_STATUS_OK, magma_image::CreateDrmImage(physical_device_index, &create_info,
+ &image_info, &buffer, &token));
if (image_info.drm_format_modifier != I915_FORMAT_MOD_Y_TILED)
EXPECT_EQ(image_info.drm_format_modifier, I915_FORMAT_MOD_X_TILED);
EXPECT_EQ(0u, image_info.plane_strides[0]);
EXPECT_EQ(0u, image_info.plane_offsets[0]);
+ EXPECT_TRUE(token);
}
diff --git a/src/virtualization/bin/vmm/device/virtio_magma.cc b/src/virtualization/bin/vmm/device/virtio_magma.cc
index b9d74ee..1aca734 100644
--- a/src/virtualization/bin/vmm/device/virtio_magma.cc
+++ b/src/virtualization/bin/vmm/device/virtio_magma.cc
@@ -32,6 +32,31 @@
#define LOG_VERBOSE(msg, ...)
#endif
+static zx_status_t InitFromImageInfo(zx::vmo vmo, ImageInfoWithToken& info,
+ fuchsia::virtualization::hardware::VirtioImage* image) {
+ if (info.token) {
+ zx_status_t status = info.token.duplicate(ZX_RIGHT_SAME_RIGHTS, &image->token);
+ if (status != ZX_OK)
+ return status;
+ }
+
+ image->vmo = std::move(vmo);
+
+ image->info.resize(sizeof(magma_image_info_t));
+ memcpy(image->info.data(), &info.info, sizeof(magma_image_info_t));
+
+ return ZX_OK;
+}
+
+static void InitFromVirtioImage(fuchsia::virtualization::hardware::VirtioImage& image, zx::vmo* vmo,
+ ImageInfoWithToken* info) {
+ *vmo = std::move(image.vmo);
+ info->token = std::move(image.token);
+
+ assert(image.info.size() >= sizeof(magma_image_info_t));
+ memcpy(&info->info, image.info.data(), sizeof(magma_image_info_t));
+}
+
VirtioMagma::VirtioMagma(sys::ComponentContext* context) : DeviceBase(context) {}
void VirtioMagma::Start(
@@ -259,22 +284,26 @@
return ZX_OK;
}
- magma_image_info_t& info = iter->second;
+ ImageInfoWithToken& info = iter->second;
- image.info.resize(sizeof(magma_image_info_t));
+ // Get the VMO handle for this buffer.
+ zx_status_t status = VirtioMagmaGeneric::Handle_export(request, response);
+ if (status != ZX_OK) {
+ LOG_VERBOSE("VirtioMagmaGeneric::Handle_export failed: %d", status);
+ return status;
+ }
- memcpy(image.info.data(), &info, sizeof(info));
+ // Take ownership of the VMO handle.
+ zx::vmo vmo(static_cast<zx_handle_t>(response->buffer_handle_out));
+ response->buffer_handle_out = 0;
+
+ status = InitFromImageInfo(std::move(vmo), info, &image);
+ if (status != ZX_OK) {
+ LOG_VERBOSE("InitFromImageInfo failed: %d", status);
+ return status;
+ }
}
- // Get the VMO handle for this buffer.
- zx_status_t status = VirtioMagmaGeneric::Handle_export(request, response);
- if (status != ZX_OK)
- return status;
-
- // Take ownership of the VMO handle.
- image.vmo = zx::vmo(static_cast<zx_handle_t>(response->buffer_handle_out));
- response->buffer_handle_out = 0;
-
// TODO(fxbug.dev/13261): improvement backlog
// Perform a blocking import of the image, then return the VFD ID in the response.
// Note that since the virtio-magma device is fully synchronous anyway, this does
@@ -282,9 +311,11 @@
// return it only when the Import call returns, processing messages from other
// instances, or even other connections, in the meantime.
uint32_t vfd_id;
- status = wayland_importer_->ImportImage(std::move(image), &vfd_id);
- if (status != ZX_OK)
+ zx_status_t status = wayland_importer_->ImportImage(std::move(image), &vfd_id);
+ if (status != ZX_OK) {
+ LOG_VERBOSE("ImportImage failed: %d", status);
return status;
+ }
response->buffer_handle_out = vfd_id;
@@ -317,9 +348,14 @@
return result;
}
+ ImageInfoWithToken info;
+ zx::vmo vmo;
+
+ InitFromVirtioImage(*image.get(), &vmo, &info);
+
{
virtio_magma_import_ctrl_t request_copy = *request;
- request_copy.buffer_handle = image->vmo.release();
+ request_copy.buffer_handle = vmo.release();
status = VirtioMagmaGeneric::Handle_import(&request_copy, response);
if (status != ZX_OK)
@@ -330,13 +366,8 @@
auto& image_map =
connection_image_map_[reinterpret_cast<magma_connection_t>(request->connection)];
- magma_image_info_t info;
- assert(image->info.size() >= sizeof(info));
-
- memcpy(&info, image->info.data(), sizeof(info));
-
magma_buffer_t image = response->buffer_out;
- image_map[image] = info;
+ image_map[image] = std::move(info);
}
return ZX_OK;
@@ -369,14 +400,14 @@
memcpy(&image_create_info, reinterpret_cast<const magma_image_create_info_t*>(request + 1),
sizeof(image_create_info));
- magma_image_info_t image_info;
+ ImageInfoWithToken image_info;
zx::vmo vmo;
response->hdr.type = VIRTIO_MAGMA_RESP_VIRT_CREATE_IMAGE;
// Assuming the current connection is on the one and only physical device.
uint32_t physical_device_index = 0;
- response->result_return =
- magma_image::CreateDrmImage(physical_device_index, &image_create_info, &image_info, &vmo);
+ response->result_return = magma_image::CreateDrmImage(physical_device_index, &image_create_info,
+ &image_info.info, &vmo, &image_info.token);
if (response->result_return != MAGMA_STATUS_OK)
return ZX_OK;
@@ -396,7 +427,7 @@
auto& map = connection_image_map_[connection];
- map[response->image_out] = image_info;
+ map[response->image_out] = std::move(image_info);
return ZX_OK;
}
@@ -416,9 +447,11 @@
return ZX_OK;
}
+ ImageInfoWithToken& image_info = iter->second;
+
auto image_info_out = reinterpret_cast<magma_image_info_t*>(
const_cast<virtio_magma_virt_get_image_info_ctrl_t*>(request) + 1);
- *image_info_out = iter->second;
+ *image_info_out = image_info.info;
response->result_return = MAGMA_STATUS_OK;
return ZX_OK;
diff --git a/src/virtualization/bin/vmm/device/virtio_magma.h b/src/virtualization/bin/vmm/device/virtio_magma.h
index 245952b..cfa8832 100644
--- a/src/virtualization/bin/vmm/device/virtio_magma.h
+++ b/src/virtualization/bin/vmm/device/virtio_magma.h
@@ -21,6 +21,11 @@
#include "src/virtualization/bin/vmm/device/virtio_magma_generic.h"
#include "src/virtualization/bin/vmm/device/virtio_queue.h"
+struct ImageInfoWithToken {
+ magma_image_info_t info;
+ zx::eventpair token;
+};
+
class VirtioMagma : public VirtioMagmaGeneric,
public DeviceBase<VirtioMagma>,
public fuchsia::virtualization::hardware::VirtioMagma {
@@ -75,7 +80,7 @@
std::unordered_multimap<zx_koid_t, std::pair<zx_vaddr_t, size_t>> buffer_maps_;
// Each connection maps images to info, populated either when image is created or imported
- using ImageMap = std::unordered_map<magma_buffer_t, magma_image_info_t>;
+ using ImageMap = std::unordered_map<magma_buffer_t, ImageInfoWithToken>;
std::unordered_map<magma_connection_t, ImageMap> connection_image_map_;
FXL_DISALLOW_COPY_AND_ASSIGN(VirtioMagma);
diff --git a/src/virtualization/bin/vmm/device/virtio_magma_test.cc b/src/virtualization/bin/vmm/device/virtio_magma_test.cc
index 1705c1e..6304357 100644
--- a/src/virtualization/bin/vmm/device/virtio_magma_test.cc
+++ b/src/virtualization/bin/vmm/device/virtio_magma_test.cc
@@ -5,8 +5,10 @@
#include "src/graphics/lib/magma/include/virtio/virtio_magma.h"
#include <drm_fourcc.h>
+#include <fuchsia/scenic/allocation/cpp/fidl.h>
#include <fuchsia/virtualization/cpp/fidl.h>
#include <fuchsia/virtualization/hardware/cpp/fidl.h>
+#include <lib/sys/cpp/component_context.h>
#include <lib/zx/socket.h>
#include <string.h>
@@ -57,12 +59,70 @@
std::unique_ptr<VirtioImage> image_;
};
+class ScenicAllocatorFake : public fuchsia::scenic::allocation::Allocator {
+ public:
+ // Must set constraints on the given buffer collection token to allow the constraints
+ // negotiation to complete.
+ void RegisterBufferCollection(
+ fuchsia::scenic::allocation::BufferCollectionExportToken export_token,
+ fidl::InterfaceHandle<fuchsia::sysmem::BufferCollectionToken> scenic_token,
+ RegisterBufferCollectionCallback callback) override {
+ fuchsia::sysmem::AllocatorSyncPtr sysmem_allocator;
+ auto context = sys::ComponentContext::Create();
+ context->svc()->Connect(sysmem_allocator.NewRequest());
+
+ fuchsia::sysmem::BufferCollectionSyncPtr buffer_collection;
+ zx_status_t status = sysmem_allocator->BindSharedCollection(std::move(scenic_token),
+ buffer_collection.NewRequest());
+ if (status != ZX_OK) {
+ FX_LOGS(ERROR) << "BindSharedCollection failed: " << status;
+ callback(
+ fit::error(fuchsia::scenic::allocation::RegisterBufferCollectionError::BAD_OPERATION));
+ return;
+ }
+
+ fuchsia::sysmem::BufferCollectionConstraints constraints;
+ constraints.min_buffer_count = 1;
+ constraints.usage.cpu =
+ fuchsia::sysmem::cpuUsageReadOften | fuchsia::sysmem::cpuUsageWriteOften;
+ constraints.has_buffer_memory_constraints = true;
+ constraints.image_format_constraints_count = 1;
+ fuchsia::sysmem::ImageFormatConstraints& image_constraints =
+ constraints.image_format_constraints[0];
+ image_constraints = fuchsia::sysmem::ImageFormatConstraints();
+ image_constraints.min_coded_width = 0;
+ image_constraints.min_coded_height = 0;
+ image_constraints.max_coded_width = 0;
+ image_constraints.max_coded_height = 0;
+ image_constraints.min_bytes_per_row = 0;
+ image_constraints.color_spaces_count = 1;
+ image_constraints.color_space[0].type = fuchsia::sysmem::ColorSpaceType::SRGB;
+ image_constraints.pixel_format.type = fuchsia::sysmem::PixelFormatType::BGRA32;
+ image_constraints.pixel_format.has_format_modifier = false;
+
+ status = buffer_collection->SetConstraints(true, constraints);
+ if (status != ZX_OK) {
+ FX_LOGS(ERROR) << "SetConstraints failed: " << status;
+ callback(
+ fit::error(fuchsia::scenic::allocation::RegisterBufferCollectionError::BAD_OPERATION));
+ return;
+ }
+
+ buffer_collection->Close();
+
+ callback(fit::ok());
+ }
+};
+
+//////////////////////////////////////////////////////////////////////////////////////////////////
+
class VirtioMagmaTest : public TestWithDevice {
public:
VirtioMagmaTest()
: out_queue_(phys_mem_, kDescriptorSize, kQueueSize),
wayland_importer_mock_binding_(&wayland_importer_mock_),
- wayland_importer_mock_loop_(&kAsyncLoopConfigNoAttachToCurrentThread) {}
+ wayland_importer_mock_loop_(&kAsyncLoopConfigNoAttachToCurrentThread),
+ scenic_allocator_loop_(&kAsyncLoopConfigNoAttachToCurrentThread) {}
void SetUp() override {
uintptr_t vmar_addr;
@@ -76,12 +136,18 @@
std::unique_ptr<sys::testing::EnvironmentServices> env_services = CreateServices();
env_services->AllowParentService("fuchsia.vulkan.loader.Loader");
env_services->AllowParentService("fuchsia.sysmem.Allocator");
+
+ ASSERT_EQ(ZX_OK, env_services->AddService(scenic_allocator_binding_set_.GetHandler(
+ &scenic_allocator_fake_, scenic_allocator_loop_.dispatcher())));
+
ASSERT_EQ(ZX_OK, LaunchDevice(kVirtioMagmaUrl, out_queue_.end(), &start_info,
std::move(env_services)));
}
// Start device execution.
ASSERT_EQ(wayland_importer_mock_loop_.StartThread(), ZX_OK);
+ ASSERT_EQ(scenic_allocator_loop_.StartThread(), ZX_OK);
+
services_->Connect(magma_.NewRequest());
{
@@ -263,6 +329,8 @@
.drm_format_modifiers = {DRM_FORMAT_MOD_INVALID},
.width = 1920,
.height = 1080,
+ // Presentable causes VirtioMagma to register buffer collection with scenic
+ .flags = MAGMA_IMAGE_CREATE_FLAGS_PRESENTABLE,
};
std::vector<uint8_t> request_buffer(sizeof(request) + sizeof(create_image));
@@ -299,6 +367,9 @@
fidl::Binding<fuchsia::virtualization::hardware::VirtioWaylandImporter>
wayland_importer_mock_binding_;
async::Loop wayland_importer_mock_loop_;
+ ScenicAllocatorFake scenic_allocator_fake_;
+ async::Loop scenic_allocator_loop_;
+ fidl::BindingSet<fuchsia::scenic::allocation::Allocator> scenic_allocator_binding_set_;
};
TEST_F(VirtioMagmaTest, HandleQuery) {
diff --git a/src/virtualization/bin/vmm/device/virtio_wl.cc b/src/virtualization/bin/vmm/device/virtio_wl.cc
index 0ad8ae7..b4489b1 100644
--- a/src/virtualization/bin/vmm/device/virtio_wl.cc
+++ b/src/virtualization/bin/vmm/device/virtio_wl.cc
@@ -106,7 +106,8 @@
nullptr);
}
- // Create a memory instance with a VirtioImage.
+ // Create a memory instance with a VirtioImage. The image VMO was moved out and passed as
+ // the first parameter here. The image may or may not have an eventpair token.
static std::unique_ptr<Memory> CreateWithImage(zx::vmo vmo,
std::unique_ptr<VirtioWl::VirtioImage> image,
zx::vmar* vmar, uint32_t map_flags) {
@@ -119,6 +120,13 @@
FX_LOGS(ERROR) << "Failed to map VMO into guest VMAR: " << status;
return nullptr;
}
+ if (image->token) {
+ // If we have an eventpair token, use it as the VFD primary handle.
+ // Don't drop the VMO, move it back into the image.
+ image->vmo = std::move(vmo);
+ return std::make_unique<Memory>(zx::handle(image->token.release()), addr, size, vmar,
+ std::move(image));
+ }
return std::make_unique<Memory>(zx::handle(vmo.release()), addr, size, vmar, std::move(image));
}
@@ -130,7 +138,7 @@
uintptr_t addr() const { return addr_; }
uint64_t size() const { return size_; }
- VirtioWl::VirtioImage* GetImage() override { return image_.get(); }
+ std::unique_ptr<VirtioWl::VirtioImage> DuplicateImage() override;
private:
zx::handle handle_;
@@ -140,6 +148,37 @@
std::unique_ptr<VirtioWl::VirtioImage> image_;
};
+std::unique_ptr<VirtioWl::VirtioImage> Memory::DuplicateImage() {
+ if (!image_)
+ return nullptr;
+
+ zx::handle handle;
+ zx_status_t status = Duplicate(&handle);
+ if (status != ZX_OK) {
+ FX_LOGS(ERROR) << "VFD duplicate failed: " << status;
+ return nullptr;
+ }
+
+ auto image = std::make_unique<VirtioWl::VirtioImage>();
+
+ // If image VMO is valid, an eventpair token was moved into the primary handle.
+ if (image_->vmo) {
+ image->token = zx::eventpair(handle.release());
+
+ status = image_->vmo.duplicate(ZX_RIGHT_SAME_RIGHTS, &image->vmo);
+ if (status != ZX_OK) {
+ FX_LOGS(ERROR) << "VMO duplicate failed: " << status;
+ return nullptr;
+ }
+ } else {
+ // VMO was moved into the primary handle, there is no eventpair token.
+ image->vmo = zx::vmo(handle.release());
+ }
+
+ image->info = image_->info;
+ return image;
+}
+
// Vfd type that holds a wayland dispatcher connection.
class Connection : public VirtioWl::Vfd {
public:
@@ -287,6 +326,7 @@
TRACE_DURATION("machina", "VirtioWl::ImportImage");
auto deferred = fit::defer(
[&]() { callback(fuchsia::virtualization::hardware::VIRTIO_WAYLAND_INVALID_VFD_ID); });
+
zx_info_handle_basic_t handle_basic_info{};
zx_status_t status = image.vmo.get_info(ZX_INFO_HANDLE_BASIC, &handle_basic_info,
sizeof(handle_basic_info), nullptr, nullptr);
@@ -294,17 +334,21 @@
FX_LOGS(ERROR) << "Image VMO failed handle type check";
return;
}
+ if (image.token) {
+ zx_info_handle_basic_t token_info{};
+ status = image.token.get_info(ZX_INFO_HANDLE_BASIC, &token_info, sizeof(token_info), nullptr,
+ nullptr);
+ if (status != ZX_OK || token_info.type != ZX_OBJ_TYPE_EVENTPAIR) {
+ FX_LOGS(ERROR) << "Image eventpair failed handle type check";
+ return;
+ }
+ }
uint32_t vfd_id = next_vfd_id_++;
PendingVfd pending_vfd{};
- status =
- zx_handle_duplicate(image.vmo.get(), ZX_RIGHT_SAME_RIGHTS, &pending_vfd.handle_info.handle);
- if (status != ZX_OK) {
- FX_LOGS(ERROR) << "Failed to duplicate image VMO";
- return;
- }
-
+ // Move the VMO into the primary "pending VFD" handle.
+ pending_vfd.handle_info.handle = image.vmo.release();
pending_vfd.handle_info.type = handle_basic_info.type;
pending_vfd.handle_info.rights = handle_basic_info.rights;
pending_vfd.vfd_id = vfd_id;
@@ -328,17 +372,12 @@
std::unique_ptr<Vfd>& vfd = iter->second;
- auto image = std::make_unique<VirtioWl::VirtioImage>();
-
- zx_status_t status = vfd->GetImage()->vmo.duplicate(ZX_RIGHT_SAME_RIGHTS, &image->vmo);
- if (status != ZX_OK) {
- FX_LOGS(ERROR) << "Duplicate failed";
+ std::unique_ptr<VirtioWl::VirtioImage> image = vfd->DuplicateImage();
+ if (!image) {
callback(ZX_ERR_INTERNAL, {});
return;
}
- image->info = vfd->GetImage()->info;
-
callback(ZX_OK, std::move(image));
}
diff --git a/src/virtualization/bin/vmm/device/virtio_wl.h b/src/virtualization/bin/vmm/device/virtio_wl.h
index 4989db4..fd39cbb 100644
--- a/src/virtualization/bin/vmm/device/virtio_wl.h
+++ b/src/virtualization/bin/vmm/device/virtio_wl.h
@@ -80,8 +80,8 @@
// Returns ZX_ERR_NOT_SUPPORTED if duplication is not supported.
virtual zx_status_t Duplicate(zx::handle* handle) { return ZX_ERR_NOT_SUPPORTED; }
- // Returns the VirtioImage associated with this VFD.
- virtual VirtioWl::VirtioImage* GetImage() { return nullptr; }
+ // Creates a duplicate VirtioImage.
+ virtual std::unique_ptr<VirtioWl::VirtioImage> DuplicateImage() { return nullptr; }
};
explicit VirtioWl(sys::ComponentContext* context);
diff --git a/src/virtualization/bin/vmm/device/virtio_wl_unittest.cc b/src/virtualization/bin/vmm/device/virtio_wl_unittest.cc
index 8a1504c..4f5a5d0d 100644
--- a/src/virtualization/bin/vmm/device/virtio_wl_unittest.cc
+++ b/src/virtualization/bin/vmm/device/virtio_wl_unittest.cc
@@ -735,4 +735,48 @@
}
}
+TEST_F(VirtioWlTest, ImportExportImageWithToken) {
+ ASSERT_EQ(CreateConnection(1u), ZX_OK);
+ RunLoopUntilIdle();
+ ASSERT_EQ(channels_.size(), 1u);
+ fuchsia::virtualization::hardware::VirtioWaylandImporterSyncPtr importer;
+ ASSERT_EQ(wl_->GetImporter(importer.NewRequest()), ZX_OK);
+
+ virtio_wl_ctrl_vfd_new_t* new_vfd_cmd;
+ uint16_t descriptor_id;
+ ASSERT_EQ(DescriptorChainBuilder(in_queue_)
+ .AppendWritableDescriptor(&new_vfd_cmd, sizeof(virtio_wl_ctrl_vfd_new_t))
+ .Build(&descriptor_id),
+ ZX_OK);
+
+ uint32_t vfd_id = 0;
+ uint64_t koid = 0;
+ {
+ fuchsia::virtualization::hardware::VirtioImage image;
+ ASSERT_EQ(zx::vmo::create(kImportVmoSize, 0u, &image.vmo), ZX_OK);
+ zx_info_handle_basic_t info;
+ ASSERT_EQ(image.vmo.get_info(ZX_INFO_HANDLE_BASIC, &info, sizeof(info), nullptr, nullptr),
+ ZX_OK);
+ koid = info.koid;
+ zx::eventpair ep;
+ ASSERT_EQ(zx::eventpair::create(0, &ep, &image.token), ZX_OK);
+ ASSERT_EQ(importer->ImportImage(std::move(image), &vfd_id), ZX_OK);
+ ASSERT_NE(vfd_id, 0u);
+ }
+ {
+ std::unique_ptr<fuchsia::virtualization::hardware::VirtioImage> image;
+ zx_status_t result;
+ ASSERT_EQ(importer->ExportImage(vfd_id, &result, &image), ZX_OK);
+ ASSERT_TRUE(image);
+ EXPECT_EQ(result, ZX_OK);
+ zx_info_handle_basic_t info;
+ ASSERT_EQ(image->vmo.get_info(ZX_INFO_HANDLE_BASIC, &info, sizeof(info), nullptr, nullptr),
+ ZX_OK);
+ EXPECT_EQ(koid, info.koid);
+ ASSERT_EQ(image->token.get_info(ZX_INFO_HANDLE_BASIC, &info, sizeof(info), nullptr, nullptr),
+ ZX_OK);
+ EXPECT_EQ(info.type, ZX_OBJ_TYPE_EVENTPAIR);
+ }
+}
+
} // namespace
diff --git a/src/virtualization/bin/vmm/meta/device_tests.cmx b/src/virtualization/bin/vmm/meta/device_tests.cmx
index e42c305..ad8a62e 100644
--- a/src/virtualization/bin/vmm/meta/device_tests.cmx
+++ b/src/virtualization/bin/vmm/meta/device_tests.cmx
@@ -1,4 +1,11 @@
{
+ "facets": {
+ "fuchsia.test": {
+ "system-services": [
+ "fuchsia.scenic.allocation.Allocator"
+ ]
+ }
+ },
"include": [
"sdk/lib/diagnostics/syslog/client.shard.cmx",
"src/lib/vulkan/test-application.shard.cmx"
@@ -11,6 +18,7 @@
"isolated-temp"
],
"services": [
+ "fuchsia.scenic.allocation.Allocator",
"fuchsia.sys.Environment",
"fuchsia.sys.Loader"
]
diff --git a/src/virtualization/bin/vmm/meta/virtio_magma.cmx b/src/virtualization/bin/vmm/meta/virtio_magma.cmx
index 8749f72..5d5d8f9 100644
--- a/src/virtualization/bin/vmm/meta/virtio_magma.cmx
+++ b/src/virtualization/bin/vmm/meta/virtio_magma.cmx
@@ -11,6 +11,7 @@
"vulkan"
],
"services": [
+ "fuchsia.scenic.allocation.Allocator",
"fuchsia.tracing.provider.Registry"
]
}
