Merge pull request #2 from badgeteam/sprite/fix_appfs_bootloader

Fix appfs bootloader code

factory_test/bootloader_components/main/appfs_flashfunctions_wrapper.c

@@ -24,6 +24,9 @@ in the loader segment instead of in random IRAM.
 #include "soc/soc_memory_types.h"
 #include "soc/soc_caps.h"
 #include <string.h>
+#include "soc/dport_reg.h"
+#include "esp32/rom/cache.h"
+
 
 static const char *TAG="appfs_wrapper";
 
@@ -51,7 +54,7 @@ static bool was_mmapped_to_appfs=false;
 
 IRAM_ATTR const void *__wrap_bootloader_mmap(uint32_t src_addr, uint32_t size) {
 	if (file_handle!=APPFS_INVALID_FD && src_addr>=ovl_start && src_addr+size<ovl_start+ovl_size) {
-		ESP_LOGI(TAG, "__wrap_bootloader_mmap: redirecting map to 0x%X", src_addr);
+		ESP_LOGD(TAG, "__wrap_bootloader_mmap: redirecting map to 0x%X", src_addr);
 		uint8_t *f=appfsBlMmap(file_handle);
 		return &f[src_addr-ovl_start];
 	} else {
@@ -61,6 +64,7 @@ IRAM_ATTR const void *__wrap_bootloader_mmap(uint32_t src_addr, uint32_t size) {
 
 IRAM_ATTR void __wrap_bootloader_munmap(const void *mapping) {
 	if (file_handle!=APPFS_INVALID_FD && was_mmapped_to_appfs) {
+		ESP_LOGD(TAG, "__wrap_bootloader_munmap");
 		appfsBlMunmap();
 		was_mmapped_to_appfs=false;
 	} else {
@@ -71,11 +75,8 @@ IRAM_ATTR void __wrap_bootloader_munmap(const void *mapping) {
 
 IRAM_ATTR esp_err_t __wrap_bootloader_flash_read(size_t src_addr, void *dest, size_t size, bool allow_decrypt) {
 	if (file_handle!=APPFS_INVALID_FD && src_addr>=ovl_start && src_addr+size<ovl_start+ovl_size) {
-		uint8_t *f=appfsBlMmap(file_handle);
+		ESP_LOGD(TAG, "__wrap_bootloader_flash_read: 0x%X->0x%X, %d bytes", src_addr, (int)dest, size);
-		ESP_LOGI(TAG, "__wrap_bootloader_flash_read: 0x%X->0x%X, %d bytes", src_addr, (int)dest, size);
+		return appfs_bootloader_read(file_handle, src_addr-ovl_start, dest, size);
-		memcpy(dest, &f[src_addr-ovl_start], size);
-		appfsBlMunmap();
-		return ESP_OK;
 	} else {
 		return __real_bootloader_flash_read(src_addr, dest, size, allow_decrypt);
 	}
@@ -90,13 +91,25 @@ IRAM_ATTR static bool should_map(uint32_t load_addr) {
 //mapping. That is done, but with wrong addresses. We need to re-do that here and then call into
 //the app.
 static IRAM_ATTR void mmap_and_start_app() {
-	ESP_LOGI(TAG, "mmap_and_start_app()");
+	ESP_LOGD(TAG, "mmap_and_start_app()");
+	//First, check if we actually need to do this. If loading the appfs app failed (e.g. because it
+	//got corrupted), the previous routine will fall back to e.g. the factory app. If we would
+	//adjust the MMU assuming the appfs app had loaded, we would crash.
+	//Note that this is ESP32-specific.
+	for (int i = 0; i < DPORT_FLASH_MMU_TABLE_SIZE; i++) {
+		if (DPORT_PRO_FLASH_MMU_TABLE[i] != DPORT_FLASH_MMU_TABLE_INVALID_VAL) {
+			int page=DPORT_PRO_FLASH_MMU_TABLE[i]&255;
+			int addr=page*0x10000;
+			if (addr<ovl_start || addr>ovl_start+ovl_size) {
+				ESP_LOGI(TAG, "Not booting appfs app; not adjusting mmu.");
+				return;
+			}
+		}
+	}
+
 	//Undo bootloader mapping. If we don't call this, the rest of the code thinks there's still
 	//something mapped. Note that for now the address doesn't matter, we feed it 0.
 	__real_bootloader_munmap(0);
-	//Appfs is not gonna like that its metadata is not mmapped. We call this routine as what it does
-	//'under the hood' is clear the mmu and reset it to mmap only the appfs meta info.
-	appfsBlMunmap();
 
 	//Map the executable file so we can read its header.
 	uint8_t *appBytes=appfsBlMmap(file_handle);
@@ -123,11 +136,11 @@ static IRAM_ATTR void mmap_and_start_app() {
 		p+=l;
 	}
 
-	ESP_LOGI(TAG, "Unmap");
+	ESP_LOGD(TAG, "Unmap");
 	appfsBlMunmap();
 	appfsBlMapRegions(file_handle, mapRegions, noMaps);
 
-	ESP_LOGD(TAG, "start: 0x%08x", entry_addr);
+	ESP_LOGD(TAG, "Appfs MMU adjustments done. Starting app at 0x%08x", entry_addr);
 	typedef void (*entry_t)(void);
 	entry_t entry = ((entry_t) entry_addr);
 	(*entry)();
@@ -156,4 +169,9 @@ IRAM_ATTR void appfs_bootloader_munmap(const void *mapping) {
 	return __real_bootloader_munmap(mapping);
 }
 
+IRAM_ATTR esp_err_t appfs_bootloader_flash_read(size_t src_addr, void *dest, size_t size, bool allow_decrypt)  {
+	return __real_bootloader_flash_read(src_addr, dest, size, allow_decrypt);
+}
+
+
 #endif

factory_test/bootloader_components/main/bootloader_start.c

@@ -109,6 +109,9 @@ void __attribute__((noreturn)) call_start_cpu0(void)
 
 	ESP_LOGI(TAG, "Wrapping flash functions and booting app...");
 	appfs_wrapper_init(handle, appfs_pos, appfs_len);
+	//De-init the high-level parts of appfs. Reading/mmap'ping a file handle still is explicitly
+	//allowed after this, though.
+	appfsBlDeinit();
 	//Note that the rest of the bootloader code has no clue about appfs, and as such won't try
 	//to boot it. We 'fix' that by chucking the appfs partition (which is now wrapped so the rest
 	//of the bootloader reads from the selected file when it thinks it loads from the app) into

factory_test/components/appfs/appfs.c

@@ -255,6 +255,13 @@ IRAM_ATTR __attribute__ ((weak)) void appfs_bootloader_munmap(const void *mappin
 	bootloader_munmap(mapping);
 }
 
+IRAM_ATTR __attribute__ ((weak)) esp_err_t appfs_bootloader_flash_read(size_t src_addr, void *dest, size_t size, bool allow_decrypt)  {
+	return bootloader_flash_read(src_addr, dest, size, allow_decrypt);
+}
+
+static uint8_t next_page_for[256];
+static int keep_meta_mapped=0;
+
 esp_err_t appfsBlInit(uint32_t offset, uint32_t len) {
 	//Compile-time sanity check on size of structs
 	_Static_assert(sizeof(AppfsHeader)==APPFS_META_DESC_SZ, "sizeof AppfsHeader != 128bytes");
@@ -268,30 +275,36 @@ esp_err_t appfsBlInit(uint32_t offset, uint32_t len) {
 		ESP_LOGE(TAG, "No valid meta info found. Bailing out.");
 		return ESP_ERR_NOT_FOUND;
 	}
+	for (int i=0; i<256; i++) {
+		next_page_for[i]=appfsMeta[appfsActiveMeta].page[i].next;
+	}
 	appfsPartOffset=offset;
+	keep_meta_mapped=1;
 	ESP_LOGD(TAG, "Initialized.");
 	return ESP_OK;
 }
 
 void appfsBlDeinit() {
 	ESP_LOGI(TAG, "Appfs deinit");
-	appfs_bootloader_munmap(appfsMeta);
+	if (appfsMeta) appfs_bootloader_munmap(appfsMeta);
+	appfsMeta=NULL;
+	keep_meta_mapped=0;
 }
 
 #define MMU_BLOCK0_VADDR  0x3f400000
 #define MMU_BLOCK50_VADDR 0x3f720000
 
 IRAM_ATTR esp_err_t appfsBlMapRegions(int fd, AppfsBlRegionToMap *regions, int noRegions) {
-	if (appfsMeta==NULL) ESP_LOGE(TAG, "EEK! appfsBlMapRegions called without meta mapped");
 	uint8_t pages[255];
-	int pageCt=0;
+	int page_count=0;
 	int page=fd;
 	do {
-		pages[pageCt++]=page;
+		pages[page_count++]=page;
-		page=appfsMeta[appfsActiveMeta].page[page].next;
+		page=next_page_for[page];
-	} while (page!=0);
+	} while (page!=0 && page_count<255);
-	//Okay, we have our info.
+	
-	appfs_bootloader_munmap(appfsMeta);
+	if (appfsMeta) appfs_bootloader_munmap(appfsMeta);
+	appfsMeta=NULL;
 	
 	Cache_Read_Disable( 0 );
 	Cache_Flush( 0 );
@@ -314,6 +327,7 @@ IRAM_ATTR esp_err_t appfsBlMapRegions(int fd, AppfsBlRegionToMap *regions, int n
 			}
 			d+=APPFS_SECTOR_SZ;
 			p++;
+			if (p>page_count) return ESP_ERR_NO_MEM;
 		}
 	}
 	DPORT_REG_CLR_BIT( DPORT_PRO_CACHE_CTRL1_REG, (DPORT_PRO_CACHE_MASK_IRAM0) | (DPORT_PRO_CACHE_MASK_IRAM1 & 0) | (DPORT_PRO_CACHE_MASK_IROM0 & 0) | DPORT_PRO_CACHE_MASK_DROM0 | DPORT_PRO_CACHE_MASK_DRAM1 );
@@ -323,39 +337,25 @@ IRAM_ATTR esp_err_t appfsBlMapRegions(int fd, AppfsBlRegionToMap *regions, int n
 }
 
 IRAM_ATTR void* appfsBlMmap(int fd) {
-	//We want to mmap() the pages of the file into memory. However, to do that we need to kill the mmap for the 
+	if (appfsMeta) appfs_bootloader_munmap(appfsMeta);
-	//meta info. To do this, we collect the pages before unmapping the meta info.
+	appfsMeta=NULL;
-	if (appfsMeta==NULL) ESP_LOGE(TAG, "EEK! appfsBlMmap called without meta mapped");
-	uint8_t pages[255];
-	int pageCt=0;
-	int page=fd;
-	do {
-		pages[pageCt++]=page;
-		page=appfsMeta[appfsActiveMeta].page[page].next;
-	} while (page!=0);
-//	ESP_LOGI(TAG, "File %d has %d pages.", fd, pageCt);
-	
-	if (pageCt>50) {
-		ESP_LOGE(TAG, "appfsBlMmap: file too big to mmap");
-		return NULL;
-	}
-	
-	//Okay, we have our info.
-	appfs_bootloader_munmap(appfsMeta);
 	//Bootloader_mmap only allows mapping of one consecutive memory range. We need more than that, so we essentially
 	//replicate the function here.
 	
 	Cache_Read_Disable(0);
 	Cache_Flush(0);
-	for (int i=0; i<pageCt; i++) {
+	int page=fd;
+	for (int i=0; i<50; i++) {
 //		ESP_LOGI(TAG, "Mapping flash addr %X to mem addr %X for page %d", appfsPartOffset+((pages[i]+1)*APPFS_SECTOR_SZ), MMU_BLOCK0_VADDR+(i*APPFS_SECTOR_SZ), pages[i]);
 		int e = cache_flash_mmu_set(0, 0, MMU_BLOCK0_VADDR+(i*APPFS_SECTOR_SZ), 
-						appfsPartOffset+((pages[i]+1)*APPFS_SECTOR_SZ), 64, 1);
+						appfsPartOffset+((page+1)*APPFS_SECTOR_SZ), 64, 1);
 		if (e != 0) {
 			ESP_LOGE(TAG, "cache_flash_mmu_set failed: %d", e);
 			Cache_Read_Enable(0);
 			return NULL;
 		}
+		page=next_page_for[page];
+		if (page==0) break;
 	}
 	Cache_Read_Enable(0);
 	return (void *)(MMU_BLOCK0_VADDR);
@@ -366,8 +366,34 @@ IRAM_ATTR void appfsBlMunmap() {
 	Cache_Read_Disable(0);
 	Cache_Flush(0);
 	mmu_init(0);
-	//Map meta page
+	if (keep_meta_mapped) {
-	appfsMeta=appfs_bootloader_mmap(appfsPartOffset, APPFS_SECTOR_SZ);
+		appfsMeta=appfs_bootloader_mmap(appfsPartOffset, APPFS_SECTOR_SZ);
+	}
+}
+
+IRAM_ATTR esp_err_t appfs_bootloader_read(int fd, size_t src_addr, void *dest, size_t size)  {
+	int page=fd;
+	int pos=0;
+	int have_read=0;
+	uint8_t *destp=(uint8_t*)dest;
+	int offset_in_page=src_addr&(APPFS_SECTOR_SZ-1);
+	for (int i=0; i<255; i++) {
+		if (pos+APPFS_SECTOR_SZ-1>=src_addr) {
+			size_t rsize=APPFS_SECTOR_SZ-offset_in_page;
+			if (rsize>size) rsize=size;
+			esp_err_t r=appfs_bootloader_flash_read(appfsPartOffset+((page+1)*APPFS_SECTOR_SZ)+offset_in_page, destp+have_read, rsize, true);
+			if (r!=ESP_OK) return r;
+			offset_in_page=0;
+			have_read+=rsize;
+			if (have_read>=size) {
+				return ESP_OK;
+			}
+		}
+		page=next_page_for[page];
+		if (page==0) break;
+		pos+=APPFS_SECTOR_SZ;
+	}
+	return ESP_OK;
 }
 
 #else //so if !BOOTLOADER_BUILD

factory_test/components/appfs/appfs.h

@@ -242,7 +242,8 @@ typedef struct {
 esp_err_t appfsBlInit(uint32_t offset, uint32_t len);
 
 /**
- * @brief Bootloader only: de-init appfs
+ * @brief Bootloader only: de-init appfs. Note that if you have a file handle, you can still
+ *        read/mmap it. This guarantees that the metadata for the appfs is not mapped anymore.
  */
 void appfsBlDeinit();
 /**
@@ -262,6 +263,18 @@ void* appfsBlMmap(int fd);
  */
 void appfsBlMunmap();
 
+
+/**
+ * @brief Bootloader only: read data from a file
+ *
+ * @param fs File descriptor to read from
+ * @param src_addr Offset in file
+ * @param dest Dest buffer
+ * @param size Length to read
+ * @return ESP_OK if OK
+ */
+esp_err_t appfs_bootloader_read(int fd, size_t src_addr, void *dest, size_t size);
+
 /*
  * @brief Bootloader only: map multiple regions within a file to various memory addressed.
  *