From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mailman by lists.gnu.org with archive (Exim 4.43) id 1K4zve-0004F1-Ob for mharc-grub-devel@gnu.org; Sat, 07 Jun 2008 11:02:02 -0400 Received: from mailman by lists.gnu.org with tmda-scanned (Exim 4.43) id 1K4zvc-0004Et-LK for grub-devel@gnu.org; Sat, 07 Jun 2008 11:02:00 -0400 Received: from exim by lists.gnu.org with spam-scanned (Exim 4.43) id 1K4zvb-0004EU-Ij for grub-devel@gnu.org; Sat, 07 Jun 2008 11:02:00 -0400 Received: from [199.232.76.173] (port=38633 helo=monty-python.gnu.org) by lists.gnu.org with esmtp (Exim 4.43) id 1K4zvb-0004EH-CT for grub-devel@gnu.org; Sat, 07 Jun 2008 11:01:59 -0400 Received: from mta-out.inet.fi ([195.156.147.13]:46238 helo=jenni1.rokki.sonera.fi) by monty-python.gnu.org with esmtp (Exim 4.60) (envelope-from ) id 1K4zva-0005pv-EU for grub-devel@gnu.org; Sat, 07 Jun 2008 11:01:58 -0400 Received: from [127.0.0.1] (88.193.32.97) by jenni1.rokki.sonera.fi (8.5.014) id 483E82F10075677A for grub-devel@gnu.org; Sat, 7 Jun 2008 18:01:57 +0300 Message-ID: <484AA2ED.3040709@nic.fi> Date: Sat, 07 Jun 2008 18:02:05 +0300 From: =?ISO-8859-1?Q?Vesa_J=E4=E4skel=E4inen?= User-Agent: Thunderbird 2.0.0.14 (Windows/20080421) MIME-Version: 1.0 To: The development of GRUB 2 References: <1212701730.3141.25.camel@localhost> <48496649.6060905@nic.fi> <1212843337.13883.20.camel@localhost> In-Reply-To: <1212843337.13883.20.camel@localhost> X-Enigmail-Version: 0.95.6 Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: Quoted-Printable X-detected-kernel: by monty-python.gnu.org: Linux 2.6 (newer, 3) Subject: Re: [PATCH] Reimplementing the legacy "map" command X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: The development of GRUB 2 List-Id: The development of GRUB 2 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 07 Jun 2008 15:02:00 -0000 Javier Mart=EDn wrote: > El vie, 06-06-2008 a las 19:31 +0300, Vesa J=E4=E4skel=E4inen escribi=F3= : >> Did you forgot something from this patch :) ? > Er... not that I know of. What do you mean? Did I leave something > important off? If it's the licensing info, I put it under the same as > the whole GRUB2 project, I suppose. The int13 handler code, however, is > based (though changed) on the equivalent code in GRUB legacy. diff -pruN ... New files were missing from the patch. Anyway it is a good idea to=20 review the patch before sending it. >> No commented out code unless there is a really good reason for it. > Ok, I'll remove the "debug" sections in the int13 handler. Should I als= o > remove the non-error text output of the command? (like "mapping (hd1) t= o > 0x80") I have no idea what you have there as it wasn't in patch. Perhaps=20 differnet message in this case: "Mapping (hd0) as BIOS device 0x80." Shown during when hook is called? During registration of drive mapping (drivemap command), you then check=20 if this is possible or not and can give error or warning to screen. But=20 if everything is good no messages during processing of command. Only=20 show the stuff what is actually happening during the boot. Future extensions could be: drivemap --clear drivemap --ata (ata0) 0x80 drivemap --ata (ata1) --next-device drivemap --usb (usb0) --next-device drivemap --eltorito (cd0) --next-device drivemap --memdisk (hd0)/floppy-image.flp 0x00 --ata would install ATA handler (if loaded) --usb would install USB handler (if loaded) --eltorito would install el torito handler (if loaded) --memdisk would use memory disk for mapping drive (if loaded) After this sequence following would apply: 0x00: Floppy disk image on memdisk 0x80: ata0 device 0x81: ata1 device 0x82: usb device 0x83: cd-rom with el torito emulation Another extesion could be that if you can specify your harddisk with=20 some UUID it could then be used to identify disk as a source and grub=20 would find device matching it. (or use search command) >> Try to move int13h handle to module not to kernel. We do not want to p= ut >> anything more to kernel unless there is a really good reason to do so. > Seems A Good Thing (tm). However, all the loaders have their assembler > code in the kernel, and I have yet to find a single assembly file out o= f > the /kern dir (except for the setjmp.S support routines). As I state > below, I don't understand the GRUB build system quite well, and would > appreciate to have it explained so that I can break the asm part off th= e > kernel loader.S and into its own drivemap.S file. Well I think the first one to do so is gdb debugging patch recently=20 presnted on mailinglist. Check .rmk file for ideas. >> +/* This is NOT a typo: we just want this "void" var in order to take = its >> + * address with &var - used for relative addressing within the int13=20 >> handler */ >> +extern void EXPORT_VAR(grub_drivemap_int13_handler_base); >> >> Create new type for it. Or use variable type that eats that space. Thi= s=20 >> way you do not need this comment to confuse people. > Well, seems the comment was not as effective as I thought ^.^ - there i= s > _no_ space, the only purpose of that pseudo-variable is having its > address taken with the & operator. The only sensible "new type" for it > would be something akin to=20 > typedef void grub_symbol_t; > Which would be also puzzling and require a comment to stop people from > changing it to void*. However, the information would be more centralize= d > then and it would cause less head-scratching. There is space... if you do not have space, then where does the pointer=20 point :) ? If you provide external symbol let it be variable or function=20 there is always an address and space for it. Even if you say here that=20 this is uint8_t then you get same address that if you had uint32_t=20 declared as an "extern". extern there it give idea for compiler what=20 kind if data there is. It would be best to match what actually is there. So if you have 32 bits on assembler module you could do typedef grub_uint32_t my_custom_type_t; extern my_custom_type_t EXPORT_VAR(my_variable); Or just grub_uint32_t as a type. >> Please do not include .mk files on next patch. > I don't quite understand the GRUB2 build system right now, but if .mk > files don't get patched, how does it know they exist? Are those files > autogenerated somehow? I didn't find the autotools files... You modify .rmk files then run autogen.sh script on the root. >> Abort on error?... Ok... do you go to deinit everything before that wa= s=20 >> successfully installed or just give warning or ? > I don't know; either choice might be sensible and still has drawbacks > (increased complexity, potentially undoable actions, etc). My design of > this new functionality was a bit ad-hoc, and I added that flag thinking > "if the drives cannot be mapped, then the user wouldn't want the system > started". However, as you reason, with multiple preboot hooks the syste= m > could be let in a potentially inconsistent state. Right. So it might be a good idea to show message to screen and still=20 continue. User can easily see that there is a problem with something if=20 there is a clue on the screen. For some users this might generate=20 support request somewhere, but hey... it would generate it anyway should=20 it return back to grub...