Changeset 1d758fc in mainline for uspace/lib

Timestamp:

2018-02-12T10:11:47Z (8 years ago)

Author:

Ondřej Hlavatý <aearsis@…>

Branches:

lfn, master, serial, ticket/834-toolchain-update, topic/msim-upgrade, topic/simplify-dev-export

Children:

5fe3f954

Parents:

2f762a7

git-author:

Ondřej Hlavatý <aearsis@…> (2018-02-05 03:28:50)

git-committer:

Ondřej Hlavatý <aearsis@…> (2018-02-12 10:11:47)

Message:

usb: rethinking DMA buffers

Location:

uspace/lib

Files:

• drv/generic/remote_usbhc.c (modified) (4 diffs)
• drv/include/usbhc_iface.h (modified) (2 diffs)
• usb/include/usb/dma_buffer.h (modified) (1 diff)
• usb/src/dma_buffer.c (modified) (6 diffs)
• usbdev/src/pipes.c (modified) (4 diffs)
• usbhost/include/usb/host/bus.h (modified) (1 diff)
• usbhost/include/usb/host/endpoint.h (modified) (3 diffs)
• usbhost/include/usb/host/usb_transfer_batch.h (modified) (3 diffs)
• usbhost/src/bus.c (modified) (6 diffs)
• usbhost/src/ddf_helpers.c (modified) (5 diffs)
• usbhost/src/endpoint.c (modified) (4 diffs)
• usbhost/src/usb_transfer_batch.c (modified) (5 diffs)

uspace/lib/drv/generic/remote_usbhc.c

@@ -210 +210 @@
                         ? AS_AREA_WRITE : AS_AREA_READ;
 
-                const errno_t ret = async_share_out_start(exch, req->base, flags);
+                const errno_t ret = async_share_out_start(exch, req->buffer.virt, flags);
                 if (ret != EOK) {
                         async_forget(opening_request);
@@ -374 +374 @@
                 return;
         }
-        if (trans->request.base != NULL) {
-                as_area_destroy(trans->request.base);
+        if (trans->request.buffer.virt != NULL) {
+                as_area_destroy(trans->request.buffer.virt);
         }
 
@@ -422 +422 @@
         }
 
-        if ((err = async_share_out_finalize(data_callid, &trans->request.base)))
+        if ((err = async_share_out_finalize(data_callid, &trans->request.buffer.virt)))
                 return err;
 
         /*
-         * As we're going to check the mapping, we must make sure the memory is
-         * actually mapped. We must do it right now, because the area might be
-         * read-only or write-only, and we may be unsure later.
+         * As we're going to get physical addresses of the mapping, we must make
+         * sure the memory is actually mapped. We must do it right now, because
+         * the area might be read-only or write-only, and we may be unsure
+         * later.
          */
         if (flags & AS_AREA_READ) {
                 char foo = 0;
-                volatile const char *buf = trans->request.base + trans->request.offset;
+                volatile const char *buf = trans->request.buffer.virt + trans->request.offset;
                 for (size_t i = 0; i < size; i += PAGE_SIZE)
                         foo += buf[i];
         } else {
-                volatile char *buf = trans->request.base + trans->request.offset;
+                volatile char *buf = trans->request.buffer.virt + trans->request.offset;
                 for (size_t i = 0; i < size; i += PAGE_SIZE)
                         buf[i] = 0xff;
@@ -482 +483 @@
         } else {
                 /* The value was valid on the other side, for us, its garbage. */
-                trans->request.base = NULL;
+                trans->request.buffer.virt = NULL;
         }
 

uspace/lib/drv/include/usbhc_iface.h

@@ -104 +104 @@
 
 // FIXME: DMA buffers shall be part of libdrv anyway.
-typedef unsigned dma_policy_t;
+typedef uintptr_t dma_policy_t;
+
+typedef struct dma_buffer {
+        void *virt;
+        dma_policy_t policy;
+} dma_buffer_t;
 
 typedef struct usb_pipe_desc {
@@ -134 +139 @@
 
         /**
-         * Base address of the buffer to share. Must be at least offset + size
-         * large. Is patched after being transmitted over IPC, so the pointer is
-         * still valid.
-         *
-         * Note that offset might be actually more than PAGE_SIZE.
+         * The DMA buffer to share. Must be at least offset + size large. Is
+         * patched after being transmitted over IPC, so the pointer is still
+         * valid.
          */
-        void *base;
+        dma_buffer_t buffer;
         size_t offset;                  /**< Offset to the buffer */
         size_t size;                    /**< Requested size. */
-        dma_policy_t buffer_policy;     /**< Properties of the buffer. */
 } usbhc_iface_transfer_request_t;
 

uspace/lib/usb/include/usb/dma_buffer.h

@@ -37 +37 @@
  * shared through IPC).
  *
