Re: menu loop (patch)
Yoshinori K. Okuji [EMAIL PROTECTED] kirjoitti: On Saturday 19 July 2008 21:58:09 Robert Millan wrote: On Sat, Jul 19, 2008 at 10:34:46AM -0700, Colin D Bennett wrote: [...] We could support a setting such as set menuwrap=1 which would enable this feature for users who care about it. Isn't this a bit overkill? The time spent adding this config option vastly exceeds any time that could be saved by having or not having menu wrap. Actually my graphical menu currently *does* wrap around, I guess it seemed logical to me at the time I wrote the code! 8-) As you can tell, I am not firmly set on either wrapping or not wrapping. Same here.. my concern with taking arbitrary decisions is, when someone comes later and asks to have it reverted, do we accept the request, and otherwise what rationale do we give her? :-) Personally, I prefer no wrapping, because it is easier for me to use only Up/Down arrow keys, yet locate entries easily. Since I am lazy, I don't want to look for Home or End (some keyboards really suck for them!), so when I just want to jump to somewhere near to the end, I only keep pushing Down, and wait for the cursor to stop. And, this is consistent with Dired mode in Emacs, Directory Listing in VIM, and, of course, with GRUB Legacy. Hi All, I also like no wrapping mode. But lets assume we have nice graphical menu with cylinder of menu entries that roll when you press key up/down. In here it would be really convenient to have wraparound eg. user cant event think any other way here. But this is something I would put into theme configuration file. Theme could be also loadable for text menu that describes colours and such. We could default to non-wrapping as that has been our way for long time. If user wants to override this in theme then they are free to do so. In every case I would prefer that home and end keys would go to always to start and to end of the list. Page up/down would rely a bit for wrapping setting so they can clamp to list edges if needed. Thanks, Vesa Jääskeläinen ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: menu loop (patch)
Hi, On Jul/20/2008, [EMAIL PROTECTED] wrote: In every case I would prefer that home and end keys would go to always to start and to end of the list. Page up/down would rely a bit for wrapping setting so they can clamp to list edges if needed. home-end keys (without wrapping) are handled in a patch that I sent yesterday in this thread (20 Jul 2008 00:12:43 +0200). I think that Home-End keys are fine and I hope that everybody agrees :-) I will also play with PgUp and PgDown soon or later :-) -- Carles Pina i EstanyGPG id: 0x8CBDAE64 http://pinux.info Manresa - Barcelona ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: loader modules jumping back to kernel
On Sunday 20 July 2008 01:06:22 Robert Millan wrote: Anyone recalls the reason our loaders had to jump back to kernel (startup.S) to do the final part of the load? Not all of them should do that, but it might be more convenient. I look at one by one: - The chainloader needs to get back the original state (e.g. A20 disabled), so the final code must be located at below 1MB. Since the address of the startup code is well known, it is easier to use. - The linux loader does not have to overwrite the startup code, but other regions can be. So it is easier to use. - The multiboot loader had, historically speaking, a limitation that it may not load an OS image below 1MB. So it was easier to use. But I don't remember if this limitation is still present in the current implementation. IIRC this causes trouble when the loadee chose an address that precisely overwrites the loader, which is garanteed to happen when GRUB is loading itself, AFAICT. Sure. My recommendation is, in case where you might overwrite that part, that you should write relocatable code (which is rather easy for simple code on i386) at anywhere (it could be in the startup), find out a safe region when loading an OS image, copy the code to the safe region, and finalize the bootstrap in that code (e.g. relocating the OS image, initializing registers, and jumping to it). On i386, we have a reserved region to temporarily load an OS image for the very reason, so this is not difficult. Regards, Okuji ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: ReleasePlanning page on wiki
From: Yoshinori K. Okuji [EMAIL PROTECTED] I will find a way to keep it up-to-date. Thanks, it's surely interesting what things are planned for the next release and not just having that genereal todo list. but it just doesn't look good if the last entry is more then a year ago :) And thanks for doing something against these randompage/findpage spam pages on wiki. Totally forgot to tell somebody as I removed them. Just saw that on the recentchanges history MoinMoin = 1.6 has text capchas but I don't know if that would help against this. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: I've a problem to compile i386 grub-emu
From: Marco Gerards [EMAIL PROTECTED] You need to install ncurses and the -dev package. Which distribution do you use? debian unstable. I have libncurses5-dev installed I used debootstrap --variant=buildd that did it I think ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Putting core.img anywhere
Hi, I have an unusual LVM setup where GRUB cannot be installed (see last Threads on LVM) because there is no room for core.img. The only place where boot data can be put is on a logical volume, but the bootsector (one of diskboot.S or boot.S, I assume) isn't smart enough to understand LVM or filesystems. However, I can ensure that my /boot logical volume is not too far from the beginning of the disk, and I thought about the following algorithm : Every 512-byte chunk of core.img contains : 496 bytes data 2 bytes magic value 2 bytes chunk number 4 bytes random integer 4 bytes random integer in next chunk 4 bytes on-disk sector where this chunk is located (serves as hint) The boot sector is patched with the random integer found in the first chunk, and the on-disk sector where it is located. On boot, this disk sector is loaded and the boot sector code checks if the random integer matches. If yes, the disk sector and the expected random integer are updated with the values found in the newly read block and the program proceeds with the next block. If the number doesn't match, the boot code scans the disk from sector 1 until it finds the sector having the correct random integer, expected magic value and chunk number. This algorithm should work, regardless from where core.img is stored. It can even be on a filesystem, as long as file chunks match disk blocks (which is the case for ext2, I think). Filesystems doing strange things with files may not work, but usually they aren't used for /boot anyway. I'm not used to assembly programming, but I'll give it a try. Which file contains the bootcode I should edit ? boot.S or diskboot.S ? Any comments ? JC signature.asc Description: Ceci est une partie de message numériquement signée ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] grub-probe -t prefix, fix update-grub_lib for Cygwin
The shell function make_system_path_relative_to_its_root() does not work on Cygwin due to path mapping (e.g. /boot/grub/ is actually /cygwin/boot/grub). This patch adds '-t prefix' to grub-probe. It prints result from grub_get_prefix() which is already extended for Cygwin (svn rev 1584). The result is used in make_system_path_relative_to_its_root(). This keeps the platform dependent code in getroot.c. Christian 2008-07-20 Christian Franke [EMAIL PROTECTED] * util/grub-probe.c (enum): Add PRINT PREFIX. (probe): Add PRINT_PREFIX, prints result of grub_get_prefix (). (usage): Add `prefix' to `-t' usage text. Add some '\n' to avoid excess long lines. (main): Add check for `-t prefix' option. * util/update-grub_lib.in (make_system_path_relative_to_its_root): Use result of `grub-probe -t prefix' instead of checking device numbers of parent directories. The latter does not work on Cygwin. diff --git a/util/grub-probe.c b/util/grub-probe.c index a4f51e2..d36c2cf 100644 --- a/util/grub-probe.c +++ b/util/grub-probe.c @@ -46,6 +46,7 @@ enum { PRINT_FS, PRINT_FS_UUID, PRINT_DRIVE, + PRINT_PREFIX, PRINT_DEVICE, PRINT_PARTMAP, PRINT_ABSTRACTION, @@ -113,6 +114,19 @@ probe (const char *path, char *device_name) grub_device_t dev = NULL; grub_fs_t fs; + if (print == PRINT_PREFIX) +{ + if (! path) +grub_util_error (cannot find prefix for a device.\n); + char * prefix = grub_get_prefix (path); + if (! prefix) +grub_util_error (cannot find prefix for %s.\n, path); + + printf (%s\n, prefix); + free (prefix); + goto end; +} + if (path == NULL) { if (! grub_util_check_block_device (device_name)) @@ -264,8 +278,10 @@ Probe device information for a given path (or device, if the -d option is given) \n\ -d, --device given argument is a system device, not a path\n\ -m, --device-map=FILE use FILE as the device map [default=%s]\n\ - -t, --target=(fs|fs_uuid|drive|device|partmap|abstraction)\n\ -print filesystem module, GRUB drive, system device, partition map module or abstraction module [default=fs]\n\ + -t, --target=(fs|fs_uuid|drive|prefix|device|partmap|abstraction)\n\ +print filesystem module, GRUB drive, path prefix,\n\ +system device, partition map module or\n\ +abstraction module [default=fs]\n\ -h, --helpdisplay this message and exit\n\ -V, --version print version information and exit\n\ -v, --verbose print verbose messages\n\ @@ -313,6 +329,8 @@ main (int argc, char *argv[]) print = PRINT_FS_UUID; else if (!strcmp (optarg, drive)) print = PRINT_DRIVE; + else if (!strcmp (optarg, prefix)) + print = PRINT_PREFIX; else if (!strcmp (optarg, device)) print = PRINT_DEVICE; else if (!strcmp (optarg, partmap)) diff --git a/util/update-grub_lib.in b/util/update-grub_lib.in index c488a85..163b143 100644 --- a/util/update-grub_lib.in +++ b/util/update-grub_lib.in @@ -41,25 +41,14 @@ make_system_path_relative_to_its_root () # if not a directory, climb up to the directory containing it if test -d $path ; then dir=$path +file= else dir=`echo $path | sed -e s,/[^/]*$,,g` +file=`echo $path | sed -e s,^.*/,/,g` fi - num=`stat -c %d $dir` - - # this loop sets $dir to the root directory of the filesystem we're inspecting - while : ; do -parent=`readlink -f $dir/..` -if [ x`stat -c %d $parent` = x$num ] ; then : ; else - # $parent is another filesystem; we found it. - break -fi -if [ x$dir = x/ ] ; then - # / is our root. - break -fi -dir=$parent - done + # get directory prefix relative to its root + dir=`${grub_probe} -t prefix $dir` || return 1 # This function never prints trailing slashes (so that its output can be # appended a slash unconditionally). Each slash in $dir is considered a @@ -68,7 +57,8 @@ make_system_path_relative_to_its_root () dir= fi - echo $path | sed -e s,^$dir,,g + # re-append file if necessary + echo $dir$file } is_path_readable_by_grub () ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Use cases for GRUB 2 added to wiki
Hi All, It seems that I was logged from my lappy to GRUB wiki so I created a page having a list of use cases for GRUB 2. I did not add everything there... this is where you all come to picture :) So please add all use cases you think should be supported. Please try to use same formatting and if you think there should be more details on one individual page please add them there too (and to other pages if you see that fit). Pages do not need to include strategy how it will be supported (in the end they will have that information). When we have contents there it is much easier to discuss about those special use cases while not forgetting other use cases. And the direct link to the use case list: http://grub.enbug.org/UseCases Thanks, Vesa Jääskeläinen ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Endianness macros capitalization
Christian Franke wrote: Pavel Roskin wrote: On Tue, 2008-07-08 at 20:04 +0200, Christian Franke wrote: With old gcc versions without the rol optimization, even the 16 bit swap should be a function: Or better yet, an asm statement. We should consider optimized assembly vs function call. Even the 32-bit swap could be shorter: a: 86 c4 xchg %al,%ah c: c1 c0 10rol$0x10,%eax f: 86 c4 xchg %al,%ah 11: That's 7 bytes! ... But the function call in the 32-bit case requires only 5 bytes :-) Sorry, I was wrong here. The assumption about function call size was only true for module-local calls. If a module calls a function in kernel, each 5 byte call requires another 8 bytes for the ELF relocation table entry. Christian ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] make assumed terminal width on cmd help dynamic
Thanks to nyu and daChaac on IRC By default with gfxterm loaded and gfxmode set to 640*480 help command produces empty lines see [0] help command assumes a fixed width of 80 but gfxterm in 640*480 has only 77 This is a patch to fix it. [0] http://img146.imageshack.us/my.php?image=otherlinux26xkernel64biao8.png * commands/help.c: changed assumed terminal width from 80 to dynamic Index: commands/help.c === --- commands/help.c (Revision 1723) +++ commands/help.c (Arbeitskopie) @@ -21,10 +21,11 @@ #include grub/dl.h #include grub/arg.h #include grub/misc.h +#include grub/term.h /* XXX: This has to be changed into a function so the screen can be optimally used. */ -#define TERM_WIDTH 80 +#define TERM_WIDTH GRUB_TERM_WIDTH ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] make assumed terminal width on cmd help dynamic
From: Felix Zielcke [EMAIL PROTECTED] Sent: Sunday, July 20, 2008 3:47 PM To: grub-devel@gnu.org Subject: [PATCH] make assumed terminal width on cmd help dynamic * commands/help.c: changed assumed terminal width from 80 to dynamic * commands/help.c: it was using a fixed terminal width of 80. Made it dynamic Maybe still not perfect but at least better |15:51:43| fzielcke oh well i really should learn more english |15:51:49| fzielcke i think the changelog entry isn't that good Yes i'm a bit lazy :) Btw. |15:40:06| fzielcke that's cool I never bothered about grub then I started to use ext4 asked for it on grub-devel bean made a patch |15:40:13| fzielcke and now i can give you even something back :) ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] make assumed terminal width on cmd help dynamic
Felix Zielcke [EMAIL PROTECTED] * commands/help.c: Include grub/term.h. (TERM_WIDTH): Redefined to GRUB_TERM_WIDTH. Thanks to Marco for the help. I should have asked before, but i'm more a try and fail guy :) ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: I've a problem to compile i386 grub-emu
From: Marco Gerards [EMAIL PROTECTED] I wonder why this got through the autoconf test. Rerunning ./configure with --enable-grub-emu and then make grub-emu did work Maybe it's possible to disallow compiling grub-emu if --enable-grub-emu wasn't given to configure? ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Putting core.img anywhere
El sáb, 19-07-2008 a las 19:24 +0200, Jean-Christophe Haessig escribió: Hi, Hi there! I have an unusual LVM setup where GRUB cannot be installed (see last Threads on LVM) because there is no room for core.img. This should have been fixed by the transition to LZMA as the compression algorithm for PC - core.img should now be under 32K and embeddable in the 32256 bytes available before the first MBR partition. PC+GPT schemes might be clumsier, since you'd stamp the main GPT (though the system keeps working with the backup GPT at the end of the disk, as one of my laptops proves). (...) However, I can ensure that my /boot logical volume is not too far from the beginning of the disk, and I thought about the following algorithm : How can you do so? Is there a way to force LVM to put a certain LV within a particular region of a PV within the VG? At most, you could split your disk in two partitions, one small and one big; and format both as LVM PVs, then add them to the same VG. You can then force the /boot LV within the VG to lie in the small PV, but if you take the trouble to do this it's quite simpler to just create a non-LVM boot partition. Every 512-byte chunk of core.img contains : 496 bytes data 2 bytes magic value 2 bytes chunk number 4 bytes random integer 4 bytes random integer in next chunk 4 bytes on-disk sector where this chunk is located (serves as hint) The boot sector is patched with the random integer found in the first chunk, and the on-disk sector where it is located. On boot, this disk sector is loaded and the boot sector code checks if the random integer matches. If yes, the disk sector and the expected random integer are updated with the values found in the newly read block and the program proceeds with the next block. If the number doesn't match, the boot code scans the disk from sector 1 until it finds the sector having the correct random integer, expected magic value and chunk number. Theoretically it would work, but this is heavy duty work we're talking about - potentially reading the whole disk if a single file has moved due to, say, a defrag. Besides, we could hit one of the several BIOS disk size limits, so you would have to ensure (as you said above) that core.img lies within the reasonable limits for your BIOS (8G? 32G? 128G?) This algorithm should work, regardless from where core.img is stored. It can even be on a filesystem, as long as file chunks match disk blocks (which is the case for ext2, I think). Filesystems doing strange things with files may not work, but usually they aren't used for /boot anyway. It would work as long as the filesystem stores 512-byte blocks together. However, with tail-packing features _might_ pose a problem; and filesystems that altered the data in any way, like NTFS compression or some kind of inline checksumming/signing would be a no-go. signature.asc Description: Esta parte del mensaje está firmada digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] make assumed terminal width on cmd help dynamic
Felix Zielcke [EMAIL PROTECTED] writes: Thanks to nyu and daChaac on IRC By default with gfxterm loaded and gfxmode set to 640*480 help command produces empty lines see [0] help command assumes a fixed width of 80 but gfxterm in 640*480 has only 77 This is a patch to fix it. Here is the changelog entry from the other mail (just resend the patch if you have a new changelog entry): Felix Zielcke [EMAIL PROTECTED] * commands/help.c: Include grub/term.h. (TERM_WIDTH): Redefined to GRUB_TERM_WIDTH. (two newlines instead of one) It would be better to replace all users of TERM_WIDTH to use GRUB_TERM_WIDTH, like you mentioned. Only ugly thing is that this is actually a function... -- Marco ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] High resolution time support using x86 TSC
Colin D Bennett [EMAIL PROTECTED] writes: Hi Marco, On Thu, 03 Jul 2008 20:52:53 +0200 Marco Gerards [EMAIL PROTECTED] wrote: Hi Colin, Colin D Bennett [EMAIL PROTECTED] writes: I have implemented high resolution time support (through the new grub_get_time_ms() function) using the RDTSC instruction available on Pentium and higher x86 CPUs. The TSC value is simply a 64-bit block cycle counter that is zeroed at bootup, so grub_main() calls grub_time_init(), which is defined by each platform. If a platform links to kern/i386/tsc.c, then the grub_time_init() function from tsc.c is used, which calibrates the TSC rate and absolute zero reference using the RTC. What if TSC is not available? I updated the changelog entry to indicate that running on a 386 or 486 will fail, since TSC is provided in Pentium+. Do we support running on 386 or 386? Should I check for this? If so, the code will have to change a bit, and be able to select between the generic implementation and the TSC implementation at runtime. I think this would be best done letting the grub_get_time_ms implementation be selected by setting a function pointer in grub_machine_init() depending on the machine's capabilities. I would have to significantly re-structure my patch, but it might be necessary (and could lead to more understandable code?). What do you think? That would be great. I do not want to drop 486 support just because of this. You could even drop back to a lower granularity of the timer or better: submit code that does the trick for the 486 as well. -- Marco ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] TSC patch v2
Colin D Bennett [EMAIL PROTECTED] writes: Here is an updated version of the TSC patch. I addressed the issues in your comments with the exception of supporting x86 CPUs that don't have a TSC (386 and 486). I eliminated the grub_time_init() function and the call to it in grub_main() in favor of having grub_machine_init() simply call grub_tsc_calibrate(). The i386-pc and i386-efi platforms use the TSC. I also changed the grub_millisleep() generic function to use grub_get_time_ms() instead of grub_get_rtc() to achieve better precision when a higher resolution time function is available. Please take a look at it and give me your comments when you have a chance. Thanks! Great! Sorry I kept you waiting... === modified file 'ChangeLog' --- ChangeLog 2008-07-04 02:26:10 + +++ ChangeLog 2008-07-04 18:08:36 + @@ -1,3 +1,70 @@ +2008-07-04 Colin D Bennett [EMAIL PROTECTED] + + High resolution timer support. Implemented for i386 CPU using TSC. + Extracted generic grub_millisleep() so it's linked in only as needed. + This requires a Pentium compatible CPU; currently the code does not + check for this (so it will fail on 386 and 486 machines). We really need to keep the current machines working. If you will drop support for something, please don't do it in a patch but in a separate discussion so no one will miss it. But please, I do not think it will be that hard to keep this. + * conf/i386-efi.rmk: Added TSC high resolution time module, link with + generic grub_millisleep() function. + + * conf/i386-pc.rmk: Likewise. + + * conf/sparc64-ieee1275.rmk: Add kern/generic/millisleep.c and + kern/generic/get_time_ms.c to kernel, to use generic time functions. + + * conf/powerpc-ieee1275.rmk: Add kern/generic/millisleep.c to kernel, + to use generic grub_millisleep() function. + + * conf/i386-linuxbios.rmk: Added kern/generic/get_time_ms.c to the + kernel. Please describe where you change things. Just have a look at existing changelog entries how this was done. + * kern/generic/get_time_ms.c (grub_get_time_ms): New file. Platform + independent implementation of grub_get_time_ms() using the RTC that + can be linked into a platform's kernel when it does not implement its + own specialized grub_get_time_ms() function. New file. is sufficient. + * kern/generic/millisleep.c (grub_millisleep): New file. Extracted + from grub_millisleep_generic() in kern/misc.c and renamed. Changed it + to use grub_get_time_ms() instead of grub_get_rtc() for better + precision on when high resolution time is available. Same here. + * kern/misc.c (grub_millisleep_generic): Deleted. Moved to + kern/generic/millisleep.c so that it is only included in the kernel + image when a platform does not define a specialized version. Deleted. is sufficient. Or better Removed., just because most people use it :-) + * commands/sleep.c (grub_interruptible_millisleep): Uses + grub_get_time_ms() instead of grub_get_rtc() to stay in sync with + grub_millisleep() from kern/generic/millisleep.c. + + * include/grub/i386/tsc.h (grub_get_tsc): New file. Inline function + grub_get_tsc() uses x86 RDTSC instruction (available on Pentium+ CPUs) + to read the counter value for the TSC. + (grub_tsc_calibrate): Declare this function for grub_machine_init(). Same as above. + * kern/i386/tsc.c (grub_get_time_ms): x86 TSC support providing a high + resolution clock. I would simply say: New function. + (grub_tsc_calibrate): New function to calibrate the TSC using RTC. No need to explain what it does. + * include/grub/time.h (grub_get_time_ms): Added grub_get_time_ms() + function to return the current time in millseconds since the epoch. + This supports higher resolution time than grub_get_rtc() on some + platforms such as i386-pc, where the RTC has only about 1/18 s + precision but a higher precision timer such as the TSC is available. Perhaps: New prototype. You forgot to mention you included grub/types.h. Please make sure you mention all other inclusions of header files you added as well. + * kern/i386/efi/init.c (grub_millisleep): Deleted. Don't define + grub_millisleep() -- it just called grub_millisleep_generic() but now + it is linked to kern/generic/millisleep.c for the implementation. Only Removed. will be sufficient. + * kern/sparc64/ieee1275/init.c (grub_millisleep): Deleted. Hurray! ;-) + * kern/i386/pc/init.c (grub_machine_init): Call grub_tsc_calibrate(). + (grub_millisleep): Deleted. + + * kern/ieee1275/init.c (grub_millisleep): Deleted. + (grub_get_rtc): Now calls grub_get_time_ms(), which does the real + work. As you can see from my comments, you do not have to say what something does in the changelog entry. Our changelog entries log changes only, not the
Re: [PATCH] Fix warning in fs/xfs.c
Pavel Roskin [EMAIL PROTECTED] writes: On Thu, 2008-07-03 at 20:21 +0200, Marco Gerards wrote: Pavel Roskin [EMAIL PROTECTED] writes: ChangeLog: * fs/xfs.c (struct grub_xfs_dir_header): Use names similar to those in Linux XFS code. Provide a way to access 64-bit parent inode. (grub_xfs_iterate_dir): Use the new names. Avoid reading past the end of struct grub_xfs_dir_header. *please* do not look at Linux code or whatever *and* contribute to GRUB. It might cause copyright troubles I will have to deal with :-/ I just tried to make names similar without copying any code. But it's a useful reminder. What I meant is that even *looking* at code might cause problems. People can claim you have stolen their ideas. That would essentially mean the same as copying code. I just want to avoid such problems at beforehand. I do not see the advantage of this patch. Can you please explain why we need these name changes? We were casting a pointer to a 32-bit integer to a pointer to a 64-bit integer, which is bad, and gcc was emitting a warning about it. Right Worse yet, the 64-bit value was sticking beyond the end the structure we were using to describe the header. i4 and i8 are generally used by Linux XFS code to describe 32-bit and 64-bit values if either can be used. The smallino field was highly misleading because it had to be negated. It's the number of big (i8 or 64-bit) entries. If it's 0, then the entries are small. So it was natural to call it i8count. And once it was i8count, it was natural to call the first value count. If you prefer another naming convention, let's rename the entries according to it. I was thinking having 2 32-bit integers parent_hi and parent_lo or something like that. Anyway, let's not use smallino - bigentries would be better. What I suggest is that you pick the names yourself or from a standard, instead of from Linux code. -- Marco ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Enable writing to ATA devices, fix several bugs
Pavel Roskin [EMAIL PROTECTED] writes: Quoting Marco Gerards [EMAIL PROTECTED]: I cannot get the ata module to recognize the hard drive on my test machine, so more work is needed to test it on the real hardware. Right, this needs more work. I will have another look at ATA soon :-) That would be great! If you check Linux include/linux/ata.h, 1 is ATA_ERR, and we really need ata_ok(), which checks multiple flags. So it's clearly material for a separate patch. No, I cannot check the Linux code. AFAIK this can cause copyright problems. But I agree that more and better error checking is required. I know. That's why I'll write it from specifications or maybe I'll take it from the GNU/Hurd code. Taking it from Specifications will be better. I think the ATA driver of GNU Mach comes from Linux 2.0 or so. So that won't change anything for us ;(. -- Marco ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Drivemap module
Javier Martín [EMAIL PROTECTED] writes: Just an updated version of the patch that adds support for device-like names instead of raw BIOS disk numbers, i.e. this is now supported: grub drivemap (hd0) (hd1) In addition to the already supported: grub drivemap (hd0) 0x81 The effect is the same: the second BIOS hard drive will map to (hd0) through the installed int13h routine. The new syntax does not require the target device name to exist (hd1 need not exist in my example), and the parsing is very simple: it accepts names like (fdN) and (hdN) with and without parenthesis, and with N ranging from 0 to 127, thus allowing the full 0x00-0xFF range even though most BIOS-probing OSes don't bother going any further than fd7 and hd7 respectively. Did you use code from other people or projects? For newcomers, full info on the patch is available on the list archives - it was proposed on June and its discussion deferred for two or three weeks because the developers were busy. I have copied the changelog entry from your other e-mail: * commands/i386/pc/drivemap.c : New file, main part of the new drivemap module allowing BIOS drive remapping not unlike the legacy map command. This allows to boot OSes with boot-time dependencies on the particular ordering of BIOS drives or trusting their own to be 0x80, like Windows XP, with non-standard boot configurations. New file. would be sufficient * commands/i386/pc/drivemap_int13h.S : New file, INT 13h handler for the drivemap module. Installed as a TSR routine by drivemap.c, performs the actual redirection of BIOS drives. Same here. * conf/i386-pc.rmk : Added the new module Please say which variables you added in this file. You can find some examples on how to do this in the ChangeLog file. * include/grub/loader.h : Added a just-before-boot callback infrastructure used by drivemap.mod to install the INT13 handler only when the boot command has been issued. Please describe changes, not effects. So which prototypes and macros did you add? * kern/loader.c : Implement the preboot-hook described Which functions did you change and how? Please describe actual changes. The header is missing, please include it. Also newlines between the files make it easier to read. Here follows a review. Sorry I kept you waiting for this long, this feature and your work is really appreciated! Perhaps I can spot some more problems after you fixed it and supplied an updated changelog entry. There are quite some comments, but please do not let this demotivate you, it is mainly coding style related :-) Index: commands/i386/pc/drivemap.c === RCS file: commands/i386/pc/drivemap.c diff -N commands/i386/pc/drivemap.c --- /dev/null 1 Jan 1970 00:00:00 - +++ commands/i386/pc/drivemap.c 2 Jul 2008 01:12:36 - @@ -0,0 +1,391 @@ +/* drivemap.c - command to manage the BIOS drive mappings. */ +/* + * GRUB -- GRand Unified Bootloader + * Copyright (C) 2008 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/. + */ + +#include grub/normal.h +#include grub/dl.h +#include grub/mm.h +#include grub/misc.h +#include grub/disk.h +#include grub/loader.h +#include grub/machine/loader.h +#include grub/machine/biosdisk.h + +/* Uncomment the following line to enable debugging output */ +/* #define DRIVEMAP_DEBUG */ + +#ifdef DRIVEMAP_DEBUG +# define DBG_PRINTF(...) grub_printf(__VA_ARGS__) +#else +# define DBG_PRINTF(...) +#endif Please use the grub_dprintf infrastructure. +static const struct grub_arg_option options[] = { + {list, 'l', 0, show the current mappings, 0, 0}, + {reset, 'r', 0, reset all mappings to the default values, 0, 0}, + {0, 0, 0, 0, 0, 0} +}; + +/* ASSEMBLY SYMBOLS/VARS/FUNCS - start */ Useless comment +/* realmode far ptr = 2 * 16b */ +extern grub_uint32_t EXPORT_VAR (grub_drivemap_int13_oldhandler); +/* Size of the section to be copied */ +extern grub_uint16_t EXPORT_VAR (grub_drivemap_int13_size); + +/* NOT a typo - just need the symbol's address with symbol */ +typedef void grub_symbol_t; +extern grub_symbol_t EXPORT_VAR (grub_drivemap_int13_handler_base); +extern grub_symbol_t EXPORT_VAR
Re: [PATCH] make assumed terminal width on cmd help dynamic
Felix Zielcke [EMAIL PROTECTED] writes: From: Marco Gerards [EMAIL PROTECTED] It would be better to replace all users of TERM_WIDTH to use GRUB_TERM_WIDTH, like you mentioned. Only ugly thing is that this is actually a function... Here it is. Felix Zielcke [EMAIL PROTECTED] * commands/help.c: Include grub/term.h. (TERM_WIDTH): Removed. Updated all uses. I would say Updated all users. or perhaps even Updated all callers.. English is not my native language, hopefully someone can help us out ;-). If this is updated, this patch can be applied as far as I am concerned :-) Index: commands/help.c === --- commands/help.c (Revision 1723) +++ commands/help.c (Arbeitskopie) @@ -21,11 +21,8 @@ #include grub/dl.h #include grub/arg.h #include grub/misc.h +#include grub/term.h -/* XXX: This has to be changed into a function so the screen can be - optimally used. */ -#define TERM_WIDTH 80 - static grub_err_t grub_cmd_help (struct grub_arg_list *state __attribute__ ((unused)), int argc, char **args) @@ -43,16 +40,16 @@ { if (cmd-flags GRUB_COMMAND_FLAG_CMDLINE) { - char description[TERM_WIDTH / 2]; + char description[GRUB_TERM_WIDTH / 2]; int desclen = grub_strlen (cmd-summary); - /* Make a string with a length of TERM_WIDTH / 2 - 1 filled + /* Make a string with a length of GRUB_TERM_WIDTH / 2 - 1 filled with the description followed by spaces. */ - grub_memset (description, ' ', TERM_WIDTH / 2 - 1); - description[TERM_WIDTH / 2 - 1] = '\0'; + grub_memset (description, ' ', GRUB_TERM_WIDTH / 2 - 1); + description[GRUB_TERM_WIDTH / 2 - 1] = '\0'; grub_memcpy (description, cmd-summary, - (desclen TERM_WIDTH / 2 - 1 - ? desclen : TERM_WIDTH / 2 - 1)); + (desclen GRUB_TERM_WIDTH / 2 - 1 + ? desclen : GRUB_TERM_WIDTH / 2 - 1)); grub_printf (%s%s, description, (cnt++) % 2 ? \n : ); } ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode (revised version)
Hi, Bean [EMAIL PROTECTED] writes: First of all, we can still keep rescue and normal command. But instead of depending on normal.mod, normal command depends on module arg, which is an option parser. Also, these two type of commands are of the same command set. In fact, module arg is implemented as a pre parser, which goes through the list of arguments and extract the options. In the case of rescue command, the pre parser field is null, which means it wants to parse options itself. pre parser? Then, I think of a new structure to represent all configurable handlers of grub. Different types of handler have different fields, but they all share a command header: What is a handler and what are its responsibilities? struct grub_handler { .next, .name, .init, .fini }; Same type of handlers are linked together. We first define an enum to list all types. For example: enum { GRUB_HANDLER_INPUT, GRUB_HANDLER_OUTPUT, GRUB_HANDLER_CONSOLE, GRUB_HANDLER_MENU, GRUB_HANDLER_SCRIPT, GRUB_HANDLER_NUM }; Then, we define an array to point to the head of handler linked list: grub_handler[GRUB_HANDLER_NUM]; Head is the default selection. When we insert a new handler module, it would automatically become the new default, although we can switch back to old handler using a command. Here are more details about different handlers: input: This is the input component of terminal: struct grub_handler_input { .next, .name, .init, .fini, .checkkey, .getkey .flags, }; output: This is the output component of terminal: struct grub_handler_output { .next, .name, .init, .fini, .putchar, .getcharwidth, .getxy, .gotoxy, .cls, .setcolorstate, .setcursor, .flags, }; Sometimes the input and output are tightly coupled. How do you want to handle this? console interface: It represent the grub console, users type commands and execute them line by line. struct grub_handler_console { .next, .name, .init, .fini, .run }; menu interface: It represent the menu, users select a menu item and execute it. struct grub_handler_menu { .next, .name, .init, .fini, .run }; script engine: It's responsible for parsing config file to get the menu list, and execution of commands. struct grub_handler_script { .next, .name, .init, .fini, .readconfig .getmenu .execute }; What I had in mind was a twofold. I wrote a parser part that actually parses the script and code to manage the AST and to execute it. It would be easy to extend the current design with other languages. Although I think we should not add more than one to SVN, otherwise it will become bloated. The handlers are independent of each other. When they need something, they called specific function of the default handler. For example, to read a key from the console, we can use grub_handler[GRUB_HANDLER_INPUT]-getkey. Also, to get the list of items to be displayed on screen, the menu handler can call grub_handler[GRUB_HANDLER_SCRIPT]-getmenu. How does this differ from what we have now? You can register all kinds of objects and deregister them. -- Marco ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Idea: elimination of the normal mode (revised version)
On Mon, Jul 21, 2008 at 4:02 AM, Marco Gerards [EMAIL PROTECTED] wrote: Hi, Bean [EMAIL PROTECTED] writes: First of all, we can still keep rescue and normal command. But instead of depending on normal.mod, normal command depends on module arg, which is an option parser. Also, these two type of commands are of the same command set. In fact, module arg is implemented as a pre parser, which goes through the list of arguments and extract the options. In the case of rescue command, the pre parser field is null, which means it wants to parse options itself. pre parser? Maybe not the best word, the idea is that normal command is just like rescue command, except that it goes through an extra parser that convert options to grub_arg_list. Then, I think of a new structure to represent all configurable handlers of grub. Different types of handler have different fields, but they all share a command header: What is a handler and what are its responsibilities? Actually, handler is a genetic term. For terminal, there is grub_term. For video, there is grub_video_adapter. They're similar in operation. For example, there can have multiple objects, but only one is active, they all have basic field like init, fini, next, name. They can be handled by the same function. I also extend this concept to other objects, like script engine, console and menu interface. struct grub_handler { .next, .name, .init, .fini }; Same type of handlers are linked together. We first define an enum to list all types. For example: enum { GRUB_HANDLER_INPUT, GRUB_HANDLER_OUTPUT, GRUB_HANDLER_CONSOLE, GRUB_HANDLER_MENU, GRUB_HANDLER_SCRIPT, GRUB_HANDLER_NUM }; Then, we define an array to point to the head of handler linked list: grub_handler[GRUB_HANDLER_NUM]; Head is the default selection. When we insert a new handler module, it would automatically become the new default, although we can switch back to old handler using a command. Here are more details about different handlers: input: This is the input component of terminal: struct grub_handler_input { .next, .name, .init, .fini, .checkkey, .getkey .flags, }; output: This is the output component of terminal: struct grub_handler_output { .next, .name, .init, .fini, .putchar, .getcharwidth, .getxy, .gotoxy, .cls, .setcolorstate, .setcursor, .flags, }; Sometimes the input and output are tightly coupled. How do you want to handle this? Some module can export both input and output handler, like serial. But some only export input (like atkeyboard), or output (gfxterm). console interface: It represent the grub console, users type commands and execute them line by line. struct grub_handler_console { .next, .name, .init, .fini, .run }; menu interface: It represent the menu, users select a menu item and execute it. struct grub_handler_menu { .next, .name, .init, .fini, .run }; script engine: It's responsible for parsing config file to get the menu list, and execution of commands. struct grub_handler_script { .next, .name, .init, .fini, .readconfig .getmenu .execute }; What I had in mind was a twofold. I wrote a parser part that actually parses the script and code to manage the AST and to execute it. It would be easy to extend the current design with other languages. Although I think we should not add more than one to SVN, otherwise it will become bloated. Right, but actually, there are already two script engine existing. One is the normal bash-like parser, another is the command line scanner in the rescue mode. The handlers are independent of each other. When they need something, they called specific function of the default handler. For example, to read a key from the console, we can use grub_handler[GRUB_HANDLER_INPUT]-getkey. Also, to get the list of items to be displayed on screen, the menu handler can call grub_handler[GRUB_HANDLER_SCRIPT]-getmenu. How does this differ from what we have now? You can register all kinds of objects and deregister them. Yes, handler is just objects, but they share common fields, so that we can handle them together, other than writing different code for different object. -- Bean ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Enable writing to ATA devices, fix several bugs
On Sun, 2008-07-20 at 20:55 +0200, Marco Gerards wrote: Pavel Roskin [EMAIL PROTECTED] writes: I know. That's why I'll write it from specifications or maybe I'll take it from the GNU/Hurd code. Taking it from Specifications will be better. I think the ATA driver of GNU Mach comes from Linux 2.0 or so. So that won't change anything for us ;(. I don't think choosing consistent names could be interpreted as a copyright violation (except by companies like SCO, but then all bets are off). Anyway, if I ever have a chance to touch the GRUB ATA code again, I'll use FreeBSD as a reference. Using specification is probably not the best idea because we need GRUB to work on the real life hardware, and we need to be prepared to handle known quirks in popular hardware. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Fix warning in fs/xfs.c
On Sun, 2008-07-20 at 20:50 +0200, Marco Gerards wrote: Pavel Roskin [EMAIL PROTECTED] writes: i4 and i8 are generally used by Linux XFS code to describe 32-bit and 64-bit values if either can be used. The smallino field was highly misleading because it had to be negated. It's the number of big (i8 or 64-bit) entries. If it's 0, then the entries are small. So it was natural to call it i8count. And once it was i8count, it was natural to call the first value count. If you prefer another naming convention, let's rename the entries according to it. I was thinking having 2 32-bit integers parent_hi and parent_lo or something like that. Anyway, let's not use smallino - bigentries would be better. What I suggest is that you pick the names yourself or from a standard, instead of from Linux code. OK, I'll try to be more creative. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Kernel fixes for Cygwin
This adds Cygwin support to kernel sources. It handles the issues introduced by PE-ELF conversion and adds support for HAVE_ASM_USCORE. Christian 2007-07-20 Christian Franke [EMAIL PROTECTED] * include/grub/dl.h: Remove .previous, gas supports this only for ELF format. * include/grub/symbol.h [__CYGWIN__] (#define FUNCTION/VARIABLE): Remove .type, gas supports this only for ELF format. * kern/dl.c (grub_dl_resolve_symbols): Add check for grub_mod_init and grub_mod_fini for symbols without a type. Handle HAVE_ASM_USCORE case for these symbols. (grub_dl_resolve_dependencies): Add check for trailing nullbytes in symbol table. This fixes an infinite loop if table is zero filled. * kern/i386/dl.c [__CYGWIN__] (fix_pc_rel_relocation): New function to fix bad PC relative relocation produced by objcopy. [__CYGWIN__] (grub_arch_dl_relocate_symbols): Add fix of PC relative relocation. (grub_arch_dl_relocate_symbols): Abort on unknown relocation type. diff --git a/include/grub/dl.h b/include/grub/dl.h index b630c6f..3029f95 100644 --- a/include/grub/dl.h +++ b/include/grub/dl.h @@ -40,11 +40,12 @@ grub_##name##_fini (void) { grub_mod_fini (); } \ static void \ grub_mod_fini (void) +/* Note: .previous not supported for non-ELF targets. */ #define GRUB_MOD_NAME(name) \ -__asm__ (.section .modname,\S\\n.string \ #name \\n.previous) +__asm__ (.section .modname\n.string \ #name \\n) #define GRUB_MOD_DEP(name) \ -__asm__ (.section .moddeps,\S\\n.string \ #name \\n.previous) +__asm__ (.section .moddeps\n.string \ #name \\n) struct grub_dl_segment { diff --git a/include/grub/symbol.h b/include/grub/symbol.h index aa0ea5a..72209d1 100644 --- a/include/grub/symbol.h +++ b/include/grub/symbol.h @@ -28,8 +28,14 @@ # define EXT_C(sym) sym #endif +#ifndef __CYGWIN__ #define FUNCTION(x) .globl EXT_C(x) ; .type EXT_C(x), function ; EXT_C(x): #define VARIABLE(x) .globl EXT_C(x) ; .type EXT_C(x), object ; EXT_C(x): +#else +/* .type not supported for non-ELF targets. XXX: Check this in configure? */ +#define FUNCTION(x) .globl EXT_C(x) ; EXT_C(x): +#define VARIABLE(x) .globl EXT_C(x) ; EXT_C(x): +#endif /* Mark an exported symbol. */ #ifndef GRUB_SYMBOL_GENERATOR diff --git a/kern/dl.c b/kern/dl.c index c0d9f1d..7950c0d 100644 --- a/kern/dl.c +++ b/kern/dl.c @@ -53,6 +53,12 @@ typedef Elf64_Sym Elf_Sym; #endif +#ifdef HAVE_ASM_USCORE +# define SYM_USCORE _ +#else +# define SYM_USCORE +#endif + struct grub_dl_list @@ -347,17 +353,31 @@ grub_dl_resolve_symbols (grub_dl_t mod, Elf_Ehdr *e) unsigned char type = ELF_ST_TYPE (sym-st_info); unsigned char bind = ELF_ST_BIND (sym-st_info); const char *name = str + sym-st_name; - + int check_mod_func = 0; + switch (type) { case STT_NOTYPE: - /* Resolve a global symbol. */ - if (sym-st_name != 0 sym-st_shndx == 0) + if (sym-st_name != 0) { - sym-st_value = (Elf_Addr) grub_dl_resolve_symbol (name); - if (! sym-st_value) - return grub_error (GRUB_ERR_BAD_MODULE, - the symbol `%s' not found, name); + if (sym-st_shndx == 0) + { + /* Resolve a global symbol. */ + sym-st_value = (Elf_Addr) grub_dl_resolve_symbol (name); + if (! sym-st_value) + return grub_error (GRUB_ERR_BAD_MODULE, + the symbol `%s' not found, name); + } + else + { /* Static functions and global variables have no type + if initial format was not ELF. */ + sym-st_value += (Elf_Addr) grub_dl_get_section_addr (mod, + sym-st_shndx); + if (bind == STB_LOCAL) + check_mod_func = 1; + else if (grub_dl_register_symbol (name, (void *) sym-st_value, mod)) + return grub_errno; + } } else sym-st_value = 0; @@ -374,14 +394,10 @@ grub_dl_resolve_symbols (grub_dl_t mod, Elf_Ehdr *e) case STT_FUNC: sym-st_value += (Elf_Addr) grub_dl_get_section_addr (mod, sym-st_shndx); - if (bind != STB_LOCAL) - if (grub_dl_register_symbol (name, (void *) sym-st_value, mod)) - return grub_errno; - - if (grub_strcmp (name, grub_mod_init) == 0) - mod-init = (void (*) (grub_dl_t)) sym-st_value; - else if (grub_strcmp (name, grub_mod_fini) == 0) - mod-fini = (void (*) (void)) sym-st_value; + if (bind == STB_LOCAL) + check_mod_func = 1; + else if (grub_dl_register_symbol (name, (void *) sym-st_value, mod)) + return grub_errno; break; case STT_SECTION: @@ -397,6 +413,13 @@ grub_dl_resolve_symbols (grub_dl_t mod, Elf_Ehdr *e) return grub_error (GRUB_ERR_BAD_MODULE, unknown symbol type `%d', (int) type); } + if (check_mod_func) +{ + if (grub_strcmp (name, SYM_USCORE grub_mod_init) == 0) + mod-init = (void (*) (grub_dl_t)) sym-st_value; + else if (grub_strcmp (name, SYM_USCORE grub_mod_fini) == 0) + mod-fini = (void (*) (void)) sym-st_value; +}
Re: Next release?
Pavel Roskin wrote: On Mon, 2008-07-14 at 18:55 +0200, Christian Franke wrote: Thanks for considering Cygwin support for the next release. The first (and last) grub package released in the Cygwin distribution was based on grub codebase from 2008-03-26. My latest reasonably tested merge is ~2 month old. If desired, I can merge test all remaining changes to current HEAD and post the patches for review soon. I'm sorry, I forgot that story. It may be a more radical change than I expected. Anyway, let's see what remains to be done. Meantime, I merged the Cygwin version to HEAD. First tests were successful. LZMA works and is a real benefit. Core.img size was critical due to the large ntfs.mod. With LZMA, there is enough space for ntfscomp.mod and even more. I posted the 3 remaining patches today, see: [PATCH] grub-probe -t prefix, fix update-grub_lib for Cygwin [PATCH] Kernel fixes for Cygwin [PATCH] Build fixes for Cygwin With these patches, Grub compiles works out-of-the-box on Cygwin. If there are no complaints, I will commit these in a week or so. Christian ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: I've a problem to compile i386 grub-emu
On Sun, 2008-07-20 at 16:59 +0200, Felix Zielcke wrote: From: Marco Gerards [EMAIL PROTECTED] I wonder why this got through the autoconf test. Rerunning ./configure with --enable-grub-emu and then make grub-emu did work Maybe it's possible to disallow compiling grub-emu if --enable-grub-emu wasn't given to configure? It's possible, but it will make the makefiles more complex. I think the focus should be on refactoring the build system. Then making such improvements could be done in one place, not for every platform. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Endianness macros capitalization
On Sun, 2008-07-20 at 15:45 +0200, Christian Franke wrote: Christian Franke wrote: But the function call in the 32-bit case requires only 5 bytes :-) Sorry, I was wrong here. The assumption about function call size was only true for module-local calls. If a module calls a function in kernel, each 5 byte call requires another 8 bytes for the ELF relocation table entry. Actually, the new versions of gcc have __builtin_bswap32 and __builtin_bswap64, which are optimized even better. There is no __builtin_bswap16 because it is said that any correct implementation will be optimized anyway: http://gcc.gnu.org/ml/gcc-patches/2006-08/msg00079.html gcc 4.3.0 from Fedora is working fine with them. I'm not sure about gcc 4.2. There is only one bug. Suppose I use this: #define grub_swap_bytes32(x) __builtin_bswap32(x) #define grub_swap_bytes64(x) __builtin_bswap64(x) Then I get this warning: partmap/apple.c: In function 'apple_partition_map_iterate': partmap/apple.c:133: warning: format '%x' expects type 'unsigned int', but argument 8 has type 'unsigned int' partmap/apple.c:133: warning: format '%x' expects type 'unsigned int', but argument 9 has type 'unsigned int' Those arguments are produced by grub_swap_bytes32(). But if I use wrapper functions, then there are no warnings, and the code side is approximately the same. We'll need wrappers to support sparse. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Fw: Re: Strong Crypto Support for GRUB2
On Mon, 2008-07-21 at 01:49 +0200, Simon Peter wrote: Did the message below ever arrive on the list? ... Date: Sun, 3 Feb 2008 18:25:47 +0100 From: Simon Peter [EMAIL PROTECTED] To: The development of GRUB 2 grub-devel@gnu.org Cc: Marco Gerards [EMAIL PROTECTED], Robert Millan [EMAIL PROTECTED] Subject: Re: Strong Crypto Support for GRUB2 I don't see it in the archives for February 2008: http://lists.gnu.org/archive/html/grub-devel/2008-02/index.html No idea why. But this message did arrive. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Drivemap module
El dom, 20-07-2008 a las 21:40 +0200, Marco Gerards escribió: Did you use code from other people or projects? No, as far as I can control my own mind: the assembly int13h handler is loosely based on that of GRUB Legacy, but heavily rewritten. All other code was written from scratch, even the crappy linked lists all over the place. For newcomers, full info on the patch is available on the list archives - it was proposed on June and its discussion deferred for two or three weeks because the developers were busy. I have copied the changelog entry from your other e-mail: * commands/i386/pc/drivemap.c : New file, main part of the new drivemap module allowing BIOS drive remapping not unlike the legacy map command. This allows to boot OSes with boot-time dependencies on the particular ordering of BIOS drives or trusting their own to be 0x80, like Windows XP, with non-standard boot configurations. New file. would be sufficient * commands/i386/pc/drivemap_int13h.S : New file, INT 13h handler for the drivemap module. Installed as a TSR routine by drivemap.c, performs the actual redirection of BIOS drives. Same here. Hmm... isn't the ChangeLog too spartan? I thought it should be a bit informative - what about a single sentence per file? * commands/i386/pc/drivemap.c : New file - drivemap command and int13h installer * commands/i386/pc/drivemap_int13h.S : New file, resident real mode BIOS int13 handler * conf/i386-pc.rmk : Added the new module Please say which variables you added in this file. You can find some examples on how to do this in the ChangeLog file. * conf/i386-pc.rmk (pkglib_MODULES) : Added drivemap.mod (drivemap_mod_SOURCES) : New variable (drivemap_mod_ASFLAGS) : Likewise (drivemap_mod_CFLAGS) : Likewise (drivemap_mod_LDFLAGS) : Likewise And now we're being uselessly verbose. IMHO, ChangeLog should be more about semantic changes in the code and less about literal changes - we have `svn diff' for those. * include/grub/loader.h : Added a just-before-boot callback infrastructure used by drivemap.mod to install the INT13 handler only when the boot command has been issued. Please describe changes, not effects. So which prototypes and macros did you add? * include/grub/loader.h (grub_loader_register_preboot) : New function (proto). Register a new pre-boot handler (grub_loader_unregister_preboot) : Likewise. Unregister handler (grub_preboot_hookid) : New typedef. Registered hook handle * kern/loader.c : Implement the preboot-hook described Which functions did you change and how? Please describe actual changes. * kern/loader.c (grub_loader_register_preboot) : New function. (grub_loader_unregister_preboot) : Likewise. (preboot_hooks) : New variable. Linked list of preboot hooks (grub_loader_boot) : Call the list of preboot-hooks before the actual loader The header is missing, please include it. Also newlines between the files make it easier to read. What header? The drivemap module itself has no .h files. The only header I touch is loader.h, and is both in the ChangeLog entry and the patch. Here follows a review. Sorry I kept you waiting for this long, this feature and your work is really appreciated! Perhaps I can spot some more problems after you fixed it and supplied an updated changelog entry. There are quite some comments, but please do not let this demotivate you, it is mainly coding style related :-) Well, thanks to all the time I had free, I have nearly finished Final Fantasy XII, so the wait was not soo bad ^^ (...) + +/* Uncomment the following line to enable debugging output */ +/* #define DRIVEMAP_DEBUG */ + +#ifdef DRIVEMAP_DEBUG +# define DBG_PRINTF(...) grub_printf(__VA_ARGS__) +#else +# define DBG_PRINTF(...) +#endif Please use the grub_dprintf infrastructure. Done. I didn't even know it existed :S +/* realmode far ptr = 2 * 16b */ +extern grub_uint32_t EXPORT_VAR (grub_drivemap_int13_oldhandler); +/* Size of the section to be copied */ +extern grub_uint16_t EXPORT_VAR (grub_drivemap_int13_size); + +/* NOT a typo - just need the symbol's address with symbol */ +typedef void grub_symbol_t; +extern grub_symbol_t EXPORT_VAR (grub_drivemap_int13_handler_base); +extern grub_symbol_t EXPORT_VAR (grub_drivemap_int13_mapstart); Please export stuff in header files, that's the normal practise in this file as well, right? What's not a typo? EXPORT_* macros removed; seemingly they are no longer needed because all code is in the same module (initially the assembly was in kernel). What's not a typo is the definition of grub_symbol_t as void instead of something more sound to a C programmer, like void *. I don't really know how to explain it in the source, but
Re: subversion repository structure
Quoting Yoshinori K. Okuji [EMAIL PROTECTED]: On Thursday 17 July 2008 21:10:11 Pavel Roskin wrote: Let's take another project and look at it as outsiders to get some perspective. Suppose gcc 5 is rewritten in Haskell. Do we expect it to be in a separate repository? Yes, I do, if it is so different. If something is rewritten from scratch, it is not continuous development but continual one. For example, just like libxml and libxml2. OK, I presented my arguments. If they are not convincing, so be it. If we cannot use the standard layout, then the current structure is my preference, simply because moving stuff around would make the history more complex and possibly confuse some tools. Anyway, we can live with any repository structure. We just need to document it. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel