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 <[email protected]>

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)


-- 
http://www.hailfinger.org/

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)
-- 
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to