- * Note that although allocated memory is always page-aligned, the buffer itself
- * may be only a part of it, justifying the existence of page-alignment and
- * page-crossing flags.
+ * Currently, it is possible to allocate either completely contiguous buffers
+ * (with dma_map_anonymous) or arbitrary memory (with as_area_create). Shall the
+ * kernel be updated, this is a subject of major optimization of memory usage.
+ * The other way to do it without the kernel is building an userspace IO vector
+ * in a similar way how QEMU does it.
  *
- * Also, currently the buffers that are allocated are always contiguous and
- * page-aligned, regardless of whether the policy requires it. We blindly
- * believe this fact in dma_buffer_phys, which will yield wrong results if the
- * buffer is not contiguous.
+ * The structures themselves are defined in usbhc_iface, because they need to be
+ * passed through IPC.
  */
 #ifndef LIB_USB_DMA_BUFFER
 #define LIB_USB_DMA_BUFFER
 
+#include <as.h>
+#include <bitops.h>
+#include <errno.h>
 #include <stdint.h>
 #include <stdlib.h>
 #include <usbhc_iface.h>
-#include <errno.h>
 
-#define DMA_POLICY_4GiB         (1<<0)  /**< Must use only 32-bit addresses */
-#define DMA_POLICY_PAGE_ALIGNED (1<<1)  /**< The first pointer must be page-aligned */
-#define DMA_POLICY_CONTIGUOUS   (1<<2)  /**< Pages must follow each other physically */
-#define DMA_POLICY_NOT_CROSSING (1<<3)  /**< Buffer must not cross page boundary. (Implies buffer is no larger than page).  */
+/**
+ * The DMA policy describes properties of the buffer. It is used in two
+ * different contexts. Either it represents requirements, which shall be
+ * satisfied to avoid copying the buffer to a more strict one. Or, it is the
+ * actual property of the buffer, which can be more strict than requested. It
+ * always holds that more bits set means more restrictive policy, and that by
+ * computing a bitwise OR one gets the restriction that holds for both.
+ *
+ * The high bits of a DMA policy represent a physical contiguity. If bit i is
+ * set, it means that chunks of a size 2^(i+1) are contiguous in memory. It
+ * shall never happen that bit i > j is set when j is not.
+ *
+ * The previous applies for i >= PAGE_WIDTH. Lower bits are used as bit flags.
+ */
+#define DMA_POLICY_FLAGS_MASK           (PAGE_SIZE - 1)
+#define DMA_POLICY_CHUNK_SIZE_MASK      (~DMA_POLICY_FLAGS_MASK)
 
-#define DMA_POLICY_STRICT       (-1U)
-#define DMA_POLICY_DEFAULT      DMA_POLICY_STRICT
+#define DMA_POLICY_4GiB (1<<0)          /**< Must use only 32-bit addresses */
 
-typedef struct dma_buffer {
-        void *virt;
-        uintptr_t phys;
-} dma_buffer_t;
+#define DMA_POLICY_STRICT               (-1UL)
+#define DMA_POLICY_DEFAULT              DMA_POLICY_STRICT
+
+extern dma_policy_t dma_policy_create(unsigned, size_t);
+
+/**
+ * Get mask which defines bits of offset in chunk.
+ */
+static inline size_t dma_policy_chunk_mask(const dma_policy_t policy)
+{
+        return policy | DMA_POLICY_FLAGS_MASK;
+}
 
 extern errno_t dma_buffer_alloc(dma_buffer_t *db, size_t size);
 extern errno_t dma_buffer_alloc_policy(dma_buffer_t *, size_t, dma_policy_t);
 extern void dma_buffer_free(dma_buffer_t *);
-extern uintptr_t dma_buffer_phys(const dma_buffer_t *, void *);
 
-extern bool dma_buffer_check_policy(const void *, size_t, const dma_policy_t);
+extern uintptr_t dma_buffer_phys(const dma_buffer_t *, const void *);
+
+static inline uintptr_t dma_buffer_phys_base(const dma_buffer_t *db)
+{
+        return dma_buffer_phys(db, db->virt);
+}
 
 extern errno_t dma_buffer_lock(dma_buffer_t *, void *, size_t);
 extern void dma_buffer_unlock(dma_buffer_t *, size_t);
 
-static inline int dma_buffer_is_set(dma_buffer_t *db)
+extern void dma_buffer_acquire(dma_buffer_t *);
+extern void dma_buffer_release(dma_buffer_t *);
+
+static inline bool dma_buffer_is_set(const dma_buffer_t *db)
 {
         return !!db->virt;

uspace/lib/usb/src/dma_buffer.c

@@ -39 +39 @@
 #include "usb/dma_buffer.h"
 
+dma_policy_t dma_policy_create(unsigned flags, size_t chunk_size)
+{
+        assert((chunk_size & (chunk_size - 1)) == 0); /* Check if power of 2 */
+        assert(chunk_size >= PAGE_SIZE || chunk_size == 0);
+
+        return ((chunk_size - 1) & DMA_POLICY_CHUNK_SIZE_MASK)
+                | (flags & DMA_POLICY_FLAGS_MASK);
+}
+
+/**
+ * As the driver is typically using only a few buffers at once, we cache the
+ * physical mapping to avoid calling the kernel unnecessarily often. This cache
+ * is global for a task.
+ *
+ * TODO: "few" is currently limited to one.
+ */
+static struct {
+        const void *last;
+        uintptr_t phys;
+} phys_mapping_cache = { 0 };
+
+static void cache_insert(const void *v, uintptr_t p)
+{
+        phys_mapping_cache.last = v;
+        phys_mapping_cache.phys = p;
+}
+
+static void cache_evict(const void *v)
+{
+        if (phys_mapping_cache.last == v)
+                phys_mapping_cache.last = NULL;
+}
+
+static bool cache_find(const void *v, uintptr_t *p)
+{
+        *p = phys_mapping_cache.phys;
+        return phys_mapping_cache.last == v;
+}
+
 /**
  * Allocate a DMA buffer.
- *
- * XXX: Currently cannot make much use of missing constraints, as it always
- * allocates page-aligned contiguous buffer. We rely on it in dma_buffer_phys.
  *
  * @param[in] db dma_buffer_t structure to fill
@@ -62 +98 @@
         void *address = AS_AREA_ANY;
 
-        const int ret = dmamem_map_anonymous(real_size,
+        const int err = dmamem_map_anonymous(real_size,
             flags, AS_AREA_READ | AS_AREA_WRITE, 0,
             &phys, &address);
-
-        if (ret == EOK) {
-                /* Access the pages to force mapping */
-                volatile char *buf = address;
-                for (size_t i = 0; i < size; i += PAGE_SIZE)
-                        buf[i] = 0xff;
-
-                db->virt = address;
-                db->phys = phys;
-        }
-        return ret;
+        if (err)
+                return err;
+
+        /* Access the pages to force mapping */
+        volatile char *buf = address;
+        for (size_t i = 0; i < size; i += PAGE_SIZE)
+                buf[i] = 0xff;
+
+        db->virt = address;
+        db->policy = dma_policy_create(policy, 0);
+        cache_insert(db->virt, phys);
+
+        return EOK;
 }
 
@@ -102 +140 @@
                 dmamem_unmap_anonymous(db->virt);
                 db->virt = NULL;
-                db->phys = 0;
+                db->policy = 0;
         }
 }
@@ -112 +150 @@
  * @param[in] virt Pointer somewhere inside db
  */
-uintptr_t dma_buffer_phys(const dma_buffer_t *db, void *virt)
-{
-        return db->phys + (virt - db->virt);
-}
-
-/**
- * Check whether a memory area is compatible with a policy.
- *
- * Useful to skip copying when the buffer is already ready to be given to
- * hardware as is.
- *
- * Note that the "as_get_physical_mapping" fails when the page is not mapped
- * yet, and that the caller is responsible for forcing the mapping.
- */
-bool dma_buffer_check_policy(const void *buffer, size_t size, const dma_policy_t policy)
-{
-        uintptr_t addr = (uintptr_t) buffer;
-
-        const bool check_4gib       = !!(policy & DMA_POLICY_4GiB);
-        const bool check_crossing   = !!(policy & DMA_POLICY_NOT_CROSSING);
-        const bool check_alignment  = !!(policy & DMA_POLICY_PAGE_ALIGNED);
-        const bool check_contiguous = !!(policy & DMA_POLICY_CONTIGUOUS);
-
-        /* Check the two conditions that are easy */
-        if (check_crossing && (addr + size - 1) / PAGE_SIZE != addr / PAGE_SIZE)
-                goto violated;
-
-        if (check_alignment && ((uintptr_t) buffer) % PAGE_SIZE)
-                goto violated;
-
-        /*
-         * For these conditions, we need to walk through pages and check
-         * physical address of each one
-         */
-        if (check_contiguous || check_4gib) {
-                const void *virt = buffer;
-                uintptr_t phys;
-
-                /* Get the mapping of the first page */
-                if (as_get_physical_mapping(virt, &phys))
-                        goto error;
-
-                /* First page can already break 4GiB condition */
-                if (check_4gib && (phys & DMAMEM_4GiB) != 0)
-                        goto violated;
-
-                while (size >= PAGE_SIZE) {
-                        /* Move to the next page */
-                        virt += PAGE_SIZE;
-                        size -= PAGE_SIZE;
-
-                        uintptr_t last_phys = phys;
-                        if (as_get_physical_mapping(virt, &phys))
-                                goto error;
-
-                        if (check_contiguous && (phys - last_phys) != PAGE_SIZE)
-                                goto violated;
-
-                        if (check_4gib && (phys & DMAMEM_4GiB) != 0)
-                                goto violated;
-                }
-        }
-
-        /* All checks passed */
+uintptr_t dma_buffer_phys(const dma_buffer_t *db, const void *virt)
+{
+        const size_t chunk_mask = dma_policy_chunk_mask(db->policy);
+        const uintptr_t offset = (virt - db->virt) & chunk_mask;
+        const void *chunk_base = virt - offset;
+
+        uintptr_t phys;
+
+        if (!cache_find(chunk_base, &phys)) {
+                if (as_get_physical_mapping(chunk_base, &phys))
+                        return 0;
+                cache_insert(chunk_base, phys);
+        }
+
+        return phys + offset;
+}
+
+static bool dma_buffer_is_4gib(dma_buffer_t *db, size_t size)
+{
+        if (sizeof(uintptr_t) <= 32)
+                return true;
+
+        const size_t chunk_size = dma_policy_chunk_mask(db->policy) + 1;
+        const size_t chunks = chunk_size ? 1 : size / chunk_size;
+
+        for (size_t c = 0; c < chunks; c++) {
+                const void *addr = db->virt + (c * chunk_size);
+                const uintptr_t phys = dma_buffer_phys(db, addr);
+        
+                if ((phys & DMAMEM_4GiB) != 0)
+                        return false;
+        }
+
         return true;
-
-violated:
-error:
-        return false;
 }
 
@@ -192 +195 @@
 errno_t dma_buffer_lock(dma_buffer_t *db, void *virt, size_t size)
 {
+        assert(virt);
+
+        uintptr_t phys;
+
+        const errno_t err = dmamem_map(virt, size, 0, 0, &phys);
+        if (err)
+                return err;
+
         db->virt = virt;
-        return dmamem_map(db->virt, size, 0, 0, &db->phys);
+        db->policy = dma_policy_create(0, PAGE_SIZE);
+        cache_insert(virt, phys);
+
+        unsigned flags = -1U;
+        if (!dma_buffer_is_4gib(db, size))
+                flags &= ~DMA_POLICY_4GiB;
+        db->policy = dma_policy_create(flags, PAGE_SIZE);
+
+        return EOK;
 }
 
@@ -204 +223 @@
                 dmamem_unmap(db->virt, size);
                 db->virt = NULL;
-                db->phys = 0;
-        }
+                db->policy = 0;
+        }
+}
+
+/**
+ * Must be called when the buffer is received over IPC. Clears potentially
+ * leftover value from different buffer mapped to the same virtual address.
+ */
+void dma_buffer_acquire(dma_buffer_t *db)
+{
+        cache_evict(db->virt);
+}
+
+/**
+ * Counterpart of acquire.
+ */
+void dma_buffer_release(dma_buffer_t *db)
+{
+        cache_evict(db->virt);
 }
 

uspace/lib/usbdev/src/pipes.c

@@ -83 +83 @@
 
         /* Only control writes make sense without buffer */
-        if ((t->dir != USB_DIRECTION_OUT || !t->is_control)
-            && (t->req.base == NULL || t->req.size == 0))
+        if ((t->dir != USB_DIRECTION_OUT || !t->is_control) && t->req.size == 0)
                 return EINVAL;
 
         /* Nonzero size requires buffer */
-        if (t->req.base == NULL && t->req.size != 0)
+        if (!dma_buffer_is_set(&t->req.buffer) && t->req.size != 0)
                 return EINVAL;
 
@@ -119 +118 @@
 /**
  * Setup the transfer request inside transfer according to dma buffer provided.
+ *
+ * TODO: The buffer could have been allocated as a more strict one. Currently,
+ * we assume that the policy is just the requested one.
  */
 static void setup_dma_buffer(transfer_t *t, void *base, void *ptr, size_t size)
 {
-        t->req.base = base;
+        t->req.buffer.virt = base;
+        t->req.buffer.policy = t->pipe->desc.transfer_buffer_policy;
         t->req.offset = ptr - base;
         t->req.size = size;
-        t->req.buffer_policy = t->pipe->desc.transfer_buffer_policy;
 }
 
@@ -133 +135 @@
 static errno_t transfer_wrap_dma(transfer_t *t, void *buf, size_t size)
 {
+        if (size == 0) {
+                setup_dma_buffer(t, NULL, NULL, 0);
+                return transfer_common(t);
+        }
+
         void *dma_buf = usb_pipe_alloc_buffer(t->pipe, size);
         setup_dma_buffer(t, dma_buf, dma_buf, size);
@@ -364 +371 @@
         .direction = USB_DIRECTION_BOTH,
         .max_transfer_size = CTRL_PIPE_MIN_PACKET_SIZE,
+        .transfer_buffer_policy = DMA_POLICY_STRICT,
 };
 

