Flash mapping/unmapping was performed without an abstraction layer, so even the dummy flasher caused memory mappings to be set up. Add map/unmap functions to the external flasher abstraction.
Fix a possible scribble-over-low-memory corner case which fortunately never triggered so far.
With this patch, --programmer dummy works fine as non-root.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-external_map_region/flash.h =================================================================== --- flashrom-external_map_region/flash.h (Revision 489) +++ flashrom-external_map_region/flash.h (Arbeitskopie) @@ -87,6 +87,9 @@ int (*init) (void); int (*shutdown) (void);
+ void * (*map_flash_region) (const char *descr, unsigned long phys_addr, size_t len); + void (*unmap_flash_region) (void *virt_addr, size_t len); + void (*chip_writeb) (uint8_t val, volatile void *addr); void (*chip_writew) (uint16_t val, volatile void *addr); void (*chip_writel) (uint32_t val, volatile void *addr); @@ -107,6 +110,16 @@ return programmer_table[programmer].shutdown(); }
+static inline void *programmer_map_flash_region(const char *descr, unsigned long phys_addr, size_t len) +{ + return programmer_table[programmer].map_flash_region(descr, phys_addr, len); +} + +static inline void programmer_unmap_flash_region(void *virt_addr, size_t len) +{ + programmer_table[programmer].unmap_flash_region(virt_addr, len); +} + static inline void chip_writeb(uint8_t val, volatile void *addr) { programmer_table[programmer].chip_writeb(val, addr); @@ -579,6 +592,8 @@ /* dummyflasher.c */ int dummy_init(void); int dummy_shutdown(void); +void *dummy_map(const char *descr, unsigned long phys_addr, size_t len); +void dummy_unmap(void *virt_addr, size_t len); void dummy_chip_writeb(uint8_t val, volatile void *addr); void dummy_chip_writew(uint16_t val, volatile void *addr); void dummy_chip_writel(uint32_t val, volatile void *addr); Index: flashrom-external_map_region/physmap.c =================================================================== --- flashrom-external_map_region/physmap.c (Revision 489) +++ flashrom-external_map_region/physmap.c (Arbeitskopie) @@ -69,12 +69,34 @@
void physunmap(void *virt_addr, size_t len) { + if (len == 0) { + fprintf(stderr, "Not unmapping zero size at %p\n", + virt_addr); + return; + } + munmap(virt_addr, len); } #endif
void *physmap(const char *descr, unsigned long phys_addr, size_t len) { + if (len == 0) { + fprintf(stderr, "Not mapping %s, zero size at 0x%08lx\n", + descr, phys_addr); + return NULL; + } + + if ((getpagesize() - 1) & len) { + fprintf(stderr, "Mapping %s at 0x%08lx, unaligned size 0x%lx\n", + descr, phys_addr, (unsigned long)len); + } + + if ((getpagesize() - 1) & phys_addr) { + fprintf(stderr, "Mapping %s, 0x%lx bytes at unaligned 0x%08lx\n", + descr, (unsigned long)len, phys_addr); + } + void *virt_addr = sys_physmap(phys_addr, len);
if (NULL == virt_addr) { Index: flashrom-external_map_region/dummyflasher.c =================================================================== --- flashrom-external_map_region/dummyflasher.c (Revision 489) +++ flashrom-external_map_region/dummyflasher.c (Arbeitskopie) @@ -40,6 +40,19 @@ return 0; }
+void *dummy_map(const char *descr, unsigned long phys_addr, size_t len) +{ + printf("%s: Mapping %s, 0x%lx bytes at 0x%08lx\n", + __func__, descr, (unsigned long)len, phys_addr); + return (void *)phys_addr; +} + +void dummy_unmap(void *virt_addr, size_t len) +{ + printf("%s: Unmapping 0x%lx bytes at %p\n", + __func__, (unsigned long)len, virt_addr); +} + void dummy_chip_writeb(uint8_t val, volatile void *addr) { printf("%s: addr=%p, val=0x%02x\n", __func__, addr, val); Index: flashrom-external_map_region/flashrom.c =================================================================== --- flashrom-external_map_region/flashrom.c (Revision 489) +++ flashrom-external_map_region/flashrom.c (Arbeitskopie) @@ -40,6 +40,8 @@ { .init = internal_init, .shutdown = internal_shutdown, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .chip_readb = internal_chip_readb, .chip_readw = internal_chip_readw, .chip_readl = internal_chip_readl, @@ -51,6 +53,8 @@ { .init = dummy_init, .shutdown = dummy_shutdown, + .map_flash_region = dummy_map, + .unmap_flash_region = dummy_unmap, .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -65,7 +69,7 @@ void map_flash_registers(struct flashchip *flash) { size_t size = flash->total_size * 1024; - flash->virtual_registers = physmap("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size); + flash->virtual_registers = programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size); }
int read_memmapped(struct flashchip *flash, uint8_t *buf) @@ -97,23 +101,8 @@
size = flash->total_size * 1024;
- /* If getpagesize() > size -> - * "Can't mmap memory using /dev/mem: Invalid argument" - * This should never happen as we don't support any flash chips - * smaller than 4k or 8k (yet). - */ - - if (getpagesize() > size) { - /* - * if a flash size of 0 is mapped, we map a single page - * so we can probe in that area whether we know the - * vendor at least. - */ - size = getpagesize(); - } - base = flashbase ? flashbase : (0xffffffff - size + 1); - flash->virtual_memory = physmap("flash chip", base, size); + flash->virtual_memory = programmer_map_flash_region("flash chip", base, size);
if (force) break; @@ -126,7 +115,8 @@ break;
notfound: - physunmap((void *)flash->virtual_memory, size); + /* The intermediate cast to unsigned long works around a gcc warning bug. */ + programmer_unmap_flash_region((void *)(unsigned long)flash->virtual_memory, size); }
if (!flash || !flash->name)
On 09.05.2009 14:06, Carl-Daniel Hailfinger wrote:
Flash mapping/unmapping was performed without an abstraction layer, so even the dummy flasher caused memory mappings to be set up. Add map/unmap functions to the external flasher abstraction.
Fix a possible scribble-over-low-memory corner case which fortunately never triggered so far.
With this patch, --programmer dummy works fine as non-root.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
If this gets an Ack, I'll send a dummy SPI flasher driver next.
Regards, Carl-Daniel
Index: flashrom-external_map_region/flash.h
--- flashrom-external_map_region/flash.h (Revision 489) +++ flashrom-external_map_region/flash.h (Arbeitskopie) @@ -87,6 +87,9 @@ int (*init) (void); int (*shutdown) (void);
- void * (*map_flash_region) (const char *descr, unsigned long phys_addr, size_t len);
- void (*unmap_flash_region) (void *virt_addr, size_t len);
- void (*chip_writeb) (uint8_t val, volatile void *addr); void (*chip_writew) (uint16_t val, volatile void *addr); void (*chip_writel) (uint32_t val, volatile void *addr);
@@ -107,6 +110,16 @@ return programmer_table[programmer].shutdown(); }
+static inline void *programmer_map_flash_region(const char *descr, unsigned long phys_addr, size_t len) +{
- return programmer_table[programmer].map_flash_region(descr, phys_addr, len);
+}
+static inline void programmer_unmap_flash_region(void *virt_addr, size_t len) +{
- programmer_table[programmer].unmap_flash_region(virt_addr, len);
+}
static inline void chip_writeb(uint8_t val, volatile void *addr) { programmer_table[programmer].chip_writeb(val, addr); @@ -579,6 +592,8 @@ /* dummyflasher.c */ int dummy_init(void); int dummy_shutdown(void); +void *dummy_map(const char *descr, unsigned long phys_addr, size_t len); +void dummy_unmap(void *virt_addr, size_t len); void dummy_chip_writeb(uint8_t val, volatile void *addr); void dummy_chip_writew(uint16_t val, volatile void *addr); void dummy_chip_writel(uint32_t val, volatile void *addr); Index: flashrom-external_map_region/physmap.c =================================================================== --- flashrom-external_map_region/physmap.c (Revision 489) +++ flashrom-external_map_region/physmap.c (Arbeitskopie) @@ -69,12 +69,34 @@
void physunmap(void *virt_addr, size_t len) {
- if (len == 0) {
fprintf(stderr, "Not unmapping zero size at %p\n",virt_addr);return;- }
- munmap(virt_addr, len);
} #endif
void *physmap(const char *descr, unsigned long phys_addr, size_t len) {
if (len == 0) {
fprintf(stderr, "Not mapping %s, zero size at 0x%08lx\n",descr, phys_addr);return NULL;}
if ((getpagesize() - 1) & len) {
fprintf(stderr, "Mapping %s at 0x%08lx, unaligned size 0x%lx\n",descr, phys_addr, (unsigned long)len);}
if ((getpagesize() - 1) & phys_addr) {
fprintf(stderr, "Mapping %s, 0x%lx bytes at unaligned 0x%08lx\n",descr, (unsigned long)len, phys_addr);}
void *virt_addr = sys_physmap(phys_addr, len);
if (NULL == virt_addr) {
Index: flashrom-external_map_region/dummyflasher.c
--- flashrom-external_map_region/dummyflasher.c (Revision 489) +++ flashrom-external_map_region/dummyflasher.c (Arbeitskopie) @@ -40,6 +40,19 @@ return 0; }
+void *dummy_map(const char *descr, unsigned long phys_addr, size_t len) +{
- printf("%s: Mapping %s, 0x%lx bytes at 0x%08lx\n",
__func__, descr, (unsigned long)len, phys_addr);- return (void *)phys_addr;
+}
+void dummy_unmap(void *virt_addr, size_t len) +{
- printf("%s: Unmapping 0x%lx bytes at %p\n",
__func__, (unsigned long)len, virt_addr);+}
void dummy_chip_writeb(uint8_t val, volatile void *addr) { printf("%s: addr=%p, val=0x%02x\n", __func__, addr, val); Index: flashrom-external_map_region/flashrom.c =================================================================== --- flashrom-external_map_region/flashrom.c (Revision 489) +++ flashrom-external_map_region/flashrom.c (Arbeitskopie) @@ -40,6 +40,8 @@ { .init = internal_init, .shutdown = internal_shutdown,
.map_flash_region = physmap,.unmap_flash_region = physunmap,@@ -51,6 +53,8 @@ { .init = dummy_init, .shutdown = dummy_shutdown,
.map_flash_region = dummy_map,.unmap_flash_region = dummy_unmap,@@ -65,7 +69,7 @@ void map_flash_registers(struct flashchip *flash) { size_t size = flash->total_size * 1024;
- flash->virtual_registers = physmap("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
- flash->virtual_registers = programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size);
}
int read_memmapped(struct flashchip *flash, uint8_t *buf) @@ -97,23 +101,8 @@
size = flash->total_size * 1024;
/* If getpagesize() > size ->* "Can't mmap memory using /dev/mem: Invalid argument"* This should never happen as we don't support any flash chips* smaller than 4k or 8k (yet).*/if (getpagesize() > size) {/** if a flash size of 0 is mapped, we map a single page* so we can probe in that area whether we know the* vendor at least.*/size = getpagesize();}- base = flashbase ? flashbase : (0xffffffff - size + 1);
flash->virtual_memory = physmap("flash chip", base, size);
flash->virtual_memory = programmer_map_flash_region("flash chip", base, size);if (force) break;
@@ -126,7 +115,8 @@ break;
notfound:
physunmap((void *)flash->virtual_memory, size);
/* The intermediate cast to unsigned long works around a gcc warning bug. */programmer_unmap_flash_region((void *)(unsigned long)flash->virtual_memory, size);}
if (!flash || !flash->name)
On 09.05.2009 22:25, Carl-Daniel Hailfinger wrote:
On 09.05.2009 14:06, Carl-Daniel Hailfinger wrote:
Flash mapping/unmapping was performed without an abstraction layer, so even the dummy flasher caused memory mappings to be set up. Add map/unmap functions to the external flasher abstraction.
Fix a possible scribble-over-low-memory corner case which fortunately never triggered so far.
With this patch, --programmer dummy works fine as non-root.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks to Uwe for testing and reviewing.
New version: - Downgrade zero size mapping warning from stderr to printf_debug.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Index: flashrom-external_map_region/flash.h =================================================================== --- flashrom-external_map_region/flash.h (Revision 491) +++ flashrom-external_map_region/flash.h (Arbeitskopie) @@ -87,6 +87,9 @@ int (*init) (void); int (*shutdown) (void);
+ void * (*map_flash_region) (const char *descr, unsigned long phys_addr, size_t len); + void (*unmap_flash_region) (void *virt_addr, size_t len); + void (*chip_writeb) (uint8_t val, volatile void *addr); void (*chip_writew) (uint16_t val, volatile void *addr); void (*chip_writel) (uint32_t val, volatile void *addr); @@ -107,6 +110,16 @@ return programmer_table[programmer].shutdown(); }
+static inline void *programmer_map_flash_region(const char *descr, unsigned long phys_addr, size_t len) +{ + return programmer_table[programmer].map_flash_region(descr, phys_addr, len); +} + +static inline void programmer_unmap_flash_region(void *virt_addr, size_t len) +{ + programmer_table[programmer].unmap_flash_region(virt_addr, len); +} + static inline void chip_writeb(uint8_t val, volatile void *addr) { programmer_table[programmer].chip_writeb(val, addr); @@ -579,6 +592,8 @@ /* dummyflasher.c */ int dummy_init(void); int dummy_shutdown(void); +void *dummy_map(const char *descr, unsigned long phys_addr, size_t len); +void dummy_unmap(void *virt_addr, size_t len); void dummy_chip_writeb(uint8_t val, volatile void *addr); void dummy_chip_writew(uint16_t val, volatile void *addr); void dummy_chip_writel(uint32_t val, volatile void *addr); Index: flashrom-external_map_region/physmap.c =================================================================== --- flashrom-external_map_region/physmap.c (Revision 491) +++ flashrom-external_map_region/physmap.c (Arbeitskopie) @@ -69,12 +69,34 @@
void physunmap(void *virt_addr, size_t len) { + if (len == 0) { + printf_debug("Not unmapping zero size at %p\n", + virt_addr); + return; + } + munmap(virt_addr, len); } #endif
void *physmap(const char *descr, unsigned long phys_addr, size_t len) { + if (len == 0) { + printf_debug("Not mapping %s, zero size at 0x%08lx\n", + descr, phys_addr); + return NULL; + } + + if ((getpagesize() - 1) & len) { + fprintf(stderr, "Mapping %s at 0x%08lx, unaligned size 0x%lx\n", + descr, phys_addr, (unsigned long)len); + } + + if ((getpagesize() - 1) & phys_addr) { + fprintf(stderr, "Mapping %s, 0x%lx bytes at unaligned 0x%08lx\n", + descr, (unsigned long)len, phys_addr); + } + void *virt_addr = sys_physmap(phys_addr, len);
if (NULL == virt_addr) { Index: flashrom-external_map_region/dummyflasher.c =================================================================== --- flashrom-external_map_region/dummyflasher.c (Revision 491) +++ flashrom-external_map_region/dummyflasher.c (Arbeitskopie) @@ -40,6 +40,19 @@ return 0; }
+void *dummy_map(const char *descr, unsigned long phys_addr, size_t len) +{ + printf("%s: Mapping %s, 0x%lx bytes at 0x%08lx\n", + __func__, descr, (unsigned long)len, phys_addr); + return (void *)phys_addr; +} + +void dummy_unmap(void *virt_addr, size_t len) +{ + printf("%s: Unmapping 0x%lx bytes at %p\n", + __func__, (unsigned long)len, virt_addr); +} + void dummy_chip_writeb(uint8_t val, volatile void *addr) { printf("%s: addr=%p, val=0x%02x\n", __func__, addr, val); Index: flashrom-external_map_region/flashrom.c =================================================================== --- flashrom-external_map_region/flashrom.c (Revision 491) +++ flashrom-external_map_region/flashrom.c (Arbeitskopie) @@ -40,6 +40,8 @@ { .init = internal_init, .shutdown = internal_shutdown, + .map_flash_region = physmap, + .unmap_flash_region = physunmap, .chip_readb = internal_chip_readb, .chip_readw = internal_chip_readw, .chip_readl = internal_chip_readl, @@ -51,6 +53,8 @@ { .init = dummy_init, .shutdown = dummy_shutdown, + .map_flash_region = dummy_map, + .unmap_flash_region = dummy_unmap, .chip_readb = dummy_chip_readb, .chip_readw = dummy_chip_readw, .chip_readl = dummy_chip_readl, @@ -65,7 +69,7 @@ void map_flash_registers(struct flashchip *flash) { size_t size = flash->total_size * 1024; - flash->virtual_registers = physmap("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size); + flash->virtual_registers = programmer_map_flash_region("flash chip registers", (0xFFFFFFFF - 0x400000 - size + 1), size); }
int read_memmapped(struct flashchip *flash, uint8_t *buf) @@ -97,23 +101,8 @@
size = flash->total_size * 1024;
- /* If getpagesize() > size -> - * "Can't mmap memory using /dev/mem: Invalid argument" - * This should never happen as we don't support any flash chips - * smaller than 4k or 8k (yet). - */ - - if (getpagesize() > size) { - /* - * if a flash size of 0 is mapped, we map a single page - * so we can probe in that area whether we know the - * vendor at least. - */ - size = getpagesize(); - } - base = flashbase ? flashbase : (0xffffffff - size + 1); - flash->virtual_memory = physmap("flash chip", base, size); + flash->virtual_memory = programmer_map_flash_region("flash chip", base, size);
if (force) break; @@ -126,7 +115,8 @@ break;
notfound: - physunmap((void *)flash->virtual_memory, size); + /* The intermediate cast to unsigned long works around a gcc warning bug. */ + programmer_unmap_flash_region((void *)(unsigned long)flash->virtual_memory, size); }
if (!flash || !flash->name)
On Mon, May 11, 2009 at 01:07:11PM +0200, Carl-Daniel Hailfinger wrote:
With this patch, --programmer dummy works fine as non-root.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks to Uwe for testing and reviewing.
New version:
- Downgrade zero size mapping warning from stderr to printf_debug.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Uwe Hermann uwe@hermann-uwe.de
with the small changes below.
Index: flashrom-external_map_region/physmap.c
--- flashrom-external_map_region/physmap.c (Revision 491) +++ flashrom-external_map_region/physmap.c (Arbeitskopie) @@ -69,12 +69,34 @@
void physunmap(void *virt_addr, size_t len) {
- if (len == 0) {
printf_debug("Not unmapping zero size at %p\n",virt_addr);
virt_addr can be on the same line as printf_debug, no need for wrapping.
Index: flashrom-external_map_region/flashrom.c
--- flashrom-external_map_region/flashrom.c (Revision 491) +++ flashrom-external_map_region/flashrom.c (Arbeitskopie) @@ -40,6 +40,8 @@ { .init = internal_init, .shutdown = internal_shutdown,
.map_flash_region = physmap,.unmap_flash_region = physunmap,
Maybe realign the other entries with tabs, so they're vertically aligned.
Uwe.
On 11.05.2009 16:01, Uwe Hermann wrote:
On Mon, May 11, 2009 at 01:07:11PM +0200, Carl-Daniel Hailfinger wrote:
With this patch, --programmer dummy works fine as non-root.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Thanks to Uwe for testing and reviewing.
New version:
- Downgrade zero size mapping warning from stderr to printf_debug.
Signed-off-by: Carl-Daniel Hailfinger c-d.hailfinger.devel.2006@gmx.net
Acked-by: Uwe Hermann uwe@hermann-uwe.de
Thanks, committed in r493.
with the small changes below.
Index: flashrom-external_map_region/physmap.c
--- flashrom-external_map_region/physmap.c (Revision 491) +++ flashrom-external_map_region/physmap.c (Arbeitskopie) @@ -69,12 +69,34 @@
void physunmap(void *virt_addr, size_t len) {
- if (len == 0) {
printf_debug("Not unmapping zero size at %p\n",virt_addr);virt_addr can be on the same line as printf_debug, no need for wrapping.
Fixed.
Index: flashrom-external_map_region/flashrom.c
--- flashrom-external_map_region/flashrom.c (Revision 491) +++ flashrom-external_map_region/flashrom.c (Arbeitskopie) @@ -40,6 +40,8 @@ { .init = internal_init, .shutdown = internal_shutdown,
.map_flash_region = physmap,.unmap_flash_region = physunmap,Maybe realign the other entries with tabs, so they're vertically aligned.
Done.
Regards, Carl-Daniel
-
Carl-Daniel Hailfinger -
Uwe Hermann