Re: Wrong format of kernel.img if build on Cygwin (was: no subject)

2010-09-04 Thread Christian Franke

Lode Leroy wrote:




 Did the core.img actually work as expected then?


unfortunately no, it doesn't boot.
booting the iso from grub-mkrescue in qemu I get the following message :

Booting from CD-Rom...
1MB medium detected
rub_env_seteub_exitub_fatal
===  (these letters in reverse video)



This is possibly the problem that libbfd cannot not convert PE to ELF 
properly.

Unfortunately the grub-pe2elf tool does support pe-i386 but not pei-i386.

I will try to fix this next week.

--
Regards,
Christian Franke


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Wrong format of kernel.img if build on Cygwin (was: no subject)

2010-09-02 Thread Christian Franke
lode leroy wrote:
  I tried compiling grub from bzr on cygwin.
 
 grub-mkimage failed with the following error:
 
 ~/grub/build/grub-core $ ../grub-mkimage -v -O i386-pc -d . -o
 core.img biosdisk part_msdos fat ntfs
 ../grub-mkimage: info: getting the size of ./biosdisk.mod.
 [...]
 ../grub-mkimage: info: getting the size of ./kernel.img.
 ../grub-mkimage: error: invalid ELF header.
 

In the past (1.98), kernel.img was a raw binary. Now it is an ELF file
which requires extra conversion if build on Cygwin.

I will provide a patch to fix Cygwin build for 1.99.


 I found that the problem can be solved by replacing the following
 line in the Makefile
 
 strip -v -R .rel.dyn -R .reginfo -R .note -R .comment  -o kernel.img
 kernel.exec.exe
 
 by
 
 strip -Felf32-i386 -v -R .rel.dyn -R .reginfo -R .note -R .comment 
 -o kernel.img kernel.exec.exe


Did the core.img actually work as expected then?


Thanks for the problem report.

-- 
Regards,
Christian Franke




___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Build fails due to wrong placement of -llibrary options

2010-08-28 Thread Christian Franke
I'm trying to fix the build on Cygwin after the recent move to the new 
autogen/automake build system.


There is one Cygwin independent issue found so far:

$ make
[...]
gcc [...] -lintl -o grub-bin2h.exe util/grub_bin2h-bin2h.o libgrub.a
libgrub.a(...): undefined reference to `_libintl_gettext'
[...]

Workaround:

$ make LIBS=-lintl
[...]
gcc [...] -lintl -o grub-bin2h.exe util/grub_bin2h-bin2h.o libgrub.a -lintl


The placement of -llibrary options matter because the linker searches 
libraries and objects in the order specified.


$ grep ^grub_bin2h_LD Makefile
grub_bin2h_LDADD = libgrub.a
grub_bin2h_LDFLAGS = $(AM_LDFLAGS) $(LDFLAGS_PROGRAM) $(LIBINTL) 
$(LIBDEVMAPPER)


The above should be:
grub_bin2h_LDADD = libgrub.a $(LIBINTL) $(LIBDEVMAPPER)
grub_bin2h_LDFLAGS = $(AM_LDFLAGS) $(LDFLAGS_PROGRAM)

Same for all other utils. This likely needs to be fixed in gentpl.py.


--
Regards,
Christian Franke


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Build fails due to wrong placement of -llibrary options

2010-08-28 Thread Christian Franke

BVK Chaitanya wrote:

On Sat, Aug 28, 2010 at 5:53 PM, Christian Franke
...  wrote:
   

$ grep ^grub_bin2h_LD Makefile
grub_bin2h_LDADD = libgrub.a
grub_bin2h_LDFLAGS = $(AM_LDFLAGS) $(LDFLAGS_PROGRAM) $(LIBINTL)
$(LIBDEVMAPPER)

The above should be:
grub_bin2h_LDADD = libgrub.a $(LIBINTL) $(LIBDEVMAPPER)
grub_bin2h_LDFLAGS = $(AM_LDFLAGS) $(LDFLAGS_PROGRAM)

Same for all other utils. This likely needs to be fixed in gentpl.py.

 

It is not necessary to modify gentpl.py, fixing in Makefile.util.def
should be sufficient.  Attached patch fixes all these AFAICS.

   


With the patch it works, thanks! Please apply.

--
Regards,
Christian Franke


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: grub2 build under windows environment.

2010-07-28 Thread Christian Franke
BVK Chaitanya wrote:
 GRUB2 must build fine on cygwin.  You need to have necessary
 development packages installed.
 

There are also binary and source packages for GRUB 1.98 available in the
Cygwin distro (http://cygwin.com/packages/). The file
/usr/share/doc/Cygwin/grub.README file provides a list of necessary
development packages.

-- 
Regards,
Christian Franke




___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Optimise memset on i386

2010-07-23 Thread Christian Franke

richardvoigt wrote:


might I suggest:

unsigned long patternl = pattern8;
patternl |= patternl  8;
patternl |= patternl  16;
patternl |= patternl  32;
patternl |= patternl  64;

O(lg N) instead of O(N), no loop, no branches, and the compiler should 
be smart enough to optimize away the last two lines on systems with 
narrower long.


The latter is unfortunately not the case. At least gcc 4.5.0 prints a 
warning but still produces code.


$ cat EOF f.c
unsigned long f(unsigned long x)
{
  x |= x  32;
  x |= x  64;
  return x;
}

$ gcc -O3 -S f.c
x.c: In function ‘f’:
x.c:3: warning: left shift count = width of type
x.c:4: warning: left shift count = width of type

$ cat f.s
...
pushl   %ebp
movl$32, %ecx
movl%esp, %ebp
movl8(%ebp), %eax
popl%ebp
movl%eax, %edx
sall%cl, %edx
movl$64, %ecx
orl %eax, %edx
movl%edx, %eax
sall%cl, %eax
orl %edx, %eax
ret


--
Regards,
Christian Franke


___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix grub.d/10_windows for Cygwin 1.7

2010-05-09 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Christian Franke wrote:
   

This change is already included in Cygwin package grub-1.98-2.

 

-
+  test -z $needmap || catEOF
+   drivemap -s (hd0) \$root
+EOF
You can't be sure on script runtime that C: is on hd0. Ironically especially 
when drivemap is used.
But drivemap -s (hd0) (hd0) is a nop, so no harm done.
On the other hand Vista and 7 are incompatible with drivemap so you have to 
skip drivemap on those systems. Look at 30_os-prober.in:
   case ${LONGNAME} in
 Windows\ Vista*|Windows\ 7*)
 ;;
 *)
   cat  EOF
 drivemap -s (hd0) \${root}
EOF
 ;;
   esac
   


This is already considered in the script: $needmap is set if ntldr is 
found but not if bootmgr is found.



--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Fix grub.d/10_windows for Cygwin 1.7

2010-05-05 Thread Christian Franke




This change is already included in Cygwin package grub-1.98-2.

-- 
Regards,
Christian Franke



=== modified file 'ChangeLog'
--- ChangeLog	2010-05-05 19:19:55 +
+++ ChangeLog	2010-05-05 20:09:01 +
@@ -1,3 +1,9 @@
+2010-05-05  Christian Franke  fra...@computer.org
+
+	* util/grub.d/10_windows.in: Use path names instead of
+	drive letters to prevent warning from Cygwin 1.7.
+	Add drivemap command to menuentry if needed.
+
 2010-05-05  Vladimir Serbinenko  phco...@gmail.com
 
 	* video/readers/jpeg.c: Indented.

=== modified file 'util/grub.d/10_windows.in'
--- util/grub.d/10_windows.in	2009-11-20 08:41:20 +
+++ util/grub.d/10_windows.in	2010-05-05 12:22:43 +
@@ -28,8 +28,8 @@
 
 # Try C: even if current system is on other partition.
 case $SYSTEMDRIVE in
-  [Cc]:) dirlist=C:  ;;
-  [D-Zd-z]:) dirlist=C: $SYSTEMDRIVE ;;
+  [Cc]:) drives=C:  ;;
+  [D-Zd-z]:) drives=C: $SYSTEMDRIVE ;;
   *) exit 0 ;;
 esac
 
@@ -51,7 +51,13 @@
 }
 
 
-for dir in $dirlist ; do
+for drv in $drives ; do
+
+  # Convert to Cygwin path.
+  dir=`cygpath $drv`
+  test -n $dir || continue
+
+  needmap=
 
   # Check for Vista bootmgr.
   if [ -f $dir/bootmgr -a -f $dir/boot/bcd ] ; then
@@ -60,6 +66,7 @@
   # Check for NTLDR.
   elif [ -f $dir/ntldr -a -f $dir/ntdetect.com -a -f $dir/boot.ini ] ; then
 OS=`get_os_name_from_boot_ini $dir/boot.ini` || OS=Windows NT/2000/XP loader
+needmap=t
 
   else
 continue
@@ -68,14 +75,16 @@
   # Get boot /dev/ice.
   dev=`${grub_probe} -t device $dir 2/dev/null` || continue
 
-  echo Found $OS on $dir ($dev) 2
+  echo Found $OS on $drv ($dev) 2
   cat  EOF
 menuentry $OS {
 EOF
 
   save_default_entry | sed -e 's,^,\t,'
   prepare_grub_to_access_device $dev | sed 's,^,\t,'
-
+  test -z $needmap || cat EOF
+	drivemap -s (hd0) \$root
+EOF
   cat  EOF
 	chainloader +1
 }

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix Cygwin path handling

2010-04-26 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Christian Franke wrote:
   
 

+ /* Include leading slash.  */
+ offset = 0;
+ break;
Why do you make this change? It seems to make the behaviour of 
make_path_relative_to_its root inconsistent since E.g.
/boot/grub -  grub
/etc -  /etc
   


Could not reproduce. Old and new code always include the leading slash 
(except / - '').


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix Cygwin path handling

2010-04-26 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:


I actually seen the '/' case and have incorrectly assumed it's always
so. However there is still one case when you patch changes the behaviour:
$ grub-mkrelpath /

$ ./grub-mkrelpath /
/
Although:
$ ./grub-mkrelpath /mnt/boot/

$ grub-mkrelpath /mnt/boot/


   


I actually did not test the / corner case, sorry. New patch attached.

--
Regards,
Christian Franke

=== modified file 'ChangeLog'
--- ChangeLog	2010-04-26 01:35:55 +
+++ ChangeLog	2010-04-26 09:44:43 +
@@ -1,5 +1,17 @@
 2010-04-26  Christian Franke  fra...@computer.org
 
+	* util/grub-mkconfig_lib.in (make_system_path_relative_to_its_root):
+	Remove broken Cygwin path conversion.
+	* util/misc.c: [__CYGWIN__] Add include and define.
+	[__CYGWIN__] (get_win32_path): Copy function from getroot.c, modify
+	for Cygwin 1.7.
+	(make_system_path_relative_to_its_root): Simplify loop, replace early
+	return by break.
+	[__CYGWIN__] Add conversion to win32 path.
+	Include / case in trailing slash removal.
+
+2010-04-26  Christian Franke  fra...@computer.org
+
 	* include/grub/util/getroot.h (grub_get_prefix): Remove prototype.
 	* util/getroot.c [__CYGWIN__] (get_win32_path): Remove function.
 	(grub_get_prefix): Remove function.

=== modified file 'util/grub-mkconfig_lib.in'
--- util/grub-mkconfig_lib.in	2010-04-19 19:25:41 +
+++ util/grub-mkconfig_lib.in	2010-04-26 08:59:10 +
@@ -1,5 +1,5 @@
 # Helper library for grub-mkconfig
-# Copyright (C) 2007,2008,2009  Free Software Foundation, Inc.
+# Copyright (C) 2007,2008,2009,2010  Free Software Foundation, Inc.
 #
 # GRUB is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -44,21 +44,7 @@
 
 make_system_path_relative_to_its_root ()
 {
-  path=`${grub_mkrelpath} $1`
-
-  case `uname 2/dev/null` in
-CYGWIN*)
-  # Cygwin: Check if regular or emulated mount.
-  if [ -z $dir ] || [ `stat -c %D $dir/..` != 62 ] ; then
-# Reached some mount point not below /cygdrive.
-# GRUB does not know Cygwin's emulated mounts,
-# convert to Win32 path and remove drive letter.
-path=`cygpath -m $path | sed -n 's,^[A-Za-z]:,,p'`
-test ! -z $path || return 1
-  fi ;;
-  esac
-
-  echo $path
+  ${grub_mkrelpath} $1
 }
 
 is_path_readable_by_grub ()

=== modified file 'util/misc.c'
--- util/misc.c	2010-04-09 23:25:46 +
+++ util/misc.c	2010-04-26 10:08:04 +
@@ -53,6 +53,11 @@
 # include malloc.h
 #endif
 
+#ifdef __CYGWIN__
+# include sys/cygwin.h
+# define DEV_CYGDRIVE_MAJOR 98
+#endif
+
 #ifdef __MINGW32__
 #include windows.h
 #include winioctl.h
@@ -456,6 +461,27 @@
   return ret;
 }
 
+#ifdef __CYGWIN__
+/* Convert POSIX path to Win32 path,
+   remove drive letter, replace backslashes.  */
+static char *
+get_win32_path (const char *path)
+{
+  char winpath[PATH_MAX];
+  if (cygwin_conv_path (CCP_POSIX_TO_WIN_A, path, winpath, sizeof(winpath)))
+grub_util_error (cygwin_conv_path() failed);
+
+  int len = strlen (winpath);
+  int offs = (len  2  winpath[1] == ':' ? 2 : 0);
+
+  int i;
+  for (i = offs; i  len; i++)
+if (winpath[i] == '\\')
+  winpath[i] = '/';
+  return xstrdup (winpath + offs);
+}
+#endif
+
 /* This function never prints trailing slashes (so that its output
can be appended a slash unconditionally).  */
 char *
@@ -521,30 +547,31 @@
   /* offset == 1 means root directory.  */
   if (offset == 1)
 	{
-	  free (buf);
-	  len = strlen (buf2);
-	  while (buf2[len - 1] == '/'  len  1)
-	{
-	  buf2[len - 1] = '\0';
-	  len--;
-	}
-	  if (len  1)
-	return buf2;
-	  else
-	{
-	  /* This means path given is just a backslash.  As above
-		 we have to return an empty string.  */
-	  free (buf2);
-	  return xstrdup ();
-	}
+	  /* Include leading slash.  */
+	  offset = 0;
+	  break;
 	}
 }
   free (buf);
   buf3 = xstrdup (buf2 + offset);
   free (buf2);
 
+#ifdef __CYGWIN__
+  if (st.st_dev != (DEV_CYGDRIVE_MAJOR  16))
+{
+  /* Reached some mount point not below /cygdrive.
+	 GRUB does not know Cygwin's emulated mounts,
+	 convert to Win32 path.  */
+  grub_util_info (Cygwin path = %s\n, buf3);
+  char * temp = get_win32_path (buf3);
+  free (buf3);
+  buf3 = temp;
+}
+#endif
+
+  /* Remove trailing slashes, return empty string if root directory.  */
   len = strlen (buf3);
-  while (buf3[len - 1] == '/'  len  1)
+  while (len  0  buf3[len - 1] == '/')
 {
   buf3[len - 1] = '\0';
   len--;

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: NEED_REGISTER_FRAME_INFO can be replaced by -static-libgcc

2010-04-19 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Christian Franke wrote:
   


According to 'gcc -dumpspecs' of Cygwin gcc 4.3.4-3, options '-u
___[de]register_frame_info' are always passed to the linker if
-static-libgcc is not specified. This is Cygwin and MinGW specific: In
the exe startup code these symbols are loaded only if present, so the
DLL must be forced to load first.

As a consequence, the symbols are also set undefined when the GRUB
pre-*.o files are generated with 'ld -r'.

The attached patch works for me with the bzr revision preceding the
NEED_REGISTER_FRAME_INFO fix.

 

Doesn't it risk linking e.g. __bswapsi2 into every module which uses it
during partial link? It would increase module size on RISC
   


Partial linking with '-Wl,-r' does never pull any code from libraries. 
Specifying '-lgcc' has no effect in this case.


Cannot test this on RISC, it is at least true on i686-linux, -freebsd 
and -cygwin.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Remove grub_get_prefix()

2010-04-18 Thread Christian Franke
This patch removes grub_get_prefix() and replaces the two remaining 
calls with make_system_path_relative_to_its_root().

Already discussed in thread [PATCH] Fix Cygwin path handling.

Change in util/grub-emu.c was tested, but not the change in 
util/sparc64/ieee1275/grub-setup.c.


--
Regards,
Christian Franke

=== modified file 'ChangeLog'
--- ChangeLog	2010-04-18 11:00:06 +
+++ ChangeLog	2010-04-18 14:39:41 +
@@ -1,3 +1,12 @@
+2010-04-18  Christian Franke  fra...@computer.org
+
+	* include/grub/util/getroot.h (grub_get_prefix): Remove prototype.
+	* util/getroot.c [__CYGWIN__] (get_win32_path): Remove function.
+	(grub_get_prefix): Remove function.
+	* util/grub-emu.c (main): Replace grub_get_prefix () call by
+	make_system_path_relative_to_its_root ().
+	* util/sparc64/ieee1275/grub-setup.c (main): Likewise.
+
 2010-04-18  Vladimir Serbinenko  phco...@gmail.com
 
 	* po/POTFILES: Renamed multiboot_loader.c to multiboot.c
@@ -10,7 +19,7 @@
 
 	* disk/lvm.c (grub_lvm_memberlist): Issue an error if pv-disk = 0.
 
-2010-04-17  Christian Franke fra...@computer.org
+2010-04-17  Christian Franke  fra...@computer.org
 
 	* Makefile.in: Add missing localedir setting.
 

=== modified file 'include/grub/util/getroot.h'
--- include/grub/util/getroot.h	2009-04-11 09:40:39 +
+++ include/grub/util/getroot.h	2010-04-18 14:19:18 +
@@ -26,7 +26,6 @@
 };
 
 char *grub_guess_root_device (const char *dir);
-char *grub_get_prefix (const char *dir);
 int grub_util_get_dev_abstraction (const char *os_dev);
 char *grub_util_get_grub_dev (const char *os_dev);
 const char *grub_util_check_block_device (const char *blk_dev);

=== modified file 'util/getroot.c'
--- util/getroot.c	2010-04-10 15:07:33 +
+++ util/getroot.c	2010-04-18 14:21:03 +
@@ -79,103 +79,6 @@
   return path;
 }
 
-#ifdef __CYGWIN__
-/* Convert POSIX path to Win32 path,
-   remove drive letter, replace backslashes.  */
-static char *
-get_win32_path (const char *path)
-{
-  char winpath[PATH_MAX];
-  cygwin_conv_to_full_win32_path (path, winpath);
-
-  int len = strlen (winpath);
-  if (len  2  winpath[1] == ':')
-{
-  len -= 2;
-  memmove (winpath, winpath + 2, len + 1);
-}
-
-  int i;
-  for (i = 0; i  len; i++)
-if (winpath[i] == '\\')
-  winpath[i] = '/';
-  return xstrdup (winpath);
-}
-#endif
-
-char *
-grub_get_prefix (const char *dir)
-{
-  char *saved_cwd;
-  char *abs_dir, *prev_dir;
-  char *prefix;
-  struct stat st, prev_st;
-
-  /* Save the current directory.  */
-  saved_cwd = xgetcwd ();
-
-  if (chdir (dir)  0)
-grub_util_error (cannot change directory to `%s', dir);
-
-  abs_dir = xgetcwd ();
-  strip_extra_slashes (abs_dir);
-  prev_dir = xstrdup (abs_dir);
-
-  if (stat (., prev_st)  0)
-grub_util_error (cannot stat `%s', dir);
-
-  if (! S_ISDIR (prev_st.st_mode))
-grub_util_error (`%s' is not a directory, dir);
-
-  while (1)
-{
-  if (chdir (..)  0)
-	grub_util_error (cannot change directory to the parent);
-
-  if (stat (., st)  0)
-	grub_util_error (cannot stat current directory);
-
-  if (! S_ISDIR (st.st_mode))
-	grub_util_error (current directory is not a directory???);
-
-  if (prev_st.st_dev != st.st_dev || prev_st.st_ino == st.st_ino)
-	break;
-
-  free (prev_dir);
-  prev_dir = xgetcwd ();
-  prev_st = st;
-}
-
-  strip_extra_slashes (prev_dir);
-  prefix = xmalloc (strlen (abs_dir) - strlen (prev_dir) + 2);
-  prefix[0] = '/';
-  strcpy (prefix + 1, abs_dir + strlen (prev_dir));
-  strip_extra_slashes (prefix);
-
-  if (chdir (saved_cwd)  0)
-grub_util_error (cannot change directory to `%s', dir);
-
-#ifdef __CYGWIN__
-  if (st.st_dev != (DEV_CYGDRIVE_MAJOR  16))
-{
-  /* Reached some mount point not below /cygdrive.
-	 GRUB does not know Cygwin's emulated mounts,
-	 convert to Win32 path.  */
-  grub_util_info (Cygwin prefix = %s, prefix);
-  char * wprefix = get_win32_path (prefix);
-  free (prefix);
-  prefix = wprefix;
-}
-#endif
-
-  free (saved_cwd);
-  free (abs_dir);
-  free (prev_dir);
-
-  grub_util_info (prefix = %s, prefix);
-  return prefix;
-}
-
 #ifdef __MINGW32__
 
 static char *

=== modified file 'util/grub-emu.c'
--- util/grub-emu.c	2010-02-07 16:52:11 +
+++ util/grub-emu.c	2010-04-18 14:38:28 +
@@ -242,7 +242,7 @@
   if (strcmp (root_dev, host) == 0)
 dir = xstrdup (dir);
   else
-dir = grub_get_prefix (dir);
+dir = make_system_path_relative_to_its_root (dir);
   prefix = xmalloc (strlen (root_dev) + 2 + strlen (dir) + 1);
   sprintf (prefix, (%s)%s, root_dev, dir);
   free (dir);

=== modified file 'util/sparc64/ieee1275/grub-setup.c'
--- util/sparc64/ieee1275/grub-setup.c	2010-01-16 00:26:52 +
+++ util/sparc64/ieee1275/grub-setup.c	2010-04-18 14:38:02 +
@@ -635,7 +635,8 @@
 
   find_dest_dev (ginfo, argv);
 
-  ginfo.prefix = grub_get_prefix (ginfo.dir ? : DEFAULT_DIRECTORY);
+  ginfo.prefix = make_system_path_relative_to_its_root (ginfo.dir

Re: [PATCH] Fix Cygwin path handling

2010-04-17 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Christian Franke wrote:
   

The Cywin path handling is broken since
make_system_path_relative_to_its_root() functionality was moved from
the lib script to misc.c.

This patch should fix this. It reuses the Cygwin specific code from
getroot.c:grub_get_prefix() which apparently is a different
implementation of the same function.

I would suggest to remove grub_get_prefix(), it is now only used in
grub-emu.c and sparc64/ieee1275/grub-setup.c. Not included in the
patch, should be done in a separate commit.


2010-04-14  Christian Frankefra...@computer.org

 * util/grub-mkconfig_lib.in (make_system_path_relative_to_its_root):
 Remove broken Cygwin path conversion.
 * util/misc.c: [__CYGWIN__] Add include and define.
 [__CYGWIN__] (get_win32_path): Copy function from getroot.c, modify
 for Cygwin 1.7.
 

Please avoid duplicating code. Rather than that rename get_win32_path to
grub_get_win32_path and remove static attribute
   


Normally I would have done that but duplication was intentional in this 
case:
The getroot.c:get_win32_path() can later be removed together with 
grub_get_prefix(), see my suggestion above. The patch takes this into 
account and adds new private misc.c:get_win32_path() and so avoids 
unnecessary temporary changes to misc.h and getroot.c.


The actual code duplication happened when 
misc.c:make_system_path_relative_to_its_root() was added instead of 
moving and reusing getroot.c:grub_get_prefix() :-)



BTW: My last commits to grub codebase were before the move to bzr.

As far as I understand Bazaar workflow for GRUB 
(http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00175.html) 
such changes should be 'bzr push'ed to e.g. '.../branches/feature-foo' 
(e.g. '.../branches/cygwin-path' in this case) after review has finished.


Is this workflow still valid or is there a more current document?

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: NEED_REGISTER_FRAME_INFO can be replaced by -static-libgcc

2010-04-14 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Christian Franke wrote:
   


The *_frame_info symbols are set undefined to force linkage of the
libgcc_s shared library or dll.

This can be prevented by TARGET_LDFLAGS=-static-libgcc. To build from
grub-1.98 tarball on Cygwin, run configure with this argument.

May also be necessary on other build platforms when -shared-libgcc is
the default. It may be possible simply set -static-libgcc
unconditionally.

 

We already supply -lgcc --static-libgcc. Do you know why it still had
issues?
   


The above is only set when linking the kernel image.

According to 'gcc -dumpspecs' of Cygwin gcc 4.3.4-3, options '-u 
___[de]register_frame_info' are always passed to the linker if 
-static-libgcc is not specified. This is Cygwin and MinGW specific: In 
the exe startup code these symbols are loaded only if present, so the 
DLL must be forced to load first.


As a consequence, the symbols are also set undefined when the GRUB 
pre-*.o files are generated with 'ld -r'.


The attached patch works for me with the bzr revision preceding the 
NEED_REGISTER_FRAME_INFO fix.




And I also doubt usefullness of pulling these functions since reference
to them is purely dummy: no relocation uses it so it will only increase
code size. Another concern is the behviour of these functions in grub
environment if they ever get called.

   


AFAICS from the gcc source, the functions are used in EH stack unwinding 
code in libgcc. The compiler itself does not generate calls to these 
functions.



--
Regards,
Christian Franke

=== modified file 'Makefile.in'
--- Makefile.in	2010-04-10 23:14:31 +
+++ Makefile.in	2010-04-14 08:04:34 +
@@ -98,12 +98,12 @@
 OBJCONV = @OBJCONV@
 TARGET_CPPFLAGS = @TARGET_CPPFLAGS@ -I$(srcdir)/include -I$(builddir) -I$(builddir)/include \
 	-Wall -W
-TARGET_LDFLAGS = -nostdlib @TARGET_LDFLAGS@
+TARGET_LDFLAGS = -nostdlib -static-libgcc @TARGET_LDFLAGS@
 TARGET_IMG_LDSCRIPT = @TARGET_IMG_LDSCRIPT@
 TARGET_IMG_LDFLAGS = -nostdlib @TARGET_IMG_LDFLAGS@
 TARGET_IMG_CFLAGS = @TARGET_IMG_CFLAGS@
 TARGET_OBJ2ELF = @TARGET_OBJ2ELF@
-kernel_img_LDFLAGS = -static-libgcc -lgcc
+kernel_img_LDFLAGS = -lgcc
 EXEEXT = @EXEEXT@
 OBJCOPY = @OBJCOPY@
 STRIP = @STRIP@

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Fix Cygwin path handling

2010-04-14 Thread Christian Franke
The Cywin path handling is broken since 
make_system_path_relative_to_its_root() functionality was moved from the 
lib script to misc.c.


This patch should fix this. It reuses the Cygwin specific code from 
getroot.c:grub_get_prefix() which apparently is a different 
implementation of the same function.


I would suggest to remove grub_get_prefix(), it is now only used in 
grub-emu.c and sparc64/ieee1275/grub-setup.c. Not included in the patch, 
should be done in a separate commit.



2010-04-14  Christian Frankefra...@computer.org

* util/grub-mkconfig_lib.in (make_system_path_relative_to_its_root):
Remove broken Cygwin path conversion.
* util/misc.c: [__CYGWIN__] Add include and define.
[__CYGWIN__] (get_win32_path): Copy function from getroot.c, modify
for Cygwin 1.7.
(make_system_path_relative_to_its_root): Simplify loop, replace early
return by break.
[__CYGWIN__] Add conversion to win32 path.



--
Regards,
Christian Franke

=== modified file 'util/grub-mkconfig_lib.in'
--- util/grub-mkconfig_lib.in	2010-04-13 12:57:56 +
+++ util/grub-mkconfig_lib.in	2010-04-14 16:48:44 +
@@ -1,5 +1,5 @@
 # Helper library for grub-mkconfig
-# Copyright (C) 2007,2008,2009  Free Software Foundation, Inc.
+# Copyright (C) 2007,2008,2009,2010  Free Software Foundation, Inc.
 #
 # GRUB is free software: you can redistribute it and/or modify
 # it under the terms of the GNU General Public License as published by
@@ -38,21 +38,7 @@
 
 make_system_path_relative_to_its_root ()
 {
-  path=`${grub_mkrelpath} $1`
-
-  case `uname 2/dev/null` in
-CYGWIN*)
-  # Cygwin: Check if regular or emulated mount.
-  if [ -z $dir ] || [ `stat -c %D $dir/..` != 62 ] ; then
-# Reached some mount point not below /cygdrive.
-# GRUB does not know Cygwin's emulated mounts,
-# convert to Win32 path and remove drive letter.
-path=`cygpath -m $path | sed -n 's,^[A-Za-z]:,,p'`
-test ! -z $path || return 1
-  fi ;;
-  esac
-
-  echo $path
+  ${grub_mkrelpath} $1
 }
 
 is_path_readable_by_grub ()

=== modified file 'util/misc.c'
--- util/misc.c	2010-04-09 23:25:46 +
+++ util/misc.c	2010-04-14 16:48:44 +
@@ -53,6 +53,11 @@
 # include malloc.h
 #endif
 
+#ifdef __CYGWIN__
+# include sys/cygwin.h
+# define DEV_CYGDRIVE_MAJOR 98
+#endif
+
 #ifdef __MINGW32__
 #include windows.h
 #include winioctl.h
@@ -456,6 +461,31 @@
   return ret;
 }
 
+#ifdef __CYGWIN__
+/* Convert POSIX path to Win32 path,
+   remove drive letter, replace backslashes.  */
+static char *
+get_win32_path (const char *path)
+{
+  char winpath[PATH_MAX];
+  if (cygwin_conv_path (CCP_POSIX_TO_WIN_A, path, winpath, sizeof(winpath)))
+grub_util_error (cygwin_conv_path() failed);
+
+  int len = strlen (winpath);
+  if (len  2  winpath[1] == ':')
+{
+  len -= 2;
+  memmove (winpath, winpath + 2, len + 1);
+}
+
+  int i;
+  for (i = 0; i  len; i++)
+if (winpath[i] == '\\')
+  winpath[i] = '/';
+  return xstrdup (winpath);
+}
+#endif
+
 /* This function never prints trailing slashes (so that its output
can be appended a slash unconditionally).  */
 char *
@@ -521,28 +551,28 @@
   /* offset == 1 means root directory.  */
   if (offset == 1)
 	{
-	  free (buf);
-	  len = strlen (buf2);
-	  while (buf2[len - 1] == '/'  len  1)
-	{
-	  buf2[len - 1] = '\0';
-	  len--;
-	}
-	  if (len  1)
-	return buf2;
-	  else
-	{
-	  /* This means path given is just a backslash.  As above
-		 we have to return an empty string.  */
-	  free (buf2);
-	  return xstrdup ();
-	}
+	  /* Include leading slash.  */
+	  offset = 0;
+	  break;
 	}
 }
   free (buf);
   buf3 = xstrdup (buf2 + offset);
   free (buf2);
 
+#ifdef __CYGWIN__
+  if (st.st_dev != (DEV_CYGDRIVE_MAJOR  16))
+{
+  /* Reached some mount point not below /cygdrive.
+	 GRUB does not know Cygwin's emulated mounts,
+	 convert to Win32 path.  */
+  grub_util_info (Cygwin path = %s\n, buf3);
+  char * temp = get_win32_path (buf3);
+  free (buf3);
+  buf3 = temp;
+}
+#endif
+
   len = strlen (buf3);
   while (buf3[len - 1] == '/'  len  1)
 {

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


NEED_REGISTER_FRAME_INFO can be replaced by -static-libgcc

2010-04-13 Thread Christian Franke

Hi,

a note regarding this fix:

2010-04-11  Vladimir Serbinenko ...

Fix cygwin compilation.

* configure.ac: Define NEED_REGISTER_FRAME_INFO.
* include/grub/misc.h (__register_frame_info)
[NEED_REGISTER_FRAME_INFO  !UTIL]: New export.
(__deregister_frame_info) [NEED_REGISTER_FRAME_INFO  !UTIL]: 
Likewise.

* kern/misc.c (__register_frame_info)
[NEED_REGISTER_FRAME_INFO  !UTIL]: New empty function.
(__deregister_frame_info) [NEED_REGISTER_FRAME_INFO  !UTIL]: 
Likewise.



--- configure.ac2010-04-11 14:14:51 +
+++ configure.ac2010-04-12 14:58:44 +
@@ -376,7 +376,11 @@
 # For platforms where ELF is not the default link format.
 AC_MSG_CHECKING([for command to convert module to ELF format])
 case ${host_os} in
-  cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
+  cygwin) TARGET_OBJ2ELF='grub-pe2elf';
+# FIXME: put proper test here
+  AC_DEFINE([NEED_REGISTER_FRAME_INFO], 1,
+[Define to 1 if GCC generates calls to __register_frame_info()])
+  ;;
   *) ;;


The *_frame_info symbols are set undefined to force linkage of the 
libgcc_s shared library or dll.


This can be prevented by TARGET_LDFLAGS=-static-libgcc. To build from 
grub-1.98 tarball on Cygwin, run configure with this argument.


May also be necessary on other build platforms when -shared-libgcc is 
the default. It may be possible simply set -static-libgcc unconditionally.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Add missing include to fix build on Cygwin

2010-03-03 Thread Christian Franke

This fixes a regression in grub-pe2elf which breaks build on Cygwin.


2010-03-03  Christian Frankefra...@computer.org

* util/grub-pe2elf.c: Add missing include progname.h.
This fixes build on Cygwin.


--
Regards,
Christian Franke

=== modified file 'util/grub-pe2elf.c'
--- util/grub-pe2elf.c	2010-01-16 00:26:52 +
+++ util/grub-pe2elf.c	2010-03-03 19:54:44 +
@@ -29,6 +29,8 @@
 #include stdlib.h
 #include getopt.h
 
+#include progname.h
+
 static struct option options[] = {
   {help, no_argument, 0, 'h'},
   {version, no_argument, 0, 'V'},

___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: On gratuitous modularization

2010-02-07 Thread Christian Franke

Vladimir 'φ-coder/phcoder' Serbinenko wrote:

Christian Franke wrote:
   

Hi Robert,

Robert Millan wrote:

 

Please be careful when adding modules.  I see that too often new modules
are added without any real need to host this code separately.

...
kern/disk.c:struct grub_disk_ata_pass_through_parms *);

this seems unnecessary.  ata_pthru is very small.  If it's only used
by hdparm,
why not just merge it?  This also avoids the additional code in kernel.


   

The module ata_pthru.mod exists only to keep ata.mod small, see:
http://lists.gnu.org/archive/html/grub-devel/2009-02/msg00091.html

Keeping the ata.mod specific pass-through function separate from
hdparm.mod was intentional. Merging this function into hdparm.mod
would only make sense if ata.mod will the only ATA access module with
pass-through functionality in the future. Hdparm.mod would then depend
on ata.mod. A 'hdparm -h' would load ata.mod and disable biosdisk access.

I hope we will eventually have an ahci.mod :-)

 

Are the parameters of current ata_pass_through ATA-specific or would
they be the same on AHCI?
   


Like e.g. the Linux HDIO_DRIVE_TASKFILE ioctl, the ata_pass_through() 
parameters are not controller or transport specific. It should work with 
any possible future implementation of ATA pass-through functionality, e.g.

- native AHCI driver
- other native drivers
- SAT (ANSI INCITS 431-2007) ATA pass through command for:
-- SATA drives behind USB-bridges with SAT support, or:
-- SATA drives in SAS enclosures

Merging ata_pthru.mod into hdparm.mod would make hdparm specific to one 
(outdated) class of controllers: the traditional IDE-controller (T13/1510D).




BTW: I agree that using a global function pointer
'grub_disk_ata_pass_through' is a hack. A cleaner design would be
possible with a grub_disk_dev.ioctl(.) call.

 


Such an ioctl() call would IMO be the best way to handle such device 
class specific extra functionality.


--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: On gratuitous modularization

2010-01-25 Thread Christian Franke

Hi Robert,

Robert Millan wrote:


Please be careful when adding modules.  I see that too often new modules
are added without any real need to host this code separately.

There are many examples where this happened.  I just noticed:

commands/hdparm.c:  struct grub_disk_ata_pass_through_parms apt;
commands/hdparm.c:  if (grub_disk_ata_pass_through (disk,apt))
commands/hdparm.c:  struct grub_disk_ata_pass_through_parms apt;
commands/hdparm.c:  if (grub_disk_ata_pass_through (disk,apt))
commands/hdparm.c:  struct grub_disk_ata_pass_through_parms apt;
commands/hdparm.c:  if (grub_disk_ata_pass_through (disk,apt))
commands/hdparm.c:  if (! grub_disk_ata_pass_through)
disk/ata_pthru.c:  struct grub_disk_ata_pass_through_parms 
*parms)
disk/ata_pthru.c:  grub_disk_ata_pass_through = grub_ata_pass_through;
disk/ata_pthru.c:  if (grub_disk_ata_pass_through == grub_ata_pass_through)
disk/ata_pthru.c:grub_disk_ata_pass_through = NULL;
include/grub/disk.h:struct grub_disk_ata_pass_through_parms
include/grub/disk.h:extern grub_err_t (* 
EXPORT_VAR(grub_disk_ata_pass_through)) (grub_disk_t,
include/grub/disk.h:   struct grub_disk_ata_pass_through_parms *);
kern/disk.c:grub_err_t (* grub_disk_ata_pass_through) (grub_disk_t,
kern/disk.c:struct grub_disk_ata_pass_through_parms *);

this seems unnecessary.  ata_pthru is very small.  If it's only used by hdparm,
why not just merge it?  This also avoids the additional code in kernel.

   


The module ata_pthru.mod exists only to keep ata.mod small, see:
http://lists.gnu.org/archive/html/grub-devel/2009-02/msg00091.html

Keeping the ata.mod specific pass-through function separate from 
hdparm.mod was intentional. Merging this function into hdparm.mod would 
only make sense if ata.mod will the only ATA access module with 
pass-through functionality in the future. Hdparm.mod would then depend 
on ata.mod. A 'hdparm -h' would load ata.mod and disable biosdisk access.


I hope we will eventually have an ahci.mod :-)

BTW: I agree that using a global function pointer 
'grub_disk_ata_pass_through' is a hack. A cleaner design would be 
possible with a grub_disk_dev.ioctl(.) call.


--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Request: hdparm module with -X transfer mode

2010-01-11 Thread Christian Franke

Nando wrote:
It's great to have hdparm output added. Another fixup that would be 
needed would be setting the ATAPI cable mode to be 80pin, rather than 
the slower 40pin. Users who add hard drives in place of PATA optical 
drives are liked affected. Eg:


http://forum.notebookreview.com/showthread.php?p=5519419  (HP 2510P / 
optical drive caddy)
http://www.gottabemobile.com/forum/forum_posts.asp?TID=8260PID=55271 
http://www.gottabemobile.com/forum/forum_posts.asp?TID=8260PID=55271 (Dell 
XT / not sure why)


Comments/suggestions?



AFAICS this is not possible:

Cable detection can be done either by host sensing the PDIAG:CBLID 
signal, by relying on sensing info from the device, or a combination of 
both. This is rather complex such that false results may be not 
unlikely. The device reports the state of PDIAG:CBLID in IDENTIFY word 
93. The original Linux hdparm prints this info as:

HW reset results: CBLID- above/below Vih.
(I will add this to GRUB hdparm).

There is no standard ATA command to change what the device reports in 
word 93. The host OS driver may or may not use this info from the device 
to detect cable type.


Reference: T13/1532D Vol 2 Rev 4b (ATA-7 Parallel Transport) section 
8.2.11 and annex D.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: gettext: commands/hdparm.c

2009-12-09 Thread Christian Franke
Jordi Mallach wrote:
 On Sun, Dec 06, 2009 at 06:25:33PM +, Carles Pina i Estany wrote:
  grub_printf (Model:    \%.40s\\n, le16_to_char (tmp, idw[27],
  40)); grub_printf (Firmware: \%.8s\\n,  le16_to_char (tmp,
  idw[23], 8)); grub_printf (Serial:   \%.20s\\n, le16_to_char
  (tmp, idw[10], 20)); 
  To have a proper alignment it needs to code something (dynamic
  tab).
  
  I could change it to:
  grub_printf (_(Model:\t\%.40s\\n), le16_to_char (tmp, idw[27],
  40)); grub_printf (_(Firmware:\t\%.8s\\n),  le16_to_char (tmp,
  idw[23], 8)); grub_printf (_(Serial:\t\%.20s\\n), le16_to_char
  (tmp, idw[10], 20)); 
 
 It's not unusual to see stuff like this, but again, a small percent of
 translators will actually try to figure out if the string is correctly
 aligned or how many \t's they need to use (by manually calculating how
 the string will look, by reading the source code, or using qemu).
 
 If the alignment is just a cosmetic enhancement, dropping it could be
 a reasonable option.
 

Yes, it is only cosmetic - feel free to drop it.

-- 
Regards,
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Request: hdparm module with -X transfer mode

2009-12-09 Thread Christian Franke
Nando wrote:
 
 I was pleasantly surprised when I saw a hdparm module as part of
 grub2. So I tried the -X 
 parameter as is provided by the linux command line finding it
 isn't implemented. eg:
 
 # hdparm -X udma5 /dev/sda
 
 Can the development team consider adding this feature? A number of
 ppl's bios is incorrectly setting interfaces to UDMA2/MDMA2,
 particularly on pata optical drive interfaces which users are using a
 2.5 HDDs instead via an optical bay caddy. The workaround being to
 wait for a (slow) Linux/Windows boot and then typing the above
 commandline. Doing this at the grub2 bootloader level being more
 preferable.
 

OK, added to my local TODO list :-)

Please note that GRUB hdparm does only work in conjunction with the
ata+ata_pthru modules because a PC BIOS does not provide any ATA
pass-through functionality. A 'hdparm -X' would only help if the actual
boot works with ata.mod instead of biosdisk.mod.

-- 
Regards,
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Easy grub2 installation for non Linux (Windows) systems?

2009-12-09 Thread Christian Franke
Nando wrote:
 For non Linux users, it is possible to load the grub2 package and
 modules on an ntfs filesystem? ...
 

Yes, GRUB 1.97 builds and runs out-of-the-box on Cygwin.
(Note: requires previous binutils-20080624-2 due to a regression in ld
from binutils-2.19.51-1)

1.96 is available as a Cygwin package, 1.97 will be provided when
binutils regression is fixed.

-- 
Regards,
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: hdparm documentation

2009-11-08 Thread Christian Franke

Pedro A ARANDA wrote:
I'm trying to make the hdparm module run on my HP Mini 110. I have had 
no luck,
neither in doing so or finding documentation which could help me doing 
so. If I get
the right pointer and I make it run on my box, I'd be willing to post 
a HOWTO, to

make things easier to other people.

I'm currently running Ubuntu 2009.10 with grub2.

When I 


set debug=ata
insmod ata

I actually get my drive listed as (ata0,0) but when I then
try

insmod hdparm
hdparm -B 255 (ata0,0)

grub keeps telling me that the drive (hd0) is not known... 



After 'insmod ata', loading of further modules typically fails because 
the drive in the 'prefix' variable is no longer valid.


The following works for me:

# Load needed modules first
insmod hdparm

# Switch to ATA driver and load ATA pass-through support
# (this invalidates prefix)
insmod ata_pthru

# Turn APM off
hdparm -B 255 (ata0)


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ntldr support

2009-08-10 Thread Christian Franke

Robert Millan wrote:
It probably would make sense that the 'ntldr' command does simple  
signature checks and fail on unknown files unless '--force' is specified.



You mean checking for the PE signature?  Yes, this would be nice too.

  


A check of the first byte (jmp, 0xe9) and some file size range check 
(e.g. 0x3...0x4) may be enough for a first ntldr command. May 
also work for bootmgr.exe.


EXE (MZ) and PE headers appear at larger offsets:

ntldr from XP SP2: size 251184, EXE header at 0x4d30, PE at 0x4e00
ntldr from XP SP3: size 251712, EXE header at 0x4d40, PE at 0x4e10
bootmgr.exe from Vista: ???


grub4dos checks for ntldr as follows:
- file starts with 0xe9, 0x??, 0x01,
- first sector does not end with bootsector signature 0x55,0xaa,
- file size exceeds 0x3.

--
Regards
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ntldr support

2009-08-10 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:

grub4dos checks for ntldr as follows:
- file starts with 0xe9, 0x??, 0x01,
- first sector does not end with bootsector signature 0x55,0xaa,
- file size exceeds 0x3.


For me it sounds like a heuristic. I would prefer to trust user rather
than introducing heuristics to check file type.
  


I agree that such a heuristic is not needed. Grub2 uses different 
commands for different loaders (which is good) and therefore there is no 
need to auto-detect the loader type.


But a simple health check (like in 'chainloader') would IMO make sense 
for any load command. For 'ntldr': check that the file size is 
reasonable and that code starts with a jmp instruction. Allow to 
override the check with '--force'.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ntldr support

2009-08-08 Thread Christian Franke

Michal Suchanek wrote:


I tried putting the ntldr binary on an empty fat32 disk and loading
with the grub ntldr command.

The system just reboots.

  


This also happens when ntldr is loaded by the regular boot code in 
PBR+boot area.

It is apparently the normal(?-) behavior when ntdetect.com is missing.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ntldr support

2009-08-08 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:

About the command, i think that it will be simpler for the user if we have
only one command: chainloader (like in grub4dos) that will try to detect the
type of the bootloader. This is only my personal opinion.


I don't agree with this. chainloader and ntldr don't share the same
syntax: chainloader expects a bootsector whereas ntldr expects an
ntldr ot bootmgr file. GRUB2 is done to break with bad design
decisions of GRUB1 one of them being kernel command. GRUB4DOS
follows GRUB1 on this subject.
  


I agree.

It probably would make sense that the 'ntldr' command does simple 
signature checks and fail on unknown files unless '--force' is specified.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Solving the grub-pe2elf problem

2009-08-08 Thread Christian Franke

Bean wrote:

You could also check out my new object format at
http://github.com/bean123/grub, lib branch. It supports conversion of
pe/elf/mach-o object file to a grub specific format, so that you can
use native build tool in Linux/OSX/Windows, no need for linker and
converter. It also reduce compile time by half and generate module
file much smaller that the current format.

  


Tested successfully on Cygwin - Nice work!

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Solving the grub-pe2elf problem

2009-08-07 Thread Christian Franke

Robert Millan wrote:

I thought of a possible solution to the grub-pe2elf problem.  It seems that
it is burdensome to produce ELF binaries on Windows, but building PE binaries
or even PE/win32 executables on GNU/Linux is not (thanks to Mingw32 toolchain
which is available on most distributions).

If building GRUB with ./configure --host=i586-mingw32msvc works, we could
include win32 binaries in the upcoming 1.97 release.  Other GNU projects do
this as well.  Then we could drop support for toolchains that lack ELF.

  


Which binaries would you like to include?
- kernel.img and modules, and/or
- the grub-*.exe  files (Note: mingw32 .exe != cygwin .exe).?

The problem with the PE-ELF conversion affects build of the *.mod 
files, not the build of the *.exe. The linker script is necessary for 
kernel.img.




Christian and Bean, are you interested in implementing this?

  


No.

Then building a Cygwin package would rely on the existence of (tested or 
untested?) binaries in the a release tarball. Building packages from SVN 
snapshots would no longer be possible.


I really don't want to spent my open-source-time with such kind of 
feature removal :-)



--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Solving the grub-pe2elf problem

2009-08-07 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:

If you see pe2elf as being too much of a burden we may switch to
objconv: http://www.agner.org/optimize/. It's already used in
conjunction with Apple Mach-O toolchain. It's not a GNU project but is
licensed under GPL.
  


Thanks for the hint, I will try objconv it soon.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ntldr support

2009-08-04 Thread Christian Franke

Robert Millan wrote:

This patch implements a loader for NTLDR boot semantics (which are
the same in BootMGR, hence both are supported).

It still needs some cleanup in chainloader.c before it can be merged [1],
but I submit it so that others can test it and report if it works for
them.

  


Tested with WinXP in a VM:

Before the test, original boot code behind partition boot sector was 
destroyed.

(dd if=/dev/zero of=/dev/hda1 bs=512 seek=1 count=20)

Then this fails:

root=(hd0,1)
chainloader (hd0,1)+1
boot

But with the patch this still works:

root=(hd0,1)
ntldr /ntldr
boot

Thanks.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: grub-pe2elf

2009-08-04 Thread Christian Franke

Robert Millan wrote:

On Mon, Aug 03, 2009 at 04:29:18PM -0400, Pavel Roskin wrote:
  

Committing code against the wish of maintainers should not be permitted.
Those who do it should lose commit access.  They are welcome to
contribute, but their patches will need to be committed by others.




I agree...



I haven't seen any apology or explanation in this thread.



  


... but I will not apologize simply because I did not commit the 
grub-pe2elf patch.




Yes, this is what worries me the most.

  


I'm somewhat worried that these complaints are delayed by more than a year
Commit 1726 is from 2008-07-24, start was here:
http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00452.html


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ntldr support

2009-08-04 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:

On Tue, Aug 4, 2009 at 9:27 PM, Robert Millan... wrote:
  

After thinking a bit about this, I don't think we want this command in
its current form.

The problem is it is misleading.  It leads the user to think it can load
ntldr as a standalone file, but in fact it is reading the PBR behind the
scenes.



I don't think it's of any problem since ntldr uses this PBR only as
superblock to identify the partition. As such I would rather consider
this loading as a special case of passing $root, just the form of it
is a bit weird

  


Agree.

A 'ntldr' or 'chainloader --ntldr' command is not mandatory. But it is 
'nice to have' because it allows to boot even if the boot code (6 
sectors) in the area behind the PBR is not present for whatever reason. 
See my previsions mail with the test case.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: grub-pe2elf

2009-08-03 Thread Christian Franke
Robert Millan wrote:
 On Sat, Aug 01, 2009 at 05:06:46PM +0200, Christian Franke wrote:
  
  Since ~r1584, util/grub-* could be build and run on Cygwin. Commit
  r1726 added support to build kernel and modules on Cygwin.
  
 
 There has to be more to it.  I know you were using GRUB on Cygwin
 before that commit.  Was this kludge being used since the beginning? 

I'm using GRUB on Cygwin since 'somewhat' before I announced the first
patch 2007-10-16:
http://lists.gnu.org/archive/html/grub-devel/2007-10/msg00071.html

This version used the hack in grub's ELF-loader already discussed. It
also contained some non Cygwin related fixes found during testing.

The grub2 package in the Cygwin distro from 2008-03-30 was based on this
private branch merged to 1.96+epsilon.


 What are the other options?
 

I don't know.

Waiting until someone fixes BFD or maintains a Linux cross-compiler
package for Cygwin (which cannot be build OOTB, it requires build steps
on Linux) may last until time_t wraps around and is IMO not an option
:-)

Even the isolation of grub-pe2elf in a separate package would not help
much, because the linker script and some support in the build system
would be still needed in the grub2 tree.

-- 
Regards,
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: grub-pe2elf

2009-08-01 Thread Christian Franke

Robert Millan wrote:

On Fri, Jul 31, 2009 at 08:27:58PM +0200, Christian Franke wrote:
  

First of all, I'm worried that we have discussions to decide things and they
are later forgotten or ignored.  Bean, please can you provide some explanation
on what happened?

Also, I think this commit should be reverted (at least the part that adds
grub-pe2elf).  
  
I would regret this. Then you could also revert all my Cygwin related  
additions (some also included in Bean's r1726) because these no longer  
make much sense unless someone fixes BFD or maintains a ELF toolchain  
for Cygwin.



But we supported Cygwin before grub-pe2elf.  What has changed?

  


Since ~r1584, util/grub-* could be build and run on Cygwin. Commit r1726 
added support to build kernel and modules on Cygwin.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: grub-pe2elf

2009-07-31 Thread Christian Franke

Hi Robert,

Robert Millan wrote:

On a recent discussion on IRC, Bean pointed out that grub-pe2elf is essentially
a workaround for a bug in objcopy.

  


It is essentially a limitation of BFD: The object format specific 
backends are apparently not implemented such that conversion between 
different relocation formats is supported.


In particular, a relocation 'addend' (-4 in this case) is lost during 
conversion if source or target relocation format do not support it as 
separate field. Some conversions may work. PE - ELF does not work, the 
problem is in both backends in this case.




Bean explained that this bug is quite obvious.  It seems Christian sent them
a patch, but it hasn't been applied (I don't know if it was rejected, or just
ignored, Christian could you explain?).

  


The patch was actually an ad-hoc approach to fix it in objcopy itself 
for the PE-ELF case only:

http://sourceware.org/ml/binutils/2007-10/msg00302.html

A generic solution which would have more chances to get accepted would 
require substantial work in BFD itself. Conversion between relocation 
formats is apparently rarely needed. A related bugzilla entry was 
reopened in 2005 and is idle since then:

http://sourceware.org/bugzilla/show_bug.cgi?id=970



Later in July 2008, Bean added grub-pe2elf to the repository.

  


Yes, and I really appreciated this :-)



First of all, I'm worried that we have discussions to decide things and they
are later forgotten or ignored.  Bean, please can you provide some explanation
on what happened?

Also, I think this commit should be reverted (at least the part that adds
grub-pe2elf).  


I would regret this. Then you could also revert all my Cygwin related 
additions (some also included in Bean's r1726) because these no longer 
make much sense unless someone fixes BFD or maintains a ELF toolchain 
for Cygwin.




Nevertheless, it's not an obligation for us to support win32.  As long as we
can do it sanely, I'm fine with it, but I don't want to see GRUB dragged into
a situation in which we need to preserve win32 support at all costs.

  


$ find grub2 -type f -name '*.[ch]' | xargs cat | wc -l
128175

$ cat grub2/util/grub-pe2elf.c | wc -l
521

0.4% of the total LoC is IMO less than 'all costs' :-)

--
Regards
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [BUGFIX] Don't use DT_DIR: It doesn't work on non-ext* filesystems

2009-07-24 Thread Christian Franke

Pavel Roskin wrote:

On Thu, 2009-07-23 at 11:29 +0200, Vladimir 'phcoder' Serbinenko wrote:

  

-#ifdef DT_DIR
-  info.dir = (de-d_type == DT_DIR);
-#else
   info.dir = !! is_dir (path, de-d_name);
-#endif



Fine with me.  Finally a patch that reduces the number of preprocessor
directives :-)

  


:-)

Grub hostfs has no performance requirement like e.g. find. The 
optimization with d_type can simply be ignored - patch is OK.


The idea behind d_type is that it should only be set != DT_UNKNOWN if 
the type info is available for free or at low cost. It is safe for a 
filesystem implementation to return DT_UNKNOWN always.


A correct performance-aware solution would look like:

#ifdef DT_DIR
 if (de-d_type == DT_DIR)
   info.dir = 1;
 else if (de-type == DT_FILE)
   info.dir = 0;
 else
#endif
   info.dir = !! is_dir (path, de-d_name);

This was actually wrong in my original patch (svn 1353) that was 
necessary because of missing d_type on Cygwin - Sorry! Meantime I added 
d_type support to Cygwin itself and therefore learned how this is 
supposed to work :-)


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [BUGFIX] Don't use DT_DIR: It doesn't work on non-ext* filesystems

2009-07-24 Thread Christian Franke

Pavel Roskin wrote:

On Fri, 2009-07-24 at 23:02 +0200, Christian Franke wrote:

  

A correct performance-aware solution would look like:

#ifdef DT_DIR
  if (de-d_type == DT_DIR)
info.dir = 1;
  else if (de-type == DT_FILE)



There in no DT_FILE in glibc, but there is DT_REG.  


Yes correct.



DT_UNKNOWN is
present.  Perhaps the above line should be

else if (de-type != DT_UNKNOWN)

We only care if it's a directory or not.  All other objects can be
treated like files.

  


It depends: If DT_UNKNOWN is used, you have to replace stat() by lstat() 
in is_dir(). Otherwise symlinked directories would be appear as dir only 
if d_type == DT_UNKNOWN and not if d_type == DT_LNK.




I'm fine either way, whether we fix the high-performance code or
remove it, as long as we don't have to add more checks.

  


I would suggest to remove it.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] add a snapshot version string to GRUB title

2009-07-02 Thread Christian Franke
Many GNU GRUB  version 1.96 used in production are likely builds from 
more recent SVN snapshots. It would (IMO) be useful to have some info 
about the actual installed version.


This patch adds a script to generate some version info from ChangeLog 
and (if available) from SVN.


The version syntax is PACKAGE_VERSION+DATE-N-rSVN where DATE is the 
most recent date in ChangeLog and 'N' is the number of the entries for 
this date. If the last ChangeLog entry suggests that this is an official 
release, only PACKAGE_VERSION is used.


The current string is 1.96+20090702-1-r2391 or 1.96+20090702-1 if 
'svn info' is not available.


The string is added to grub title only, utils are left out for now.


Regards,
Christian Franke


2009-07-02  Christian Franke  fra...@computer.org

Auto-generate a snapshot version string for GRUB title.

* Makefile.in: Add targets for genversionstr.sh.
Add target and dependency for grub_version.h.

* configure.ac: Add genversionstr.sh to AC_CONFIG_FILES.

* genversionstr.sh.in: New script.  Generates GRUB_VERSION
string from ChangeLog.

* normal/main.c: Add include grub_version.h.
(grub_normal_init_page): Replace PACKAGE_VERSION by
GRUB_VERSION in TITLE.


diff --git a/Makefile.in b/Makefile.in
index 3d208e7..6b56c83 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -419,6 +419,16 @@ gensymlist.sh: gensymlist.sh.in config.status
 genkernsyms.sh: genkernsyms.sh.in config.status
 	$(SHELL) ./config.status
 
+genversionstr.sh: genversionstr.sh.in config.status
+	$(SHELL) ./config.status
+
+# Build version string
+grub_version.h: genversionstr.sh ChangeLog
+	$(SHELL) ./genversionstr.sh  $@ || (rm -f $@ ; exit 1)
+
+normal_mod-normal_main.o: grub_version.h
+
+
 .PHONY: all install install-strip uninstall clean mostlyclean distclean
 .PHONY: maintainer-clean info dvi dist check
 
diff --git a/configure.ac b/configure.ac
index 8b12c58..890cf3d 100644
--- a/configure.ac
+++ b/configure.ac
@@ -529,6 +529,6 @@ else
   rm -rf include/grub/machine
   cp -rp $srcdir/include/grub/$target_cpu/$platform include/grub/machine 2/dev/null
 fi
-AC_CONFIG_FILES([Makefile gensymlist.sh genkernsyms.sh])
+AC_CONFIG_FILES([Makefile gensymlist.sh genkernsyms.sh genversionstr.sh])
 AC_CONFIG_FILES([stamp-h], [echo timestamp  stamp-h])
 AC_OUTPUT
diff --git a/genversionstr.sh.in b/genversionstr.sh.in
new file mode 100644
index 000..6e58fb3
--- /dev/null
+++ b/genversionstr.sh.in
@@ -0,0 +1,77 @@
+#! /bin/sh
+#
+# Copyright (C) 2009  Free Software Foundation, Inc.
+#
+# This genversionstr.sh.in is free software; the author
+# gives unlimited permission to copy and/or distribute it,
+# with or without modifications, as long as this notice is preserved.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY, to the extent permitted by law; without
+# even the implied warranty of MERCHANTABILITY or FITNESS FOR A
+# PARTICULAR PURPOSE.
+
+### The configure script will replace these variables.
+
+srcd...@srcdir@
+
+
+cat EOF
+/* This file is automatically generated by genversionstr.sh. DO NOT EDIT! */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2009  Free Software Foundation, Inc.
+ *
+ *  GRUB is free software: you can redistribute it and/or modify
+ *  it under the terms of the GNU General Public License as published by
+ *  the Free Software Foundation, either version 3 of the License, or
+ *  (at your option) any later version.
+ *
+ *  GRUB is distributed in the hope that it will be useful,
+ *  but WITHOUT ANY WARRANTY; without even the implied warranty of
+ *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ *  GNU General Public License for more details.
+ *
+ *  You should have received a copy of the GNU General Public License
+ *  along with GRUB.  If not, see http://www.gnu.org/licenses/.
+ */
+
+EOF
+
+
+if sed -n '2,/^20/p' $srcdir/ChangeLog \
+   | grep 'configure.ac (AC_INIT): Bumped' /dev/null 2/dev/null
+then
+  # Last entry documents new PACKAGE_VERSION, assume official release
+
+  cat EOF
+#ifndef GRUB_VERSION
+  #define GRUB_VERSION PACKAGE_VERSION
+#endif
+EOF
+
+else
+  # Extract most recent date string from ChangeLog.
+  # Append number of log entries for this date.
+  datestr=`
+sed -n 's,^\(20[0-9][0-9]-[01][0-9]-[0-3][0-9]\) .*$,\1,p;1000q' $srcdir/ChangeLog \
+| uniq -c \
+| sed -n 's,^ *\([0-9][0-9]*\) \(20[0-9][0-9]\)-\([01][0-9]\)-\([0-3][0-9]\)$,+\2\3\4-\1,p;1q'
+  `
+
+  # get SVN revision if possible
+  svnstr=
+  if [ -d $srcdir/.svn ] ; then
+svnstr=`
+  ( cd $srcdir  svn info ) 2 /dev/null \
+  | sed -n 's,^Revision: *\(.*\)$,-r\1,p'
+`
+  fi
+
+  cat EOF
+#ifndef GRUB_VERSION
+  #define GRUB_VERSION PACKAGE_VERSION ${datestr}${svnstr}
+#endif
+EOF
+fi
+
diff --git a/normal/main.c b/normal/main.c
index 7f6336e..a412827 100644
--- a/normal/main.c
+++ b/normal/main.c
@@ -29,6 +29,8 @@
 #include grub

Re: [2342] 2009-06-18 Vladimir Serbinenko phco...@gmail.com

2009-06-20 Thread Christian Franke

Pavel Roskin wrote:

On Fri, 2009-06-19 at 22:05 +0200, Christian Franke wrote:
  

This was the AC_MSG_RESULT(...) for the
AC_MSG_CHECKING([for option to link raw image])
which is still present for the $grub_cv_apple_target_cc != yes  case.

I would suggest to move the AC_MSG_CHECKING outside the 'if 
$grub_cv_apple_target_cc = yes ... fi' and re-add the AC_MSG_RESULT.



Thanks for pointing it out.  Actually, the message is not needed at all.
There is no checking that needs to be communicated to the user.  It
would be appropriate if configure was running a compiler or looked for
some tool in the PATH.  In this case, it's just shell variable
assignments with on test for a file.

  


I would prefer an output here even if the test done isn't a 'real' 
configure check. Would also make sense when we later apply your

'Use common linker script for all i386-pc systems' suggestion.

But if you want to remove this messages it is also fine with me.

Same would then possibly also apply to the
AC_MSG_CHECKING([for command to convert module to ELF format])
a few lines below.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [2342] 2009-06-18 Vladimir Serbinenko phco...@gmail.com

2009-06-19 Thread Christian Franke

Vladimir Serbinenko wrote:

* configure.ac: remove a leftover AC_MSG_RESULT
...
--- trunk/grub2/configure.ac2009-06-18 13:51:06 UTC (rev 2341)
+++ trunk/grub2/configure.ac2009-06-18 14:02:23 UTC (rev 2342)
@@ -308,7 +308,6 @@
 AC_SUBST(TARGET_IMG_LDSCRIPT)
 AC_SUBST(TARGET_IMG_LDFLAGS)
 AC_SUBST(TARGET_IMG_CFLAGS)
-AC_MSG_RESULT([$TARGET_IMG_LDFLAGS_AC])
 
  


This was the AC_MSG_RESULT(...) for the
AC_MSG_CHECKING([for option to link raw image])
which is still present for the $grub_cv_apple_target_cc != yes  case.

I would suggest to move the AC_MSG_CHECKING outside the 'if 
$grub_cv_apple_target_cc = yes ... fi' and re-add the AC_MSG_RESULT.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [2290] 2009-06-10 Pavel Roskin pro...@gnu.org

2009-06-11 Thread Christian Franke

Pavel Roskin wrote:

Revision: 2290
  http://svn.sv.gnu.org/viewvc/?view=revroot=grubrevision=2290
Author:   proski
Date: 2009-06-10 18:32:13 + (Wed, 10 Jun 2009)
Log Message:
---
2009-06-10  Pavel Roskin  pro...@gnu.org

* configure.ac: Use -nostdlib when probing for the target.  It
should not be required to have libc for the target.
...
 # Set them to their new values for the tests below.
 CC=$TARGET_CC
-CFLAGS=$TARGET_CFLAGS
+CFLAGS=$TARGET_CFLAGS -nostdlib
 CPPFLAGS=$TARGET_CPPFLAGS
 LDFLAGS=$TARGET_LDFLAGS
 

  


This change breaks build at least on Cygwin, ./configure fails with:

checking whether target compiler is working... no
configure: error: cannot compile for the target

The link test in grub_PROG_TARGET_CC fails due to missing '__main' 
symbol from libgcc. According to gcc 4.3.2 man page, '-nostdlib' usually 
requires '-lgcc'.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [2290] 2009-06-10 Pavel Roskin pro...@gnu.org

2009-06-11 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:


The link test in grub_PROG_TARGET_CC fails due to missing '__main' symbol
from libgcc. According to gcc 4.3.2 man page, '-nostdlib' usually requires
'-lgcc'.


Try compiling but not linking using -c option. If it works you can
commit it since we never do complete linking for target
  


Complete linking is not used during build but during configure. The 
following tests rely on it:


grub_PROG_LD_BUILD_ID_NONE
grub_CHECK_BSS_START_SYMBOL
grub_CHECK_END_SYMBOL

I would suggest to undo commit 2290 for now.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [2290] 2009-06-10 Pavel Roskin pro...@gnu.org

2009-06-11 Thread Christian Franke

Pavel Roskin wrote:

We can use -Wl,--defsym,___main=0x8100 as it's done elsewhere in
configure.ac.  Let me just check it doesn't break anything.

  

This (r2310) works on Cygwin, thanks.

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: status grub2 port of grub-legasy map command

2009-05-31 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:


Do not do this. Some BIOS functions (like ah=08h) return data in dl.
Clients should not expect data in registers to be preserved across
interrupt calls. I don't know if there is something like a 8086/PC-BIOS
ABI document that we can find to confirm the non-preservation of dl, but
the FreeDOS MBR should be fixed then.


Thank you for pointing ah=8 function
AFAIK there is no normative reference. The source which was mainly
used for long years is helppc and it states following:
- registers DS, BX, CX and DX are preserved
(http://heim.ifi.uio.no/~stanisls/helppc/int_13.html)
But now this reference is terribly outdated.
SeaBIOS preserves %dl too
  


T13 EDD provides a probably more up to date documentation of int13.

There was no requirement to preserve registers is in EDD (2000) and 
EDD-2 (2002). It appeared in EDD-3 (2004) and is still in first EDD-4 
draft (2009):


The values in all registers that are not explicitly defined in the 
following sections shall be preserved at the completion of each function 
call
From Section 8 of: BIOS Enhanced Disk Drive Services - 3 (T13/1572D 
Revision 3)


Fortunately, T13 docs are (unlike T10 and SATA-IO) still publicly available:
http://www.t13.org/Documents/MinutesDefault.aspx?DocumentType=4DocumentStage=2

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ata.mod under qemu

2009-05-25 Thread Christian Franke

Pavel Roskin wrote:

On Sat, 2009-05-23 at 23:04 +0200, Christian Franke wrote:
  


New patch attached. Please test on the above hardware if possible.



It's working fine.  (ata6) and (ata7) appear, but (ata0) and (ata1)
doesn't.  When inserting ata.mod, I get:

error: error reading ATA IDENTIFY data
error: error reading ATA IDENTIFY data
  


Looks like the (ata0/1) controller returns !BSY, DRQ, !ERR in the status 
register if no device is connected.

This is now detected after the bogus IDENTIFY data is read.


That must be the ghost devices.  While it would be nice to get rid of
such messages, I'm fine if your patch is committed as is.

  


Modified version committed, messages should no longer appear.

Thanks!

--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ata.mod under qemu

2009-05-23 Thread Christian Franke

Christian Franke wrote:

Pavel Roskin wrote:
 

It helps detect CD-ROM under qemu.  But on the real hardware, it
introduces ghost drives.

Without the patch, I have (ata7) for the SATA hard drive.  With the
patch, I have (ata7) for the SATA hard drive, (ata6) for the SATA DVD-RW
and two bogus unreadable drives (ata0) and (ata1).

  


New patch attached. Please test on the above hardware if possible.

--
Regards,
Christian Franke

diff --git a/disk/ata.c b/disk/ata.c
index ea42d59..5fa0ef5 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -41,11 +41,14 @@ grub_ata_wait_not_busy (struct grub_ata_device *dev, int milliseconds)
   grub_millisleep (1);
 
   int i = 1;
-  while (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_BUSY)
+  grub_uint8_t sts;
+  while ((sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS))
+	  GRUB_ATA_STATUS_BUSY)
 {
   if (i = milliseconds)
 {
-	  grub_dprintf (ata, timeout: %dms\n, milliseconds);
+	  grub_dprintf (ata, timeout: %dms, status=0x%x\n,
+			milliseconds, sts);
 	  return grub_error (GRUB_ERR_TIMEOUT, ATA timeout);
 	}
 
@@ -151,6 +154,7 @@ grub_atapi_identify (struct grub_ata_device *dev)
 return grub_errno;
 
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, 0xE0 | dev-device  4);
+  grub_ata_wait ();
   if (grub_ata_check_ready (dev))
 {
   grub_free (info);
@@ -248,6 +252,7 @@ grub_ata_identify (struct grub_ata_device *dev)
   info16 = (grub_uint16_t *) info;
 
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, 0xE0 | dev-device  4);
+  grub_ata_wait ();
   if (grub_ata_check_ready (dev))
 {
   grub_free (info);
@@ -259,24 +264,38 @@ grub_ata_identify (struct grub_ata_device *dev)
 
   if (grub_ata_wait_drq (dev, 0, GRUB_ATA_TOUT_STD))
 {
-  if (grub_ata_regget (dev, GRUB_ATA_REG_ERROR)  0x04) /* ABRT */
-	{
-	  /* Device without ATA IDENTIFY, try ATAPI.  */
-	  grub_free(info);
-	  grub_errno = GRUB_ERR_NONE;
-	  return grub_atapi_identify (dev);
-	}
+  grub_free(info);
+  grub_errno = GRUB_ERR_NONE;
+  grub_uint8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
+
+  if ((sts  (GRUB_ATA_STATUS_BUSY | GRUB_ATA_STATUS_DRQ
+	  | GRUB_ATA_STATUS_ERR)) == GRUB_ATA_STATUS_ERR
+	   (grub_ata_regget (dev, GRUB_ATA_REG_ERROR)  0x04 /* ABRT */))
+	/* Device without ATA IDENTIFY, try ATAPI.  */
+	return grub_atapi_identify (dev);
+
+  else if (sts == 0x00)
+	/* No device, return error but don't print message.  */
+	return GRUB_ERR_UNKNOWN_DEVICE;
+
   else
-	{
-	  /* Error.  */
-	  grub_free(info);
-	  return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
-			 device can not be identified);
-	}
+	/* Other Error.  */
+	return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+			   device can not be identified);
 }
 
   grub_ata_pio_read (dev, info, GRUB_DISK_SECTOR_SIZE);
 
+  /* Re-check status to avoid bogus identify data due to stuck DRQ.  */
+  grub_uint8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
+  if (sts  (GRUB_ATA_STATUS_BUSY | GRUB_ATA_STATUS_DRQ | GRUB_ATA_STATUS_ERR))
+{
+  grub_dprintf(ata, bad status=0x%x, sts);
+  grub_free(info);
+  return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+			 error reading ATA IDENTIFY data);
+}
+
   /* Now it is certain that this is not an ATAPI device.  */
   dev-atapi = 0;
 
@@ -334,26 +353,12 @@ grub_ata_device_initialize (int port, int device, int addr, int addr2)
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, dev-device  4);
   grub_ata_wait ();
 
-  /* If status is 0x00, it is safe to assume that there
- is no device (or only a !READY) device connected.  */
-  grub_int8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
-  grub_dprintf (ata, status=0x%x\n, sts);
-  if (sts == 0x00)
-{
-  grub_free(dev);
-  return 0;
-}
-
   /* Try to detect if the port is in use by writing to it,
  waiting for a while and reading it again.  If the value
- was preserved, there is a device connected.
- But this tests often detects a second (slave) device
- connected to a SATA controller which supports only one
- (master) device.  In this case, the status register
- check above usually works.  */
+ was preserved, there is a device connected.  */
   grub_ata_regset (dev, GRUB_ATA_REG_SECTORS, 0x5A);  
   grub_ata_wait ();
-  grub_int8_t sec = grub_ata_regget (dev, GRUB_ATA_REG_SECTORS);
+  grub_uint8_t sec = grub_ata_regget (dev, GRUB_ATA_REG_SECTORS);
   grub_dprintf (ata, sectors=0x%x\n, sec);
   if (sec != 0x5A)
 {
@@ -361,6 +366,12 @@ grub_ata_device_initialize (int port, int device, int addr, int addr2)
   return 0;
 }
 
+  /* The above test may detect a second (slave) device
+ connected to a SATA controller which supports only one
+ (master) device.  It is not safe to use the status register
+ READY bit to check for controller channel existence.  Some
+ ATAPI commands (RESET, DIAGNOSTIC) may clear this bit.  */
+
   /* Use the IDENTIFY DEVICE command to query the device.  */
   if (grub_ata_identify

Re: [PATCH] ata.mod under qemu

2009-05-21 Thread Christian Franke

Pavel Roskin wrote:
On Wed, 2009-05-20 at 23:40 +0200, Christian Franke wrote: 
  
The attached patch should fix this. It should also prevents misleading 
error messages if a device does not exist.


It helps detect CD-ROM under qemu.  But on the real hardware, it
introduces ghost drives.

Without the patch, I have (ata7) for the SATA hard drive.  With the
patch, I have (ata7) for the SATA hard drive, (ata6) for the SATA DVD-RW
and two bogus unreadable drives (ata0) and (ata1).

  
Thanks for this test. I could not reproduce this yet. I presume that the 
controller behind (ata0) and (ata1) is the chipset's PATA controller.
If no device is connected, these controllers often echo a write to (e.g. 
command) register address back to the next read of (e.g. status) 
register address if the time between is too short. (No VM emulates this 
behavior :-)


Could you possibly repeat this test with 'debug=ata' enabled? If this 
removes the bogus drives, it is likely a problem of too short wait times.


I will prepare a new patch with longer timeouts and more thorough 
register tests during identify.


--
Regards,
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ata.mod under qemu

2009-05-20 Thread Christian Franke
The attached patch should fix this. It should also prevents misleading 
error messages if a device does not exist.


2009-05-20  Christian Franke  fra...@computer.org

* disk/ata.c: (grub_ata_wait_not_busy): Add debug output of status
register.
(grub_ata_identify): Suppress error message if status register
is 0x00 after command failure.
(grub_device_initialize): Remove unsafe status register check.
Thanks to 'phcoder' for problem report and patch.
Prevent sign extension in debug message.


--
Regards,
Christian Franke


diff --git a/disk/ata.c b/disk/ata.c
index ea42d59..651937d 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -41,11 +41,14 @@ grub_ata_wait_not_busy (struct grub_ata_device *dev, int milliseconds)
   grub_millisleep (1);
 
   int i = 1;
-  while (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_BUSY)
+  grub_uint8_t sts;
+  while ((sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS))
+	  GRUB_ATA_STATUS_BUSY)
 {
   if (i = milliseconds)
 {
-	  grub_dprintf (ata, timeout: %dms\n, milliseconds);
+	  grub_dprintf (ata, timeout: %dms, status=0x%x\n,
+			milliseconds, sts);
 	  return grub_error (GRUB_ERR_TIMEOUT, ATA timeout);
 	}
 
@@ -259,20 +262,21 @@ grub_ata_identify (struct grub_ata_device *dev)
 
   if (grub_ata_wait_drq (dev, 0, GRUB_ATA_TOUT_STD))
 {
+  grub_free(info);
+  grub_errno = GRUB_ERR_NONE;
+
   if (grub_ata_regget (dev, GRUB_ATA_REG_ERROR)  0x04) /* ABRT */
-	{
-	  /* Device without ATA IDENTIFY, try ATAPI.  */
-	  grub_free(info);
-	  grub_errno = GRUB_ERR_NONE;
-	  return grub_atapi_identify (dev);
-	}
+	/* Device without ATA IDENTIFY, try ATAPI.  */
+	return grub_atapi_identify (dev);
+
+  else if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS) == 0x00)
+	/* No device, return error but don't print message.  */
+	return GRUB_ERR_UNKNOWN_DEVICE;
+
   else
-	{
-	  /* Error.  */
-	  grub_free(info);
-	  return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
-			 device can not be identified);
-	}
+	/* Other Error.  */
+	return grub_error (GRUB_ERR_UNKNOWN_DEVICE,
+			   device can not be identified);
 }
 
   grub_ata_pio_read (dev, info, GRUB_DISK_SECTOR_SIZE);
@@ -334,26 +338,12 @@ grub_ata_device_initialize (int port, int device, int addr, int addr2)
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, dev-device  4);
   grub_ata_wait ();
 
-  /* If status is 0x00, it is safe to assume that there
- is no device (or only a !READY) device connected.  */
-  grub_int8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
-  grub_dprintf (ata, status=0x%x\n, sts);
-  if (sts == 0x00)
-{
-  grub_free(dev);
-  return 0;
-}
-
   /* Try to detect if the port is in use by writing to it,
  waiting for a while and reading it again.  If the value
- was preserved, there is a device connected.
- But this tests often detects a second (slave) device
- connected to a SATA controller which supports only one
- (master) device.  In this case, the status register
- check above usually works.  */
+ was preserved, there is a device connected.  */
   grub_ata_regset (dev, GRUB_ATA_REG_SECTORS, 0x5A);  
   grub_ata_wait ();
-  grub_int8_t sec = grub_ata_regget (dev, GRUB_ATA_REG_SECTORS);
+  grub_uint8_t sec = grub_ata_regget (dev, GRUB_ATA_REG_SECTORS);
   grub_dprintf (ata, sectors=0x%x\n, sec);
   if (sec != 0x5A)
 {
@@ -361,6 +351,12 @@ grub_ata_device_initialize (int port, int device, int addr, int addr2)
   return 0;
 }
 
+  /* The above test may detect a second (slave) device
+ connected to a SATA controller which supports only one
+ (master) device.  It is not safe to use the status register
+ READY bit to check for controller channel existence.  Some
+ ATAPI commands (RESET, DIAGNOSTIC) may clear this bit.  */
+
   /* Use the IDENTIFY DEVICE command to query the device.  */
   if (grub_ata_identify (dev))
 {
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] ata.mod under qemu

2009-05-19 Thread Christian Franke
Pavel Roskin wrote:
 On Mon, 2009-05-18 at 19:36 -0400, Pavel Roskin wrote:
 
  I did a quick check, and your patch has no effect on my system.  I'm
  using Fedora 11 with qemu 0.10.  After inserting the ata module, ls
  shows no disks, with or without your patch.
  
 
 Never mind, my quick check was too quick.  I forgot to change
 prefix, so it's just ls that was failing.
 
 Your patch makes the CD-ROM visible.  Without your patch, only hard
 drivers are seen.
 

Thanks for the problem report.

According to 'qemu-0.10.4/hw/ide.c', the (READY bit of the) status
register is actually cleared by some commands like RESET and DIAGNOSE if
an ATAPI device is connected. This conforms to ATA parallel transport
standard.

When I added the '(sts == 0x00)' check, I made the false assumption that
device detection through the READY bit is safe. It was added to speed-up
the detection of the missing slave port of some SATA controllers working
in IDE-mode. The check can be safely removed, the only drawback is an
extra ~1s timeout for each SATA controller.

I will commit a fix soon.

-- 
Regards,
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Use common linker script for all i386-pc systems

2009-05-18 Thread Christian Franke

Vladimir 'phcoder' Serbinenko wrote:

Hello.

On Sun, May 17, 2009 at 7:33 AM, Pavel Roskin ... wrote:
  

This allows us to remove checks for the linker symbols for the beginning
and the end of the .bss section.  Instead, we use the names from the
linker script.  Another benefit is a better unification of the build
system.




  


Good idea. Patch works on Cygwin. Does it work on Linux, FreeBSD, ... ?

The variable TARGET_IMG_LDFLAGS_AC can also be removed, see attached patch.

The linker script is actually identical to the script from my first 
Cygwin patch. It can be further simplified if desired.



@@ -0,0 +1,53 @@
+/* Linker script to create grub .img files on Cygwin.  */


This comment is out of sync
  

 VARIABLE(grub_end_addr)
-   .long   END_SYMBOL
+   .long   __bss_end__


This variable isn't used at all. In my Apple's CC patch I just remove it

  


'grub_start_addr' and '.globl start, _start' are also not used.


--
Christian Franke

diff --git a/configure.ac b/configure.ac
index a048828..e799a79 100644
--- a/configure.ac
+++ b/configure.ac
@@ -212,15 +212,13 @@ AC_MSG_CHECKING([for option to link raw image])
 if test -f ${srcdir}/conf/${target_cpu}-${platform}.lds; then
   TARGET_IMG_LDSCRIPT='$(top_srcdir)'/conf/${target_cpu}-${platform}.lds
   TARGET_IMG_LDFLAGS=-Wl,-T${TARGET_IMG_LDSCRIPT}
-  TARGET_IMG_LDFLAGS_AC=-Wl,-T${srcdir}/conf/${target_cpu}-${platform}.lds
 else
   TARGET_IMG_LDSCRIPT=
   TARGET_IMG_LDFLAGS='-Wl,-N'
-  TARGET_IMG_LDFLAGS_AC='-Wl,-N'
 fi
 AC_SUBST(TARGET_IMG_LDSCRIPT)
 AC_SUBST(TARGET_IMG_LDFLAGS)
-AC_MSG_RESULT([$TARGET_IMG_LDFLAGS_AC])
+AC_MSG_RESULT([$TARGET_IMG_LDFLAGS])
 
 # For platforms where ELF is not the default link format.
 AC_MSG_CHECKING([for command to convert module to ELF format])
@@ -379,11 +377,6 @@ grub_PROG_OBJCOPY_ABSOLUTE
 grub_PROG_LD_BUILD_ID_NONE
 grub_ASM_USCORE
 if test x$target_cpu = xi386; then
-  if test ! -z $TARGET_IMG_LDSCRIPT; then
-# Check symbols provided by linker script.
-CFLAGS=$TARGET_CFLAGS -nostdlib $TARGET_IMG_LDFLAGS_AC 
-Wl,-Ttext,8000,--defsym,___main=0x8100
-  fi
-  CFLAGS=$TARGET_CFLAGS
   grub_I386_ASM_PREFIX_REQUIREMENT
   grub_I386_ASM_ADDR32
   grub_I386_ASM_ABSOLUTE_WITHOUT_ASTERISK
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [2216] Remove incorrect comment that the code must be position independent.

2009-05-15 Thread Christian Franke

Pavel Roskin wrote:

Revision: 2216
  http://svn.sv.gnu.org/viewvc/?view=revroot=grubrevision=2216
Author:   proski
Date: 2009-05-15 14:47:44 + (Fri, 15 May 2009)
Log Message:
---
Remove incorrect comment that the code must be position independent.

Modified Paths:
--
trunk/grub2/commands/i386/pc/drivemap_int13h.S

  


Function drivemap.c:install_int13_handler() copies the 
grub_drivemap_handler code to some low memory allocated by 
grub_mmap_malign_and_register(). No relocation is done. IMO the code 
must be position independent.


--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Eliminating nested functions

2009-04-17 Thread Christian Franke

Pavel Roskin wrote:

I suggest that we eliminate all nested functions.  The reasons are:

1) They make the code less readable, as they make the parent functions
longer.

2) They have problems with some popular compilers, as recent as gcc-4.0
when regparm(3) is used.

3) We failed to implement a reliable test for such problems.  We are
using regparm(1) for all compilers.

4) The existing test is one of the obstacles making it impossible to
compile without having libc for the target (x86_64-i386 would be really
nice), as we need to run the compiled test executable.

5) Non-i386 architectures define NESTED_FUNC_ATTR as an empty symbol, so
developers on such architectures don't see if they use it correctly.

6) NESTED_FUNC_ATTR tends to proliferate to the file scope functions, as
it happened with grub_pci_iterate().  It only takes one caller using a
nested function to force NESTED_FUNC_ATTR on all functions used as an
argument to the same function.

  


7) The grub utils may not run on some 'security hardened' platform which 
does not allow to enable stack execution.



--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: precision formatting in grub_printf

2009-03-24 Thread Christian Franke

Deepak Vankadaru wrote:

Hi,

There was a redundant piece of code in the diff file I sent earlier. I 
have removed it and prepared another patch file. I am attaching the 
same. Please use this one instead.


Following is the changelog.

Changelog:
2008-08-17  Deepak Vankadaru  deepak.vankad...@gmail.com 
mailto:deepak.vankad...@gmail.com


   Support for precision formatting in grub_printf

   * kern/misc.c: modified grub_vsprintf to parse and handle 
precision formatting




Thanks.

Unfortunately, the patch can no longer be applied to current svn without 
a conflict.
The precision parsing and '%s' precision (truncation) handling is 
already implemented.


But the '%d' precision (zero fill) handling part of your patch is still 
missing in the code.


Could you please repost an updated patch in diff -u format?
Note that the current code does not use a format2_default flag buts sets 
format2 to ~0U ('infinite' :-) if no precision is specified.


Thanks

--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: precision formatting in grub_printf

2009-03-18 Thread Christian Franke
Robert Millan wrote:
  Changelog:
  2008-08-17  Deepak Vankadaru  deepak.vankad...@gmail.com
  
  Support for precision formatting in grub_printf
  
  * kern/misc.c: modified grub_vsprintf to parse and handle precision
  formatting
  
 
 Thanks.  Please could you provide this in unified diff format? (diff
 -u) 


I already added (fixed) the precision parsing and added handling of '%s'
precision (truncation):
http://svn.savannah.gnu.org/viewvc/trunk/grub2/kern/misc.c?root=grubr1=1936r2=1954
The parsing part of this new patch might conflict with this change.

-- 
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] implement grub_millisleep in util/misc.c for grub-emu

2009-02-28 Thread Christian Franke

Robert Millan wrote:

On Sun, Feb 15, 2009 at 07:49:38PM +0100, Felix Zielcke wrote:
  

--- util/misc.c (revision 1996)
+++ util/misc.c (working copy)
@@ -27,6 +27,9 @@
 #include sys/time.h
 #include unistd.h
 
+#define _POSIX_C_SOURCE 199309L

+#include time.h



I'm not sure this is garanteed to work unless it's defined before any
header is included.  Did it compile without warnings?

  


At least with glibc headers, the #define from the patch has no effect, 
because at least unistd.h has already included features.h which handles 
these settings. There is no warning, because none of the headers define 
_POSIX_C_SOURCE.


From features.h:
/* If none of the ANSI/POSIX macros are defined, use POSIX.1 and POSIX.2
  (and IEEE Std 1003.1b-1993 unless _XOPEN_SOURCE is defined).  */
#if (...!defined (_POSIX_C_SOURCE)...)
# define _POSIX_SOURCE  1
# if defined _XOPEN_SOURCE ...
#  ...
# else
#  define _POSIX_C_SOURCE   200112L
# endif
#endif

I would suggest to remove the #define and commit the patch.

BTW: compiles fine on Cygwin, with or without the #define.

--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] implement grub_millisleep in util/misc.c for grub-emu

2009-02-28 Thread Christian Franke

Christian Franke wrote:

There is no warning, because none of the headers define _POSIX_C_SOURCE.


This should read: There is a warning, because features.h defines 
_POSIX_C_SOURCE :-)


--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Building grub2 for windows

2009-02-27 Thread Christian Franke

Alexandre Bique wrote:

On Thu, Feb 26, 2009 at 2:35 PM, Christian Franke
... wrote:
  

What is the error message you got on Cygwin ?



The problem is that make install installs elf binaries and not win32
binaries. But win32 binaries are compiled. I moved win32 binaries by
hand.

  


I could not reproduce this.

Is the Cygwin PATH correct, such that the Cygwin tools /usr/bin/make, 
/usr/bin/install ... are actually used during build?


Where do those 'elf binaries' come from? The Cygwin gcc toolchain cannot 
produce elf binaries. That's why grub-pe2elf exists to convert the *.mod 
files to ELF format.




There is also a problem if you specify a root directory with space, i
think that some variables are not protected with $my_var in some
shell scripts.

  


Yes, that is correct and should IMO be fixed in the future.


--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Building grub2 for windows

2009-02-26 Thread Christian Franke
Alexandre Bique wrote:
 
 I am trying to build grub2-svn for windows by using mingw32 or cygwin
 but it fails to generate valid windows executables (grub-setup,
 grub-mkimage, etc...).
 
 My configure line:
 ./configure --prefix $HOME/grub-install
 
 using mingw32, msys, gcc 3.4.5 (mingw-vista special r3), binutils
 2.19.1 
 I also tried with cygwin, gcc 3.4.4, binutils 2.18.50.20080625.
 

Grub2 should build out of the box on Cygwin. The following works for me
with current grub2 svn (revision 2001) on current Cygwin 1.5.25-15:

$ ./configure --enable-grub-fstest --prefix=/tmp/inst
$ make
$ make install
$ /tmp/inst/bin/grub-mkrescue grub-rescue.iso
... boot grub-rescue.iso ...

There is actually one issue in current grub2 svn: Build with
--enable-grub-emu configure option does not work. It fails with
undefined symbol 'grub_millisleep'.

What is the error message you got on Cygwin ?

-- 
Christian Franke





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] hdparm.mod - get/set ATA disk parameters

2009-02-21 Thread Christian Franke

Robert Millan wrote:

On Sat, Feb 14, 2009 at 03:13:31PM +0100, Christian Franke wrote:
  

insmod ata_pthru



(note that module dependencies should make this unnecessary)

  


This insmod is necessary for now. hdparm.mod does not directly call 
ata_pthru.mod, it uses kern/disk.c::grub_disk_ata_pass_through function 
pointer instead. Otherwise, using other future ata_pass_through 
functions (e.g. from some ahci.mod :-) would not be possible.


Another drawback of a hdparm-ata_pthru dependency would be that 'help' 
or 'hdparm -h' would load ata.mod which disables e.g. biosdisk.mod.




insmod hdparm

# Make sure disks cannot be locked by an ATA password
hdparm --quiet --security-freeze (ata4)
hdparm --quiet --security-freeze (ata6)

menuentry Boot {

 # Check health
 if hdparm --quiet --health (ata4) ; then echo -n ; else
   echo Warning: SMART status check failed
   read
 fi

 # Set boot disk to fast, disable spin down
 hdparm --quiet --aam=254 --standby-timeout=0 (ata4)

 # Set other disk to quiet, spin down after 5min inactivity
 hdparm --quiet --aam=128 --standby-timeout=60 (ata6)

 # Boot ...
}

menuentry Memory Test {

 # Spin down both disks after 10min
 hdparm --quiet --standby-timeout=120 (ata4)
 hdparm --quiet --standby-timeout=120 (ata6)

 # Load memtest ...
}



Very interesting.  Do you think any of these features could be useful as a
default option in grub-mkconfig?

  


At least the --health check and --security-freeze are IMO recommended 
for each disk. Both is also done by a typical PC BIOS. AAM and standby 
settings are more user specific. The problem is that this relies on 
native ATA support which is now only available for controllers supported 
by ata.mod.


At least for the boot disk, such hdparm calls could be added by e.g. 
extending 'prepare_grub_to_access_device'.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] hdparm.mod - get/set ATA disk parameters

2009-02-21 Thread Christian Franke

Robert Millan wrote:

On Sat, Feb 21, 2009 at 07:00:06PM +0100, Christian Franke wrote:
  

Very interesting.  Do you think any of these features could be useful as a
default option in grub-mkconfig?

  
  
At least the --health check and --security-freeze are IMO recommended  
for each disk. Both is also done by a typical PC BIOS. AAM and standby  
settings are more user specific. The problem is that this relies on  
native ATA support which is now only available for controllers supported  
by ata.mod.


At least for the boot disk, such hdparm calls could be added by e.g.  
extending 'prepare_grub_to_access_device'.



But if the firmware already did, wouldn't this be a waste of boot time?

  


Yes, it would. Most BIOS perform both health check and security freeze, 
but some don't. For coreboot, it is not a waste of boot time.


So if this is added to grub-mkconfig, it should be configurable by some 
variable.


--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: Bugfix: ata pass-through broke compilation

2009-02-14 Thread Christian Franke

phcoder wrote:


Hello. Here is bugfix


Thanks!

Yes, I should rename all occurences...
(gcc 3.4.4 did not detect this).

Committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] hdparm.mod - get/set ATA disk parameters

2009-02-09 Thread Christian Franke

Robert Millan wrote:

On Sun, Feb 08, 2009 at 12:12:32AM +0100, Christian Franke wrote:
  

Robert Millan wrote:

I would suggest to move grub_ata_pass_through() to a new module (e.g. 
ata2.mod, atax.mod, ataex.mod, ...)
   


How about ata_something.mod?  (consistent with ntfs.mod  ntfs_comp.mod)

  


It is actually ntfscomp.mod.


 
  

ata_pthru.mod ?



Deal!

  


I will prepare a patch - ata_pthru.mod, atapthru.mod or a more generic 
name to allow to add other ata_extension functions ?



Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] hdparm.mod - get/set ATA disk parameters

2009-02-07 Thread Christian Franke

Robert Millan wrote:

On Sat, Jan 24, 2009 at 11:59:01PM +0100, Christian Franke wrote:
  
This patch adds a command which allows to change a few (S)ATA drive 
settings. It relies on a new pass-through function in ata.mod.



Is this function going to be used for normal use of ata?  Note that ata.mod
should be as small as possible, since coreboot users usually want to include
it the GRUB that is installed to flash.

  


Like 'hdparm.mod' itself, the ATA pass-through function is not needed 
for normal boot.




Would it make sense to put the function elsewhere?

  


I would suggest to move grub_ata_pass_through() to a new module (e.g. 
ata2.mod, atax.mod, ataex.mod, ...)


The function should not be in hdparm.mod itself, because the hdparm 
command would also work with other ATA pass-through functions. For 
example with some future ahci.mod or even in conjunction with native USB 
support through a SAT tunnel.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] hdparm.mod - get/set ATA disk parameters

2009-02-07 Thread Christian Franke

Robert Millan wrote:


I would suggest to move grub_ata_pass_through() to a new module (e.g. 
ata2.mod, atax.mod, ataex.mod, ...)



How about ata_something.mod?  (consistent with ntfs.mod  ntfs_comp.mod)

  

ata_pthru.mod ?


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Skip identical lines in hexdump

2009-02-02 Thread Christian Franke

Christian Franke wrote:
This skips identical lines in hexdump output, like 'od' or 'xxd -a' 
also do.




Committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Allow more than one line in option help text

2009-01-30 Thread Christian Franke

Christian Franke wrote:

This small patch allows '\n' in option help texts, for example:


Patch simplified  committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Skip identical lines in hexdump

2009-01-30 Thread Christian Franke

This skips identical lines in hexdump output, like 'od' or 'xxd -a' also do.

Christian

PS: I would suggest to change hexdump 'buf' parameter from 'char *' to 
'const void *' to avoid unnecessary casts.



2009-01-30  Christian Franke  fra...@computer.org

* lib/hexdump.c (hexdump): Print at most 3 lines if data is identical.


diff --git a/lib/hexdump.c b/lib/hexdump.c
index 9b79f45..7689edf 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -61,6 +61,22 @@ hexdump (unsigned long bse, char *buf, int len)
 
   grub_printf (%s\n, line);
 
+  /* Print only first and last line of more than 3 lines are identical.  */
+  if (len = 4 * 16
+	   ! grub_memcmp (buf, buf + 1 * 16, 16)
+	   ! grub_memcmp (buf, buf + 2 * 16, 16)
+	   ! grub_memcmp (buf, buf + 3 * 16, 16))
+	{
+	  grub_printf (*\n);
+	  do
+	{
+	  bse += 16;
+	  buf += 16;
+	  len -= 16;
+	}
+	  while (len = 3 * 16  ! grub_memcmp (buf, buf + 2 * 16, 16));
+	}
+
   bse += 16;
   buf += 16;
   len -= cnt;
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] remove target_os

2009-01-27 Thread Christian Franke

Javier Martín wrote:
Unfortunately, gcc has no '-fno_os' option to specify the bare CPU as 
target.



Might -ffreestanding be what you are looking for?

  


The option '-ffreestanding' is the same as '-fno-hosted'.

According to gcc (4.3.1) source, '-fno-hosted' clears variable 
'flag_hosted' and sets '-fno-builtin'. The latter is already set within 
GRUB build.


A cleared 'flag_hosted' apparently has only 2 effects:
- disable the special handling of 'main()'.
- #define __STDC_HOSTED__ to 0 instead of 1

There is no effect on the target_os dependent parts of the gcc code 
generation. For example, on i386, __enable_execute_stack() calls are 
generated for target_os netbsd, openbsd and cygwin, but not for linux. 
The emit call is hard-coded in 
gcc/configs/i386/i386.c:x86_initialize_trampoline().


Other workarounds are needed to support building GRUB with code 
generators tailored for various target_os.






 AC_MSG_CHECKING([for command to convert module to ELF format])
-case ${host_os}:${target_os} in
-  cygwin:cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
+case ${host_os} in
+  cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
   *) ;;
 esac
  
  
This won't work for a Linux cross compiler hosted on Cygwin. It would 
emit ELF format and does not need pe2elf.



A, say, AMD64 Linux cross compiler hosted on x86 Cygwin would have
$build=i686-pc-cygwin and $host=amd64-linux-gnu. Thus, no conflict ought
to arise even with cross compilation enabled.

  


But the opposite won't work: $host=i686-pc-cygwin would enable 
grub-pe2elf, even if this gcc emits ELF for a linux target.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] remove target_os

2009-01-27 Thread Christian Franke

Javier Martín wrote:

El mar, 27-01-2009 a las 18:21 +0100, Christian Franke escribió:
  

Javier Martín wrote:

Unfortunately, gcc has no '-fno_os' option to specify the bare CPU as 
target.



Might -ffreestanding be what you are looking for?

  
  

The option '-ffreestanding' is the same as '-fno-hosted'.

According to gcc (4.3.1) source, '-fno-hosted' clears variable 
'flag_hosted' and sets '-fno-builtin'. The latter is already set within 
GRUB build.


A cleared 'flag_hosted' apparently has only 2 effects:
- disable the special handling of 'main()'.
- #define __STDC_HOSTED__ to 0 instead of 1

There is no effect on the target_os dependent parts of the gcc code 
generation. For example, on i386, __enable_execute_stack() calls are 
generated for target_os netbsd, openbsd and cygwin, but not for linux. 
The emit call is hard-coded in 
gcc/configs/i386/i386.c:x86_initialize_trampoline().



In that case, we are dealing with a GCC bug. We might want to require
the user to create a bare no-OS cross compiler. 

  


Is building a bare 'no-OS' compiler supported by the upstream GCC sources?
Probably a too strict prerequisite for building GRUB.



 AC_MSG_CHECKING([for command to convert module to ELF format])
-case ${host_os}:${target_os} in
-  cygwin:cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
+case ${host_os} in
+  cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
   *) ;;
 esac
  
  
  
This won't work for a Linux cross compiler hosted on Cygwin. It would 
emit ELF format and does not need pe2elf.



A, say, AMD64 Linux cross compiler hosted on x86 Cygwin would have
$build=i686-pc-cygwin and $host=amd64-linux-gnu. Thus, no conflict ought
to arise even with cross compilation enabled.

  
  
But the opposite won't work: $host=i686-pc-cygwin would enable 
grub-pe2elf, even if this gcc emits ELF for a linux target.



The opposite of a the described situation would be a Linux AMD64
machine cross compiling for a Cygwin x86 machine, that is
$build=amd64-linux-gnu and $host=i686-pc-cygwin. In that case,
i686-pc-cygwin-gcc would generate PE executables, and thus grub-pe2elf
_is_ required indeed.
  


Yes - I was wrong, sorry.


Robert: Patch is IMO GTG.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] remove target_os

2009-01-26 Thread Christian Franke

Robert Millan wrote:

Hi,

Based on the description of host/target triplets in configure.ac:

dnl   build  -- the environment for building GRUB
dnl   host   -- the environment for running utilities
dnl   target -- the environment for running GRUB

it seems that target_os is an oxymoron.  There's no OS in the environment
where GRUB will run (well, there's the firmware, but we already use
target_vendor for that, and _os has a well-defined meaning).

Attached patch fixes that by supressing all references to target_os, and
replacing them with host_os where suitable.

Christian: since this mostly affects Cygwin, could you verify that it doesn't
cause breakage before I commit it?

  


Hi Robert,

thanks for sending the patch first. Cygwin build looks good.


But even if GRUB itself is build for some $target_cpu-$target_vendor 
(i386-pc), the target_os is not useless if cross-compilation support is 
desired:


- GRUB tools like grub-setup are build for the target_os
- Package structure may depend on target_os.

- The gcc code generator may emit special code tailored for the assumed 
target_os

Examples:
Stack frame checks
Nested function trampolines (__enable_execute_stack())

Unfortunately, gcc has no '-fno_os' option to specify the bare CPU as 
target.

Fortunately, all current issues can be handled by autoconf checks.



 AC_MSG_CHECKING([for command to convert module to ELF format])
-case ${host_os}:${target_os} in
-  cygwin:cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
+case ${host_os} in
+  cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
   *) ;;
 esac
  


This won't work for a Linux cross compiler hosted on Cygwin. It would 
emit ELF format and does not need pe2elf.


But all this is theoretical unless cross compilation is really needed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] (ata.mod) Fix read/write, improve status check

2009-01-22 Thread Christian Franke

Christian Franke wrote:

This patch fixes these open issues:


- grub_pio_read/write() check the ERR bit without ensuring !BSY.
- ata_read fails if (batch % size) == 0.
- ata_write does not work at all, it uses the read cmd.



Tested on several PC + Committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Add 'precision' to grub_printf %s format

2009-01-22 Thread Christian Franke

Christian Franke wrote:

This patch adds 'precision' support to grub_printf '%s' format, e.g.:

Committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Add 'precision' to grub_printf %s format

2009-01-21 Thread Christian Franke

This patch adds 'precision' support to grub_printf '%s' format, e.g.:

grub_printf(data=%.20s...\n, data);

This feature of standard printf() is useful to limit output length or to 
print strings which are not 0-terminated.


Christian


2009-01-21  Christian Franke  fra...@computer.org 


* kern/misc.c (grub_vsprintf): Fix size and termination of `format2'
(precision) digit string.  Allow `.format2' without `format1' (width).
Limit input chars for `%s' output to `format2' if specified.  This is
compatible with standard printf ().


diff --git a/kern/misc.c b/kern/misc.c
index 6fbc651..da64eb0 100644
--- a/kern/misc.c
+++ b/kern/misc.c
@@ -700,7 +700,7 @@ grub_vsprintf (char *str, const char *fmt, va_list args)
 	  char tmp[32];
 	  char *p;
 	  unsigned int format1 = 0;
-	  unsigned int format2 = 3;
+	  unsigned int format2 = ~ 0U;
 	  char zerofill = ' ';
 	  int rightfill = 0;
 	  int n;
@@ -727,20 +727,22 @@ grub_vsprintf (char *str, const char *fmt, va_list args)
 		zerofill = '0';
 	  format1 = grub_strtoul (s, 0, 10);
 	  fmt = p;
-	  if (*p  *p == '.')
+	}
+
+	  if (*p  *p == '.')
+	{
+	  p++;
+	  fmt++;
+	  while (*p  grub_isdigit (*p))
+		p++;
+
+	  if (p  fmt)
 		{
-		  p++;
-		  fmt++;
-		  while (*p  grub_isdigit (*p))
-		p++;
-		  
-		  if (p  fmt)
-		{
-		  char fstr[p - fmt];
-		  grub_strncpy (fstr, fmt, p - fmt);
-		  format2 = grub_strtoul (fstr, 0, 10);
-		  fmt = p;
-		}
+		  char fstr[p - fmt + 1];
+		  grub_strncpy (fstr, fmt, p - fmt);
+		  fstr[p - fmt] = 0;
+		  format2 = grub_strtoul (fstr, 0, 10);
+		  fmt = p;
 		}
 	}
 
@@ -847,13 +849,19 @@ grub_vsprintf (char *str, const char *fmt, va_list args)
 	  p = va_arg (args, char *);
 	  if (p)
 		{
-		  if (!rightfill  grub_strlen (p)  format1)
-		write_fill (zerofill, format1 - grub_strlen (p));
-		  
-		  write_str (p);
-		  
-		  if (rightfill  grub_strlen (p)  format1)
-		write_fill (zerofill, format1 - grub_strlen (p));
+		  grub_size_t len = 0;
+		  while (len  format2  p[len])
+		len++;
+
+		  if (!rightfill  len  format1)
+		write_fill (zerofill, format1 - len);
+
+		  grub_size_t i;
+		  for (i = 0; i  len; i++)
+		write_char (*p++);
+
+		  if (rightfill  len  format1)
+		write_fill (zerofill, format1 - len);
 		}
 	  else
 		write_str ((null));
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] (ata.mod) Fix ATAPI protocol

2009-01-20 Thread Christian Franke
Marco Gerards wrote:
  Committed, plus an additional wait in grub_atapi_identify.
 
 Great! :-)
 
  Open issues:
  - grub_pio_read/write() check the ERR bit without ensuring !BSY.
  - ata_read fails if (batch % size) == 0.
  - ata_write does not work at all, it uses the read cmd.
 
 grub_ata_write does not use the read cmd, or do you mean indirectly?
 In that case, where does it use the read command?
 

cmd_write is set, but not used:

grub_ata_readwrite (...)
{
  ...
  cmd = GRUB_ATA_CMD_READ_SECTORS;
  cmd_write = GRUB_ATA_CMD_WRITE_SECTORS;


  if (rw == 0)
{
  ...
} else {
  /* Write sectors.  */
  if (grub_ata_cmd (dev, cmd))
-^^^ GRUB_ATA_CMD_READ_SECTORS
  ...
}
  ...
}

I will post a patch for all 3 issues soon.

Christian





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] (ata.mod) Fix read/write, improve status check

2009-01-20 Thread Christian Franke

This patch fixes these open issues:


- grub_pio_read/write() check the ERR bit without ensuring !BSY.
- ata_read fails if (batch % size) == 0.
- ata_write does not work at all, it uses the read cmd.



Christian


2009-01-20  Christian Franke  fra...@computer.org

* disk/ata.c (grub_ata_wait_status): Replace by ...
(grub_ata_wait_not_busy): ... this function.  Checks only BSY bit,
other status bits may be invalid while BSY is asserted.
(grub_ata_check_ready): New function.
(grub_ata_cmd): Removed.
(grub_ata_wait_drq): New function.
(grub_ata_strncpy): Remove inline.
(grub_ata_pio_read): Reduce to actual block transfer.  BSY wait
and error check now done by grub_ata_wait_drq ().
(grub_ata_pio_write): Likewise.
(grub_atapi_identify): Set DEV before check for !BSY.  Use
grub_ata_wait_drq () to wait for data.
(grub_atapi_wait_drq): Use grub_ata_wait_not_busy ().
(grub_atapi_packet): Set DEV before check for !BSY.  Replace
transfer loop by grub_ata_pio_write ().
(grub_ata_identify): Set DEV before check for !BSY. Use
grub_ata_wait_drq () to wait for data.
	(grub_ata_setaddress): Set DEV before check for !BSY. 
	(grub_ata_readwrite): Remove duplicate code, handle batch/rest and

read/write in one loop.  Fix invalid command on write.  Fix incomplete
command on (size % batch) == 0.  Add missing error check after write of
last block.  Add debug messages.
(grub_atapi_read):  Replace transfer loop by grub_ata_pio_read ().


diff --git a/disk/ata.c b/disk/ata.c
index c0904fb..c710f13 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -152,25 +152,29 @@ grub_ata_regget2 (struct grub_ata_device *dev, int reg)
   return grub_inb (dev-ioaddress2 + reg);
 }
 
-static inline grub_err_t
-grub_ata_wait_status (struct grub_ata_device *dev,
-		  grub_uint8_t maskset, grub_uint8_t maskclear,
-		  int milliseconds)
+/* Wait for !BSY.  */
+static grub_err_t
+grub_ata_wait_not_busy (struct grub_ata_device *dev, int milliseconds)
 {
-  int i;
+  /* ATA requires 400ns (after a write to CMD register) or
+ 1 PIO cycle (after a DRQ block transfer) before
+ first check of BSY.  */
+  grub_millisleep (1);
 
-  for (i = 0; i  milliseconds; i++)
+  int i = 1;
+  while (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_BUSY)
 {
-  grub_uint8_t reg;
-
-  reg = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
-  if ((reg  maskset) == maskset  (reg  maskclear) == 0)
-	return GRUB_ERR_NONE;
+  if (i = milliseconds)
+{
+	  grub_dprintf (ata, timeout: %dms\n, milliseconds);
+	  return grub_error (GRUB_ERR_TIMEOUT, ATA timeout);
+	}
 
   grub_millisleep (1);
+  i++;
 }
 
-  return grub_error (GRUB_ERR_TIMEOUT, ata timeout);
+  return GRUB_ERR_NONE;
 }
 
 static inline void
@@ -179,19 +183,42 @@ grub_ata_wait (void)
   grub_millisleep (50);
 }
 
+/* Check for !BSY before issuing a new command.  */
+static inline grub_err_t
+grub_ata_check_ready (struct grub_ata_device *dev)
+{
+  if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_BUSY)
+return grub_ata_wait_not_busy (dev, GRUB_ATA_TOUT_STD);
+
+  return GRUB_ERR_NONE;
+}
+
+/* Wait for !BSY, DRQ.  */
 static grub_err_t
-grub_ata_cmd (struct grub_ata_device *dev, int cmd)
+grub_ata_wait_drq (struct grub_ata_device *dev, int rw,
+		   int milliseconds)
 {
-  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY, GRUB_ATA_TOUT_STD))
+  if (grub_ata_wait_not_busy (dev, milliseconds))
 return grub_errno;
 
-  grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd);
+  /* !DRQ implies error condition.  */
+  grub_uint8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
+  if ((sts  (GRUB_ATA_STATUS_DRQ | GRUB_ATA_STATUS_ERR))
+  != GRUB_ATA_STATUS_DRQ)
+{
+  grub_dprintf (ata, ata error: status=0x%x, error=0x%x\n,
+		sts, grub_ata_regget (dev, GRUB_ATA_REG_ERROR));
+  if (! rw)
+return grub_error (GRUB_ERR_READ_ERROR, ATA read error);
+  else
+return grub_error (GRUB_ERR_WRITE_ERROR, ATA write error);
+}
 
   return GRUB_ERR_NONE;
 }
 
 /* Byteorder has to be changed before strings can be read.  */
-static inline void
+static void
 grub_ata_strncpy (char *dst, char *src, grub_size_t len)
 {
   grub_uint16_t *src16 = (grub_uint16_t *) src;
@@ -203,52 +230,26 @@ grub_ata_strncpy (char *dst, char *src, grub_size_t len)
   dst[len] = '\0';
 }
 
-static grub_err_t
-grub_ata_pio_read (struct grub_ata_device *dev, char *buf,
-		   grub_size_t size, int milliseconds)
+static void
+grub_ata_pio_read (struct grub_ata_device *dev, char *buf, grub_size_t size)
 {
   grub_uint16_t *buf16 = (grub_uint16_t *) buf;
   unsigned int i;
 
-  if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_ERR)
-return grub_error (GRUB_ERR_READ_ERROR, ATA read error);
-
-  /* Wait until the data is available.  */
-  if (grub_ata_wait_status (dev

Re: [PATCH] (ata.mod) Fix ATAPI protocol

2009-01-19 Thread Christian Franke
Marco Gerards wrote:
 Christian Franke ... writes:
 
  This patch fixes the data I/O protocol of the ATA PACKET command.
  
  The current implementation does not read the byte count
  registers. ATAPI read may not work if the drive sends the data in
  more than one block.
  
  In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD
  drives should now work.
  
 
 Sorry for not commenting on many things on this list.  I am too busy
 to check all this.  

N.P.

Meantime, I have tested the SCSI and ATAPI fixes on several PC. If there
are no further comments, I will commit these soon.

Christian





___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] (scsi.mod) Fix SCSI read for blocksize != 512

2009-01-19 Thread Christian Franke

Christian Franke wrote:

This patch fixes SCSI blocksize handling, necessary for ATAPI CD/DVD.


Committed.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] (ata.mod) Fix ATAPI protocol

2009-01-19 Thread Christian Franke

Christian Franke wrote:

Marco Gerards wrote:
  

Christian Franke ... writes:



This patch fixes the data I/O protocol of the ATA PACKET command.

The current implementation does not read the byte count
registers. ATAPI read may not work if the drive sends the data in
more than one block.

In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD
drives should now work.

  

Sorry for not commenting on many things on this list.  I am too busy
to check all this.  



N.P.

Meantime, I have tested the SCSI and ATAPI fixes on several PC. If there
are no further comments, I will commit these soon.

  


Committed, plus an additional wait in grub_atapi_identify.

Open issues:
- grub_pio_read/write() check the ERR bit without ensuring !BSY.
- ata_read fails if (batch % size) == 0.
- ata_write does not work at all, it uses the read cmd.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] (ata.mod) Fix ATAPI protocol

2009-01-18 Thread Christian Franke

This patch fixes the data I/O protocol of the ATA PACKET command.

The current implementation does not read the byte count registers. ATAPI 
read may not work if the drive sends the data in more than one block.


In conjunction with my recent SCSI blocksize patch, ATAPI CD/DVD drives 
should now work.


Christian

2008-01-18  Christian Franke  fra...@computer.org

* disk/ata.c (GRUB_ATAPI_REG_*): New defines.
(GRUB_ATAPI_IREASON_*): Likewise.
(grub_ata_pio_write): Fix timeout error return.
(grub_atapi_wait_drq): New function.
(grub_atapi_packet): New parameter `size'.
Use grub_atapi_wait_drq () and direct write instead of
grub_ata_pio_write ().
(grub_atapi_read): Replace grub_ata_pio_read () by a loop which
reads the number of bytes requested by the device for each DRQ
assertion.
(grub_atapi_write): Remove old implementation, return not
implemented instead.


diff --git a/disk/ata.c b/disk/ata.c
index af7158c..bb891d4 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -44,12 +44,15 @@ static const int grub_ata_ioaddress2[] = { 0x3f6, 0x376 };
 #define GRUB_ATA_REG_ERROR	1
 #define GRUB_ATA_REG_FEATURES	1
 #define GRUB_ATA_REG_SECTORS	2
+#define GRUB_ATAPI_REG_IREASON	2
 #define GRUB_ATA_REG_SECTNUM	3
 #define GRUB_ATA_REG_CYLLSB	4
 #define GRUB_ATA_REG_CYLMSB	5
 #define GRUB_ATA_REG_LBALOW	3
 #define GRUB_ATA_REG_LBAMID	4
+#define GRUB_ATAPI_REG_CNTLOW	4
 #define GRUB_ATA_REG_LBAHIGH	5
+#define GRUB_ATAPI_REG_CNTHIGH	5
 #define GRUB_ATA_REG_DISK	6
 #define GRUB_ATA_REG_CMD	7
 #define GRUB_ATA_REG_STATUS	7
@@ -65,6 +68,13 @@ static const int grub_ata_ioaddress2[] = { 0x3f6, 0x376 };
 #define GRUB_ATA_STATUS_READY	0x40
 #define GRUB_ATA_STATUS_BUSY	0x80
 
+/* ATAPI interrupt reason values (I/O, D/C bits).  */
+#define GRUB_ATAPI_IREASON_MASK 0x3
+#define GRUB_ATAPI_IREASON_DATA_OUT 0x0
+#define GRUB_ATAPI_IREASON_CMD_OUT  0x1
+#define GRUB_ATAPI_IREASON_DATA_IN  0x2
+#define GRUB_ATAPI_IREASON_ERROR0x3
+
 enum grub_ata_commands
   {
 GRUB_ATA_CMD_READ_SECTORS = 0x20,
@@ -229,7 +239,7 @@ grub_ata_pio_write (struct grub_ata_device *dev, char *buf,
 
   /* Wait until drive is ready to read data.  */
   if (grub_ata_wait_status (dev, GRUB_ATA_STATUS_DRQ, 0, milliseconds))
-return 0;
+return grub_errno;
 
   /* Write the data, word by word.  */
   for (i = 0; i  size / 2; i++)
@@ -300,21 +310,65 @@ grub_atapi_identify (struct grub_ata_device *dev)
 }
 
 static grub_err_t
-grub_atapi_packet (struct grub_ata_device *dev, char *packet)
+grub_atapi_wait_drq (struct grub_ata_device *dev,
+		 grub_uint8_t ireason,
+		 int milliseconds)
 {
+  grub_millisleep (1); /* ATA allows 400ns to assert BSY.  */
+
+  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY, milliseconds))
+return grub_errno;
+
+  grub_uint8_t sts = grub_ata_regget (dev, GRUB_ATA_REG_STATUS);
+  grub_uint8_t irs = grub_ata_regget (dev, GRUB_ATAPI_REG_IREASON);
+
+  /* OK if DRQ is asserted and interrupt reason is as expected.  */
+  if ((sts  GRUB_ATA_STATUS_DRQ)
+   (irs  GRUB_ATAPI_IREASON_MASK) == ireason)
+return GRUB_ERR_NONE;
+
+  /* No DRQ implies error condition.  */
+  grub_dprintf(ata, atapi error: status=0x%x, ireason=0x%x, error=0x%x\n,
+	   sts, irs, grub_ata_regget (dev, GRUB_ATA_REG_ERROR));
+
+  if (! (sts  GRUB_ATA_STATUS_DRQ)
+   (irs  GRUB_ATAPI_IREASON_MASK) == GRUB_ATAPI_IREASON_ERROR)
+{
+  if (ireason == GRUB_ATAPI_IREASON_CMD_OUT)
+	return grub_error (GRUB_ERR_READ_ERROR, ATA PACKET command error);
+  else
+	return grub_error (GRUB_ERR_READ_ERROR, ATAPI read error);
+}
+
+  return grub_error (GRUB_ERR_READ_ERROR, ATAPI protocol error);
+}
+
+static grub_err_t
+grub_atapi_packet (struct grub_ata_device *dev, char *packet,
+		   grub_size_t size)
+{
+  if (grub_ata_wait_status(dev, 0, GRUB_ATA_STATUS_BUSY, GRUB_ATA_TOUT_STD))
+return grub_errno;
+
+  /* Send ATA PACKET command.  */
   grub_ata_regset (dev, GRUB_ATA_REG_DISK, dev-device  4);
   grub_ata_regset (dev, GRUB_ATA_REG_FEATURES, 0);
-  grub_ata_regset (dev, GRUB_ATA_REG_SECTORS, 0);
-  grub_ata_regset (dev, GRUB_ATA_REG_LBAHIGH, 0xFF);
-  grub_ata_regset (dev, GRUB_ATA_REG_LBAMID, 0xFF);
+  grub_ata_regset (dev, GRUB_ATAPI_REG_IREASON, 0);
+  grub_ata_regset (dev, GRUB_ATAPI_REG_CNTHIGH, size  8);
+  grub_ata_regset (dev, GRUB_ATAPI_REG_CNTLOW, size  0xFF);
 
   grub_ata_regset (dev, GRUB_ATA_REG_CMD, GRUB_ATA_CMD_PACKET);
 
-  grub_ata_wait ();
-
-  if (grub_ata_pio_write (dev, packet, 12, GRUB_ATA_TOUT_STD))
+  /* Wait for !BSY, DRQ, !I/O, C/D.  */
+  if (grub_atapi_wait_drq (dev, GRUB_ATAPI_IREASON_CMD_OUT, GRUB_ATA_TOUT_STD))
 return grub_errno;
 
+  /* Write the packet.  */
+  grub_uint16_t *buf16 = (grub_uint16_t *) packet;
+  unsigned i;
+  for (i = 0; i  12 / 2; i++)
+grub_outw (grub_cpu_to_le16 (buf16[i]), dev-ioaddress + GRUB_ATA_REG_DATA);
+
   return GRUB_ERR_NONE;
 }
 
@@ -847,27 +901,51

[PATCH] (scsi.mod) Fix SCSI read for blocksize != 512

2009-01-17 Thread Christian Franke

This patch fixes SCSI blocksize handling, necessary for ATAPI CD/DVD.

OT: There might be a memory leak in scsi_open(): free(scsi) is missing, 
at least on open error.


Christian


2009-01-17  Christian Franke  fra...@computer.org

* disk/scsi.c (grub_scsi_read10): Use scsi-blocksize instead
of 512 to calculate data size.
(grub_scsi_read12): Likewise.
(grub_scsi_write10): Likewise.
(grub_scsi_write12): Likewise.
(grub_scsi_read): Adjust size according to blocksize.
Add checks for invalid blocksize and unaligned transfer.


diff --git a/disk/scsi.c b/disk/scsi.c
index b7b6834..d165309 100644
--- a/disk/scsi.c
+++ b/disk/scsi.c
@@ -119,7 +119,7 @@ grub_scsi_read10 (grub_disk_t disk, grub_disk_addr_t sector,
   rd.reserved2 = 0;
   rd.pad = 0;
 
-  return scsi-dev-read (scsi, sizeof (rd), (char *) rd, size * 512, buf);
+  return scsi-dev-read (scsi, sizeof (rd), (char *) rd, size * scsi-blocksize, buf);
 }
 
 /* Send a SCSI request for DISK: read SIZE sectors starting with
@@ -140,7 +140,7 @@ grub_scsi_read12 (grub_disk_t disk, grub_disk_addr_t sector,
   rd.reserved = 0;
   rd.control = 0;
 
-  return scsi-dev-read (scsi, sizeof (rd), (char *) rd, size * 512, buf);
+  return scsi-dev-read (scsi, sizeof (rd), (char *) rd, size * scsi-blocksize, buf);
 }
 
 #if 0
@@ -163,7 +163,7 @@ grub_scsi_write10 (grub_disk_t disk, grub_disk_addr_t sector,
   wr.reserved2 = 0;
   wr.pad = 0;
 
-  return scsi-dev-write (scsi, sizeof (wr), (char *) wr, size * 512, buf);
+  return scsi-dev-write (scsi, sizeof (wr), (char *) wr, size * scsi-blocksize, buf);
 }
 
 /* Send a SCSI request for DISK: write the data stored in BUF to SIZE
@@ -184,7 +184,7 @@ grub_scsi_write12 (grub_disk_t disk, grub_disk_addr_t sector,
   wr.reserved = 0;
   wr.pad = 0;
 
-  return scsi-dev-write (scsi, sizeof (wr), (char *) wr, size * 512, buf);
+  return scsi-dev-write (scsi, sizeof (wr), (char *) wr, size * scsi-blocksize, buf);
 }
 #endif
 
@@ -325,8 +325,22 @@ grub_scsi_read (grub_disk_t disk, grub_disk_addr_t sector,
 
   /* SCSI sectors are variable in size.  GRUB uses 512 byte
  sectors.  */
-  sector = grub_divmod64 (sector, scsi-blocksize  GRUB_DISK_SECTOR_BITS,
-			  NULL);
+  if (scsi-blocksize != GRUB_DISK_SECTOR_SIZE)
+{
+  unsigned spb = scsi-blocksize  GRUB_DISK_SECTOR_BITS;
+  if (! (spb != 0  (scsi-blocksize  GRUB_DISK_SECTOR_SIZE) == 0))
+	return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			   Unsupported SCSI block size);
+
+  grub_int32_t sector_mod = 0;
+  sector = grub_divmod64 (sector, spb, sector_mod);
+
+  if (! (sector_mod == 0  size % spb == 0))
+	return grub_error (GRUB_ERR_NOT_IMPLEMENTED_YET,
+			   Unaligned SCSI read not supported);
+
+  size /= spb;
+}
 
   /* Depending on the type, select a read function.  */
   switch (scsi-devtype)
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] (ata.mod) Add variable timeouts, fix identify, fix device selection

2009-01-16 Thread Christian Franke

Christian Franke wrote:
This patch fixes the following issues in ata.mod I found during 
testing on several PC and VMs:

...


Committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] (ata.mod) Add variable timeouts, fix identify, fix device selection

2009-01-16 Thread Christian Franke

Christian Franke wrote:

...
Other issues found, not addressed in this patch:
...
- atapi_read does not always work (occasional timeouts, crash on VmWare)


Root of this problem is in scsi.mod: It always assumes a sector size of 
512 bytes which is not the case for ATAPI CD/DVD.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] (ata.mod) Add variable timeouts, fix identify, fix device selection

2009-01-14 Thread Christian Franke
This patch fixes the following issues in ata.mod I found during testing 
on several PC and VMs:


- The 1s timeout is too short for read, at least for CD/DVD. Now use 
variable timeout depending on command.


- The DEV bit is not set before other registers are set. If both master 
and slave are present, the registers of the wrong device may be set.


- DEVICE DIAGNOSTICS may not work for device detection for several 
reasons, see changelog.
Now uses only IDENTIFY and then IDENTIFY PACKET, like (at least) the 
traditional Linux IDE driver did.


Other issues found, not addressed in this patch:
- ata_read fails if (batch % size) == 0.
- ata_write does not work at all, it uses the read cmd.
- atapi_read does not always work (occasional timeouts, crash on VmWare)
I will provide patches for these later.

Christian

2009-01-14  Christian Franke  fra...@computer.org

* disk/ata.c (enum grub_ata_commands): Remove EXEC_DEV_DIAGNOSTICS.
(enum grub_ata_timeout_milliseconds): New enum.
(grub_ata_wait_status): Add parameter milliseconds.
(grub_ata_cmd): Remove variable `err'.  Remove wait for !DRQ to allow
recovery from timed-out commands.
(grub_ata_pio_read): Add parameter milliseconds.  Fix error return,
return grub_errno instead of REG_ERROR.
(grub_ata_pio_write): Add parameter milliseconds.
(grub_atapi_identify): Fix size of ATAPI IDENTIFY sector.
Pass milliseconds to grub_ata_wait_status () and
grub_ata_pio_read ().
(grub_atapi_packet): Pass milliseconds to grub_ata_pio_write ().
(grub_ata_identify): Remove variable `ataerr'.  Pass milliseconds to
grub_ata_wait_status ().  Fix IDENTIFY timeout check.
(grub_ata_device_initialize): Remove EXECUTE DEVICE DIAGNOSTICS.
It is not suitable for device detection, because DEV bit is ignored,
the command may run too long, and not all devices set the signature
properly.
(grub_ata_pciinit): Clear grub_errno before grub_ata_device_initialize 
().
(grub_ata_setaddress): Pass milliseconds to grub_ata_wait_status ().
Fix device selection, DEV bit must be set first to address the registers
of the correct device.
(grub_ata_readwrite): Pass milliseconds to grub_ata_wait_status () and
grub_ata_pio_read/write ().
(grub_atapi_read): Pass milliseconds to grub_ata_pio_read ().
(grub_atapi_write): Pass milliseconds to grub_ata_pio_write ().


diff --git a/disk/ata.c b/disk/ata.c
index 4ca63c2..af7158c 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -74,7 +74,12 @@ enum grub_ata_commands
 GRUB_ATA_CMD_IDENTIFY_DEVICE = 0xEC,
 GRUB_ATA_CMD_IDENTIFY_PACKET_DEVICE = 0xA1,
 GRUB_ATA_CMD_PACKET = 0xA0,
-GRUB_ATA_CMD_EXEC_DEV_DIAGNOSTICS = 0x90
+  };
+
+enum grub_ata_timeout_milliseconds
+  {
+GRUB_ATA_TOUT_STD  =  1000,  /* 1s standard timeout.  */
+GRUB_ATA_TOUT_DATA = 1   /* 10s DATA I/O timeout.  */
   };
 
 struct grub_ata_device
@@ -139,11 +144,12 @@ grub_ata_regget2 (struct grub_ata_device *dev, int reg)
 
 static inline grub_err_t
 grub_ata_wait_status (struct grub_ata_device *dev,
-		  grub_uint8_t maskset, grub_uint8_t maskclear)
+		  grub_uint8_t maskset, grub_uint8_t maskclear,
+		  int milliseconds)
 {
   int i;
 
-  for (i = 0; i  1000; i++)
+  for (i = 0; i  milliseconds; i++)
 {
   grub_uint8_t reg;
 
@@ -166,12 +172,8 @@ grub_ata_wait (void)
 static grub_err_t
 grub_ata_cmd (struct grub_ata_device *dev, int cmd)
 {
-  grub_err_t err;
-
-  err = grub_ata_wait_status (dev, 0, 
-			  GRUB_ATA_STATUS_DRQ | GRUB_ATA_STATUS_BUSY);
-  if (err)
-return err;
+  if (grub_ata_wait_status (dev, 0, GRUB_ATA_STATUS_BUSY, GRUB_ATA_TOUT_STD))
+return grub_errno;
 
   grub_ata_regset (dev, GRUB_ATA_REG_CMD, cmd);
 
@@ -193,16 +195,16 @@ grub_ata_strncpy (char *dst, char *src, grub_size_t len)
 
 static grub_err_t
 grub_ata_pio_read (struct grub_ata_device *dev, char *buf,
-		   grub_size_t size)
+		   grub_size_t size, int milliseconds)
 {
   grub_uint16_t *buf16 = (grub_uint16_t *) buf;
   unsigned int i;
 
   if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_ERR)
-return grub_ata_regget (dev, GRUB_ATA_REG_ERROR);
+return grub_error (GRUB_ERR_READ_ERROR, ATA read error);
 
   /* Wait until the data is available.  */
-  if (grub_ata_wait_status (dev, GRUB_ATA_STATUS_DRQ, 0))
+  if (grub_ata_wait_status (dev, GRUB_ATA_STATUS_DRQ, 0, milliseconds))
 return grub_errno;;
 
   /* Read in the data, word by word.  */
@@ -217,16 +219,16 @@ grub_ata_pio_read (struct grub_ata_device *dev, char *buf,
 
 static grub_err_t
 grub_ata_pio_write (struct grub_ata_device *dev, char *buf,
-		grub_size_t size)
+		grub_size_t size, int milliseconds)
 {
   grub_uint16_t *buf16 = (grub_uint16_t *) buf;
   unsigned int i;
 
   if (grub_ata_regget (dev, GRUB_ATA_REG_STATUS)  GRUB_ATA_STATUS_ERR)
-return grub_ata_regget (dev

[PATCH] Fix ATA compatibility mode check

2009-01-11 Thread Christian Franke
For the compatibility mode check, the ATA driver uses the bits in the 
Revision byte instead of the Programming Interface Byte of the class 
value (See T13/1510D). The compat_use[] flags are never set.


This patch fixes both issues.

Christian

2009-01-11  Christian Franke  fra...@computer.org

* disk/ata.c (grub_ata_pciinit): Fix bit numbers of compatibility
mode check.  Fix setting of compat_use[].


diff --git a/disk/ata.c b/disk/ata.c
index 6e0b8b8..4ca63c2 100644
--- a/disk/ata.c
+++ b/disk/ata.c
@@ -504,7 +504,7 @@ grub_ata_pciinit (int bus, int device, int func,
   for (i = 0; i  2; i++)
 {
   /* Set to 0 when the channel operated in compatibility mode.  */
-  int compat = (class  (2 * i))  1;
+  int compat = (class  (8 + 2 * i))  1;
 
   rega = 0;
   regb = 0;
@@ -515,7 +515,7 @@ grub_ata_pciinit (int bus, int device, int func,
 	{
 	  rega = grub_ata_ioaddress[i];
 	  regb = grub_ata_ioaddress2[i];
-	  compat_use[i] = 0;
+	  compat_use[i] = 1;
 	}
   else if (compat)
 	{
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: __FILE__ (Re: [PATCH] framework for building modules externally)

2008-11-06 Thread Christian Franke

Robert Millan wrote:

On Wed, Nov 05, 2008 at 10:41:20PM +0100, Christian Franke wrote:
  
PS: The current use of __FILE__ may also add extra unexpected size: For 
packaging, configure is often run outside of $srcdir with a absolute 
path name. This may result in long __FILE__ strings, like

/home/maintainer/packaging/grub/tmp/grub-1.96+20081105-1/src/grub-1.96/kern/disk.c



This has annoyed me for a while.  Do you know a proper fix?

  
There is apparently no option to modify __FILE__ expansion. Therefore, 
the relative file name has to be specified in the module itself.


See attached patch for a possible fix: Each module using grub_dprintf 
(here disk.c) may specify its name in 'this_file'. When all modules are 
changed, the '#define this_file' and all #undefs can be removed.


Christian

diff --git a/include/grub/misc.h b/include/grub/misc.h
index 15c18f5..36e4bcc 100644
--- a/include/grub/misc.h
+++ b/include/grub/misc.h
@@ -27,7 +27,9 @@
 
 #define ALIGN_UP(addr, align) (((grub_uint64_t)addr + align - 1)  ~(align - 1))
 
-#define grub_dprintf(condition, fmt, args...) grub_real_dprintf(__FILE__, __LINE__, condition, fmt, ## args);
+#define grub_dprintf(condition, fmt, args...) grub_real_dprintf(this_file, __LINE__, condition, fmt, ## args);
+#define this_file __FILE__
+
 /* XXX: If grub_memmove is too slow, we must implement grub_memcpy.  */
 #define grub_memcpy(d,s,n)	grub_memmove ((d), (s), (n))
 
diff --git a/kern/disk.c b/kern/disk.c
index ed82506..039c011 100644
--- a/kern/disk.c
+++ b/kern/disk.c
@@ -25,6 +25,9 @@
 #include grub/time.h
 #include grub/file.h
 
+#undef this_file
+static const char this_file[] = kern/disk.c;
+
 #define	GRUB_CACHE_TIMEOUT	2
 
 /* The last time the disk was used.  */
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] framework for building modules externally

2008-11-04 Thread Christian Franke

Robert Millan wrote:

...
How do you treat differences in ABI ?

Dis-allow loading of module with different value of ABI ? Or were you
planning that module itself adapts to different versions of GRUB 2 ABI's?



The module itself could:

GRUB_MOD_INIT(foo)
{
  if (grub_abi != GRUB_ABI)
{
  grub_printf (abi mismatch!\n);
  return;
}

  /* register our commands/terminals/disks/whatever */
}

  


Alternative: Export a symbol describing the ABI version in kernel 
(grub_abi_3_14). Access this symbol in each module (this can be hidden 
in GRUB_MOD_INIT).


Then loading a module with wrong ABI version fails due to undefined symbol.

--
Christian Franke



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Add __enable_execute_stack() if required

2008-09-19 Thread Christian Franke

walt wrote:

Robert Millan wrote:

On Tue, Sep 16, 2008 at 09:42:50PM +0200, Christian Franke wrote:

Some gcc versions generate a call to __enable_execute_stack() in
trampolines for nested functions. This is the case for new Cygwin 
gcc-4.3.2.


Other GRUB2 target platforms may be affected - the following files in
'gcc-4.3.2/gcc/config' source directory contains implementations of 
this

function for libgcc:

alpha/osf.h
darwin.h
netbsd.h


I recall this was also an issue for NetBSD indeed.  Could someone 
confirm it

fixes the problem there?


It does fix the problem on NetBSD, thanks Christian.



You're welcome. Thanks for testing.

Patch committed.

If more such functions are necessary in the future, it is probably 
better to collect those in some libgcc.a replacement module.
This would be the case if e.g. for the 64-bit integer division. On 
32-bit architectures, gcc typically generates a call to __divdi3().


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Add __enable_execute_stack() if required

2008-09-17 Thread Christian Franke

Robert Millan wrote:

On Tue, Sep 16, 2008 at 09:42:50PM +0200, Christian Franke wrote:
  
Some gcc versions generate a call to __enable_execute_stack() in 
trampolines for nested functions. This is the case for new Cygwin gcc-4.3.2.


Other GRUB2 target platforms may be affected - the following files in 
'gcc-4.3.2/gcc/config' source directory contains implementations of this 
function for libgcc:


alpha/osf.h
darwin.h
netbsd.h



I recall this was also an issue for NetBSD indeed.  Could someone confirm it
fixes the problem there?

  


There was also a related discussion for Mac OS X here:
http://lists.gnu.org/archive/html/grub-devel/2005-12/msg00044.html

If there are no complaints, I will commit the patch on friday.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] fix disk-id abuse

2008-09-02 Thread Christian Franke

Robert Millan wrote:

On Tue, Sep 02, 2008 at 09:12:04PM +0200, Christian Franke wrote:
  
If disk-id is supposed to be a GUID ('Grub Unique Identifier' in this 
case :-), then a pointer to the private data structure for the disk 
should work. This id is unique until disk close.


For drivers without disk-data, simply use the address of e.g. the open 
function itself.



This is fine for single-disk drivers, but for multi-disk ones we need it to
be unique among different disks provided by the same driver.

  


I apparently made the false assumption, that multi-disk drivers always 
have disk-data as a real pointer.




Although, of course, I don't see why can't we just make it use a pointer to
itself:

  disk-id = (unsigned long) disk-id;

but then what's the point of storing that in a variable anyway.  We might as
well just remove this variable and whoever uses it can use a pointer to the
structure instead?

This works on the assumption that disk structures are never reallocated, but
I suppose that's a sane thing to assume...

  


Sounds good.

Probably add an inline function to improve readability:

typedef unsigned long grub_disk_id_t;

inline grub_disk_id_t disk_id (const grub_disk_t * disk)
{
 return (grub_id_t)(disk);
}

...
data = grub_disk_cache_fetch (disk-dev-id, disk_id(disk), start_sector);


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Mingw support for grub2

2008-08-24 Thread Christian Franke

Bean wrote:

Hi,

This patch add support for mingw, now you can create native executable
for windows.

  


Nice!

Does grub-setup work?



...
--- a/include/grub/util/misc.h
+++ b/include/grub/util/misc.h
...
+#ifdef __MINGW32__
+
+#include windows.h
+
+grub_int64_t fseeko (FILE *fp, grub_int64_t offset, int whence);
+grub_int64_t ftello (FILE *fp);
  


The mingw runtime provides fseeko64/ftello64(), see 
/usr/include/mingw/stdio.h


So the following may work:

#ifdef __MINGW32__
#define fseeko fseeko64
#define ftello ftello64
#endif

or use inline functions.



+void sync (void);
+int asprintf (char **buf, const char *fmt, ...);
+
  


I would suggest to add AC_CHECK_FUNC(asprintf) to configure.
asprintf() is a GNU extension and not part of C99 or POSIX.



+grub_int64_t grub_util_get_disk_size (char *name);
+
+#define sleep  Sleep
  


The Sleep() parameter specifies milliseconds.

#define sleep(s) Sleep((s)*1000)
or
inline void sleep(unsigned s) { Sleep(s * 1000); }
or
add sleep() to util/misc.c to avoid global inclusion of the namespace 
polluter windows.h :-)



Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Mingw support for grub2

2008-08-24 Thread Christian Franke

Vesa Jääskeläinen wrote:

Bean wrote:
  

On Mon, Aug 25, 2008 at 12:04 AM, Vesa Jääskeläinen [EMAIL PROTECTED] wrote:


Bean wrote:
  

Yes, it works, although mingw can't use device names like /dev/sda,
but it can use windows special name //./PHYSICALDRIVE0.


That should be with back slashes ('\'). Or is there some hack somewhere
to support that?
  

Hi,

Mingw would translate / to \, so both are ok. But using / is more
clear, as \ is the escape character in cygwin shell, so we need to use
two of them, .\\PHYSICALDRIVE0.



In cygwin shell... yes... but mingw programs are not meant to be run
under that. Mingw programs are native windows programs.


  


Unlike Cygwin, Mingw does not provide an extra library layer. Most 
standard library functions directly link to msvcrt.dll.


The Win32 API (like the DOS API) itself accepts both / and \, only the 
user level tools usually don't. This is likely because in the early 
(CP/M-DOS) days, someone decided to use '/' as the option char. Later, 
when path names are 'borrowed' from Unix, it was too late :-)


In fact, all these work to open the first disk:
h = CreateFile(.\\PhysicalDrive0, ...)
h = CreateFile(./PhysicalDrive0, ...)
h = CreateFile(//.\\PhysicalDrive0, ...)
h = CreateFile(//./PhysicalDrive0, ...)

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Don't install Cygwin specific files elsewhere

2008-08-09 Thread Christian Franke

Christian Franke wrote:

This patch prevents the install of grub.d/10_windows on other OS.

grub-pe2elf is only installed if requested by --enable-grub-pe2elf. 
Even on Cygwin, grub-pe2elf is only required during build.





Committed.



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: bug: cygwin support breaks cross compiles on cygwin

2008-08-09 Thread Christian Franke

Felix Zielcke wrote:

Am Freitag, den 08.08.2008, 23:13 +0200 schrieb Christian Franke:
  
I agree. I posted a patch which handles both grub.d/10_windows and 
grub-pe2elf.





Thanks I even saw that mail before this.
I even reverted now my rm -f 10_windows change to current Debian trunk,
but to commit I wait untill your patch is commited upstream ;)



  


Done.

The quick fix for the cross-compiler problem is also included. Does not 
actually check the output format of the target compiler, but should work 
for now.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: bug: cygwin support breaks cross compiles on cygwin

2008-08-08 Thread Christian Franke

Patrick Georgi wrote:

Hi,

I used cygwin to build grub2/i386-coreboot using an i386-elf cross 
compiler. unfortunately the test in configure only looks for the host 
platform and enables pe2elf in that case, even though my compiler 
emits ELF already.


my workaround is to just change that test (there's no OS cyg_win), but 
of course that's no permanent solution.
Bean proposed on IRC to look if the target compiler emits PE, and 
enable pe2elf based on that.




Thanks for the info.

A simple fix is probably a test for target_os also, please try attached 
patch (and re-run autoconf).


Christian

diff --git a/configure.ac b/configure.ac
index 7a5938a..41e08f1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -206,9 +206,10 @@ AC_MSG_RESULT([$TARGET_IMG_LDFLAGS_AC])
 
 # For platforms where ELF is not the default link format.
 AC_MSG_CHECKING([for command to convert module to ELF format])
-if test $host_os = cygwin; then
-  TARGET_OBJ2ELF='grub-pe2elf.exe'
-fi
+case ${host_os}:${target_os} in
+  cygwin:cygwin) TARGET_OBJ2ELF='grub-pe2elf.exe' ;;
+  *) ;;
+esac
 AC_SUBST(TARGET_OBJ2ELF)
 AC_MSG_RESULT([$TARGET_OBJ2ELF])
 
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: bug: cygwin support breaks cross compiles on cygwin

2008-08-08 Thread Christian Franke

Felix Zielcke wrote:

Hello,

Am Freitag, den 08.08.2008, 14:59 +0200 schrieb Christian Franke:
  
No, grub-pe2elf is build and installed even if is not necessary for the 
build process. This should be no problem.





Well it's the same as with that --enable-debug thingy ;)

In our Debian trunk SVN I already commited a change to remove
grub.d/10_windows because Debian/Ubuntu users just don't have a need for
this.
It would be better if Robert and me don't need to do such changes.
We could just remove grub-pe2elf, too.
But why should it be build for everybody if only Cygwin users have a use
for it?
Then clearly the whole build process needs a change so that grub-pe2elf
is only build on Cygwin.
  


I agree. I posted a patch which handles both grub.d/10_windows and 
grub-pe2elf.


Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Don't install Cygwin specific files elsewhere

2008-08-08 Thread Christian Franke

This patch prevents the install of grub.d/10_windows on other OS.

grub-pe2elf is only installed if requested by --enable-grub-pe2elf. Even 
on Cygwin, grub-pe2elf is only required during build.


Christian

2008-08-08  Christian Franke  [EMAIL PROTECTED]

* Makefile.in: Add `target_os' and `enable_grub_pe2elf'.
* conf/common.rmk: Install `grub-pe2elf' only if requested.
Install `grub.d/10_windows' only on Cygwin.
* configure.ac: Add subst of `target_os'.
Check `target_os' also before setting TARGET_OBJ2ELF.
Add `--enable-grub-pe2elf'.


diff --git a/Makefile.in b/Makefile.in
index 2121431..6137974 100644
--- a/Makefile.in
+++ b/Makefile.in
@@ -48,6 +48,7 @@ PACKAGE_STRING = @PACKAGE_STRING@
 PACKAGE_BUGREPORT = @PACKAGE_BUGREPORT@
 
 target_cpu = @target_cpu@
+target_os = @target_os@
 platform = @platform@
 
 INSTALL = @INSTALL@
@@ -92,6 +93,7 @@ UNIFONT_HEX = @UNIFONT_HEX@
 # Options.
 enable_grub_emu = @enable_grub_emu@
 enable_grub_fstest = @enable_grub_fstest@
+enable_grub_pe2elf = @enable_grub_pe2elf@
 enable_lzo = @enable_lzo@
 
 ### General variables.
diff --git a/conf/common.rmk b/conf/common.rmk
index 3d674a6..95859f7 100644
--- a/conf/common.rmk
+++ b/conf/common.rmk
@@ -100,7 +100,10 @@ grub_editenv_SOURCES = util/grub-editenv.c lib/envblk.c util/misc.c kern/misc.c
 CLEANFILES += grub-editenv
 
 # for grub-pe2elf
+ifeq ($(enable_grub_pe2elf), yes)
 bin_UTILITIES += grub-pe2elf
+endif
+
 grub_pe2elf_SOURCES = util/grub-pe2elf.c util/misc.c
 CLEANFILES += grub-pe2elf
 
@@ -120,7 +123,11 @@ CLEANFILES += update-grub_lib
 %: util/grub.d/%.in config.status
 	./config.status --file=$@:$
 	chmod +x $@
-update-grub_SCRIPTS = 00_header 10_linux 10_hurd 10_windows 30_os-prober 40_custom
+update-grub_SCRIPTS = 00_header 10_linux 10_hurd 30_os-prober 40_custom
+ifeq ($(target_os), cygwin)
+update-grub_SCRIPTS += 10_windows
+endif
+
 CLEANFILES += $(update-grub_SCRIPTS)
 
 update-grub_DATA += util/grub.d/README
diff --git a/configure.ac b/configure.ac
index 7a5938a..f561b8b 100644
--- a/configure.ac
+++ b/configure.ac
@@ -100,6 +100,7 @@ case $target_cpu in
 esac
 
 AC_SUBST(target_cpu)
+AC_SUBST(target_os)
 AC_SUBST(platform)
 
 #
@@ -206,9 +207,10 @@ AC_MSG_RESULT([$TARGET_IMG_LDFLAGS_AC])
 
 # For platforms where ELF is not the default link format.
 AC_MSG_CHECKING([for command to convert module to ELF format])
-if test $host_os = cygwin; then
-  TARGET_OBJ2ELF='grub-pe2elf.exe'
-fi
+case ${host_os}:${target_os} in
+  cygwin:cygwin) TARGET_OBJ2ELF='grub-pe2elf' ;;
+  *) ;;
+esac
 AC_SUBST(TARGET_OBJ2ELF)
 AC_MSG_RESULT([$TARGET_OBJ2ELF])
 
@@ -385,6 +387,11 @@ AC_ARG_ENABLE([grub-fstest],
  [build and install the `grub-fstest' debugging utility])])
 AC_SUBST([enable_grub_fstest])
 
+AC_ARG_ENABLE([grub-pe2elf],
+	  [AS_HELP_STRING([--enable-grub-pe2elf],
+ [build and install the `grub-pe2elf' conversion utility])])
+AC_SUBST([enable_grub_pe2elf])
+
 # Output files.
 AC_CONFIG_LINKS([include/grub/cpu:include/grub/$target_cpu
 	include/grub/machine:include/grub/$target_cpu/$platform])
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


[PATCH] Fix infinite loop in grub_pit_wait()

2008-08-07 Thread Christian Franke
grub2 from current SVN hangs if run in VirtualPC, the problem was 
introduced with 'svn diff -r 1779:1780'.


Here is a proposed fix.

Christian

2008-08-07  Christian Franke  [EMAIL PROTECTED]

* kern/i386/pit.c (TIMER2_SPEAKER): New define.
(TIMER2_GATE): Likewise.
(grub_pit_wait): Add enable/disable of the timer2 gate
bit of port 0x61. This fixes a possible infinite loop.


diff --git a/kern/i386/pit.c b/kern/i386/pit.c
index d0a6eda..a3fab26 100644
--- a/kern/i386/pit.c
+++ b/kern/i386/pit.c
@@ -28,13 +28,26 @@
 #define TIMER_ENABLE_LSB	0x20
 #define TIMER_ENABLE_MSB	0x10
 #define TIMER2_LATCH		0x20
+#define TIMER2_SPEAKER		0x02
+#define TIMER2_GATE		0x01
 
 void
 grub_pit_wait (grub_uint16_t tics)
 {
+  /* Disable timer2 gate and speaker.  */
+  grub_outb (grub_inb (TIMER2_REG_LATCH)  ~ (TIMER2_SPEAKER | TIMER2_GATE), TIMER2_REG_LATCH);
+
+  /* Set tics.  */
   grub_outb (TIMER2_SELECT | TIMER_ENABLE_LSB | TIMER_ENABLE_MSB, TIMER_REG_COMMAND);
   grub_outb (tics  0xff, TIMER2_REG_CONTROL);
   grub_outb (tics  8, TIMER2_REG_CONTROL);
 
+  /* Enable timer2 gate, keep speaker disabled.  */
+  grub_outb ((grub_inb (TIMER2_REG_LATCH)  ~ TIMER2_SPEAKER) | TIMER2_GATE, TIMER2_REG_LATCH);
+
+  /* Wait.  */
   while ((grub_inb (TIMER2_REG_LATCH)  TIMER2_LATCH) == 0x00);
+
+  /* Disable timer2 gate and speaker.  */
+  grub_outb (grub_inb (TIMER2_REG_LATCH)  ~ (TIMER2_SPEAKER | TIMER2_GATE), TIMER2_REG_LATCH);
 }
___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] Fix infinite loop in grub_pit_wait()

2008-08-07 Thread Christian Franke

Robert Millan wrote:

On Thu, Aug 07, 2008 at 05:32:42PM +0200, Christian Franke wrote:
  
grub2 from current SVN hangs if run in VirtualPC, the problem was 
introduced with 'svn diff -r 1779:1780'.



I'm not familiar with this part of the PIT interface;  why does it hang
only on VirtualPC?

  


Possibly different BIOS init of 0x61? Or different implementation of the 
legacy hardware, which IIRC looks like this:


. +---+ /-0x61[5]
....--Clock--| Timer | |
.0x61[0]---Gate--|   2   |-Out-+-+---+
. +---+|  |--Speaker
.0x61[1]--+---+


Resulting values of tsc_ticks_per_ms (AMD [EMAIL PROTECTED]):

.   origwith patch
VMware: ~1500~200
VirtualBox: ~200 ~200
VirtualPC:  hang ~200



...
+  /* Disable timer2 gate and speaker.  */
+  grub_outb (grub_inb (TIMER2_REG_LATCH)  ~ (TIMER2_SPEAKER | TIMER2_GATE), 
TIMER2_REG_LATCH);
 }



This doesn't AFAICT preserve the existing value of the timer2 gate and speaker.
I assume that is ok?

  


IMO yes. The timer state itself cannot be preserved, so it should be 
safe to leave it disabled. To test, try If BIOS speaker output (echo -e 
\a) still works.




Thanks!

  


You're welcome.

Patch committed.

Christian



___
Grub-devel mailing list
Grub-devel@gnu.org
http://lists.gnu.org/mailman/listinfo/grub-devel


  1   2   3   >