Re: Regarding virtual to physical memory mapping..
El jue, 31-12-2009 a las 13:31 +0100, Vladimir 'φ-coder/phcoder' Serbinenko escribió: kiran pawar wrote: Also, when I allocated memory using grub_malloc() and printed the address returned from the function it was (0x7da6d810). This seems to be a 32-bit address. I wanted to know whether I can allocate some memory in the region below 1MB? Only in experimental branch. But we're not currently sure if we will use this. If you want to make a BIOS call then use memory at GRUB_SCRATCH_ADDR. If you do something else please explain How come not? Here's how it's done in drivemap, for example handler_base = grub_mmap_malign_and_register (16, total_size, drivemap_mmap, GRUB_MACHINE_MEMORY_RESERVED, GRUB_MMAP_MALLOC_LOW); -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit 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] Disable debug messages in reiserfs.mod
El sáb, 15-08-2009 a las 16:12 +0200, Vladimir 'phcoder' Serbinenko escribió: Hello. Recently we had no bug-reports about ReiserFS and debug messages and disabling them saves 533 bytes. I find it important especially because core.img with reiserfs is currently the biggest among filesystems supported in mainstream Completely removing debug messages from the compiled binaries should be selectable as a GRUB-wide configure-time switch, defaulting to _include_ such strings. Furthermore, removing the possibility of enabling debug messages at runtime from certain modules (reiserfs) but not others (ext4? vbe? drivemap?) will present the user with an non-uniform interface - confusing and maybe frustrating. In my opinion, there are better, wiser ways of reducing the size of core.img, such as the new object format proposed by Bean, so unless there is some kind of emergency like some recent patch brought a common pc+biosdisk+reiserfs core.img over the size limit, I don't think this patch would be a good decision. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Centralizing understanding of far pointers
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Robert Millan escribió: First of all, please don't call them far pointers. They're an i8086 legacy cruft, which have nothing to do with far or close really (although we seem to have some code that makes this reference already). So... how do we call them? I am an utter failure at making up sensible names. What you're doing is basically the same as real2pm() function we already had. It seems this function should move elsewhere so it can have the generic use you intended (it can be reimplemented as well, if that makes it cleaner). Indeed, that's what I thought when I saw that function in some vbe-related file. I remembered that I wrote similar code for drivemap, and then reasoned that mmap would used too, as it modified the IVT. Thus, we are uselessly duplicating code that is simple but at the same time error-prone. The idea of this patch is to provide a way to handle the abstraction of a pointer that is somehow mangled by the architecture, but the user code doesn't need to actually know how. For example, the patched drivemap would just take whatever linear2real(ptr) gives and stores it into the IVT slot. Of course, this is not as magical as an ideal world would have it, because the code still has to know the IVT to place it there, but I think it's still a plus. +typedef union +{ + struct + { +/* Given that i386 is little-endian, this order matches the in-memory + format of CPU realmode far pointers. */ +grub_uint16_t offset; +grub_uint16_t segment; + } __attribute__ ((packed)); + grub_uint32_t raw_bits; Is there a usefulness in this `raw_bits' member? It doesn't have any real meaning, as it doesn't correspond to an actual address. Mainly, the possibility of checking equality against a particular bit pattern without using another instance of the structure/union. Also, some code might want to still use the raw bits for some reason, instead of either the nearly-opaque handling described above or the more detailed view provided by the inner structure. +} grub_machine_farptr; I admit this is a bit confusing, but the `machine' namespace is for things specific to a given firmware. i8086 mode is part of the i386 architecture, so we'd put this in the `cpu' namespace instead. OK then... That means the code will have to move from i386/pc/memory.h to i386/memory.h. And what about x86_64? -BEGIN PGP SIGNATURE- Version: GnuPG v2.0.12 (MingW32) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iQIcBAEBAgAGBQJKb5qBAAoJEKSl+Fbdeo72EfMP/0JVO7mO92Pyk9l5tiGP5IQ+ PbrLJBX1HtAemwMufW5wR+OoQjyb9LGf1JR0ff7o7Xkbtnc3iN7EI8RV4WAzuLp3 9tYpYZkIwLWa86GM8Oy8QWer58vNlHYNtH+Zmrv+341umtFZERRjWk6NwpNNgFtm uX8FoYGqHLIhkzLWvd5vM7V+8S9LJPjq2ws/F1NbS5Sd33wTz81eFz6TNmoALNuD KeIR9Vo6f1zN3lp0M9+CL1fzX7uP3e3dWJ+KmOlkJcEJgpsRJhz76Ux9Byq0Tby9 SLXQmEfBmSNk95HpXnn2AFuiH/o8SYyWExPAEbz//Ttym4ElGGfS+tE3d8b7auga Y2ReU8I9XlbMczIjvnh6MaLOwzGuu6mtPFvec5PHSK9sekJ2TWYCosXpUkMo58TS vXPou5VDAkRb6sbuMx8g7fGws5vgYuEYcIZAkW3ejcJhIkMSGODEYRsT2A8ZREzx Qs/B48Z9Tk4nVxdPrIfDg7eB4TtEpJak84uw/fV6/WclY5ipk/SGf0iyhDfuU7vp 6w+6Kbvny0dOyssMz8cN2MlL7DinAtx2Zzi0jVHBNl253Af/idPJAPw69LDFWroN EHjLHxxnnkF4MOiR7WzH49QRzLCuGmMfgnqKEdEW1JxNFqG5+zE0L3p1Sp/pITs7 q+2CU6+NR3veOdzg3yRh =dKOS -END PGP SIGNATURE- ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Check for the appropriate condition in types.h
El jue, 23-07-2009 a las 16:57 -0400, Pavel Roskin escribió: On Thu, 2009-07-23 at 22:30 +0200, Vladimir 'phcoder' Serbinenko wrote: [GRUB_CPU_SIZEOF_VOID_P == 8]: Changed to ... [GRUB_CPU_SIZEOF_LONG == 8]: ... this. Ok, let's adopt this form instead. The proposed ChangeLog would now be: From the GNU Coding Standards: C programs often contain compile-time `#if' conditionals. Many changes are conditional; sometimes you add a new definition which is entirely contained in a conditional. It is very useful to indicate in the change log the conditions for which the change applies. Our convention for indicating conditional changes is to use square brackets around the name of the condition. It means that the square brackets are used if the changes only affect the code under the condition specified in brackets. This is not what is happening here. This is exactly what happens here: we change only what happens in conditionals [GRUB_CPU_SIZEOF_VOID_P == 8] and [GRUB_CPU_SIZEOF_LONG == 8] I'm not interested to discuss the right interpretation of the coding standards. Frankly, I'm not a fan of keeping ChangeLog, as it's too formal and it's a point of contention for parallel development. I prefer the Linux kernel style of descriptions. But since we are updating ChangeLog, let's keep it readable. So, how would this precise change be specified? I don't mean to seem anxious, but I wouldn't want such a simple patch to be ensnared by what I personally consider a Byzantine argument. I propose the following wording, but we need someone more familiar with the FSF bureaucracy to review it: O Robert, we summon Thou ;) 2009-07-26 Javier Martin lordhab...@gmail.com * include/grub/types.h (GRUB_CPU_SIZEOF_VOID_P == 8): Change... (GRUB_CPU_SIZEOF_LONG == 8) ... into this where appropriate (UINT_TO_PTR): move outside wordsize conditionals (PTR_TO_UINT): new macro (UINT_TO_PTR): move outside wordsize conditionals (PTR_TO_UINT): new macro We should remove PTR_TO_UINT32 and PTR_TO_UINT64 with PTR_TO_UINT everywhere. I've checked that it doesn't introduce any warnings on any platform. Sometimes PTR_TOUINT32 with precision loss is exactly what the coder wants. A typical example is booting 32-bit OS on 64-bit platform. This is the case of booting linux on efi64. Then the code has to ensure that the target is below 4GiB (see my mm propositions for more on how to ensure it). But I'm ok with requirement of additional explicit cast in such cases as (grub_uint32_t) PTR_TO_UINT (x) That would be much better that PTR_TO_UINT32, as it makes the cast explicit in the code that should ensure that the cast is valid. We can find such cases by compiling with -Wconversion and finding the newly appearing warnings after PTR_TO_UINT32 and PTR_TO_UINT64 are replaced with PTR_TO_UINT. I also agree that PTR_TO_UINTnn should not exist: the eventual precision loss should be shown in the actual source using/abusing it. After this patch is merged, I could send in another one replacing all occurrences of those two macros by PTR_TO_UINT with the right casts. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
[PATCH] Centralizing understanding of far pointers
This patch modifies the machine-specific memory.h (currently, just the i386-specific file), adding a new type grub_machine_farptr and two functions to convert between such far pointers and normal C pointers. The code performing the mapping between realmode and pmode addresses is simple, and thus is repeated in many source files like drivemap, vbe*, mmap, etc. However, as simple as it is, its ad-hoc application tends to be quite unwieldy, generating long code that is verges close to the tag of write-only code. The i386 farptr type has been implemented as an union between the uint32 that was used until now (.raw_bits) and a structure separating the uint16 segment and offset parts for easy access and debug printing. This post has three attachments: the patch to memory.h itself, a patch to the i386 mmap.c and its helper asm file, to show the impact of this patch (I've also taken the liberty of adding an offset macro just like in drivemap, and make the changed code more elegant in my opinion), and another patch doing the same for drivemap. NOTE: this patch depends on the PTR_TO_UINT macro added by another patch still on discussion, [1]. [1] http://lists.gnu.org/archive/html/grub-devel/2009-07/msg00398.html -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit Index: include/grub/i386/pc/memory.h === --- include/grub/i386/pc/memory.h (revisión: 2447) +++ include/grub/i386/pc/memory.h (copia de trabajo) @@ -126,6 +126,32 @@ } #endif +typedef union +{ + struct + { +/* Given that i386 is little-endian, this order matches the in-memory + format of CPU realmode far pointers. */ +grub_uint16_t offset; +grub_uint16_t segment; + } __attribute__ ((packed)); + grub_uint32_t raw_bits; +} grub_machine_farptr; + +static inline void * +grub_machine_real2linear (grub_machine_farptr ptr) +{ + return UINT_TO_PTR grub_uint32_t) ptr.segment) 4) + ptr.offset); +} + +static inline grub_machine_farptr +grub_machine_linear2real (void *addr) +{ + grub_machine_farptr ret = { { PTR_TO_UINT32 (addr) 0x000F, + (PTR_TO_UINT32 (addr) 0x0000) 4 } }; + return ret; +} + #endif #endif /* ! GRUB_MEMORY_MACHINE_HEADER */ Index: mmap/i386/pc/mmap_helper.S === --- mmap/i386/pc/mmap_helper.S (revisión: 2447) +++ mmap/i386/pc/mmap_helper.S (copia de trabajo) @@ -48,9 +48,8 @@ popf /* ljmp */ .byte 0xea -VARIABLE (grub_machine_mmaphook_int15offset) +VARIABLE (grub_machine_mmaphook_oldint15) .word 0 -VARIABLE (grub_machine_mmaphook_int15segment) .word 0 e801: Index: mmap/i386/pc/mmap.c === --- mmap/i386/pc/mmap.c (revisión: 2447) +++ mmap/i386/pc/mmap.c (copia de trabajo) @@ -25,17 +25,20 @@ #define min(a,b) (((a) (b)) ? (a) : (b)) +#define OFFSET_IN_ASM(addr) ( PTR_TO_UINT (addr) \ + - PTR_TO_UINT (grub_machine_mmaphook_start) ) + static void *hooktarget = 0; +/* Not a null pointer, the real mode IVT is actually at address 0:0 */ +static grub_machine_farptr *grub_machine_ivt = 0; -extern grub_uint8_t grub_machine_mmaphook_start; -extern grub_uint8_t grub_machine_mmaphook_end; -extern grub_uint8_t grub_machine_mmaphook_int12; -extern grub_uint8_t grub_machine_mmaphook_int15; +extern const void grub_machine_mmaphook_start; +extern const void grub_machine_mmaphook_end; +extern const void grub_machine_mmaphook_int12; +extern const void grub_machine_mmaphook_int15; -static grub_uint16_t grub_machine_mmaphook_int12offset = 0; -static grub_uint16_t grub_machine_mmaphook_int12segment = 0; -extern grub_uint16_t grub_machine_mmaphook_int15offset; -extern grub_uint16_t grub_machine_mmaphook_int15segment; +static grub_machine_farptr grub_machine_mmaphook_oldint12; +extern grub_machine_farptr grub_machine_mmaphook_oldint15; extern grub_uint16_t grub_machine_mmaphook_mmap_num; extern grub_uint16_t grub_machine_mmaphook_kblow; @@ -73,9 +76,8 @@ grub_dprintf (mmap, installing preboot handlers\n); - hookmmapcur = hookmmap = (struct grub_e820_mmap_entry *) -((grub_uint8_t *) hooktarget + (grub_machine_mmaphook_end -- grub_machine_mmaphook_start)); + hookmmapcur = hookmmap = UINT_TO_PTR (PTR_TO_UINT (hooktarget) + + OFFSET_IN_ASM(grub_machine_mmaphook_end)); grub_mmap_iterate (fill_hook); grub_machine_mmaphook_mmap_num = hookmmapcur - hookmmap; @@ -90,24 +92,23 @@ *((grub_uint16_t *) 0x413) = grub_mmap_get_lower () 10; /* Save old interrupt handlers. */ - grub_machine_mmaphook_int12offset = *((grub_uint16_t *) 0x48); - grub_machine_mmaphook_int12segment = *((grub_uint16_t *) 0x4a); - grub_machine_mmaphook_int15offset = *((grub_uint16_t *) 0x54); - grub_machine_mmaphook_int15segment = *((grub_uint16_t *) 0x56); + grub_machine_mmaphook_oldint12 = grub_machine_ivt[0x12]; +
[PATCH] Check for the appropriate condition in types.h
Currently grub/types.h verifies that sizeof(void*)==sizeof(long) and then proceeds to define most fixed-length types based on sizeof(void*). While simplification seems a good idea on paper, it is always good practice to check specifically for what we want to know. In particular, the assumption that long can hold a pointer will go down when (if) mingw64 support is merged, because Win64 follows the LLP64 model instead of the LP64 *nix model. Given that currently both values are, by precondition, the same, this change is guaranteed not to create any problems, while it might avoid them in the future. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit diff --git a/include/grub/types.h b/include/grub/types.h index 8e2ad15..50b253a 100644 --- a/include/grub/types.h +++ b/include/grub/types.h @@ -62,7 +62,7 @@ typedef signed char grub_int8_t; typedef short grub_int16_t; typedef int grub_int32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 typedef long grub_int64_t; #else typedef long long grub_int64_t; @@ -71,7 +71,7 @@ typedef long long grub_int64_t; typedef unsigned char grub_uint8_t; typedef unsigned short grub_uint16_t; typedef unsigned grub_uint32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 typedef unsigned long grub_uint64_t; #else typedef unsigned long long grub_uint64_t; @@ -100,7 +100,7 @@ typedef grub_uint32_t grub_size_t; typedef grub_int32_t grub_ssize_t; #endif -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 # define GRUB_ULONG_MAX 18446744073709551615UL # define GRUB_LONG_MAX 9223372036854775807L # define GRUB_LONG_MIN (-9223372036854775807L - 1) signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Check for the appropriate condition in types.h
El jue, 23-07-2009 a las 10:37 -0400, Pavel Roskin escribió: On Thu, 2009-07-23 at 13:10 +0200, Javier Martín wrote: Currently grub/types.h verifies that sizeof(void*)==sizeof(long) and then proceeds to define most fixed-length types based on sizeof(void*). While simplification seems a good idea on paper, it is always good practice to check specifically for what we want to know. In particular, the assumption that long can hold a pointer will go down when (if) mingw64 support is merged, because Win64 follows the LLP64 model instead of the LP64 *nix model. Given that currently both values are, by precondition, the same, this change is guaranteed not to create any problems, while it might avoid them in the future. The patch is good. Please include the ChangeLog entry. Thanks. I have though of an additional change: if it seems acceptable, apply the new version of the patch that I attach with this message. If it is not, just use the old version in the original post and I'll send the new change as another patch. The additional change is the refactoring of the UINT_TO_PTR macro. Given that we have a grub_addr_t type fulfilling a role similar to the standard uintptr_t (an integral type long enough to hold a pointer and back), there is no need for the conditional definition of UINT_TO_PTR as either casting to a 32 or 64-bit type: grub_addr_t is defined exactly like that. Furthermore, I have added a PTR_TO_UINT macro as a means to perform safer* hard pointer arithmetic that frequently comes up in low-level code without having to think of the types to cast to and back. This way, this random code: (in acpi.c) target = (grub_uint8_t *) long) target - 1) | 0xf) + 1); Would become: target = UINT_TO_PTR(((PTR_TO_UINT(target) - 1) | 0xf) + 1); Which is safe* as long as grub_addr_t is properly defined. Thus, this new macro will allow coders to gather scattered typing decisions and centralize them to types.h. The process would be just: PTR_TO_UINT - perform weird arithmetic - UINT_TO_PTR. * safer in the sense of not risking any truncations or undefined behavior if the addr_t types change, of course, not for the actual arithmetic being performed Regarding the ChangeLog entry, I always have problems with them. What about this one?: 2009-07-23 Javier Martin lordhab...@gmail.com * include/grub/types.h (GRUB_CPU_SIZEOF_VOID_P): substitute for GRUB_CPU_SIZEOF_LONG where appropriate (UINT_TO_PTR): move outside wordsize conditionals (PTR_TO_UINT): new macro -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit diff --git a/include/grub/types.h b/include/grub/types.h index 8e2ad15..72f1288 100644 --- a/include/grub/types.h +++ b/include/grub/types.h @@ -62,7 +62,7 @@ typedef signed char grub_int8_t; typedef short grub_int16_t; typedef int grub_int32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 typedef long grub_int64_t; #else typedef long long grub_int64_t; @@ -71,7 +71,7 @@ typedef long long grub_int64_t; typedef unsigned char grub_uint8_t; typedef unsigned short grub_uint16_t; typedef unsigned grub_uint32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 typedef unsigned long grub_uint64_t; #else typedef unsigned long long grub_uint64_t; @@ -100,7 +100,7 @@ typedef grub_uint32_t grub_size_t; typedef grub_int32_t grub_ssize_t; #endif -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 # define GRUB_ULONG_MAX 18446744073709551615UL # define GRUB_LONG_MAX 9223372036854775807L # define GRUB_LONG_MIN (-9223372036854775807L - 1) @@ -110,12 +110,12 @@ typedef grub_int32_t grub_ssize_t; # define GRUB_LONG_MIN (-2147483647L - 1) #endif +#define UINT_TO_PTR(x) ((void*)(grub_addr_t)(x)) +#define PTR_TO_UINT(x) ((grub_addr_t)(x)) #if GRUB_CPU_SIZEOF_VOID_P == 4 -#define UINT_TO_PTR(x) ((void*)(grub_uint32_t)(x)) #define PTR_TO_UINT64(x) ((grub_uint64_t)(grub_uint32_t)(x)) #define PTR_TO_UINT32(x) ((grub_uint32_t)(x)) #else -#define UINT_TO_PTR(x) ((void*)(grub_uint64_t)(x)) #define PTR_TO_UINT64(x) ((grub_uint64_t)(x)) #define PTR_TO_UINT32(x) ((grub_uint32_t)(grub_uint64_t)(x)) #endif signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] Check for the appropriate condition in types.h
El jue, 23-07-2009 a las 17:40 +0200, Vladimir 'phcoder' Serbinenko escribió: Actualy I should have used ALIGN_UP here It's just a random sample of code, brought up by a search for (long). I'm pretty sure more examples abound. Regarding the ChangeLog entry, I always have problems with them. What about this one?: 2009-07-23 Javier Martin lordhab...@gmail.com * include/grub/types.h (GRUB_CPU_SIZEOF_VOID_P): substitute for This would suggest that you modify GRUB_CPU_SIZEOF_VOID_P macro which isn't the case. I would prefer something like: [GRUB_CPU_SIZEOF_VOID_P == 8]: Changed to ... [GRUB_CPU_SIZEOF_LONG == 8]: ... this. Ok, let's adopt this form instead. The proposed ChangeLog would now be: 2009-07-23 Javier Martin lordhab...@gmail.com * include/grub/types.h [GRUB_CPU_SIZEOF_VOID_P == 8]: Changed to ... [GRUB_CPU_SIZEOF_LONG == 8]: ...this (UINT_TO_PTR): move outside wordsize conditionals (PTR_TO_UINT): new macro -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC, PATCH] C99 format specifiers for fixed-length integer types
Well, I'll try to address you both at once... Let me say that seeing more messages arrive with fresh arguments as I tried to build a coherent response was distressing ;) -- Pavel, you are right. I recognize I've gone overboard trying to be the paladin of standards, and thus I'd like to withdraw most of the patch. The only part that is useful and might someday become necessary is that dealing with 64-bit integers. In particular, if and when mingw64 support is added, for the reasons stated before. You could argue that we could just use long long and its format specifiers for all 64-bit integers everywhere, since it has that length in all supported platforms. However, we have no guarantees this will continue to be so in the future even for our currently supported platforms, and GCC 5.0 might as well have a 128-bit long long (MIPS already has an uint128_t), so making printf extract 16 bytes with %ll and give it only 8 with an uint64_t argument could be the kind of fun that takes a programmer hours to debug. With the reduced version of the patch I'm putting forward, such a (hypothetical, indeed) change will only impact types.h, while otherwise many source files will need to be modified in a hunt for %lls and their variations. We can consider lower types safe since the autopromotion rules will keep the compiler happy even if int becomes 64-bit. -- Vladimir, I've followed your advice and restored the names for the remaining macros in this new patch. I'm open to any compromise between the GRUB_ convention and the need for less verbosity, given that 64-bit numbers are used widely throughout the code base. Conventions are just ways to make the life of us coders easier, but, without showing disrespect for them, they are not dogma. We must be ready to shove them apart if they get in the way. Respect to the utility of the [U]INTn_C(x) macros, they append the required modifiers to make sure the compiler interprets x as an [u]intn_t constant. For example, UINT64_C(1) - 1uL in my amd64 Ubuntu box, but 1uLL in my WinXP Pro x64 box with mingw64 gcc. I agree that adding them goes outside the scope of the announced format specifiers, but one without the other is pretty much useless in most real use cases, as I noticed when trying to use my initial patch in real GRUB code. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit Index: include/grub/types.h === --- include/grub/types.h (revision 2440) +++ include/grub/types.h (working copy) @@ -64,8 +64,14 @@ typedef int grub_int32_t; #if GRUB_CPU_SIZEOF_VOID_P == 8 typedef long grub_int64_t; +#define GRUB_PRIi64 li +#define GRUB_PRId64 ld +#define GRUB_INT64_C(x) (x ## L) #else typedef long long grub_int64_t; +#define GRUB_PRIi64 lli +#define GRUB_PRId64 lld +#define GRUB_INT64_C(x) (x ## LL) #endif typedef unsigned char grub_uint8_t; @@ -73,8 +79,18 @@ typedef unsigned grub_uint32_t; #if GRUB_CPU_SIZEOF_VOID_P == 8 typedef unsigned long grub_uint64_t; +#define GRUB_PRIu64 lu +#define GRUB_PRIo64 lo +#define GRUB_PRIx64 lx +#define GRUB_PRIX64 lX +#define GRUB_UINT64_C(x) (x ## UL) #else typedef unsigned long long grub_uint64_t; +#define GRUB_PRIu64 llu +#define GRUB_PRIo64 llo +#define GRUB_PRIx64 llx +#define GRUB_PRIX64 llX +#define GRUB_UINT64_C(x) (x ## ULL) #endif /* Misc types. */ signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] New object format
El mié, 22-07-2009 a las 19:12 +0800, Bean escribió: Fix some wrong assumption in types.h and efi header files. For example, grub_efi_uint_t is defined as unsigned long, but it should be grub_uint64_t in 64-bit EFI, this problem won't show previously as unsigned long is 64-bit in elf64 gcc, but it's 32-bit in mingw-w64 gcc. I think you haven't corrected _all_ such assumptions. For example, in your grub/types.h: #if GRUB_CPU_SIZEOF_VOID_P == 8 # define GRUB_ULONG_MAX 18446744073709551615UL # define GRUB_LONG_MAX 9223372036854775807L # define GRUB_LONG_MIN (-9223372036854775807L - 1) #else # define GRUB_ULONG_MAX 4294967295UL # define GRUB_LONG_MAX 2147483647L # define GRUB_LONG_MIN (-2147483647L - 1) #endif In mingw64, sizeof(void*) = 8, but ULONG_MAX = 2^32-1. grub/machine/types.h defines a GRUB_TARGET_SIZEOF_LONG that might be suitable for this. Or am I mixing target with host? -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] New object format
El mié, 22-07-2009 a las 21:34 +0800, Bean escribió: 2009/7/22 Javier Martín lordhab...@gmail.com: El mié, 22-07-2009 a las 19:12 +0800, Bean escribió: Fix some wrong assumption in types.h and efi header files. For example, grub_efi_uint_t is defined as unsigned long, but it should be grub_uint64_t in 64-bit EFI, this problem won't show previously as unsigned long is 64-bit in elf64 gcc, but it's 32-bit in mingw-w64 gcc. I think you haven't corrected _all_ such assumptions. For example, in your grub/types.h: #if GRUB_CPU_SIZEOF_VOID_P == 8 # define GRUB_ULONG_MAX 18446744073709551615UL # define GRUB_LONG_MAX 9223372036854775807L # define GRUB_LONG_MIN (-9223372036854775807L - 1) #else # define GRUB_ULONG_MAX 4294967295UL # define GRUB_LONG_MAX 2147483647L # define GRUB_LONG_MIN (-2147483647L - 1) #endif In mingw64, sizeof(void*) = 8, but ULONG_MAX = 2^32-1. grub/machine/types.h defines a GRUB_TARGET_SIZEOF_LONG that might be suitable for this. Or am I mixing target with host? Hi, Oh, thanks for the note. In fact, we can use GRUB_CPU_SIZEOF_LONG, its value is GRUB_TARGET_SIZEOF_LONG when building target, and SIZEOF_LONG when building utilities. Wonderful idea. I'm researching this for a possible implementation of a GRUB equivalent to the C99 fixed-length integers print specifiers like PRIx64. It would most likely go into grub/types.h, so I'm paying special attention to anyone working on that file. As GRUB_CPU_SIZEOF_LONG doesn't necessary equal to GRUB_CPU_SIZEOF_VOID_P. (It actually avoid this by generating an #errror message when GRUB_CPU_SIZEOF_VOID_P != GRUB_CPU_SIZEOF_LONG). I think you lost me here. This check is in the trunk GRUB, but not in your lib branch, and actually it would make it impossible to build on mingw64, where void* is 64-bit and long is 32-bit. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] UUID support for UFS
El mié, 22-07-2009 a las 19:47 +0200, Robert Millan escribió: On Tue, Jul 21, 2009 at 11:34:42PM +0200, Javier Martín wrote: If int and int32_t are different types, gcc will warn about it, at least for implicit conversion with data loss. Oh, yes... with the current build system and without -Werror, warnings are _very_ visible. /sarcasm Javier, I think I told you once already, but if you're interested in productive discussion you should drop that tone. As for -Werror, we're already discussing about enabling it in another thread. I'm sorry for the exabrupt. Yes, you told me once and at 20 I should be mature enough to not let myself down this path... I'll open a new thread with the changes I'm proposing to types.h. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
[RFC,PATCH] C99 format specifiers for fixed-length integer types
This patch modifies the global types.h header to define a number of macros used for the formatted output of fixed-length integers like grub_uint64_t. Currently we use the traditional %llu format specifiers, adding casts as required to assuage the loud GCC. However, casts to shut the compiler up are generally a bad idea, and the fact that this kind of output occurs mostly in debug code (printing an inode number, for example) which is dispersed about the GRUB codebase makes it very likely that the eventual breakage of one of these assumptions will result in either a stream of warnings/errors or, given that we are using casts to shut the compiler up, runtime weirdness. Using these C99-like format specifiers will allow us to have the relevant assumptions centralized in a single file, namely grub/types.h. Thus, if and when one is broken - for example, when sizeof(void*)!=sizeof(long), as it happens in mingw64 - the fix will be way simpler to develop and deploy. For example, using a fictional MyFS: typedef struct { grub_uint64_t ino_num; grub_uint16_t perm_flags; } myfs_ino_t; myfs_ino_t *cur = ... The patch will turn something like this: grub_dprintf(myfs, Inode number %llu has permissions %03o, (unsigned long) grub_be_to_cpu64(cur-ino_num), grub_be_to_cpu16(cur-perm_flags)); Into this: grub_dprintf(myfs, Inode number %GRUB_PRI64u has permissions %03GRUB_PRI16o, grub_be_to_cpu64(cur-ino_num), grub_be_to_cpu16(cur-perm_flags)); -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit Index: include/grub/types.h === --- include/grub/types.h (revision 2438) +++ include/grub/types.h (working copy) @@ -58,23 +58,53 @@ # endif #endif -/* Define various wide integers. */ +/* Define various wide integers and their format specifiers. */ typedef signed char grub_int8_t; typedef short grub_int16_t; typedef int grub_int32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#define GRUB_PRIi8 i +#define GRUB_PRIi16 i +#define GRUB_PRIi32 i +#define GRUB_PRId8 d +#define GRUB_PRId16 d +#define GRUB_PRId32 d +#if GRUB_CPU_SIZEOF_LONG == 8 typedef long grub_int64_t; +#define GRUB_PRIi64 li +#define GRUB_PRId64 ld #else typedef long long grub_int64_t; +#define GRUB_PRIi64 lli +#define GRUB_PRId64 lld #endif typedef unsigned char grub_uint8_t; typedef unsigned short grub_uint16_t; typedef unsigned grub_uint32_t; -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#define GRUB_PRIu8 u +#define GRUB_PRIu16 u +#define GRUB_PRIu32 u +#define GRUB_PRIo8 o +#define GRUB_PRIo16 o +#define GRUB_PRIo32 o +#define GRUB_PRIx8 x +#define GRUB_PRIx16 x +#define GRUB_PRIx32 x +#define GRUB_PRIX8 X +#define GRUB_PRIX16 X +#define GRUB_PRIX32 X +#if GRUB_CPU_SIZEOF_LONG == 8 typedef unsigned long grub_uint64_t; +#define GRUB_PRIu64 lu +#define GRUB_PRIo64 lo +#define GRUB_PRIx64 lx +#define GRUB_PRIX64 lX #else typedef unsigned long long grub_uint64_t; +#define GRUB_PRIu64 llu +#define GRUB_PRIo64 llo +#define GRUB_PRIx64 llx +#define GRUB_PRIX64 llX #endif /* Misc types. */ @@ -100,7 +130,7 @@ typedef grub_int32_t grub_ssize_t; #endif -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 # define GRUB_ULONG_MAX 18446744073709551615UL # define GRUB_LONG_MAX 9223372036854775807L # define GRUB_LONG_MIN (-9223372036854775807L - 1) signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC,PATCH] C99 format specifiers for fixed-length integer types
El mié, 22-07-2009 a las 21:08 -0400, Pavel Roskin escribió: I doubt about runtime weirdness. gcc is good at catching such problems at the compile time. Yes, in naked expressions. However, once we start using casts the C compiler tends to be quite silent about the whole nitty-gritty process. For example, the following code generates no warnings (with -ansi -pedantic -Wall -Wconversion): unsigned short a = (unsigned short) 123456789; uint16_t b = (uint16_t) UINT32_C(123456789); That is, casts shut the compiler up (without the casts gcc just shows a warning about the truncation being performed). The runtime value of both a and b is 0xcd15 in my x86_64 Linux box. It would be a good idea to use standard modifiers if they were not so hard on the eye. Well, I suggested GRUB_x names for the macros, where x is the C99 standard name, but we could use them directly as PRIx64 and the like. About verbosity, though... Have you programmed in Java? Some appearances of GRUB_PRId32 are nothing compared to System.out.println or java.util.ArrayListInteger, yet I write the last two religiously. That's a biased example where the size of the arguments is obvious. And even then, it's hard to read. And if you put GRUB_PRI32o instead of GRUB_PRI16o there, the compiler won't tell you anything. Hey, I'm the one proposing the patch. Surely you wouldn't expect me to be unbiased ;) Nevertheless, the issue you pointed happens because variadic arguments are automatically promoted with the following rules [1][2] from the ancient days of C: - Integral types narrower than int - int - Floating point types narrower than double - double So, you could argue, we could do without at the very least PRI?8 (and, here in grub, PRI?16 too). However, given how obscure this feature is (see [1]), I'd go for keeping all of them: orthogonality means less surprises for the future coders. The patch includes the easy part, namely adding the macros. But I doesn't think it would be so pretty with the ugly part, that is, fixing every almost every *printf call. It will be very hard to review. Well, of course. This patch only adds the infrastructure, or else it would be too invasive. Once it is in, a small number of people can start checking most source files and replace the old specifiers when appropriate. -#if GRUB_CPU_SIZEOF_VOID_P == 8 +#if GRUB_CPU_SIZEOF_LONG == 8 This belongs to a separate patch. Oops, sorry. It seems I had a little mixup between the trunk files and Bean's lib branch. I might post this change as a separate patch indeed. [1] I would point at section 6.5.2.2.6 (function calls, automatic argument promotions) of the C99 standard, but I really do not understand that paragraph quite well. [2] Here it says so with more certainty, but they seem to be class notes http://www.eumus.edu.uy/eme/c/c-notes_summit/intermediate/sx11.html#sx11c -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] UUID support for UFS
Pavel Roskin escribió: On Tue, 2009-07-21 at 23:45 +0200, Vladimir 'phcoder' Serbinenko wrote: This change would allow to produce a code which is cleaner, easier to read and understand. However I'm opposed to modifying printf function for it. I think Javier misspoke or didn't realize that *printf doesn't need to be modified. Indeed, I didn't realise so. All that's required is a grub_inttypes header that either takes from the system stdint.h types or just defines the macros you were discussing. Instead we could just define somewhere: GRUB_PRIx32 %x Better yet, x, as in libc. That would allow for %08x that we need for UUID. Yeah, that is the format used in libc. I copied the example wrong, without the %08s. But I would prefer that we work on fixing bugs rather than non-bugs. Fixing this would allow us to have cleaner code, and separate casual variables from fixed-length variables. If we print int with %d and int32_t with PRId32, the impact of the subtle bugs that appear when we port across architectures will be reduced. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH] UUID support for UFS
Pavel Roskin escribió: On Tue, 2009-07-21 at 22:14 +0200, Javier Martín wrote: But I would prefer that we work on fixing bugs rather than non-bugs. Fixing this would allow us to have cleaner code, and separate casual variables from fixed-length variables. If we print int with %d and int32_t with PRId32, the impact of the subtle bugs that appear when we port across architectures will be reduced. If int and int32_t are different types, gcc will warn about it, at least for implicit conversion with data loss. Oh, yes... with the current build system and without -Werror, warnings are _very_ visible. /sarcasm Besides, do we really have -Wconversion enabled? I don't know, but gcc tends to be quite silent when it comes to type conversion, mainly due to the laxitude it's used in *nix C programs. The cast in that code was all but implicit, and explicit casts tend to shut the compiler up. It's more likely that bugs will be introduced by that change, not fixed. Besides, the code will be harder to read. You say it'd be harder to read because the macros are newly-introduced (C99) and thus not widely know. However, they are pretty clear and self-explanatory once you google them the first time... and at the very least they call attention to themselves: an unknowing programmer would wonder what they are. Using a normal print specifier and a type cast does not. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Fw: gettext support
El mar, 23-06-2009 a las 12:05 +0200, Jordi Mallach escribió: On Mon, Jun 22, 2009 at 09:42:17PM +0200, Javier Martín wrote: msgid Use the %C and %C keys to select which entry is highlighted. msgid Press enter to boot the selected OS, 'e' to edit the msgid commands before booting or 'c' for a command-line. Actually, the first string could stand on its own just fine, but the second and the third should be joined. I don't agree. This is part of a full paragraph, and the ideal string would be just one. It's a mere coincidence that the first example msgid I wrote ends on a full stop. A translation would possibly span to two lines, and then you would have a problem, with the first single line taking one complete line and possibly just a few columns of the second, but you wouldn't be able to add more to the second line, as the msgid ended there. Why not? A msgid does not necessarily mean a \n, you can printf(%s %s, _(first), _(second)) just OK without an intervening line break. In this particular case the gettext directives [0] don't tell us either to merge or split: the two sentences are related, yes, but are perfectly translatable on their own as their particular meaning does not depend on one another. I guess it's a design decision. Well, s/Utilize/Utilice/ s/Enter/Intro/ ;) Heh... This is cruel and unusual punishment, I'm in exams (rocket science, literally) and too stressed to be able to use my own native language properly. Nevertheless, if you want to be a purist, you might want to s/Enter/Entrar/ or even s/Enter/Retorno de carro/. ;P -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit [0] - http://www.gnu.org/software/gettext/manual/gettext.html#Preparing-Strings signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Fw: gettext support
El lun, 22-06-2009 a las 21:03 +0200, Jordi Mallach escribió: What is VERY important is that all the \n lines are part of a single msgid. What would be problematic (really!) would be something like: msgid Use the %C and %C keys to select which entry is highlighted. msgid Press enter to boot the selected OS, 'e' to edit the msgid commands before booting or 'c' for a command-line. Actually, the first string could stand on its own just fine, but the second and the third should be joined. Don't forget that the spirit of gettext is translating full sentences when possible, and semantically complete messages when not. Otherwise the translators can get lost with usually ridiculous consequences. By the way, I offer myself for the Spanish translation; msgid Use the %C and %C keys to select which entry is highlighted. msgstr Utilize las teclas %C y %C para seleccionar la entrada resaltada. msgid Press enter to boot the selected OS, 'e' to edit the commands before booting or 'c' for a command-line. msgstr Pulse Enter para iniciar el SO seleccionado, 'e' para editar la lista de comandos antes de arrancar, ó 'c' para abrir un intérprete. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
I think you should move this patch to a new thread, first because most people in the list seem to no longer watch this one, and second because it touches many files apart from drivemap itself. That said, I still don't see the situation in which such an override would be used. Oh, and I would rather see disk-dev-id == GRUB_DISK_DEVICE_BIOSDISK_ID than a strcmp with a magic string literal. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El lun, 01-06-2009 a las 11:53 +0200, Vladimir 'phcoder' Serbinenko escribió: Hmm... from those docs, and accepting that we ignore TSRs, we need to save %ah and %dl at handler entry, then check the saved %ah at exit, like the old handler from GRUB Legacy did - by the way, when writing the new handler, I asked what that code did and noone was able to tell me ¬¬ I've the check before calling original handler. Let's hope it makes code more readable. Personally, I don't think the readability gain is so big to offset the code size increase, but this is just a single opinion. I think the other part of the patch should be committed right now, though. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El sáb, 30-05-2009 a las 17:28 +0200, Vladimir 'phcoder' Serbinenko escribió: I'm fine with the change from const void to const char, but we need to remove a preceding comment about void labels. It's not that I'm opposed to void in principle. Just using the same constructions to do the same things in different files makes code easier to learn and port I wonder if we can go the other way and use void for all labels without storage. Indeed, it's too easy to misuse a char variable by forgetting the ampersand before it. Not char [] What if someone sees the code and thinks it's truly a char array? I insist: you are stirring a tempest in a teapot. Even without taking into account that const void label is more elegant and logical for a simple label than using a char array, the only argument you give is consistency with the rest of GRUB code. This is an important plus for a decision, but consistency should never be an argument in and of itself. Furthermore, a distinct C type for labels like const void, or even better, grub_asm_label_t would help a programmer identify them all with a simple find in files command, while char arrays may abound and be tricky to tell apart. As for the parse_biosdisk() change, I'd like to see an explanation. The explanation is that if user uses ata or usbms code and code calls biosdisk, BIOS may issue a command which may conflict with ata/usbms. Unfortunately it's not a scenario we're able to circumvent (BIOS is headache) so I prefer to err on a safe side I agree that we should avoid touching the hardware. Besides, after loading ata we may not see some drives that BIOS can see. Validation of user input is good, but only if it's implemented correctly. Another approach may be to use biosdisk calls only if biosdisk is active. Otherwise, trust the user and turn off validation. I've checked and seen that ata disables biosdisk. This mean that we need to disable this validation. As for calling biosdisk only if it's active: active isn't well-defined with grub2 and it will add unnecessary complexity Put it that way, I agree with your change. To the scrapper with parse_biosdisk then! I think the new behaviour should be mentioned in the drivemap --help command, though I'm too tangled up with finals to be of any utility right now. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El dom, 31-05-2009 a las 13:36 +0200, Vladimir 'phcoder' Serbinenko escribió: 2009/5/31 Javier Martín lordhab...@gmail.com: El sáb, 30-05-2009 a las 17:28 +0200, Vladimir 'phcoder' Serbinenko escribió: Put it that way, I agree with your change. To the scrapper with parse_biosdisk then! Sorry, I didn't mean to upset you. Here is a patch to do it You did not upset me at all! I'm sorry if I seemed rude or hostile. I truly meant to give you a go-ahead at scrapping that code. Good luck with your exams Thank you very much, I'll need it. Doing some tests I discovered that freedos MBR (but not the bootsector) relies on %dl being preserved across int 0x13. In the patch you can see additional hassle to preserve %dx. 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. Another problem I discovered is that %dl (chainloader) and bootdrive (multiboot) passed to payload is out of sync. Does anyone has a suggestion how to do it cleanly. The problems are: -chainloader/multiboot shouldn't depend on biosdisk if possible -%dl should be set accordingly to mapping -%dl should be correct even if grub2 uses its own drivers (now it's set to nonsense) I have some ideas on this subject but would like to hear what others think. I managed to get the right %dl passed with the following procedure: let's say I want to boot FreeDOS on hd1, but I will drivemap it so that it will become hd0. Instead of doing this: root=(hd1,1) drivemap -s (hd0) (hd1) chainloader +1 I do the following: root=(hd0) drivemap -s (hd0) (hd1) chainloader (hd1,1)+1 This gets the right number into %dl (I have not checked multiboot), but I acknowledge that it's nothing more than a hack. I cannot see a way for drivemap to access that data and modify it according to the mappings without breaking modularity with multiboot and chainloader (and what about other loaders?). Regarding grub2 using its own drivers, we have no reliable way of conclusively saying that (ata2)==(hd1), so we'd better say nothing at all and keep dl=ffh as we do currently. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El dom, 31-05-2009 a las 18:00 +0200, Vladimir 'phcoder' Serbinenko escribió: 2009/5/31 Javier Martín lordhab...@gmail.com: El dom, 31-05-2009 a las 13:36 +0200, Vladimir 'phcoder' Serbinenko escribió: Doing some tests I discovered that freedos MBR (but not the bootsector) relies on %dl being preserved across int 0x13. In the patch you can see additional hassle to preserve %dx. 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 According to wikipedia ah=8 is the only function which returns in %dl. I will write an updated version which restores %dx only if ah!=8 Do not waste your time doing so. There are many other functions that return data in DX, some of them specific to certain systems or TSRs - check Ralf Brown's Interrupt List [0] for a possibly very incomplete reference. The fuss about DS, BS, CX and DX being preserved is quite false, as even Wikipedia shows at [[INT 13]]: just check ah=41h, which returns in both BX and CX. The only semi-rational action we can take is to reverse the mapping _only_ if %dl still contains the mapped data we passed into the int13 handler, but that leaves the question - how do we know that some 0x81 is actually the 0x81 we put there in substitution of the 0x80 the payload called with, and not the true result of the call? I managed to get the right %dl passed with the following procedure: let's say I want to boot FreeDOS on hd1, but I will drivemap it so that it will become hd0. Instead of doing this: root=(hd1,1) drivemap -s (hd0) (hd1) chainloader +1 I do the following: root=(hd0) drivemap -s (hd0) (hd1) chainloader (hd1,1)+1 This gets the right number into %dl (I have not checked multiboot), but I acknowledge that it's nothing more than a hack. And it sets es:di (pointer to partition table entry) to incorrect value I have yet to find a bootsector that uses such information. IIrc, there were so many buggy BIOSes out there that it was unreliable anyways: MBRs just check their own memory and bootsectors forget about it entirely. Nevertheless, this issue deserves a deeper look. I cannot see a way for drivemap to access that data and modify it according to the mappings without breaking modularity with multiboot and chainloader I think of something like having a function pointer in kernel int (*grub_get_biosdisk) (grub_device_t dev) = grub_get_biosdisk_basic; static int grub_get_biosdisk_basic (grub_device_t dev) { if (! dev) return 0xff; if (! dev-disk) return 0xff; if (! dev-disk-dev) return 0xff; if (dev-disk-dev-id != GRUB_DISK_DEVICE_BIOSDISK_ID) return 0xff; return (int) dev-disk-id; } Which can be easily overridden by drivemap But I don't really like this approach Neither do I, but it might be the only non-100%-hackish approach. I think this part should be addressed by a wider audience (coughs at the passive listeners in the list). Regarding grub2 using its own drivers, we have no reliable way of conclusively saying that (ata2)==(hd1), so we'd better say nothing at all and keep dl=ffh as we do currently. I thought of introducing a variable biosdrive and doing something like root=ata1 biosdrive=0x81 chainloader +1 boot Setting %dl to 0xff if drive can't be determined reliably and isn't supplied by user is a good possibility. I'll do so. Er... AfaIk that's what chainloader and multiboot do when they cannot determine the boot disk. Regarding that biosdrive, it would add more complexity to what already is a non-simple system. I'd say that either this mapping gets done automagically or the current hack is better, since it is so obviously a hack that if we cannot solve it future developers will still have an itch to get rid of it. [0] http://www.ctyme.com/intr/int-13.htm -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El dom, 31-05-2009 a las 20:05 +0200, Vladimir 'phcoder' Serbinenko escribió: The only semi-rational action we can take is to reverse the mapping _only_ if %dl still contains the mapped data we passed into the int13 handler, but that leaves the question - how do we know that some 0x81 is actually the 0x81 we put there in substitution of the 0x80 the payload called with, and not the true result of the call? Or we can maintain a list of interrupts under which we restore %dx. It would contain only well documented BIOS interrupts and not TSR ones. This approach will work with normal interrupts and has high chance of working with TSR. But I don't think TSR compatibility is important. I have yet to find a bootsector that uses such information. AFAIR CHS FAT32 FreeDOS sector does No, it doesn't: check it out. It overwrites es and di quite soon indeed: real_start: cld cli sub ax, ax mov ds, ax mov bp, 0x7c00 mov ax, 0x1FE0 mov es, ax mov si, bp mov di, bp Same applies to the other two FreeDOS bootsectors. Regarding that biosdrive, it would add more complexity to what already is a non-simple system. I'd say that either this mapping gets done automagically biosdrive is necessary only when authomatic routines fail (no or wrong result) The point is writing the automatic routine you described so that biosdrive is not required. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El dom, 31-05-2009 a las 21:35 +0200, Christian Franke escribió: 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 Hmm... from those docs, and accepting that we ignore TSRs, we need to save %ah and %dl at handler entry, then check the saved %ah at exit, like the old handler from GRUB Legacy did - by the way, when writing the new handler, I asked what that code did and noone was able to tell me ¬¬ The only functions in the standard that return in %dl are 08h and 15h, so the check should be simple. If we want to be even more extensible, we could have a 32-byte bitmap, one bit per %ah function, and restore %dl depending on the value of the particular bit. However, I think that would be going too far. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: can't compile r220 because of setjmp.S
El sáb, 16-05-2009 a las 19:22 +0200, Felix Zielcke escribió: Am Samstag, den 16.05.2009, 18:52 +0200 schrieb Felix Zielcke: As said on IRC, it seems like the -m32 was missing there, even though it should be set through the COMMON_CSFLAGS in i386-pc.rmk. But that did the trick in i386.rmk: -setjmp_mod_CFLAGS = $(COMMON_CFLAGS) +setjmp_mod_ASFLAGS = $(COMMON_ASFLAGS) Commited this change as Acked-by Vladimir on IRC. You only changed it in i386. Given that a .S file is always compiled by the assembler, shouldn't the change be applied to all other platforms too? -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El jue, 14-05-2009 a las 20:38 +0200, Vladimir 'phcoder' Serbinenko escribió: I'm fine with the change from const void to const char, but we need to remove a preceding comment about void labels. It's not that I'm opposed to void in principle. Just using the same constructions to do the same things in different files makes code easier to learn and port I wonder if we can go the other way and use void for all labels without storage. Indeed, it's too easy to misuse a char variable by forgetting the ampersand before it. If you define it as char [] (as done everywhere in grub2), not char, you need no ampersand. Indeed, but I think you are creating a tempest in a teapot. Your proposal only saves the ampersand in the code, while using void (or better, some grub_asmlabel_t typedef'ed to void with a comment explaining why) requires the ampersand - which is not illogic, since you are taking the address of the label - but protects against accidental modifications or the surprise factor of why is mymod_interrupthandler a char array?. PS for the audience: Sorry for being away from the list the days the patch was finally committed. I'm a bit tied up right now with finals preparation. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El dom, 10-05-2009 a las 13:47 +0200, Vladimir 'phcoder' Serbinenko escribió: I guess you were trying to say that the casts are _not_ needed if I declared it as such. What I'm trying to say is that in its only _direct_ use (in memcpy), its const void signature is perfectly fine for the callee. All other uses are hidden within a nasty macro INT13_OFFSET, which would still be used with your proposed change. Thus, nothing is gained, but a way to modify the int13h handler code on memory opens, without requiring a cast. Thus, I think such a change would be for the worse. I see your reasoning. I personally prefer char[] and considering grub2 works with different compilers I prefer to avoid constructs which might be absent in some C dialects. I think we need a third opinion on this According to the last C standard, section 6.2.5.19, The void type comprises an empty set of values; it is an incomplete type that cannot be completed. Thus, declaring a variable as extern void is perfectly legal C, just like this program is perfectly legal: extern struct s b; int main(int argc, char** argv) { return 0; } Notice that the declaration of struct s is absent, and so it is an incomplete type, but still the declaration of a variable as extern struct s is legal (using b, however, would not, since the compiler does not know the fields of struct s). Thus, declaring an extern variable as the incomplete type void is legal too, and the only operation that can be performed on it is taking its address with . We can try to get third opinions if you want, though. AfaIr, most operating systems only implement very basic support for BIOS disks (because they switch to their own drivers as soon as possible). Many, among them Linux iIrc, just probe them sequentially and stop when they find the first non-existant drive, while others probe hd0 thru hd7 and ignore the rest of them. In the first kind of OSes, mapping hd0=hd6 when you have no hd6 will make hd1 disappear along with hd0, which may be very confusing to the user. Also, making a BIOS disk disappear will not banish it from being detected by the OS normal drivers, so the utility of this is pretty much restricted to DOS-like systems. Of course it's not much additional benefit however I don't see any real reason not to let the user do it For me, the possibility of creating an inconsistent situation in which hd0 is erased through drivemap and hd1 disappears along with it is enough. However, I think we could settle on making it a switch of the drivemap command (like --force or SLT). Nevertheless, this would change the meaning of the drivemap arguments from the BIOS drive that is represented in GRUB by hdN to the Nth BIOS hard drive, which might be confusing. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El sáb, 09-05-2009 a las 16:04 +0200, Vladimir 'phcoder' Serbinenko escribió: Hello 2009/5/9 Javier Martín lordhab...@gmail.com El sáb, 09-05-2009 a las 11:17 +0200, Vladimir 'phcoder' Serbinenko escribió: +/* Realmode far ptr (2 * 16b) to the previous INT13h handler. */ +extern grub_uint32_t grub_drivemap_int13_oldhandler; I prefer it to be 2 variables because of the way they interract so nobody would do something like (char *) grub_drivemap_int13_oldhandler; Two variables? I don't understand. You want the CS and IP parts of the far pointer separated? Yes. Especially that in .S you declared it as two words. But it's not an absolute must I don't see the advantage, particularly taking into account that the current code is very happy to treat it as a black box that is taken from the IVT and not touch it. However, if you insist, I'll change it. Why would anyone try to use it like you said? The comment explicits that it is a realmode pointer, and as such it is not usable as a linear pmode pointer. +/* The type void is used for imported assembly labels, takes no storage and + serves just to take the address with label. Do NOT put void* here. */ +/* Start of the handler bundle. */ +extern void grub_drivemap_int13_handler_base; The common practice is to use declarations like extern char grub_drivemap_int13_handler_base[]; it makes pointer arithmetics easy That variable is used only twice in the code: one in a call to memcpy and another one inside the macro INT13_OFFSET. It would still be so even if it were changed to a char array, with the added risk that a char array can be modified with almost nothing to gain (as casts would still be needed). The casts are needed if you declare it as char foo[]; If someone modifies an array he does it on purpose I guess you were trying to say that the casts are _not_ needed if I declared it as such. What I'm trying to say is that in its only _direct_ use (in memcpy), its const void signature is perfectly fine for the callee. All other uses are hidden within a nasty macro INT13_OFFSET, which would still be used with your proposed change. Thus, nothing is gained, but a way to modify the int13h handler code on memory opens, without requiring a cast. Thus, I think such a change would be for the worse. It's not necessary to try to open the disk actually because for some reason biosdisk module may be not loaded (e.g. grub may use ata) and loading it may cause nasty effects (e.g. conflict between ata and biosdisk) Same applies for revparse_biosdisk. And if user wants to map to unexistant disk it's ok. So just use tryparse_diskstring Hmm... this is a profound design issue: you are arguing that drivemap should not care about actual BIOS disks, but just their drive numbers, and let you map 0x86 to 0x80 even if you have no (hd6). I do not agree, since the main use for GRUB is a bootloader, not a test platform (which would be the only reason to perform that kind of mappings which are sure to cause havoc). Thus, by default drivemap only lets you perform mappings that will work - any stress tester is free to modify the code. Regarding opening the disk, the biosdisk interface does not assure in any way (and again there is almost no API documentation) that hd0 will be 0x80, though it's the sensible thing to expect. However, expecting sensible things from an unstable API prone to redesigns (like the command API, for example) Mapping something to non-existant drive makes the drive non-existant. I think it's fair It's almost guaranteed that numbering of disks won't change AfaIr, most operating systems only implement very basic support for BIOS disks (because they switch to their own drivers as soon as possible). Many, among them Linux iIrc, just probe them sequentially and stop when they find the first non-existant drive, while others probe hd0 thru hd7 and ignore the rest of them. In the first kind of OSes, mapping hd0=hd6 when you have no hd6 will make hd1 disappear along with hd0, which may be very confusing to the user. Also, making a BIOS disk disappear will not banish it from being detected by the OS normal drivers, so the utility of this is pretty much restricted to DOS-like systems. usually leads to nasty surprises later on, so unless 1
Re: Grub Scripting
El vie, 08-05-2009 a las 13:16 +0200, Vladimir 'phcoder' Serbinenko escribió: On Fri, May 8, 2009 at 9:48 AM, Chris Umphress umphr...@gmail.com wrote: Hey, We have some computers that need a boot loader to self-destruct at the end of a school year. In order to accomplish this we would like to use Grub scripting. From what I have been able to find, the support is incomplete and we would have to write the code to self-destruct somehow. Why do you need this? Remember that destroying bootloader leaves all the data intact. Furthermore it looks like that your goal is to restrict a legitimate freedom. I can't be sure of it since I don't know more details but don't expect us to help you to restrict freedom. It does not only leave all the data intact, but also most computers nowadays allow selecting the boot device with a key press on boot, plus booting from USB drives and/or memory cards. Put those things together and your scheme becomes useless. I think it's way simpler to just lock the computer room at the end of the school year. The opposite extreme would be to implement an actual hard drive nuking at the end of the school year (with DBAN for example, www.dban.org), which would not be that much of a problem if your computers are similar enough to keep just a handful of disk images handy to restore them to life next September. However, implementing a self-destruct scheme would, even if possible, be risky - just suppose that the BIOS date gets somehow changed: bang! you're dead. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
Here is a new version of the patch. As you suggested, grub_symbol_t was replaced with void. Also, drivemap.h no longer exists, its little content integrated into drivemap.c. Last but not least, I've mostly adopted your version of the assembly file (indenting, labels, etc.) -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit Index: conf/i386-pc.rmk === --- conf/i386-pc.rmk (revision 2195) +++ conf/i386-pc.rmk (working copy) @@ -185,8 +185,15 @@ aout.mod bsd.mod pxe.mod pxecmd.mod datetime.mod date.mod \ datehook.mod lsmmap.mod ata_pthru.mod hdparm.mod \ usb.mod uhci.mod ohci.mod usbtest.mod usbms.mod usb_keyboard.mod \ - efiemu.mod mmap.mod acpi.mod + efiemu.mod mmap.mod acpi.mod drivemap.mod +# For drivemap.mod. +drivemap_mod_SOURCES = commands/i386/pc/drivemap.c \ + commands/i386/pc/drivemap_int13h.S +drivemap_mod_ASFLAGS = $(COMMON_ASFLAGS) +drivemap_mod_CFLAGS = $(COMMON_CFLAGS) +drivemap_mod_LDFLAGS = $(COMMON_LDFLAGS) + # For efiemu.mod. efiemu_mod_SOURCES = efiemu/main.c efiemu/i386/loadcore32.c \ efiemu/i386/loadcore64.c efiemu/i386/pc/cfgtables.c \ Index: commands/i386/pc/drivemap.c === --- commands/i386/pc/drivemap.c (revision 0) +++ commands/i386/pc/drivemap.c (revision 0) @@ -0,0 +1,475 @@ +/* drivemap.c - command to manage the BIOS drive mappings. */ +/* + * GRUB -- GRand Unified Bootloader + * Copyright (C) 2008, 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/. + */ + +#include grub/extcmd.h +#include grub/dl.h +#include grub/mm.h +#include grub/misc.h +#include grub/disk.h +#include grub/machine/biosdisk.h +#include grub/loader.h +#include grub/machine/memory.h + + +#define MODNAME drivemap + +static grub_extcmd_t cmd_reghandle; + +/* Remember to update enum opt_idxs accordingly. */ +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}, + {swap, 's', 0, perform both direct and reverse mappings, 0, 0}, + {0, 0, 0, 0, 0, 0} +}; + +/* Remember to update options[] accordingly. */ +enum opt_idxs +{ + OPTIDX_LIST = 0, + OPTIDX_RESET, + OPTIDX_SWAP, +}; + +/* Realmode far ptr (2 * 16b) to the previous INT13h handler. */ +extern grub_uint32_t grub_drivemap_int13_oldhandler; + +/* The type void is used for imported assembly labels, takes no storage and + serves just to take the address with label. Do NOT put void* here. */ +/* Start of the handler bundle. */ +extern void grub_drivemap_int13_handler_base; +/* Start of the drive mappings area (space reserved at runtime). */ +extern void grub_drivemap_int13_mapstart; +/* The assembly function to replace the old INT13h handler. It should not be + called because it does not follow any C callspecs and returns with IRET. */ +extern void grub_drivemap_int13_handler; + +typedef struct drivemap_node +{ + grub_uint8_t newdrive; + grub_uint8_t redirto; + struct drivemap_node *next; +} drivemap_node_t; + +static drivemap_node_t *map_head; +static void *insthandler_hook; +static int handlermem_hnd; +static grub_err_t install_int13_handler (int noret __attribute__ ((unused))); +static grub_err_t uninstall_int13_handler (void); + +/* Puts the specified mapping into the table, replacing an existing mapping + for newdrive or adding a new one if required. */ +static grub_err_t +drivemap_set (grub_uint8_t newdrive, grub_uint8_t redirto) +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = map_head; + while (search) +{ + if (search-newdrive == newdrive) + { + mapping = search; + break; + } + search = search-next; +} + + + /* Check for pre-existing mappings to modify before creating a new one. */ + if (mapping) +mapping-redirto = redirto; + else +{ + mapping = grub_malloc (sizeof (drivemap_node_t)); + if (!mapping) + return grub_error (GRUB_ERR_OUT_OF_MEMORY, + cannot allocate map entry, not enough memory); + mapping-newdrive = newdrive; + mapping-redirto = redirto; + mapping-next = map_head; + map_head = mapping; +} + return GRUB_ERR_NONE; +} + +/* Removes the mapping for newdrive from the table. If there is no mapping, + then this function behaves like
Re: status grub2 port of grub-legasy map command
El dom, 03-05-2009 a las 16:59 -0400, Pavel Roskin escribió: On Sun, 2009-05-03 at 02:02 +0200, Javier Martín wrote: I am glad to inform that, with the new version of the mmap patch, drivemap now boots all my hd1 installs of: - Windows XP (Pro x64) - ReactOS - FreeDOS I confirm that 32-bit Windows XP is working too. I would suggest, however, that the return type of grub_mmap_malign_and_register be changed to void* from char*, just like the return type from malloc, because it's the meaningful data type to indicate a pointer to generic memory _and_ it automatically casts to any pointer type the caller uses (which is the reason it's used in malloc). Done. A few comments regarding the code. The patch adds many trailing spaces. I suggest that you run GNU indent on drivemap.c. It will take care of most of the trailing spaces. Comments will still need to be fixed. Assembler files use different formatting in GRUB. Also, it's better to use meaningful names for the labels. Label 4 is unused. Where can I find those assembly formatting conventions? From what I see in your version of the patch, the gist is that asm instructions start at the 8th column (not after a \t). This carries an unpleasant FORTRAN-esque smell that I would rather avoid. Some comments are excessive or unnecessary. These functions defined in this file may be called from C - irrelevant, we have no such functions. Complaints that the processor is not in 64-bit mode are also useless. I don't understand what bundle means in the comments. Sorry, I copied the initial comment from another .S file in GRUB2 as a kind-of-boilerplate. In this context, bundle means the whole int13 handler package that is deployed to the reserved memory address, consisting of the old IVT pointer, the actual int13h handler code and the entry map. Why do we have duplication between grub_drivemap_int13_mapstart and grub_drivemap_int13_size? It was initially added so that install_int13_handler would make no assumptions about the structure of the bundle it was copying. However, given that it already assumes that the map space is the last thing in the bundle, your change is fine. What is (void) mod;? It doesn't prevent any warnings for me. Once again, boilerplate code copied from hello.c long ago. I suspect this stopped being required when the command framework was revamped. grub_symbol_t is already used in kern/dl.c with a different meaning. Why not just use void? The reason to avoid using a plain void is that it might be a slightly confusing sight, so a future coder might have an idea-of-the-moment and change it to a meaningful type like void*. The presence of an explicit type with a big comment is supposed to at least make people think twice before changing it. Please use void in the argument list if the function takes no arguments, as in uninstall_int13_handler(). two arguments required may be misleading. In some cases, only one argument is required, such as -l. Let's make drivemap without arguments show the map. Good idea. Improved patch is attached. Thanks. I will read it thoroughly tomorrow when I'm back from uni. I think that drivemap.h could be scrapped, its contents incorporated into drivemap.c so as to reduce clutter and bitrot potential, and would reduce the impact of the declaration of grub_symbol_t even more. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
I am glad to inform that, with the new version of the mmap patch, drivemap now boots all my hd1 installs of: - Windows XP (Pro x64) - ReactOS - FreeDOS I would suggest, however, that the return type of grub_mmap_malign_and_register be changed to void* from char*, just like the return type from malloc, because it's the meaningful data type to indicate a pointer to generic memory _and_ it automatically casts to any pointer type the caller uses (which is the reason it's used in malloc). I've also added an undo function for install_int13_handler, as partially required by the preboot hook interface, but I really have no idea how to test it, so... is this implementation sensible? It just restores the old int13 handler and frees the allocated memory. -- -- Lazy, Oblivious, Recurrent Disaster -- Habbit Index: commands/i386/pc/drivemap.c === --- commands/i386/pc/drivemap.c (revisión: 0) +++ commands/i386/pc/drivemap.c (revisión: 0) @@ -0,0 +1,454 @@ +/* drivemap.c - command to manage the BIOS drive mappings. */ +/* + * GRUB -- GRand Unified Bootloader + * Copyright (C) 2008, 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/. + */ + +#include grub/machine/drivemap.h +#include grub/extcmd.h +#include grub/dl.h +#include grub/mm.h +#include grub/misc.h +#include grub/disk.h +#include grub/machine/biosdisk.h +#include grub/loader.h +#include grub/machine/memory.h + + +#define MODNAME drivemap + +static grub_extcmd_t cmd_reghandle; + +/* Remember to update enum opt_idxs accordingly. */ +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}, + {swap, 's', 0, perform both direct and reverse mappings, 0, 0}, + {0, 0, 0, 0, 0, 0} +}; + +/* Remember to update options[] accordingly. */ +enum opt_idxs { + OPTIDX_LIST = 0, + OPTIDX_RESET, + OPTIDX_SWAP, +}; + +typedef struct drivemap_node +{ + grub_uint8_t newdrive; + grub_uint8_t redirto; + struct drivemap_node *next; +} drivemap_node_t; + +static drivemap_node_t *map_head; +static void* insthandler_hook; +static int handlermem_hnd; +static grub_err_t install_int13_handler (int noret __attribute__((unused))); +static grub_err_t uninstall_int13_handler (void); + +/* Puts the specified mapping into the table, replacing an existing mapping + for newdrive or adding a new one if required. */ +static grub_err_t +drivemap_set (grub_uint8_t newdrive, grub_uint8_t redirto) +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = map_head; + while (search) +{ + if (search-newdrive == newdrive) +{ + mapping = search; + break; +} + search = search-next; +} + + + /* Check for pre-existing mappings to modify before creating a new one. */ + if (mapping) +mapping-redirto = redirto; + else +{ + mapping = grub_malloc (sizeof (drivemap_node_t)); + if (!mapping) +return grub_error (GRUB_ERR_OUT_OF_MEMORY, + cannot allocate map entry, not enough memory); + mapping-newdrive = newdrive; + mapping-redirto = redirto; + mapping-next = map_head; + map_head = mapping; +} + return GRUB_ERR_NONE; +} + +/* Removes the mapping for newdrive from the table. If there is no mapping, + then this function behaves like a no-op on the map. */ +static void +drivemap_remove (grub_uint8_t newdrive) +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = map_head; + drivemap_node_t *previous = 0; + + while (search) +{ + if (search-newdrive == newdrive) +{ + mapping = search; + break; +} + previous = search; + search = search-next; +} + + if (mapping) +{ + if (previous) +previous-next = mapping-next; + else /* Entry was head of list. */ +map_head = mapping-next; + grub_free (mapping); +} +} + +/* Given a device name, resolves its BIOS disk number and stores it in the + passed location, which should only be trusted if ERR_NONE is returned. */ +static grub_err_t +parse_biosdisk (const char *name, grub_uint8_t *disknum) +{ + grub_disk_t disk; + if (!name || *name == 0) +return grub_error (GRUB_ERR_BAD_ARGUMENT, device name empty); + /* Skip
Re: status grub2 port of grub-legasy map command
Hello there. I am the original author of the drivemap command. I was told by a friend that it was being discussed again on the GRUB list, so I came back and examined the two new enabling functionalities, preboot hooks and mmap. I've adapted drivemap to use them both, and I'm polishing some rough edges. I might send a new version of the patch this weekend. Should people, however, prefer John's version, I'd be glad to assist. El mié, 15-04-2009 a las 11:34 +0200, phcoder escribió: Yes it is. Also it's better to use grub_mmap_iterate instead of basing the location on 0x413 value. How to do it look at mmap/i386/pc/mmap.c The preboot hooks patch works beautifully. However, mmap causes some sort of glitch that makes FreeDOS (one of my three drivemap tests, together with ReactOS and XP) triple-fault. I've been trying to debug it with GDB-over-QEMU, but it is a real PITA. I haven't found any sort of bug in the mmap code, so I'm quite dumbfounded right now. Nevertheless, I'm proceeding to the XP test, which would likely be the most common use case for drivemap. -- -- Lazy, Oblivious, Rational Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ 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
El vie, 17-04-2009 a las 20:01 -0400, John Stanley escribió: Ok, that sounds good... I feel better about it then. I didn't mean directly modifying bios data, but through doc'd bios ints of course. It been a long, long time since I've looked at (or dealt with) bios code, so I was was simply wondering if boot device naming could be modified in a documented way nowadays-- after all, the functionality has been available in the bios setup in pcs for at least 10 years or so. Indeed. However, this is not necessary as most modern OSes only use the BIOS routines for the initial startup, switching sooner than later to their own device drivers. The problem with some of them, among them MS Windows, is that the loader stage fails to either notice the boot drive number info passed in %dl by the BIOS (or, in this case, by GRUB) and/or propagate it to latter stages of their multi-stage pre-boot process. As an example, FreeDOS ignores the parameter and fails to locate KERNEL.SYS, while ReactOS's freeldr uses it and is able to load its config file, but then fails to boot the (drive-#-hardcoded) paths in that same file. This sorry state of things motivates most BIOSes to actually swap your designated boot device with the actual first HD, so that systems that assume they were installed on hd0 will work. Drivemap basically replicates this functionality using the canonical real-mode interrupt handler chaining pattern: it intercepts calls to the BIOS drive access interrupt and calls the previous handler after performing its role. This is not a dirty hack (well, it _is_ by current standards), and in fact it was the way TSRs worked in DOS. Here's what happens when the Windows loader (or FreeDOS, etc.) tries to access the drives: ntldr --(int13h: r,0x80)-- drivemap_int13h --(lcall: f,r,0x81)--BIOS Drivemap (and other TSRs) actually _simulate_ an interrupt call by pushing the flags on the stack, then doing a far call to the address of the handler they replaced (the address that occupied the related slot in the IVT when they were installed). This was, for example, how resident anti-virus programs were implemented in DOS, and I suppose that other programs like DoubleSpace and SmartDrive used it too. For modern OSes, once the initial stage has loaded the kernel and the relevant modules, it usually fires up its own disk drivers that reflect the true (as defined by the disk controller) order of the HDs, so neither the BIOS nor drivemap affect that. Moreover, many OSes currently select their root device based on its UUID, label, etc, so there is no problem switching the BIOS routines because they won't be used again after the initial loader is finished. The exception is, of course, FreeDOS, where the drives stay swapped for the whole session because it has no disk drivers other than those provided by BIOS. I hope this has reassured you that neither mmap nor drivemap are doing any dark things nor 1980s DOS/BIOS black magic. The only BIOS data table modified (by mmap) should be the available free memory at the BDA, which is a completely documented procedure and quite about the only way of telling a DOS do not use this memory. Uhh... btw, I think I found the problem with mmap and FreeDOS: both the mmap int handler and the drivemap int handler reserve their memory as high as possible under 1M. Is FreeDOS aware of this? I mean, we can tell it not to touch memory under 640K through the BDA and INT12, but how do we tell it not to touch the 640K-1M area? I will check if this is the cause of the disaster... tomorrow - it's 4AM and I'm sleepy. Goodbye! -- -- Lazy, Oblivious, Rational Disaster -- Habbit signature.asc Description: Esto es una parte de mensaje firmado digitalmente ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [PATCH 1/7]: Fix sparc64 setjmp implementation.
El mié, 04-03-2009 a las 03:36 -0800, David Miller escribió: From: Vesa Jääskeläinen ch...@nic.fi Date: Wed, 04 Mar 2009 13:34:46 +0200 David Miller wrote: diff --git a/include/grub/sparc64/setjmp.h b/include/grub/sparc64/setjmp.h index 12d8e01..183a820 100644 --- a/include/grub/sparc64/setjmp.h +++ b/include/grub/sparc64/setjmp.h @@ -19,8 +19,7 @@ #ifndef GRUB_SETJMP_CPU_HEADER #define GRUB_SETJMP_CPU_HEADER 1 -/* FIXME (sparc64). */ -typedef unsigned long grub_jmp_buf[20]; +typedef unsigned long grub_jmp_buf[3]; I assume unsigned long is 64bit in sparc? Yes, for sparc64 long is always 64-bit, no exceptions. Would it be more practical to use grub_uint64_t? This is a sparc64 file, so I don't think so. Next month, or maybe next year, someone will need to fix some bug on sparc64 code without being sparc64-savvy, maybe because (like it's already happened) there are no sparc people available. That day people like Bean, Felix, Marco or Robert (to name just a few of the big gurus here) may have to decide between editing a sparc64 file or give up on maintaining the platform. That day it would be really good for them to visually know that unsigned long is 64-bit in sparc, instead of having to go see the docs. This is the real utility of typedefs (and the [u]intNN_t types from C99): if you statically _know_ that some variable is going to be 64 bits long because the _platform_ requires it, why not make it explicit? It will be easier for readers and maintainers. As an example of what this entails, the failure to think with the future in mind has extraordinarily hampered the 32-64 bit transition: many Windows apps (among them the Vorbis codecs!) had be nearly rewritten because they used unsigned long interchangeably with pointer types. I still wonder if it would have been so difficult to use void*, and the proper types for pointer arithmetic. -- -- Lazy, Oblivious, Rational Disaster -- Habbit 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] r1986 broke FAT detection
El sáb, 21-02-2009 a las 14:13 +0100, Robert Millan escribió: On Tue, Feb 10, 2009 at 12:44:14PM +0100, Javier Martín wrote: You're welcome. I see that nevertheless the 0 != comparisons were substituted for standard C int-to-bool-conversion-based comparisons. Maybe people should know the signature _and_ semantic contract of strncmp, but frequently they don't (I had to look it up in the handbook), and while the code that was committed may look like an obvious error to a wanderer (because, of course, comparison functions should return a semantic-bool, shouldn't they?), the version with the explicit 0 != checks at least looks like it was written like that _on purpose_ (and the actual binary cost should be zero with any sensible compiler), thus making future developers on bug-fixing quests at least scratch their heads before proposing the change to the if (!strncmp) error. So, keeping the coding style consistent is important, but I think a balance with readability is in order. Thus, you are the maintainers and you know what you're doing, but I think it's not worth to keep the coding style so strict as to become confusing. I think you're confusing things. C has no boolean type. I know strcmp gives more info than just a semantic boolean, but in this case it's not interesting to us. My point is that the current code _looks_ confusing: due to the lack of a proper boolean type in C (and no, C99 _Bool does not count either), functions that return semantic-booleans return int's instead, with the C convention of 0-false, other-true. This is a common convention, and there are heaps of functions that work like that. So many, in fact, that those invocations of strncmp are likely to look odd to someone that does not have the semantic contract (and not just the formal C signature) in mind; because under the usual convention the code _seems_ to be checking whether the filesystem is fat12, fat16 _and_ fat32 at once. However, the return value of strncmp is actually a semantic integer, and thus the Right Thing (TM, and sorry if I sound preaching) is to perform the very exact comparison we want, that is strncmp() == 0. As I already said, the explicit integer comparison would, given its relative rarity in normal C code, at least make people scratch their heads before thinking hey, this comparison should use 'or's instead of 'and's: in fact, I think that's the advantage of my version - it keeps the original x or y or z logical structure of the comparison. Given that the cost in binary size would most likely be zero for any average compiler, I think the change is worth performing. -- -- Lazy, Oblivious, Rational Disaster -- Habbit 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: SHA-1 MBR
El vie, 20-02-2009 a las 20:02 -0500, Isaac Dupree escribió: Jan Alsenz wrote: Yes, that was my point. You need a trusted first step. But the only thing besides a TPM, that can be used for this is the BIOS, which can be flashed. And even, if we assume, that we can construct a BIOS that only boots if the MBR hash matches and can not be flashed prior to this point, there are still two points missing: - After the system has started, the BIOS could be flashed. This is a very possible scenario in a multi user environment. - They could take out the disk and put it in another machine, tamper with the boot code and switch it on. And all your protection is gone. Ok, you could try to put a needed key in the BIOS too, but then we're back to problem one - and the BIOS can not check if a request for the key is valid. I'm not even sure, if something in the BIOS can be read protected. BIOS could be in ROM, un-flashable, including hash/keys and all! Refuse to boot if the hash doesn't match! Admittedly this poses some limitations on whether the system can be upgraded, depending how sophisticated you want to be. This paranoid security talk is growing some big pink elephants which are being conveniently ignored: you people are trying to protect a HD within a computer that could be stolen, but you trust that the BIOS chip (in ROM and whatever you want), which performs the systems initialization (including RAM and the TPM) cannot be tampered with or even replaced. When someone pointed the key-in-RAM problem the answer was I'll just glue it with epoxy resin! For crying out loud! Without taking into account that most epoxy resins take weeks to solidify under 100 ºC, if the computer is physically stolen it could be subjected to EM-field analysis. What will be the next madness? Will you wrap every RAM module in tinfoil, a-la Faraday cage? Will you build a plasma shield around them? This is going way out of proportion: with non-interactive key initialization, if the computer is stolen, you are screwed period, because you have a Mallory-on-steroids-holding-Alice-ransom instead of a tame Eve. It does not take a rocket scientist (what I'm studying) nor a cryptographer (one of my hobbies) to notice! -- -- Lazy, Oblivious, Rational Disaster -- Habbit 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] r1986 broke FAT detection
El mar, 10-02-2009 a las 10:50 +0100, Felix Zielcke escribió: Am Dienstag, den 10.02.2009, 01:19 +0100 schrieb Javier Martín: At r1985, sudo ./grub-probe -t fs -d /dev/fd0 outputs fat with a freshly-formatted VFAT floppy in the drive. At r1986, it spits error: unknown filesystem. The cause is this error, repeated three times: if (! grub_strncmp(something, FAT12, 5)) goto fail; Strncmp does not return a boolean result (i.e. matches or doesn't), but an _integer_ that is supposed to establish a comparison order between strings. Thus, a return value of 0 is actually a match. See why I insist on treating semantic-ints different than semantic-bools even though the language does not? The correction is obvious (a patch is attached): Thanks for your patch. Commited. You're welcome. I see that nevertheless the 0 != comparisons were substituted for standard C int-to-bool-conversion-based comparisons. Maybe people should know the signature _and_ semantic contract of strncmp, but frequently they don't (I had to look it up in the handbook), and while the code that was committed may look like an obvious error to a wanderer (because, of course, comparison functions should return a semantic-bool, shouldn't they?), the version with the explicit 0 != checks at least looks like it was written like that _on purpose_ (and the actual binary cost should be zero with any sensible compiler), thus making future developers on bug-fixing quests at least scratch their heads before proposing the change to the if (!strncmp) error. So, keeping the coding style consistent is important, but I think a balance with readability is in order. Thus, you are the maintainers and you know what you're doing, but I think it's not worth to keep the coding style so strict as to become confusing. -- Lazy Oblivious, Rational Disaster -- Habbit 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
[PATCH] r1986 broke FAT detection
At r1985, sudo ./grub-probe -t fs -d /dev/fd0 outputs fat with a freshly-formatted VFAT floppy in the drive. At r1986, it spits error: unknown filesystem. The cause is this error, repeated three times: if (! grub_strncmp(something, FAT12, 5)) goto fail; Strncmp does not return a boolean result (i.e. matches or doesn't), but an _integer_ that is supposed to establish a comparison order between strings. Thus, a return value of 0 is actually a match. See why I insist on treating semantic-ints different than semantic-bools even though the language does not? The correction is obvious (a patch is attached): if (0 != grub_strncmp(something, FAT12,5)) goto fail; And I remark the 0 != instead of a simple if (strncmp()) test. BTW, I think the FATx constants should be made into macros or SLT... Magic constants creep me out. -- Lazy Oblivious, Rational Disaster -- Habbit Index: fs/fat.c === --- fs/fat.c (revision 1987) +++ fs/fat.c (working copy) @@ -187,9 +187,9 @@ if (grub_disk_read (disk, 0, 0, sizeof (bpb), (char *) bpb)) goto fail; - if (! grub_strncmp((const char *) bpb.version_specific.fat12_or_fat16.fstype, FAT12,5) - || ! grub_strncmp((const char *) bpb.version_specific.fat12_or_fat16.fstype, FAT16,5) - || ! grub_strncmp((const char *) bpb.version_specific.fat32.fstype, FAT32,5)) + if (0 != grub_strncmp((const char *) bpb.version_specific.fat12_or_fat16.fstype, FAT12, 5) + 0 != grub_strncmp((const char *) bpb.version_specific.fat12_or_fat16.fstype, FAT16, 5) + 0 != grub_strncmp((const char *) bpb.version_specific.fat32.fstype, FAT32, 5)) goto fail; /* Get the sizes of logical sectors and clusters. */ 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: grub-probe detects ext4 wronly as ext2
El sáb, 07-02-2009 a las 20:30 +0100, Felix Zielcke escribió: Am Mittwoch, den 04.02.2009, 14:08 +0100 schrieb Javier Martín: Well, I am happy to post a diff of the patch against current SVN head (r1973). I have personally confirmed (in a VM) that it: 1) Still builds (and even runs! ^^) 2) Works with existing ext2/3 file systems (I haven't checked ext4 FSs but the extents bit is marked as supported, so it should work) 3) Correctly rejects journal devices, which will then appear as unknown filesystem when accessed. FLEX_BG needs to be added to the list of ignored flags. As Robert already said in his last reply to this thread [0] const char *local_error = 0; Please use NULL. +EXT2_DRIVER_MOUNT_FAIL(0); I share his opinion that this isn't needed. If you fix this and write a changelog then I commit this. [0] http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00645.html Oops... I was going to send a new version of the patch with those fixed, but when doing a svn up so that it would be against HEAD, I've noticed that Robert has just integrated a much cleaner version without the macro and local_error thingies. Well, the only thing left to do is adding flex_bg - here goes the patch. It also clarifies a comment and corrects those added in my original patch and Robert's cleaned-up version that don't end with . */ as they should. -- Lazy, Oblivious, Rational Disaster -- Habbit BTW: Robert, you're having a total mailing spree today! What's it been, 30 posts? Evolution nearly choked, and my spam filter was about to ban you as mass mailing - possible spam ;) Index: fs/ext2.c === --- fs/ext2.c (revision 1977) +++ fs/ext2.c (working copy) @@ -73,7 +73,7 @@ /* Superblock filesystem feature flags (RW compatible) * A filesystem with any of these enabled can be read and written by a driver - * that does not understand them without causing metadata/data corruption */ + * that does not understand them without causing metadata/data corruption. */ #define EXT2_FEATURE_COMPAT_DIR_PREALLOC 0x0001 #define EXT2_FEATURE_COMPAT_IMAGIC_INODES 0x0002 #define EXT3_FEATURE_COMPAT_HAS_JOURNAL 0x0004 @@ -83,7 +83,7 @@ /* Superblock filesystem feature flags (RO compatible) * A filesystem with any of these enabled can be safely read by a driver that * does not understand them, but should not be written to, usually because - * additional metadata is required */ + * additional metadata is required. */ #define EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001 #define EXT2_FEATURE_RO_COMPAT_LARGE_FILE 0x0002 #define EXT2_FEATURE_RO_COMPAT_BTREE_DIR 0x0004 @@ -93,7 +93,7 @@ /* Superblock filesystem feature flags (back-incompatible) * A filesystem with any of these enabled should not be attempted to be read * by a driver that does not understand them, since they usually indicate - * metadata format changes that might confuse the reader. */ + * metadata format changes that might confuse the reader. */ #define EXT2_FEATURE_INCOMPAT_COMPRESSION 0x0001 #define EXT2_FEATURE_INCOMPAT_FILETYPE 0x0002 #define EXT3_FEATURE_INCOMPAT_RECOVER 0x0004 /* Needs recovery */ @@ -104,17 +104,17 @@ #define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200 /* The set of back-incompatible features this driver DOES support. Add (OR) - * flags here as the related features are implemented into the driver */ + * flags here as the related features are implemented into the driver. */ #define EXT2_DRIVER_SUPPORTED_INCOMPAT ( EXT2_FEATURE_INCOMPAT_FILETYPE \ - | EXT4_FEATURE_INCOMPAT_EXTENTS ) + | EXT4_FEATURE_INCOMPAT_EXTENTS \ + | EXT4_FEATURE_INCOMPAT_FLEX_BG ) /* List of rationales for the ignored incompatible features: * needs_recovery: Not really back-incompatible - was added as such to forbid * ext2 drivers from mounting an ext3 volume with a dirty * journal because they will ignore the journal, but the next * ext3 driver to mount the volume will find the journal and * replay it, potentially corrupting the metadata written by - * the ext2 drivers - */ + * the ext2 drivers. Safe to ignore for this RO driver. */ #define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER ) 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: grub-probe detects ext4 wronly as ext2
El mié, 04-02-2009 a las 08:41 +0100, Felix Zielcke escribió: Am Montag, den 11.08.2008, 16:14 +0200 schrieb Javier Martín: Hi there, After reading Felix's reply I've once again found your post and implemented your request, so here is a new version of the patch (version 6). Sorry for missing your message in the first instance... u_u I'd like to bring this up again. Now with that journal_dev bug (See [0] or [1]) it would be really nice to have this commited. [0] http://lists.gnu.org/archive/html/grub-devel/2009-02/msg00018.html [1] http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=502333 Well, I am happy to post a diff of the patch against current SVN head (r1973). I have personally confirmed (in a VM) that it: 1) Still builds (and even runs! ^^) 2) Works with existing ext2/3 file systems (I haven't checked ext4 FSs but the extents bit is marked as supported, so it should work) 3) Correctly rejects journal devices, which will then appear as unknown filesystem when accessed. Index: fs/ext2.c === --- fs/ext2.c (revisión: 1973) +++ fs/ext2.c (copia de trabajo) @@ -71,8 +71,53 @@ ? EXT2_GOOD_OLD_INODE_SIZE \ : grub_le_to_cpu16 (data-sblock.inode_size)) -#define EXT3_FEATURE_COMPAT_HAS_JOURNAL 0x0004 +/* Superblock filesystem feature flags (RW compatible) + * A filesystem with any of these enabled can be read and written by a driver + * that does not understand them without causing metadata/data corruption */ +#define EXT2_FEATURE_COMPAT_DIR_PREALLOC 0x0001 +#define EXT2_FEATURE_COMPAT_IMAGIC_INODES 0x0002 +#define EXT3_FEATURE_COMPAT_HAS_JOURNAL 0x0004 +#define EXT2_FEATURE_COMPAT_EXT_ATTR 0x0008 +#define EXT2_FEATURE_COMPAT_RESIZE_INODE 0x0010 +#define EXT2_FEATURE_COMPAT_DIR_INDEX 0x0020 +/* Superblock filesystem feature flags (RO compatible) + * A filesystem with any of these enabled can be safely read by a driver that + * does not understand them, but should not be written to, usually because + * additional metadata is required */ +#define EXT2_FEATURE_RO_COMPAT_SPARSE_SUPER 0x0001 +#define EXT2_FEATURE_RO_COMPAT_LARGE_FILE 0x0002 +#define EXT2_FEATURE_RO_COMPAT_BTREE_DIR 0x0004 +#define EXT4_FEATURE_RO_COMPAT_GDT_CSUM 0x0010 +#define EXT4_FEATURE_RO_COMPAT_DIR_NLINK 0x0020 +#define EXT4_FEATURE_RO_COMPAT_EXTRA_ISIZE 0x0040 +/* Superblock filesystem feature flags (back-incompatible) + * A filesystem with any of these enabled should not be attempted to be read + * by a driver that does not understand them, since they usually indicate + * metadata format changes that might confuse the reader. */ +#define EXT2_FEATURE_INCOMPAT_COMPRESSION 0x0001 +#define EXT2_FEATURE_INCOMPAT_FILETYPE 0x0002 +#define EXT3_FEATURE_INCOMPAT_RECOVER 0x0004 /* Needs recovery */ +#define EXT3_FEATURE_INCOMPAT_JOURNAL_DEV 0x0008 /* Volume is journal device */ +#define EXT2_FEATURE_INCOMPAT_META_BG 0x0010 +#define EXT4_FEATURE_INCOMPAT_EXTENTS 0x0040 /* Extents used */ +#define EXT4_FEATURE_INCOMPAT_64BIT 0x0080 +#define EXT4_FEATURE_INCOMPAT_FLEX_BG 0x0200 +/* The set of back-incompatible features this driver DOES support. Add (OR) + * flags here as the related features are implemented into the driver */ +#define EXT2_DRIVER_SUPPORTED_INCOMPAT ( EXT2_FEATURE_INCOMPAT_FILETYPE \ + | EXT4_FEATURE_INCOMPAT_EXTENTS ) +/* List of rationales for the ignored incompatible features: + * needs_recovery: Not really back-incompatible - was added as such to forbid + * ext2 drivers from mounting an ext3 volume with a dirty + * journal because they will ignore the journal, but the next + * ext3 driver to mount the volume will find the journal and + * replay it, potentially corrupting the metadata written by + * the ext2 drivers + */ +#define EXT2_DRIVER_IGNORED_INCOMPAT ( EXT3_FEATURE_INCOMPAT_RECOVER ) + + #define EXT3_JOURNAL_MAGIC_NUMBER 0xc03b3998U #define EXT3_JOURNAL_DESCRIPTOR_BLOCK 1 @@ -486,10 +531,12 @@ return 0; } +#define EXT2_DRIVER_MOUNT_FAIL(message) { local_error = (message); goto fail; } static struct grub_ext2_data * grub_ext2_mount (grub_disk_t disk) { struct grub_ext2_data *data; + const char *local_error = 0; data = grub_malloc (sizeof (struct grub_ext2_data)); if (!data) @@ -498,13 +545,19 @@ /* Read the superblock. */ grub_disk_read (disk, 1 * 2, 0, sizeof (struct grub_ext2_sblock), (char *) data-sblock); - if (grub_errno) -goto fail; + if (grub_errno != GRUB_ERR_NONE) +EXT2_DRIVER_MOUNT_FAIL(0); /* Make sure this is an ext2 filesystem. */ if (grub_le_to_cpu16 (data-sblock.magic) != EXT2_MAGIC) -goto fail; +EXT2_DRIVER_MOUNT_FAIL(not an ext2 filesystem); + /* Check the FS doesn't have feature bits enabled that we don't support. */ + if (grub_le_to_cpu32 (data-sblock.feature_incompat
Re: grub-efi x86_64 on macbook air?
El mié, 28-01-2009 a las 17:42 +0100, step21 escribió: Hey, while investigating possibilities on how it might be possible to install/run something wubi like on macs (without having windows installed) I ran into a couple of issues. I post here mainly in hope that one of the folks that actually wrote the code for this or at least have experience with it read this, cause most ppl on irc (while being very helpful) seem to know more about the pure x86/bios/linux side of things. Besides some (minor?) issues like that the command line freezes/becomes unresponsive after a (relatively short) amount of time and the fact that booting back to OS X (which everyone says should work 100%) does not work at all (it complains that it can't find the specified os x *efi, but it is there, and search correctly finds the right drive) my biggest problem right now is the following: I got some kerne/initrd combo to boot (2.6.26 from debian lenny netinstall iirc) but it gets to the language selection screen, first thing you notice, the keyboard is dead. I supplied init=/bin/sh to be able to read at least the last of the error messages. As it turns out to me it seems that seemingly everything pci-related is unavailable. Error messages are as follows: [time after boot]PCI No IRQ known for interrup pin C of device :00:1a.7. Please Try using pci=biosirq [time after boot]ehci_hcd :00:1a.7 Found HC with no IRQ. Check BIOS/PCI :00:1a.7 setup! [time after boot]ehci_hcd :00:1a.7 fail, -19 (of course time/device ids and pins (A-D) changed, but otherwise they were the same I think) I tried supplying pci=biosirq although I didn't think it would work, and it didn't, or some solution for a similiar issue from the debian wiki for the macbook (using a usb keyboard) which didn't work either. The usb keyboard works for refit and grub, but not once booted into linux. On some other wiki page it was suggested to supply noapic acpi=force and maybe irqpoll which I tried in various combinations without a change. The machine I use for testing this is a first generation macbook air. Now while there are numerous reports about people running linux on it (pretty well actually) by providing it with a legacy bios environment, I didn't find a report about one running grub-efi on it successfully to boot linux. The most relevant posts my searches come up with are my own posts on the ubunutu forums. So, I'm not sure who is to blame here, or if it's maybe just my own fault, but I decided that it would be nice to get the opinion/solutions? of some ppl who might be more intimately familiar with the workings of the mac boot firmware etc. and maybe know if this actually has been tested and should work, or not. Thanks for an help in advance. Are you sure that Debian lenny supports booting from EFI? I'm quite literally talking out of my ass here, so I don't have a clue, but it seems that the kernel you're trying to boot is trying to use BIOS functionality. GRUB does _not_ include BIOS emulation, so that will certainly not work. In order for a Linux kernel to boot from EFI, you have to enable the switch in the pre-build kernel configuration. From my past knowledge (on my brother's Macbook), at least Ubuntu/x86 kernels _do_ have the EFI switch enabled, so you might give it a try and check if that's what's going wrong. On the OS X and keyboard issues, I'm afraid I cannot help you. Perhaps our elders would be wiser? PS: you could try, however, to enable debug output when compiling your GRUB so that you could see if there was anything wrong when loading the file, like a hypothetical hfs.mod error (again, talking out of my non-talking orifices) 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: grub-efi x86_64 on macbook air?
El mié, 28-01-2009 a las 18:30 +0100, step21 escribió: On Wed, Jan 28, 2009 at 5:56 PM, Javier Martín lordhab...@gmail.com wrote: El mié, 28-01-2009 a las 17:42 +0100, step21 escribió: Hey, while investigating possibilities on how it might be possible to install/run something wubi like on macs (without having windows installed) I ran into a couple of issues. I post here mainly in hope that one of the folks that actually wrote the code for this or at least have experience with it read this, cause most ppl on irc (while being very helpful) seem to know more about the pure x86/bios/linux side of things. Besides some (minor?) issues like that the command line freezes/becomes unresponsive after a (relatively short) amount of time and the fact that booting back to OS X (which everyone says should work 100%) does not work at all (it complains that it can't find the specified os x *efi, but it is there, and search correctly finds the right drive) my biggest problem right now is the following: I got some kerne/initrd combo to boot (2.6.26 from debian lenny netinstall iirc) but it gets to the language selection screen, first thing you notice, the keyboard is dead. I supplied init=/bin/sh to be able to read at least the last of the error messages. As it turns out to me it seems that seemingly everything pci-related is unavailable. Error messages are as follows: [time after boot]PCI No IRQ known for interrup pin C of device :00:1a.7. Please Try using pci=biosirq [time after boot]ehci_hcd :00:1a.7 Found HC with no IRQ. Check BIOS/PCI :00:1a.7 setup! [time after boot]ehci_hcd :00:1a.7 fail, -19 (of course time/device ids and pins (A-D) changed, but otherwise they were the same I think) I tried supplying pci=biosirq although I didn't think it would work, and it didn't, or some solution for a similiar issue from the debian wiki for the macbook (using a usb keyboard) which didn't work either. The usb keyboard works for refit and grub, but not once booted into linux. On some other wiki page it was suggested to supply noapic acpi=force and maybe irqpoll which I tried in various combinations without a change. The machine I use for testing this is a first generation macbook air. Now while there are numerous reports about people running linux on it (pretty well actually) by providing it with a legacy bios environment, I didn't find a report about one running grub-efi on it successfully to boot linux. The most relevant posts my searches come up with are my own posts on the ubunutu forums. So, I'm not sure who is to blame here, or if it's maybe just my own fault, but I decided that it would be nice to get the opinion/solutions? of some ppl who might be more intimately familiar with the workings of the mac boot firmware etc. and maybe know if this actually has been tested and should work, or not. Thanks for an help in advance. Are you sure that Debian lenny supports booting from EFI? I'm quite literally talking out of my ass here, so I don't have a clue, but it seems that the kernel you're trying to boot is trying to use BIOS functionality. GRUB does _not_ include BIOS emulation, so that will certainly not work. In order for a Linux kernel to boot from EFI, you have to enable the switch in the pre-build kernel configuration. From my past knowledge (on my brother's Macbook), at least Ubuntu/x86 kernels _do_ have the EFI switch enabled, so you might give it a try and check if that's what's going wrong. On the OS X and keyboard issues, I'm afraid I cannot help you. Perhaps our elders would be wiser? PS: you could try, however, to enable debug output when compiling your GRUB so that you could see if there was anything wrong when loading the file, like a hypothetical hfs.mod error (again, talking out of my non-talking orifices) ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel Well, I didn't check specifically, but afaik the kernel has it enabled. I tried with an ubuntu kernel before which didn't even get that far. (just loaded kernel/initrd on commandline, but after boot nothing happened) Yes, I know grub does not supply bios emulation, but maybe the kernel just fails to recongize that it's supposed to use efi? On compiling kernel and grub2: I couldn't get compiling grub2 to work on os x, even with a seperate gnu gcc, then it failed on linking and it seems there is no gnu version of ld available for os x. (people say it just doesn't run) so I kinda gave up. Also, which applies also to kernel compiling, currently this is the only machine that totally belongs to myself. I might drob wubi or something on some windows box nobody really uses, and hope ppl don't hate me too much for that, then I could compile grub2/my own kernel where I know I have everything
Re: [PATCH] remove target_os
El mar, 27-01-2009 a las 18:51 +0200, Vesa Jääskeläinen escribió: Javier Martín wrote: 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. AFAIK, the full power of $build+$host+$target only matters when building _compilers_ (and binutils, etc.), because you might want to use an AMD64/Linux machine to build a compiler that will run on PPC/Darwin but produce executables for a x86/Cygwin machine Now lets move this idea to GRUB 2: There is a build server is running on PPC/Linux and it wants to build all architectures supported by GRUB 2 (provide proper boot code for all arch and tools) and then makes software bundles (rpm,deb, or so) for all architectures. In example if all shell tools are to be ran on x86-64/Linux and then boot code needs to be compatible with x86-32 as there is PC-BIOS present on target system. So you propose --target to be reused like this (a PS3 building GRUB2 for an amd64 BIOS PC with Linux) : ./configure --build=ppc64-linux --host=x86_64-linux-gnu --target=i686-pc ^--compiling system ^--tools system ^--boot arch I think we already have a better-geared switch for that (--with-platform it is?), which is currently used to distinguish between BIOS and EFI boot code on both x86 and x86-64. ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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] remove target_os
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. These thingies are most likely only known to OS developers, but you can build a perfectly good i686-pc-elf GCC (only with no libc, of course). This might be the Right Way (TM) of building boot code, instead of the current way of using the OS-targetter compiler and trying to tell it not to do what it was built to do. 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. 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. Christian ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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] Build system improvement
El dom, 25-01-2009 a las 12:34 +0200, Vesa Jääskeläinen escribió: Javier Martín wrote: This patch modifies several files in the build system (mainly common.rmk and genmk.rb) to reduce the general verbosity of the build process to a manageable, semi-informative level. Thus, what currently appears as gcc calls, several lines long each is turned into lines like: [M xfs.mod] COMPILE ../src/fs/xfs.c - xfs_mod-fs_xfs.o [M xfs.mod] LINK xfs_mod-fs_xfs.o - pre-xfs.o [M xfs.mod] Looking for EXPORTED SYMBOL definitions: pre-xfs.o And so on. The change also makes warning-hunting marginally easier, though not by much since the patch intentionally shows a line for nearly every process that did so previously. This behavior could be simplified further if needed - this post is more of an RFC than anything else. Also, it is by no means thorough or complete - only the most common processes have been addressed - as I'm a bit busy with exams. The patch makes the new behavior the default one, so a new make-time option is added: V (for verbose), which must have the value 1 in order to get the behavior, as in make V=1 First of all I would like compiling process to be more tidier. However, what is the difference between Colin's similar patch? Oops... I did not remember Colin's patch, it's been so long since it was last discussed that my mind considered it deceased for all intents and purposes. I'll look into the post history and see what it did compared to mine - if I get it to compile against a current SVN grub, that is. 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
[PATCH] Build system improvement
This patch modifies several files in the build system (mainly common.rmk and genmk.rb) to reduce the general verbosity of the build process to a manageable, semi-informative level. Thus, what currently appears as gcc calls, several lines long each is turned into lines like: [M xfs.mod] COMPILE ../src/fs/xfs.c - xfs_mod-fs_xfs.o [M xfs.mod] LINK xfs_mod-fs_xfs.o - pre-xfs.o [M xfs.mod] Looking for EXPORTED SYMBOL definitions: pre-xfs.o And so on. The change also makes warning-hunting marginally easier, though not by much since the patch intentionally shows a line for nearly every process that did so previously. This behavior could be simplified further if needed - this post is more of an RFC than anything else. Also, it is by no means thorough or complete - only the most common processes have been addressed - as I'm a bit busy with exams. The patch makes the new behavior the default one, so a new make-time option is added: V (for verbose), which must have the value 1 in order to get the behavior, as in make V=1 Index: Makefile.in === --- Makefile.in (revisión: 1954) +++ Makefile.in (copia de trabajo) @@ -138,18 +138,30 @@ CLEANFILES += $(pkglib_DATA) $(pkgdata_DATA) pkglib_DATA += moddep.lst command.lst fs.lst partmap.lst moddep.lst: $(DEFSYMFILES) $(UNDSYMFILES) genmoddep.awk - cat $(DEFSYMFILES) /dev/null \ +ifneq ($(V),1) + @echo [L moddep.lst] Building MODULE DEPENDENCIES list +endif + $(SILENT)cat $(DEFSYMFILES) /dev/null \ | $(AWK) -f $(srcdir)/genmoddep.awk $(UNDSYMFILES) $@ \ || (rm -f $@; exit 1) command.lst: $(COMMANDFILES) - cat $^ /dev/null | sort $@ +ifneq ($(V),1) + @echo [L command.lst] Building COMMAND list +endif + $(SILENT)cat $^ /dev/null | sort $@ fs.lst: $(FSFILES) - cat $^ /dev/null | sort $@ +ifneq ($(V),1) + @echo [L fs.lst] Building FILESYSTEM list +endif + $(SILENT)cat $^ /dev/null | sort $@ partmap.lst: $(PARTMAPFILES) - cat $^ /dev/null | sort $@ +ifneq ($(V),1) + @echo [L partmap.lst] Building PARTMAP list +endif + $(SILENT)cat $^ /dev/null | sort $@ ifeq (, $(UNIFONT_BDF)) else Index: conf/common.rmk === --- conf/common.rmk (revisión: 1954) +++ conf/common.rmk (copia de trabajo) @@ -1,5 +1,9 @@ # -*- makefile -*- +ifneq ($(V),1) +SILENT = @ +endif + # For grub-mkelfimage. bin_UTILITIES += grub-mkelfimage grub_mkelfimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \ @@ -125,26 +129,27 @@ # For grub-mkconfig grub-mkconfig: util/grub-mkconfig.in config.status - ./config.status --file=$@:$ - chmod +x $@ + $(SILENT)./config.status --file=$@:$ + $(SILENT)chmod +x $@ sbin_SCRIPTS += grub-mkconfig CLEANFILES += grub-mkconfig grub-mkconfig_lib: util/grub-mkconfig_lib.in config.status - ./config.status --file=$@:$ - chmod +x $@ + $(SILENT)./config.status --file=$@:$ + $(SILENT)chmod +x $@ lib_DATA += grub-mkconfig_lib CLEANFILES += grub-mkconfig_lib update-grub_lib: util/update-grub_lib.in config.status - ./config.status --file=$@:$ - chmod +x $@ + $(SILENT)./config.status --file=$@:$ + $(SILENT)chmod +x $@ lib_DATA += update-grub_lib CLEANFILES += update-grub_lib %: util/grub.d/%.in config.status - ./config.status --file=$@:$ - chmod +x $@ +#no need to echo nothing here: config.status already does so + $(SILENT)./config.status --file=$@:$ + $(SILENT)chmod +x $@ grub-mkconfig_SCRIPTS = 00_header 10_linux 10_hurd 10_freebsd 30_os-prober 40_custom ifeq ($(target_os), cygwin) grub-mkconfig_SCRIPTS += 10_windows Index: genmk.rb === --- genmk.rb (revisión: 1954) +++ genmk.rb (copia de trabajo) @@ -57,10 +57,16 @@ MOSTLYCLEANFILES += #{deps_str} #...@name}: #{exe} +ifneq ($(V),1) + @echo [I #...@name}] OBJCOPY $ '-' $@ +endif $(OBJCOPY) -O binary -R .note -R .comment -R .note.gnu.build-id $ $@ #{exe}: #{objs_str} - $(TARGET_CC) -o $@ $^ $(TARGET_LDFLAGS) $(#{prefix}_LDFLAGS) +ifneq ($(V),1) + @echo [I #...@name}] LINK $ '-' $@ +endif + $(SILENT)$(TARGET_CC) -o $@ $^ $(TARGET_LDFLAGS) $(#{prefix}_LDFLAGS) + objs.collect_with_index do |obj, i| src = sources[i] @@ -71,7 +77,10 @@ dir = File.dirname(src) #{obj}: #{src} $(#{src}_DEPENDENCIES) - $(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) #{extra_flags} $(TARGET_#{flag}) $(#{prefix}_#{flag}) -MD -c -o $@ $ +ifneq ($(V),1) + @echo [I #...@name}] COMPILE $ '-' $@ +endif + $(SILENT)$(TARGET_CC) -I#{dir} -I$(srcdir)/#{dir} $(TARGET_CPPFLAGS) #{extra_flags} $(TARGET_#{flag}) $(#{prefix}_#{flag}) -MD -c -o $@ $ -include #{dep} @@ -113,29 +122,47 @@ UNDSYMFILES += #{undsym}
Re: stat for FreeBSD
El dom, 14-12-2008 a las 02:12 +0100, Robert Millan escribió: On Tue, Dec 09, 2008 at 03:26:28PM -0800, Andrey Shuvikov wrote: Hello, I tried to compile Grub2 under FreeBSD, it compiles but doesn't work (grub-setup). The problem seems to be in the stat() call, which is used to determine disk size. The call returns 0 for st_size, and grub-setup complains for Read out of range. Is it a known issue? Is there a way to use Grub2 under FreeBSD? Do you know how to obtain the disk size on FreeBSD ? IF parted works on FreeBSD, we could look at what it does, since it's a GNU project, copyrights assigned to the FSF and all that (isn't it?) 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: localization of Grub
El lun, 29-09-2008 a las 21:48 +0300, Vesa Jääskeläinen escribió: Robert Millan wrote: On Sun, Sep 28, 2008 at 11:49:54PM +0200, Carles Pina i Estany wrote: - gettextise the util tools, so they can be translated as normal programs. ok! (I guess/hope that will be more burocratic work than new work) It _is_ technical work I think, but less fun than the other part :-/ I don't expect to have time until 22th October (aprox.). I have more interest in the second part (it's newer) than first part, but first part has a practical and fast effects with (I think) less investment. The second part also builds on the first, to some extent. I.e. if you want to test gettext support in GRUB itself, you need some strings to translate, and these are provided by the .mo files which are only built if the build system supports that, etc. Before you guy's go too deep in detail I would like to remind special requirements for graphical user interface related to localization. You can't just printf stuff to screen in there. There has to be some changes in logic how information is presented to user. Currently there is lot of printf's in the code to display information and that is not going to be too pretty for graphical menu as we need to display some kind of console on event when there is something to be displayed. Also try to think how different languages differentiate for displaying certain types of information. Here is some simple example. (Bear in mind if there are grammatical errors or typos or so :)) (eng) See Figure 2 in page 14 for more details.' - (fin) Sivulla 14 olevassa kuvassa 2 on enemmän tietoa asiasta.' Please notice difference in order of arguments in the languages. Also there are some weird scripts that change order of characters. In example for some right-to-left scripts seem to have this feature. On example that I think belongs to this group is hebrew where they normally write from right-to-left, but for English (or foreign) texts are still visually correct. In example: nativeThis is native 1/nativeenglish2and this is English3/englishnative4, so weird./native Would be something like: .rdiew os ,42and this is English31 evitan si sihT So this subject really has more details than meets the eye in first sight :) Well, no one proposed a full implementation of the Unicode bi-di algorithm, and I think such a complex feat goes way beyond the development power behind GRUB right now. However, gettext does allow for the changes in the argument order you are worried about. Thus, there should be no problem translating the interface to LTR languages for the time being. -Habbit 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] Move kern/loader.c to boot.mod and add preboot_support (was Re: Sendkey patch)
El vie, 05-09-2008 a las 18:13 +0200, phcoder escribió: Hello. As I said in another email there is no need for it. I send a patch for it. Well, I won't deny the patch is clever and looks good (I haven't tested it yet). However, taking the generic boot command out of kernel and leaving the several loader-specific assembly routines (which would be the parts needing relocation) there is as elegant as vogon poetry. However, if there is no opposition and your patch works fine - I'm about to test it right now - we can first commit it (thus making core.img smaller) and then work on getting the loader asm routines out of kernel. -Habbit Vladimir 'phcoder' Serbinenko Javier Martín wrote: El mié, 03-09-2008 a las 20:53 +0300, Vesa Jääskeläinen escribió: phcoder wrote: Hello. In this case we can transfer the whole functionality located in kern/loader.c to a dedicated module boot.mod. This module will also register boot command. In this way the encapsulation won't be broken and kernel will become even smaller. Remember that realmode code needs to reside below 1 MiB. That is the reason realmode code is embedded in kernel. In there you only see jumps to RM and back to PM. We could use the relocator functionality that was once discussed here (I don't know if it was finally committed) so that modules could declare bundles of code+data to be deployed to RM area. Or make sure every single instruction in there uses 32-bit addressing, together with artificial EIP-relativization of addresses like in drivemap_int13.S -Habbit ___ 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 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: Sendkey patch
El mié, 03-09-2008 a las 20:53 +0300, Vesa Jääskeläinen escribió: phcoder wrote: Hello. In this case we can transfer the whole functionality located in kern/loader.c to a dedicated module boot.mod. This module will also register boot command. In this way the encapsulation won't be broken and kernel will become even smaller. Remember that realmode code needs to reside below 1 MiB. That is the reason realmode code is embedded in kernel. In there you only see jumps to RM and back to PM. We could use the relocator functionality that was once discussed here (I don't know if it was finally committed) so that modules could declare bundles of code+data to be deployed to RM area. Or make sure every single instruction in there uses 32-bit addressing, together with artificial EIP-relativization of addresses like in drivemap_int13.S -Habbit 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: Sendkey patch
Hi again, El mié, 03-09-2008 a las 11:10 +0200, phcoder escribió: I'll have a look at it but not sure to find anything since I'm not familiar with either ata or reiserfs internal structure. I was not suggesting that it was you or me who did it; it was a general call for GRUB devs... (ahem xD) I understand. I was saying that I'll just have a look at least to know what I'm talking about. But even if we manage to decrease the size of reiserfs module there are still other FS which could result in big modules e.g. ZFS. Of course, but even though we should strive to support te widest module configurations possible, GRUB is no panacea. Even when an initialized GRUB could cope with a ZFS partition in a LVM+RAID on SATA drives not supported by the BIOS, one should not really expect to be able to boot directly off it - get a simpler boot scheme, man! extN, FAT or even SFS are way simpler filesystems which should be used for boot partitions. More complex filesystems such as ReiserFS or NTFS require _less_ complex schemes such as no LVM+RAID in order to keep the total core size manageable. 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: Sendkey patch
El mié, 03-09-2008 a las 20:07 +0300, Vesa Jääskeläinen escribió: Hi, How about following (may not be syntaxically correct): In kernel: /* Variable holding pointer to preboot hook function. */ preboot_hook: .long 0 ... /* Check if there is preboot hook installed. */ movlpreboot_hook, %eax testl %eax, %eax jne 1f call%eax 1: /* Continue boot. */ Then in module code: void grub_preboot_hook_handler(void) { /* Process list of registered preboot hooks. */ } void grub_preboot_register_hook(...); void grub_preboot_unregister_hook(...); GRUB_MOD_INIT(preboot) { preboot_hook = grub_preboot_hook_handler; } GRUB_MOD_FINI(preboot) { preboot_hook = 0; } If preboot.mod gets loaded then it goes and registers preboot handler to kernel to do its tricks. Other modules then would use preboot.mod to register their own handlers. This is certainly a good solution, and would probably add less than 10 bytes to the kernel. I only have to add two things: * This is not as elegant as it could be, as it forces us to expose preboot_hook in the kernel, thus breaking encapsulation a bit. But it is a price that I'm more than willing to pay. * I doubt: is the current DL system in GRUB able to hand dependencies directly? In other words, is our insmod more like modprobe or Linux insmod? This affects the handling of insmod drivemap or insmod sendkey if preboot.mod is not loaded beforehand. -Habbit 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: Sendkey patch
El mar, 02-09-2008 a las 16:23 +0200, phcoder escribió: For implementing this functionality I needed my function to be executed right before booting. To do it I added a simple interface for adding preboot functions: struct grub_preboot_t *grub_loader_add_preboot (grub_err_t (*preboot_func) (int)); As parameter this function recieves a callback (integer parameter to callback is noreturn variable). This function returns a handle with which the preboot function may be removed using void grub_loader_remove_preboot (struct grub_preboot_t *p); Implementation uses linked lists and tries to implement it in as few commands as possible since this code is a part of core image. This interface can later be reused for adding different hooks to bios (map, memdisk,...). Now the questions are: Whether such interface is OK for grub2? An interface like this is implemented in another patch in discussion, my drivemap patch (see the August list archives). As yours, it is linked-list based and very similar in the prototypes. I haven't checked if your code has any kind of errors/corner cases, but it seems terser than mine even though it's a bit more difficult to understand because you use double pointers to avoid my handling of the head case. I don't understand the purpose of doubly-linking the list though... Whether we need also interface for adding postboot commands? (in case boot_function returns) I don't think it would offer a lot of functionality because most loaders don't return on failure, they just get stuck or their payload triple-faults and reboots. The same question applies for the failure of one of the preboot routines. Sure, things like an INT13 handler are easy to remove and restore the old machine state, but I really think the best option here would be panic and offer to reboot. -Habbit 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: Sendkey patch
El mar, 02-09-2008 a las 17:58 +0200, phcoder escribió: but it seems terser than mine even though it's a bit more difficult to understand because you use double pointers to avoid my handling of the head case. I don't understand the purpose of doubly-linking the list though... The goal is to avoid walking through list when deleting an entry from it. But you negate any performance gain when you _do_ traverse the list to add an entry to it instead of just make it the new head as I do. Besides, even for that, double indirection should be avoided in the structure previous pointer because it makes things oh-so-incredibly confusing. Besides, I think the user (i.e. module) visible type returned by _add and taken by _remove should be a blank hidden type, i.e. you don't need to declare struct grub_preboot_t in loader.h because the public interface only uses _pointers_ to it, whose size is known. This is all the C compiler requires and you avoid polluting the namespace with internal implementation details. I recommend the following typedefs: typedef struct grub_preboot_t* grub_preboot_hnd; typedef grub_err_t *(grub_preboot_func)(int noreturn); So that the prototypes would look grub_preboot_hnd add(grub_preboot_func f); void remove(grub_preboot_hnd handle); Whether we need also interface for adding postboot commands? (in case boot_function returns) I don't think it would offer a lot of functionality because most loaders don't return on failure, they just get stuck or their payload triple-faults and reboots. It's the case for i386-pc loaders but not the case of some other targets (e.g. EFI). So the question remains. And same question for abortion procedures. This is a spinous matter because clean handling is difficult, not always possible and requires registering undo functions, i.e. more bloat in kernel. -Habbit 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: [RFC] make output of compiling a bit less verbose to make warnings more visible
El mar, 02-09-2008 a las 18:43 +0200, Felix Zielcke escribió: Hello, I think it would be good if the output from compiling wouldn't be that verbose. Just like on compiling the Linux kernel. It shows only something like: CC source.c LD built-in.o VDSO-SYM? bla instead of that whole `gcc -D GRUB_UTIL -o xx x.c ...' thing. I haven't bothered to check yet how complicated it would be to implement this but I think it would be better to ask first if you even like this idea :) This was discussed earlier; some people argued about warnings and such, and the OP said this would make warnings stand out _more_, not less. After some more posts that I don't really remember the discussion died. the link to the full thread is at [0]. I tried to implement the proposal myself, but modifying the Ruby file that creates the *.mk files from the *.rmk files proved to be a royal PITA. I recall I proposed a switch over to a more common tool as Python and... western-rolling-ball-crossing-the-street. Feel free to try! -Habbit [0] http://lists.gnu.org/archive/html/grub-devel/2008-06/msg00248.html 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: Sendkey patch
El mar, 02-09-2008 a las 20:39 +0200, phcoder escribió: +void +grub_loader_remove_preboot (void *p) +{ + if (!p) +return; + *(PREBOOT_HND(p)-prev_pointer)=PREBOOT_HND(p)-next; This line will crash if p is the head of the list (with prev_pointer being 0). I quote crash because a crash is what happens under an OS: under GRUB you just overwrite address 0x0 which in i386-pc is the start of the real mode IVT. + if (PREBOOT_HND(p)-next) +PREBOOT_HND(p)-next-prev_pointer=PREBOOT_HND(p)-prev_pointer; +grub_free (p); +} All these macro plays are nonsense and a hindrance to readability just because you did not want to add a local variable and do the cast once. Here is my version of your patch, without the double indirection and the strange plays. The overhead is 103 bytes without the error line against 63 of yours, but I really think that the symmetric and understandable handling of previous and next is worth 40 bytes. PS: +void *EXPORT_FUNC(grub_loader_add_preboot) (grub_err_t ... I think that (only) in function declarations it's better to write void* f() than void *f() because otherwise the * can be easily overlooked. However, this is my word and does not come from the GCS. Index: kern/loader.c === --- kern/loader.c (revisión: 1845) +++ kern/loader.c (copia de trabajo) @@ -22,12 +22,54 @@ #include grub/err.h #include grub/kernel.h +struct grub_preboot_t +{ + grub_err_t (*preboot_func) (int); + struct grub_preboot_t *next; + struct grub_preboot_t *previous; +}; + static grub_err_t (*grub_loader_boot_func) (void); static grub_err_t (*grub_loader_unload_func) (void); static int grub_loader_noreturn; static int grub_loader_loaded; +static struct grub_preboot_t *grub_loader_preboots=0; + +void * +grub_loader_add_preboot (grub_err_t (*preboot_func) (int)) +{ + struct grub_preboot_t *cur; + if (!preboot_func) +return 0; + cur = (struct grub_preboot_t *) grub_malloc (sizeof (struct grub_preboot_t)); + if (!cur) +{ + grub_error (GRUB_ERR_OUT_OF_MEMORY, no memory left to register hook); + return 0; +} + cur-next = grub_loader_preboots; + cur-previous = 0; + cur-next-previous = cur; + cur-preboot_func = preboot_func; + grub_loader_preboots = cur; + return cur; +} + +void +grub_loader_remove_preboot (void *p) +{ + struct grub_preboot_t *hnd = (struct grub_preboot_t *)p; + if (!hnd) +return; + if (hnd-previous) +hnd-previous-next = hnd-next; + if (hnd-next) +hnd-next-previous = hnd-previous; + grub_free (hnd); +} + int grub_loader_is_loaded (void) { @@ -64,12 +106,18 @@ grub_err_t grub_loader_boot (void) { + struct grub_preboot_t *iter=grub_loader_preboots; if (! grub_loader_loaded) return grub_error (GRUB_ERR_NO_KERNEL, no loaded kernel); if (grub_loader_noreturn) grub_machine_fini (); - + while (iter) +{ + if (iter-preboot_func) + iter-preboot_func (grub_loader_noreturn); + iter=iter-next; +} return (grub_loader_boot_func) (); } Index: include/grub/loader.h === --- include/grub/loader.h (revisión: 1845) +++ include/grub/loader.h (copia de trabajo) @@ -25,6 +25,8 @@ #include grub/err.h #include grub/types.h +typedef grub_err_t (*grub_preboot_func) (int noreturn); + /* Check if a loader is loaded. */ int EXPORT_FUNC(grub_loader_is_loaded) (void); @@ -37,6 +39,12 @@ /* Unset current loader, if any. */ void EXPORT_FUNC(grub_loader_unset) (void); +/*Add a preboot function*/ +void* EXPORT_FUNC(grub_loader_add_preboot) (grub_preboot_func func); + +/*Remove given preboot function*/ +void EXPORT_FUNC(grub_loader_remove_preboot) (void *hnd); + /* Call the boot hook in current loader. This may or may not return, depending on the setting by grub_loader_set. */ grub_err_t EXPORT_FUNC(grub_loader_boot) (void); 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: Sendkey patch
El mié, 03-09-2008 a las 00:22 +0200, phcoder escribió: Here is my version of your patch, without the double indirection and the strange plays. The overhead is 103 bytes without the error line against 63 of yours, but I really think that the symmetric and understandable handling of previous and next is worth 40 bytes. Well let's summ up what we have: -my version in 63 bytes but difficult to read (could some comments help with it?) -your version much easier to read but in 103 bytes Neither of versions is right or wrong. It's a question of priorities. I had some experiences that past some point it's difficult to decrease size past some point. Core image has to be embed in first cylinder. There we have 62 sectors=31744 bytes. We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the first is used by the MBR). And now our core image (my version with error reporting) with ata,pc and reiserfs is 29654 bytes. This leaves us 2090 bytes. This combination is not something completely out of the ordinary. So I suggest to give the priority to the size of the kernel over readability (perhaps adding some comments to my version). I was wondering why my data did not match yours, and then I realized I was using my usual extreme combination of modules: biosdisk pc ext2 lvm raid yields 29298 bytes, plenty of space. But ata and reiserfs are quite the bloaters... The reiserfs case is particularly strange: in the linux kernel, the reiserfs module is 50% bigger than ext3.ko; while in GRUB, reiserfs.mod (without journaling) is twice the size of ext2.mod (which includes full support for ext2, partial journal support in ext3 and extents in ext4). Thus, while you are right in prioritizing kernel size; why not optimize reiserfs a bit instead of killing our (and future maintainers') eyes and brains to shave less than 40 bytes from kernel? I suppose the story would be similar with ata, because it is a new module that is yet in development. -Habbit 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: Sendkey patch
El mié, 03-09-2008 a las 02:08 +0200, phcoder escribió: Hello, again. Javier Martín wrote: We have 63 sectors = 32256 bytes (sectors range from 0 to 63 and the first is used by the MBR). I've just rechecked on my system. My first partition begins at sector number 63. This is the value I've seen at most systems. So last usable sector is 62. Sector 0 is MBR. So we have 62 sectors Oops, true! How strange. So there was no sector 63 in the CHS model and that is why the first sector of cyl 1 (the start of the first partition) is LBA 63... Does anyone know the historical reasons for this? And now our core image (my version with error reporting) with ata,pc and reiserfs is 29654 bytes. This leaves us 2090 bytes. This combination is not something completely out of the ordinary. So I suggest to give the priority to the size of the kernel over readability (perhaps adding some comments to my version). I was wondering why my data did not match yours, and then I realized I was using my usual extreme combination of modules: biosdisk pc ext2 lvm raid yields 29298 bytes, plenty of space. But ata and reiserfs are quite the bloaters... The reiserfs case is particularly strange: in the linux kernel, the reiserfs module is 50% bigger than ext3.ko; while in GRUB, reiserfs.mod (without journaling) is twice the size of ext2.mod (which includes full support for ext2, partial journal support in ext3 and extents in ext4). I'll have a look at it but not sure to find anything since I'm not familiar with either ata or reiserfs internal structure. I was not suggesting that it was you or me who did it; it was a general call for GRUB devs... (ahem xD) Thus, while you are right in prioritizing kernel size; why not optimize reiserfs a bit instead of killing our (and future maintainers') eyes and brains to shave less than 40 bytes from kernel? I suppose the story would be similar with ata, because it is a new module that is yet in development. Well the point is that if we don't do it now and then one day we'll have to squeeze the core it will be very difficult to find places like this. Yes, but my point is that 40 bytes is so small a difference that it can offset by simply rewriting error strings in kernel and other smallish non-intrusive changes and thus we should prioritize future maintainability. However, eventually this is not our decision to make: the Overlords here (mainly Marco, but also Robert, Vesa and Bean) are the ones who should, likely only once the interface is established, decide how to conjugate the Prime Directive of keep kernel small with code complexity in this particular case. -Habbit 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: grub-probe detects ext4 wronly as ext2
Hello there! It's nice to be in town again, though post-vacation syndrome is hitting me full... Resuming the thread, El sáb, 30-08-2008 a las 13:17 +0200, Robert Millan escribió: On Mon, Aug 11, 2008 at 04:14:00PM +0200, Javier Martín wrote: This overrides the grub_errno and grub_errmsg provided by grub_disk_read and replaces them with values that hide the true problem. If there was a disk read error, we really want to know about it from the lower layer. Well, the old version did just the same (even worse, because the message was generic). What would be the correct path of action here? I mean, how can we propagate the error messages? It shouldn't call grub_error(). So in the cases the fail is caused by an underlying error (like I/O) the code should just return failure and leave the old error in errno and errmsg? Yes. Ok, so that's already done in the previous patch. static struct grub_ext2_data * grub_ext2_mount (grub_disk_t disk) { struct grub_ext2_data *data; + const char *local_error = 0; Please use NULL for pointers. I don't object at all, but 0 is used throughout the GRUB code to represent null pointers (see the args struct in any command source file), so I don't understand the criterion being applied here. - if (grub_errno) + if (grub_errno != GRUB_ERR_NONE) Why? 1st, because the condition being checked is explicit and thus clearer. These int-bool C conventions are, even though enshrined by ANSI and thus pretty standard and reliable, irky at best and due only to the lack of a proper boolean type. As I think I stated before, the only cases I use such cast is when checking for null pointers and when using variables that are explicitly boolean in nature. 2nd, because the new code does not assume that GRUB_ERR_NONE is defined to zero. Even though this definition will most likely never change, we might one day decide that -42 is a better value for success. 3rd, because in addition to all that, with any sensible compiler, the additional binary size cost is nothing at all. -goto fail; +EXT2_DRIVER_MOUNT_FAIL(0); I find very little use in this. I assume it's supposed to simplify things, but in fact it adds an extra level of indirection for someone who's reading the code. It provides no runtime improvement, and it's inconsistent with what we do elsewhere. It is not meant to provide any runtime improvement, it's just for consistency: since local_error is already zero, a goto fail would suffice but I think this uniformity adds readability, not hinders it: the referenced macro is at the very top of the function, not on a included header, so the reference is not very time-consuming; and it's not very complex, so it only needs to be read once. Besides, most compilers should just optimize the extra assignment away given that the value of local_error is ct-known for the code paths leading to that statement and it is not a volatile variable. + /* Only call grub_error if the fail was _not_ caused by underlying errors. */ No need to document this. It's the same deal in a huge amount of routines throurough the GRUB source. OK, removed the comment. Maybe a similar comment will be fine over the macro, though. That was all, folks! Given that I (think I) addressed your two main objections, I won't send a new patch right now. I will continue to read the list this month, but my availability is likely to vary wildly, as I will be trapped by the ensnaring bureaucracy of the Spanish universities, scholarships, etc. -Habbit 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: Modifying GRUB to read DMI data
2008/8/24 W. Michael Petullo [EMAIL PROTECTED]: I am interested in modifying GRUB to read DMI data from my BIOS (i.e., the data dmidecode reads). The SMBIOS specification seems to state that the Plug-n-Play function interface is deprecated while the table-based interface requires a processor running in 32- or 64-bit protected mode. Does anyone know where I could find information on a non-deprecated method to read DMI data while in real mode? But you just said this can be done by parsing tables in i386 32-bit mode, why not just do that? Btw, which information do you need to obtain from there? I was under the impression that the DMI tables could be stored above 1MB, and that these memory locations were not easily accessible when in real mode. I have now found that, at least in my hardware's case, the DMI tables are below 1MB and easily read. Is this the case for all hardware? I have a client that has a piece of hardware that provides an electronic key via DMI when a physical key is inserted into the device. I am writing a command that will read that key and store it in GRUB's environment for Michael Gorven's LUKS module to pick up. Mike ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel The environment of a GRUB module is 32-bit pmode - you can directly access DMI: why do you need real mode? -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Dell Media Direct button
2008/8/21 Stefan Reinauer [EMAIL PROTECTED]: Robert Millan wrote: On Wed, Aug 20, 2008 at 10:57:22AM +0200, Per Öberg wrote: Hi Some laptops, e.g., from Dell have a special button that they use to boot a special embedded OS for media only instead of the ordinary OS. For my Dell XPS1330M I can determine if the Media button was pressed by first writing 0xf9 to port 0x70 and then testing bit 0x08 of port 0x71. It would be really nice if such a test could be enabled in grub so that grub can go directly to a specific menu alternative without showing the gui if the media button was pressed. Is this interesting? I'd like to contribute but I don't know where to start. Sounds interesting, but this needs some thought on how to design it. I suppose what you want is change the 'default' variable. Perhaps increase it by 1? But then, where do you do this? grub_machine_init is too early as 'default' hasn't been set yet. Maybe we could have a global 'int default_offset' variable that is initialized in grub_machine_init and later on used by normal.mod? The sequence of writing to port 0x70 / reading from port 0x71 reflects reading from the computer's cmos nvram memory. bit 7 of 0x70 is reserved for disabling NMIs, so the actual information is stored in byte 0x79[8] in the cmos. To allow full flexibility, there should just be a module that allows reading / writing the cmos values (could also be useful for other things, such as reading a boot order set by the bios). Everything else makes more sense in scripting: - changing default - changing timeout - support for bit operations in the parser - etc... -- coresystems GmbH • Brahmsstr. 16 • D-79104 Freiburg i. Br. Tel.: +49 761 7668825 • Fax: +49 761 7664613 Email: [EMAIL PROTECTED] • http://www.coresystems.de/ Registergericht: Amtsgericht Freiburg • HRB 7656 Geschäftsführer: Stefan Reinauer • Ust-IdNr.: DE245674866 ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel So what about a new nvram module in i386-pc that creates a variable $NVRAM hooked to routines getting/setting the live contents of the cmos memory? Then menu scripts can check its contents, among them the MediaDirect button. -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: Dell Media Direct button
2008/8/20 Colin D Bennett [EMAIL PROTECTED]: On Wed, 20 Aug 2008 12:12:59 +0200 Robert Millan [EMAIL PROTECTED] wrote: On Wed, Aug 20, 2008 at 10:57:22AM +0200, Per Öberg wrote: Hi Some laptops, e.g., from Dell have a special button that they use to boot a special embedded OS for media only instead of the ordinary OS. For my Dell XPS1330M I can determine if the Media button was pressed by first writing 0xf9 to port 0x70 and then testing bit 0x08 of port 0x71. It would be really nice if such a test could be enabled in grub so that grub can go directly to a specific menu alternative without showing the gui if the media button was pressed. Is this interesting? I'd like to contribute but I don't know where to start. Sounds interesting, but this needs some thought on how to design it. I suppose what you want is change the 'default' variable. Perhaps increase it by 1? But then, where do you do this? grub_machine_init is too early as 'default' hasn't been set yet. Maybe we could have a global 'int default_offset' variable that is initialized in grub_machine_init and later on used by normal.mod? If I pressed the Media Direct button, I would also want to have a timeout of 0, since by pressing the Media Direct button instead of the power button, I've already indicated which entry I want to boot, and there is no need to show the menu -- unless, perhaps, we decided to show a different media menu... so hopefully giving all the power to the grub.cfg script would be enough to make all this possible. Regards, Colin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel Well, I own a XPS1330 myself (I'm writing from it now, as I'm taking a week at the beach in Benidorm ^^). This functionality could be implemented by a module that used the pre-menu hooks proposed by Bean: it would check the status before the menu is shown, then act on the result. However, all this assumes that pressing that key creates no additional side effect like the replacement of the MBR by the system firmware, booting another MBR written in flash memory or something like that: I thought the choice took place at the real MBR, but disassembling it revealed that it does not use the mentioned I/O ports interface, so most likely what the system firmware does is set the active flag in the MediaDirect partition. I'm ready to check any related patches on the actual hardware _once I'm back in Madrid_ because here I have no recovery tools. -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] Platform information services
El vie, 15-08-2008 a las 19:31 +0300, Vesa Jääskeläinen escribió: Javier Martín wrote: WRT kernel and modules going hand by hand, think about external modules: if the drivemap module is finally rejected for introduction in GRUB, I will not scrap it, but keep it as a module external to the official GNU sources and possibly offer it in a web in the form of patches to the official GRUB2. In this case, changes made to the kernel would not take into account that module, which would break if I weren't monitoring this list daily. Then it is really your problem ;) Indeed, but bitrot is not just the real of external modules: it's happening right now even within the GRUB trunk as you admit in the Build problems on powerpc thread... Additionally, the cost of this function in platforms which don't have any structs registered yet, as the function could be a stub like this: void* grub_machine_get_platform_structure (int stidx) { grub_error (GRUB_ERR_BAD_ARGUMENT, Struct %d not supported, stidx); return 0; } The kernel space taken would most likely be less than 50 bytes. For i386-pc, it could be like this (also lightweight) function: void* grub_machine_get_platform_structure (int stidx) { grub_errno = GRUB_ERR_NONE; switch (stidx) { case GRUB_MACHINE_I386_IVT: return /* Call to asm function that runs SIDT in real mode */ ; case GRUB_MACHINE_I386_BDA: return (void*)0x400; default: grub_error (GRUB_ERR_BAD_ARGUMENT, Struct %d not supported, stidx); return 0; } } And what lets assume couple of extra platforms... how about x86-32bit-efi and ppc. What do they do? Implement their own enum entries for those indexes and only use their own indices...? At first, they would just have the stub which does not recognize any index, but yes, i386-efi devs could decide that certain firmware-provided structure (like a video modes info table or such, I don't know the internals of EFI) might be interesting to a module they're creating, so they create an index for it and add it to the version of the function in their platform. If I had not mentioned it before, the function would be declared in a cross-platform file, but _implemented_ in platform-specific files, and the indices would be declared in the platform-specific machine.h. Thus, there would not be a single indices namespace: structure #1 might be the IVT in i386-pc, but some devices info table in powerpc-ieee1275. Where here we are sharing any code? (if we do not count the name of the fuction.) Interface is kinda useless if there is no possibility that no-one is sharing its functionality... The idea is a single function to retrieve the addresses of firmware-provided/used structures. This includes the IVT and BDA in i386-pc, but as I said before it could also be used by other platforms for their own structures. The alternative would be just creating such get struct X functions on each platform as they are needed, but I imagined that a single interface (with such a low cost in space) would be a more elegant solution. -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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: [RFC] Platform information services
El vie, 15-08-2008 a las 20:14 +0300, Vesa Jääskeläinen escribió: Javier Martín wrote: El vie, 15-08-2008 a las 19:31 +0300, Vesa Jääskeläinen escribió: Javier Martín wrote: WRT kernel and modules going hand by hand, think about external modules: if the drivemap module is finally rejected for introduction in GRUB, I will not scrap it, but keep it as a module external to the official GNU sources and possibly offer it in a web in the form of patches to the official GRUB2. In this case, changes made to the kernel would not take into account that module, which would break if I weren't monitoring this list daily. Then it is really your problem ;) Indeed, but bitrot is not just the real of external modules: it's happening right now even within the GRUB trunk as you admit in the Build problems on powerpc thread... And? If Power PC maintainer is nowhere to update to newest additions then it is rightly in in-compilable state and if it rots too long its support will be removed. That's life. Yes, but you implicitly and particularly Robert explicitly argued that when kernel changes devs here take the time to update modules in consonance and vice versa. The PPC build problem shows that, for whatever reason, this is not always true. Additionally, the cost of this function in platforms which don't have any structs registered yet, as the function could be a stub like this: void* grub_machine_get_platform_structure (int stidx) { grub_error (GRUB_ERR_BAD_ARGUMENT, Struct %d not supported, stidx); return 0; } The kernel space taken would most likely be less than 50 bytes. For i386-pc, it could be like this (also lightweight) function: void* grub_machine_get_platform_structure (int stidx) { grub_errno = GRUB_ERR_NONE; switch (stidx) { case GRUB_MACHINE_I386_IVT: return /* Call to asm function that runs SIDT in real mode */ ; case GRUB_MACHINE_I386_BDA: return (void*)0x400; default: grub_error (GRUB_ERR_BAD_ARGUMENT, Struct %d not supported, stidx); return 0; } } And what lets assume couple of extra platforms... how about x86-32bit-efi and ppc. What do they do? Implement their own enum entries for those indexes and only use their own indices...? At first, they would just have the stub which does not recognize any index, but yes, i386-efi devs could decide that certain firmware-provided structure (like a video modes info table or such, I don't know the internals of EFI) might be interesting to a module they're creating, so they create an index for it and add it to the version of the function in their platform. If I had not mentioned it before, the function would be declared in a cross-platform file, but _implemented_ in platform-specific files, and the indices would be declared in the platform-specific machine.h. Thus, there would not be a single indices namespace: structure #1 might be the IVT in i386-pc, but some devices info table in powerpc-ieee1275. Where here we are sharing any code? (if we do not count the name of the fuction.) Interface is kinda useless if there is no possibility that no-one is sharing its functionality... The idea is a single function to retrieve the addresses of firmware-provided/used structures. This includes the IVT and BDA in i386-pc, but as I said before it could also be used by other platforms for their own structures. The alternative would be just creating such get struct X functions on each platform as they are needed, but I imagined that a single interface (with such a low cost in space) would be a more elegant solution. I still do not see the need to create cross platform function that cannot be used for cross platform purposes. It is much more reasonable to write such needs as in kernel for the platform or in platform specific modules. I do not have a problem with function to retrieve pointer to some platform specific function on platfrom specific code. That is normal life of the platform. Well, think of this as a platform-specific function to retrieve the address of platform-specific (firmware-provided) structures. The only difference is that in my proposal such a function has the same signature (prototype) for every platform. And if we think about code safety, casting is bad. And if you keep indices colliding on different platforms then you are just calling for problems. It completely removes any advantage what this kinda wrapper could have had. Why? any module that is advanced (in the sense of complicated) enough to require the use of platform-specific structures will be platform-specific itself, or at least the part of it that accesses the structures, and thus the risk of indices actually colliding is nearly zero. -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: [RFC] Platform information services
El vie, 15-08-2008 a las 21:35 +0300, Vesa Jääskeläinen escribió: Javier Martín wrote: I still do not see the need to create cross platform function that cannot be used for cross platform purposes. It is much more reasonable to write such needs as in kernel for the platform or in platform specific modules. I do not have a problem with function to retrieve pointer to some platform specific function on platfrom specific code. That is normal life of the platform. Well, think of this as a platform-specific function to retrieve the address of platform-specific (firmware-provided) structures. The only difference is that in my proposal such a function has the same signature (prototype) for every platform. Well... THEN WHY NOT MAKE A PLATFORM SPECIFIC FUCTION THAT IS CONVENIENT TO USE ON PLATFORM WHERE IT IS MEANT TO BE USED FOR AND IT ACTUALLY HAVE A MEANING ON WHERE IT IS ACCESSIBLE. I take that by convenient to use and your earlier code safety reference, you imply a function that returns a typed pointer to the structure, like bda* var = grub_getbda(); in i386-pc, so that fields of such struct can be directly accessed by var-free_mem_kb. However, this requires that a function is created for each firmware structure that is deemed interesting by GRUB developers. In this proposal, only one (very lightweight) function is created and then IDs are defined for the structs. WRT code safety, malloc and friends also return a void* that you have to cast, so nothing new under the sky. If you want to learn something, study interfaces of disk-biosdisk-bios and video-vbe-video bios... There are also several other good spots on GRUB 2 code base that you could see... Will do, particularly the biosdisk part (video code nearly always makes my head spin). You have still not provided a single GOOD example where this would be used AND would provide some ADDED VALUE. Unless you provide a convincing one (while I pretty much doubt) I do not see this feature to be in our official tree. The added value (that expression reminds me of taxes) is: -Compared to SVN HEAD: a way to retrieve the _module-accessible_ address of firmware-provided structures. This is a protection against future changes in the GRUB kernel (like enabling paging, changing memory mapping schemes, etc.) that would otherwise force to inspect every single module for accesses to such structures by their real addresses. -Compared to SVN HEAD plus the many ad-hoc functions you seem to support: for most of the structures an asm helper is not required, thus less kernel space overhead than with many functions. Also, focus of efforts - maybe the oversight on the core function would add additional bitrot protection compared to the individuals functions. An use case would be in the proposed drivemap module, in the function installing an interrupt handler in the real-mode IVT: instead of the current code with magic addresses, /* Real mode IVT slot (seg:off far pointer) for interrupt 0x13. */ grub_uint32_t *ivtslot = (grub_uint32_t*)0x004c; The module could be like this: /* Real mode IVT slot (seg:off far pointer) for interrupt 0x13. */ grub_uint32_t *ivtslot = 19 + (grub_uint32_t*)grub_machine_get_fwstruct(GRUB_I386_PC_IVT); Then, if anything of the following ever happens: * The real mode IVT was live-relocated, either by GRUB or by the BIOS, by the use of LIDT * The real mode IVT was destroyed by GRUB, which keeps a copy at another address and sets it up, with the rest of a 8086-compatible environment only when it is required (memory pressure maybe?). * Linear addresses 0-1M do not correspond with physical addresses 0-1M because of whatever memory management the GRUB kernel is doing The above code would continue to work, while the even-more-above code would fail with unpredictable results. -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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: [RFC] Platform information services
El vie, 15-08-2008 a las 22:46 +0300, Vesa Jääskeläinen escribió: Javier Martín wrote: I take that by convenient to use and your earlier code safety reference, you imply a function that returns a typed pointer to the structure, like bda* var = grub_getbda(); in i386-pc, so that fields of such struct can be directly accessed by var-free_mem_kb. However, this requires that a function is created for each firmware structure that is deemed interesting by GRUB developers. In this proposal, only one (very lightweight) function is created and then IDs are defined for the structs. WRT code safety, malloc and friends also return a void* that you have to cast, so nothing new under the sky. In your case there is really no change. You create new switch branch (which is most likely implemented by function somewhere). In order to access this void * you have to know its contents. Why not use structure to define its contents where viable? True, I just wanted to avoid defining structures that would only be used once or twice, but you're right. When using malloc() you know that memory returned is not formatted in anyway. Therefore it is safe to cast. You just want to make it more complex that it needs to be in my eyes. You have still not provided a single GOOD example where this would be used AND would provide some ADDED VALUE. Unless you provide a convincing one (while I pretty much doubt) I do not see this feature to be in our official tree. An use case would be in the proposed drivemap module, in the function installing an interrupt handler in the real-mode IVT: instead of the current code with magic addresses, /* Real mode IVT slot (seg:off far pointer) for interrupt 0x13. */ grub_uint32_t *ivtslot = (grub_uint32_t*)0x004c; The module could be like this: /* Real mode IVT slot (seg:off far pointer) for interrupt 0x13. */ grub_uint32_t *ivtslot = 19 + (grub_uint32_t*)grub_machine_get_fwstruct(GRUB_I386_PC_IVT); The better way that complies with all your error cases provided: grub_ivt_entry_t *entry; entry = grub_get_ivt_entry(0x13); *entry = new_value; This was another of the solutions I was considering in this thread (the many-functions approach), I just found the single-function solution more elegant in the case of large numbers (10) of structures becoming visible through this method, but your snippet looks fine and safer. of course grub_ivt_entry_t could be renamed to be something like grub_realmode_addr_t. grub_realmode_farptr_t would suit better IMO. -Habbit . ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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: [RFC] Platform information services
El vie, 15-08-2008 a las 18:38 -0400, Isaac Dupree escribió: my view of the interface breakage problem in this project: Either the property of the kernel you're relying on is going to change, or it isn't; it doesn't/shouldn't have stable APIs, and it will change only if there's a reason for it to change. So I think the best we can do is to document what code relies on what properties of the kernel. Wherever's appropriate and will likely be noticed and kept up-to-date. for example: if you rely on a property of the kernel, you should document it somewhere in the kernel, and document everyone (hopefully) who relies on that property; or the users could have some symbol in the source that you can search for. It doesn't have to be a function that takes up any actual kernel disk-space. a bit at a distance, -Isaac ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel The problem is exactly that: for the sake of modularity we are writing module code that requires the manipulation of firmware-provided structures, but we are relying on kernel implementation details that are nowhere specified in the kernel-module interfaces like the GRUB kernel will not activate any kind of paging and in any event the 0-1M+64K memory region will always be identity-mapped. Even when some parts _might_ be documented in the wiki (like the MemoryMap page), other parts of the wiki are so outdated and wrong that one cannot trust the good pages! Besides, does the info in MemoryMap describe the layout of the physical address space or the linear address space that modules see? etc Thus, either we create interfaces in the kernel to deal with the possible breakage of these assumptions (i.e. the platform info functions we've been discussing in this thread would take care of translating addresses across any memory mappings) or we set those assumptions in stone and turn them into part of the kernel-module interface, _documenting them_. Seriously, InteralsIntro et al. need a lot more love... -Habbit 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: [RFC] Platform information services
2008/8/14 Vesa Jääskeläinen [EMAIL PROTECTED]: Javier Martín wrote: Hi there everybody, I'm opening the RFC because I want to add some kind of infrastructure to retrieve the address of system/platform structures. I will explain myself: my use case is in i386-pc and for the drivemap module, in which a function installs a TSR int13h handler. This requires the function to have access to two real mode structures, namely the BIOS Data Area, which is based at 0040:h; and the Interrupt Vector Table, which conventionally starts at 0 but that could have been placed elsewhere by the use of the LIDT instruction. But it is designed to use linear address space for memory so no need to worry about it. I do not see any reason why there would be paging or non-linear memory mapping in GRUB 2 (i386-pcbios). Yes, but this is a kernel design decision that is specified nowhere to the modules writers. Thus, this could change tomorrow and the scheme would break down. Or did I miss something? So basically I do not see need for such services. As this does not even need to be platform independent. If you need to alter IVT you can modify it on the fly. Though you have to remember where to use only LOW mem addresses in there. Not just low mem addresses, I need to compute the real mode far pointer (seg:off). Besides, while the BDA always starts at 0040:, the IVT could have been relocated by either the BIOS or GRUB itself, so I need to use SIDT, which is not a privileged instruction itself but requires a privileged drop to r-mode to get the IVT address instead of the pmode IDT. -Habbit ___ 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: [RFC] Platform information services
El jue, 14-08-2008 a las 20:00 +0200, Robert Millan escribió: On Thu, Aug 14, 2008 at 06:38:41PM +0200, Javier Martín wrote: Yes, but this is a kernel design decision that is specified nowhere to the modules writers. Thus, this could change tomorrow and the scheme would break down. GRUB is being developed as a whole, which includes kernel and modules. If we modify the kernel in a way that could break modules, it's part of the procedure to check modules and make sure they don't break because of this (although, of course, we could always make mistakes). That is a very sane policy, but in this case I'm arguing that an improvement could be made so that such process would not be necessary, or would be much lighter, regarding a particular section of the kernel because modules would not rely on certain assumptions about the internal implementation of GRUB: separation of interface and implementation. However, there is not a consistent, fully-documented kernel-module interface, and module developers usually do not know what invariants hold in their environment - they find it by trial and error. This needs to change eventually, particularly in the documentation part (please avoid GRUB ending like ALSA): the GRUBInternals page in the wiki should be filled up and updated a bit more often. WRT kernel and modules going hand by hand, think about external modules: if the drivemap module is finally rejected for introduction in GRUB, I will not scrap it, but keep it as a module external to the official GNU sources and possibly offer it in a web in the form of patches to the official GRUB2. In this case, changes made to the kernel would not take into account that module, which would break if I weren't monitoring this list daily. Additionally, the cost of this function in platforms which don't have any structs registered yet, as the function could be a stub like this: void* grub_machine_get_platform_structure (int stidx) { grub_error (GRUB_ERR_BAD_ARGUMENT, Struct %d not supported, stidx); return 0; } The kernel space taken would most likely be less than 50 bytes. For i386-pc, it could be like this (also lightweight) function: void* grub_machine_get_platform_structure (int stidx) { grub_errno = GRUB_ERR_NONE; switch (stidx) { case GRUB_MACHINE_I386_IVT: return /* Call to asm function that runs SIDT in real mode */ ; case GRUB_MACHINE_I386_BDA: return (void*)0x400; default: grub_error (GRUB_ERR_BAD_ARGUMENT, Struct %d not supported, stidx); return 0; } } BTW, I've noticed that when using the function, if the result is stored in void* p, then the success check cannot only rely on if (p), because 0 is also a legal address (i.e. for the IVT). Thus, the checks should be like if (p || grub_errno == GRUB_ERR_NONE) with the implementation ensuring that errno is correctly set on success. -Habbit 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: [RFC] Platform information services
El jue, 14-08-2008 a las 20:41 +0300, Vesa Jääskeläinen escribió: Javier Martín wrote: 2008/8/14 Vesa Jääskeläinen [EMAIL PROTECTED]: Javier Martín wrote: Hi there everybody, I'm opening the RFC because I want to add some kind of infrastructure to retrieve the address of system/platform structures. I will explain myself: my use case is in i386-pc and for the drivemap module, in which a function installs a TSR int13h handler. This requires the function to have access to two real mode structures, namely the BIOS Data Area, which is based at 0040:h; and the Interrupt Vector Table, which conventionally starts at 0 but that could have been placed elsewhere by the use of the LIDT instruction. But it is designed to use linear address space for memory so no need to worry about it. I do not see any reason why there would be paging or non-linear memory mapping in GRUB 2 (i386-pcbios). Yes, but this is a kernel design decision that is specified nowhere to the modules writers. Thus, this could change tomorrow and the scheme would break down. Still I do not that being changed on GRUB 2 (i386-pcbios). And basically generic module writers do not need to care, and if it is platform specific module then you can adapt to that platform if needed. Think it like device driver. Device drivers are just the kind of example that I'm basing upon: the part related to their hardware they manipulate as directly and recklessly as the OS kernel allows them, but WRT other parts of the system they rely on a very defined kernel-driver interface (NDIS, anyone?) that simply is very nebulous in GRUB. You say that paging will never be enabled in GRUB2, and it is very likely that your assertion holds, but what if we suddenly find some kind of benefit to enable it and some other rock to be walked around requires that the first MB of memory not be identity mapped? Kaboom for modules assuming it is: they were assuming things about the implementation of the kernel that do _not_ have to hold as they are nowhere specified Or did I miss something? So basically I do not see need for such services. As this does not even need to be platform independent. If you need to alter IVT you can modify it on the fly. Though you have to remember where to use only LOW mem addresses in there. Not just low mem addresses, I need to compute the real mode far pointer (seg:off). Besides, while the BDA always starts at 0040:, the IVT could have been relocated by either the BIOS or GRUB itself, so I need to use SIDT, which is not a privileged instruction itself but requires a privileged drop to r-mode to get the IVT address instead of the pmode IDT. Last time I checked seg:off conversion was (seg 4) | off giving access to 1 MiB of memory. I meant that _if_ some kind of mapping was enabled, such conversion would be non-trivial and possibly only known to the kernel. BIOS will not relocate this as it is assumed in later stages to be on that address. And I do not see GRUB doing that as it would need to keep that table in same place as where BIOS has stored it. All x86-32/64 BIOSes could perfectly relocate the IVT because they feel like it, as the LIDT/SIDT operations were introduced with the 80286. Most current OSes (Linux, Windows from the NT family, etc.) don't care about the IVT just because they have no need of using it ever again, start with a cli and jump into pmode as soon as they can. If you still prefer to query it on x86, then there is nothing there to limit you on doing that. You can always write code to low mem area to have a real mode call to query any addresses you wish. But I still do not see any reason why you would write heavy wrapper for that kind of information especially when those are really platform specific and are usually not shared between systems. Every platform should use familiar and convenient method do query that kind of information. As I showed in my reply to Robert's message, the wrapper need not be heavy: most likely the opposite, as a stub implementation for platforms not providing any structures is under 50 bytes, and the wrapper for other platforms consists mainly of a switch-case. -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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] Drivemap module
El jue, 14-08-2008 a las 19:15 +0200, Marco Gerards escribió: Javier Martín [EMAIL PROTECTED] writes: Ok, making a mixup reply... Please don't. Unless you do not like it that your mails go unread. Having a small kernel is highly desireable for most users. If the kernel is too big, it won't fit and then either we have to use blocklists (which are unreliable), or we have to abort the install. Please, try to find a way that doesn't increase kernel size significantly. If the kernel interfaces are not extensible enough, you could try to readjust them for your needs. This approach works well most of the time (although I haven't studied this particular problem in detail). Like discussed before. Bring up such modifications like hooks up in a *separate* thread. I already said that not everyone reads this discussion. I will not accept a patch that changes the kernel if it is part of a bigger patch that not many people read. Please don't discuss this over with Robert and me, you know that it was pointed out that this has to be a patch in a separate thread. Furthermore, this is a way to get some feedback from Bean who wants something similar, IIRC. I know I will be regretting saying this, but it is _very_ rude to review some five versions of the patch, spotting mostly coding-style errors on each, and then, on version 8, tell me that you won't accept a patch that contains blah (with blah being essential for the patch to work). Quite the proverbial slap in the face to me. I did NOT say that your patch will not be accepted. In one of the earlier reviews (perhaps even the first) I mentioned that certain parts should be reviewed separately. It is a slap in the face that you imply I am a bad person when I repeat that you need to bring up some issues *separately*. I would consider it bad style from me towards other developers to change code they might have a strong opinion on. I don't imply you're a bad person or anything like that, sorry if it seemed so. By the way, I searched the whole thread of [PATCH] drivemap module (starting in july 4-20) and the only instances in which separate, separately, split or similar words are related to phrases like put the return on a separate line or split these declarations - even though I _do_ remember you telling me something like what you say. I have actually taken the time to re-read all three threads related to drivemap [0][1][2] and the main opposition (from Vesa) was due to what the boot command would do if one of the hooks with abort_on_error set failed and the abort_on_error bit itself. Nothing _fundamental_ about the hook system. The only other objection has surfaced recently (from Robert) and is about the size such system would add to kernel - which mostly depends on the implementation of the hook system (single variable vs. array vs. linked list, for example), not the interface. And you can say what you want, but if you are stubborn enough to ignore what I say in the reviews, you shouldn't be surprised if I didn't change my opinion in the next review and still ask you to change something. In fact, it is rude just to send in a new patch while you did not take the review seriously. There are a lot of projects that reject patches on beforehand if you didn't at least bother to get familiar with their coding styles. (kneels on the floor japanese-teen-style) That is true and completely my fault: I was stubbornly trying to convince you to change a big part of the coding style without thinking that conventions are there for a reason. I apologize for all the headaches I may have caused. Furthermore, I reviewed your patch mainly because I care, not because I am the foremost expert in BIOS disks. It's for the best if other people can look at specific parts of your patch, especially if it changes the kernel. True, but the list is public and we can't be blamed if others are not interested in parts of the patch. Besides, even though I've shown that the strong opinions of Vesa and Robert did not affect any fundamental part of the hook system, I _could_ split it from drivemap and put it up for discussion on a separate thread. Perhaps you do not notice how many time I spent on reviewing patches on this list lately. Anyways, if it is not appreciated, I will leave the reviews to other people. But I simply will reject all code I do not like without actually bothering to explain why. I rather spend my time on coding instead of feeding trolls. It _is_ appreciated: if it weren't for you, this patch would have been lost in the blue long ago, just because no-one would bother to look at it. Besides, if you don't feed the trolls (me?), I'll have to go back to Wikipedia, where my 42 sockpuppets have already been banned! ^^ Seriously, though, I don't mean to be trollish at all, nor to bludgeon others into submission - that's just politics... -Habbit [0] http
Re: Idea: use menu hook to implement the savedefault command
El mié, 13-08-2008 a las 12:31 +0200, Marco Gerards escribió: Bean [EMAIL PROTECTED] writes: Hi, Now it's possible to implement savedefault with load_env and save_env, but the problem is we need to add it to every menuitem, it's tedious process, and new item don't get it automatically. I'm thinking about using menu hook to solve this. I can think of two implementation: 1. Function interface We can install hooks, which get called just before the menu is invoked. 2. Script interface We can use certain variable to specific the command to use, for example: set MENU_PRELOAD=save_env default Wouldn't this be a BOOT_PRELOAD? I thought about this before (see archives about scripting), making it possible for the user to add hooks. In C: ... menu code ... /* The `menu' hook has one argument. */ grub_hook_invoke (menu, 1, arglist); In scripting: function menu_hook_handler() save_env; hook --register --hook=menu --script menu_hook_handler Or do you think I am crazy now? ;-) This will involve some scripting hacking, but might pay off. Do you think this is too complex for users? It might be unnecessarily complex, yes. What about a C#-delegate-like approach? e.g.: function myhook() { save_env } BOOT_PRELOAD += myhook ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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] Home-End keys in menu
El mié, 13-08-2008 a las 12:50 +0200, Robert Millan escribió: On Wed, Aug 13, 2008 at 12:15:37PM +0200, Marco Gerards wrote: Hi, Carles Pina i Estany [EMAIL PROTECTED] writes: [...] 2008-08-06 Carles Pina i Estany [EMAIL PROTECTED] * menu/normal.c (run_menu): Add Home and End keys in grub-menu. This looks fine to me at first sight. Do others have problems with this? Otherwise it can be committed. Why don't we use PgUp / PgDn instead? That's more intuitive. Users with long menu lists may expect PgUp/PgDn to take them exactly _one_ page up or down, not the whole list... I personally think that Home/End are more intuitive for this particular UI interaction. 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: Idea: use menu hook to implement the savedefault command
El mié, 13-08-2008 a las 13:48 +0200, Robert Millan escribió: On Wed, Aug 13, 2008 at 01:36:44PM +0200, Javier Martín wrote: It might be unnecessarily complex, yes. What about a C#-delegate-like approach? e.g.: function myhook() { save_env } While in the process of designing interfaces, finding inspiration in patent encumbered technologies is one of the last things I would recommend. While parts of the .NET class library most certainly contain patented code, the C# language itself is part of an ECMA specification, and thus the delegate _syntax_ (not the actual implementation) would be safe to use... Or so I think - patents make my mind spin T_T Sometimes the GNU project has gone to great lengths to avoid being covered by patents, like in the development of gzip, or in its endorsement of the Xiph Foundation. I don't personally think we need to be so radical for GRUB, but we could at least be careful. 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] Drivemap module
El mié, 13-08-2008 a las 12:13 +0200, Marco Gerards escribió: Javier Martín [EMAIL PROTECTED] writes: In this reply-to-myself hoping to keep the thread continuity, I put forth the new version 7 of the patch with the following changes: - A new switch -s/--swap has been implemented, so that running drivemap -s hd0 hd1 is equivalent to issuing the command twice with the arguments normal, then reversed. There is one exception: if the first mapping fails or the second drive does not exist, no mapping is performed, whereas hand-performing the swap would have successfully assigned BIOS disk #1 to hd0, then failed to assign bd#0 to non-existent hd1. - Raw BIOS disk number parsing has been removed: the syntax drivemap hd1 0x80 is no longer legal. However, one can still map non-existent BIOS disk numbers with drivemap hd0 hd42 for example and (more controversially, maybe I'll add a check eventually) assign a floppy to an HD and back. Ah good :-) The only file changed from the last patch (version 6) is drivemap.c: the rest of it should be the same, so if anyone was reviewing it you can seamlessly jump to version 7. In particular, the functional changes are localized in the drivemap_cmd function proper, and there are cosmetic changes elsewhere (spurious tabs removed, etc.). You only forgot the changelog entry ;-) Oops, sorry... the only addition would be the new file drivemap.h, so the final (I hope) version would be like this: 2008-08-13 Javier Martin [EMAIL PROTECTED] * commands/i386/pc/drivemap.c: New file. * commands/i386/pc/drivemap_int13h.S: New 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. * include/grub/i386/pc/drivemap.h: New file. * include/grub/loader.h (grub_loader_register_preboot): New function prototype. (grub_loader_unregister_preboot): Likewise. (grub_preboot_hookid): New typedef. * kern/loader.c (grub_loader_register_preboot): New function. (grub_loader_unregister_preboot): Likewise. (preboot_hooks): New variable. (grub_loader_boot): Call the list of preboot-hooks before the actual loader. WRT the code, all your concerns were taken care of (I keep forgetting the . in comments, sorry), and about this snippet: +static grub_err_t +grub_cmd_drivemap (struct grub_arg_list *state, int argc, char **args) +{ + if (state[OPTIDX_LIST].set) +{ + /* Show: list mappings. */ + if (!drivemap) +grub_printf (No drives have been remapped); + else +{ + grub_printf (Showing only remapped drives.\n); + grub_printf (BIOS disk #num GRUB device\n); BIOS disk #num? Can you give an example? The drivemap -l command lists all remapped drives. The format of the listing has been reworked in order to be both understandable for end-users and unambiguous for devs and power users. It is now something akin to: BIOS disk #num GRUB device FD #0 (0x00) (fd3) (...) FD #127 (0x7f) (hd15) HD #0 (0x80) (hd2) (...) HD #127 (0xff) (fd0) Of course, it only shows remapped drives, not the full 255 possibilities. Well, here is version 8 of the patch... hope you like it ;) -Habbit Index: commands/i386/pc/drivemap.c === --- commands/i386/pc/drivemap.c (revisión: 0) +++ commands/i386/pc/drivemap.c (revisión: 0) @@ -0,0 +1,434 @@ +/* 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/machine/drivemap.h +#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 + +#define MODNAME drivemap + +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}, + {swap, 's', 0, perform both direct and reverse
Re: [PATCH] Drivemap module
El mié, 13-08-2008 a las 15:00 +0200, Robert Millan escribió: Hi, Marco asked me to review this. So he finally got fed up of me... Understandable ^^ I haven't followed the earlier discussion, so if I say or ask something that was discussed before, please bear with me and just point me to that. On Wed, Aug 13, 2008 at 02:16:25PM +0200, Javier Martín wrote: + +#define MODNAME drivemap + [...] + grub_dprintf (MODNAME, Removing the mapping for %s (%02x), args[0], mapfrom); I don't think this MODNAME approach is a bad idea per se [1][2], but if we are to do it, IMHO this should really be done globally for consistency, and preferably separately from this patch. [1] But I'd use a const char[] instead of a macro to save space. Maybe significant space can be saved when doing this throurough the code! [2] In fact, I think it's a nice idea. Ok, so following your [1], what about replacing the define with... ? static const char[] MODNAME = drivemap; +/* Int13h handler installer - reserves conventional memory for the handler, + copies it over and sets the IVT entry for int13h. + This code rests on the assumption that GRUB does not activate any kind of + memory mapping apart from identity paging, since it accesses realmode + structures by their absolute addresses, like the IVT at 0 or the BDA at + 0x400; and transforms a pmode pointer into a rmode seg:off far ptr. */ +static grub_err_t +install_int13_handler (void) +{ Can this be made generic? Like install_int_handler (int n). We're probably going to need interrupts for other things later on. Or is this code suitable for i8086 mode interrupts only? Anyway, if it's made generic, remember the handler itself becomes suitable for all i386 ports, not just i386-pc (for directory placement). It _could_ be made generic, but the function as it is currently designed installs a TSR-like assembly routine (more properly a bundle formed by a routine and its data) in conventional memory that it has previously reserved. Furthermore, it accesses the real-mode IVT at its standard location of 0, which could be a weakness since from the 286 on even the realmode IVT can be relocated with lidt. Nevertheless, I don't think this functionality is so badly needed on its own that it would be good to delay the implementation of drivemap to wait for the re-engineering of this function. + drivemap_node_t *curentry = drivemap; We use 'curr' as a handle for 'current' in lots of places throurough the code (rgrep for curr[^e]). I think it's better to be consistent with that. Done. +/* Far pointer to the old handler. Stored as a CS:IP in the style of real-mode + IVT entries (thus PI:SC in mem). */ +VARIABLE(grub_drivemap_int13_oldhandler) + .word 0xdead, 0xbeef Is this a signature? Then a macro would be preferred, so that it can be shared with whoever checks for it. What is it used for, anyway? In general, I like to be careful when using signatures because they introduce a non-deterministic factor (e.g. GRUB might have a 1/64k possibility to missbehave). Sorry, it was a leftover from early development, in which I had to debug the installing code to see whether the pointer to the old int13 was installer: a pointer of beef:dead was a clue that it didn't work. Removed and replaced with 32 bits of zeros. +FUNCTION(grub_drivemap_int13_handler) + push %bp + mov %sp, %bp + push %ax /* We'll need it later to determine the used BIOS function. */ Please use size modifiers (pushw, movw, etc). What for? the operands are clearly unambiguous. As you can see with the xchgw %ax, -4(%bp) line, I only use them for disambiguation. Assembly language is cluttered enough - and ATT syntax is twisted enough as it is. In fact, given that the code is specific for i386, I'd like to rewrite this code in GAS-Intel syntax so that memory references are not insane: -4(%bp)? please... we can have a simpler [bp - 4]. Index: include/grub/loader.h === --- include/grub/loader.h (revisión: 1802) +++ include/grub/loader.h (copia de trabajo) @@ -37,7 +37,23 @@ /* Unset current loader, if any. */ void EXPORT_FUNC(grub_loader_unset) (void); -/* Call the boot hook in current loader. This may or may not return, +typedef struct hooklist_node *grub_preboot_hookid; + +/* Register a function to be called before the loader boot function. Returns + an id that can be later used to unregister the preboot (i.e. on module + unload). If ABORT_ON_ERROR is set, the boot sequence will abort if any of + the registered functions return anything else than GRUB_ERR_NONE. + On error, the return value will compare equal to 0 and the error information + will be available in grub_errno. However, if the call is successful that + variable is _not_ modified
Re: [PATCH] Drivemap module
Ok, making a mixup reply... El mié, 13-08-2008 a las 17:14 +0200, Robert Millan escribió: On Wed, Aug 13, 2008 at 04:28:24PM +0200, Javier Martín wrote: I don't think this MODNAME approach is a bad idea per se [1][2], but if we are to do it, IMHO this should really be done globally for consistency, and preferably separately from this patch. [1] But I'd use a const char[] instead of a macro to save space. Maybe significant space can be saved when doing this throurough the code! [2] In fact, I think it's a nice idea. Ok, so following your [1], what about replacing the define with... ? static const char[] MODNAME = drivemap; Yes, but I'd merge this change separately from your drivemap patch (either before or after, as you prefer), for the whole of the codebase. Doesn't make sense to do it in some places but not in others, IMHO. Urgh... It's already been implemented in drivemap, why not put it in with it, then change everything else? Or should I just keep the #define in the meantime? It _could_ be made generic, but the function as it is currently designed installs a TSR-like assembly routine (more properly a bundle formed by a routine and its data) in conventional memory that it has previously reserved. Furthermore, it accesses the real-mode IVT at its standard location of 0, which could be a weakness since from the 286 on even the realmode IVT can be relocated with lidt. Nevertheless, I don't think this functionality is so badly needed on its own that it would be good to delay the implementation of drivemap to wait for the re-engineering of this function. Fair enough. The addr=0 assumption sounds troubling, though. Why not use sidt instead? Well, so is the assumption that GRUB does not enable any kind of paging or memory remapping for that matter. WRT sidt, I think that could be better implemented as a function in kern/i386/pc called grub_machine_get_rmove_ivt() or SLT, because it requires dropping to rmode, executing sidt, going back to pmode and then returning the address, modified to be valid in pmode as required. This approach would cost a few bytes in kernel (I can hear you screaming already), but it would be extensible for the future interrupt installer you envisioned, and it would take care of the paging/mappings if there ever were any. Same for a function to get the address of the BDA or other machine-specific addresses. +/* Far pointer to the old handler. Stored as a CS:IP in the style of real-mode + IVT entries (thus PI:SC in mem). */ +VARIABLE(grub_drivemap_int13_oldhandler) + .word 0xdead, 0xbeef Is this a signature? Then a macro would be preferred, so that it can be shared with whoever checks for it. What is it used for, anyway? In general, I like to be careful when using signatures because they introduce a non-deterministic factor (e.g. GRUB might have a 1/64k possibility to missbehave). Sorry, it was a leftover from early development, in which I had to debug the installing code to see whether the pointer to the old int13 was installer: a pointer of beef:dead was a clue that it didn't work. Removed and replaced with 32 bits of zeros. Ok. +FUNCTION(grub_drivemap_int13_handler) + push %bp + mov %sp, %bp + push %ax /* We'll need it later to determine the used BIOS function. */ Please use size modifiers (pushw, movw, etc). What for? the operands are clearly unambiguous. For consistency (I'm so predictable). We do the same everywhere else. Also, I think some versions of binutils reject instructions that don't have size qualifiers. Version 0.1 (alpha 2) maybe? IIRC, it's been long since the size qualifiers inference is available in GAS. And it's more readable for people used to gas (I know, it's also less readable for people used to tasm or so). TASM? Don't even name the devil. I like GAS and its directives (even better and NASM and YASM), but many of the conventions of the ATT syntax are at best broken (i.e. src, dest looks good, but breaks on FP instructions), and memory references are a royal PITA. Given that the code is _not_ platform-portable and that a PPC dev will not understand it even in ATT syntax, why bother with it... However, I take it that your advertised predictability means that I should consider the int13h in intel syntax request denied. Nevertheless, I've attached the two versions of the asm file, so you can check which one is less of a mind boggler. Both assemble to _exactly_ the same machine code (checked with gcc + objdump). El mié, 13-08-2008 a las 17:57 +0200, Marco Gerards escribió: Robert Millan [EMAIL PROTECTED] writes: [...] This is a lot of code being added to kernel, and space in kernel is highly valuable. Would the same functionality work if put inside a module? For the reasons discussed above in the loader.h snippet, I don't think
[RFC] Platform information services
Hi there everybody, I'm opening the RFC because I want to add some kind of infrastructure to retrieve the address of system/platform structures. I will explain myself: my use case is in i386-pc and for the drivemap module, in which a function installs a TSR int13h handler. This requires the function to have access to two real mode structures, namely the BIOS Data Area, which is based at 0040:h; and the Interrupt Vector Table, which conventionally starts at 0 but that could have been placed elsewhere by the use of the LIDT instruction. Currently, the code just hopes for the best and accesses 0x0 and 0x400 directly as protected mode addresses from within the GRUB environment, which has the additional assumption that GRUB has not enabled any kind of paging or memory mapping. Obviously, the Right Way for this would be for the code to check where its targets are, but even when the location of the IVT can be queried by the non-privileged instruction SIDT, that would require a privileged trip to real mode and back from it in order to query the location of the real mode IVT instead of the pmode IDT that is in effect in the GRUB environment (but this still does not deal with a possible paging). Thus, the best course of action I see would be to add a platform information infrastructure in kernel (that last word has probably put me on the death list of several people here ¬¬). My idea would add a cross-platform header platform.h declaring the following prototype: void* grub_machine_get_platform_structure (int stidx); Then, the machine.h file for each platform would declare which structures are available, like the IVT and the BDA for i386-pc. The code would be used like this: #include grub/platform.h #include grub/machine/machine.h /* (...) Now from within a function */ grub_uint8_t* bda = (grub_uint8_t*)grub_machine_get_platform_structure (GRUB_MACHINE_I386_BDA); if (!bda) /* Error info in errno errmsg - bail out */ else grub_printf (Avail mem: %d KiB, *((grub_uint16_t*)(bda + 0x13))); The pointers provided by this new function would be guaranteed to: - Be able to access the whole requested structure (if any segmentation is in effect, the whole struct is addressable from that base address) - Have been mapping-marshaled, i.e. if any kind of paging has been enabled, the address returned can be directly used as a C pointer. - Be read-write? Maybe this could be requested through an additional flags parameter to the function... Initially this addition would only benefit i386-pc and in particular my drivemap patch, but maybe it can also be used by modules to query EFI info structures and such... What are your thoughts on the matter? (fanatics with death threats for trying to add something to kernel, please abstain) -Habbit 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: grub-probe detects ext4 wronly as ext2
Hi there, After reading Felix's reply I've once again found your post and implemented your request, so here is a new version of the patch (version 6). Sorry for missing your message in the first instance... u_u 2008/7/19 Robert Millan [EMAIL PROTECTED]: On Sat, Jul 05, 2008 at 08:36:13PM +0200, Javier Martín wrote: grub_disk_read (disk, 1 * 2, 0, sizeof (struct grub_ext2_sblock), (char *) data-sblock); if (grub_errno) -goto fail; +EXT2_DRIVER_MOUNT_FAIL(could not read the superblock) This overrides the grub_errno and grub_errmsg provided by grub_disk_read and replaces them with values that hide the true problem. If there was a disk read error, we really want to know about it from the lower layer. Well, the old version did just the same (even worse, because the message was generic). What would be the correct path of action here? I mean, how can we propagate the error messages? It shouldn't call grub_error(). So in the cases the fail is caused by an underlying error (like I/O) the code should just return failure and leave the old error in errno and errmsg? OK then, II adapted the code to cope with this: now it only raises its own errors when appropriate. fail: - grub_error (GRUB_ERR_BAD_FS, not an ext2 filesystem); + if (!err_msg) +err_msg = DEBUG: mount failed but no error message supplied!; No need to check for consistency in your own code. This might be a good practice in userland programs but here it's a waste of space. Just make sure your code is correct. Ok, removed this particular check (but now there is another one to check whether raising a local error is required). -Habbit -- Robert Millan GPLv2 I know my rights; I want my phone call! DRM What good is a phone call… if you are unable to speak? (as seen on /.) ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel ext2_incompat.patch.6 Description: Binary data ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
Re: grub-probe detects ext4 wronly as ext2
El mié, 06-08-2008 a las 12:36 +0200, Felix Zielcke escribió: Am Dienstag, den 05.08.2008, 19:23 +0200 schrieb Felix Zielcke: Am Freitag, den 04.07.2008, 03:20 +0200 schrieb Javier Martín: That was it. I will post no more in this thread. Do whatever you please with the patch - I'll just request some more people from the GRUB dev team to review the thing, instead of the tennis match we've had here (and I appreciate all matches, even the ones I lose). I'd like to bring this topic now up again and yes I know this isn't the last message about it :) Maybe it helps more if I give you a link to the thread start on the archive if you want to read through the whole story again ;) [0] The last mails aboit this was only between Javier and me about which flags should be ignored and which should be marked as supported. Robert was the only one from the official's who commented on the code and from his is even the last message about the topic actually [1]. I really think that it's a good idea. For example currently there exists INCOMPAT_64BIT which only the kernel currently supports but not the e2fsprogs. AFAIK it's probable used for filesystems = max ext3 size, the german Wikipedia ext3 article says 32 TiB the english one 16 TiB Anyway if you use such a real big filesystem in the future even for /boot then in the beginning the /boot stuff is probable at the very beginning of it, but with the time you probable want to make use of it. And then maybe update once the kernel which then probable moves to an area which needs 64bit inode support or whatever this 64bit are used for. Then I think it's better to refuse to install grub to it (e.g. by failing grub-probe) then people leaving in the uncertainness that they may not be able anymore to boot this system. Ok probable nobody ever uses such a big filesystem for their /boot too, but as Javier already said in the thread: There's maybe an ext5, ext6 and so on. By the way: I suggest to rename ext2.mod to extN.mod, on IRC there was already a guy who wondered why it says ext2 instead of ext3 which he had and ext4 extents are now supported which will probable never backported to ext2. [0] http://lists.gnu.org/archive/html/grub-devel/2008-07/msg8.html [1] http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00333.html Thanks for raising the topic again. If it serves any purpose, I'll say that the last patch I sent (version 5) is still valid against the current HEAD (rev. 1798) -Habbit ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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] Drivemap module
In this reply-to-myself hoping to keep the thread continuity, I put forth the new version 7 of the patch with the following changes: - A new switch -s/--swap has been implemented, so that running drivemap -s hd0 hd1 is equivalent to issuing the command twice with the arguments normal, then reversed. There is one exception: if the first mapping fails or the second drive does not exist, no mapping is performed, whereas hand-performing the swap would have successfully assigned BIOS disk #1 to hd0, then failed to assign bd#0 to non-existent hd1. - Raw BIOS disk number parsing has been removed: the syntax drivemap hd1 0x80 is no longer legal. However, one can still map non-existent BIOS disk numbers with drivemap hd0 hd42 for example and (more controversially, maybe I'll add a check eventually) assign a floppy to an HD and back. The only file changed from the last patch (version 6) is drivemap.c: the rest of it should be the same, so if anyone was reviewing it you can seamlessly jump to version 7. In particular, the functional changes are localized in the drivemap_cmd function proper, and there are cosmetic changes elsewhere (spurious tabs removed, etc.). -Habbit Index: commands/i386/pc/drivemap.c === --- commands/i386/pc/drivemap.c (revisión: 0) +++ commands/i386/pc/drivemap.c (revisión: 0) @@ -0,0 +1,433 @@ +/* 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/machine/drivemap.h +#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 + +#define MODNAME drivemap + +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}, + {swap, 's', 0, perform both direct and reverse mappings, 0, 0}, + {0, 0, 0, 0, 0, 0} +}; + +enum opt_idxs { + OPTIDX_LIST = 0, + OPTIDX_RESET, + OPTIDX_SWAP, +}; + +typedef struct drivemap_node +{ + grub_uint8_t newdrive; + grub_uint8_t redirto; + struct drivemap_node *next; +} drivemap_node_t; + +static drivemap_node_t *drivemap; +static grub_preboot_hookid insthandler_hook; +static grub_err_t install_int13_handler (void); + +/* Puts the specified mapping into the table, replacing an existing mapping + for newdrive or adding a new one if required. */ +static grub_err_t +drivemap_set (grub_uint8_t newdrive, grub_uint8_t redirto) +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = drivemap; + while (search) +{ + if (search-newdrive == newdrive) +{ + mapping = search; + break; +} + search = search-next; +} + + + /* Check for pre-existing mappings to modify before creating a new one. */ + if (mapping) +mapping-redirto = redirto; + else +{ + mapping = grub_malloc (sizeof (drivemap_node_t)); + if (!mapping) +return grub_error (GRUB_ERR_OUT_OF_MEMORY, + cannot allocate map entry, not enough memory); + mapping-newdrive = newdrive; + mapping-redirto = redirto; + mapping-next = drivemap; + drivemap = mapping; +} + return GRUB_ERR_NONE; +} + +/* Removes the mapping for newdrive from the table. If there is no mapping, + then this function behaves like a no-op on the map. */ +static void +drivemap_remove (grub_uint8_t newdrive) +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = drivemap; + drivemap_node_t *previous = 0; + + while (search) +{ + if (search-newdrive == newdrive) +{ + mapping = search; + break; +} + previous = search; + search = search-next; +} + + if (mapping) +{ + if (previous) +previous-next = mapping-next; + else /* Entry was head of list. */ +drivemap = mapping-next; + grub_free (mapping); +} +} + +/* Given a device name, resolves its BIOS disk number and stores it in the + passed location, which should only be trusted if ERR_NONE is returned. */ +static grub_err_t +parse_biosdisk (const char *name, grub_uint8_t *disknum) +{
Re: Windows boot
Hi again, 2008/8/7 Viswesh S [EMAIL PROTECTED]: Hi, I installed ubuntu in the same harddisk(hda) as windows. After installing ubuntu, using grub legacy, I was able to log in to windows, using the entries, rootnoverify and chainloader +1. But when I install grub2 and then modify the grub.cfg with the following, statements, it is not working, says ...A disk read error occurred ,Press Ctrl+Alt+Del . This is my grub.cfg menuentry Windows { set root=(hd0,3) chainloader +1 } My windows file sytem is in NTFS and not vfat One more thing to check with is, the need of ntfs mod present as part of grub2.So where exactly that is getting used,though I have tried to load the module ntfs and ntfscomp in grub using insmod in grub menu,...but no positive results. The FS only matters if you try to access files on the volume, but chainloader just reads raw sectors from disk, so the FS module does not matter: you could have an SFS partition for the sake of it and it would work if the superblock was executable. What might be in grub2, which prevents windows to boot. Hmm... you could try booting (hd1) instead of (hd1,1), i.e. Windows' MBR instead of its boot sector directly. I'll be back in town tomorrow night and then I'll be able to look at my own GRUB config file, but Windows has always been a wild beast to master. For the record, my tests were with Windows XP Home in an Athlon X2 and with Windows XP x64 on an Athlon 64. Thus, I don't know if Intel CPUs triple-fault on my code (improbable, because it runs on QEMU) or if Windows Vista (which has a changed boot process) works with it. Viswesh Unlimited freedom, unlimited storage. Get it now ___ 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: [PATCH] Drivemap module
El mié, 06-08-2008 a las 07:43 -0700, Viswesh S escribió: Hi, Could you let all know how to use your drivemap command.Is it similar to the map command in legacy grub. Hi there. Information about the use of a command is better placed in the wiki than in this list, however this command is not merged in yet, and I reckon its help feature could be quite better. Currently, the safe GRUB2 commands for booting from (hd1,1) is: drivemap (hd0) (hd1) drivemap (hd1) (hd0) set root=(hd0) chainloader (hd1,1)+1 boot Maybe an explanation of what drivemap does would put a bit of light here: * The BIOS hard disks are numbered from 0 to 128 (actually from 0x80 to 0xFF because the lower numbers are reserved for floppy disks). These numbers are used to select the target drive when issuing I/O requests through the BIOS routines (INT 13h) * When the BIOS loads a boot sector and transfers control to it, the DL register contains the boot disk number, i.e. the disk from which the bootsector was loaded. This allows the bootsector to load its OS from the same disk it was run, instead of having to probe every single disk in the system. * The chainloader command works like the BIOS: it loads a bootsector into memory and jumps to it. In this case, the value in DL corresponds to the disk that was set as root drive to GRUB. If no root drive is set, the OS receives 0xFF, which should be recognized as an impossible drive. Some OSes will just trust this and fail (i.e. FreeDOS) while others will try to boot from the first hard disk (0x80). * The drivemap command acts as the old TSRs from the DOS times: when the boot command is issued, it installs its own handler for INT 13h requests, which performs the requested mappings and then call the old (usually BIOS) handler with the changed parameters. Thus, an OS accessing the drive through the BIOS routines will see the drives moved, duplicated or whatever the mappings provided. Again: drivemap does NOT modify the live drive mappings within GRUB; its changes only affect the booted OS. The strange root=(hd0) line that appears to contradict the chainloader line is there because drivemap has no communication with the particular loader. If you set root to (hd1,1) and then issue chainloader and boot, the OS will receive 0x81 in DL because hd1 was the second hard drive when chainloader was issued (remember that drivemap doesn't act until boot). Thus, the target OS will try to boot from what _it_ sees as the second hard disk, which will now be the old hd0 - wrong. If the target OS does not need to access the old hd0 or it only uses the BIOS routines for the boot process (i.e. it later uses ATA drivers and will redetect everything from scratch), you can leave the second drivemap line out and use a more compact script like this: drivemap (hd0) (hd1) set root=(hd1,1) chainloader +1 boot This will work on Windows, because no matter where it tries to boot from it will find the same disk: both the first and second BIOS disks point to hd1 now. However, if you use a DOS-like system which uses the BIOS routines exclusively (i.e. FreeDOS) then your hd0 disk would have disappeared to it and you'd have D: to be a mirror of C:. In order to have hd0 as D:, hd1 as C: and everything working, you need the first script I posted, which makes the complete swap and then makes FreeDOS load from the first hard drive (i.e. hd1). This is the setup I have extensively tested in QEMU, while the Windows boot has been tested in both QEMU (with ReactOS) and some random computers including mine (with Windows XP Home and Windows XP Pro x64). I'm looking forward to streamline the drivemap syntax, so that the two drivemap lines can be fused into one like drivemap --swap (hd0) (hd1). However, given that it's not a bug and that GRUB is still in heavy development (and thus syntax changes are acceptable), I'd prefer to have the patch integrated as it is - so we have the functionality - and then modify what's needed - so we have it pretty. -Habbit 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: Encryption Support for GRUB
El mié, 06-08-2008 a las 10:11 -0700, Colin D Bennett escribió: On Wed, 6 Aug 2008 11:04:16 -0500 (CDT) W. Michael Petullo [EMAIL PROTECTED] wrote: 1. How do I know exactly what subset of libc is available to me as a GRUB developer? Obviously, system calls would not be expected to work because the operating system has not yet been loaded, but I would expect libmath routines to be usable by GRUB. What about other libraries? No libc is available. Only functions implemented by GRUB itself are available. See ``kern/misc.c`` and ``include/grub/misc.h`` in the GRUB 2 source tree for implementations of the most important things that are normally provided by libc, such as strcpy (grub_strcpy), memcmp (grub_memcmp), etc. Maybe we should separate those headers in ANSI-like files, following a structure like: include/grub/gstdlib.h - grub_malloc et al include/grub/gstdio.h - grub_printf and friends etc Of course, most of them would not be complete (i.e. gstdlib.h would not have process control functions) but at least we'd separate support functions from pure GRUB and questions like the OP's would be fast to answer: look at gstd*.h. Furthermore, such a separation would force us to commit to a semi-permanent specification of which subset of libc is available to module writers, which is good if people wants to write and maintain modules outside the GRUB tree. -Habbit link to online svn for misc.c: http://svn.savannah.gnu.org/viewvc/trunk/grub2/kern/misc.c?revision=1774root=grubview=markup GRUB implements dynamic memory allocation through grub_malloc, grub_free, grub_realloc. See ``kern/mm.c``. No math library is available, but I think you could create a 'math' module in GRUB and implement the required math functions there. The main thing is to keep the GRUB core small. It needs to fit in 32 KB, I think. GRUB has its own file I/O api (no stdio -- instead use grub_file_open, grub_file_read, etc.). If you want to see how to use the GRUB library stuff, look at some of the built in commands such as ``commands/ls.c``, etc. Regards, Colin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel 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: Remove conf/*.mk from svn
El mar, 05-08-2008 a las 17:48 +0200, Marco Gerards escribió: Vesa Jääskeläinen [EMAIL PROTECTED] writes: Marco Gerards wrote: Colin D Bennett [EMAIL PROTECTED] writes: I think we should remove conf/*.mk from the Subversion repository. If people are going to be developing on GRUB and checking out svn branches, then I think it's fine to require them to have Ruby. For released tarballs that we expect non-developers to use, we just need to generate the *.mk files and include them in the tarball. I do not have problems with this. Besides this, it will stop people from sending in patches with .mk changes in it :-) I think Okuji's objection is based on fact that it makes it harder for people to compile from sources. Now what if we would generate those files when making a release? Of course this should be enabled to script/makefile to make it automatically so it is not forgotten ;) Right. Just to be clear, personally I didn't have these objections but Okuji has. Actually, since ruby is required to generate these files, I guess we can better keep the .mk files. Why not rewrite genmk.rb in a more common language (i.e. with an interpreter more commonly found in stock GNU installs) like Python or Perl? Habbit 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] Drivemap module
Hi there and thanks for playing The Game! El lun, 04-08-2008 a las 20:50 -0400, Isaac Dupree escribió: Javier Martín wrote: You understand my concern, but seemingly do not understand that in order to conform to the Holy Coding Style you are asking me to write code that can become buggy (and with a very hard to trace bug) with a simple deltion! (point: did you notice that last word _without_ a spelling checker? Now try to do so in a multithousand-line program). well, maybe a bit off topic. But I can't imagine how, after code is written, I could accidentally delete an = character even when editing it. I prefer the (to me) intuitive meaning of (variable == value) in my own code. That particular problem has never bitten me. Although, in C++ coding style, a lot more local variables are const and therefore the error could be caught by the compiler anyway. It seems like an odd paranoia to choose. Say, take uint32_t. It's only a one-character deletion to become int32_t and then there is subtle breakage. htons and many other functions with similar names and suffixes. Etc.? It's half C language and culture, and half inevitable in programming, IMHO. Sigh... Maybe I'm just a bit paranoid because I have been bitten by similar problems (not in ifs, but mainly in loop condition checks) and they were painstakingly difficult to hunt down even in a good IDE like Netbeans: I sometimes considered the possibility of my computer having lost all sense of logic because I saw it singlestep into the wrong branch until I noticed the = instead of ==. True, the possibilities of causing such a change are very low. point[2]: I half did notice the typo (only half because I've trained myself not to be too distracted by people's spelling), and I'm generally more precise when looking at code (maybe). ;-) So I thought, but even my own code dodges me at times, not to speak of others' code... Anyway, since they are more likely to maintain the code in the long run than you, in general, the question is whether the code is more likely to become buggy by their hacking on it, if it follows project coding style or someone else's (your) safer coding style. Likely it's safer if using a consistent programming style. Although I personally don't see that it's very helpful to have a style that makes things down to the order of == arguments be consistent within the project; haphazard only slows reading the tiniest bit, and I don't think it makes a different what order the arguments are... Hmm... I was partially expecting a flamefest to start. Pity ^^ Well, let's spill a little napalm: the GNU style bracing is extremely silly!! Why the hell are the if and else blocks indented differently in this example? if (condition) return 0; else { return -1; } Nah, I'm not really bringing that issue, I was just joking, and in fact I'm reconsidering my objections to the operator== arguments order rule, even though I still consider my style safer and more sensible. If someone else wants to express their opinion on that issue, do it fast before I completely submit to Master Marco's will :D Habbit 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] Drivemap module
After your latest replay, I reevaluated my stubbornness WRT some of your advices, and I've changed a few things: - Variables are now declared (and, if possible, initialized) before precondition checks, even simple ones. The install_int13_handler function has not been modified, however, since I find it a bit nonsensical to put bunch of declarations without an obvious meaning just after the else line: grub_uint32_t *ivtslot; grub_uint16_t *bpa_freekb; grub_size_t total_size; grub_uint16_t payload_sizekb; grub_uint8_t *handler_base; int13map_node_t *handler_map; grub_uint32_t ivtentry; - Only one declaration per line: even though C is a bit absurd in not recognizing T* as a first class type and instead thinking of * as a qualifier to the variable name; and even though my code does not incur into such ambiguities. - Comments moved as you required, reworded as needed - Extensive printf showing quasi-help in the show mode trimmed down to just the first sentence. - Internal helper functions now use the standard error handling, i.e. return grub_error (err, fmt, ...) - Comment about the strange void type instead of void* rephrased to be clearer There is, however, one point in which I keep my objection: comparisons between a variable and a constant should be of the form CONSTANT == variable and not in the reverse order, since an erroneous but quite possible change of == for = results in a compile-time error instead of a _extremely_ difficult to trace runtime bug. Such kind of bugs are quite excruciating to find in userspace applications within an IDE, so I can't even consider the pain to debug them in a bootloader. WRT accepting raw BIOS disk numbers, I agree with you in principle, but I'm keeping the functionality for now, since I don't quite like the drivemap (hd0) (hd1) syntax - which device is which?. I'd rather have something akin to drivemap (hd0) (bios:hd1), but I want to hear the opinions of people in this list. The new version of the patch is attached, and here is my suggested CLog: 2008-08-XX Javier Martin [EMAIL PROTECTED] * commands/i386/pc/drivemap.c : New file. * commands/i386/pc/drivemap_int13h.S : New 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 * include/grub/loader.h (grub_loader_register_preboot) : New function prototype. Register a new pre-boot handler (grub_loader_unregister_preboot) : Likewise. Unregister handler (grub_preboot_hookid) : New typedef. Registered hook handle * 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 Index: commands/i386/pc/drivemap.c === --- commands/i386/pc/drivemap.c (revisión: 0) +++ commands/i386/pc/drivemap.c (revisión: 0) @@ -0,0 +1,417 @@ +/* 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 + +#define MODNAME drivemap + +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} +}; + +/* Syms/vars/funcs exported from drivemap_int13h.S - start. */ + +/* Realmode far ptr = 2 * 16b */ +extern grub_uint32_t grub_drivemap_int13_oldhandler; +/* Size of the section to be copied */ +extern grub_uint16_t grub_drivemap_int13_size; + +/* This type is used for imported assembly labels, takes no storage and is only + used to take the symbol address with label. Do NOT put void* here. */ +typedef void grub_symbol_t; +extern grub_symbol_t grub_drivemap_int13_handler_base; +extern grub_symbol_t
Re: Is it possble to use /dev/disk/by-id/ in grub.conf?
2008/7/30 Aniruddha [EMAIL PROTECTED]: On Wed, 2008-07-30 at 12:35 -0400, Pavel Roskin wrote: I believe disk IDs are not supported yet. If you describe your problem in more details, it may motivate somebody to add disk ID support. Or perhaps you are missing a simple solution possible with the current code. Thanks for your quick response. The problem is that udev keeps changing my harddrive names at each boot. Normally my root partition resides on /dev/sdc2. However with each reboot this name changes (the last three boots it changed from /dev/sdb2 to /dev/sdf2 to /dev/sdc2). I solved this partially by changing the /dev/sdc2 entry in fstab with a /dev/disk/by-id/my_device_id entry. Unfortunately grub has still /dev/sdc2 configured as root partition. This means I have to guess every boot which partition-name the root partition resides. I do this by entering devices names (after the error message that grub couldn't find my root partition). I start with /dev/sda2 trying each letter (sdb2, sdc2, sdd2) until I find the root partition. If your problem is what device name to pass to the kernel command line, you can use a line like: linux /vmlinuz root=UUID=diskuuidhere and the same with LABEL= if your initrd supports them (like Ubuntu's default initrd or Gentoo's `genkernel' initrd). If, on the other hand, your problem is what device to use in GRUB, I think there is a `search' command you can investigate. In order to solve this problem I would like to set the /dev/disk/by-id/ parameter in my grub.conf. I hope that explains the problem clearly. Thanks in advance! Regards, Aniruddha ___ 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: Is it possble to use /dev/disk/by-id/ in grub.conf?
2008/7/30 Aniruddha [EMAIL PROTECTED]: On Wed, 2008-07-30 at 20:00 +0200, Javier Martín wrote: If your problem is what device name to pass to the kernel command line, you can use a line like: linux /vmlinuz root=UUID=diskuuidhere and the same with LABEL= if your initrd supports them (like Ubuntu's default initrd or Gentoo's `genkernel' initrd). If, on the other hand, your problem is what device to use in GRUB, I think there is a `search' command you can investigate. If I understand you correctly I have to add the following line to my grub.conf: title=Gentoo Linux (2.6.25-gentoo-r6) root (hd0,1) kernel /boot/kernel-genkernel-x86-2.6.25-gentoo-r6 root=/dev/ram0 init=/linuxrc ramdisk=8192 real_root=LABEL=my_label udev Oh, sorry, I didn't realize you were talking about GRUB Legacy (this list is mostly about the development of GRUB 2). I think you are missing some initrd (GRUB2) or module (GRUB1?) line after the linux (GRUB2) or kernel (GRUB1) line - and the genkernel line should not be in the GRUB config files at all. The exact boot parameters and configuration should be in the Gentoo documentation: if I remember my Gentoo times correctly, the Handbook should have all the info you need. If you mail me a link to the post related to this issue in the Gentoo forums, I can answer you better than in this list. genkernel --install --bootloader=grub --menuconfig --disklabel all Furthermore I have to build genkernel with the --disklabel option to make it work right? genkernel --install --bootloader=grub --menuconfig --disklabel all Thanks, Aniruddha ___ 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: Windows
2008/7/30 Viswesh S [EMAIL PROTECTED]: The problem is that, I am not able to boot windows by this configuration. I am getting the error A disk read Error Occurred If I make the windows disk as the first boot device,then windows is coming up properly. Drivemap module to the rescue!! ^^ Hoping to resurrect the discussion about it, I developed the module for just that purpose: I have a similar configuration, in which Windows refuses to boot if its drive is not the first BIOS drive. In some cases and versions of Windows you can make it work without the drivemap module with: menuentry Windows { set root=(hd1,1) chainloader +1 } But not all versions of NTLDR obey the boot drive parameter passed in DL: in GRUB legacy, this was solved by the map command which made Windows believed it was booting from the first BIOS drive by intercepting all BIOS I/O calls and mapping the drive numbers - the proposed drivemap module does the same in GRUB2, but it's not been accepted yet (ahem xD). Hence I feel this is something to do with the grub2 configuration which is missing. If any needs any information(about my machine) to configure grub2 for windows,please let me know. Regards, Viswesh - Original Message From: Pavel Roskin [EMAIL PROTECTED] To: The development of GRUB 2 grub-devel@gnu.org Sent: Wednesday, 30 July, 2008 11:39:56 PM Subject: Re: Windows On Wed, 2008-07-30 at 11:00 -0700, Viswesh S wrote: menuentry Windows { chainloader (hd1,1)+1 } Is all this is sufficient or is there anything else I should add. I think you are better off describing the problem, not asking others to guess what could be wrong without knowing what the problem is. -- Regards, Pavel Roskin ___ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel Add more friends to your messenger and enjoy! Invite them now. ___ 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: [PATCH] File readahead buffering
El mié, 23-07-2008 a las 10:05 +0800, Bean escribió: On Wed, Jul 23, 2008 at 3:06 AM, Colin D Bennett [EMAIL PROTECTED] wrote: On Tue, 22 Jul 2008 14:48:31 -0400 Pavel Roskin [EMAIL PROTECTED] wrote: However, I have not been able to load any PNG images that I have tried to use. Something about the chunk type not being supported. Hi, Please upload the png file that cause problem. Also note that png use DEFLATE compression. I write a decoder in png, which may be a little slow. Perhaps I can import the decoder from zlib, if copyright allows. Does the GRUB decoder distinguish between critical and ancillary chunks? A decoder finding a critical chunk that it does not understand should abort, but finding an unknown ancillary chunk is not an error - maybe his file has an ancillary chunk like zTXt and the GRUB png reader is rejecting it as having unknown chunks. I didn't even take a look at the decoder code so I could be dead wrong, but I think it's a possibility worth investigating... 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: Putting core.img anywhere
El lun, 21-07-2008 a las 11:13 +0200, Jean-Christophe Haessig escribió: Le dimanche 20 juillet 2008 à 17:06 +0200, Javier Martín a écrit : Hi, 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. As I stated earlier my disks are not DOS-partitioned (e.g. pvcreate /dev/hda). I didn't want to have one useless level of the old DOS partition scheme since LVM already does the job (better). Well, I can't dissent on that, but then your whole HD, from the first to the last LBA block, is full with data that can move at LVM's whim. 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? Short answer : yes. Longer answer : LVM does not (yet) gratuitously move LVs among PVs, LVs stay where they have been created and when you create a LV on an empty PV, it is normally allocated at the beginning of the disk in continuous extents. And yes, using pvmove you can move data anywhere you like. You can move a LV to a certain PV, but apart from that there is not a point in the LVM specification or documentation that I've seen that can allow you to move data into _specific_ PEs _and_ keep it there: pvmove does the former but does not guarantee they will still be there in, say, a week: the only way I can think of would be to tie the /boot LV to a PV near the start of the disk, but as you said, then you wouldn't be in this dilemma. 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. That's why I included the random-number-chain feature. If the file is accidentally moved, it can still be found by the bootsector. Additionnally, GRUB could display a message about this to the user, telling him that it is in degraded mode and that he should re-run some utility to reindex the file. As for reading the entire disk, an arbitrary upper limit could be put to force the boot sector to fail. First of all, LVM2 LVs don't have to keep consistency iIrc: its PEs can be (though usually aren't) scattered all over the VG PVs, which means that part of core.img could be in LBA blocks 300-330 but the other fragment might be in blocks 814567-814592. In the worst case, your system might have to read the whole disk _once per block_ in core.img, which is about 50 blocks long. That might be a _big_ hit. If that weren't enough, all your system would have to fit in the already-cramped bootsector image, since you said that there is not a single block in the HD available for embedding. In fact, do you know if LVM leaves space in block #0 (its superblock) for boot code like GRUB's? If not, you can't even install GRUB, since there is no place to install it to. A suggestion: if you want to keep your no-partitions schema, why don't you boot from a floppy, a CD or a USB pendrive? You would save a lot of headaches... 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. Sure, but who would use NTFS for their /boot ? Most filesystems should work already, I think, otherwise even LILO couldn't work… The same kind of geek* that does not partition his hard drives ;) Besides, I was only using NTFS compression as an example: extN filesystems also have a compression feature (though it's not very used). Any other kind of inline addition/alteration of the data on store by the FS (like the addition of a checksum each 128 bytes or so) or the breakage of the assumption that _our_ blocks are still block-aligned on the filesystem would break your system too. Nevertheless, those assumptions, particularly the latter, usually hold, so your system is already quite robust as things go. Cheers! Habbit * I did the same once, formatting a whole disk reiserfs for a temporary storage addition. I've also partitioned an OLD pc laptop with GPT, so yeah, I kinda like experimenting with partitions 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] Kernel fixes for Cygwin
El lun, 21-07-2008 a las 12:33 +0200, Christian Franke escribió: Bean wrote: BTW, if you have time, you can consider writing a tool that convert pe to elf directly, thus avoiding objcopy altogether. This shouldn't be too difficult, you can take a look at util/i386/efi/grub-mkimage.c, which does exactly the opposite, converting elf to pe32. due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here. Why do we even consider a PE-ELF converter? I think the easier way to go would have the people building GRUB in cygwin (not exactly newbies) to have an i386-pc-elf cross compiler built first, then use that for the bootloader programs and the normal gcc for tools. Even a naked (i.e. libraryless) cross compiler would work, since the bootloader part of GRUB is does not need libs (in C terminology, it's freestanding). That way, we are free from objcopy bugs or BFD design limitations. This scheme is kinda like what's done with x86_64-pc-linux, with the difference that in that case we use the same gcc for both host and target by adding -m32 to the latter, but it's essentially the same concept. Habbit 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] Drivemap module
El lun, 21-07-2008 a las 02:55 +0200, Javier Martín escribió: (...) Phew! That was long, even after removing some parts. Well, this is the new version of the patch. Cheers! As always, I'm so stupid that I left the patch out... Sigh. Here it is. Habbit Index: commands/i386/pc/drivemap.c === --- commands/i386/pc/drivemap.c (revisión: 0) +++ commands/i386/pc/drivemap.c (revisión: 0) @@ -0,0 +1,393 @@ +/* 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 + +#define MODNAME drivemap + +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} +}; + +/* Syms/vars/funcs exported from drivemap_int13h.S - start. */ + +/* realmode far ptr = 2 * 16b */ +extern grub_uint32_t grub_drivemap_int13_oldhandler; +/* Size of the section to be copied */ +extern grub_uint16_t grub_drivemap_int13_size; + +/* NOT a typo - just need the symbol's address with symbol. */ +typedef void grub_symbol_t; +extern grub_symbol_t grub_drivemap_int13_handler_base; +extern grub_symbol_t grub_drivemap_int13_mapstart; + +void grub_drivemap_int13_handler (void); + +/* Syms/vars/funcs exported from drivemap_int13h.S - end. */ + + +static grub_preboot_hookid insthandler_hook = 0; + +typedef struct drivemap_node +{ + grub_uint8_t newdrive; + grub_uint8_t redirto; + struct drivemap_node *next; +} drivemap_node_t; + +static drivemap_node_t *drivemap = 0; +static grub_err_t install_int13_handler (void); + +/* Puts the specified mapping into the table, replacing an existing mapping + for newdrive or adding a new one if required. */ +static grub_err_t +drivemap_set (grub_uint8_t newdrive, grub_uint8_t redirto) + +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = drivemap; + while (search) +{ + if (search-newdrive == newdrive) +{ + mapping = search; + break; +} + search = search-next; +} + + + if (mapping) /* There was a mapping already in place, modify it. */ +mapping-redirto = redirto; + else /* Create a new mapping and add it to the head of the list. */ +{ + mapping = grub_malloc (sizeof (drivemap_node_t)); + if (!mapping) +return grub_error (GRUB_ERR_OUT_OF_MEMORY, + cannot allocate map entry, not enough memory); + mapping-newdrive = newdrive; + mapping-redirto = redirto; + mapping-next = drivemap; + drivemap = mapping; +} + return GRUB_ERR_NONE; +} + +/* Removes the mapping for newdrive from the table. If there is no mapping, + then this function behaves like a no-op on the map. */ +static void +drivemap_remove (grub_uint8_t newdrive) +{ + drivemap_node_t *mapping = 0; + drivemap_node_t *search = drivemap; + drivemap_node_t *previous = 0; + while (search) +{ + if (search-newdrive == newdrive) +{ + mapping = search; + break; +} + previous = search; + search = search-next; +} + if (mapping) /* Found. */ +{ + if (previous) +previous-next = mapping-next; + else /* Entry was head of list. */ +drivemap = mapping-next; + grub_free (mapping); +} +} + +static grub_err_t +parse_biosdisk (const char *name, grub_uint8_t *disknum) +{ + if (!name) return GRUB_ERR_BAD_ARGUMENT; + if (*name == '(') +name++; /* Skip the first ( in (hd0) - disk_open wants just the name. */ + grub_disk_t disk = grub_disk_open (name); + if (!disk) +return GRUB_ERR_UNKNOWN_DEVICE; + else +{ + enum grub_disk_dev_id id = disk-dev-id; + if (disknum) +*disknum = disk-id; /* Only sound if it's a biosdisk - check later. */ + grub_disk_close (disk); + return (GRUB_DISK_DEVICE_BIOSDISK_ID != id) ? + GRUB_ERR_BAD_DEVICE : GRUB_ERR_NONE; +} +} + +static grub_err_t +revparse_biosdisk(const grub_uint8_t dnum, const char **output
Re: [PATCH] Enable writing to ATA devices, fix several bugs
El lun, 21-07-2008 a las 14:49 +0200, Marco Gerards escribió: Pavel Roskin [EMAIL PROTECTED] writes: 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). No, you are right. But it means that you have a look at the Linux ATA code. If you copy Linux names into our code, people could claim that we looked at Linux and based our code on it. So what? Aren't both Linux and GRUB under the GPL? That _should_ mean that we can look at their code and put it into GRUB (create a derivative work) either as-is or modified. 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. We were talking about not looking at copyrighted code as a reference... But looking at FreeBSD would be better than looking at Linux if we want to avoid possible copyright problems. I still don't understand this: the GPL includes an irrevocable grant as long as the license is obeyed. As for copyright problems, Linux has had several clashes (SCO et al), but in each and every instance people has raised against the attacker, defended Linux and won in court. I say it offers quite good copyright shielding. Habbit 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] Enable writing to ATA devices, fix several bugs
El lun, 21-07-2008 a las 17:20 +0200, Marco Gerards escribió: Javier Martín [EMAIL PROTECTED] writes: El lun, 21-07-2008 a las 14:49 +0200, Marco Gerards escribió: Pavel Roskin [EMAIL PROTECTED] writes: 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). No, you are right. But it means that you have a look at the Linux ATA code. If you copy Linux names into our code, people could claim that we looked at Linux and based our code on it. So what? Aren't both Linux and GRUB under the GPL? That _should_ mean that we can look at their code and put it into GRUB (create a derivative work) either as-is or modified. For GRUB 2 we require copyright assignments. 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. We were talking about not looking at copyrighted code as a reference... But looking at FreeBSD would be better than looking at Linux if we want to avoid possible copyright problems. I still don't understand this: the GPL includes an irrevocable grant as long as the license is obeyed. As for copyright problems, Linux has had several clashes (SCO et al), but in each and every instance people has raised against the attacker, defended Linux and won in court. I say it offers quite good copyright shielding. This isn't about licenses. This is about copyright. I know, I know... What I'm asking is _why_ this whole obsession about copyright assignments. Is there a page in the wiki explaining it? 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] Kernel fixes for Cygwin
El lun, 21-07-2008 a las 11:51 -0400, Pavel Roskin escribió: On Mon, 2008-07-21 at 13:02 +0200, Javier Martín wrote: El lun, 21-07-2008 a las 12:33 +0200, Christian Franke escribió: due to the complexity of PE, a stand-alone converter may likely be larger than the ~680 LoC converter I already offered here. Why do we even consider a PE-ELF converter? I think the easier way to go would have the people building GRUB in cygwin (not exactly newbies) to have an i386-pc-elf cross compiler built first, then use that for the bootloader programs and the normal gcc for tools. Even a naked (i.e. libraryless) cross compiler would work, since the bootloader part of GRUB is does not need libs (in C terminology, it's freestanding). That way, we are free from objcopy bugs or BFD design limitations. Well, if we want users to recompile their toolchain first, it's too much to ask. I think it is not, since people building GRUB in Cygwin are not exactly newcomers to the land of compiling: this package requires that its files and modules be in ELF format; your compiler does not do it, so you need another compiler. End of the problem. Of course, another way to go could be to allow the bootloader part of GRUB to be built in PE format: it would just be a matter of writing the PE counterparts to kern/elf.c and abstracting kern/dl.c a bit (i.e. a lot of work). The downside to this, apart from the unspecified work required, is that Windows-built i386-pc-pe modules are no longer compatible with Linux-built i386-pc-elf. Not a showstopper, but might require a sober thinking. As I have a lot of free time right now, I'll try to think whether it's possible or not. Maybe we could treat ELF header like a multiboot header? That means that we write the header fields in the assembly language, substitute the necessary variables and ask objcopy to make a raw binary that would actually be an ELF file? As far as I understand the ELF format, this would be too complex to get right: there's a lot of info in there. We could actually do it for all platforms, so that we won't depend on the object file format. 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] Kernel fixes for Cygwin
El lun, 21-07-2008 a las 13:03 -0400, Pavel Roskin escribió: On Mon, 2008-07-21 at 18:50 +0200, Javier Martín wrote: Of course, another way to go could be to allow the bootloader part of GRUB to be built in PE format: it would just be a matter of writing the PE counterparts to kern/elf.c and abstracting kern/dl.c a bit (i.e. a lot of work). The downside to this, apart from the unspecified work required, is that Windows-built i386-pc-pe modules are no longer compatible with Linux-built i386-pc-elf. Not a showstopper, but might require a sober thinking. As I have a lot of free time right now, I'll try to think whether it's possible or not. I think it's important to have a consistent format for modules. I thought that two hours ago, but now I find no advantage other than the sharing of modules between similar computers. 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