From 7182c91db32986788f6156733fca60c83022d881 Mon Sep 17 00:00:00 2001 From: Scott Graham <scottmg@google.com> Date: Fri, 22 Mar 2019 22:35:59 +0000 Subject: [PATCH] [syscalls][usb][xhci] Introduce scheduler profile to devhost ... and use to remove zx_thread_set_priority() call from usb-xhci. devhost connects to scheduler.ProfileProvider, and allows drivers to request a profile. This is an intermediate state until we decide if/how we want to allow drivers to consume fidl services. ZX-3703 #comment [syscalls][usb][xhci] Introduce scheduler profile to devhost ZX-2914 #comment [syscalls][usb][xhci] Introduce scheduler profile to devhost Test: CQ, manual instrumentation/logging Change-Id: I23359f2062b77cf84e3bccef72682668817618ad --- zircon/system/core/devmgr/devhost/BUILD.gn | 2 + zircon/system/core/devmgr/devhost/api.cpp | 6 +++ zircon/system/core/devmgr/devhost/devhost.cpp | 7 +++ .../core/devmgr/devhost/scheduler_profile.cpp | 49 +++++++++++++++++++ .../core/devmgr/devhost/scheduler_profile.h | 15 ++++++ zircon/system/dev/usb/xhci/usb-xhci.cpp | 18 ++++--- zircon/system/dev/usb/xhci/usb-xhci.h | 2 +- zircon/system/dev/usb/xhci/xhci.h | 2 + zircon/system/ulib/ddk/include/ddk/driver.h | 11 +++++ 9 files changed, 104 insertions(+), 8 deletions(-) create mode 100644 zircon/system/core/devmgr/devhost/scheduler_profile.cpp create mode 100644 zircon/system/core/devmgr/devhost/scheduler_profile.h diff --git a/zircon/system/core/devmgr/devhost/BUILD.gn b/zircon/system/core/devmgr/devhost/BUILD.gn index a328c7e803f..e04dc350c4b 100644 --- a/zircon/system/core/devmgr/devhost/BUILD.gn +++ b/zircon/system/core/devmgr/devhost/BUILD.gn @@ -23,6 +23,7 @@ library("driver") { "core.cpp", "devhost.cpp", "rpc-server.cpp", + "scheduler_profile.cpp", "tracing.cpp", "zx-device.cpp", ] @@ -32,6 +33,7 @@ library("driver") { "$zx/system/fidl/fuchsia-device:c", "$zx/system/fidl/fuchsia-device-manager:c", "$zx/system/fidl/fuchsia-io:c", + "$zx/system/fidl/fuchsia-scheduler:c", "$zx/system/ulib/async:async-cpp.static", "$zx/system/ulib/async:async-default.static", "$zx/system/ulib/async:static", diff --git a/zircon/system/core/devmgr/devhost/api.cpp b/zircon/system/core/devmgr/devhost/api.cpp index ad882b83769..6317813acc3 100644 --- a/zircon/system/core/devmgr/devhost/api.cpp +++ b/zircon/system/core/devmgr/devhost/api.cpp @@ -5,6 +5,7 @@ #include <zircon/compiler.h> #include "devhost.h" +#include "scheduler_profile.h" #include <ddk/debug.h> #include <ddk/device.h> #include <zircon/device/vfs.h> @@ -140,6 +141,11 @@ __EXPORT void device_make_visible(zx_device_t* dev) { devhost_make_visible(dev_ref); } +__EXPORT zx_status_t device_get_profile(zx_device_t* dev, uint32_t priority, const char* name, + zx_handle_t* out_profile) { + return devhost_get_scheduler_profile(priority, name, out_profile); +} + __EXPORT const char* device_get_name(zx_device_t* dev) { return dev->name; } diff --git a/zircon/system/core/devmgr/devhost/devhost.cpp b/zircon/system/core/devmgr/devhost/devhost.cpp index 9fc0440f7e8..d2379ceecf2 100644 --- a/zircon/system/core/devmgr/devhost/devhost.cpp +++ b/zircon/system/core/devmgr/devhost/devhost.cpp @@ -46,6 +46,7 @@ #include "../shared/log.h" #include "composite-device.h" #include "main.h" +#include "scheduler_profile.h" #include "tracing.h" zx_status_t zx_driver::Create(fbl::RefPtr<zx_driver>* out_driver) { @@ -1350,6 +1351,12 @@ __EXPORT int device_host_main(int argc, char** argv) { } } + r = devhost_connect_scheduler_profile_provider(); + if (r != ZX_OK) { + log(INFO, "devhost: error connecting to profile provider: %d\n", r); + return -1; + } + if ((r = SetupRootDevcoordinatorConnection(std::move(root_conn_channel))) != ZX_OK) { log(ERROR, "devhost: could not watch rpc channel: %d\n", r); return -1; diff --git a/zircon/system/core/devmgr/devhost/scheduler_profile.cpp b/zircon/system/core/devmgr/devhost/scheduler_profile.cpp new file mode 100644 index 00000000000..67f3f419dfa --- /dev/null +++ b/zircon/system/core/devmgr/devhost/scheduler_profile.cpp @@ -0,0 +1,49 @@ +// Copyright 2019 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "scheduler_profile.h" + +#include <stdio.h> +#include <string.h> +#include <lib/fdio/directory.h> +#include <lib/zx/channel.h> +#include <lib/zx/profile.h> + +#include <fuchsia/scheduler/c/fidl.h> + +namespace devmgr { + +zx_handle_t scheduler_profile_provider; + +zx_status_t devhost_connect_scheduler_profile_provider() { + zx::channel registry_client; + zx::channel registry_service; + zx_status_t status = zx::channel::create(0u, ®istry_client, ®istry_service); + if (status != ZX_OK) + return status; + + status = fdio_service_connect("/svc/" fuchsia_scheduler_ProfileProvider_Name, + registry_service.release()); + if (status != ZX_OK) + return status; + + scheduler_profile_provider = registry_client.release(); + return ZX_OK; +} + +zx_status_t devhost_get_scheduler_profile(uint32_t priority, const char* name, + zx_handle_t* profile) { + zx_status_t fidl_status = ZX_ERR_INTERNAL; + zx_status_t status = fuchsia_scheduler_ProfileProviderGetProfile( + scheduler_profile_provider, priority, name, strlen(name), &fidl_status, profile); + if (status != ZX_OK) { + return status; + } + if (fidl_status != ZX_OK) { + return fidl_status; + } + return ZX_OK; +} + +} // namespace devmgr diff --git a/zircon/system/core/devmgr/devhost/scheduler_profile.h b/zircon/system/core/devmgr/devhost/scheduler_profile.h new file mode 100644 index 00000000000..26cb61ffce3 --- /dev/null +++ b/zircon/system/core/devmgr/devhost/scheduler_profile.h @@ -0,0 +1,15 @@ +// Copyright 2019 The Fuchsia Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#pragma once + +#include <zircon/types.h> + +namespace devmgr { + +zx_status_t devhost_connect_scheduler_profile_provider(); +zx_status_t devhost_get_scheduler_profile(uint32_t priority, const char* name, + zx_handle_t* profile); + +} // namespace devmgr diff --git a/zircon/system/dev/usb/xhci/usb-xhci.cpp b/zircon/system/dev/usb/xhci/usb-xhci.cpp index 9d2b53bb5ce..4daf1b09937 100644 --- a/zircon/system/dev/usb/xhci/usb-xhci.cpp +++ b/zircon/system/dev/usb/xhci/usb-xhci.cpp @@ -36,9 +36,6 @@ namespace usb_xhci { #define MAX_SLOTS 255 -#define DEFAULT_PRIORITY 16 -#define HIGH_PRIORITY 24 - #define PDEV_MMIO_INDEX 0 #define PDEV_IRQ_INDEX 0 @@ -217,7 +214,9 @@ int UsbXhci::CompleterThread(void* arg) { // TODO(johngro): See ZX-940. Get rid of this. For now we need thread // priorities so that realtime transactions use the completer which ends // up getting realtime latency guarantees. - zx_thread_set_priority(completer->priority); + if (completer->high_priority) { + zx_object_set_profile(zx_thread_self(), xhci->profile_handle.get(), 0); + } while (1) { zx_status_t wait_res; @@ -252,7 +251,7 @@ int UsbXhci::StartThread() { if (!ac.check()) { return ZX_ERR_NO_MEMORY; } - + for (uint32_t i = 0; i < xhci_->num_interrupts; i++) { auto* completer = &completers_[i]; completer->xhci = xhci_.get(); @@ -260,8 +259,7 @@ int UsbXhci::StartThread() { // We need a high priority thread for isochronous transfers. // If there is only one interrupt available, that thread will need // to be high priority. - completer->priority = (i == ISOCH_INTERRUPTER || xhci_->num_interrupts == 1) ? - HIGH_PRIORITY : DEFAULT_PRIORITY; + completer->high_priority = i == ISOCH_INTERRUPTER || xhci_->num_interrupts == 1; } // xhci_start will block, so do this part here instead of in usb_xhci_bind @@ -297,6 +295,12 @@ zx_status_t UsbXhci::FinishBind() { return status; } + status = device_get_profile(zxdev_, /*HIGH_PRIORITY*/ 24, "zircon/system/dev/usb/xhci/usb-xhci", + xhci_->profile_handle.reset_and_get_address()); + if (status != ZX_OK) { + return status; + } + thrd_t thread; thrd_create_with_name(&thread, [](void* arg) -> int { diff --git a/zircon/system/dev/usb/xhci/usb-xhci.h b/zircon/system/dev/usb/xhci/usb-xhci.h index e0070f5c370..72da710714a 100644 --- a/zircon/system/dev/usb/xhci/usb-xhci.h +++ b/zircon/system/dev/usb/xhci/usb-xhci.h @@ -58,7 +58,7 @@ private: struct Completer { xhci_t *xhci; uint32_t interrupter; - uint32_t priority; + bool high_priority; }; int StartThread(); diff --git a/zircon/system/dev/usb/xhci/xhci.h b/zircon/system/dev/usb/xhci/xhci.h index acdb616ea52..e8024791bda 100644 --- a/zircon/system/dev/usb/xhci/xhci.h +++ b/zircon/system/dev/usb/xhci/xhci.h @@ -14,6 +14,7 @@ #include <lib/sync/completion.h> #include <lib/zx/bti.h> #include <lib/zx/interrupt.h> +#include <lib/zx/profile.h> #include <usb/usb-request.h> #include <zircon/hw/usb.h> #include <zircon/listnode.h> @@ -209,6 +210,7 @@ struct xhci_t { ddk::IoBuffer scratch_pad_index_buffer; zx::bti bti_handle; + zx::profile profile_handle; // pool of control requests that can be reused usb_request_pool_t free_reqs = {}; diff --git a/zircon/system/ulib/ddk/include/ddk/driver.h b/zircon/system/ulib/ddk/include/ddk/driver.h index 832a6c4beb1..b25f5867024 100644 --- a/zircon/system/ulib/ddk/include/ddk/driver.h +++ b/zircon/system/ulib/ddk/include/ddk/driver.h @@ -142,6 +142,17 @@ zx_status_t device_remove(zx_device_t* device); zx_status_t device_rebind(zx_device_t* device); void device_make_visible(zx_device_t* device); +// Retrieves a profile handle into |out_profile| from the scheduler for the +// given |priority| and |name|. Ownership of |out_profile| is given to the +// caller. See fuchsia.scheduler.ProfileProvider for more detail. +// +// The profile handle can be used with zx_object_set_profile() to control thread +// priority. +// +// The current arguments are transitional, and will likely change in the future. +zx_status_t device_get_profile(zx_device_t* device, uint32_t priority, const char* name, + zx_handle_t* out_profile); + // A description of a part of a device component. It provides a bind program // that will match a device on the path from the root of the device tree to the // target device. -- GitLab