[devmgr] Pipelined open for device_add.
This allows a client to open a connection to the device
when adding a device, instead of having to wait for the device
appear in the filesystem and open it.
If the device is added with DEVICE_ADD_MUST_ISOLATE,
they will get connected to the proxy device instead.
If the device is added with DEVICE_ADD_INVISIBLE,
they will not be be connected until device_make_visible is called.
ZX-2956 #comment
TEST= I wrote a small device and proxy and verified the channel
worked.
Change-Id: I5e1eed1eedaa6be65d0f0053c4c68dee0ead3058
diff --git a/system/core/devmgr/devhost/api.cpp b/system/core/devmgr/devhost/api.cpp
index ef1717f..e1475a4 100644
--- a/system/core/devmgr/devhost/api.cpp
+++ b/system/core/devmgr/devhost/api.cpp
@@ -52,43 +52,59 @@
return ZX_ERR_INVALID_ARGS;
}
- ApiAutoLock lock;
- r = devhost_device_create(drv, parent_ref, args->name, args->ctx, args->ops, &dev);
- if (r != ZX_OK) {
- return r;
- }
- if (args->proto_id) {
- dev->protocol_id = args->proto_id;
- dev->protocol_ops = args->proto_ops;
- }
- if (args->flags & DEVICE_ADD_NON_BINDABLE) {
- dev->flags |= DEV_FLAG_UNBINDABLE;
- }
- if (args->flags & DEVICE_ADD_INVISIBLE) {
- dev->flags |= DEV_FLAG_INVISIBLE;
- }
+ // If the device will be added in the same devhost and visible,
+ // we can connect the client immediately after adding the device.
+ // Otherwise we will pass this channel to the devcoordinator via devhost_device_add.
+ zx::channel client_remote(args->client_remote);
- // out must be set before calling devhost_device_add().
- // devhost_device_add() may result in child devices being created before it returns,
- // and those children may call ops on the device before device_add() returns.
- // This leaked-ref will be accounted below.
- if (out) {
- *out = dev.get();
- }
-
- if (args->flags & DEVICE_ADD_MUST_ISOLATE) {
- r = devhost_device_add(dev, parent_ref, args->props, args->prop_count, args->proxy_args);
- } else if (args->flags & DEVICE_ADD_INSTANCE) {
- dev->flags |= DEV_FLAG_INSTANCE | DEV_FLAG_UNBINDABLE;
- r = devhost_device_add(dev, parent_ref, nullptr, 0, nullptr);
- } else {
- r = devhost_device_add(dev, parent_ref, args->props, args->prop_count, nullptr);
- }
- if (r != ZX_OK) {
- if (out) {
- *out = nullptr;
+ {
+ ApiAutoLock lock;
+ r = devhost_device_create(drv, parent_ref, args->name, args->ctx, args->ops, &dev);
+ if (r != ZX_OK) {
+ return r;
}
- dev.reset();
+ if (args->proto_id) {
+ dev->protocol_id = args->proto_id;
+ dev->protocol_ops = args->proto_ops;
+ }
+ if (args->flags & DEVICE_ADD_NON_BINDABLE) {
+ dev->flags |= DEV_FLAG_UNBINDABLE;
+ }
+ if (args->flags & DEVICE_ADD_INVISIBLE) {
+ dev->flags |= DEV_FLAG_INVISIBLE;
+ }
+
+ // out must be set before calling devhost_device_add().
+ // devhost_device_add() may result in child devices being created before it returns,
+ // and those children may call ops on the device before device_add() returns.
+ // This leaked-ref will be accounted below.
+ if (out) {
+ *out = dev.get();
+ }
+
+ if (args->flags & DEVICE_ADD_MUST_ISOLATE) {
+ r = devhost_device_add(dev, parent_ref, args->props, args->prop_count,
+ args->proxy_args, std::move(client_remote));
+ } else if (args->flags & DEVICE_ADD_INSTANCE) {
+ dev->flags |= DEV_FLAG_INSTANCE | DEV_FLAG_UNBINDABLE;
+ r = devhost_device_add(dev, parent_ref, nullptr, 0, nullptr,
+ zx::channel() /* client_remote */);
+ } else {
+ bool pass_client_remote = args->flags & DEVICE_ADD_INVISIBLE;
+ r = devhost_device_add(dev, parent_ref, args->props, args->prop_count, nullptr,
+ pass_client_remote ? std::move(client_remote) : zx::channel());
+ }
+ if (r != ZX_OK) {
+ if (out) {
+ *out = nullptr;
+ }
+ dev.reset();
+ }
+ }
+
+ // This needs to be called outside the ApiAutoLock, as device_open_at will be called.
+ if (dev && client_remote.is_valid()) {
+ devhost_device_connect(dev, std::move(client_remote));
}
// Leak the reference that was written to |out|, it will be recovered in
diff --git a/system/core/devmgr/devhost/core.cpp b/system/core/devmgr/devhost/core.cpp
index 4629a79..cf5188b 100644
--- a/system/core/devmgr/devhost/core.cpp
+++ b/system/core/devmgr/devhost/core.cpp
@@ -385,7 +385,7 @@
zx_status_t devhost_device_add(const fbl::RefPtr<zx_device_t>& dev,
const fbl::RefPtr<zx_device_t>& parent,
const zx_device_prop_t* props, uint32_t prop_count,
- const char* proxy_args)
+ const char* proxy_args, zx::channel client_remote)
REQ_DM_LOCK {
auto fail = [&dev](zx_status_t status) {
if (dev) {
@@ -460,7 +460,7 @@
if (!(dev->flags & DEV_FLAG_INSTANCE)) {
// devhost_add always consumes the handle
- status = devhost_add(parent, dev, proxy_args, props, prop_count);
+ status = devhost_add(parent, dev, proxy_args, props, prop_count, std::move(client_remote));
if (status < 0) {
printf("devhost: %p(%s): remote add failed %d\n",
dev.get(), dev->name, status);
diff --git a/system/core/devmgr/devhost/devhost.cpp b/system/core/devmgr/devhost/devhost.cpp
index 818405c..f7e44db 100644
--- a/system/core/devmgr/devhost/devhost.cpp
+++ b/system/core/devmgr/devhost/devhost.cpp
@@ -873,7 +873,8 @@
// parent device. Called under devhost api lock.
zx_status_t devhost_add(const fbl::RefPtr<zx_device_t>& parent,
const fbl::RefPtr<zx_device_t>& child, const char* proxy_args,
- const zx_device_prop_t* props, uint32_t prop_count) {
+ const zx_device_prop_t* props, uint32_t prop_count,
+ zx::channel client_remote) {
char buffer[512];
const char* path = mkdevpath(parent, buffer, sizeof(buffer));
log(RPC_OUT, "devhost[%s] add '%s'\n", path, child->name);
@@ -903,13 +904,13 @@
rpc.get(), hsend.release(), reinterpret_cast<const uint64_t*>(props), prop_count,
child->name, strlen(child->name), child->protocol_id,
child->driver->libname().data(), child->driver->libname().size(), proxy_args,
- proxy_args_len, &call_status);
+ proxy_args_len, client_remote.release(), &call_status);
} else {
status = fuchsia_device_manager_CoordinatorAddDevice(
rpc.get(), hsend.release(), reinterpret_cast<const uint64_t*>(props), prop_count,
child->name, strlen(child->name), child->protocol_id,
child->driver->libname().data(), child->driver->libname().size(), proxy_args,
- proxy_args_len, &call_status);
+ proxy_args_len, client_remote.release(), &call_status);
}
if (status != ZX_OK) {
log(ERROR, "devhost[%s] add '%s': rpc sending failed: %d\n", path, child->name, status);
diff --git a/system/core/devmgr/devhost/devhost.h b/system/core/devmgr/devhost/devhost.h
index 5b25b43..8731f3d 100644
--- a/system/core/devmgr/devhost/devhost.h
+++ b/system/core/devmgr/devhost/devhost.h
@@ -143,10 +143,12 @@
extern zx_protocol_device_t device_default_ops;
+// |client_remote| will only be a valid handle if the device was added with
+// DEVICE_ADD_INVISIBLE or DEVICE_ADD_MUST_ISOLATE.
zx_status_t devhost_device_add(const fbl::RefPtr<zx_device_t>& dev,
const fbl::RefPtr<zx_device_t>& parent,
const zx_device_prop_t* props, uint32_t prop_count,
- const char* proxy_args) REQ_DM_LOCK;
+ const char* proxy_args, zx::channel client_remote) REQ_DM_LOCK;
// Note that devhost_device_remove() takes a RefPtr rather than a const RefPtr&.
// It intends to consume a reference.
zx_status_t devhost_device_remove(fbl::RefPtr<zx_device_t> dev) REQ_DM_LOCK;
@@ -209,12 +211,18 @@
zx_status_t devhost_device_connect(const fbl::RefPtr<zx_device_t>& dev, uint32_t flags,
const char* path_data, size_t path_size, zx::channel c);
+// Attaches channel |c| to new state representing an open connection to |dev|.
+void devhost_device_connect(const fbl::RefPtr<zx_device_t>& dev, zx::channel c);
+
zx_status_t devhost_start_connection(fbl::unique_ptr<DevfsConnection> ios, zx::channel h);
// routines devhost uses to talk to dev coordinator
+// |client_remote| will only be a valid handle if the device was added with
+// DEVICE_ADD_INVISIBLE or DEVICE_ADD_MUST_ISOLATE.
zx_status_t devhost_add(const fbl::RefPtr<zx_device_t>& dev,
const fbl::RefPtr<zx_device_t>& child, const char* proxy_args,
- const zx_device_prop_t* props, uint32_t prop_count) REQ_DM_LOCK;
+ const zx_device_prop_t* props, uint32_t prop_count,
+ zx::channel client_remote) REQ_DM_LOCK;
zx_status_t devhost_remove(const fbl::RefPtr<zx_device_t>& dev) REQ_DM_LOCK;
void devhost_make_visible(const fbl::RefPtr<zx_device_t>& dev);
diff --git a/system/core/devmgr/devhost/rpc-server.cpp b/system/core/devmgr/devhost/rpc-server.cpp
index b075f22..bfab85a 100644
--- a/system/core/devmgr/devhost/rpc-server.cpp
+++ b/system/core/devmgr/devhost/rpc-server.cpp
@@ -310,6 +310,10 @@
return ZX_OK;
}
+void devhost_device_connect(const fbl::RefPtr<zx_device_t>& dev, zx::channel c) {
+ devhost_get_handles(std::move(c), dev, nullptr /* path */, 0 /* flags */);
+}
+
static zx_status_t fidl_directory_open(void* ctx, uint32_t flags, uint32_t mode,
const char* path_data, size_t path_size,
zx_handle_t object) {
diff --git a/system/core/devmgr/devmgr/coordinator.cpp b/system/core/devmgr/devmgr/coordinator.cpp
index 8280b48..ef29fce 100644
--- a/system/core/devmgr/devmgr/coordinator.cpp
+++ b/system/core/devmgr/devmgr/coordinator.cpp
@@ -661,7 +661,8 @@
uint32_t protocol_id,
fbl::StringPiece driver_path,
fbl::StringPiece args,
- bool invisible) {
+ bool invisible,
+ zx::channel client_remote) {
// If this is true, then |name_data|'s size is properly bounded.
static_assert(fuchsia_device_manager_DEVICE_NAME_MAX == ZX_DEVICE_NAME_MAX);
static_assert(fuchsia_device_manager_PROPERTIES_MAX <= UINT32_MAX);
@@ -751,6 +752,8 @@
parent->children.push_back(dev.get());
parent->AddRef();
+ dev->client_remote = std::move(client_remote);
+
devices_.push_back(dev.get());
log(DEVLC, "devcoord: dev %p name='%s' ++ref=%d (child)\n",
@@ -1090,16 +1093,18 @@
const char* name_data, size_t name_size,
uint32_t protocol_id,
const char* driver_path_data, size_t driver_path_size,
- const char* args_data, size_t args_size, fidl_txn_t* txn) {
+ const char* args_data, size_t args_size,
+ zx_handle_t raw_client_remote, fidl_txn_t* txn) {
auto parent = static_cast<Device*>(ctx);
zx::channel rpc(raw_rpc);
fbl::StringPiece name(name_data, name_size);
fbl::StringPiece driver_path(driver_path_data, driver_path_size);
fbl::StringPiece args(args_data, args_size);
+ zx::channel client_remote(raw_client_remote);
zx_status_t status = parent->coordinator->AddDevice(parent, std::move(rpc), props_data,
props_count, name, protocol_id, driver_path,
- args, false);
+ args, false, std::move(client_remote));
return fuchsia_device_manager_CoordinatorAddDevice_reply(txn, status);
}
@@ -1109,16 +1114,17 @@
uint32_t protocol_id,
const char* driver_path_data, size_t driver_path_size,
const char* args_data, size_t args_size,
- fidl_txn_t* txn) {
+ zx_handle_t raw_client_remote, fidl_txn_t* txn) {
auto parent = static_cast<Device*>(ctx);
zx::channel rpc(raw_rpc);
fbl::StringPiece name(name_data, name_size);
fbl::StringPiece driver_path(driver_path_data, driver_path_size);
fbl::StringPiece args(args_data, args_size);
+ zx::channel client_remote(raw_client_remote);
zx_status_t status = parent->coordinator->AddDevice(parent, std::move(rpc), props_data,
props_count, name, protocol_id, driver_path,
- args, true);
+ args, true, std::move(client_remote));
return fuchsia_device_manager_CoordinatorAddDeviceInvisible_reply(txn, status);
}
@@ -1674,6 +1680,11 @@
log(ERROR, "devcoord: dh_send_connect_proxy: %d\n", r);
}
}
+ if (dev->client_remote.is_valid()) {
+ if ((r = devfs_connect(dev->proxy, std::move(dev->client_remote))) != ZX_OK) {
+ log(ERROR, "devcoord: devfs_connnect: %d\n", r);
+ }
+ }
}
return ZX_OK;
@@ -1707,6 +1718,14 @@
}
void Coordinator::HandleNewDevice(Device* dev) {
+ // If the device has a proxy, we actually want to wait for the proxy device to be
+ // created and connect to that.
+ if (dev->client_remote.is_valid() && !(dev->flags & DEV_CTX_MUST_ISOLATE)) {
+ zx_status_t r;
+ if ((r = devfs_connect(dev, std::move(dev->client_remote))) != ZX_OK) {
+ log(ERROR, "devcoord: devfs_connnect: %d\n", r);
+ }
+ }
for (auto& drv : drivers_) {
if (dc_is_bindable(&drv, dev->protocol_id,
dev->props.get(), dev->prop_count, true)) {
diff --git a/system/core/devmgr/devmgr/coordinator.h b/system/core/devmgr/devmgr/coordinator.h
index 15e44de..1fe944d 100644
--- a/system/core/devmgr/devmgr/coordinator.h
+++ b/system/core/devmgr/devmgr/coordinator.h
@@ -114,6 +114,10 @@
Device* parent = nullptr;
Device* proxy = nullptr;
+ // For attaching as an open connection to the proxy device,
+ // or once the device becomes visible.
+ zx::channel client_remote;
+
// listnode for this device in its parent's
// list-of-children
fbl::DoublyLinkedListNodeState<Device*> node;
@@ -388,6 +392,7 @@
void devfs_unpublish(Device* dev);
void devfs_advertise(Device* dev);
void devfs_advertise_modified(Device* dev);
+zx_status_t devfs_connect(Device* dev, zx::channel client_remote);
// Values parsed out of argv. All paths described below are absolute paths.
struct DevmgrArgs {
@@ -435,7 +440,8 @@
zx_status_t AddDevice(Device* parent, zx::channel rpc, const uint64_t* props_data,
size_t props_count, fbl::StringPiece name, uint32_t protocol_id,
- fbl::StringPiece driver_path, fbl::StringPiece args, bool invisible);
+ fbl::StringPiece driver_path, fbl::StringPiece args, bool invisible,
+ zx::channel client_remote);
zx_status_t MakeVisible(Device* dev);
zx_status_t RemoveDevice(Device* dev, bool forced);
diff --git a/system/core/devmgr/devmgr/devfs.cpp b/system/core/devmgr/devmgr/devfs.cpp
index 9259bc7..26c03c4 100644
--- a/system/core/devmgr/devmgr/devfs.cpp
+++ b/system/core/devmgr/devmgr/devfs.cpp
@@ -681,6 +681,15 @@
}
}
+zx_status_t devfs_connect(Device* dev, zx::channel client_remote) {
+ if (!client_remote.is_valid()) {
+ return ZX_ERR_BAD_HANDLE;
+ }
+ fuchsia_io_DirectoryOpen(dev->hrpc.get(), 0 /* flags */, 0 /* mode */,
+ ".", 1, client_remote.release());
+ return ZX_OK;
+}
+
// Helper macros for |DevfsFidlHandler| which make it easier
// avoid typing generated names.
diff --git a/system/dev/display/vim-display/vim-display.cpp b/system/dev/display/vim-display/vim-display.cpp
index 8a36c6f..cd15907 100644
--- a/system/dev/display/vim-display/vim-display.cpp
+++ b/system/dev/display/vim-display/vim-display.cpp
@@ -803,6 +803,7 @@
.proto_ops = &display_controller_ops,
.proxy_args = nullptr,
.flags = 0,
+ .client_remote = ZX_HANDLE_INVALID,
};
status = device_add(display->parent, &add_args, &display->mydevice);
diff --git a/system/fidl/fuchsia-device-manager/coordinator.fidl b/system/fidl/fuchsia-device-manager/coordinator.fidl
index 206c51a..227b0a6 100644
--- a/system/fidl/fuchsia-device-manager/coordinator.fidl
+++ b/system/fidl/fuchsia-device-manager/coordinator.fidl
@@ -86,11 +86,13 @@
/// For binding purposes, it is has properties |props|. |name| and |driver_path|
/// are informational and used for debugging. The device will have |protocol_id|
/// as its primary protocol id. |args| should only be used for shadowed devices,
- /// and will be forwarded to the shadow device.
+ /// and will be forwarded to the shadow device. |client_remote|, if present,
+ /// will be passed to the device as an open connection for the client.
0x10000011: AddDevice(handle<channel> rpc,
vector<DeviceProperty>:PROPERTIES_MAX props,
string:DEVICE_NAME_MAX name, uint32 protocol_id,
- string:PATH_MAX? driver_path, string:DEVICE_ARGS_MAX? args)
+ string:PATH_MAX? driver_path, string:DEVICE_ARGS_MAX? args,
+ handle<channel>? client_remote)
-> (zx.status status);
/// Behaves as AddDevice, but marks the device as initially invisible. This means
@@ -100,7 +102,8 @@
vector<DeviceProperty>:PROPERTIES_MAX props,
string:DEVICE_NAME_MAX name, uint32 protocol_id,
string:PATH_MAX? driver_path,
- string:DEVICE_ARGS_MAX? args) -> (zx.status status);
+ string:DEVICE_ARGS_MAX? args, handle<channel>? client_remote)
+ -> (zx.status status);
/// Record the removal of this device.
0x10000013: RemoveDevice() -> (zx.status status);
diff --git a/system/ulib/ddk/include/ddk/driver.h b/system/ulib/ddk/include/ddk/driver.h
index b8daf80..81e9cb1 100644
--- a/system/ulib/ddk/include/ddk/driver.h
+++ b/system/ulib/ddk/include/ddk/driver.h
@@ -100,6 +100,12 @@
// One or more of DEVICE_ADD_*
uint32_t flags;
+
+ // Optional channel passed to the |dev| that serves as an open connection for the client.
+ // If DEVICE_ADD_MUST_ISOLATE is set, the client will be connected to the proxy instead.
+ // If DEVICE_ADD_INVISIBLE is set, the client will not be connected until
+ // device_make_visible is called.
+ zx_handle_t client_remote;
} device_add_args_t;
struct zx_driver_rec {
diff --git a/system/ulib/ddktl/include/ddktl/device.h b/system/ulib/ddktl/include/ddktl/device.h
index 8b3a9af..6f2713b 100644
--- a/system/ulib/ddktl/include/ddktl/device.h
+++ b/system/ulib/ddktl/include/ddktl/device.h
@@ -323,7 +323,8 @@
public:
zx_status_t DdkAdd(const char* name, uint32_t flags = 0, zx_device_prop_t* props = nullptr,
uint32_t prop_count = 0, uint32_t proto_id = 0,
- const char* proxy_args = nullptr) {
+ const char* proxy_args = nullptr,
+ zx_handle_t client_remote = ZX_HANDLE_INVALID) {
if (zxdev_ != nullptr) {
return ZX_ERR_BAD_STATE;
}
@@ -340,6 +341,7 @@
args.prop_count = prop_count;
args.proto_id = proto_id;
args.proxy_args = proxy_args;
+ args.client_remote = client_remote;
AddProtocol(&args);
return device_add(parent_, &args, &zxdev_);
