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

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,
>               .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/


-- 
coreboot mailing list: [email protected]
http://www.coreboot.org/mailman/listinfo/coreboot

Reply via email to