Merge branch 'fix/usb_comprehensive_heap' into 'master'

fix(usb/host): Do not call heap_caps_get_allocated_size() in USB host driver

Closes IDFGH-15138

See merge request espressif/esp-idf!38715

Author: tore-espressif

components/usb/hcd_dwc.c

@@ -16,21 +16,24 @@
 #include "esp_err.h"
 #include "esp_log.h"
 
+#include "soc/soc_caps.h"
 #include "soc/usb_dwc_periph.h"
 #include "hal/usb_dwc_hal.h"
 #include "hcd.h"
 #include "usb_private.h"
 #include "usb/usb_types_ch9.h"
 
-#include "soc/soc_caps.h"
-#if SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
 #include "esp_cache.h"
-#endif // SOC_CACHE_INTERNAL_MEM_VIA_L1CACHE
+#include "esp_private/esp_cache_private.h"
 
 // ----------------------------------------------------- Macros --------------------------------------------------------
 
+#define ALIGN_UP(num, align)    ((align) == 0 ? (num) : (((num) + ((align) - 1)) & ~((align) - 1)))
+
 // --------------------- Constants -------------------------
 
+#define XFER_DESC_LIST_CAPS                     (MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED | MALLOC_CAP_INTERNAL)
+
 #define INIT_DELAY_MS                           30  // A delay of at least 25ms to enter Host mode. Make it 30ms to be safe
 #define DEBOUNCE_DELAY_MS                       CONFIG_USB_HOST_DEBOUNCE_DELAY_MS
 #define RESET_HOLD_MS                           CONFIG_USB_HOST_RESET_HOLD_MS
@@ -1518,16 +1521,18 @@ static dma_buffer_block_t *buffer_block_alloc(usb_transfer_type_t type)
     }
 
     // Transfer descriptor list: Must be 512 aligned and DMA capable (USB-DWC requirement) and its size must be cache aligned
-    void *xfer_desc_list = heap_caps_aligned_calloc(USB_DWC_QTD_LIST_MEM_ALIGN, desc_list_len * sizeof(usb_dwc_ll_dma_qtd_t), 1, MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED | MALLOC_CAP_INTERNAL);
+    void *xfer_desc_list = heap_caps_aligned_calloc(USB_DWC_QTD_LIST_MEM_ALIGN, desc_list_len * sizeof(usb_dwc_ll_dma_qtd_t), 1, XFER_DESC_LIST_CAPS);
     if (xfer_desc_list == NULL) {
         free(buffer);
         heap_caps_free(xfer_desc_list);
         return NULL;
     }
     buffer->xfer_desc_list = xfer_desc_list;
-    // For targets with L1CACHE, the allocated size might be bigger than requested, this value is than used during memory sync
-    // We save this value here, so we don't have to call 'heap_caps_get_allocated_size()' during every memory sync
-    buffer->xfer_desc_list_len_bytes = heap_caps_get_allocated_size(xfer_desc_list);
+
+    // Note for developers: We do not use heap_caps_get_allocated_size() because it is broken with HEAP_POISONING=COMPREHENSIVE
+    size_t cache_align = 0;
+    esp_cache_get_alignment(XFER_DESC_LIST_CAPS, &cache_align);
+    buffer->xfer_desc_list_len_bytes = ALIGN_UP(desc_list_len * sizeof(usb_dwc_ll_dma_qtd_t), cache_align);
     return buffer;
 }
 

components/usb/include/usb/usb_host.h

@@ -569,6 +569,7 @@ esp_err_t usb_host_endpoint_clear(usb_device_handle_t dev_hdl, uint8_t bEndpoint
  *
  * - This function allocates a transfer object
  * - Each transfer object has a fixed sized buffer specified on allocation
+ * - The resulting data_buffer_size can be bigger that the requested size. This is to ensure that the data buffer is cache aligned
  * - A transfer object can be re-used indefinitely
  * - A transfer can be submitted using usb_host_transfer_submit() or usb_host_transfer_submit_control()
  *

components/usb/private_include/usb_private.h

@@ -77,7 +77,8 @@ typedef bool (*usb_proc_req_cb_t)(usb_proc_req_source_t source, bool in_isr, voi
  *
  * - Data buffer is allocated in DMA capable memory
  * - The constant fields of the URB are also set
- * - The data_buffer field of the URB is set to point to start of the allocated data buffer.
+ * - The data_buffer field of the URB is set to point to start of the allocated data buffer
+ * - The resulting data_buffer_size can be bigger that the requested size. This is to ensure that the data buffer is cache aligned
  *
  * @param[in] data_buffer_size Size of the URB's data buffer
  * @param[in] num_isoc_packets Number of isochronous packet descriptors

components/usb/test_apps/common/CMakeLists.txt

@@ -7,4 +7,6 @@ idf_component_register(SRCS "dev_hid.c"
                        INCLUDE_DIRS
                              "../../private_include" # hcd and urb
                              "."
-                       REQUIRES usb unity)
+                       REQUIRES usb unity
+                       PRIV_REQUIRES esp_mm
+                       )

components/usb/test_apps/common/hcd_common.c

@@ -260,18 +260,27 @@ void test_hcd_pipe_free(hcd_pipe_handle_t pipe_hdl)
     vQueueDelete(pipe_evt_queue);
 }
 
