[flashrom] [PATCH] DOS support for flashrom, first try

[Carl-Daniel Hailfinger]

On 09.03.2010 21:06, Carl-Daniel Hailfinger wrote:
> Rudolf Marek added some preliminary DOS support to flashrom. He kindly
> gave me the tarball, and I tried to generate a patch from it. Any
> omissions are my fault.
>
> If anyone wants a big (130 MB) tarball with cross compiler, matching
> libpci and other stuff, just ask me. The server hosting the tarball is
> has a transfer quota, and I'd rather avoid exhausting it with everybody
> downloading the thing in question.
>
> (It's Rudolf's code, so I'll let him sign off on it.)
>   

Review follows.
If anyone feels up to the task, just repost the code with the requested
changes.


> Index: hwaccess.c
> ===================================================================
> --- hwaccess.c	(revision 927)
> +++ hwaccess.c	(working copy)
> @@ -37,7 +37,7 @@
>  #elif defined(__FreeBSD__) || defined (__DragonFly__)
>  	if ((io_fd = open("/dev/io", O_RDWR)) < 0) {
>  #else
> -	if (iopl(3) != 0) {
> +	if (0) {
>  #endif
>   

This needs to be

#elif defined (__MSDOS__)
	if (0) {
#else
	if (iopl(3) != 0) {
#endif



>  		fprintf(stderr, "ERROR: Could not get I/O privileges (%s).\n"
>  			"You need to be root.\n", strerror(errno));
> Index: hwaccess.h
> ===================================================================
> --- hwaccess.h	(revision 927)
> +++ hwaccess.h	(working copy)
> @@ -67,12 +67,21 @@
>    #define INW  inw
>    #define INL  inl
>  #else
>   

Use #elif defined (__MSDOS__) for all changes in this file.


> -  #define OUTB outb
> -  #define OUTW outw
> -  #define OUTL outl
> -  #define INB  inb
> -  #define INW  inw
> -  #define INL  inl
> +//#error "a"
> +
> +//#include <pc.h>
> +
> +  #define OUTB(x,y) outportb(y, x)
> +  #define OUTW(x,y) outportw(y, x)
> +  #define OUTL(x,y) outportl(y, x)
> +
> +
> +  #define INB  inportb
> +  #define INW  inportw
> +  #define INL  inportl
> +
> +
> +
>  #endif
>  #endif
>  
> @@ -89,47 +98,6 @@
>      #endif
>    #include <stdint.h>
>  
> -static inline void
> -outb(uint8_t value, uint16_t port)
> -{
> -	asm volatile ("outb %b0,%w1": :"a" (value), "Nd" (port));
> -}
> -
> -static inline uint8_t
> -inb(uint16_t port)
> -{
> -	uint8_t value;
> -	asm volatile ("inb %w1,%0":"=a" (value):"Nd" (port));
> -	return value;
> -}
> -
> -static inline void
> -outw(uint16_t value, uint16_t port)
> -{
> -	asm volatile ("outw %w0,%w1": :"a" (value), "Nd" (port));
> -}
> -
> -static inline uint16_t
> -inw(uint16_t port)
> -{
> -	uint16_t value;
> -	asm volatile ("inw %w1,%0":"=a" (value):"Nd" (port));
> -	return value;
> -}
> -
> -static inline void
> -outl(uint32_t value, uint16_t port)
> -{
> -	asm volatile ("outl %0,%w1": :"a" (value), "Nd" (port));
> -}
> -
> -static inline uint32_t
> -inl(uint16_t port)
> -{
> -	uint32_t value;
> -	asm volatile ("inl %1,%0":"=a" (value):"Nd" (port));
> -	return value;
> -}
>    #endif
>  #endif
>  
> Index: physmap.c
> ===================================================================
> --- physmap.c	(revision 927)
> +++ physmap.c	(working copy)
> @@ -58,21 +58,38 @@
>  static int fd_mem = -1;
>  static int fd_mem_cached = -1;
>  
>   

Use #if defined (__MSDOS__) for all changes in this file.


> +#define MAP_FAILED 0
> +
> +//***************** call DPMI physical memory mapping and allocate LDT descriptor for the memory block
>   

Comments should use /**/ instead of //


> +void *map_physical_memory(uint32_t phys_base, uint32_t size) // phys_base must be page aligned
> +{							     // size will be aligned automatically
>   

Explanations for phys_base and size should be in a doxygen comment above
the function.


> +  void *p;
> +  if (size == 0)		       // if zero block size, do nothing
>   

Comments above each line, not to the right of the code.


> +    return(NULL);
> +  if ((size & 0x0FFF) != 0)	       // if size is not page-aligned
> +    size = (size + 4096) & (~0x0FFF);  // make it page-aligned
> +  if ((p = valloc(size)) == NULL)      // allocate page-aligned memory (__djgpp_map_physical_memory required this)
> +    {				       // allocation failed
> +    printf("ERROR: cannot allocate %lu bytes for memory mapped device\n",size);
>   

Please use msg_perr instead of printf, try to keep the 80 column limit
by breaking the string apart. gcc can handle stuff like below without
problems:

msg_perr("ERROR: foo bar "
	 "baz bang\n");



> +    exit(-1);			       // exit program
> +    }
> +  if (__djgpp_map_physical_memory(p,size,phys_base))
> +    {				       // failed to map physmem
> +    printf("ERROR: failed to map physical memory range: 0x%08lX - 0x%08lX\nDPMI 1.0 server or support of DPMI function 0508h is needed.\nIt doesn't work under Windows, use win32 version instead.\n",phys_base,phys_base+size-1);
>   

msg_perr instead of printf, 80 column limit.


> +    return MAP_FAILED;			       // exit program
> +    }
> +  return(p);			       // return pointer to memory block aliasing physical memory area
> +}
> +
> +
> +
>  /* For MMIO access. Must be uncached, doesn't make sense to restrict to ro. */
>  void *sys_physmap_rw_uncached(unsigned long phys_addr, size_t len)
>  {
>  	void *virt_addr;
>  
> -	if (-1 == fd_mem) {
> -		/* Open the memory device UNCACHED. Important for MMIO. */
> -		if (-1 == (fd_mem = open(MEM_DEV, O_RDWR | O_SYNC))) {
> -			perror("Critical error: open(" MEM_DEV ")");
> -			exit(2);
> -		}
> -	}
>  
> -	virt_addr = mmap(0, len, PROT_WRITE | PROT_READ, MAP_SHARED,
> -			 fd_mem, (off_t)phys_addr);
> +	virt_addr = map_physical_memory(phys_addr, len)   ;
>  	return MAP_FAILED == virt_addr ? NULL : virt_addr;
>  }
>  
> @@ -83,17 +100,10 @@
>  {
>  	void *virt_addr;
>  
> -	if (-1 == fd_mem_cached) {
> -		/* Open the memory device CACHED. */
> -		if (-1 == (fd_mem_cached = open(MEM_DEV, O_RDWR))) {
> -			perror("Critical error: open(" MEM_DEV ")");
> -			exit(2);
> -		}
> -	}
>  
> -	virt_addr = mmap(0, len, PROT_READ, MAP_SHARED,
> -			 fd_mem_cached, (off_t)phys_addr);
> +	virt_addr = map_physical_memory(phys_addr, len)    ;
>  	return MAP_FAILED == virt_addr ? NULL : virt_addr;
> +
>  }
>  
>  void physunmap(void *virt_addr, size_t len)
> @@ -103,7 +113,7 @@
>  		return;
>  	}
>  		
> -	munmap(virt_addr, len);
> +//	munmap(virt_addr, len);
>  }
>  #endif
>  
>   


> Index: getopt.c
> ===================================================================
> --- getopt.c	(revision 0)
> +++ getopt.c	(revision 0)
> Index: getopt.h
> ===================================================================
> --- getopt.h	(revision 0)
> +++ getopt.h	(revision 0)
>   

A copy of the getopt header. If anything, this needs to be merged
separately, but it would be preferable to just pick it up from some
generic include path.


> Index: pci/pci.h
> ===================================================================
> --- pci/pci.h	(revision 0)
> +++ pci/pci.h	(revision 0)
> Index: pci/types.h
> ===================================================================
> --- pci/types.h	(revision 0)
> +++ pci/types.h	(revision 0)
> Index: pci/config.h
> ===================================================================
> --- pci/config.h	(revision 0)
> +++ pci/config.h	(revision 0)
> Index: pci/header.h
> ===================================================================
> --- pci/header.h	(revision 0)
> +++ pci/header.h	(revision 0)
>   

A copy of the important libpci headers. If anything, this needs to be
merged separately, but it would be preferable to just pick it up from
some generic include path.


> Index: Makefile
> ===================================================================
> --- Makefile	(revision 927)
> +++ Makefile	(working copy)
>   

Superseded by my Makefile patch.

Regards,
Carl-Daniel

-- 
"I do consider assignment statements and pointer variables to be among
computer science's most valuable treasures."
-- Donald E. Knuth