From b156c60ad4dfb53f483a829cda02b80fdb8ac8e6 Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Mon, 7 May 2018 15:14:47 -0700 Subject: [PATCH 1/3] adb: Expose device usb_handle through libadbd Fastbootd will reuses adb's functionfs transport implementation. Move it to daemon/include as well so it can be accessed with "adbd/usb.h". Otherwise usb.h will conflict with other imports. Test: adb builds and works Bug: 78793464 Change-Id: If3ba190d5c74b5f3633411f0484195e5a2a34d44 --- adb/Android.bp | 4 ++++ adb/daemon/{ => include/adbd}/usb.h | 2 ++ adb/daemon/usb.cpp | 20 ++++++++------------ 3 files changed, 14 insertions(+), 12 deletions(-) rename adb/daemon/{ => include/adbd}/usb.h (97%) diff --git a/adb/Android.bp b/adb/Android.bp index c0e4c5758..2a9a57985 100644 --- a/adb/Android.bp +++ b/adb/Android.bp @@ -299,6 +299,10 @@ cc_library_static { "libqemu_pipe", "libbase", ], + + export_include_dirs: [ + "daemon/include", + ], } cc_binary { diff --git a/adb/daemon/usb.h b/adb/daemon/include/adbd/usb.h similarity index 97% rename from adb/daemon/usb.h rename to adb/daemon/include/adbd/usb.h index 15a7f6539..ee81e25c9 100644 --- a/adb/daemon/usb.h +++ b/adb/daemon/include/adbd/usb.h @@ -19,6 +19,7 @@ #include #include #include +#include #include @@ -56,3 +57,4 @@ struct usb_handle { struct aio_block write_aiob; }; +usb_handle *create_usb_handle(); diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp index c724b1102..8c14955f6 100644 --- a/adb/daemon/usb.cpp +++ b/adb/daemon/usb.cpp @@ -41,7 +41,7 @@ #include #include "adb.h" -#include "daemon/usb.h" +#include "adbd/usb.h" #include "transport.h" using namespace std::chrono_literals; @@ -250,7 +250,7 @@ static int getMaxPacketSize(int ffs_fd) { } } -bool init_functionfs(struct usb_handle* h) { +static bool init_functionfs(struct usb_handle* h) { LOG(INFO) << "initializing functionfs"; ssize_t ret; @@ -336,9 +336,7 @@ err: return false; } -static void usb_ffs_open_thread(void* x) { - struct usb_handle* usb = (struct usb_handle*)x; - +static void usb_ffs_open_thread(usb_handle *usb) { adb_thread_setname("usb ffs open"); while (true) { @@ -505,9 +503,7 @@ static void usb_ffs_close(usb_handle* h) { h->notify.notify_one(); } -static void usb_ffs_init() { - D("[ usb_init - using FunctionFS ]"); - +usb_handle *create_usb_handle() { usb_handle* h = new usb_handle(); if (android::base::GetBoolProperty("sys.usb.ffs.aio_compat", false)) { @@ -523,15 +519,15 @@ static void usb_ffs_init() { } h->kick = usb_ffs_kick; h->close = usb_ffs_close; - - D("[ usb_init - starting thread ]"); - std::thread(usb_ffs_open_thread, h).detach(); + return h; } void usb_init() { + D("[ usb_init - using FunctionFS ]"); dummy_fd = adb_open("/dev/null", O_WRONLY); CHECK_NE(dummy_fd, -1); - usb_ffs_init(); + + std::thread(usb_ffs_open_thread, create_usb_handle()).detach(); } int usb_write(usb_handle* h, const void* data, int len) { From 16b78db9454b8a42b453c815c058fb52874b3104 Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Tue, 15 May 2018 16:20:41 -0700 Subject: [PATCH 2/3] adb: Have device usb_handle return io size Previously, read and write would return 0 on success. Now it will return the number of bytes read/write. This is more consistent with other usb handles and is needed in order to handle partial packets (for fastbootd). Update usb_write in other usb handles to return amount written. Change transport_usb accordingly. Test: adb works Bug: 78793464 Change-Id: If07ff05fbc8120343f20661475d34f4e5ff805de --- adb/client/usb_libusb.cpp | 2 +- adb/client/usb_linux.cpp | 4 ++-- adb/client/usb_osx.cpp | 4 ++-- adb/client/usb_windows.cpp | 2 +- adb/daemon/usb.cpp | 10 +++++++--- adb/transport_usb.cpp | 11 ++++++----- 6 files changed, 19 insertions(+), 14 deletions(-) diff --git a/adb/client/usb_libusb.cpp b/adb/client/usb_libusb.cpp index 46c3f58ec..10b609067 100644 --- a/adb/client/usb_libusb.cpp +++ b/adb/client/usb_libusb.cpp @@ -589,7 +589,7 @@ int usb_write(usb_handle* h, const void* d, int len) { int rc = perform_usb_transfer(h, info, std::move(lock)); LOG(DEBUG) << "usb_write(" << len << ") = " << rc; - return rc; + return info->transfer->actual_length; } int usb_read(usb_handle* h, void* d, int len) { diff --git a/adb/client/usb_linux.cpp b/adb/client/usb_linux.cpp index 1f376a4c9..aace16dad 100644 --- a/adb/client/usb_linux.cpp +++ b/adb/client/usb_linux.cpp @@ -418,11 +418,11 @@ int usb_write(usb_handle *h, const void *_data, int len) if (h->zero_mask && !(len & h->zero_mask)) { // If we need 0-markers and our transfer is an even multiple of the packet size, // then send a zero marker. - return usb_bulk_write(h, _data, 0); + return usb_bulk_write(h, _data, 0) == 0 ? n : -1; } D("-- usb_write --"); - return 0; + return n; } int usb_read(usb_handle *h, void *_data, int len) diff --git a/adb/client/usb_osx.cpp b/adb/client/usb_osx.cpp index 8a95a1907..49baf3699 100644 --- a/adb/client/usb_osx.cpp +++ b/adb/client/usb_osx.cpp @@ -497,8 +497,8 @@ int usb_write(usb_handle *handle, const void *buf, int len) } } - if (0 == result) - return 0; + if (!result) + return len; LOG(ERROR) << "usb_write failed with status: " << std::hex << result; return -1; diff --git a/adb/client/usb_windows.cpp b/adb/client/usb_windows.cpp index f529e8f32..d2aeb2604 100644 --- a/adb/client/usb_windows.cpp +++ b/adb/client/usb_windows.cpp @@ -365,7 +365,7 @@ int usb_write(usb_handle* handle, const void* data, int len) { } } - return 0; + return written; fail: // Any failure should cause us to kick the device instead of leaving it a diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp index 8c14955f6..5242718a2 100644 --- a/adb/daemon/usb.cpp +++ b/adb/daemon/usb.cpp @@ -368,6 +368,7 @@ static int usb_ffs_write(usb_handle* h, const void* data, int len) { D("about to write (fd=%d, len=%d)", h->bulk_in, len); const char* buf = static_cast(data); + int orig_len = len; while (len > 0) { int write_len = std::min(USB_FFS_BULK_SIZE, len); int n = adb_write(h->bulk_in, buf, write_len); @@ -380,13 +381,14 @@ static int usb_ffs_write(usb_handle* h, const void* data, int len) { } D("[ done fd=%d ]", h->bulk_in); - return 0; + return orig_len; } static int usb_ffs_read(usb_handle* h, void* data, int len) { D("about to read (fd=%d, len=%d)", h->bulk_out, len); char* buf = static_cast(data); + int orig_len = len; while (len > 0) { int read_len = std::min(USB_FFS_BULK_SIZE, len); int n = adb_read(h->bulk_out, buf, read_len); @@ -399,7 +401,7 @@ static int usb_ffs_read(usb_handle* h, void* data, int len) { } D("[ done fd=%d ]", h->bulk_out); - return 0; + return orig_len; } static int usb_ffs_do_aio(usb_handle* h, const void* data, int len, bool read) { @@ -447,6 +449,7 @@ static int usb_ffs_do_aio(usb_handle* h, const void* data, int len, bool read) { if (num_bufs == 1 && aiob->events[0].res == -EINTR) { continue; } + int ret = 0; for (int i = 0; i < num_bufs; i++) { if (aiob->events[i].res < 0) { errno = -aiob->events[i].res; @@ -454,8 +457,9 @@ static int usb_ffs_do_aio(usb_handle* h, const void* data, int len, bool read) { << " total bufs " << num_bufs; return -1; } + ret += aiob->events[i].res; } - return 0; + return ret; } } diff --git a/adb/transport_usb.cpp b/adb/transport_usb.cpp index 94b2e377c..602970ca6 100644 --- a/adb/transport_usb.cpp +++ b/adb/transport_usb.cpp @@ -122,7 +122,7 @@ static int remote_read(apacket* p, usb_handle* usb) { // On Android devices, we rely on the kernel to provide buffered read. // So we can recover automatically from EOVERFLOW. static int remote_read(apacket* p, usb_handle* usb) { - if (usb_read(usb, &p->msg, sizeof(amessage))) { + if (usb_read(usb, &p->msg, sizeof(amessage)) != sizeof(amessage)) { PLOG(ERROR) << "remote usb: read terminated (message)"; return -1; } @@ -134,7 +134,8 @@ static int remote_read(apacket* p, usb_handle* usb) { } p->payload.resize(p->msg.data_length); - if (usb_read(usb, &p->payload[0], p->payload.size())) { + if (usb_read(usb, &p->payload[0], p->payload.size()) + != static_cast(p->payload.size())) { PLOG(ERROR) << "remote usb: terminated (data)"; return -1; } @@ -154,14 +155,14 @@ bool UsbConnection::Read(apacket* packet) { } bool UsbConnection::Write(apacket* packet) { - unsigned size = packet->msg.data_length; + int size = packet->msg.data_length; - if (usb_write(handle_, &packet->msg, sizeof(packet->msg)) != 0) { + if (usb_write(handle_, &packet->msg, sizeof(packet->msg)) != sizeof(packet->msg)) { PLOG(ERROR) << "remote usb: 1 - write terminated"; return false; } - if (packet->msg.data_length != 0 && usb_write(handle_, packet->payload.data(), size) != 0) { + if (packet->msg.data_length != 0 && usb_write(handle_, packet->payload.data(), size) != size) { PLOG(ERROR) << "remote usb: 2 - write terminated"; return false; } From cda7c3b27c4bbea5e2bf0ece91ce77452ccd374e Mon Sep 17 00:00:00 2001 From: Jerry Zhang Date: Mon, 21 May 2018 14:22:43 -0700 Subject: [PATCH 3/3] adb: Add io size and zero packet to usb_handle Fastboot protocol doesn't include zero packets, so make it possible to configure that. Allow fastbootd to see how many bytes the handle can read/write at once. Test: adb works Bug: 78793464 Change-Id: I31e444f384d9d0cdf1ea936439b2028f8151c3b8 --- adb/daemon/include/adbd/usb.h | 5 ++++- adb/daemon/usb.cpp | 30 ++++++++++++++++-------------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/adb/daemon/include/adbd/usb.h b/adb/daemon/include/adbd/usb.h index ee81e25c9..7905d9d27 100644 --- a/adb/daemon/include/adbd/usb.h +++ b/adb/daemon/include/adbd/usb.h @@ -55,6 +55,9 @@ struct usb_handle { // read and write threads. struct aio_block read_aiob; struct aio_block write_aiob; + + bool reads_zero_packets; + size_t io_size; }; -usb_handle *create_usb_handle(); +usb_handle *create_usb_handle(unsigned num_bufs, unsigned io_size); diff --git a/adb/daemon/usb.cpp b/adb/daemon/usb.cpp index 5242718a2..4165c7b3e 100644 --- a/adb/daemon/usb.cpp +++ b/adb/daemon/usb.cpp @@ -53,7 +53,7 @@ using namespace std::chrono_literals; #define USB_FFS_BULK_SIZE 16384 // Number of buffers needed to fit MAX_PAYLOAD, with an extra for ZLPs. -#define USB_FFS_NUM_BUFS ((MAX_PAYLOAD / USB_FFS_BULK_SIZE) + 1) +#define USB_FFS_NUM_BUFS ((4 * MAX_PAYLOAD / USB_FFS_BULK_SIZE) + 1) #define cpu_to_le16(x) htole16(x) #define cpu_to_le32(x) htole32(x) @@ -226,16 +226,16 @@ static const struct { }, }; -static void aio_block_init(aio_block* aiob) { - aiob->iocb.resize(USB_FFS_NUM_BUFS); - aiob->iocbs.resize(USB_FFS_NUM_BUFS); - aiob->events.resize(USB_FFS_NUM_BUFS); +static void aio_block_init(aio_block* aiob, unsigned num_bufs) { + aiob->iocb.resize(num_bufs); + aiob->iocbs.resize(num_bufs); + aiob->events.resize(num_bufs); aiob->num_submitted = 0; - for (unsigned i = 0; i < USB_FFS_NUM_BUFS; i++) { + for (unsigned i = 0; i < num_bufs; i++) { aiob->iocbs[i] = &aiob->iocb[i]; } memset(&aiob->ctx, 0, sizeof(aiob->ctx)); - if (io_setup(USB_FFS_NUM_BUFS, &aiob->ctx)) { + if (io_setup(num_bufs, &aiob->ctx)) { D("[ aio: got error on io_setup (%d) ]", errno); } } @@ -318,6 +318,7 @@ static bool init_functionfs(struct usb_handle* h) { h->read_aiob.fd = h->bulk_out; h->write_aiob.fd = h->bulk_in; + h->reads_zero_packets = true; return true; err: @@ -408,7 +409,7 @@ static int usb_ffs_do_aio(usb_handle* h, const void* data, int len, bool read) { aio_block* aiob = read ? &h->read_aiob : &h->write_aiob; bool zero_packet = false; - int num_bufs = len / USB_FFS_BULK_SIZE + (len % USB_FFS_BULK_SIZE == 0 ? 0 : 1); + int num_bufs = len / h->io_size + (len % h->io_size == 0 ? 0 : 1); const char* cur_data = reinterpret_cast(data); int packet_size = getMaxPacketSize(aiob->fd); @@ -418,7 +419,7 @@ static int usb_ffs_do_aio(usb_handle* h, const void* data, int len, bool read) { } for (int i = 0; i < num_bufs; i++) { - int buf_len = std::min(len, USB_FFS_BULK_SIZE); + int buf_len = std::min(len, static_cast(h->io_size)); io_prep(&aiob->iocb[i], aiob->fd, cur_data, buf_len, 0, read); len -= buf_len; @@ -427,7 +428,7 @@ static int usb_ffs_do_aio(usb_handle* h, const void* data, int len, bool read) { if (len == 0 && buf_len % packet_size == 0 && read) { // adb does not expect the device to send a zero packet after data transfer, // but the host *does* send a zero packet for the device to read. - zero_packet = true; + zero_packet = h->reads_zero_packets; } } if (zero_packet) { @@ -507,7 +508,7 @@ static void usb_ffs_close(usb_handle* h) { h->notify.notify_one(); } -usb_handle *create_usb_handle() { +usb_handle *create_usb_handle(unsigned num_bufs, unsigned io_size) { usb_handle* h = new usb_handle(); if (android::base::GetBoolProperty("sys.usb.ffs.aio_compat", false)) { @@ -518,9 +519,10 @@ usb_handle *create_usb_handle() { } else { h->write = usb_ffs_aio_write; h->read = usb_ffs_aio_read; - aio_block_init(&h->read_aiob); - aio_block_init(&h->write_aiob); + aio_block_init(&h->read_aiob, num_bufs); + aio_block_init(&h->write_aiob, num_bufs); } + h->io_size = io_size; h->kick = usb_ffs_kick; h->close = usb_ffs_close; return h; @@ -531,7 +533,7 @@ void usb_init() { dummy_fd = adb_open("/dev/null", O_WRONLY); CHECK_NE(dummy_fd, -1); - std::thread(usb_ffs_open_thread, create_usb_handle()).detach(); + std::thread(usb_ffs_open_thread, create_usb_handle(USB_FFS_NUM_BUFS, USB_FFS_BULK_SIZE)).detach(); } int usb_write(usb_handle* h, const void* data, int len) {