fix(esp_mm): for existing mmap region, consider new offset for virtual addr

While returning virtual address for existing memory mapped region, newly
supplied offset from the physical address was not getting considered.

This was a regression present from ESP-IDF 5.1 release.

Added test case in spi_flash component that fails without this fix.

Closes https://github.com/espressif/esp-idf/issues/13929
This commit is contained in:
Mahavir Jain 2024-07-10 15:51:01 +05:30
parent 1fd11f6e0c
commit f5349ca342
No known key found for this signature in database
GPG Key ID: 99324EF4A00734E0
3 changed files with 37 additions and 2 deletions

View File

@ -510,7 +510,15 @@ esp_err_t esp_mmu_map(esp_paddr_t paddr_start, size_t size, mmu_target_t target,
if (is_enclosed) {
ESP_LOGW(TAG, "paddr block is mapped already, vaddr_start: %p, size: 0x%x", (void *)mem_block->vaddr_start, mem_block->size);
*out_ptr = (void *)mem_block->vaddr_start;
/*
* This condition is triggered when `s_is_enclosed` is true and hence
* we are sure that `paddr_start` >= `mem_block->paddr_start`.
*
* Add the offset of new physical address while returning the virtual
* address.
*/
const uint32_t new_paddr_offset = paddr_start - mem_block->paddr_start;
*out_ptr = (void *)mem_block->vaddr_start + new_paddr_offset;
return ESP_ERR_INVALID_STATE;
}

View File

@ -75,7 +75,7 @@ typedef uint32_t esp_paddr_t;
* - ESP_ERR_NOT_SUPPORTED: Only on ESP32, PSRAM is not a supported physical memory target
* - ESP_ERR_NOT_FOUND: No enough size free block to use
* - ESP_ERR_NO_MEM: Out of memory, this API will allocate some heap memory for internal usage
* - ESP_ERR_INVALID_STATE: Paddr is mapped already, this API will return corresponding vaddr_start of the previously mapped block.
* - ESP_ERR_INVALID_STATE: Paddr is mapped already, this API will return corresponding `vaddr_start + new_block_offset` as per the previously mapped block.
* Only to-be-mapped paddr block is totally enclosed by a previously mapped block will lead to this error. (Identical scenario will behave similarly)
* new_block_start new_block_end
* |-------- New Block --------|

View File

@ -101,6 +101,33 @@ static void setup_mmap_tests(void)
}
}
TEST_CASE("Can get correct data in existing mapped region", "[spi_flash][mmap]")
{
setup_mmap_tests();
printf("Mapping %"PRIx32" (+%"PRIx32")\n", start, end - start);
const void *ptr1;
TEST_ESP_OK( spi_flash_mmap(start, end - start, SPI_FLASH_MMAP_DATA, &ptr1, &handle1) );
printf("mmap_res: handle=%"PRIx32" ptr=%p\n", (uint32_t)handle1, ptr1);
/* Remap in the previously mapped region itself */
uint32_t new_start = start + CONFIG_MMU_PAGE_SIZE;
printf("Mapping %"PRIx32" (+%"PRIx32")\n", new_start, end - new_start);
const void *ptr2;
TEST_ESP_OK( spi_flash_mmap(new_start, end - new_start, SPI_FLASH_MMAP_DATA, &ptr2, &handle2) );
printf("mmap_res: handle=%"PRIx32" ptr=%p\n", (uint32_t)handle2, ptr2);
const void *src1 = (void *) ((uint32_t) ptr1 + CONFIG_MMU_PAGE_SIZE);
const void *src2 = ptr2;
/* Memory contents should be identical - as the region is same */
TEST_ASSERT_EQUAL(0, memcmp(src1, src2, end - new_start));
spi_flash_munmap(handle1);
handle1 = 0;
spi_flash_munmap(handle2);
handle2 = 0;
TEST_ASSERT_EQUAL_PTR(NULL, spi_flash_phys2cache(start, SPI_FLASH_MMAP_DATA));
}
TEST_CASE("Can mmap into data address space", "[spi_flash][mmap]")
{