uspace/lib/usbhost/include/usb/host/bus.h

@@ -153 +153 @@
 int bus_device_offline(device_t *);
 
-int bus_device_send_batch(device_t *, usb_target_t,
-    usb_direction_t direction, char *, size_t, uint64_t,
-    usbhc_iface_transfer_callback_t, void *, const char *);
+/**
+ * A proforma to USB transfer batch. As opposed to transfer batch, which is
+ * supposed to be a dynamic structrure, this one is static and descriptive only.
+ * Its fields are copied to the final batch.
+ */
+typedef struct transfer_request {
+        usb_target_t target;
+        usb_direction_t dir;
+
+        dma_buffer_t buffer;
+        size_t offset, size;
+        uint64_t setup;
+
+        usbhc_iface_transfer_callback_t on_complete;
+        void *arg;
+
+        const char *name;
+} transfer_request_t;
+
+int bus_issue_transfer(device_t *, const transfer_request_t *);
 
 errno_t bus_device_send_batch_sync(device_t *, usb_target_t,

uspace/lib/usbhost/include/usb/host/endpoint.h

@@ -51 +51 @@
 typedef struct bus bus_t;
 typedef struct device device_t;
+typedef struct transfer_request transfer_request_t;
 typedef struct usb_transfer_batch usb_transfer_batch_t;
 
@@ -98 +99 @@
         /** Maximum size of one transfer */
         size_t max_transfer_size;
-        /** Policy for transfer buffers */
-        dma_policy_t transfer_buffer_policy;
+
+        /* Policies for transfer buffers */
+        dma_policy_t transfer_buffer_policy;            /**< A hint for optimal performance. */
+        dma_policy_t required_transfer_buffer_policy;   /**< Enforced by the library. */
 
         /**
@@ -122 +125 @@
 extern void endpoint_deactivate_locked(endpoint_t *);
 
-int endpoint_send_batch(endpoint_t *, usb_target_t, usb_direction_t,
-    char *, size_t, uint64_t, usbhc_iface_transfer_callback_t, void *,
-    const char *);
+int endpoint_send_batch(endpoint_t *, const transfer_request_t *);
 
 static inline bus_t *endpoint_get_bus(endpoint_t *ep)

uspace/lib/usbhost/include/usb/host/usb_transfer_batch.h

@@ -70 +70 @@
         } setup;
 
+        /** DMA buffer with enforced policy */
+        dma_buffer_t dma_buffer;
+        /** Size of memory buffer */
+        size_t offset, size;
+
         /**
          * In case a bounce buffer is allocated, the original buffer must to be
          * stored to be filled after the IN transaction is finished.
          */
-        char *buffer;
-        /** Size of memory buffer */
-        size_t buffer_size;
-
-        /** DMA buffer with enforced policy */
-        dma_buffer_t dma_buffer;
+        char *original_buffer;
         bool is_bounced;
 
@@ -107 +107 @@
         usb_str_transfer_type_short((batch).ep->transfer_type), \
         usb_str_direction((batch).dir), \
-        (batch).buffer_size, (batch).ep->max_packet_size
+        (batch).size, (batch).ep->max_packet_size
 
 /** Wrapper for bus operation. */
@@ -115 +115 @@
 void usb_transfer_batch_init(usb_transfer_batch_t *, endpoint_t *);
 
+/** Buffer handling */
+bool usb_transfer_batch_bounce_required(usb_transfer_batch_t *);
 errno_t usb_transfer_batch_bounce(usb_transfer_batch_t *);
-/** Buffer preparation */
-errno_t usb_transfer_batch_prepare_buffer(usb_transfer_batch_t *, char *);
 
 /** Batch finalization. */

uspace/lib/usbhost/src/bus.c

@@ -46 +46 @@
 #include <str_error.h>
 #include <usb/debug.h>
+#include <usb/dma_buffer.h>
 
 #include "endpoint.h"
@@ -389 +390 @@
                 endpoint_init(ep, device, desc);
         }
+
+        assert((ep->required_transfer_buffer_policy & ~ep->transfer_buffer_policy) == 0);
 
         /* Bus reference */