+#include "esp_private/esp_cache_private.h"
+#define DATA_BUFFER_CAPS        (MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED)
+#define ALIGN_UP(num, align)    ((align) == 0 ? (num) : (((num) + ((align) - 1)) & ~((align) - 1)))
+
 urb_t *test_hcd_alloc_urb(int num_isoc_packets, size_t data_buffer_size)
 {
     // Allocate a URB and data buffer
     urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (sizeof(usb_isoc_packet_desc_t) * num_isoc_packets), MALLOC_CAP_DEFAULT);
-    void *data_buffer = heap_caps_malloc(data_buffer_size, MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED);
+
+    size_t cache_align = 0;
+    esp_cache_get_alignment(DATA_BUFFER_CAPS, &cache_align);
+    data_buffer_size = ALIGN_UP(data_buffer_size, cache_align);
+    void *data_buffer = heap_caps_malloc(data_buffer_size, DATA_BUFFER_CAPS);
+
     TEST_ASSERT_NOT_NULL_MESSAGE(urb, "Failed to allocate URB");
     TEST_ASSERT_NOT_NULL_MESSAGE(data_buffer, "Failed to allocate transfer buffer");
 
     // Initialize URB and underlying transfer structure. Need to cast to dummy due to const fields
     usb_transfer_dummy_t *transfer_dummy = (usb_transfer_dummy_t *)&urb->transfer;
     transfer_dummy->data_buffer = data_buffer;
-    transfer_dummy->data_buffer_size = heap_caps_get_allocated_size(data_buffer);
+    transfer_dummy->data_buffer_size = data_buffer_size;
     transfer_dummy->num_isoc_packets = num_isoc_packets;
     return urb;
 }

components/usb/usb_private.c

@@ -1,32 +1,40 @@
 /*
- * SPDX-FileCopyrightText: 2015-2024 Espressif Systems (Shanghai) CO LTD
+ * SPDX-FileCopyrightText: 2015-2025 Espressif Systems (Shanghai) CO LTD
  *
  * SPDX-License-Identifier: Apache-2.0
  */
 
+#include "sdkconfig.h"
 #include "esp_heap_caps.h"
 #include "usb_private.h"
 #include "usb/usb_types_ch9.h"
 
+#if !CONFIG_IDF_TARGET_LINUX
+#include "esp_private/esp_cache_private.h"
+#define ALIGN_UP(num, align)    ((align) == 0 ? (num) : (((num) + ((align) - 1)) & ~((align) - 1)))
+#endif
+#define DATA_BUFFER_CAPS        (MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED)
+
 urb_t *urb_alloc(size_t data_buffer_size, int num_isoc_packets)
 {
     urb_t *urb = heap_caps_calloc(1, sizeof(urb_t) + (sizeof(usb_isoc_packet_desc_t) * num_isoc_packets), MALLOC_CAP_DEFAULT);
-    void *data_buffer = heap_caps_malloc(data_buffer_size, MALLOC_CAP_DMA | MALLOC_CAP_CACHE_ALIGNED);
+
+#if !CONFIG_IDF_TARGET_LINUX
+    // Note for developers: We do not use heap_caps_get_allocated_size() because it is broken with HEAP_POISONING=COMPREHENSIVE
+    size_t cache_align = 0;
+    esp_cache_get_alignment(DATA_BUFFER_CAPS, &cache_align);
+    data_buffer_size = ALIGN_UP(data_buffer_size, cache_align);
+#endif
+
+    void *data_buffer = heap_caps_malloc(data_buffer_size, DATA_BUFFER_CAPS);
     if (urb == NULL || data_buffer == NULL)     {
         goto err;
     }
 
-#if CONFIG_IDF_TARGET_LINUX
-    // heap_caps_get_allocated_size() return 0 on Linux target
-    const size_t allocated_size = data_buffer_size;
-#else
-    const size_t allocated_size = heap_caps_get_allocated_size(data_buffer);
-#endif
-
     // Cast as dummy transfer so that we can assign to const fields
     usb_transfer_dummy_t *dummy_transfer = (usb_transfer_dummy_t *)&urb->transfer;
     dummy_transfer->data_buffer = data_buffer;
-    dummy_transfer->data_buffer_size = allocated_size;
+    dummy_transfer->data_buffer_size = data_buffer_size;
     dummy_transfer->num_isoc_packets = num_isoc_packets;
     return urb;
 err: