From 193d11b6b4cc2b0dd8e63a3a373ee891f1da51e5 Mon Sep 17 00:00:00 2001 From: Fergus Dall Date: Wed, 5 Dec 2018 22:36:59 +0000 Subject: [PATCH] Add methods to UsbDevice to get USB bus and port numbers for the device Crosvm needs the physical USB bus and port number to be able to attach a USB device to a VM. Currently, there is no way to get this from a UsbDevice. This CL adds to the UsbDevice interface functions to obtain those values, and implementations for all four implementations. Bug: 891381 Change-Id: I28dbf20d948187e28de3c8b7ef63bc590e88eb67 Reviewed-on: https://chromium-review.googlesource.com/c/1349141 Reviewed-by: Jochen Eisinger Reviewed-by: Reilly Grant Commit-Queue: Fergus Dall Cr-Commit-Position: refs/heads/master@{#614141} --- .../usb_internals_page_handler.cc | 3 +- device/usb/mock_usb_device.cc | 9 +++-- device/usb/usb_device.cc | 15 +++++++-- device/usb/usb_device.h | 13 ++++++-- device/usb/usb_device_android.cc | 7 +++- device/usb/usb_device_impl.cc | 4 ++- device/usb/usb_device_linux.cc | 6 ++-- device/usb/usb_device_linux.h | 3 +- device/usb/usb_device_win.cc | 10 +++--- device/usb/usb_device_win.h | 5 ++- device/usb/usb_service_linux.cc | 23 ++++++++++--- device/usb/usb_service_linux.h | 3 +- device/usb/usb_service_win.cc | 33 +++++++++++++------ device/usb/usb_service_win.h | 2 +- 14 files changed, 97 insertions(+), 39 deletions(-) diff --git a/chrome/browser/ui/webui/usb_internals/usb_internals_page_handler.cc b/chrome/browser/ui/webui/usb_internals/usb_internals_page_handler.cc index b62096267ee2f6..57264e37c9fd15 100644 --- a/chrome/browser/ui/webui/usb_internals/usb_internals_page_handler.cc +++ b/chrome/browser/ui/webui/usb_internals/usb_internals_page_handler.cc @@ -43,7 +43,8 @@ TestUsbDevice::TestUsbDevice(const std::string& name, 0x0100, base::string16(), base::UTF8ToUTF16(name), - base::UTF8ToUTF16(serial_number)) { + base::UTF8ToUTF16(serial_number), + 0, 0) { webusb_landing_page_ = landing_page; } diff --git a/device/usb/mock_usb_device.cc b/device/usb/mock_usb_device.cc index e2d28460552ec7..f9921b086e57e0 100644 --- a/device/usb/mock_usb_device.cc +++ b/device/usb/mock_usb_device.cc @@ -25,7 +25,8 @@ MockUsbDevice::MockUsbDevice(uint16_t vendor_id, 0x0100, // device_version base::UTF8ToUTF16(manufacturer_string), base::UTF8ToUTF16(product_string), - base::UTF8ToUTF16(serial_number)) {} + base::UTF8ToUTF16(serial_number), + 0, 0) {} // Bus and port number MockUsbDevice::MockUsbDevice(uint16_t vendor_id, uint16_t product_id, @@ -42,7 +43,8 @@ MockUsbDevice::MockUsbDevice(uint16_t vendor_id, 0x0100, // device_version base::UTF8ToUTF16(manufacturer_string), base::UTF8ToUTF16(product_string), - base::UTF8ToUTF16(serial_number)) { + base::UTF8ToUTF16(serial_number), + 0, 0) { // Bus and port number webusb_landing_page_ = webusb_landing_page; } @@ -67,7 +69,8 @@ MockUsbDevice::MockUsbDevice( 0x0100, // device_version base::string16(), base::string16(), - base::string16()) { + base::string16(), + 0, 0) { // Bus and port number descriptor_.configurations = configurations; } diff --git a/device/usb/usb_device.cc b/device/usb/usb_device.cc index d2182feb408d94..59e260aa2b1ec8 100644 --- a/device/usb/usb_device.cc +++ b/device/usb/usb_device.cc @@ -14,16 +14,22 @@ UsbDevice::Observer::~Observer() = default; void UsbDevice::Observer::OnDeviceRemoved(scoped_refptr device) {} -UsbDevice::UsbDevice() : guid_(base::GenerateGUID()) {} +UsbDevice::UsbDevice(uint32_t bus_number, uint32_t port_number) + : bus_number_(bus_number), + port_number_(port_number), + guid_(base::GenerateGUID()) {} UsbDevice::UsbDevice(const UsbDeviceDescriptor& descriptor, const base::string16& manufacturer_string, const base::string16& product_string, - const base::string16& serial_number) + const base::string16& serial_number, + uint32_t bus_number, uint32_t port_number) : descriptor_(descriptor), manufacturer_string_(manufacturer_string), product_string_(product_string), serial_number_(serial_number), + bus_number_(bus_number), + port_number_(port_number), guid_(base::GenerateGUID()) {} UsbDevice::UsbDevice(uint16_t usb_version, @@ -35,10 +41,13 @@ UsbDevice::UsbDevice(uint16_t usb_version, uint16_t device_version, const base::string16& manufacturer_string, const base::string16& product_string, - const base::string16& serial_number) + const base::string16& serial_number, + uint32_t bus_number, uint32_t port_number) : manufacturer_string_(manufacturer_string), product_string_(product_string), serial_number_(serial_number), + bus_number_(bus_number), + port_number_(port_number), guid_(base::GenerateGUID()) { descriptor_.usb_version = usb_version; descriptor_.device_class = device_class; diff --git a/device/usb/usb_device.h b/device/usb/usb_device.h index b9cdc68046ffd5..41a8eb3adb92e6 100644 --- a/device/usb/usb_device.h +++ b/device/usb/usb_device.h @@ -51,6 +51,8 @@ class UsbDevice : public base::RefCountedThreadSafe { const std::string& guid() const { return guid_; } // Accessors to basic information. + uint32_t bus_number() const { return bus_number_; } + uint32_t port_number() const { return port_number_; } uint16_t usb_version() const { return descriptor_.usb_version; } uint8_t device_class() const { return descriptor_.device_class; } uint8_t device_subclass() const { return descriptor_.device_subclass; } @@ -92,11 +94,12 @@ class UsbDevice : public base::RefCountedThreadSafe { protected: friend class UsbService; - UsbDevice(); + UsbDevice(uint32_t bus_number, uint32_t port_number); UsbDevice(const UsbDeviceDescriptor& descriptor, const base::string16& manufacturer_string, const base::string16& product_string, - const base::string16& serial_number); + const base::string16& serial_number, + uint32_t bus_number, uint32_t port_number); UsbDevice(uint16_t usb_version, uint8_t device_class, uint8_t device_subclass, @@ -106,7 +109,8 @@ class UsbDevice : public base::RefCountedThreadSafe { uint16_t device_version, const base::string16& manufacturer_string, const base::string16& product_string, - const base::string16& serial_number); + const base::string16& serial_number, + uint32_t bus_number, uint32_t port_number); virtual ~UsbDevice(); void ActiveConfigurationChanged(int configuration_value); @@ -136,6 +140,9 @@ class UsbDevice : public base::RefCountedThreadSafe { void OnDisconnect(); void HandleClosed(UsbDeviceHandle* handle); + const uint32_t bus_number_; + const uint32_t port_number_; + const std::string guid_; // The current device configuration descriptor. May be null if the device is diff --git a/device/usb/usb_device_android.cc b/device/usb/usb_device_android.cc index de04cfa7f40630..940e6f3096aeba 100644 --- a/device/usb/usb_device_android.cc +++ b/device/usb/usb_device_android.cc @@ -113,7 +113,12 @@ UsbDeviceAndroid::UsbDeviceAndroid( device_version, manufacturer_string, product_string, - serial_number), + serial_number, + // We fake the bus and port number, because the underlying Java + // UsbDevice class doesn't offer an interface for getting these + // values, and nothing on Android seems to require them at this + // time (23-Nov-2018) + 0, 0), device_id_(Java_ChromeUsbDevice_getDeviceId(env, wrapper)), service_(service), j_object_(wrapper) { diff --git a/device/usb/usb_device_impl.cc b/device/usb/usb_device_impl.cc index e7e3f86cc3ecb4..ef4fb9c870f313 100644 --- a/device/usb/usb_device_impl.cc +++ b/device/usb/usb_device_impl.cc @@ -39,7 +39,9 @@ UsbDeviceImpl::UsbDeviceImpl(ScopedLibusbDeviceRef platform_device, descriptor.bcdDevice, base::string16(), base::string16(), - base::string16()), + base::string16(), + libusb_get_bus_number(platform_device.get()), + libusb_get_port_number(platform_device.get())), platform_device_(std::move(platform_device)) { CHECK(platform_device_.IsValid()) << "platform_device must be valid"; ReadAllConfigurations(); diff --git a/device/usb/usb_device_linux.cc b/device/usb/usb_device_linux.cc index 4e1b72931aa29f..5d77ff41d03222 100644 --- a/device/usb/usb_device_linux.cc +++ b/device/usb/usb_device_linux.cc @@ -32,11 +32,13 @@ UsbDeviceLinux::UsbDeviceLinux(const std::string& device_path, const std::string& manufacturer_string, const std::string& product_string, const std::string& serial_number, - uint8_t active_configuration) + uint8_t active_configuration, + uint32_t bus_number, uint32_t port_number) : UsbDevice(descriptor, base::UTF8ToUTF16(manufacturer_string), base::UTF8ToUTF16(product_string), - base::UTF8ToUTF16(serial_number)), + base::UTF8ToUTF16(serial_number), + bus_number, port_number), device_path_(device_path) { ActiveConfigurationChanged(active_configuration); } diff --git a/device/usb/usb_device_linux.h b/device/usb/usb_device_linux.h index f18710e7abc919..84f55d3d6e41ba 100644 --- a/device/usb/usb_device_linux.h +++ b/device/usb/usb_device_linux.h @@ -47,7 +47,8 @@ class UsbDeviceLinux : public UsbDevice { const std::string& manufacturer_string, const std::string& product_string, const std::string& serial_number, - uint8_t active_configuration); + uint8_t active_configuration, + uint32_t bus_number, uint32_t port_number); ~UsbDeviceLinux() override; diff --git a/device/usb/usb_device_win.cc b/device/usb/usb_device_win.cc index f8e4aefba81c9e..edad5bbc5399c9 100644 --- a/device/usb/usb_device_win.cc +++ b/device/usb/usb_device_win.cc @@ -25,15 +25,17 @@ const uint16_t kUsbVersion2_1 = 0x0210; UsbDeviceWin::UsbDeviceWin( const std::string& device_path, const std::string& hub_path, - int port_number, + uint32_t bus_number, + uint32_t port_number, const std::string& driver_name, scoped_refptr blocking_task_runner) - : device_path_(device_path), + : UsbDevice(bus_number, port_number), + device_path_(device_path), hub_path_(hub_path), - port_number_(port_number), driver_name_(driver_name), task_runner_(base::ThreadTaskRunnerHandle::Get()), - blocking_task_runner_(std::move(blocking_task_runner)) {} + blocking_task_runner_(std::move(blocking_task_runner)) { +} UsbDeviceWin::~UsbDeviceWin() {} diff --git a/device/usb/usb_device_win.h b/device/usb/usb_device_win.h index 597166f3b1e4fe..d51782f6d100dc 100644 --- a/device/usb/usb_device_win.h +++ b/device/usb/usb_device_win.h @@ -31,14 +31,14 @@ class UsbDeviceWin : public UsbDevice { // Called by UsbServiceWin only; UsbDeviceWin(const std::string& device_path, const std::string& hub_path, - int port_number, + uint32_t bus_number, + uint32_t port_number, const std::string& driver_name, scoped_refptr task_runner); ~UsbDeviceWin() override; const std::string& device_path() const { return device_path_; } - int port_number() const { return port_number_; } const std::string& driver_name() const { return driver_name_; } // Opens the device's parent hub in order to read the device, configuration @@ -65,7 +65,6 @@ class UsbDeviceWin : public UsbDevice { const std::string device_path_; const std::string hub_path_; - const int port_number_; const std::string driver_name_; scoped_refptr task_runner_; diff --git a/device/usb/usb_service_linux.cc b/device/usb/usb_service_linux.cc index b876dff7093f2e..6faefa89681961 100644 --- a/device/usb/usb_service_linux.cc +++ b/device/usb/usb_service_linux.cc @@ -169,10 +169,21 @@ void UsbServiceLinux::FileThreadHelper::OnDeviceAdded( if (value) base::StringToUint(value, &active_configuration); + unsigned bus_number = 0; + value = udev_device_get_sysattr_value(device.get(), "busnum"); + if (value) + base::StringToUint(value, &bus_number); + + unsigned port_number = 0; + value = udev_device_get_sysattr_value(device.get(), "devnum"); + if (value) + base::StringToUint(value, &port_number); + task_runner_->PostTask( - FROM_HERE, base::BindOnce(&UsbServiceLinux::OnDeviceAdded, service_, - device_path, descriptor, manufacturer, product, - serial_number, active_configuration)); + FROM_HERE, + base::BindOnce(&UsbServiceLinux::OnDeviceAdded, service_, device_path, + descriptor, manufacturer, product, serial_number, + active_configuration, bus_number, port_number)); } void UsbServiceLinux::FileThreadHelper::OnDeviceRemoved( @@ -213,7 +224,8 @@ void UsbServiceLinux::OnDeviceAdded(const std::string& device_path, const std::string& manufacturer, const std::string& product, const std::string& serial_number, - uint8_t active_configuration) { + uint8_t active_configuration, + uint32_t bus_number, uint32_t port_number) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); if (ContainsKey(devices_by_path_, device_path)) { @@ -229,7 +241,8 @@ void UsbServiceLinux::OnDeviceAdded(const std::string& device_path, scoped_refptr device( new UsbDeviceLinux(device_path, descriptor, manufacturer, product, - serial_number, active_configuration)); + serial_number, active_configuration, + bus_number, port_number)); devices_by_path_[device->device_path()] = device; if (device->usb_version() >= kUsbVersion2_1) { device->Open( diff --git a/device/usb/usb_service_linux.h b/device/usb/usb_service_linux.h index c22bc48556b176..39b87957520a67 100644 --- a/device/usb/usb_service_linux.h +++ b/device/usb/usb_service_linux.h @@ -37,7 +37,8 @@ class UsbServiceLinux : public UsbService { const std::string& manufacturer, const std::string& product, const std::string& serial_number, - uint8_t active_configuration); + uint8_t active_configuration, + uint32_t bus_number, uint32_t port_number); void DeviceReady(scoped_refptr device, bool success); void OnDeviceRemoved(const std::string& device_path); void HelperStarted(); diff --git a/device/usb/usb_service_win.cc b/device/usb/usb_service_win.cc index c4cd0242f7f6d9..99dd64f05d6a8a 100644 --- a/device/usb/usb_service_win.cc +++ b/device/usb/usb_service_win.cc @@ -82,7 +82,7 @@ bool GetDeviceStringProperty(HDEVINFO dev_info, bool GetDeviceInterfaceDetails(HDEVINFO dev_info, SP_DEVICE_INTERFACE_DATA* device_interface_data, std::string* device_path, - uint32_t* port_number, + uint32_t* bus_number, uint32_t* port_number, std::string* parent_instance_id, std::string* service_name) { DWORD required_size = 0; @@ -112,6 +112,14 @@ bool GetDeviceInterfaceDetails(HDEVINFO dev_info, base::SysWideToUTF8(device_interface_detail_data->DevicePath); } + if (bus_number) { + if (!GetDeviceUint32Property(dev_info, &dev_info_data, + DEVPKEY_Device_BusNumber, bus_number)) { + USB_PLOG(ERROR) << "Failed to get device bus number"; + return false; + } + } + if (port_number) { if (!GetDeviceUint32Property(dev_info, &dev_info_data, DEVPKEY_Device_Address, port_number)) { @@ -165,7 +173,8 @@ bool GetHubDevicePath(const std::string& instance_id, } return GetDeviceInterfaceDetails(dev_info.get(), &device_interface_data, - device_path, nullptr, nullptr, nullptr); + device_path, nullptr, nullptr, nullptr, + nullptr); } } // namespace @@ -195,11 +204,12 @@ class UsbServiceWin::BlockingTaskHelper { i, &device_interface_data); ++i) { std::string device_path; + uint32_t bus_number; uint32_t port_number; std::string parent_instance_id; std::string service_name; if (!GetDeviceInterfaceDetails(dev_info.get(), &device_interface_data, - &device_path, &port_number, + &device_path, &bus_number, &port_number, &parent_instance_id, &service_name)) { continue; } @@ -216,7 +226,7 @@ class UsbServiceWin::BlockingTaskHelper { service_task_runner_->PostTask( FROM_HERE, base::Bind(&UsbServiceWin::CreateDeviceObject, service_, device_path, - hub_path, port_number, service_name)); + hub_path, bus_number, port_number, service_name)); } if (GetLastError() != ERROR_NO_MORE_ITEMS) @@ -243,12 +253,13 @@ class UsbServiceWin::BlockingTaskHelper { return; } + uint32_t bus_number; uint32_t port_number; std::string parent_instance_id; std::string service_name; if (!GetDeviceInterfaceDetails(dev_info.get(), &device_interface_data, - nullptr, &port_number, &parent_instance_id, - &service_name)) { + nullptr, &bus_number, &port_number, + &parent_instance_id, &service_name)) { return; } @@ -264,7 +275,7 @@ class UsbServiceWin::BlockingTaskHelper { service_task_runner_->PostTask( FROM_HERE, base::Bind(&UsbServiceWin::CreateDeviceObject, service_, device_path, - hub_path, port_number, service_name)); + hub_path, bus_number, port_number, service_name)); } private: @@ -347,7 +358,8 @@ void UsbServiceWin::HelperStarted() { void UsbServiceWin::CreateDeviceObject(const std::string& device_path, const std::string& hub_path, - int port_number, + uint32_t bus_number, + uint32_t port_number, const std::string& driver_name) { // Devices that appear during initial enumeration are gathered into the first // result returned by GetDevices() and prevent device add/remove notifications @@ -355,8 +367,9 @@ void UsbServiceWin::CreateDeviceObject(const std::string& device_path, if (!enumeration_ready()) ++first_enumeration_countdown_; - scoped_refptr device(new UsbDeviceWin( - device_path, hub_path, port_number, driver_name, blocking_task_runner())); + scoped_refptr device( + new UsbDeviceWin(device_path, hub_path, bus_number, port_number, + driver_name, blocking_task_runner())); devices_by_path_[device->device_path()] = device; device->ReadDescriptors(base::Bind(&UsbServiceWin::DeviceReady, weak_factory_.GetWeakPtr(), device)); diff --git a/device/usb/usb_service_win.h b/device/usb/usb_service_win.h index a40d058ccb69ac..9a14b3a9e5dc51 100644 --- a/device/usb/usb_service_win.h +++ b/device/usb/usb_service_win.h @@ -39,7 +39,7 @@ class UsbServiceWin : public DeviceMonitorWin::Observer, public UsbService { void HelperStarted(); void CreateDeviceObject(const std::string& device_path, const std::string& hub_path, - int port_number, + uint32_t bus_number, uint32_t port_number, const std::string& driver_name); void DeviceReady(scoped_refptr device, bool success);