@@ -557 +560 @@
 
 /**
- * Initiate a transfer on the bus. Finds the target endpoint, creates
- * a transfer batch and schedules it.
- *
- * @param device Device for which to send the batch
- * @param target The target of the transfer.
- * @param direction A direction of the transfer.
- * @param data A pointer to the data buffer.
- * @param size Size of the data buffer.
- * @param setup_data Data to use in the setup stage (Control communication type)
- * @param on_complete Callback which is called after the batch is complete
- * @param arg Callback parameter.
- * @param name Communication identifier (for nicer output).
+ * Assert some conditions on transfer request. As the request is an entity of
+ * HC driver only, we can force these conditions harder. Invalid values from
+ * devices shall be caught on DDF interface already.
+ */
+static void check_request(const transfer_request_t *request)
+{
+        assert(usb_target_is_valid(&request->target));
+        assert(request->dir != USB_DIRECTION_BOTH);
+        /* Non-zero offset => size is non-zero */
+        assert(request->offset == 0 || request->size != 0);
+        /* Non-zero size => buffer is set */
+        assert(request->size == 0 || dma_buffer_is_set(&request->buffer));
+        /* Non-null arg => callback is set */
+        assert(request->arg == NULL || request->on_complete != NULL);
+        assert(request->name);
+}
+
+/**
+ * Initiate a transfer with given device.
+ *
  * @return Error code.
  */
