diff --git a/zircon/system/core/devmgr/devcoordinator/composite-device.cpp b/zircon/system/core/devmgr/devcoordinator/composite-device.cpp index 32198221fb253e400321c30de8de21f920c674da..a9aad304b2fedaaa67b7a19ea30abb7eff8a356d 100644 --- a/zircon/system/core/devmgr/devcoordinator/composite-device.cpp +++ b/zircon/system/core/devmgr/devcoordinator/composite-device.cpp @@ -66,20 +66,20 @@ bool CompositeDevice::TryMatchComponents(const fbl::RefPtr<Device>& dev, size_t* if (itr->TryMatch(dev)) { log(ERROR, "devcoordinator: ambiguous composite bind! composite='%s', dev1='%s', dev2='%s'\n", - name_.data(), itr->bound_device()->name.data(), dev->name.data()); + name_.data(), itr->bound_device()->name().data(), dev->name().data()); return false; } } for (auto itr = unbound_.begin(); itr != unbound_.end(); ++itr) { if (itr->TryMatch(dev)) { log(SPEW, "devcoordinator: found match for composite='%s', dev='%s'\n", - name_.data(), dev->name.data()); + name_.data(), dev->name().data()); *index_out = itr->index(); return true; } } log(SPEW, "devcoordinator: no match for composite='%s', dev='%s'\n", - name_.data(), dev->name.data()); + name_.data(), dev->name().data()); return false; } @@ -140,9 +140,9 @@ zx_status_t CompositeDevice::TryAssemble() { // We need to create it. Double check that we haven't ended up in a state // where the proxies would need to be in different processes. - if (devhost != nullptr && component_dev->proxy != nullptr && - component_dev->proxy->host() != nullptr && - component_dev->proxy->host() != devhost) { + if (devhost != nullptr && component_dev->proxy() != nullptr && + component_dev->proxy()->host() != nullptr && + component_dev->proxy()->host() != devhost) { log(ERROR, "devcoordinator: cannot create composite, proxies in different processes\n"); return ZX_ERR_BAD_STATE; } @@ -153,11 +153,11 @@ zx_status_t CompositeDevice::TryAssemble() { } // If we hadn't picked a devhost, use the one that was created just now. if (devhost == nullptr) { - devhost = component_dev->proxy->host(); + devhost = component_dev->proxy()->host(); ZX_ASSERT(devhost != nullptr); } // Stash the local ID after the proxy has been created - component_local_ids[component.index()] = component_dev->proxy->local_id(); + component_local_ids[component.index()] = component_dev->proxy()->local_id(); } zx::channel rpc_local, rpc_remote; diff --git a/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp b/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp index f10e114447298e4179313103dc057874fd4844c4..2a29e6ea3c096272dd89884c0d4d312b4da1e3ec 100644 --- a/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp +++ b/zircon/system/core/devmgr/devcoordinator/coordinator-test.cpp @@ -429,7 +429,7 @@ protected: zx::channel local; zx_status_t status = zx::channel::create(0, &local, &platform_bus_.remote); ASSERT_EQ(ZX_OK, status); - status = coordinator_.AddDevice(coordinator_.sys_device()->proxy, std::move(local), + status = coordinator_.AddDevice(coordinator_.sys_device()->proxy(), std::move(local), nullptr /* props_data */, 0 /* props_count */, "platform-bus", 0, nullptr /* driver_path */, nullptr /* args */, false /* invisible */, diff --git a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp index 652e8a0f80cf1a96afa56d0ac61581e40a223647..ef4b167b6591524f8c2701e808708a36b3a10ced 100644 --- a/zircon/system/core/devmgr/devcoordinator/coordinator.cpp +++ b/zircon/system/core/devmgr/devcoordinator/coordinator.cpp @@ -113,63 +113,22 @@ bool Coordinator::InSuspend() const { } zx_status_t Coordinator::InitializeCoreDevices(const char* sys_device_driver) { - root_device_ = fbl::MakeRefCounted<Device>(this, nullptr, ZX_PROTOCOL_ROOT); - misc_device_ = fbl::MakeRefCounted<Device>(this, root_device_, ZX_PROTOCOL_MISC_PARENT); - sys_device_ = fbl::MakeRefCounted<Device>(this, root_device_, 0); - test_device_ = fbl::MakeRefCounted<Device>(this, root_device_, ZX_PROTOCOL_TEST_PARENT); - - fbl::AllocChecker ac; - { - root_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND; - root_device_->name = fbl::String("root", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - root_device_->args = fbl::String("root,", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - } - - { - misc_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND; - misc_device_->name = fbl::String("misc", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - misc_device_->args = fbl::String("misc,", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - } - - { - sys_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE; - sys_device_->name = fbl::String("sys", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - sys_device_->libname = fbl::String(sys_device_driver, &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - sys_device_->args = fbl::String("sys,", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - } - - { - test_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND; - test_device_->name = fbl::String("test", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - test_device_->args = fbl::String("test,", &ac); - if (!ac.check()) { - return ZX_ERR_NO_MEMORY; - } - } + root_device_ = fbl::MakeRefCounted<Device>(this, "root", fbl::String(), "root,", nullptr, + ZX_PROTOCOL_ROOT, zx::channel()); + root_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND; + + misc_device_ = fbl::MakeRefCounted<Device>(this, "misc", fbl::String(), "misc,", root_device_, + ZX_PROTOCOL_MISC_PARENT, + zx::channel()); + misc_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND; + + sys_device_ = fbl::MakeRefCounted<Device>(this, "sys", sys_device_driver, "sys,", root_device_, + 0, zx::channel()); + sys_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE; + + test_device_ = fbl::MakeRefCounted<Device>(this, "test", fbl::String(), "test,", root_device_, + ZX_PROTOCOL_TEST_PARENT, zx::channel()); + test_device_->flags = DEV_CTX_IMMORTAL | DEV_CTX_MUST_ISOLATE | DEV_CTX_MULTI_BIND; return ZX_OK; } @@ -278,15 +237,15 @@ void Coordinator::DumpDevice(VmoWriter* vmo, const Device* dev, size_t indent) c extra[0] = 0; } if (pid == 0) { - vmo->Printf("%*s[%s]%s\n", (int)(indent * 3), "", dev->name.data(), extra); + vmo->Printf("%*s[%s]%s\n", (int)(indent * 3), "", dev->name().data(), extra); } else { vmo->Printf("%*s%c%s%c pid=%zu%s %s\n", (int)(indent * 3), "", - dev->flags & DEV_CTX_PROXY ? '<' : '[', dev->name.data(), - dev->flags & DEV_CTX_PROXY ? '>' : ']', pid, extra, dev->libname.data()); + dev->flags & DEV_CTX_PROXY ? '<' : '[', dev->name().data(), + dev->flags & DEV_CTX_PROXY ? '>' : ']', pid, extra, dev->libname().data()); } - if (dev->proxy) { + if (dev->proxy()) { indent++; - DumpDevice(vmo, dev->proxy.get(), indent); + DumpDevice(vmo, dev->proxy().get(), indent); } for (const auto& child : dev->children()) { DumpDevice(vmo, &child, indent + 1); @@ -302,8 +261,10 @@ void Coordinator::DumpState(VmoWriter* vmo) const { void Coordinator::DumpDeviceProps(VmoWriter* vmo, const Device* dev) const { if (dev->host()) { - vmo->Printf("Name [%s]%s%s%s\n", dev->name.data(), dev->libname.empty() ? "" : " Driver [", - dev->libname.empty() ? "" : dev->libname.data(), dev->libname.empty() ? "" : "]"); + vmo->Printf("Name [%s]%s%s%s\n", dev->name().data(), + dev->libname().empty() ? "" : " Driver [", + dev->libname().empty() ? "" : dev->libname().data(), + dev->libname().empty() ? "" : "]"); vmo->Printf("Flags :%s%s%s%s%s%s\n", dev->flags & DEV_CTX_IMMORTAL ? " Immortal" : "", dev->flags & DEV_CTX_MUST_ISOLATE ? " Isolate" : "", dev->flags & DEV_CTX_MULTI_BIND ? " MultiBind" : "", @@ -336,8 +297,8 @@ void Coordinator::DumpDeviceProps(VmoWriter* vmo, const Device* dev) const { vmo->Printf("\n"); } - if (dev->proxy) { - DumpDeviceProps(vmo, dev->proxy.get()); + if (dev->proxy()) { + DumpDeviceProps(vmo, dev->proxy().get()); } for (const auto& child : dev->children()) { DumpDeviceProps(vmo, &child); @@ -401,7 +362,7 @@ zx_status_t Coordinator::GetTopologicalPath(const fbl::RefPtr<const Device>& dev const char* name; if (itr->parent()) { - name = itr->name.data(); + name = itr->name().data(); } else { name = "dev"; } @@ -630,7 +591,7 @@ zx_status_t Coordinator::AddDevice(const fbl::RefPtr<Device>& parent, zx::channe // If we're creating a device that's using the component driver, inform the // component. - if (component_driver_ != nullptr && dev->libname == component_driver_->libname) { + if (component_driver_ != nullptr && dev->libname() == component_driver_->libname) { CompositeDeviceComponent* component = parent->component(); component->set_component_device(dev); status = component->composite()->TryAssemble(); @@ -642,7 +603,7 @@ zx_status_t Coordinator::AddDevice(const fbl::RefPtr<Device>& parent, zx::channe if (!invisible) { log(DEVLC, "devcoord: publish %p '%s' props=%zu args='%s' parent=%p\n", dev.get(), - dev->name.data(), dev->props().size(), dev->args.data(), dev->parent().get()); + dev->name().data(), dev->props().size(), dev->args().data(), dev->parent().get()); status = dev->SignalReadyForBind(); if (status != ZX_OK) { return status; @@ -676,17 +637,17 @@ zx_status_t Coordinator::RemoveDevice(const fbl::RefPtr<Device>& dev, bool force if (dev->flags & DEV_CTX_DEAD) { // This should not happen log(ERROR, "devcoordinator: cannot remove dev %p name='%s' twice!\n", dev.get(), - dev->name.data()); + dev->name().data()); return ZX_ERR_BAD_STATE; } if (dev->flags & DEV_CTX_IMMORTAL) { // This too should not happen log(ERROR, "devcoordinator: cannot remove dev %p name='%s' (immortal)\n", dev.get(), - dev->name.data()); + dev->name().data()); return ZX_ERR_BAD_STATE; } - log(DEVLC, "devcoordinator: remove %p name='%s' parent=%p\n", dev.get(), dev->name.data(), + log(DEVLC, "devcoordinator: remove %p name='%s' parent=%p\n", dev.get(), dev->name().data(), dev->parent().get()); dev->flags |= DEV_CTX_DEAD; @@ -698,8 +659,8 @@ zx_status_t Coordinator::RemoveDevice(const fbl::RefPtr<Device>& dev, bool force // TODO(teisenbe): Should we mark it as failed if this is a forced removal? dev->CompleteSuspend(ZX_OK); - if (dev->proxy) { - zx_status_t r = dh_send_remove_device(dev->proxy.get()); + if (dev->proxy()) { + zx_status_t r = dh_send_remove_device(dev->proxy().get()); if (r != ZX_OK) { log(ERROR, "devcoordinator: failed to send message in dc_remove_device: %d\n", r); } @@ -711,7 +672,7 @@ zx_status_t Coordinator::RemoveDevice(const fbl::RefPtr<Device>& dev, bool force } // Check if this device is a composite component device - if (component_driver_ != nullptr && dev->libname == component_driver_->libname) { + if (component_driver_ != nullptr && dev->libname() == component_driver_->libname) { // If it is, then its parent will know about which one (since the parent // is the actual device matched by the component description). const auto& parent = dev->parent(); @@ -779,7 +740,7 @@ zx_status_t Coordinator::RemoveDevice(const fbl::RefPtr<Device>& dev, bool force !(parent->host()->flags() & Devhost::Flags::kDying))) { log(DEVLC, "devcoordinator: bus device %p name='%s' is unbound\n", parent.get(), - parent->name.data()); + parent->name().data()); if (parent->retries > 0) { // Add device with an exponential backoff. @@ -832,12 +793,12 @@ zx_status_t Coordinator::AddCompositeDevice( size_t index; if (new_device->TryMatchComponents(dev_ref, &index)) { log(SPEW, "devcoordinator: dev='%s' matched component %zu of composite='%s'\n", - dev.name.data(), index, new_device->name().data()); + dev.name().data(), index, new_device->name().data()); status = new_device->BindComponent(index, dev_ref); if (status != ZX_OK) { log(ERROR, "devcoordinator: dev='%s' failed to bind component %zu of composite='%s': %s\n", - dev.name.data(), index, new_device->name().data(), + dev.name().data(), index, new_device->name().data(), zx_status_get_string(status)); } } @@ -895,7 +856,7 @@ zx_status_t Coordinator::GetMetadataRecurse(const fbl::RefPtr<Device>& dev, uint // search dev and its parent devices for a match fbl::RefPtr<Device> test = dev; while (true) { - for (const auto& md : test->metadata) { + for (const auto& md : test->metadata()) { if (md.type == type) { if (buffer != nullptr) { if (md.length > buflen) { @@ -970,7 +931,7 @@ zx_status_t Coordinator::AddMetadata(const fbl::RefPtr<Device>& dev, uint32_t ty md->type = type; md->length = length; memcpy(md->Data(), data, length); - dev->metadata.push_front(std::move(md)); + dev->AddMetadata(std::move(md)); return ZX_OK; } @@ -989,7 +950,7 @@ zx_status_t Coordinator::PublishMetadata(const fbl::RefPtr<Device>& dev, const c fbl::RefPtr<Device> itr = dev; // Adding metadata to arbitrary paths is restricted to drivers running in the sys devhost. while (itr && itr != sys_device_) { - if (itr->proxy) { + if (itr->proxy()) { // this device is in a child devhost return ZX_ERR_ACCESS_DENIED; } @@ -1129,9 +1090,9 @@ static zx_status_t dh_create_device(const fbl::RefPtr<Device>& dev, Devhost* dh, return r; } - if (dev->libname[0]) { + if (dev->libname().size() != 0) { zx::vmo vmo; - if ((r = dev->coordinator->LibnameToVmo(dev->libname, &vmo)) != ZX_OK) { + if ((r = dev->coordinator->LibnameToVmo(dev->libname(), &vmo)) != ZX_OK) { return r; } @@ -1178,7 +1139,7 @@ zx_status_t Coordinator::PrepareProxy(const fbl::RefPtr<Device>& dev, Devhost* t ZX_ASSERT(!(dev->flags & DEV_CTX_PROXY) && (dev->flags & DEV_CTX_MUST_ISOLATE)); // proxy args are "processname,args" - const char* arg0 = dev->args.data(); + const char* arg0 = dev->args().data(); const char* arg1 = strchr(arg0, ','); if (arg1 == nullptr) { return ZX_ERR_INTERNAL; @@ -1190,13 +1151,13 @@ zx_status_t Coordinator::PrepareProxy(const fbl::RefPtr<Device>& dev, Devhost* t snprintf(devhostname, sizeof(devhostname), "devhost:%.*s", (int)arg0len, arg0); zx_status_t r; - if (dev->proxy == nullptr && (r = dev->CreateProxy()) != ZX_OK) { + if (dev->proxy() == nullptr && (r = dev->CreateProxy()) != ZX_OK) { log(ERROR, "devcoord: cannot create proxy device: %d\n", r); return r; } // if this device has no devhost, first instantiate it - if (dev->proxy->host() == nullptr) { + if (dev->proxy()->host() == nullptr) { zx::channel h0, h1; // the immortal root devices do not provide proxy rpc bool need_proxy_rpc = !(dev->flags & DEV_CTX_IMMORTAL); @@ -1214,8 +1175,8 @@ zx_status_t Coordinator::PrepareProxy(const fbl::RefPtr<Device>& dev, Devhost* t return r; } } - dev->proxy->set_host(target_devhost); - if ((r = dh_create_device(dev->proxy, dev->proxy->host(), arg1, std::move(h1))) < 0) { + dev->proxy()->set_host(target_devhost); + if ((r = dh_create_device(dev->proxy(), dev->proxy()->host(), arg1, std::move(h1))) < 0) { log(ERROR, "devcoordinator: dh_create_device: %d\n", r); return r; } @@ -1229,8 +1190,9 @@ zx_status_t Coordinator::PrepareProxy(const fbl::RefPtr<Device>& dev, Devhost* t log(ERROR, "devcoordinator: fdio_service_connect %s: %d\n", kItemsPath, r); } } - if (dev->client_remote.is_valid()) { - if ((r = devfs_connect(dev->proxy.get(), std::move(dev->client_remote))) != ZX_OK) { + zx::channel client_remote = dev->take_client_remote(); + if (client_remote.is_valid()) { + if ((r = devfs_connect(dev->proxy().get(), std::move(client_remote))) != ZX_OK) { log(ERROR, "devcoordinator: devfs_connnect: %d\n", r); } } @@ -1258,7 +1220,7 @@ zx_status_t Coordinator::AttemptBind(const Driver* drv, const fbl::RefPtr<Device return r; } - r = dh_bind_driver(dev->proxy, drv->libname.c_str()); + r = dh_bind_driver(dev->proxy(), drv->libname.c_str()); // TODO(swetland): arrange to mark us unbound when the proxy (or its devhost) goes away if ((r == ZX_OK) && !(dev->flags & DEV_CTX_MULTI_BIND)) { dev->flags |= DEV_CTX_BOUND; @@ -1269,10 +1231,13 @@ zx_status_t Coordinator::AttemptBind(const Driver* drv, const fbl::RefPtr<Device void Coordinator::HandleNewDevice(const fbl::RefPtr<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 status = devfs_connect(dev.get(), std::move(dev->client_remote)); - if (status != ZX_OK) { - log(ERROR, "devcoordinator: devfs_connnect: %d\n", status); + if (!(dev->flags & DEV_CTX_MUST_ISOLATE)) { + zx::channel client_remote = dev->take_client_remote(); + if (client_remote.is_valid()) { + zx_status_t status = devfs_connect(dev.get(), std::move(client_remote)); + if (status != ZX_OK) { + log(ERROR, "devcoordinator: devfs_connnect: %d\n", status); + } } } // TODO(tesienbe): We probably should do something with the return value @@ -1302,7 +1267,7 @@ static int suspend_timeout_thread(void* arg) { void Coordinator::Suspend(SuspendContext ctx) { // The sys device should have a proxy. If not, the system hasn't fully initialized yet and // cannot go to suspend. - if (!sys_device_->proxy) { + if (!sys_device_->proxy()) { return; } if (suspend_context().flags() == SuspendContext::Flags::kSuspend) { @@ -1490,11 +1455,12 @@ zx_status_t Coordinator::BindDriverToDevice(const fbl::RefPtr<Device>& dev, cons return ZX_ERR_NEXT; } - log(SPEW, "devcoordinator: drv='%s' bindable to dev='%s'\n", drv->name.data(), dev->name.data()); + log(SPEW, "devcoordinator: drv='%s' bindable to dev='%s'\n", drv->name.data(), + dev->name().data()); zx_status_t status = AttemptBind(drv, dev); if (status != ZX_OK) { log(ERROR, "devcoordinator: failed to bind drv='%s' to dev='%s': %s\n", drv->name.data(), - dev->name.data(), zx_status_get_string(status)); + dev->name().data(), zx_status_get_string(status)); } if (status == ZX_ERR_NEXT) { // Convert ERR_NEXT to avoid confusing the caller @@ -1547,7 +1513,7 @@ zx_status_t Coordinator::BindDevice(const fbl::RefPtr<Device>& dev, fbl::StringP size_t index; if (composite.TryMatchComponents(dev, &index)) { log(SPEW, "devcoordinator: dev='%s' matched component %zu of composite='%s'\n", - dev->name.data(), index, composite.name().data()); + dev->name().data(), index, composite.name().data()); return composite.BindComponent(index, dev); } diff --git a/zircon/system/core/devmgr/devcoordinator/devfs.cpp b/zircon/system/core/devmgr/devcoordinator/devfs.cpp index c7270bb95464d5d28672df578642baef516480c4..ef895f21a9de7477b11d0e0683794d3072f32f54 100644 --- a/zircon/system/core/devmgr/devcoordinator/devfs.cpp +++ b/zircon/system/core/devmgr/devcoordinator/devfs.cpp @@ -598,7 +598,7 @@ zx_status_t devfs_publish(const fbl::RefPtr<Device>& parent, const fbl::RefPtr<D return ZX_ERR_INTERNAL; } - fbl::unique_ptr<Devnode> dnself = devfs_mknode(dev, dev->name); + fbl::unique_ptr<Devnode> dnself = devfs_mknode(dev, dev->name()); if (dnself == nullptr) { return ZX_ERR_NO_MEMORY; } @@ -619,7 +619,7 @@ zx_status_t devfs_publish(const fbl::RefPtr<Device>& parent, const fbl::RefPtr<D dir = proto_dir(dev->protocol_id()); if (dir != nullptr) { char tmp[32]; - const char* name = dev->name.data(); + const char* name = dev->name().data(); if (dev->protocol_id() != ZX_PROTOCOL_CONSOLE) { for (unsigned n = 0; n < 1000; n++) { diff --git a/zircon/system/core/devmgr/devcoordinator/device.cpp b/zircon/system/core/devmgr/devcoordinator/device.cpp index ac03e06194cd14e27f021e980855f087a9cb1924..093409ded049a59b0850c015b5bc41e7ebec29bf 100644 --- a/zircon/system/core/devmgr/devcoordinator/device.cpp +++ b/zircon/system/core/devmgr/devcoordinator/device.cpp @@ -15,9 +15,12 @@ namespace devmgr { -Device::Device(Coordinator* coord, fbl::RefPtr<Device> parent, uint32_t protocol_id) - : coordinator(coord), parent_(std::move(parent)), protocol_id_(protocol_id), - publish_task_([this] { coordinator->HandleNewDevice(fbl::WrapRefPtr(this)); }) {} +Device::Device(Coordinator* coord, fbl::String name, fbl::String libname, fbl::String args, + fbl::RefPtr<Device> parent, uint32_t protocol_id, zx::channel client_remote) + : coordinator(coord), name_(std::move(name)), libname_(std::move(libname)), + args_(std::move(args)), parent_(std::move(parent)), protocol_id_(protocol_id), + publish_task_([this] { coordinator->HandleNewDevice(fbl::WrapRefPtr(this)); }), + client_remote_(std::move(client_remote)) {} Device::~Device() { // Ideally we'd assert here that immortal devices are never destroyed, but @@ -26,7 +29,7 @@ Device::~Device() { // holding a reference we shouldn't be able to hit that check, in which case // the flag is only used to modify the proxy library loading behavior. - log(DEVLC, "devcoordinator: destroy dev %p name='%s'\n", this, this->name.data()); + log(DEVLC, "devcoordinator: destroy dev %p name='%s'\n", this, name_.data()); devfs_unpublish(this); @@ -34,7 +37,7 @@ Device::~Device() { set_host(nullptr); fbl::unique_ptr<Metadata> md; - while ((md = this->metadata.pop_front()) != nullptr) { + while ((md = metadata_.pop_front()) != nullptr) { if (md->has_path) { // return to published_metadata list coordinator->AppendPublishedMetadata(std::move(md)); @@ -61,7 +64,9 @@ zx_status_t Device::Create(Coordinator* coordinator, const fbl::RefPtr<Device>& real_parent = parent; } - auto dev = fbl::MakeRefCounted<Device>(coordinator, real_parent, protocol_id); + auto dev = fbl::MakeRefCounted<Device>(coordinator, std::move(name), std::move(driver_path), + std::move(args), real_parent, protocol_id, + std::move(client_remote)); if (!dev) { return ZX_ERR_NO_MEMORY; } @@ -70,14 +75,10 @@ zx_status_t Device::Create(Coordinator* coordinator, const fbl::RefPtr<Device>& return status; } - dev->name = std::move(name); - dev->libname = std::move(driver_path); - dev->args = std::move(args); dev->set_channel(std::move(rpc)); - dev->client_remote = std::move(client_remote); // If we have bus device args we are, by definition, a bus device. - if (dev->args.size() > 0) { + if (dev->args_.size() > 0) { dev->flags |= DEV_CTX_MUST_ISOLATE; } @@ -103,7 +104,8 @@ zx_status_t Device::Create(Coordinator* coordinator, const fbl::RefPtr<Device>& dev->host_->devices().push_back(dev.get()); } real_parent->children_.push_back(dev.get()); - log(DEVLC, "devcoord: dev %p name='%s' (child)\n", real_parent.get(), real_parent->name.data()); + log(DEVLC, "devcoord: dev %p name='%s' (child)\n", real_parent.get(), + real_parent->name().data()); *device = std::move(dev); return ZX_OK; @@ -117,7 +119,9 @@ zx_status_t Device::CreateComposite(Coordinator* coordinator, Devhost* devhost, composite_props.size()); memcpy(props.get(), composite_props.get(), props.size() * sizeof(props[0])); - auto dev = fbl::MakeRefCounted<Device>(coordinator, nullptr, ZX_PROTOCOL_COMPOSITE); + auto dev = fbl::MakeRefCounted<Device>(coordinator, composite.name(), fbl::String(), + fbl::String(), nullptr, ZX_PROTOCOL_COMPOSITE, + zx::channel()); if (!dev) { return ZX_ERR_NO_MEMORY; } @@ -127,7 +131,6 @@ zx_status_t Device::CreateComposite(Coordinator* coordinator, Devhost* devhost, return status; } - dev->name = composite.name(); dev->set_channel(std::move(rpc)); // We exist within our parent's device host dev->set_host(devhost); @@ -147,43 +150,46 @@ zx_status_t Device::CreateComposite(Coordinator* coordinator, Devhost* devhost, dev->host_->AddRef(); dev->host_->devices().push_back(dev.get()); - log(DEVLC, "devcoordinator: composite dev created %p name='%s'\n", dev.get(), dev->name.data()); + log(DEVLC, "devcoordinator: composite dev created %p name='%s'\n", dev.get(), + dev->name().data()); *device = std::move(dev); return ZX_OK; } zx_status_t Device::CreateProxy() { - ZX_ASSERT(this->proxy == nullptr); + ZX_ASSERT(proxy_ == nullptr); - auto dev = fbl::MakeRefCounted<Device>(this->coordinator, fbl::WrapRefPtr(this), protocol_id_); - if (dev == nullptr) { - return ZX_ERR_NO_MEMORY; - } - dev->name = this->name; - dev->libname = this->libname; + fbl::String driver_path = libname_; // non-immortal devices, use foo.proxy.so for // their proxy devices instead of foo.so if (!(this->flags & DEV_CTX_IMMORTAL)) { - const char* begin = dev->libname.data(); + const char* begin = driver_path.data(); const char* end = strstr(begin, ".so"); - fbl::StringPiece prefix(begin, end == nullptr ? dev->libname.size() : end - begin); + fbl::StringPiece prefix(begin, end == nullptr ? driver_path.size() : end - begin); fbl::AllocChecker ac; - dev->libname = fbl::String::Concat({prefix, ".proxy.so"}, &ac); + driver_path = fbl::String::Concat({prefix, ".proxy.so"}, &ac); if (!ac.check()) { return ZX_ERR_NO_MEMORY; } } + auto dev = fbl::MakeRefCounted<Device>(this->coordinator, name_, std::move(driver_path), + fbl::String(), fbl::WrapRefPtr(this), protocol_id_, + zx::channel()); + if (dev == nullptr) { + return ZX_ERR_NO_MEMORY; + } + dev->flags = DEV_CTX_PROXY; - this->proxy = std::move(dev); - log(DEVLC, "devcoord: dev %p name='%s' (proxy)\n", this, this->name.data()); + proxy_ = std::move(dev); + log(DEVLC, "devcoord: dev %p name='%s' (proxy)\n", this, name_.data()); return ZX_OK; } void Device::DetachFromParent() { if (this->flags & DEV_CTX_PROXY) { - parent_->proxy = nullptr; + parent_->proxy_ = nullptr; } else { parent_->children_.erase(*this); } @@ -210,7 +216,7 @@ zx_status_t Device::SendSuspend(uint32_t flags, SuspendCompletion completion) { // We already have a pending suspend return ZX_ERR_UNAVAILABLE; } - log(DEVLC, "devcoordinator: suspend dev %p name='%s'\n", this, this->name.data()); + log(DEVLC, "devcoordinator: suspend dev %p name='%s'\n", this, name_.data()); zx_status_t status = dh_send_suspend(this, flags); if (status != ZX_OK) { return status; @@ -245,7 +251,7 @@ void Device::HandleRpc(fbl::RefPtr<Device>&& dev, async_dispatcher_t* dispatcher if ((r = dev->HandleRead()) < 0) { if (r != ZX_ERR_STOP) { log(ERROR, "devcoordinator: device %p name='%s' rpc status: %d\n", dev.get(), - dev->name.data(), r); + dev->name().data(), r); } // If this device isn't already dead (removed), remove it. RemoveDevice() may // have been called by the RPC handler, in particular for the RemoveDevice RPC. @@ -259,7 +265,8 @@ void Device::HandleRpc(fbl::RefPtr<Device>&& dev, async_dispatcher_t* dispatcher return; } if (signal->observed & ZX_CHANNEL_PEER_CLOSED) { - log(ERROR, "devcoordinator: device %p name='%s' disconnected!\n", dev.get(), dev->name.data()); + log(ERROR, "devcoordinator: device %p name='%s' disconnected!\n", dev.get(), + dev->name().data()); dev->coordinator->RemoveDevice(dev, true); // Do not start waiting again on this device's channel again return; @@ -370,14 +377,14 @@ zx_status_t Device::HandleRead() { &fidl_msg, &err_msg); if (r != ZX_OK) { log(ERROR, "devcoordinator: rpc: bind-driver '%s' received malformed reply: %s\n", - this->name.data(), err_msg); + name_.data(), err_msg); return ZX_ERR_IO; } auto resp = reinterpret_cast<fuchsia_device_manager_DeviceControllerBindDriverResponse*>( fidl_msg.bytes); if (resp->status != ZX_OK) { - log(ERROR, "devcoordinator: rpc: bind-driver '%s' status %d\n", this->name.data(), + log(ERROR, "devcoordinator: rpc: bind-driver '%s' status %d\n", name_.data(), resp->status); } // TODO: try next driver, clear BOUND flag @@ -388,27 +395,27 @@ zx_status_t Device::HandleRead() { &fidl_msg, &err_msg); if (r != ZX_OK) { log(ERROR, "devcoordinator: rpc: suspend '%s' received malformed reply: %s\n", - this->name.data(), err_msg); + name_.data(), err_msg); return ZX_ERR_IO; } auto resp = reinterpret_cast<fuchsia_device_manager_DeviceControllerSuspendResponse*>( fidl_msg.bytes); if (resp->status != ZX_OK) { - log(ERROR, "devcoordinator: rpc: suspend '%s' status %d\n", this->name.data(), + log(ERROR, "devcoordinator: rpc: suspend '%s' status %d\n", name_.data(), resp->status); } if (!suspend_completion_) { log(ERROR, "devcoordinator: rpc: unexpected suspend reply for '%s' status %d\n", - this->name.data(), resp->status); + name_.data(), resp->status); return ZX_ERR_IO; } - log(DEVLC, "devcoordinator: suspended dev %p name='%s'\n", this, this->name.data()); + log(DEVLC, "devcoordinator: suspended dev %p name='%s'\n", this, name_.data()); CompleteSuspend(resp->status); } else { log(ERROR, "devcoordinator: rpc: dev '%s' received wrong unexpected reply %08x\n", - this->name.data(), hdr->ordinal); + name_.data(), hdr->ordinal); zx_handle_close_many(fidl_msg.handles, fidl_msg.num_handles); return ZX_ERR_IO; } @@ -493,11 +500,12 @@ static zx_status_t fidl_AddDeviceInvisible(void* ctx, zx_handle_t raw_rpc, static zx_status_t fidl_RemoveDevice(void* ctx, fidl_txn_t* txn) { auto dev = fbl::WrapRefPtr(static_cast<Device*>(ctx)); if (dev->coordinator->InSuspend()) { - log(ERROR, "devcoordinator: rpc: remove-device '%s' forbidden in suspend\n", dev->name.data()); + log(ERROR, "devcoordinator: rpc: remove-device '%s' forbidden in suspend\n", + dev->name().data()); return fuchsia_device_manager_CoordinatorRemoveDevice_reply(txn, ZX_ERR_BAD_STATE); } - log(RPC_IN, "devcoordinator: rpc: remove-device '%s'\n", dev->name.data()); + log(RPC_IN, "devcoordinator: rpc: remove-device '%s'\n", dev->name().data()); // TODO(teisenbe): RemoveDevice and the reply func can return errors. We should probably // act on it, but the existing code being migrated does not. dev->coordinator->RemoveDevice(dev, false); @@ -510,10 +518,11 @@ static zx_status_t fidl_RemoveDevice(void* ctx, fidl_txn_t* txn) { static zx_status_t fidl_MakeVisible(void* ctx, fidl_txn_t* txn) { auto dev = fbl::WrapRefPtr(static_cast<Device*>(ctx)); if (dev->coordinator->InSuspend()) { - log(ERROR, "devcoordinator: rpc: make-visible '%s' forbidden in suspend\n", dev->name.data()); + log(ERROR, "devcoordinator: rpc: make-visible '%s' forbidden in suspend\n", + dev->name().data()); return fuchsia_device_manager_CoordinatorMakeVisible_reply(txn, ZX_ERR_BAD_STATE); } - log(RPC_IN, "devcoordinator: rpc: make-visible '%s'\n", dev->name.data()); + log(RPC_IN, "devcoordinator: rpc: make-visible '%s'\n", dev->name().data()); // TODO(teisenbe): MakeVisibile can return errors. We should probably // act on it, but the existing code being migrated does not. dev->coordinator->MakeVisible(dev); @@ -525,10 +534,10 @@ static zx_status_t fidl_BindDevice(void* ctx, const char* driver_path_data, size auto dev = fbl::WrapRefPtr(static_cast<Device*>(ctx)); fbl::StringPiece driver_path(driver_path_data, driver_path_size); if (dev->coordinator->InSuspend()) { - log(ERROR, "devcoordinator: rpc: bind-device '%s' forbidden in suspend\n", dev->name.data()); + log(ERROR, "devcoordinator: rpc: bind-device '%s' forbidden in suspend\n", dev->name().data()); return fuchsia_device_manager_CoordinatorBindDevice_reply(txn, ZX_ERR_BAD_STATE); } - log(RPC_IN, "devcoordinator: rpc: bind-device '%s'\n", dev->name.data()); + log(RPC_IN, "devcoordinator: rpc: bind-device '%s'\n", dev->name().data()); zx_status_t status = dev->coordinator->BindDevice(dev, driver_path, false /* new device */); return fuchsia_device_manager_CoordinatorBindDevice_reply(txn, status); } diff --git a/zircon/system/core/devmgr/devcoordinator/device.h b/zircon/system/core/devmgr/devcoordinator/device.h index d019569f70a7972d3fadbfaddb089f4483194af8..e4447fe603bf3fe669a2820b264b383f047d2f05 100644 --- a/zircon/system/core/devmgr/devcoordinator/device.h +++ b/zircon/system/core/devmgr/devcoordinator/device.h @@ -56,27 +56,6 @@ class SuspendTask; // clang-format on struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHandler<Device> { - Device(Coordinator* coord, fbl::RefPtr<Device> parent, uint32_t protocol_id); - ~Device(); - - // Create a new device with the given parameters. This sets up its - // relationship with its parent and devhost and adds its RPC channel to the - // coordinator's async loop. This does not add the device to the - // coordinator's devices_ list, or trigger publishing - static zx_status_t Create(Coordinator* coordinator, const fbl::RefPtr<Device>& parent, - fbl::String name, fbl::String driver_path, fbl::String args, - uint32_t protocol_id, fbl::Array<zx_device_prop_t> props, - zx::channel rpc, bool invisible, zx::channel client_remote, - fbl::RefPtr<Device>* device); - static zx_status_t CreateComposite(Coordinator* coordinator, Devhost* devhost, - const CompositeDevice& composite, zx::channel rpc, - fbl::RefPtr<Device>* device); - zx_status_t CreateProxy(); - - static void HandleRpc(fbl::RefPtr<Device>&& dev, async_dispatcher_t* dispatcher, - async::WaitBase* wait, zx_status_t status, - const zx_packet_signal_t* signal); - // Node for entry in device child list struct Node { static fbl::DoublyLinkedListNodeState<Device*>& node_state(Device& obj) { @@ -84,6 +63,18 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan } }; + struct DevhostNode { + static fbl::DoublyLinkedListNodeState<Device*>& node_state(Device& obj) { + return obj.devhost_node_; + } + }; + + struct AllDevicesNode { + static fbl::DoublyLinkedListNodeState<Device*>& node_state(Device& obj) { + return obj.all_devices_node_; + } + }; + // This iterator provides access to a list of devices that does not provide // mechanisms for mutating that list. With this, a user can get mutable // access to a device in the list. This is achieved by making the linked @@ -201,6 +192,31 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan DeviceType* device_; }; + Device(Coordinator* coord, fbl::String name, fbl::String libname, fbl::String args, + fbl::RefPtr<Device> parent, uint32_t protocol_id, zx::channel client_remote); + ~Device(); + + // Create a new device with the given parameters. This sets up its + // relationship with its parent and devhost and adds its RPC channel to the + // coordinator's async loop. This does not add the device to the + // coordinator's devices_ list, or trigger publishing + static zx_status_t Create(Coordinator* coordinator, const fbl::RefPtr<Device>& parent, + fbl::String name, fbl::String driver_path, fbl::String args, + uint32_t protocol_id, fbl::Array<zx_device_prop_t> props, + zx::channel rpc, bool invisible, zx::channel client_remote, + fbl::RefPtr<Device>* device); + static zx_status_t CreateComposite(Coordinator* coordinator, Devhost* devhost, + const CompositeDevice& composite, zx::channel rpc, + fbl::RefPtr<Device>* device); + zx_status_t CreateProxy(); + + static void HandleRpc(fbl::RefPtr<Device>&& dev, async_dispatcher_t* dispatcher, + async::WaitBase* wait, zx_status_t status, + const zx_packet_signal_t* signal); + + // We do not want to expose the list itself for mutation, even if the + // children are allowed to be mutated. We manage this by making the + // iterator opaque. using NonConstChildListIterator = ChildListIterator<fbl::DoublyLinkedList<Device*, Node>::iterator, Device>; using ConstChildListIterator = @@ -209,9 +225,6 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan ChildListIteratorFactory<Device, NonConstChildListIterator>; using ConstChildListIteratorFactory = ChildListIteratorFactory<const Device, ConstChildListIterator>; - // We do not want to expose the list itself for mutation, even if the - // children are allowed to be mutated. We manage this by making the - // iterator opaque. NonConstChildListIteratorFactory children() { return NonConstChildListIteratorFactory(this); } @@ -229,52 +242,9 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan // given completion will be invoked. zx_status_t SendSuspend(uint32_t flags, SuspendCompletion completion); - // Run the completion for the outstanding suspend, if any. This method is - // only exposed currently because RemoveDevice is on Coordinator instead of - // Device. - void CompleteSuspend(zx_status_t status); - - Coordinator* coordinator; - uint32_t flags = 0; - - fbl::String name; - fbl::String libname; - fbl::String args; - // The backoff between each driver retry. This grows exponentially. - zx::duration backoff = zx::msec(250); - // The number of retries left for the driver. - uint32_t retries = 4; - Devnode* self = nullptr; - Devnode* link = nullptr; - fbl::RefPtr<Device> proxy; - - // 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 devhost's - // list-of-devices - fbl::DoublyLinkedListNodeState<Device*> dhnode; - struct DevhostNode { - static fbl::DoublyLinkedListNodeState<Device*>& node_state(Device& obj) { - return obj.dhnode; - } - }; - - // listnode for this device in the all devices list - fbl::DoublyLinkedListNodeState<Device*> anode; - struct AllDevicesNode { - static fbl::DoublyLinkedListNodeState<Device*>& node_state(Device& obj) { - return obj.anode; - } - }; - // Break the relationship between this device object and its parent void DetachFromParent(); - // Metadata entries associated to this device. - fbl::DoublyLinkedList<fbl::unique_ptr<Metadata>, Metadata::Node> metadata; - // Sets the properties of this device. Returns an error if the properties // array contains more than one property from the BIND_TOPO_* range. zx_status_t SetProps(fbl::Array<const zx_device_prop_t> props); @@ -284,6 +254,9 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan const fbl::RefPtr<Device>& parent() { return parent_; } fbl::RefPtr<const Device> parent() const { return parent_; } + const fbl::RefPtr<Device>& proxy() { return proxy_; } + fbl::RefPtr<const Device> proxy() const { return proxy_; } + uint32_t protocol_id() const { return protocol_id_; } bool is_bindable() const { @@ -318,6 +291,37 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan Devhost* host() const { return host_; } uint64_t local_id() const { return local_id_; } + const fbl::DoublyLinkedList<fbl::unique_ptr<Metadata>, Metadata::Node>& metadata() const { + return metadata_; + } + void AddMetadata(fbl::unique_ptr<Metadata> md) { + metadata_.push_front(std::move(md)); + } + + // Creates a new suspend task if necessary and returns a reference to it. + // If one is already in-progress, a reference to it is returned instead + fbl::RefPtr<SuspendTask> RequestSuspendTask(uint32_t suspend_flags); + // Run the completion for the outstanding suspend, if any. This method is + // only exposed currently because RemoveDevice is on Coordinator instead of + // Device. + void CompleteSuspend(zx_status_t status); + + zx::channel take_client_remote() { return std::move(client_remote_); } + + const fbl::String& name() const { return name_; } + const fbl::String& libname() const { return libname_; } + const fbl::String& args() const { return args_; } + + Coordinator* coordinator; + uint32_t flags = 0; + + // The backoff between each driver retry. This grows exponentially. + zx::duration backoff = zx::msec(250); + // The number of retries left for the driver. + uint32_t retries = 4; + Devnode* self = nullptr; + Devnode* link = nullptr; + // TODO(teisenbe): We probably want more states. For example, the DEAD flag // should probably move in to here. enum class State { @@ -326,16 +330,18 @@ struct Device : public fbl::RefCounted<Device>, public AsyncLoopRefCountedRpcHan }; State state() const { return state_; } - - // Creates a new suspend task if necessary and returns a reference to it. - // If one is already in-progress, a reference to it is returned instead - fbl::RefPtr<SuspendTask> RequestSuspendTask(uint32_t suspend_flags); private: zx_status_t HandleRead(); + const fbl::String name_; + const fbl::String libname_; + const fbl::String args_; + fbl::RefPtr<Device> parent_; const uint32_t protocol_id_; + fbl::RefPtr<Device> proxy_; + fbl::Array<const zx_device_prop_t> props_; // If the device has a topological property in |props|, this points to it. const zx_device_prop_t* topo_prop_ = nullptr; @@ -350,6 +356,15 @@ private: // precludes using the same intrusive nodes as single-parent devices. fbl::DoublyLinkedList<Device*, Node> children_; + // Metadata entries associated to this device. + fbl::DoublyLinkedList<fbl::unique_ptr<Metadata>, Metadata::Node> metadata_; + + // listnode for this device in the all devices list + fbl::DoublyLinkedListNodeState<Device*> all_devices_node_; + + // listnode for this device in its devhost's list-of-devices + fbl::DoublyLinkedListNodeState<Device*> devhost_node_; + // - If this device is part of a composite device, this is inhabited by // CompositeDeviceComponent* and it points to the component that matched it. // Note that this is only set on the device that matched the component, not @@ -375,6 +390,10 @@ private: // completed. It will likely mark |active_suspend_| as completed and clear // it. SuspendCompletion suspend_completion_; + + // For attaching as an open connection to the proxy device, + // or once the device becomes visible. + zx::channel client_remote_; }; } // namespace devmgr diff --git a/zircon/system/core/devmgr/devcoordinator/fidl.cpp b/zircon/system/core/devmgr/devcoordinator/fidl.cpp index 3b6f55a8062d1b8b22cec77309d74205fff9c49c..3a64cda2d604b99e44086de62550e98b36f28c27 100644 --- a/zircon/system/core/devmgr/devcoordinator/fidl.cpp +++ b/zircon/system/core/devmgr/devcoordinator/fidl.cpp @@ -29,7 +29,7 @@ zx_status_t dh_send_remove_device(const Device* dev) { zx_status_t dh_send_create_device(Device* dev, Devhost* dh, zx::channel rpc, zx::vmo driver, const char* args, zx::handle rpc_proxy) { - size_t driver_path_size = dev->libname.size(); + size_t driver_path_size = dev->libname().size(); size_t args_size = strlen(args); uint32_t wr_num_bytes = static_cast<uint32_t>(sizeof(fuchsia_device_manager_DevhostControllerCreateDeviceRequest) + @@ -49,7 +49,7 @@ zx_status_t dh_send_create_device(Device* dev, Devhost* dh, zx::channel rpc, zx: req->driver_path.size = driver_path_size; req->driver_path.data = reinterpret_cast<char*>(FIDL_ALLOC_PRESENT); - memcpy(driver_path_data, dev->libname.data(), driver_path_size); + memcpy(driver_path_data, dev->libname().data(), driver_path_size); req->driver = FIDL_HANDLE_PRESENT; req->parent_proxy = rpc_proxy.is_valid() ? FIDL_HANDLE_PRESENT : FIDL_HANDLE_ABSENT; diff --git a/zircon/system/core/devmgr/devcoordinator/suspend-task.cpp b/zircon/system/core/devmgr/devcoordinator/suspend-task.cpp index 2821d3d581344f0d9e74aaf5f2928bbf5a925919..d9767d0c8da235b0a22c271f884c9325f49c10d6 100644 --- a/zircon/system/core/devmgr/devcoordinator/suspend-task.cpp +++ b/zircon/system/core/devmgr/devcoordinator/suspend-task.cpp @@ -38,11 +38,11 @@ void SuspendTask::Run() { // Handle the device proxy, if it exists, after children since they might // depend on it. - if (device_->proxy != nullptr) { - switch (device_->proxy->state()) { + if (device_->proxy() != nullptr) { + switch (device_->proxy()->state()) { case Device::State::kSuspended: break; case Device::State::kActive: { - AddDependency(device_->proxy->RequestSuspendTask(flags_)); + AddDependency(device_->proxy()->RequestSuspendTask(flags_)); return; } }