-int bus_device_send_batch(device_t *device, usb_target_t target,
-    usb_direction_t direction, char *data, size_t size, uint64_t setup_data,
-    usbhc_iface_transfer_callback_t on_complete, void *arg, const char *name)
-{
-        assert(device->address == target.address);
+int bus_issue_transfer(device_t *device, const transfer_request_t *request)
+{
+        assert(device);
+        assert(request);
+
+        check_request(request);
+        assert(device->address == request->target.address);
 
         /* Temporary reference */
-        endpoint_t *ep = bus_find_endpoint(device, target.endpoint, direction);
+        endpoint_t *ep = bus_find_endpoint(device, request->target.endpoint, request->dir);
         if (ep == NULL) {
                 usb_log_error("Endpoint(%d:%d) not registered for %s.",
-                    device->address, target.endpoint, name);
+                    device->address, request->target.endpoint, request->name);
                 return ENOENT;
         }
@@ -587 +600 @@
         assert(ep->device == device);
 
-        /*
-         * This method is already callable from HC only, so we can force these
-         * conditions harder.
-         * Invalid values from devices shall be caught on DDF interface already.
-         */
-        assert(usb_target_is_valid(&target));
-        assert(direction != USB_DIRECTION_BOTH);
-        assert(size == 0 || data != NULL);
-        assert(arg == NULL || on_complete != NULL);
-        assert(name);
-
-        const int err = endpoint_send_batch(ep, target, direction,
-            data, size, setup_data, on_complete, arg, name);
+        const int err = endpoint_send_batch(ep, request);
 
         /* Temporary reference */
@@ -650 +651 @@
     const char *name, size_t *transferred_size)
 {
+        int err;
         sync_data_t sd = { .done = false };
         fibril_mutex_initialize(&sd.done_mtx);
         fibril_condvar_initialize(&sd.done_cv);
 
-        const int ret = bus_device_send_batch(device, target, direction,
-            data, size, setup_data, sync_transfer_complete, &sd, name);
-        if (ret != EOK)
-                return ret;
+        transfer_request_t request = {
+                .target = target,
+                .dir = direction,
+                .offset = ((uintptr_t) data) % PAGE_SIZE,
+                .size = size,
+                .setup = setup_data,
+                .on_complete = sync_transfer_complete,
+                .arg = &sd,
+                .name = name,
+        };
+
+        if (data &&
+            (err = dma_buffer_lock(&request.buffer, data - request.offset, size)))
+                return err;
+
+        if ((err = bus_issue_transfer(device, &request))) {
+                dma_buffer_unlock(&request.buffer, size);
+                return err;
+        }
 
         /*
@@ -668 +685 @@
         fibril_mutex_unlock(&sd.done_mtx);
 
+        dma_buffer_unlock(&request.buffer, size);
+
         if (transferred_size)
                 *transferred_size = sd.transferred_size;

uspace/lib/usbhost/src/ddf_helpers.c

@@ -46 +46 @@
 #include <usb/descriptor.h>
 #include <usb/usb.h>
+#include <usb/dma_buffer.h>
 #include <usb_iface.h>
 #include <usbhc_iface.h>
@@ -271 +272 @@
  * @return Error code.
  */
-static errno_t transfer(ddf_fun_t *fun, const usbhc_iface_transfer_request_t *req,
+static errno_t transfer(ddf_fun_t *fun,
+    const usbhc_iface_transfer_request_t *ifreq,
     usbhc_iface_transfer_callback_t callback, void *arg)
 {
@@ -280 +282 @@
         const usb_target_t target = {{
                 .address = dev->address,
-                .endpoint = req->endpoint,
-                .stream = req->stream,
+                .endpoint = ifreq->endpoint,
+                .stream = ifreq->stream,
         }};
 
@@ -287 +289 @@
                 return EINVAL;
 
-        if (req->size > 0 && req->base == NULL)
+        if (ifreq->offset > 0 && ifreq->size == 0)
+                return EINVAL;
+
+        if (ifreq->size > 0 && !dma_buffer_is_set(&ifreq->buffer))
                 return EBADMEM;
 
@@ -293 +298 @@
                 return EBADMEM;
 
-        const char *name = (req->dir == USB_DIRECTION_IN) ? "READ" : "WRITE";
-
-        char *buffer = req->base + req->offset;
-
-        return bus_device_send_batch(dev, target, req->dir,
-            buffer, req->size, req->setup,
-            callback, arg, name);
+        const transfer_request_t request = {
+                .target = target,
+                .dir = ifreq->dir,
+                .buffer = ifreq->buffer,
+                .offset = ifreq->offset,
+                .size = ifreq->size,
+                .setup = ifreq->setup,
+                .on_complete = callback,
+                .arg = arg,
+                .name = (ifreq->dir == USB_DIRECTION_IN) ? "READ" : "WRITE",
+        };
+
+        return bus_issue_transfer(dev, &request);
 }
 

uspace/lib/usbhost/src/endpoint.c

@@ -75 +75 @@
         ep->max_transfer_size = ep->max_packet_size * ep->packets_per_uframe;
         ep->transfer_buffer_policy = DMA_POLICY_STRICT;
+        ep->required_transfer_buffer_policy = DMA_POLICY_STRICT;
 }
 
@@ -207 +208 @@
  *
  * @param endpoint Endpoint for which to send the batch
- * @param target The target of the transfer.
- * @param direction A direction of the transfer.
- * @param data A pointer to the data buffer.
- * @param size Size of the data buffer.
- * @param setup_data Data to use in the setup stage (Control communication type)
- * @param on_complete Callback which is called after the batch is complete
- * @param arg Callback parameter.
- * @param name Communication identifier (for nicer output).
- */
-errno_t endpoint_send_batch(endpoint_t *ep, usb_target_t target,
-    usb_direction_t direction, char *data, size_t size, uint64_t setup_data,
-    usbhc_iface_transfer_callback_t on_complete, void *arg, const char *name)
-{
-        if (!ep)
-                return EBADMEM;
+ */
+errno_t endpoint_send_batch(endpoint_t *ep, const transfer_request_t *req)
+{
+        assert(ep);
+        assert(req);
 
         if (ep->transfer_type == USB_TRANSFER_CONTROL) {
-                usb_log_debug("%s %d:%d %zu/%zuB, setup %#016" PRIx64, name,
-                    target.address, target.endpoint, size, ep->max_packet_size,
-                    setup_data);
+                usb_log_debug("%s %d:%d %zu/%zuB, setup %#016" PRIx64, req->name,
+                    req->target.address, req->target.endpoint,
+                    req->size, ep->max_packet_size,
+                    req->setup);
         } else {
-                usb_log_debug("%s %d:%d %zu/%zuB", name, target.address,
-                    target.endpoint, size, ep->max_packet_size);
+                usb_log_debug("%s %d:%d %zu/%zuB", req->name,
+                    req->target.address, req->target.endpoint,
+                    req->size, ep->max_packet_size);
         }
 
@@ -244 +237 @@
         }
 
+        size_t size = req->size;
         /*
          * Limit transfers with reserved bandwidth to the amount reserved.
          * OUT transfers are rejected, IN can be just trimmed in advance.
          */
-        if ((ep->transfer_type == USB_TRANSFER_INTERRUPT || ep->transfer_type == USB_TRANSFER_ISOCHRONOUS) && size > ep->max_transfer_size) {
-                if (direction == USB_DIRECTION_OUT)
+        if (size > ep->max_transfer_size &&
+            (ep->transfer_type == USB_TRANSFER_INTERRUPT
+             || ep->transfer_type == USB_TRANSFER_ISOCHRONOUS)) {
+                if (req->dir == USB_DIRECTION_OUT)
                         return ENOSPC;
                 else
                         size = ep->max_transfer_size;
-
         }
 
@@ -266 +261 @@
         }
 
-        batch->target = target;
-        batch->setup.packed = setup_data;
-        batch->dir = direction;
-        batch->buffer_size = size;
-
-        errno_t err;
-        if ((err = usb_transfer_batch_prepare_buffer(batch, data))) {
-                usb_log_warning("Failed to prepare buffer for batch: %s", str_error(err));
-                usb_transfer_batch_destroy(batch);
-                return err;
-        }
-
-        batch->on_complete = on_complete;
-        batch->on_complete_data = arg;
+        batch->target = req->target;
+        batch->setup.packed = req->setup;
+        batch->dir = req->dir;
+        batch->size = size;
+        batch->offset = req->offset;
+        batch->dma_buffer = req->buffer;
+
+        dma_buffer_acquire(&batch->dma_buffer);
+
+        if (batch->offset != 0) {
+                usb_log_debug("A transfer with nonzero offset requested.");
+                usb_transfer_batch_bounce(batch);
+        }
+
+        if (usb_transfer_batch_bounce_required(batch))
+                usb_transfer_batch_bounce(batch);
+
+        batch->on_complete = req->on_complete;
+        batch->on_complete_data = req->arg;
 
         const int ret = ops->batch_schedule(batch);

uspace/lib/usbhost/src/usb_transfer_batch.c

@@ -103 +103 @@
 }
 
+bool usb_transfer_batch_bounce_required(usb_transfer_batch_t *batch)
+{
+        if (!batch->size)
+                return false;
+
+        unsigned flags = batch->dma_buffer.policy & DMA_POLICY_FLAGS_MASK;
+        unsigned required_flags =
+            batch->ep->required_transfer_buffer_policy & DMA_POLICY_FLAGS_MASK;
+
+        if (required_flags & ~flags)
+                return true;
+
+        size_t chunk_mask = dma_policy_chunk_mask(batch->dma_buffer.policy);
+        size_t required_chunk_mask =
+             dma_policy_chunk_mask(batch->ep->required_transfer_buffer_policy);
+
+        /* If the chunks are at least as large as required, we're good */
+        if ((required_chunk_mask & ~chunk_mask) == 0)
+                return false;
+
+        size_t start_chunk = batch->offset & ~chunk_mask;
+        size_t end_chunk = (batch->offset + batch->size - 1) & ~chunk_mask;
+
+        /* The requested area crosses a chunk boundary */
+        if (start_chunk != end_chunk)
+                return true;
+
+        return false;
+}
+
 errno_t usb_transfer_batch_bounce(usb_transfer_batch_t *batch)
 {
@@ -108 +138 @@
         assert(!batch->is_bounced);
 
-        if (dma_buffer_is_set(&batch->dma_buffer))
-                dma_buffer_unlock(&batch->dma_buffer, batch->buffer_size);
+        dma_buffer_release(&batch->dma_buffer);
+
+        batch->original_buffer = batch->dma_buffer.virt + batch->offset;
 
         usb_log_debug("Batch(%p): Buffer cannot be used directly, "
@@ -115 +146 @@
 
         const errno_t err = dma_buffer_alloc_policy(&batch->dma_buffer,
-            batch->buffer_size, batch->ep->transfer_buffer_policy);
+            batch->size, batch->ep->transfer_buffer_policy);
         if (err)
                 return err;
@@ -121 +152 @@
         /* Copy the data out */
         if (batch->dir == USB_DIRECTION_OUT)
-                memcpy(batch->dma_buffer.virt, batch->buffer, batch->buffer_size);
+                memcpy(batch->dma_buffer.virt,
+                    batch->original_buffer,
+                    batch->size);
 
         batch->is_bounced = true;
+        batch->offset = 0;
+
         return err;
-}
-
-/**
- * Prepare a DMA buffer according to endpoint policy.
- *
- * If the buffer is suitable to be used directly, it is. Otherwise, a bounce
- * buffer is created.
- */
-errno_t usb_transfer_batch_prepare_buffer(usb_transfer_batch_t *batch, char *buf)
-{
-        /* Empty transfers do not need a buffer */
-        if (batch->buffer_size == 0)
-                return EOK;
-
-        batch->buffer = buf;
-
-        const dma_policy_t policy = batch->ep->transfer_buffer_policy;
-
-        /*
-         * We don't have enough information (yet, WIP) to know if we can skip
-         * the bounce, so check the conditions carefully.
-         */
-        if (!dma_buffer_check_policy(buf, batch->buffer_size, policy))
-                return usb_transfer_batch_bounce(batch);
-
-        /* Fill the buffer with virtual address and lock it for DMA */
-        return dma_buffer_lock(&batch->dma_buffer, buf, batch->buffer_size);
 }
 
@@ -167 +175 @@
             batch, USB_TRANSFER_BATCH_ARGS(*batch));
 
-        if (batch->error == EOK && batch->buffer_size > 0) {
-                if (!batch->is_bounced) {
-                        /* Unlock the buffer for DMA */
-                        dma_buffer_unlock(&batch->dma_buffer,
-                            batch->buffer_size);
+        if (batch->error == EOK && batch->size > 0) {
+                if (batch->is_bounced) {
+                        /* We we're forced to use bounce buffer, copy it back */
+                        if (batch->dir == USB_DIRECTION_IN)
+                        memcpy(batch->original_buffer,
+                            batch->dma_buffer.virt,
+                            batch->transferred_size);
+
+                        dma_buffer_free(&batch->dma_buffer);
                 }
                 else {
-                        /* We we're forced to use bounce buffer, copy it back */
-                        if (batch->dir == USB_DIRECTION_IN)
-                                memcpy(batch->buffer,
-                                    batch->dma_buffer.virt,
-                                    batch->transferred_size);
-
-                        dma_buffer_free(&batch->dma_buffer);
+                        dma_buffer_release(&batch->dma_buffer);
                 }
         }