Google SoC, Fancy Menus or USB keyboard

[Colin D Bennett]

I am interested in possibly contributing to the GRUB project in the 
Google Summer of Code, and I was interested in further discussing the 
Fancy Menus idea.


I have not used GRUB 2 yet (I am using Gentoo Linux primarily, with GRUB 
0.9x), though I have downloaded and browsed the source code. I assume 
that fancy menus will still use the keyboard exclusively as the input 
device, for the time being. (Is there talk of mouse support?)


Does GRUB code run in real mode? I was thinking that double-buffering 
the graphics might be important for fancy graphics effects, and wondered 
whether is it difficult to allocate and deal with a megabyte or so block 
of memory? When I last programmed real mode programs in DOS, I know the 
limitations based on segment sizes, etc., but it's been a while.


Thanks!
Best Regards,
Colin



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

Re: Google SoC, Fancy Menus or USB keyboard

[Colin D Bennett]

Pavel Roskin wrote:

On Wed, 2008-04-02 at 17:43 -0700, Colin D Bennett wrote:

  
Does GRUB code run in real mode? I was thinking that double-buffering 
the graphics might be important for fancy graphics effects, and wondered 
whether is it difficult to allocate and deal with a megabyte or so block 
of memory? When I last programmed real mode programs in DOS, I know the 
limitations based on segment sizes, etc., but it's been a while.



GRUB runs in protected mode, but it can switch to the real mode
temporarily to call BIOS.  In any case, I believe that the menu
implementation should be portable rather than x86 PC specific.
  
Thanks for the info. Of course I also agree that the menu should be 
portable; my mention of real mode was, I guess, a question of whether we 
have a 32-bit linear address space, rather than specifics about the 
processor mode.



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

Re: Google SoC, Fancy Menus or USB keyboard

[Colin D Bennett]

Pavel Roskin wrote:

On Wed, 2008-04-02 at 18:54 -0700, Colin D Bennett wrote:
  
I was debugging the code to switch to the protected mode lately, so I

can assure you that the address space is linear :)
  
Is there anyone in particular on the GRUB project that I should be in 
contact with about the fancy menus task before submitting my SoC 
application?


Thanks,
Colin


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

Re: Some GRUB ideas

[Colin D Bennett]

komputes wrote:
Something like that would be nice. Or at least a passing mention in 
menu.lst that Grub starts counting from 0 not 1 in its drive/partition 
designations--i.e. (hd0,1) = /dev/sda2, etc. That confuses a lot of 
newcommers to linux, who would appreciate a command that will help 
them select or at least see how to correctly transport hard disk 
"locations" from a shell to a menu.lst. I just thought I would bring 
this to the attention of GRUB developers to see if this can be 
included in an upcoming version/update.
I like your idea -- that would have come in very handy for me many times 
before.


Note that in GRUB 2, partition numbering has been changed to begin at 1.
See: http://grub.enbug.org/grub.cfg

Colin


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

Re: Google's Summer of Code 2008 / GRUB 2

[Colin D Bennett]

On Tue, 22 Apr 2008 00:00:06 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:
> We want to welcome both Colin and Marco to be part of GRUB 2!

Hi,
I'm going to be working on the "Fancy Menus" feature for GRUB 2.

Once I begin producing real code, I will have my Bazaar VCS
development branch available on my web site for anyone interested, as
well as periodic tarballs. I'll use the GRUB 2 wiki to document the
design and implementation as things move forward.

Currently I am getting familiar with the GRUB codebase and design. I'm
going to try to get GRUB 2 booting in QEMU so I can get faster feedback
during development.

Best Regards,
Colin


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

Re: Google's Summer of Code 2008 / GRUB 2

[Colin D Bennett]

Hi Marco,
Nice to meet you!

On Wed, 23 Apr 2008 11:47:36 +0200
Marco Gerards <[EMAIL PROTECTED]> wrote:

> First of all, the code.  I think it would be a good idea if I would
> send in patches for my code so it will be immediately integrated into
> GRUB 2.  This would work as everyone will make sure that this code
> will not be touched by anyone and patches to this will be ignored.
> Otherwise I can work in a branch, my own bzr repository or whatever.
> I hope I will get feedback on this :-)

I'm going to be using bzr for my personal development work. Currently I
am using 'tailor' to synchronize my local bzr "upstream GRUB 2" branch
from CVS. It seems to work fairly well. If you want more info on
setting up tailor, let me know and I can send you my config file
(really simple, actually).

Pavel Roskin posted a link yesterday to his git mirror of the GRUB CVS.
(http://repo.or.cz/w/grub2.git) That would be another option, and I
need to consider whether I should mirror directly from CVS or via the
git mirror. Interfacing bzr to git is a more direct mapping since they
both have the concept of changesets (revisions of the entire
repository). There is a bzr-git tool (https://launchpad.net/bzr-git)
that is supposed to integrate with git in a really slick way. I might
go straight to CVS, though, since the git repository is just a mirror
itself.

Something else you might find useful is the 'loom' plugin for Bazaar.
It helps you manage sets of patches against an upstream branch of a
project.

Regards,
Colin


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

Coding style / indent options

[Colin D Bennett]

Does anyone have a set of GNU indent (or astyle) options that correspond
to the proper format for GRUB source code? I want to make sure I'm using
the correct style so that I don't produce spurious changes when files
I'm editing are reformatted.

I tried using GNU indent (with the default GNU style) on a few GRUB 2
source files to see what differences resulted, and I observed the
following differences:

 GRUB  GNU 
   ===
(1) Space after unary '!' operator   Yes   No 
(2) Space between macro name and open paren  NoYes
(3) Max line length  >=80  <80
(4) Space between __attribute__ and parens   NoYes
(5) Keep indenting spaces on blank lines Yes   No

I did notice, however, that in some places  (e.g.,
normal/functions.c lines 84-87) the space/TAB usage is inconsistent). I
assume that replacing 8 spaces with TAB characters is the desired
usage (as GNU style suggests).

I just want to keep the code I modify and produce as clean and uniform
as possible, and produce clean patches.

Thanks,
Colin


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

Re: retiring device.map

[Colin D Bennett]

On Thu, 8 May 2008 01:39:47 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Thu, May 8, 2008 at 1:22 AM, Pavel Roskin <[EMAIL PROTECTED]> wrote:
> > On Wed, 2008-05-07 at 15:03 +0200, Robert Millan wrote:
> >  > On Tue, May 06, 2008 at 11:43:48AM -0400, Pavel Roskin wrote:
> >  > > On Tue, 2008-05-06 at 17:18 +0200, Robert Millan wrote:
> >  > >
> >  > > > I completely agree.  How do we go about this?  First of all
> >  > > > we need support for labels, right?
> >  > >
> >  > > We have it already in the "search" command.  Of course, there
> >  > > is always room for improvement.  GRUB doesn't recognize Linux
> >  > > swap labels.  It cannot set root to, say, partition 3 on a
> >  > > disk where partition 1 has the given label.
> >  >
> >  > Sorry, I was confusing labels with UUIDs.  The problem with
> >  > labels is they're not garanteed to be unique, right?  So maybe
> >  > we need support for UUIDs..
> >
> >  Right.
> 
> Yes, UUID can be useful, it's better to add it to grub2.

I think that while labels (like "linux boot", "swap 2", etc.) are
useful and easy to use for fixed disk drives (and conveniently avoid
problems when drive order is changed!), they are inadequate for
removable drives such as USB flash drives or external hard drives,
where it is easy to run into name clashes--for instance, when swapping
external drives between machines, they might have the same name but have
slightly different purposes/content and need different treatment. In
some cases like this, the UUID seems a better choice than a label (copy
and paste is your friend, however).

Regards,
Colin


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

Higher resolution timers

[Colin D Bennett]

To implement a nice GUI with some animations, it will be important to
have a timer with a better resolution than 1/18 second (which appears
to be the current GRUB timer resolution on the PC).

What would be the best way to do this? Possibilities that I am aware of
are: 
 * RDTSC instruction (must calibrate with RTC at startup?)
 * HPET (complex to use?)
 * RTC (can we set the timer interrupt rate to >18 Hz?)

Does anyone have any thoughts on these options?

I think that using the TSC (w/ RDTSC instruction) and calibrating it
with a quick 2-3 RTC tick loop at startup might be the easiest option.

Regards,
Colin


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

Re: Higher resolution timers

[Colin D Bennett]

On Sun, 18 May 2008 19:27:05 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Colin D Bennett wrote:
> > I think that using the TSC (w/ RDTSC instruction) and calibrating it
> > with a quick 2-3 RTC tick loop at startup might be the easiest
> > option.
> 
> Hi Colin,
> 
> What kind of accuracy would you need?
> 
> I am just wondering if you just define function like:
> 
> grub_uint64_t
> grub_timer_[nu]time(void);
> 
> This could return time in nanoseconds, or microseconds from epoch.
> 
> Then during grub init you would call some platform function to 
> initialize time (calibrate when using rdtsc), and set proper offset 
> value so you get correct time when asking for time. On every system
> it would return time in this format, but granularity would be
> different.
> 
> This could be:
> 
> void
> grub_timer_init(void)
> 
> What do you think?

Vesa,
It sounds good.  Actually, for the kind of the situations I am
thinking of, high resolution timing is not required to be based on any
absolute real time, so we would not necessarily need to base it on RTC
time. Synchronizing it to RTC time would probably not be difficult, but
I thought I would mention what my requirements will be.

Here is the kind of situation I envision, in an event loop that handles
accepting user input and updating the screen at regular intervals
(possibly at 30 to 60 frames/sec, hence the need for higher resolution
timers) to support the user interface:

 /* Event loop */
 grub_uint32_t nextframe_time = 0;
 grub_uint32_t now;
 while (1)
 {
   now = grub_timer_utime();
   if (now >= nextframe_time)
   {
 /* Schedule the next frame. */
 nextframe_time = now + 100 / FRAMES_PER_SEC;
 do_animation_step();
   }
   if (key_is_pressed())
   {
 /* Handle key presses... */
   }
 }

So all I care about is the value relative to other calls to
grub_timer_utime() during a particular runtime of grub. It could start
at 0 if that is easier, but either way is fine.

Colin


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

Re: Higher resolution timers

[Colin D Bennett]

On Mon, 12 May 2008 09:33:54 -0700
Colin D Bennett <[EMAIL PROTECTED]> wrote:

> I think that using the TSC (w/ RDTSC instruction) and calibrating it
> with a quick 2-3 RTC tick loop at startup might be the easiest option.

Just wondered if something like this would be a practical way to go:

/***/
/* How many ticks to use for calibration loop. (>= 1) */
#define CALIBRATION_TICKS 2

/* Output: Number of TSC cycles per second. */
grub_uint64_t tsc_cycles_per_second;

void grub_timer_tsc_calibrate(void)
{
/* Calibrate to real time (relative, not absolute). */
grub_uint64_t start_tsc;
grub_uint64_t end_tsc;
grub_uint32_t initial_tick;
grub_uint32_t start_tick;
grub_uint32_t end_tick;

/* Wait for the start of the next tick;
   we'll base out timing off this edge. */
initial_tick = grub_get_rtc();
do 
{
start_tick = grub_get_rtc();
}
while (start_tick == initial_tick);
start_tsc = rdtsc();

/* Wait for the start of the next tick.  This will
   be the end of the 1-tick period. */
do 
{
end_tick = grub_get_rtc();
}
while (end_tick - start_tick < CALIBRATION_TICKS);
end_tsc = rdtsc();

tsc_cycles_per_second = 
(end_tsc - start_tsc) / (end_tick - start_tick) 
* GRUB_TICKS_PER_SEC;
}
/***/


Colin


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

Re: Higher resolution timers

[Colin D Bennett]

On Fri, 23 May 2008 13:55:21 +0200
Marco Gerards <[EMAIL PROTECTED]> wrote:

> Colin D Bennett <[EMAIL PROTECTED]> writes:
> 
> > On Mon, 12 May 2008 09:33:54 -0700
> > Colin D Bennett <[EMAIL PROTECTED]> wrote:
> >
> >> I think that using the TSC (w/ RDTSC instruction) and calibrating
> >> it with a quick 2-3 RTC tick loop at startup might be the easiest
> >> option.
> >
> > Just wondered if something like this would be a practical way to go:
> 
> Well, I only had a quick look and this might be a good way to measure
> the clock speed.  BTW, can you please make sure you use our coding
> style?  Are the 2-3 loops based on some calculations or only a guess?

Of course I will adhere to the GRUB coding style.  The snippet I posted
was just basically just quick pseudocode (not actually compiled) based
on 10 minutes of thought.  I did not take time to format the code style
but surely I would before submitting an actual patch.

I think we only need 1 full RTC timer tick (i.e. 1/18 of a
second) to figure out how many TSC cycles correspond to one second of
real time.  The code has to first synchronize to the starting edge of
an RTC tick, so that is the loop:
   do { ... } while (start_tick == initial_tick);
Then the number of elapsed TSC cycles until the next RTC tick is
calculated and the TSC cycles per second is calculated.

(As I said, this is untested and incomplete scratch work as a starting
point for a discussion, and it might not even compile.)


> There might be a problem with using this instruction.  Are clock
> cycles actually measured after using the "halt" instruction?  If not,
> the use of such instruction might not generally used in GRUB :-/

Thanks for thinking of that. According to
<http://www.scl.ameslab.gov/Projects/Rabbit/menu4.html>, the TSC
continues counting even when the CPU executes the HLT instruction:

   The TSC is set to 0 at system reset. It presently increments once per
   processor cycle, and is 64 bits wide. Only CPL = 0 can modify TSC,
   and then only the lower 32 bits can be set, while the upper 32 bits
   are cleared. We do not recommend changing TSC, and the library does
   not permit it. The TSC will continue to increment in the halt state
   (HLT instruction).

Therefore, I think that that the HLT instruction in GRUB will not
negatively impact the use of RDTSC.

Regards,
Colin


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

GRUB integral types? Why grub_uint32_t and not uint32_t?

[Colin D Bennett]

I was wondering why it is necessary to use the integral types with
the "grub_" prefix instead of the standard uint32_t, int16_t, etc.?

It makes the most simple code much more verbose when we have to write
"grub_" so many times, and this seems like a case where it is not
needed.

Colin


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

Re: why not support gzipped font file?

[Colin D Bennett]

On Sun, 1 Jun 2008 10:50:05 +0800 "y.volta" <[EMAIL PROTECTED]> wrote:
> Hi,
> 
> I noticed that, /font/manager.c:51:
> 
>   file = grub_file_open (filename); 
> 
> so, we can only use pure font file, this will need more than 1MB
> bytes for font file, and more time to load it into memory. 
> 
> well, how about change it to:
> 
>   file = grub_gzfile_open (filename, 1); 

Are you noticing that it is slow to load the font into memory?
Currently the whole font is not loaded at once, but rather the glyphs
are loaded on demand, as they are needed, so opening the font file
should be fairly fast.

You probably don't want to spend time working on the existing GRUB 2
'PFF' font code since I am working on creating a new font file format
for GRUB, and I will remove the limitation on size of 16 pixels wide.

Using gzip on the whole file is not a great option since we want random
access to the file contents since we only need to read in the glyphs
that are used, which in general, for a font covering most of Unicode,
is a very small proportion of the characters in the file.

We could find ways to compress the font, but simply gzipping the font
file is not a good option when you don't need to read the whole font
anyway.

Perhaps there are better compression techniques we can use for the font
that will support random access to the font, but we should determine
whether there is a need for compression (or if compression will provide
a performance benefit).

Regards,
Colin


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

Re: Floating point usage

[Colin D Bennett]

On Thu, 05 Jun 2008 17:23:00 +0200
Stefan Reinauer <[EMAIL PROTECTED]> wrote:

> Jan Kleinsorge wrote:
> > Pavel Roskin wrote:
> >   
> >> Hello!
> >>
> >> I tried to compile the current grub on Fedora 9 for PowerPC, but it
> >> failed due to floating point usage in ls.  Sorry, I don't have the
> >> exact message handy.
> >>
> >> Anyway, there are several references to "float" and "double" in the
> >> code, and I think it's wrong.  GRUB should be able to work on
> >> systems with no floating point support and on systems where the
> >> floating point device needs to be activated.  Even Linux kernel
> >> avoids using floating point.
> >>
> >> If everyone is fine, I'll try to remove all floating point stuff
> >> from the code.
> >>
> >> 
> >  Hi Pavel,
> >
> > there is no need for hard-float support on the target machine. The
> > libgcc provides all the software-emulation routines. Given that
> > GRUB is linked against it. Which is likely as it is a necessity
> > when compiling with gcc. Maybe your compilation is simply broken
> > because of that one missing. Could you please post the error
> > message?
> Nonetheless, using Floating Point operations in bootloaders and
> firmware  is highly discouraged. If Floating Point is required for a
> bootloader (except, maybe, possibly, at all, for encryption) the
> bootloader is definitely broken, either in implementation or in
> design.
> 
> Also keep in mind, most operating systems expect the FPU to be in
> power on state.
> 
> Stefan

The bootloader is "broken" if it uses floating point arithmetic?  I
think that while GRUB should not REQUIRE an FPU, it could use software
floating point if there was a good reason to do so (and it would not
be broken as long is it was done portably). Though I only have minimal
knowledge of encryption algorithms, I don't think that floating point
math is often used.

I think that avoiding floating point math as Pavel Roskin suggested is
good, though I don't agree that Stefan's assertion that any use of
floating point math is necessarily "broken" -- it might be better to
say that we'd prefer not to pull in softfloat libraries (necessary
since many targets have no FPU); therefore it is desirable to avoid
floating point math.

Colin


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

Installing GRUB 2 on a CompactFlash card

[Colin D Bennett]

I want to install GRUB 2 on a CompactFlash card, which I will boot on
my Mini-ITX VIA C3 system for testing, by attaching the CF card to an
IDE-CF adapter.

Before I run grub-install, I wanted to get a confirmation that I'm
doing the right thing, so I don't end up wiping out my development
system's boot partition or something.  Here's my setup:

 Development workstation:
  Running Linux.
  Hard drives on /dev/hdXX and /dev/sdXX.
  The CompactFlash card will appear as /dev/sdc or something.
(let's assume it's /dev/sdc)

 Target system:
  IDE controller - connected to IDE-CompactFlash adapter.


If I create one partition on the CF card, for GRUB, I assume I should
set up my grub.cfg to refer to it as (hd0,1) since this will only be
used on the TARGET SYSTEM, right?

Then on my development workstation I will do:
   grub-install /dev/sdc

But will this affect my development workstation's "/boot" ?  I want to
install only to my CompactFlash card; is this possible without messing
with my dev machine's filesystem?

If I have /dev/sdc1 (the CF card's partition) mounted
at /media/grubtest, can I just do
   grub-install --root-directory=/media/grubtest /dev/sdc


Thanks!
Colin


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

Re: Installing GRUB 2 on a CompactFlash card

[Colin D Bennett]

On Fri, 06 Jun 2008 18:57:39 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Fri, 2008-06-06 at 02:11 -0400, Pavel Roskin wrote:
> > Quoting Colin D Bennett <[EMAIL PROTECTED]>:
> > 
> > > If I create one partition on the CF card, for GRUB, I assume I
> > > should set up my grub.cfg to refer to it as (hd0,1) since this
> > > will only be used on the TARGET SYSTEM, right?
> > 
> > I suggest that you use the current GRUB from CVS and use UUID to
> > refer to any drives.  update-grub will generate the new style
> > grub.cfg, but be sure to back up your current grub.cfg in case it
> > gets overwritten.
> > 
> > > Then on my development workstation I will do:
> > >grub-install /dev/sdc
> > >
> > > But will this affect my development workstation's "/boot" ?
> > 
> > I don't think current GRUB will allow you to do that, but if  
> > cross-device installs were supported, it would indeed overwrite  
> > /boot/grub
> 
> P.S. Current GRUB is broken.  Please use the version as of June 5.  If
> ext3 is involved, use the version from May 19.  That won't have UUID,
> but it would be safer.
> 
> Things may be fixed soon, even hours from now, but I cannot give any
> promises.  Sorry for inconvenience.

What is the problem with post-June 5 GRUB?  Has it been fixed?  I will
use June 5 CVS for now then.


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

64 bit arithmetic?

[Colin D Bennett]

I'm working on implementing a higher resolution timer using the x86
TSC.  However, this entails 64-bit arithmetic since on a 2GHz machine
it will overflow a 32-bit number every couple of seconds (the TSC
counter CPU cycles since boot).

So to convert the TSC value into a value in milliseconds, I need to do
64-bit division, but when I try to build GRUB now, I get

   undefined reference to `__udivdi3'

Is this expected?  Should it be possible for me to do division on
grub_uint64_t types?

BTW, I'm trying to add it to the kernel image, if this makes a
difference.

Thanks,
Colin



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

Re: 64 bit arithmetic?

[Colin D Bennett]

On Wed, 11 Jun 2008 00:54:35 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Wed, Jun 11, 2008 at 12:26 AM, Colin D Bennett <[EMAIL PROTECTED]>
> wrote:
> > I'm working on implementing a higher resolution timer using the x86
> > TSC.  However, this entails 64-bit arithmetic since on a 2GHz
> > machine it will overflow a 32-bit number every couple of seconds
> > (the TSC counter CPU cycles since boot).
> >
> > So to convert the TSC value into a value in milliseconds, I need to
> > do 64-bit division, but when I try to build GRUB now, I get
> >
> >   undefined reference to `__udivdi3'
> >
> > Is this expected?  Should it be possible for me to do division on
> > grub_uint64_t types?
> >
> > BTW, I'm trying to add it to the kernel image, if this makes a
> > difference.
> 
> Hi.
> 
> You can use grub_divmod64, which is defined in kern/misc.c

Thanks! That works perfectly.


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

Re: [PATCH] Drivemap module

[Colin D Bennett]

On Fri, 13 Jun 2008 00:43:31 +0200
Javier Martín <[EMAIL PROTECTED]> wrote:

> Hi there, I checked your comment, reread the GNU coding guidelines and
> fixed some things (stated below the relevant parts of your message).
> Here is the new version of the patch.
> 
> El jue, 12-06-2008 a las 16:31 -0400, Pavel Roskin escribió:
> > Please don't add any trailing whitespace.  Line in the patch that
> > start with a plus should not end with a space or a tab.
> I could not find the line you were referring to. I could only find one
> line starting with a plus (in install_int13_handler), but it does not
> end with a space/tab. However, I did find one trailing tab after a
> comment, and removed it.

I think Pavel meant the lines added by the patch, i.e., beginning with
a plus in the patch, not in the original source.  Here's an
illustration to help find and visualize the invisible trailing
whitespace:

[EMAIL PROTECTED] ~ $ perl -ne '/^\+.* +$/ and y/ \t/_~/ and
print' /home/cdb/drivemap.patch
+__else_if_(mapto_==_mapfrom)__/*_Reset_to_default_*/__ +__
+__/*_Save_the_pointer_to_the_old_int13h_handler_*/
+__
+VARIABLE(grub_drivemap_int13_handler_base)__
+__
+__push_%bp__
+___(grub_err_t_(*hook)_(void),_int_abort_on_error);_
+__

I highly recommend running 

  indent --no-tabs

on your source code, as long as it doesn't cause too many spurious
changes to pre-existing code.  This should take care of most formatting
inconsistencies and frees you a little but from concerning yourself
with trivial details.

Colin


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

Re: [PATCH] Drivemap module

[Colin D Bennett]

On Thu, 12 Jun 2008 21:00:21 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:
> On Thu, 2008-06-12 at 15:58 -0700, Colin D Bennett wrote:
> > I highly recommend running 
> > 
> >   indent --no-tabs
> 
> Why no tabs?  Tabs are heavily used in the code.  GNU indent is
> supposed to default to GNU Coding Standards, and it uses tabs.

The video module code uses only spaces for indentation.
Since I've been working mostly on the video module, so far, I was
misled to believe that spaces are the preferred GRUB style.

I checked all the C files in GRUB2 CVS and found that you are right,
tabs are heavily used, except in the code that I have been focused
on.   The following C files do not contains any tabs:

./kern/i386/efi/init.c
./commands/videotest.c
./util/powerpc/ieee1275/misc.c
./util/i386/pc/misc.c
./util/i386/get_disk_name.c
./util/ieee1275/get_disk_name.c
./util/lvm.c
./video/video.c
./video/i386/pc/vbeutil.c
./video/i386/pc/vbefill.c
./video/i386/pc/vbeblit.c
./video/i386/pc/vbe.c
./video/readers/tga.c
./video/bitmap.c
./disk/ieee1275/nand.c

> > on your source code, as long as it doesn't cause too many spurious
> > changes to pre-existing code.
> 
> It will.

This depends on the file.  Running format (with the same choice of
tabs+spaces or spaces-only for indentation) on a GRUB source file
usually results in minimal changes, in my experience.

> > This should take care of most formatting
> > inconsistencies and frees you a little but from concerning yourself
> > with trivial details.
> 
> We can use a script to re-indent everything.  Then new patches could
> be easily run through the same script.  Otherwise, it's not going to
> be simple.

Interesting idea!

Anyway, my point is not to get into the religious war of spaces vs.
tabs in indentation, since I am comfortable with both, although I think
that mixing spaces and tabs together (using TAB characters as a
substitute for 8 spaces) negates the benefits of both approaches.  But
that's all I'll say on that.

I will indent code that I work on with the same style that it already
has, so that my patches are clean and contain, in general, only changes
related to actual features that the patch adds.

Colin


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

[PATCH] Tidy up 'make' output

[Colin D Bennett]

When thousands of long, wrapped lines full of command line options and
file names are scrolling by on your terminal, it is very hard to pick
out the irregularities in the build process, such as error and warnings.

To make the output of ``make`` easier to parse by eye as it scrolls by,
I added support for "pretty" output for make. 

For example, it replaces the full gcc commands that look like

  gcc -Iutil -I../util -I. -Iinclude -I../include -Wall -W
  -DGRUB_LIBDIR=\"/usr/local/lib/`echo grub/i386-pc | sed 's,x,x,'`\" -g
  -O2 -DGRUB_UTIL=1  -MD -c -o
  grub_setup-util_getroot.o ../util/getroot.c gcc -Ikern -I../kern -I.
  -Iinclude -I../include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo
  grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1  -MD -c -o
  grub_setup-kern_device.o ../kern/device.c ../kern/device.c: In
  function 'grub_device_iterate': ../kern/device.c:84: warning:
  generating trampoline in object (requires executable
  stack) ../kern/device.c:84: warning: generating trampoline in object
  (requires executable stack) gcc -Ikern -I../kern -I. -Iinclude
  -I../include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo
  grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1  -MD -c -o
  grub_setup-kern_disk.o ../kern/disk.c gcc -Ikern -I../kern -I.
  -Iinclude -I../include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo
  grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1  -MD -c -o
  grub_setup-kern_err.o ../kern/err.c gcc -Ikern -I../kern -I. -Iinclude
  -I../include -Wall -W -DGRUB_LIBDIR=\"/usr/local/lib/`echo
  grub/i386-pc | sed 's,x,x,'`\" -g -O2 -DGRUB_UTIL=1  -MD -c -o
  grub_setup-kern_misc.o ../kern/misc.c

with output that, in my opinion, makes it easier to see warnings and
errors:

  COMPILE ../util/getroot.c
  COMPILE ../kern/device.c
  ../kern/device.c: In function 'grub_device_iterate':
  ../kern/device.c:84: warning: generating trampoline in object
  (requires executable stack)
  ../kern/device.c:84: warning: generating trampoline in object
  (requires executable stack)
  COMPILE ../kern/disk.c 
  COMPILE ../kern/err.c
  COMPILE ../kern/misc.c

If the variable ``V`` (for verbose) is set to 1, then the traditional
full output is displayed.  This is useful when you need to see which
compiler flags are being used or which source files are being linked
into an object file. Just run

  make V=1

to get the full output.

This patch can be refined further by adding 'pretty' rules for other
build operations to Makefile.in.

Regards,
Colin=== modified file 'Makefile.in'
--- Makefile.in	2008-04-25 19:41:48 +
+++ Makefile.in	2008-06-11 05:50:59 +
@@ -84,6 +84,39 @@
 YACC = @YACC@
 UNIFONT_HEX = @UNIFONT_HEX@
 
+### Pretty output control ###
+# Set up compiler and linker commands that either is quiet (does not print
+# the command line being executed) or verbose (print the command line).
+_CC := $(CC)
+_TARGET_CC := $(TARGET_CC)
+_STRIP := $(STRIP)
+_GENMODSRC := sh $(srcdir)/genmodsrc.sh
+ifeq ($(V),1)
+ override V_PREFIX :=
+ override CC = $(_CC)
+ override TARGET_CC = $(_CC)
+ override STRIP = $(_STRIP)
+ override GENMODSRC = $(_GENMODSRC)
+ override INFO_GENCMDLIST =
+ override INFO_GENFSLIST =
+ override INFO_GENPARTMAPLIST =
+ override INFO_GEN_FINAL_COMMAND_LIST =
+ override INFO_GEN_FINAL_FS_LIST =
+ override INFO_GEN_FINAL_PARTMAP_LIST =
+else
+ override V_PREFIX := @
+ override CC = @echo "COMPILE $<"; $(_CC)
+ override TARGET_CC = @echo "COMPILE(TARGET) $<"; $(_TARGET_CC)
+ override STRIP = @echo "STRIP   $@"; $(_STRIP)
+ override GENMODSRC = @echo "GENMODSRC   $@"; $(_GENMODSRC)
+ override INFO_GENCMDLIST = @echo "GENCMDLIST  $@"
+ override INFO_GENFSLIST = @echo "GENFSLIST   $@"
+ override INFO_GENPARTMAPLIST = @echo "GENPARTMAPLIST  $@"
+ override INFO_GEN_FINAL_COMMAND_LIST = @echo "GENCMDLIST[final]  $@"
+ override INFO_GEN_FINAL_FS_LIST = @echo "GENFSLIST[final]   $@"
+ override INFO_GEN_FINAL_PARTMAP_LIST = @echo "GENPARTMAPLIST[final]  $@"
+endif
+
 # Options.
 enable_grub_emu = @enable_grub_emu@
 enable_grub_fstest = @enable_grub_fstest@
@@ -130,13 +163,16 @@
 	  || (rm -f $@; exit 1)
 
 command.lst: $(COMMANDFILES)
-	cat $^ /dev/null | sort > $@
+	$(INFO_GEN_FINAL_COMMAND_LIST)
+	$(V_PREFIX)cat $^ /dev/null | sort > $@
 
 fs.lst: $(FSFILES)
-	cat $^ /dev/null | sort > $@
+	$(INFO_GEN_FINAL_FS_LIST)
+	$(V_PREFIX)cat $^ /dev/null | sort > $@
 
 partmap.lst: $(PARTMAPFILES)
-	cat $^ /dev/null | sort > $@
+	$(INFO_GEN_FINAL_PARTMAP_LIST)
+	$(V_PREFIX)cat $^ /dev/null | sort > $@
 
 ifeq (, $(UNIFONT_HEX))
 else

=== modified file 'genmk.rb'
--- genmk.rb	2008-04-17 14:14:09 +
+++ genmk.rb	2008-06-11 05:50:59 +
@@ -125,7 +125,7 @@
 	$(TARGET_CC) $(TARGET_CPPFLAGS) $(TARGET_CFLAGS) $(#{prefix}_CFLAGS) -c -o $@ $<
 
 #{mod_src}: moddep.lst genmodsrc.sh
-	sh $(srcdir)/genmodsrc.sh '#{mod_name}' $< > $@ || (rm -f $@; exit 1)
+	$(GENMODSRC) '#{mod_name}' $< > $@ || (rm -f $@; exit 1)
 
 ifneq ($(#{pr

[PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

I fixed ``grub-install`` so it can function when installed
to a prefix other than ``/usr/local``.  This simply meant adding

  --directory="${pkglibdir}

to the command line arguments for grub_mkimage.

Regards,
Colin=== modified file 'util/i386/pc/grub-install.in'
--- util/i386/pc/grub-install.in	2008-06-14 19:04:44 +
+++ util/i386/pc/grub-install.in	2008-06-17 17:07:08 +
@@ -234,7 +234,10 @@
 # The order in this list is critical.  Be careful when modifiing it.
 modules="$modules $fs_module $partmap_module biosdisk $devabstraction_module"
 
-$grub_mkimage --output=${grubdir}/core.img --prefix=`make_system_path_relative_to_its_root ${grubdir}`/ $modules || exit 1
+$grub_mkimage --output=${grubdir}/core.img \
+  --directory="${pkglibdir}" \
+  --prefix=`make_system_path_relative_to_its_root ${grubdir}`/ \
+	  $modules || exit 1
 
 # Now perform the installation.
 $grub_setup --directory=${grubdir} --device-map=${device_map} \

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

Re: [PATCH] Tidy up 'make' output

[Colin D Bennett]

On Tue, 17 Jun 2008 15:57:47 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-06-17 at 10:44 -0700, Colin D Bennett wrote:
> > When thousands of long, wrapped lines full of command line options
> > and file names are scrolling by on your terminal, it is very hard
> > to pick out the irregularities in the build process, such as error
> > and warnings.
> 
> I like the idea, but the massive use of "override" doesn't looks
> right. I'd rather see variables with different names used throughout
> the makefiles.  Linux makefiles don't use "override" at all.
> "override" should be the last resort if everything else fails.

Ok.  Well, are implicit make rules ever used in the makefiles?  If so,
we would have to make them explicit in order to use a different
compiler variable.  I tried to touch the least number of things
possible with my patch.

If you wish, I can take a crack at eliminating the use of 'override'
and using a different set of make variables for the instrumented calls
to commands.

Regards,
Colin


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

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

On Tue, 17 Jun 2008 15:46:12 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-06-17 at 10:51 -0700, Colin D Bennett wrote:
> > I fixed ``grub-install`` so it can function when installed
> > to a prefix other than ``/usr/local``.  This simply meant adding
> > 
> >   --directory="${pkglibdir}
> > 
> > to the command line arguments for grub_mkimage.
> 
> The default is GRUB_LIBDIR, which is set to pkglibdir at the compile
> time.  Your patch would have no effect if anything is compiled and
> installed properly.


Ok, I tried it again with current GRUB CVS and it works.  I was working
with a CVS version from a couple of weeks ago; maybe the problem was
related to that.

Anyway, it sounds like my patch is unnecessary, so you can ignore it
now.

Regards,
Colin


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

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

On Tue, 17 Jun 2008 15:46:12 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-06-17 at 10:51 -0700, Colin D Bennett wrote:
> > I fixed ``grub-install`` so it can function when installed
> > to a prefix other than ``/usr/local``.  This simply meant adding
> > 
> >   --directory="${pkglibdir}
> > 
> > to the command line arguments for grub_mkimage.
> 
> The default is GRUB_LIBDIR, which is set to pkglibdir at the compile
> time.  Your patch would have no effect if anything is compiled and
> installed properly.
> 
> Maybe you are trying to change prefix during "make install"?  That's
> not the best approach.  DESTDIR should be used to install to a
> temporary location.
> 
> Or maybe you have some other scenario in mind?

I'm not sure.  All I know is that I did 
  ./configure --prefix=/home/cdb/bin/grub && make && make install,
then I tried to do /home/cdb/bin/grub/bin/grub-install /media/sdc1 and
grub-mkimage was getting called with /usr/local/lib/something.  Since I
specified --prefix to configure, I expect grub never ever to refer
to /usr/local.  This patch fixed my problem.

Colin


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

Re: [PATCH] Tidy up 'make' output

[Colin D Bennett]

On Tue, 17 Jun 2008 22:37:10 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> On Tue, Jun 17, 2008 at 10:44:48AM -0700, Colin D Bennett wrote:
> > with output that, in my opinion, makes it easier to see warnings and
> > errors:
> > 
> >   COMPILE ../util/getroot.c
> >   COMPILE ../kern/device.c
> >   ../kern/device.c: In function 'grub_device_iterate':
> >   ../kern/device.c:84: warning: generating trampoline in object
> >   (requires executable stack)
> >   ../kern/device.c:84: warning: generating trampoline in object
> >   (requires executable stack)
> >   COMPILE ../kern/disk.c 
> >   COMPILE ../kern/err.c
> >   COMPILE ../kern/misc.c
> 
> I don't like the idea of hiding information this way.  If the goal is
> to catch warnings, I think -Werror can do a much better job (and
> catching errors shouldn't be a problem unless you're using make -j or
> -k).

What about all the "trampoline" and strict-aliasing warnings (there must
be hundreds of them)?  I'm all for warning-free code, but if we try to
use -Werror, the code won't even begin to compile in the current state.


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

Re: [PATCH] Fix grub-install to use prefix for modules dir

[Colin D Bennett]

On Tue, 17 Jun 2008 20:04:29 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-06-17 at 13:04 -0700, Colin D Bennett wrote:
> 
> > I'm not sure.  All I know is that I did 
> >   ./configure --prefix=/home/cdb/bin/grub && make && make install,
> > then I tried to do /home/cdb/bin/grub/bin/grub-install /media/sdc1
> > and grub-mkimage was getting called with /usr/local/lib/something.
> > Since I specified --prefix to configure, I expect grub never ever
> > to refer to /usr/local.  This patch fixed my problem.
> 
> I think I know what you actually did:
> 
> ./configure
> make
> ./configure --prefix=/home/cdb/bin/grub
> make
> make install
> 
> "/usr/local" was hardcoded into grub-mkimage in the first run.  In the
> second run, grub-mkimage wasn't rebuilt because it's not a script and
> because config.h didn't change.

You are probably right... sorry to bother you with my error.

> We don't want wrong path to be hardcoded in grub-mkimage.  Working it
> around in scripts would keep grub-mkimage incorrect.
> 
> I think the best solution would be to add Makefile to
> the grub-mkimage.o dependencies.  GRUB_LIBDIR is written to Makefile
> and grub-mkimage.c uses it.  Here's the patch (*.mk changes are not
> included):
> 
> diff --git a/conf/i386-efi.rmk b/conf/i386-efi.rmk
> index 87626c2..311b6ab 100644
> --- a/conf/i386-efi.rmk
> +++ b/conf/i386-efi.rmk
> @@ -17,6 +17,7 @@ sbin_UTILITIES = grub-mkdevicemap
>  # For grub-mkimage.
>  grub_mkimage_SOURCES = util/i386/efi/grub-mkimage.c util/misc.c \
>   util/resolve.c
> +util/i386/efi/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-setup.
>  #grub_setup_SOURCES = util/i386/pc/grub-setup.c
> util/biosdisk.c   \ diff --git a/conf/i386-ieee1275.rmk
> b/conf/i386-ieee1275.rmk index ac2f45d..e4f2a66 100644
> --- a/conf/i386-ieee1275.rmk
> +++ b/conf/i386-ieee1275.rmk
> @@ -50,6 +50,7 @@ endif
>  grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
>   util/resolve.c
>  grub_mkimage_LDFLAGS = $(LIBLZO)
> +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c
> util/misc.c   \ diff --git a/conf/i386-linuxbios.rmk
> b/conf/i386-linuxbios.rmk index 31f03a9..d2546be 100644
> --- a/conf/i386-linuxbios.rmk
> +++ b/conf/i386-linuxbios.rmk
> @@ -49,6 +49,7 @@ endif
>  grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
>   util/resolve.c
>  grub_mkimage_LDFLAGS = $(LIBLZO)
> +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c
> util/misc.c   \ diff --git a/conf/i386-pc.rmk
> b/conf/i386-pc.rmk index a3f8467..7b7924f 100644
> --- a/conf/i386-pc.rmk
> +++ b/conf/i386-pc.rmk
> @@ -76,6 +76,7 @@ grub_mkimage_SOURCES = util/i386/pc/grub-mkimage.c
> util/misc.c \ util/resolve.c
>  grub_mkimage_CFLAGS =
> -DGRUB_MEMORY_MACHINE_LINK_ADDR=$(GRUB_MEMORY_MACHINE_LINK_ADDR)
> grub_mkimage_LDFLAGS = $(LIBLZO)
> +util/i386/pc/grub-mkimage.c_DEPENDENCIES = Makefile 
>  # For grub-setup.
>  util/i386/pc/grub-setup.c_DEPENDENCIES = grub_setup_init.h
> diff --git a/conf/powerpc-ieee1275.rmk b/conf/powerpc-ieee1275.rmk
> index 2c16bc1..51442c6 100644
> --- a/conf/powerpc-ieee1275.rmk
> +++ b/conf/powerpc-ieee1275.rmk
> @@ -37,6 +37,7 @@ endif
>  # For grub-mkimage.
>  grub_mkimage_SOURCES = util/elf/grub-mkimage.c util/misc.c \
>  util/resolve.c 
> +util/elf/grub-mkimage.c_DEPENDENCIES = Makefile
>  
>  # For grub-mkdevicemap.
>  grub_mkdevicemap_SOURCES = util/grub-mkdevicemap.c
> util/misc.c   \

Sounds good to me!

Thanks,
Colin


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

DEFLATE compression

[Colin D Bennett]

How does GRUB support decompressing DEFLATE compressed data?  Based on
my reading of io/gzio.c, it looks like a gzip header is required to
inflate deflated data.

I am implementing a new font format and will be embedding multiple
blocks of compressed bitmaps in the font file for a good combination of
space efficient on-disk storage and fast read performance during use.
A block of character bitmaps will only be read from the file
and decompressed at run time when a character within that block is
needed. After that, when any character in the block is needed, it is
already decompressed in memory for quick access.

I was going to use DEFLATE compression, it looks like I may have to add
the gzip structure to it, which seems silly since it's just a block of
data within my file. 

Also, is it possible to tell the gzio module to decompress a block of
data beginning at a specific offset in the file?

Regards,
Colin


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

[RFC] High resolution time support using x86 TSC

[Colin D Bennett]

I have implemented high resolution time support (through the
new grub_get_time_ms() function) using the RDTSC instruction available
on Pentium and higher x86 CPUs.  The TSC value is simply a 64-bit block
cycle counter that is zeroed at bootup, so grub_main() calls
grub_time_init(), which is defined by each platform.  If a platform
links to kern/i386/tsc.c, then the grub_time_init() function from tsc.c
is used, which calibrates the TSC rate and absolute zero reference using
the RTC.

I also extracted the grub_millisleep_generic() function into
kern/generic/millisleep.c and renamed it to grub_millisleep() so that
platforms linking to it don't need to define a grub_millisleep() just
to call grub_millisleep_generic() anymore.  Simply linking in
kern/generic/millisleep.c will use the generic implementation.

I have only tested this patch on the i386-pc platform, so if anyone
tests it on another platform and finds problems, please let me know and
I can try to fix it.

Regards,
Colin
=== modified file 'ChangeLog'
--- ChangeLog	2008-06-21 14:21:03 +
+++ ChangeLog	2008-06-23 14:40:22 +
@@ -1,3 +1,87 @@
+2008-06-22  Colin D Bennett <[EMAIL PROTECTED]>
+
+	High resolution timer support.  Implemented for i386 CPU using TSC.
+	Extracted generic grub_millisleep() so it's linked in only as needed.
+
+	* conf/i386-efi.rmk: Added TSC high resolution time module, link with
+	generic grub_millisleep() function.
+
+	* conf/i386-pc.rmk: Likewise.
+
+	* conf/sparc64-ieee1275.rmk: Add kern/generic/millisleep.c and
+	kern/generic/get_time_ms.c to kernel, to use generic time functions.
+
+	* conf/powerpc-ieee1275.rmk: Add kern/generic/millisleep.c to kernel,
+	to use generic grub_millisleep() function.
+
+	* kern/generic/get_time_ms.c (grub_get_time_ms): New file.  Platform
+	independent implementation of grub_get_time_ms() using the RTC that
+	can be linked into a platform's kernel when it does not implement its
+	own specialized grub_get_time_ms() function.
+
+	* kern/generic/millisleep.c (grub_millisleep): New file.  Extracted
+	from grub_millisleep_generic() in kern/misc.c and renamed.  Now it is
+	not included in the kernel when a platform defines a specialized
+	grub_millisleep() implementation.
+
+	* include/grub/i386/tsc.h (grub_get_tsc): New file.  Inline function
+	grub_get_tsc() uses x86 RDTSC instruction (available on Pentium+ CPUs)
+	to read the counter value for the TSC.
+
+	* kern/i386/tsc.c (grub_get_time_ms): x86 TSC support providing a high
+	resolution clock.
+	(grub_tsc_calibrate): Static function to calibrate the TSC using RTC.
+	(grub_time_init): Implemented to call the static grub_tsc_calibrate()
+	function to calibrate the TSC.
+
+	* include/grub/kernel.h (grub_time_init): Add declaration for the
+	grub_time_init() platform specific time initialization function.  This
+	function should be implemented by all platforms.  If kern/i386/tsc.c
+	is linked in, it will provide grub_time_init().
+
+	* include/grub/time.h (grub_get_time_ms): Added grub_get_time_ms()
+	function to return the current time in millseconds since the epoch.
+	This supports higher resolution time than grub_get_rtc() on some
+	platforms such as i386-pc, where the RTC has only about 1/18 s
+	precision but a higher precision timer such as the TSC is available.
+
+	* kern/i386/efi/init.c (grub_millisleep): Don't define
+	grub_millisleep() -- it just called grub_millisleep_generic() but now
+	we just need to link with kern/generic/millisleep.c to use the generic
+	implementation.
+
+	* kern/i386/pc/init.c (grub_millisleep): Likewise.
+
+	* kern/ieee1275/init.c (grub_millisleep): Don't define
+	grub_millisleep() -- it just called grub_millisleep_generic() but now
+	we just need to link with kern/generic/millisleep.c to use the generic
+	implementation.
+	(grub_get_time_ms): Implement this required function.  Simply uses the
+	prior implementation of grub_get_rtc().
+	(grub_get_rtc): Now calls grub_get_time_ms(), which does the real
+	work.
+	(grub_time_init): Define this empty but required function. No further
+	initialization necessary.
+
+	* kern/sparc64/ieee1275/init.c (grub_millisleep): Don't define
+	grub_millisleep(), just link with kern/generic/millisleep.c.
+	(grub_time_init): Define this required function. Does nothing since
+	the generic RTC-based functions are used.
+
+	* kern/i386/linuxbios/init.c (grub_get_time_ms):
+	Define grub_get_time_ms() to always return 0.  This should be fixed
+	when RTC functionality is implemented.
+	(grub_time_init): Define this required function as a no-op. Inserted
+	a comment to remind us delete this function when proper time support 
+	is added to i386-linuxbios.
+
+	* kern/main.c (grub_main): Call grub_time_init() right after
+	grub_machine_init().
+
+	* kern/misc.c (grub_millisleep_generic): Moved to
+	kern/generic/millisleep.c so that it is only included in the kernel
+	image when a platform does not define a specialized version.
+
 2008-06-21  

Re: DEFLATE compression

[Colin D Bennett]

On Mon, 23 Jun 2008 23:25:13 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Mon, Jun 23, 2008 at 11:14 PM, Vesa Jääskeläinen <[EMAIL PROTECTED]>
> wrote:
> > Hi Bean,
> >
> > Can you check the interface as you have previously worked with it?
> > This would free Colin to work on more important aspects.
> 
> Hi,
> 
> Ok. But I have other things to fix at the moment, is this urgent ?

It is not urgent.  I will defer implementing font compression until a
later date, as it is not critical to the design or functionality: it is
merely a nice optimization.

Thanks,
Colin


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

grub_strdup() error; or: grub_malloc() zeroes memory?

[Colin D Bennett]

It looks like grub_strdup() does not terminate the returned string with
a 0 byte.  The only way I could see it working is if grub_malloc()
filled the returned memory with zeroes.  Does it?

From kern/misc.c: (circa line 476)

   char *
   grub_strdup (const char *s)
   {
 grub_size_t len;
 char *p;
 
 len = grub_strlen (s) + 1;
 p = (char *) grub_malloc (len);
 if (! p)
   return 0;

 return grub_memcpy (p, s, len);
   }

But right after that, we have

   char *
   grub_strndup (const char *s, grub_size_t n)
   {
 grub_size_t len;
 char *p;
 
 len = grub_strlen (s);
 if (len > n)
   len = n;
 p = (char *) grub_malloc (len + 1);
 if (! p)
   return 0;
 
 grub_memcpy (p, s, len);
 p[len] = '\0';
 return p;
   }

which explicitly stores a terminating null byte.  If grub_malloc() did
initialize the memory to zero, then this explicity store would be
unnecessary.

Am I missing something?

Regards,
Colin


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

Re: grub_strdup() error; or: grub_malloc() zeroes memory?

[Colin D Bennett]

On Sat, 28 Jun 2008 18:25:01 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Colin D Bennett wrote:
> > It looks like grub_strdup() does not terminate the returned string
> > with a 0 byte.  The only way I could see it working is if
> > grub_malloc() filled the returned memory with zeroes.  Does it?
> > 
> > From kern/misc.c: (circa line 476)
> > 
> >char *
> >grub_strdup (const char *s)
> >{
> >  grub_size_t len;
> >  char *p;
> >  
> >  len = grub_strlen (s) + 1;
> >  p = (char *) grub_malloc (len);
> >  if (! p)
> >return 0;
> > 
> >  return grub_memcpy (p, s, len);
> >}
> 
> Zero is copied from source string... notice strlen() + 1.

Doh!
/me smacks his forehead.
Thanks for pointing that out.

Cheers,
Colin


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

Re: [RFC] High resolution time support using x86 TSC

[Colin D Bennett]

On Thu, 03 Jul 2008 20:52:53 +0200
Marco Gerards <[EMAIL PROTECTED]> wrote:

> Hi Colin,
> 
> Colin D Bennett <[EMAIL PROTECTED]> writes:
> 
> > +   * kern/i386/linuxbios/init.c (grub_get_time_ms):
> > +   Define grub_get_time_ms() to always return 0.  This should
> > be fixed
> > +   when RTC functionality is implemented.
> > +   (grub_time_init): Define this required function as a
> > no-op. Inserted
> > +   a comment to remind us delete this function when proper
> > time support 
> > +   is added to i386-linuxbios.
> > +
> > +   * kern/main.c (grub_main): Call grub_time_init() right
> > after
> > +   grub_machine_init().
> 
> I think this should go into grub_machine_init?  Why didn't you just
> add it there?

Originally, I had grub_machine_init() call grub_tsc_calibrate(), but
these are comments Vesa made to me about that code:

Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote
on Mon, 16 Jun 2008 20:34:54 +0300:

> Colin D Bennett wrote:
> > This week I implemented high resolution timer support using the x86
> > TSC (see attached patch ).  It is
> > calibrated at GRUB startup using the RTC as a reference. The core
> > TSC function is ``grub_get_tsc()`` -- corresponding to
> > ``grub_get_rtc()``, but returning a uint64 value representing the
> > number of CPU cycles elapsed since boot.
> > 
> > In most situations, you will want to use ``grub_get_time_ms()`` to
> > get the system time in milliseconds based on the TSC value.  The
> > calibration function ``grub_tsc_calibrate()``, calculates the
> > proper scale factor and absolute offset so that the millisecond
> > value represents real time.
> > 
> > The ``grub_get_time_ms()`` function is implemented for non-x86
> > platforms to simply call ``grub_get_time_ms_generic()`` (defined in
> > kern/misc.c), which uses the RTC to get the time in milliseconds.  
> 
> We would probably want to leave that generic out from kernel, and let 
> every platform either use this generic code or implement their own 
> mechanism to do the job. Perhaps we should make own folder for
> generic stuff that can be included for arch specific build if there
> is no better replacement.
> 
> How about calling function like grub_time_init() which would then be 
> platform specific? Then platform can implement whatever way to
> calibrate (if needed) as long as it provides this grub_get_time_ms()
> function (also being platform specific). This would make
> initialization non-specific to arch while leaving more room for
> implementation.

Therefore, I thought this was the right way to do it.  Do you want me
to instead call grub_time_init() from grub_machine_init()?

Regards,
Colin


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

Re: [RFC] High resolution time support using x86 TSC

[Colin D Bennett]

Hi Marco,

On Thu, 03 Jul 2008 20:52:53 +0200
Marco Gerards <[EMAIL PROTECTED]> wrote:

> Hi Colin,
> 
> Colin D Bennett <[EMAIL PROTECTED]> writes:
> 
> > I have implemented high resolution time support (through the
> > new grub_get_time_ms() function) using the RDTSC instruction
> > available on Pentium and higher x86 CPUs.  The TSC value is simply
> > a 64-bit block cycle counter that is zeroed at bootup, so
> > grub_main() calls grub_time_init(), which is defined by each
> > platform.  If a platform links to kern/i386/tsc.c, then the
> > grub_time_init() function from tsc.c is used, which calibrates the
> > TSC rate and absolute zero reference using the RTC.
> What if TSC is not available?

I updated the changelog entry to indicate that running on a 386 or 486
will fail, since TSC is provided in Pentium+.  Do we support running on
386 or 386?  Should I check for this?  If so, the code will have to
change a bit, and be able to select between the generic implementation
and the TSC implementation at runtime.

I think this would be best done letting the "grub_get_time_ms"
implementation be selected by setting a function pointer in
grub_machine_init() depending on the machine's capabilities.  I would
have to significantly re-structure my patch, but it might be
necessary (and could lead to more understandable code?).
What do you think?

>...
> > +   * kern/i386/pc/init.c (grub_millisleep): Likewise.
> > +
> > +   * kern/ieee1275/init.c (grub_millisleep): Don't define
> > +   grub_millisleep() -- it just called
> > grub_millisleep_generic() but now
> > +   we just need to link with kern/generic/millisleep.c to use
> > the generic
> > +   implementation.
> 
> No need to mention how it has to be linked.  I just assume you made
> this change already?

Yes.  I removed that part of the changelog comment.

>...
> > +   (grub_time_init): Define this required function. Does
> > nothing since
> > +   the generic RTC-based functions are used.
> 
> No need to mention what a function does.  Only describe the changes
> you make.  Other information should go into the file itself as
> comments, if it is important enough.  Here it is only noise...

Ok.  I tried to clean this up.

> > +   * kern/i386/linuxbios/init.c (grub_get_time_ms):
> > +   Define grub_get_time_ms() to always return 0.  This should
> > be fixed
> > +   when RTC functionality is implemented.
> > +   (grub_time_init): Define this required function as a
> > no-op. Inserted
> > +   a comment to remind us delete this function when proper
> > time support 
> > +   is added to i386-linuxbios.
> > +
> > +   * kern/main.c (grub_main): Call grub_time_init() right
> > after
> > +   grub_machine_init().
> 
> I think this should go into grub_machine_init?  Why didn't you just
> add it there?

I commented on this in a separate message a few minutes ago.

> > +grub_uint64_t
> > +grub_get_time_ms (void)
> > +{
> > +  /* FIXME: Delete this function and link to `kern/i386/tsc.c'
> > once RTC
> > +   * is implemented.  See also comment below in grub_time_init().
> > */
> > +  return 0;
> > +}
> 
> Please do not use comments with *'s on each line.

Sorry, it was a habit of mine.  Fixed.

>...
> > +void
> > +grub_time_init (void)
> > +{
> > +  /* FIXME: Delete this function and link with `kern/i386/tsc.c'
> > once RTC
> > +   * is implemented.  Until then, this dummy function simply
> > defines 
> > +   * grub_time_init(), which is called by grub_main() in
> > `kern/main.c'.
> > +   * Without RTC, TSC calibration will hang waiting for an RTC
> > edge. */ +}
> > +
> 
> Same here.  Can you explain what is going on here?

Since grub_main() calls grub_time_init(), it must be defined.  However,
we can't use the TSC implementation of grub_time_init() from
kern/i386/tsc.c, since it will not be able to calibrate (it will hang)
if the RTC always returns the same value, which the i386-linuxbios
kernel does.

>...
> > +/* Reference for TSC=0.  This defines the time since the epoch
>in 
> > + * milliseconds that TSC=0 refers to. */
> > +static grub_uint64_t tsc_boot_time = 0;
> 
> Please do not use a * on each line for comments.  No need to set this
> to zero explicitly.

Ok.

> 
> > +/* TSC rate. TSC ticks per millisecond. 
> > + * Begin with default (incorrect) value of assuming a 1 GHz
> > machine.
> > + * grub_tsc_calibrate() must be called to set this properly. */
> > +static grub_uint64_t tsc_ticks_per_ms = 100;
> 
> Same as above.
> 
> The value is not correct

Re: [RFC] TSC patch v2

[Colin D Bennett]

Here is an updated version of the TSC patch.

I addressed the issues in your comments with the exception of
supporting x86 CPUs that don't have a TSC (386 and 486).

I eliminated the grub_time_init() function and the call to it in
grub_main() in favor of having grub_machine_init() simply call
grub_tsc_calibrate().  The i386-pc and i386-efi platforms use the TSC.

I also changed the grub_millisleep() generic function to use
grub_get_time_ms() instead of grub_get_rtc() to achieve better
precision when a higher resolution time function is available.

Please take a look at it and give me your comments when you have a
chance.  Thanks!

Regards,
Colin
=== modified file 'ChangeLog'
--- ChangeLog	2008-07-04 02:26:10 +
+++ ChangeLog	2008-07-04 18:08:36 +
@@ -1,3 +1,70 @@
+2008-07-04  Colin D Bennett <[EMAIL PROTECTED]>
+
+	High resolution timer support.  Implemented for i386 CPU using TSC.
+	Extracted generic grub_millisleep() so it's linked in only as needed.
+	This requires a Pentium compatible CPU; currently the code does not
+	check for this (so it will fail on 386 and 486 machines).
+
+	* conf/i386-efi.rmk: Added TSC high resolution time module, link with
+	generic grub_millisleep() function.
+
+	* conf/i386-pc.rmk: Likewise.
+
+	* conf/sparc64-ieee1275.rmk: Add kern/generic/millisleep.c and
+	kern/generic/get_time_ms.c to kernel, to use generic time functions.
+
+	* conf/powerpc-ieee1275.rmk: Add kern/generic/millisleep.c to kernel,
+	to use generic grub_millisleep() function.
+
+	* conf/i386-linuxbios.rmk: Added kern/generic/get_time_ms.c to the
+	kernel.
+
+	* kern/generic/get_time_ms.c (grub_get_time_ms): New file.  Platform
+	independent implementation of grub_get_time_ms() using the RTC that
+	can be linked into a platform's kernel when it does not implement its
+	own specialized grub_get_time_ms() function.
+
+	* kern/generic/millisleep.c (grub_millisleep): New file.  Extracted
+	from grub_millisleep_generic() in kern/misc.c and renamed.  Changed it
+	to use grub_get_time_ms() instead of grub_get_rtc() for better
+	precision on when high resolution time is available.
+
+	* kern/misc.c (grub_millisleep_generic): Deleted.  Moved to
+	kern/generic/millisleep.c so that it is only included in the kernel
+	image when a platform does not define a specialized version.
+
+	* commands/sleep.c (grub_interruptible_millisleep): Uses
+	grub_get_time_ms() instead of grub_get_rtc() to stay in sync with
+	grub_millisleep() from kern/generic/millisleep.c.
+
+	* include/grub/i386/tsc.h (grub_get_tsc): New file.  Inline function
+	grub_get_tsc() uses x86 RDTSC instruction (available on Pentium+ CPUs)
+	to read the counter value for the TSC.
+	(grub_tsc_calibrate): Declare this function for grub_machine_init().
+
+	* kern/i386/tsc.c (grub_get_time_ms): x86 TSC support providing a high
+	resolution clock.
+	(grub_tsc_calibrate): New function to calibrate the TSC using RTC.
+
+	* include/grub/time.h (grub_get_time_ms): Added grub_get_time_ms()
+	function to return the current time in millseconds since the epoch.
+	This supports higher resolution time than grub_get_rtc() on some
+	platforms such as i386-pc, where the RTC has only about 1/18 s
+	precision but a higher precision timer such as the TSC is available.
+
+	* kern/i386/efi/init.c (grub_millisleep): Deleted.  Don't define
+	grub_millisleep() -- it just called grub_millisleep_generic() but now
+	it is linked to kern/generic/millisleep.c for the implementation.
+
+	* kern/sparc64/ieee1275/init.c (grub_millisleep): Deleted.
+
+	* kern/i386/pc/init.c (grub_machine_init): Call grub_tsc_calibrate().
+	(grub_millisleep): Deleted.
+
+	* kern/ieee1275/init.c (grub_millisleep): Deleted.
+	(grub_get_rtc): Now calls grub_get_time_ms(), which does the real
+	work.
+
 2008-07-04  Pavel Roskin  <[EMAIL PROTECTED]>
 
 	* kern/i386/linuxbios/init.c (grub_machine_init): Cast addr to

=== modified file 'commands/sleep.c'
--- commands/sleep.c	2008-05-16 20:55:29 +
+++ commands/sleep.c	2008-07-04 16:55:48 +
@@ -43,15 +43,15 @@
   grub_printf ("%d", n);
 }
 
-/* Based on grub_millisleep() from kern/misc.c.  */
+/* Based on grub_millisleep() from kern/generic/millisleep.c.  */
 static int
 grub_interruptible_millisleep (grub_uint32_t ms)
 {
-  grub_uint32_t end_at;
-  
-  end_at = grub_get_rtc () + grub_div_roundup (ms * GRUB_TICKS_PER_SECOND, 1000);
-  
-  while (grub_get_rtc () < end_at)
+  grub_uint64_t start;
+
+  start = grub_get_time_ms ();
+
+  while (grub_get_time_ms () - start < ms)
 if (grub_checkkey () >= 0 &&
 	GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
   return 1;

=== modified file 'conf/i386-efi.rmk'
--- conf/i386-efi.rmk	2008-06-19 04:14:16 +
+++ conf/i386-efi.rmk	2008-07-03 04:19:16 +
@@ -84,7 +84,9 @@
 	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
 	kern/i386/dl.c kern/i386/efi/init.c kern/parser.c kern/parti

Re: [RFC] TSC patch v2

[Colin D Bennett]

On Fri, 04 Jul 2008 22:49:59 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Hi Colin,
> 
> Colin D Bennett wrote:
> > Here is an updated version of the TSC patch.
> > 
> > I addressed the issues in your comments with the exception of
> > supporting x86 CPUs that don't have a TSC (386 and 486).
> > 
> > I eliminated the grub_time_init() function and the call to it in
> > grub_main() in favor of having grub_machine_init() simply call
> > grub_tsc_calibrate().  The i386-pc and i386-efi platforms use the
> > TSC.
> 
> Please still use grub_time_init(). Detect TSC, if it is there
> calibrate and adapt for it. If it is not there, use RTC.

Should grub_time_init() be a CPU or machine independent function?
Should it use conditional compilation to be usable on different CPUs?
Or should grub_time_init() be specific to i386 kernels at this point?
(Since we don't have anything to initialize for non-TSC-supporting
platforms.)

If grub_time_init() is specific to the i386 CPU, then grub_time_init()
should check whether the CPU has TSC support, and if so,
grub_tsc_calibrate() should be called, and calls to grub_get_time_ms()
should be directed to the TSC implementation.  If no TSC support is
available, then no calibration is done, and calls to grub_get_time_ms()
should be directed to the generic get_time_ms() implementation that
uses grub_get_rtc() to do its job.

I want to make sure that I clearly understand what you want, since I
have made major re-designs of the TSC support in GRUB in an attempt to
do what you want.  Just in the past couple of days I have spent about 8
hours getting the TSC patch where it is now. (I know that seems
like a lot since it's not that much code, but I am just beginning to get
familiar with the GRUB kernel organization, so simple changes have been
taking me a long time.)

> grub_time_init() is just more clearer term than tsc. At least for
> people that are not familiar with it. in grub_time_init() you can
> have anything then. And people that are not interested on time stuff
> do not need to bother their heads about it. Also modifying logic
> afterwards do not change other parts and it will be less error prone.

OK.

Regards,
Colin


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

Re: subversion repository structure

[Colin D Bennett]

On Sun, 13 Jul 2008 03:54:44 +0200
"Yoshinori K. Okuji" <[EMAIL PROTECTED]> wrote:

> Hello,
> 
> Thank all of you for making effort on moving the version control
> system to Subversion. I think this is a big improvement, and I like
> it.
> 
> Now, I would like to talk about the structure. Currently, it looks
> like this:
> 
> trunk/
>   grub/
>   grub2/
> tags/
>   release*/
> branches/
>   prepare*/
> 
> IMO, this is not very convenient. This means that, when we want to
> create, say, version 1.99, we would tag both grub and grub2. This
> does not sound good to me.
> 
> So, I think that this would be more appropriate:
> 
> grub/
>   trunk/
>   tags/
>   branches/
> grub2/
>   trunk/
>   tags/
>   branches/
> 
> Since grub and grub2 are totally different, it would be better to
> segregate them completely this way.
> 
> What is your opinion?

I agree!  Your suggestion seems very logical.

Can we simply copy trunk/grub2 to grub2/trunk (and so on for the other
"branches")?  Then those of us mirroring the subversion tree might be
able to just switch to the new URL for trunk... I hope.

Regards,
Colin


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

Re: Next release?

[Colin D Bennett]

On Tue, 15 Jul 2008 19:52:15 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Wed, 2008-07-16 at 01:32 +0200, Yoshinori K. Okuji wrote:
> 
> > If a boot drive is the same as a root drive, you are right.
> > Otherwise we need to do so.
> >
> > I think we have seen tons of examples with GRUB Legacy which may
> > not be solved automatically in all cases. If one digs into the
> > archive of bug-grub, I guess several cases would be found easily.
> > With GRUB 2, we can avoid embedding BIOS drive numbers in many
> > cases, using UUIDs or labels or files. But this does not always
> > work, so I am afraid that we need to support device.map, even if it
> > is an evil necessity.
> 
> That's a very advanced setup.  I actually cannot imagine why anyone
> would use different boot and root drives.  Well, maybe the boot drive
> has no partitions that GRUB or the host OS can access? 

I have used machines that have multiple Linux versions spread across
two drives, but one common /boot partition so they can all be booted
from GRUB.  This doesn't seem unusual to me.


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

Re: subversion repository structure

[Colin D Bennett]

On Thu, 17 Jul 2008 11:20:47 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Wed, Jul 16, 2008 at 6:23 AM, Pavel Roskin <[EMAIL PROTECTED]> wrote:
> > On Tue, 2008-07-15 at 23:02 +0200, Yoshinori K. Okuji wrote:
> >
> >> I don't agree on this. GRUB Legacy and GRUB 2 are developed fully
> >> independently (if any). If we follow your way, the repository
> >> would look like this:
> >>
> >> branches/
> >>   grub-legacy/
> >>   prepare_0_97/
> >>   prepare_0_98/
> >>   prepare_1_98/
> >>   prepare_1_99/
> >>   ...
> >>
> >> I feel that this has no logical structure. When we make a branch
> >> for GRUB 2, we put it under branches, when we modify the "trunk"
> >> of GRUB Legacy, we do under branches, when we make a branch for
> >> GRUB Legacy, we use branches...
> >
> > Yes, that's my suggestion.  I understand that you may feel uneasy
> > about it, but I don't think we are going to do many releases from
> > the legacy branch, maybe one or none at all.
> >
> > It's OK to have stable and development branches.  grub-legacy is
> > essentially our stable branch, even though it didn't start as a
> > branch.
> >
> > CVS is inherently asymmetric.  Certain things just don't work on
> > branches the way they work on trunk.  That's why it was reasonable
> > to avoid branches with CVS for anything but release preparation.
> >
> > Subversion is (more) symmetric.  It's possible to develop on any
> > branch, check the entire history of files, merge changes from other
> > branches. Separating trunk from other branches in the standard
> > Subversion repository layout is primarily to give comfort to CVS
> > users, who are used to having one trunk with a special status.
> >
> > We could have GRUB 2 under branches too and have no trunk.  But
> > having GRUB 2 as the trunk gives us the standard layout, which is a
> > good thing. In any case, I think it's better than any of the
> > "two-headed" solutions.
> 
> Hi,
> 
> If we're using branches, I suggest the following layout:
> 
> branches
>grub-legacy
> trunk
> tags
>   grub-0.97
>   grub-1.96
>   ...
> 
> trunk is grub2, the current develop branch, and grub-legacy is under
> branches.

This layout makes perfect sense if we are treating GRUB legacy and GRUB
2 as the same project, but where GRUB 2 is now the development mainline
and GRUB legacy is a past release codeline.

On the other hand, if GRUB legacy and GRUB 2 are considered separate
projects, then having completely separate sub-trees like:

grub-legacy/
  trunk/ - 0.9x mainline
  tags/
grub-0.97- tagged release
  branches/
grub/
  trunk/ - GRUB 2 mainline
  tags/
grub-1.96- GRUB v1.96 release
  branches/
  
makes more sense.  I think we could actually go either way.  It's not
as if there is going to be much activity on grub-legacy anyway, right?

Regards,
Colin


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

Re: Idea: elimination of the normal mode (revised version)

[Colin D Bennett]

On Thu, 17 Jul 2008 11:24:23 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Mon, Jul 7, 2008 at 8:29 AM, Bean <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > First of all, we can still keep rescue and normal command. But
> > instead of depending on normal.mod, normal command depends on
> > module arg, which is an option parser. Also, these two type of
> > commands are of the same command set. In fact, module arg is
> > implemented as a pre parser, which goes through the list of
> > arguments and extract the options. In the case of rescue command,
> > the pre parser field is null, which means it wants to parse options
> > itself.
> >
> > Then, I think of a new structure to represent all configurable
> > handlers of grub. Different types of handler have different fields,
> > but they all share a command header:
> >
> > struct grub_handler
> > {
> >  .next,
> >  .name,
> >  .init,
> >  .fini
> > };
> >
> > Same type of handlers are linked together. We first define an enum
> > to list all types. For example:
> >
> > enum {
> >  GRUB_HANDLER_INPUT,
> >  GRUB_HANDLER_OUTPUT,
> >  GRUB_HANDLER_CONSOLE,
> >  GRUB_HANDLER_MENU,
> >  GRUB_HANDLER_SCRIPT,
> >  GRUB_HANDLER_NUM
> > };
> >
> > Then, we define an array to point to the head of handler linked
> > list: grub_handler[GRUB_HANDLER_NUM];
> >
> > Head is the default selection. When we insert a new handler module,
> > it would automatically become the new default, although we can
> > switch back to old handler using a command.
> >
> > Here are more details about different handlers:
> >
> > input:
> > This is the input component of terminal:
> >
> > struct grub_handler_input
> > {
> >  .next,
> >  .name,
> >  .init,
> >  .fini,
> >  .checkkey,
> >  .getkey
> >  .flags,
> > };
> >
> > output:
> > This is the output component of terminal:
> >
> > struct grub_handler_output
> > {
> >  .next,
> >  .name,
> >  .init,
> >  .fini,
> >  .putchar,
> >  .getcharwidth,
> >  .getxy,
> >  .gotoxy,
> >  .cls,
> >  .setcolorstate,
> >  .setcursor,
> >  .flags,
> > };
> >
> > console interface:
> > It represent the grub console, users type commands and execute them
> > line by line.
> >
> > struct grub_handler_console
> > {
> >  .next,
> >  .name,
> >  .init,
> >  .fini,
> >  .run
> > };
> >
> > menu interface:
> > It represent the menu, users select a menu item and execute it.
> >
> > struct grub_handler_menu
> > {
> >  .next,
> >  .name,
> >  .init,
> >  .fini,
> >  .run
> > };
> >
> > script engine:
> > It's responsible for parsing config file to get the menu list, and
> > execution of commands.
> >
> > struct grub_handler_script
> > {
> >  .next,
> >  .name,
> >  .init,
> >  .fini,
> >  .readconfig
> >  .getmenu
> >  .execute
> > };
> >
> > The handlers are independent of each other. When they need
> > something, they called specific function of the default handler.
> > For example, to read a key from the console, we can use
> > grub_handler[GRUB_HANDLER_INPUT]->getkey. Also, to get the list of
> > items to be displayed on screen, the menu handler can call
> > grub_handler[GRUB_HANDLER_SCRIPT]->getmenu.
> 
> Any comment for this idea ?

I like the idea, especially the idea of separating the script engine
and menu loading code from the character-terminal-based menu system.

I wonder whether it would be better just to have separate variables for
the different handler types like

struct grub_handler_input grub_handler_input;
struct grub_handler_output grub_handler_output;
struct grub_handler_console grub_handler_console;

instead of an array indexed by a constant representing the type, for
the following reasons:

1. You would have to use casting to be able to call the functions in
the handler other than the common 4 (next, name, init, fini).  For
instance, you couldn't do 
  grub_handler[GRUB_HANDLER_INPUT]->getkey()
since the type of grub_handler is struct grub_handler[].

2. If we did use casting for each call to a handler, it is more error
prone since we would not get compiler type checking and could
accidentally write GRUB_HANDLER_INPUT when we meant GRUB_HANDLER_OUTPUT
with obvious consequences.

3. It is more code to write to do
  grub_handler[GRUB_HANDLER_INPUT]->getkey()
instead of
  grub_handler_input->getkey()


Now, suppose we did define grub_handler_input, grub_handler_output,
etc.  Then if there were a reason to iterate over all types of
handlers, or treat handlers generically for some other reason, we could
certainly also create and array of pointers to the handlers for that
purpose:

extern struct grub_handler *grub_handlers[GRUB_HANDLER_NUM];
extern const char *grub_handler_types[GRUB_HANDLER_NUM];

void print_current_handlers (void)
{
  int i;
  for (i = 0; i < GRUB_HANDLER_NUM; i++)
grub_printf ("handler for %s: %s\n",
  grub_handler_types[i],
  grub_handlers[i]);
}

We would then have to make sure the list of all handlers was kept in
sync with the separate variables.  This would mean a little more code
for each handler type (a glo

Re: menu loop (patch)

[Colin D Bennett]

On Sat, 19 Jul 2008 17:40:43 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> On Thu, Jul 17, 2008 at 11:22:27PM +0200, Carles Pina i Estany wrote:
> > 
> > Hello,
> 
> Carles!  Que n'és de petit el món ;-)
> 
> > I'm a Grub user (thanks for your nice work!) and I always wanted to
> > have a menu that "loops". Like, if you press down and you are in
> > the last option it goes to the first one, and if you press up but
> > you are int he first option goes to the last.
> > 
> > Attached comes a patch against revision 1718.
> > 
> > Of course, feel free to comment anything about it. I'm open to
> > change, fix, etc.
> > 
> > I also don't know if this functionality is not in Grub because you
> > thought that was better to not have this functionality. 
> > 
> > I've copied/change a few lines (after the patch, lines 425-428 are
> > the same than lines 347-350). I don't think that it's needed to
> > refactor but if you think so I can do it.
> 
> I'm not sure if this is intentional.  Size is not important in this
> part of the code, but in other UI menus (e.g. Iceweasel or GNOME) I
> notice this is not done.  It could also be a nuissance for users who
> want to go rapidly to the top or bottom (sure, they can use
> PgUp/PgDn, but then aren't we making the behaviour of those keys
> inconsistent with arrows?).
> 
> Also, if you (or anyone) can point to a "usability best practices"
> reference saying this has been carefuly analized and one of the
> options was found to be better, I think this would help us make a
> decision.
> 
> What does everyone else think?

I think that wrapping around the menu with the arrow keys *can* be
useful in some situations, but in general it seems like we should try
to be consistent with the common UI behavior that is expected, at least
by default.  We could support a setting such as "set menuwrap=1" which
would enable this feature for users who care about it.  Then the users
who don't care won't miss it, but the users who do care can still have
it by adding a single line to grub.cfg.

> Colin, does this affect your graphical menu work in some way?  I
> suppose the same situation applies the same way to the upcoming new
> menu.  Or perhaps this can be made more flexible and be toggled by
> some CSS magic?

This would be no problem at all; it's essentially identical to how the
text mode menu from normal.mod works.  It could go either way, and
certainly would be easy to make it a setting in the theme file (or more
likely in a user-configurable place such as grub.cfg (?) -- since we
might not expect most users to modify theme files but only choose which
one to use).

Actually my graphical menu currently *does* wrap around, I guess it
seemed logical to me at the time I wrote the code!  8-)  As you can
tell, I am not firmly set on either wrapping or not wrapping.

Regards,
Colin


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

Re: menu loop (patch)

[Colin D Bennett]

On Sun, 20 Jul 2008 00:06:40 +0200
Carles Pina i Estany <[EMAIL PROTECTED]> wrote:

> 
> Hello,
> 
> On Jul/19/2008, Colin D Bennett wrote:
> > On Sat, 19 Jul 2008 17:40:43 +0200
> > Robert Millan <[EMAIL PROTECTED]> wrote:
> > 
> > > On Thu, Jul 17, 2008 at 11:22:27PM +0200, Carles Pina i Estany
> > > wrote:
> 
> > > > I'm a Grub user (thanks for your nice work!) and I always
> > > > wanted to have a menu that "loops". Like, if you press down and
> > > > you are in the last option it goes to the first one, and if you
> > > > press up but you are int he first option goes to the last.
> 
> [...]
> 
> > > What does everyone else think?
> > 
> > I think that wrapping around the menu with the arrow keys *can* be
> > useful in some situations, but in general it seems like we should
> > try to be consistent with the common UI behavior that is expected,
> > at least by default.  We could support a setting such as "set
> > menuwrap=1" which would enable this feature for users who care
> > about it.  Then the users who don't care won't miss it, but the
> > users who do care can still have it by adding a single line to
> > grub.cfg.
> 
> in one way, add an options for this looks a bit like "overkill" (as
> Robert said). In the other hand, I have my hopes that distributions
> would like it and (mainly) all of them would add this option by
> default :-) so could be a nice way to know what some people think.

I agree it is probably not worth creating an option for.  It is
probably best to just decide to wrap or not to wrap... "that is the
question"  ;-)

> > > Colin, does this affect your graphical menu work in some way?  I
> > > suppose the same situation applies the same way to the upcoming
> > > new menu.  Or perhaps this can be made more flexible and be
> > > toggled by some CSS magic?
> > 
> > This would be no problem at all; it's essentially identical to how
> > the text mode menu from normal.mod works.  It could go either way,
> > and certainly would be easy to make it a setting in the theme file
> > (or more likely in a user-configurable place such as grub.cfg (?)
> > -- since we might not expect most users to modify theme files but
> > only choose which one to use).
> > 
> > Actually my graphical menu currently *does* wrap around, I guess it
> 
> I'm new in this list and in Grub2 (I hvae not used, actually). Are you
> saying that there is some graphical menu that does wrap?

I'm working on a graphical menu system for the Google Summer of Code
2008.  You can find out more about it at:
 
  http://grub.gibibit.com/Journal

It's not in the GRUB codebase yet, but it under heavy development.

> > seemed logical to me at the time I wrote the code!  8-)  As you can
> > tell, I am not firmly set on either wrapping or not wrapping.
> 
> my workmate wanted it, i wanted and I've spoke with my brother and he
> really wanted. But maybe it's because brother-love :-) (this is not
> proving anything, of course)

I'm fine with either option -- I'll just make it consistent with
whatever the core GRUB developers agree on.

Regards,
Colin


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

Re: menu loop (patch)

[Colin D Bennett]

On Sun, 20 Jul 2008 01:31:55 +0200
Carles Pina i Estany <[EMAIL PROTECTED]> wrote:

> 
> Hello,
> 
> On Jul/19/2008, Isaac Dupree wrote:
> > Carles Pina i Estany wrote:
> >> fast check in KDE: it's looping
> >>
> >> Also, Iceweasel menus are looping here (maybe because it's executed
> >> in KDE and there is some GTK Widget with a different behaviour? I
> >> have no idea)
> >
> > GTK menubar-menus are looping for me.  But I'm not sure about menus
> > that consist of selecting from a list in a box (maybe some login
> > window?).  I don't care personally, because my GRUB-lists are not
> > terribly long... hmm, if you hold down a (arrow) key in GRUB, is
> > there auto-repeat?
> 
> no, there isn't auto-repeat (this is what the patch that we are
> talking does).

I think Isaac means: is there key-repeat functionality, and the answer
is yes; the BIOS int 16h key functions supply the key auto-repeat
functionality.  You are talking about "looping" around the menu when an
attempt is made to go past the beginning or end.

Regards,
Colin


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

Re: Strong Crypto Support for GRUB2

[Colin D Bennett]

On Mon, 21 Jul 2008 01:49:52 +0200
Simon Peter <[EMAIL PROTECTED]> wrote:

> Did the message below ever arrive on the list?
> 
> I'm still interested in getting strong crypto into grub mainline and
> while it's still not in, I just saw you guys proposed this as a
> project for Google's summer of code. Are you going to point students
> at the code I already produced? Would be a shame if you have someone
> replicate the work already done.
> 
> What's missing to get my code into GRUB2 mainline?

I think you'll have to assign copyright to the FSF.

Regards,
Colin


> Thanks!
> Simon
> 
> Begin forwarded message:
> 
> Date: Sun, 3 Feb 2008 18:25:47 +0100
> From: Simon Peter <[EMAIL PROTECTED]>
> To: The development of GRUB 2 
> Cc: Marco Gerards <[EMAIL PROTECTED]>, Robert Millan
> <[EMAIL PROTECTED]> Subject: Re: Strong Crypto Support for GRUB2
> 
> 
> > Sorry for the *very* late reply.
> 
> Sorry for my late reply. I wasn't subscribed anymore (and I'm still
> not) and only now I found out that you replied...
> 
> Attached is my patch against current CVS with the changes you proposed
> incorporated.
> 
> I'd appreciate very much if you could review this complete patch
> again. Sorry for any inconvenience with my first weird double-patch.
> 
> > > +int
> > > +aes_self_test (int verbose)
> > >  {
> [...]
> > > + printf ("passed\n");
> > Where does printf come from?
> 
> It's in a self-test routine that's actually never included in the GRUB
> source. Shall I remove the whole thing?
> 
> Thanks!
> Simon
> 
> 


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

[PATCH] File readahead buffering

[Colin D Bennett]

This patch speeds up loading a TGA image on my test system from 29
seconds to approximately 1 second.

I noticed that on my 1 GHz test system running from an IDE CompactFlash
drive, loading a certain TGA image in GRUB takes about 29 seconds.
This seems pretty outrageous, so after looking at the code I hacked the
TGA reader code to read the entire TGA image file into memory with a
single grub_file_read() call.  This made it much faster.  Therefore, I
concluded that the problem was that each of the one- to four-byte
reads done by the TGA reader was causing an actual device read.  It
seems that it is the file subsystem's responsibility to do file
buffering, so I implemented it in `kern/file.c'.

It turns out that when booting GRUB from IDE, if file buffering is
used, GRUB hangs right after the "GRUB" message, early in the boot
process.  So I added a flag that allows grub_main() to enable buffering
when it is safe to do so.  It always worked file from an ISO image
(generated with grub-mkrescue) in VMware and QEMU, but when I actually
installed GRUB to my CompactFlash drive and booted it, it hung after
"GRUB" if file buffering was enabled at the start.

It may be poor style to expose `grub_file_buffering_enabled' as a
global variable in file.h, but after I refactored it as a function, I
wondered whether the overhead of creating a whole new function as a
'setter' was wasteful since it would be called only once.  Please
advise if you prefer it as a function, or if there is a better solution.

Regards,
Colin
=== modified file 'ChangeLog'
--- ChangeLog	2008-07-21 09:40:01 +
+++ ChangeLog	2008-07-22 15:42:00 +
@@ -1,3 +1,17 @@
+2008-07-22  Colin D Bennett <[EMAIL PROTECTED]>
+
+	* include/grub/file.h (grub_file_buffering_enabled): New global
+	variable.
+
+	* kern/file.c (BUFFER_SIZE): New constant.
+	(grub_file_buffering_enabled): New global variable.
+	(grub_file_open): Allocate file readahead buffer.
+	(grub_file_close): Free file readahead buffer.
+	(grub_file_read): Use file readahead buffer.
+
+	* kern/main.c (grub_main): Enable file readahead buffering when it is
+	safe to do so.
+
 2008-07-21  Bean  <[EMAIL PROTECTED]>
 
 	* kern/i386/pc/startup.S (gate_a20_try_bios): Change test order for

=== modified file 'include/grub/file.h'
--- include/grub/file.h	2007-08-02 17:40:37 +
+++ include/grub/file.h	2008-07-22 15:23:31 +
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2007,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
@@ -39,6 +39,19 @@
   /* The file size.  */
   grub_off_t size;
 
+  /* The file offset when `buffer' starts, or -1 to indicate that
+ buffer has no data.  */
+  grub_off_t buffer_offset;
+
+  /* The number of bytes in `buffer'.  */
+  grub_size_t buffer_length;
+
+  /* The number of bytes allocated for `buffer'.  */
+  grub_size_t buffer_capacity;
+
+  /* Readahead buffer.  */
+  char *buffer;
+
   /* Filesystem-specific data.  */
   void *data;
 
@@ -48,6 +61,11 @@
 };
 typedef struct grub_file *grub_file_t;
 
+/* Enable file readahead buffering if nonzero.  Buffering is disabled by
+   default, since it seems to cause problems during the early GRUB boot
+   process.  The grub_main function enables it.  */
+extern int grub_file_buffering_enabled;
+
 /* Get a device name from NAME.  */
 char *EXPORT_FUNC(grub_file_get_device_name) (const char *name);
 

=== modified file 'kern/file.c'
--- kern/file.c	2008-01-25 20:57:40 +
+++ kern/file.c	2008-07-22 15:23:31 +
@@ -1,7 +1,7 @@
 /* file.c - file I/O functions */
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2002,2006,2007  Free Software Foundation, Inc.
+ *  Copyright (C) 2002,2006,2007,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
@@ -24,6 +24,12 @@
 #include 
 #include 
 
+/* File readahead buffer size in bytes.  */
+#define BUFFER_SIZE 1024
+
+/* Use readahead buffering for file reads?  */
+int grub_file_buffering_enabled = 0;
+
 /* Get the device part of the filename NAME. It is enclosed by parentheses.  */
 char *
 grub_file_get_device_name (const char *name)
@@ -81,6 +87,10 @@
   
   file->device = device;
   file->offset = 0;
+  file->buffer_offset = -1;
+  file->buffer_length = 0;
+  file->buffer_capacity = 0;
+  file->buffer = 0;
   file->data = 0;
   file->read_hook = 0;
 
@@ -97,6 +107,13 @@
   if ((file->fs->open) (file, file_name) != GRUB_ERR_NONE)
 goto fail;
 
+  if (grub_file_buffering_enabled)
+{
+  file->buffer = grub_malloc (BUFFER_SIZE);
+  if (file->buffer)
+   

Re: [PATCH] File readahead buffering

[Colin D Bennett]

On Tue, 22 Jul 2008 14:48:31 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-07-22 at 08:43 -0700, Colin D Bennett wrote:
> > This patch speeds up loading a TGA image on my test system from 29
> > seconds to approximately 1 second.
> > 
> > I noticed that on my 1 GHz test system running from an IDE
> > CompactFlash drive, loading a certain TGA image in GRUB takes about
> > 29 seconds.
> 
> I'm sorry for straying from your point, but maybe we should drop TGA
> support.  It was the first image format for GRUB to support, but now
> PNG is supported, and it should be better in all aspects.

I agree that TGA is not, in general, a great choice for an image format
(unless it is faster to load a large background image -- a 1024x768 RGB
PNG file may take more time to decompress than a TGA image would take
to load -- although perhaps an uncompressed PNG file would be
comparable in speed to load).  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.

> > It turns out that when booting GRUB from IDE, if file buffering is
> > used, GRUB hangs right after the "GRUB" message, early in the boot
> > process.  So I added a flag that allows grub_main() to enable
> > buffering when it is safe to do so.  It always worked file from an
> > ISO image (generated with grub-mkrescue) in VMware and QEMU, but
> > when I actually installed GRUB to my CompactFlash drive and booted
> > it, it hung after "GRUB" if file buffering was enabled at the start.
> 
> I think we should be prefer simple and reliable code, rather than add
> complexity and risks of failure to achieve higher speeds.

If the buffering is not done in the file I/O layer, then the font
loading, theme file loading, image file loading, etc. will all need to
do their own buffering, which IMHO is more error prone and makes those
modules use more code to handle low level I/O stuff, which detracts
from their specific purpose.

Also, this is no small increase in speed, but from 10x to 100x increase
in performance for some cases where small sequential reads are
performed.

Regards,
Colin


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

Re: [PATCH] File readahead buffering

[Colin D Bennett]

On Tue, 22 Jul 2008 17:44:53 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> On Tue, 2008-07-22 at 12:06 -0700, Colin D Bennett wrote:
> 
> > I agree that TGA is not, in general, a great choice for an image
> > format (unless it is faster to load a large background image -- a
> > 1024x768 RGB PNG file may take more time to decompress than a TGA
> > image would take to load -- although perhaps an uncompressed PNG
> > file would be comparable in speed to load).  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.
> 
> Strange.  It's working for me.  You may want to post that file to the
> list.

Actually I'm testing it now, and PNG images are working... I'll post an
update when I have finished testing.  Maybe the failures I experienced
in the past were due to an old version of GRUB.
 
> > If the buffering is not done in the file I/O layer, then the font
> > loading, theme file loading, image file loading, etc. will all need
> > to do their own buffering, which IMHO is more error prone and makes
> > those modules use more code to handle low level I/O stuff, which
> > detracts from their specific purpose.
> > 
> > Also, this is no small increase in speed, but from 10x to 100x
> > increase in performance for some cases where small sequential reads
> > are performed.
> 
> OK, then you may have a case.

FWIW, here are some results of timing the loading of the same 1024x768
image in TGA, PNG, and JPEG formats -- first without the file buffering
patch, and then with the file buffering patch:

(Loading times in seconds.)

   TGA  PNG 0  PNG 100  JPEG q80  JPEG q100
 -  -  ---    -
   No buffering 17 912 1  7 
  Buffering  1 1 1 1  1

I see that PNG files load at least as fast as TGA files, so that is
great news.  It also seems to show the degree of benefit provided by
using buffered file reads.  These tests were done by using the
'tgatest', 'pngtest', and 'jpegtest' commands, and manually timing the
results (accuracy +/- one second or so).

Regards,
Colin


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

Re: [PATCH] File readahead buffering

[Colin D Bennett]

On Wed, 23 Jul 2008 10:05:36 +0800
Bean <[EMAIL PROTECTED]> wrote:

> 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:
> >
> >> On Tue, 2008-07-22 at 08:43 -0700, Colin D Bennett wrote:
> >> > This patch speeds up loading a TGA image on my test system from
> >> > 29 seconds to approximately 1 second.
> >> >
> >> > I noticed that on my 1 GHz test system running from an IDE
> >> > CompactFlash drive, loading a certain TGA image in GRUB takes
> >> > about 29 seconds.
> >>
> >> I'm sorry for straying from your point, but maybe we should drop
> >> TGA support.  It was the first image format for GRUB to support,
> >> but now PNG is supported, and it should be better in all aspects.
> >
> > I agree that TGA is not, in general, a great choice for an image
> > format (unless it is faster to load a large background image -- a
> > 1024x768 RGB PNG file may take more time to decompress than a TGA
> > image would take to load -- although perhaps an uncompressed PNG
> > file would be comparable in speed to load).  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.

Ok, after some more testing, I have found that if I create PNG images
with ImageMagick or The GIMP, they work.  However, I have an image that
I was trying to use as a GRUB background which is from a MythTV theme
called Titivillus.  The PNG file from the MythTV theme doesn't load in
GRUB.  I get the message "error: png: block type fixed not supported".

The file is at 
http://gibibit.com/upload/bg-myth.png
and ImageMagick's "identify -verbose" tells me it was created with
Adobe ImageReady.

Since the free tools that I've tried seem to work fine, this is not a
big deal to me, but perhaps before a widespread GRUB 2 release we might
want to improve the PNG reader compatibility.

Regards,
Colin


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

Re: [PATCH] File readahead buffering

[Colin D Bennett]

On Wed, 23 Jul 2008 07:33:32 -0700
Colin D Bennett <[EMAIL PROTECTED]> wrote:

> On Wed, 23 Jul 2008 10:05:36 +0800
> Bean <[EMAIL PROTECTED]> wrote:
> 
> > 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:
> > >
> > >> On Tue, 2008-07-22 at 08:43 -0700, Colin D Bennett wrote:
> > >> > This patch speeds up loading a TGA image on my test system from
> > >> > 29 seconds to approximately 1 second.
> > >> >
> > >> > I noticed that on my 1 GHz test system running from an IDE
> > >> > CompactFlash drive, loading a certain TGA image in GRUB takes
> > >> > about 29 seconds.
> > >>
> > >> I'm sorry for straying from your point, but maybe we should drop
> > >> TGA support.  It was the first image format for GRUB to support,
> > >> but now PNG is supported, and it should be better in all aspects.
> > >
> > > I agree that TGA is not, in general, a great choice for an image
> > > format (unless it is faster to load a large background image -- a
> > > 1024x768 RGB PNG file may take more time to decompress than a TGA
> > > image would take to load -- although perhaps an uncompressed PNG
> > > file would be comparable in speed to load).  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.
> 
> Ok, after some more testing, I have found that if I create PNG images
> with ImageMagick or The GIMP, they work.  However, I have an image
> that I was trying to use as a GRUB background which is from a MythTV
> theme called Titivillus.  The PNG file from the MythTV theme doesn't
> load in GRUB.  I get the message "error: png: block type fixed not
> supported".

Oops!  After more testing, I have found PNG images that I saved in The
GIMP which also refuse to load due to the "block type fixed not
supported" error.  One ZIP file [1] contains images that all fail to
load in GRUB with that error, and the other [2] contains similar images
that load just fine.

[1] gibibit.com/upload/grub-png-bad.zip
[2] gibibit.com/upload/grub-png-good.zip

Regards,
Colin


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

Re: gfxterm distinction between fg color and cursor color

[Colin D Bennett]

On Thu, 24 Jul 2008 18:18:40 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> Why does gfxterm make a distinction between foreground color and
> cursor color?  Do we really want this?
> 
> Currently, the variable-based hooks for changing colors have no
> effect in cursor color because of this.
> 
> How about supressing separate handling of cursor color, and simply
> use the foreground color to draw it?
> 
> Or maybe it'd be better to have setcolor() set cursor color
> implicitly?

I think it would probably be fine to simply use the foreground color as
the cursor color.


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

Re: Linking

[Colin D Bennett]

On Fri, 25 Jul 2008 03:22:57 -0700 (PDT)
Viswesh S <[EMAIL PROTECTED]> wrote:

> The reason for hello to be put in a directory because it is a module
> and not a simple command.Understood myself

Actually, no.  Every C source file in the 'commands' directory is
currently a module itself.

I think the reason that 'hello.mod' is separated out is to make it
easier to see that there is only one source file as part of 'hello.mod'.

Regards,
Colin


> - Original Message 
> From: Viswesh S <[EMAIL PROTECTED]>
> To: The development of GRUB 2 
> Sent: Friday, 25 July, 2008 3:44:48 PM
> Subject: Re: Linking
> 
> 
> Hi,
>  
> So if I add a module or add a new command,which are the files which I
> should modify in general and which all gets generated automatically. 
> Is this somewhere documented ?
>  
> Why is it that hello command is put in a separate directory, while
> lspci is put inside the commands directory. 
> Is there a specific reason for this.
>  
>  
> Viswesh
> 
> 
> 
> - Original Message 
> From: Pavel Roskin <[EMAIL PROTECTED]>
> To: grub-devel@gnu.org
> Sent: Friday, 25 July, 2008 11:59:21 AM
> Subject: Re: Linking
> 
> Quoting Viswesh S <[EMAIL PROTECTED]>:
> 
> > Hi Pavel,
> > Could you please attach the modified file ?
> > Viswesh
> 
> No, I'm just giving you an idea what you may be doing wrong.
> 


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

Re: Linking

[Colin D Bennett]

On Fri, 25 Jul 2008 07:33:41 -0700 (PDT)
Viswesh S <[EMAIL PROTECTED]> wrote:

> Suppose I am adding a new command,so should I create a new file for
> that command and modify only the command,rmk accordingly? All the
> other files get autogenerated by autogen.sh ? Is this sufficient ?
> Viswesh

Yes.  Actually when you run 'make', the conf/*.mk files are regenerated
from the conf/*.rmk files if necessary, so you don't need to re-run
autogen.sh after changing .rmk files.

I was puzzled with the conf/* files at first too, and though I had to
change both, and after looking at the .mk files, I was saddened.
However, now that I know to change only the .rmk files, life is happy
and the build system is actually easy to work with.

Regards,
Colin


> - Original Message 
> From: Colin D Bennett <[EMAIL PROTECTED]>
> To: grub-devel@gnu.org
> Sent: Friday, 25 July, 2008 7:31:28 PM
> Subject: Re: Linking
> 
> On Fri, 25 Jul 2008 03:22:57 -0700 (PDT)
> Viswesh S <[EMAIL PROTECTED]> wrote:
> 
> > The reason for hello to be put in a directory because it is a module
> > and not a simple command.Understood myself
> 
> Actually, no.  Every C source file in the 'commands' directory is
> currently a module itself.
> 
> I think the reason that 'hello.mod' is separated out is to make it
> easier to see that there is only one source file as part of
> 'hello.mod'.
> 
> Regards,
> Colin
> 
> 
> > - Original Message 
> > From: Viswesh S <[EMAIL PROTECTED]>
> > To: The development of GRUB 2 
> > Sent: Friday, 25 July, 2008 3:44:48 PM
> > Subject: Re: Linking
> > 
> > 
> > Hi,
> >  
> > So if I add a module or add a new command,which are the files which
> > I should modify in general and which all gets generated
> > automatically. Is this somewhere documented ?
> >  
> > Why is it that hello command is put in a separate directory, while
> > lspci is put inside the commands directory. 
> > Is there a specific reason for this.
> >  
> >  
> > Viswesh
> > 
> > 
> > 
> > - Original Message 
> > From: Pavel Roskin <[EMAIL PROTECTED]>
> > To: grub-devel@gnu.org
> > Sent: Friday, 25 July, 2008 11:59:21 AM
> > Subject: Re: Linking
> > 
> > Quoting Viswesh S <[EMAIL PROTECTED]>:
> > 
> > > Hi Pavel,
> > > Could you please attach the modified file ?
> > > Viswesh
> > 
> > No, I'm just giving you an idea what you may be doing wrong.
> > 
> 
> 
> ___
> Grub-devel mailing list
> Grub-devel@gnu.org
> http://lists.gnu.org/mailman/listinfo/grub-devel
> 
> 
> 
>   Share files, take polls, and make new friends - all under one
> roof. Go to http://in.promos.yahoo.com/groups/


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

Remove conf/*.mk from svn (was: Re: Linking)

[Colin D Bennett]

On Thu, 24 Jul 2008 22:17:06 -0700 (PDT)
Viswesh S <[EMAIL PROTECTED]> wrote:

> I have modified the conf/common.mk accordingly as shown below.

GRUB developers,

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.

What is the benefit of having conf/*.mk files in svn?  They cause lots
of merge conflicts: sure, the conflicts are easy to resolve by simply
choosing to take one of the revisions, but then that merge really has
not meaning, since it won't be in sync with your .rmk file until it's
once again regenerated and checked in.

Removing the autogenerated .mk files would also eliminate problems new
developers like Viswesh and I have encountered -- we won't be tempted
to modify a non-versioned file like this.

Regards,
Colin


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

Re: atoi

[Colin D Bennett]

On Sat, 26 Jul 2008 06:52:31 -0700 (PDT)
Viswesh S <[EMAIL PROTECTED]> wrote:

> Hi,
> Just a confirmation, 
> Is there an implementation for atoi or similar conversion implemented
> in grub2 already. I suppose, I need to implement it myself.
> I am planning to implement a setpci command in grub2.Hence the
> question Viswesh

Try grub_strtoul(n, 0, 0) from 'kern/misc.c'.  It is better than atoi
since it provides error checking (if you choose to specify a char **end
as the 2nd arg instead of 0) and supports multiple bases (the 3rd arg;
0 means auto detect).

Regards,
Colin


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

Re: [PATCH] File readahead buffering

[Colin D Bennett]

On Thu, 24 Jul 2008 17:51:23 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Wed, Jul 23, 2008 at 10:56 PM, Colin D Bennett <[EMAIL PROTECTED]>
> wrote:
> > On Wed, 23 Jul 2008 07:33:32 -0700
> > Colin D Bennett <[EMAIL PROTECTED]> wrote:
> >
> >> Ok, after some more testing, I have found that if I create PNG
> >> images with ImageMagick or The GIMP, they work.  However, I have
> >> an image that I was trying to use as a GRUB background which is
> >> from a MythTV theme called Titivillus.  The PNG file from the
> >> MythTV theme doesn't load in GRUB.  I get the message "error: png:
> >> block type fixed not supported".
> >
> > Oops!  After more testing, I have found PNG images that I saved in
> > The GIMP which also refuse to load due to the "block type fixed not
> > supported" error.  One ZIP file [1] contains images that all fail to
> > load in GRUB with that error, and the other [2] contains similar
> > images that load just fine.
> >
> > [1] gibibit.com/upload/grub-png-bad.zip
> > [2] gibibit.com/upload/grub-png-good.zip
> 
> Hi,
> 
> This patch should fix the problem.

Hi Bean,

Thanks for the PNG fix!  I tested it and it fixes the problem.  8-)
I am now using PNG files in my testing and they are working great.

I'm also using my file I/O buffering patch and it helps performance
massively when loading images.  I hope that in using the file
buffering patch myself I can find any potential problems with it.

Regards,
Colin


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

Re: [PATCH] buffered file read

[Colin D Bennett]

On Mon, 28 Jul 2008 01:52:40 +0800
Bean <[EMAIL PROTECTED]> wrote:

> Hi,
> 
> This patch add a new module bufio, which reads block of data at a
> time, and return the required range to the upper level. This is
> extremely useful in modules like png, where data are normally read one
> byte at a time.
> 
> To use buffered io service, just include header file ,
> and replace grub_file_open with grub_buffile_open.
> 
> Changelog:
> 
> 2008-07-27  Bean  <[EMAIL PROTECTED]>
>   
>   * conf/common.rmk (pkglib_MODULES): Add bufio.mod.
>   (bufio_mod_SOURCES): New macro.
>   (bufio_mod_CFLAGS): Likewise.
>   (bufio_mod_LDFLAGS): Likewise.
> 
>   * include/grub/bufio.h: New file.
> 
>   * io/bufio.c: Likewise.
> 
>   * video/png.c (grub_video_reader_png): Use grub_buffile_open
> to open file.
> 
>   * video/jpeg.c (grub_video_reader_jpeg): Likewise.
> 
>   * video/tga.c (grub_video_reader_tga): Likewise.

Hi Bean,

Beautiful code!  Thanks so much for doing this work -- using buffered
file reads is absolutely necessary from a performance perspective, and
I am glad to see it implemented so cleanly.

My poor hack of grub_file_read() bows to your far superior patch!  I
really appreciate the simplicity and clarity of both its design and its
use.  I like how you implemented it using the Decorator pattern (from
the GoF design patterns) to wrap the basic file with the buffered file
layer in a way transparent to users. This is very intuitive to me,
especially since Java does exactly the same thing with InputStream and
BufferedInputStream.

The best part is that users of file I/O only have to change one
function call to get all the benefits of the buffering.

I will begin testing with this patch instead of mine shortly.

Thanks again!
Regards,
Colin


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

[PATCH] High resolution time/TSC patch v3

[Colin D Bennett]

Another updated TSC patch.  Now it detects TSC support for x86 CPUs at
runtime and selects either the TSC or RTC time source.  This way 386
and 486 CPUs without RDTSC instruction support are supported.

Robert Millan was interested in getting this patch merged for the
Coreboot port, so I decided to take another crack at it.

Comments are welcome!

Regards,
Colin
=== modified file 'ChangeLog'
--- ChangeLog	2008-07-27 19:57:43 +
+++ ChangeLog	2008-07-28 16:51:30 +
@@ -1,3 +1,68 @@
+2008-07-28  Colin D Bennett <[EMAIL PROTECTED]>
+
+	High resolution timer support.  Implemented for x86 CPUs using TSC.
+	Extracted generic grub_millisleep() so it's linked in only as needed.
+	This requires a Pentium compatible CPU; if the RDTSC instruction is
+	not supported, then it falls back on the generic grub_get_time_ms()
+	implementation that uses the machine's RTC.
+
+	* conf/i386-efi.rmk: Added new source files to kernel_elf_SOURCES.
+
+	* conf/i386-pc.rmk: Likewise.
+
+	* conf/sparc64-ieee1275.rmk: Likewise.
+
+	* conf/powerpc-ieee1275.rmk: Likewise.
+
+	* conf/i386-coreboot.rmk: Likewise.
+
+	* kern/generic/rtc_get_time_ms.c: New file.
+
+	* kern/generic/millisleep.c: New file.
+
+	* kern/misc.c (grub_millisleep_generic): Removed.
+
+	* commands/sleep.c (grub_interruptible_millisleep): Uses
+	grub_get_time_ms() instead of grub_get_rtc() to stay in sync with
+	grub_millisleep() from kern/generic/millisleep.c.
+
+	* include/grub/i386/tsc.h (grub_get_tsc): New file.  New inline
+	function.
+	(grub_cpu_is_cpuid_supported): New inline function.
+	(grub_cpu_is_tsc_supported): New inline function.
+	(grub_tsc_init): New function prototype.
+	(grub_tsc_get_time_ms): New function prototype.
+
+	* kern/i386/tsc.c (grub_get_time_ms): New file.  New function.
+	(calibrate_tsc): New static function.
+	(grub_tsc_init): New function.
+
+	* include/grub/time.h (grub_millisleep_generic): Removed.
+	(grub_get_time_ms): New function.
+	(grub_install_get_time_ms): New function.
+
+	* kern/time.c (grub_get_time_ms): New function.
+	(grub_install_get_time_ms): New function.
+
+	* kern/i386/efi/init.c (grub_millisleep): Removed.
+	(grub_machine_init): Call grub_tsc_init.
+
+	* kern/i386/linuxbios/init.c (grub_machine_init): Install the RTC
+	get_time_ms() implementation.
+
+	* kern/sparc64/ieee1275/init.c (grub_millisleep): Removed.
+	(ieee1275_get_time_ms): New function.
+	(grub_machine_init): Install get_time_ms() implementation.
+
+	* kern/i386/pc/init.c (grub_machine_init): Call grub_tsc_init().
+	(grub_millisleep): Removed.
+
+	* kern/ieee1275/init.c (grub_millisleep): Removed.
+	(grub_machine_init): Install ieee1275_get_time_ms() implementation.
+	(ieee1275_get_time_ms): New static function.
+	(grub_get_rtc): Now calls ieee1275_get_time_ms(), which does the real
+	work.
+
 2008-07-27  Robert Millan  <[EMAIL PROTECTED]>
 
 	* disk/ata.c (grub_ata_dumpinfo): Use grub_dprintf() for debugging

=== modified file 'commands/sleep.c'
--- commands/sleep.c	2008-05-16 20:55:29 +
+++ commands/sleep.c	2008-07-04 16:55:48 +
@@ -43,15 +43,15 @@
   grub_printf ("%d", n);
 }
 
-/* Based on grub_millisleep() from kern/misc.c.  */
+/* Based on grub_millisleep() from kern/generic/millisleep.c.  */
 static int
 grub_interruptible_millisleep (grub_uint32_t ms)
 {
-  grub_uint32_t end_at;
-  
-  end_at = grub_get_rtc () + grub_div_roundup (ms * GRUB_TICKS_PER_SECOND, 1000);
-  
-  while (grub_get_rtc () < end_at)
+  grub_uint64_t start;
+
+  start = grub_get_time_ms ();
+
+  while (grub_get_time_ms () - start < ms)
 if (grub_checkkey () >= 0 &&
 	GRUB_TERM_ASCII_CHAR (grub_getkey ()) == GRUB_TERM_ESC)
   return 1;

=== modified file 'conf/i386-coreboot.rmk'
--- conf/i386-coreboot.rmk	2008-07-27 12:51:30 +
+++ conf/i386-coreboot.rmk	2008-07-28 16:23:46 +
@@ -16,6 +16,7 @@
 	kern/main.c kern/device.c \
 	kern/disk.c kern/dl.c kern/file.c kern/fs.c kern/err.c \
 	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
+	kern/time.c \
 	kern/i386/dl.c kern/parser.c kern/partition.c \
 	kern/env.c \
 	term/i386/pc/console.c \

=== modified file 'conf/i386-efi.rmk'
--- conf/i386-efi.rmk	2008-07-27 12:51:30 +
+++ conf/i386-efi.rmk	2008-07-28 16:23:46 +
@@ -84,7 +84,10 @@
 	kern/misc.c kern/mm.c kern/loader.c kern/rescue.c kern/term.c \
 	kern/i386/dl.c kern/i386/efi/init.c kern/parser.c kern/partition.c \
 	kern/env.c symlist.c kern/efi/efi.c kern/efi/init.c kern/efi/mm.c \
-	term/efi/console.c disk/efi/efidisk.c
+	term/efi/console.c disk/efi/efidisk.c \
+	kern/i386/tsc.c \
+	kern/generic/rtc_get_time_ms.c \
+	kern/generic/millisleep.c
 kernel_mod_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
 	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \

=== modified file 'conf/i386-ieee1275.rmk'
--- co

Re: [PATCH] buffered file read

[Colin D Bennett]

On Wed, 30 Jul 2008 11:48:19 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Mon, Jul 28, 2008 at 1:08 PM, Bean <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > I have added a buffer length parameter to grub_buffile_open:
> >
> 
...
> 
> If no one objects, I'd commit this patch soon.
> 

Sounds good to me!

Regards,
Colin


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

Re: Set the menu choice for next reboot without access to conf file?

[Colin D Bennett]

On Wed, 30 Jul 2008 15:25:11 -0700
"David Mathog" <[EMAIL PROTECTED]> wrote:

> We have some systems which are remotely administered and dual boot
> linux and Windows.  Under linux there is access to the grub config
> file, but under Windows there is not.  The default grub menu option
> boots Windows. 
> 
> On linux
> 
>   grub-set-default 1
> 
> will tell grub to use the 2nd menu option (and so boot linux instead
> of Windows)) only for the next boot, so long as the grub config file
> is appropriately configured with "default saved".
> 
> If this command were applied from Windows would "grub-set-default"
> require access to the grub config file, or does grub-set-default just
> store an integer somewhere in the MBR, or elsewhere in the first disk
> track?

Hi David,

Are you using GRUB 2 or GRUB legacy (0.9x)?

Regards,
Colin


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

Re: TSC on coreboot (Re: [PATCH] High resolution time/TSC patch v3)

[Colin D Bennett]

On Mon, 4 Aug 2008 01:53:48 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> On Sun, Aug 03, 2008 at 09:48:16PM +0200, Robert Millan wrote:
> > On Mon, Jul 28, 2008 at 10:05:33AM -0700, Colin D Bennett wrote:
> > > +/* Calibrate the TSC based on the RTC.  */
> > > +static void
> > > +calibrate_tsc (void)
> > > +{
> > > +  /* First calbrate the TSC rate (relative, not absolute time).
> > > */
> > > +  grub_uint64_t start_tsc;
> > > +  grub_uint64_t end_tsc;
> > > +  grub_uint32_t initial_tick;
> > > +  grub_uint32_t start_tick;
> > > +  grub_uint32_t end_tick;
> > > +
> > > +  /* Wait for the start of the next tick;
> > > + we'll base out timing off this edge. */
> > > +  initial_tick = grub_get_rtc ();
> > 
> > Ah, I see the problem.  It calls grub_get_rtc() which in
> > grub-coreboot is just a stub.
> > 
> > How about using the interval timer for calibration instead?
> 
> Here.  With this patch your code works on coreboot too.
> 
> Note:  AFAICT we can't calculate the epoch without RTC.  But then
> again, this epoch is just as defined by the time BIOS enables RTC
> interrupts, so why not define it ourselves?  I propose that we define
> epoch as the time in which our TSC code is initialized.
> 
> If knowing the time in which BIOS was started is really useful, maybe
> we could #ifdef it instead.  Though if it's not I'd prefer the
> simplicity.

So far we never use the value returned by ``grub_get_time_ms()`` as an
absolute time.  It is currently only used in relative terms, so only
the rate at which it changes is important, not the definition of the
zero point.  Perhaps that simplifies things?  I think it's fine to go
with that definition for now -- if we need to use absolute time at some
point, we can deal with it then.

Regards,
Colin


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

[PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

I following the GRUB 2 gdb debugging instructions this week and I was
successful in using QEMU and gdb to debug GRUB.  I wondered if we could
make this a bit easier -- primarily by making GRUB's build system
generate the '.elf' files with full symbol info without requiring a
patch.  Attached is a patch against GRUB 2 trunk (this is the one
from the GRUB 2 debugging guide, just fixed to apply on the latest
GRUB sources).

We could make it a 'configure' option like '--enable-debug' or
something, but it seems like minimal overhead to generate the .elf
files for gdb in addition to "real" GRUB files.

Let me know what you all think.  Thanks!

Regards,
Colin
=== modified file 'genmk.rb'
--- genmk.rb	2008-07-28 21:35:40 +
+++ genmk.rb	2008-08-05 14:36:06 +
@@ -101,10 +101,11 @@
 mod_obj = mod_src.suffix('o')
 defsym = 'def-' + @name.suffix('lst')
 undsym = 'und-' + @name.suffix('lst')
+exec = @name.suffix('elf')
 mod_name = File.basename(@name, '.mod')
 symbolic_name = mod_name.sub(/\.[^\.]*$/, '')
 
-"CLEANFILES += [EMAIL PROTECTED] #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{undsym}
+"CLEANFILES += [EMAIL PROTECTED] #{mod_obj} #{mod_src} #{pre_obj} #{objs_str} #{undsym} #{exec}
 ifneq ($(#{prefix}_EXPORTS),no)
 CLEANFILES += #{defsym}
 DEFSYMFILES += #{defsym}
@@ -112,11 +113,14 @@
 MOSTLYCLEANFILES += #{deps_str}
 UNDSYMFILES += #{undsym}
 
[EMAIL PROTECTED]: #{pre_obj} #{mod_obj} $(TARGET_OBJ2ELF)
[EMAIL PROTECTED]: #{exec}
+	-rm -f $@
+	$(OBJCOPY) --strip-unneeded -K grub_mod_init -K grub_mod_fini -R .note -R .comment $^ $@
+
+#{exec}: #{pre_obj} #{mod_obj} $(TARGET_OBJ2ELF)
 	-rm -f $@
 	$(TARGET_CC) $(#{prefix}_LDFLAGS) $(TARGET_LDFLAGS) $(MODULE_LDFLAGS) -Wl,-r,-d -o $@ #{pre_obj} #{mod_obj}
 	if test ! -z $(TARGET_OBJ2ELF); then ./$(TARGET_OBJ2ELF) $@ || (rm -f $@; exit 1); fi
-	$(STRIP) --strip-unneeded -K grub_mod_init -K grub_mod_fini -K _grub_mod_init -K _grub_mod_fini -R .note -R .comment $@
 
 #{pre_obj}: $(#{prefix}_DEPENDENCIES) #{objs_str}
 	-rm -f $@

=== modified file 'kern/main.c'
--- kern/main.c	2008-08-04 21:54:06 +
+++ kern/main.c	2008-08-05 14:36:06 +
@@ -122,6 +122,9 @@
   grub_print_error ();
 }
 
+void
+main (void) __attribute__ ((alias("grub_main")));
+
 /* The main routine.  */
 void
 grub_main (void)



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

Re: [PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

On Tue, 05 Aug 2008 17:05:57 +0200
Felix Zielcke <[EMAIL PROTECTED]> wrote:

> Am Dienstag, den 05.08.2008, 07:42 -0700 schrieb Colin D Bennett:
> > I following the GRUB 2 gdb debugging instructions this week and I
> > was successful in using QEMU and gdb to debug GRUB.  I wondered if
> > we could make this a bit easier -- primarily by making GRUB's build
> > system generate the '.elf' files with full symbol info without
> > requiring a patch.  Attached is a patch against GRUB 2 trunk (this
> > is the one from the GRUB 2 debugging guide, just fixed to apply on
> > the latest GRUB sources).
> > 
> Well I already brought this wiki page up on here, but nobody really
> cared about updating this.
> I even did the same as you updateing that patch to current SVN ;)
> 
> See here:
> http://lists.gnu.org/archive/html/grub-devel/2008-07/msg00585.html
> http://lists.gnu.org/archive/html/grub-devel/2008-08/msg00037.html
> 
> > Let me know what you all think. Thanks!
> 
> Thanks that at least you try to that GDB + Qemu stuff easier.
> 
> Even with the patches from Lubomir from May I wasn't able to get the
> symbols of the loaded modules.
> I have used that load_modules function which is in the new gdb-macro
> patch but it didn't work.
> And the gdb-stub didn't apply and I was unsure if it was safe to just
> adjust the values in the header file which was changed.
> 
> I think I'm not that good in this stuff to get it working, so it's
> very nice that you want to do it now :)

Interesting.  Well, I think I used the 'load_all_modules' function
instead of just 'load_modules'.  And IIRC, I first used the GRUB legacy
floppy image that was provided to chainload GRUB 2 over QEMU's built-in
tftp, but I think that directly booting GRUB 2 in QEMU for debugging
did not work... I'll have to try it again to verify that.

Cheers,
Colin


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

Re: [PATCH] Drivemap module

[Colin D Bennett]

On Tue, 05 Aug 2008 01:10:25 +0200
Javier Martín <[EMAIL PROTECTED]> wrote:

> Besides, sometimes checks like "if (a=b)", or more
> frequently "if (a=f())" are intentionally used in C, so the error
> might become even more difficult to spot and correct.

Respect and heed the gcc warnings!!

Note that gcc will warn you (you *are* using -Wall, at least, right?)
if you try to do

  if (a = b)

but if you really mean it, you should surround the expression with an
extra pair of parentheses:

  if ((a = b))

Use of the value of an assignment in an expression is common in code
like

  if ((n = read (fd, buf, sz)) > 0)
/* ... use the data in buf ... */

Here's an example showing gcc being helpful:

  $ cat a.c
  int f()
  {
  int a = 1;
  int b;
  if ((b = a))
return 1;
  return 0;
  }
  $ cat b.c
  int f()
  {
  int a = 1;
  int b;
  if (b = a)
return 1;
  return 0;
  }
  $ gcc -Wall -c a.c
  $ gcc -Wall -c b.c
  b.c: In function ‘f’:
  b.c:5: warning: suggest parentheses around assignment used as truth
  $ 

> New version of the patch follows. I must say that the discussion here
> has become pretty interesting without degrading into flamewarring -
> who whats to call Mr. Torvalds and Prof. Tanembaum? ^^

Well, it sounds to me like you are trying your best to set Marco ablaze,
but he is delightfully fire-retardant.

Regards,
Colin


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

Re: [PATCH] Drivemap module

[Colin D Bennett]

On Tue, 05 Aug 2008 13:23:11 +0200
Marco Gerards <[EMAIL PROTECTED]> wrote:

> Hi Javier,
> 
> Javier Martín <[EMAIL PROTECTED]> writes:
> > While tools like `svn diff' can help in these kind of problems,
> > their utility is greatly diminished if the offending change is part
> > of a multi-line change. Besides, sometimes checks like "if (a=b)",
> > or more frequently "if (a=f())" are intentionally used in C, so the
> > error might become even more difficult to spot and correct. I ask
> > for a deep reflection on this issue: maybe I'm dead wrong and an
> > arrogant brat in my attempt to tackle the Holy GNU Coding
> > Standards, but I'd like to ask input from more people on this.
> 
> I will refuse patches with "if (a = f())", if that makes you sleep
> better ;-)

Since that will generate a gcc warning, I hope that the author would at
least do

  if ((a = f ()))

to indicate to gcc that you mean what you said.

Regards,
Colin


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

Re: [PATCH] Misc patches for grub2

[Colin D Bennett]

On Tue, 5 Aug 2008 22:14:37 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Tue, Aug 5, 2008 at 6:36 PM, Marco Gerards <[EMAIL PROTECTED]>
> wrote:
> > For some reason I have some doubts about lib/.  But I do not have a
> > better name in mind either.  What does belong in there?  Do you
> > happen to have other names in mind as a suggestion? :-)
> 
> The files in this directory are used by both modules and utilities,
> while files in util/ are only used by utilities.
> 
> I also sense that lib is a little strange, but I can't think of a
> better name. Some alternative name could be: shared, common, helper.

Until recently, it looked like only the LZMA code was in 'lib/'.  This
led me to think along the following lines:  Since the lzma code is in
lib, which is essentially just a modified import of the LZMA SDK 4.58
beta, I thought that lib's intent was something like:

  Code imported from other projects.
  Although this code may have been modified from its upstream form to
  work for GRUB, it is still, in essence, a library that is maintained
  separately from GRUB, and we should be able to stay in sync with the
  upstream project by integrating new versions when we want to.

By keeping imported code as close to its original form as possible
(and in a logical, self contained source tree location such as
'lib/lzma/'), it will make it much easier to upgrade that imported
code to a new version of the upstream.  Perhaps when LZMA SDK 4.58
final is released they will have fixed a bug or two... then it's easy
to bring that forward if we haven't changed the code too much in GRUB.

Just a thought on what 'lib/' *could* be...

Regards,
Colin


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

Re: Remove conf/*.mk from svn

[Colin D Bennett]

On Tue, 05 Aug 2008 20:41:53 +0200
Javier Martín <[EMAIL PROTECTED]> wrote:

> 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?

Fine with me.  It shouldn't be too hard for someone who understands it.

Based on the discussions following my initial suggestion, it sounds
like it is considered too much work for people compiling from a svn
checkout to install Ruby?

Did I emphasize enough that released tarballs or any sort of archived
snapshot should be generated *with* the .mk files?  I am concerned only
with the files under version control -- the point being that files
generated from often-modified files (such as the ``conf/*.rmk`` which
are often modified) have no business being under version control.  I
think that this is even more important than lesser modified files such
as the configure script, etc., since fewer developers have to touch
that.

Regards,
Colin


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

On Tue, 5 Aug 2008 22:14:03 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> On Tue, Aug 05, 2008 at 07:42:29AM -0700, Colin D Bennett wrote:
> > 
> > We could make it a 'configure' option like '--enable-debug' or
> > something, but it seems like minimal overhead to generate the .elf
> > files for gdb in addition to "real" GRUB files.
> 
> I'd favour the --enable-debug.  Even if it's a small overhead, it
> becomes a big problem when you are out of space and need to start
> looking around to see what can be shaved off.

In that case, should --enable-debug be the default, or --disable-debug?

Colin


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

Re: [PATCH] Warning fixes

[Colin D Bennett]

On Wed, 06 Aug 2008 09:39:02 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> Hello!
> 
> I've fixed the trivial stuff, but this may require comments.  I'll be
> mostly offline for the next 3 weeks, so if it's OK, please apply it
> for me.

Along these lines, can we use a gcc option to turn off the "trampoline
requires executable stack" warnings that are generated by the dozens?
I looked for the option in the gcc manual and could not find it.

I think that the trampoline warning is the main thing left holding us
back from a warning-free build.  (IIRC there may be some
strict-aliasing warnings too... but those can be fixed by fixing the
code.)

Regards,
Colin


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

Re: [PATCH] Warning fixes

[Colin D Bennett]

On Wed, 06 Aug 2008 10:32:26 -0400
Pavel Roskin <[EMAIL PROTECTED]> wrote:

> Quoting Colin D Bennett <[EMAIL PROTECTED]>:
> 
> > Along these lines, can we use a gcc option to turn off the
> > "trampoline requires executable stack" warnings that are generated
> > by the dozens? I looked for the option in the gcc manual and could
> > not find it.
> 
> Please quote the exact message.  What platform is it?  I haven't seen
> it.

I have been using gcc 4.2.4 on x86 Linux.  However, I just upgraded a
machine to gcc 4.3.1, and it looks like the trampoline warnings are not
produced by gcc 4.3.1... interesting.  But they certainly are on 4.2.4:
there are 103 trampoline warnings in all (see the end of this message).

> > I think that the trampoline warning is the main thing left holding
> > us back from a warning-free build.  (IIRC there may be some
> > strict-aliasing warnings too... but those can be fixed by fixing the
> > code.)

Ok, I checked and there are no strict aliasing warnings.  Good.

> Yes, either we should make the build less noisy by default, or we  
> should treat warnings as errors.  Or maybe both.

I agree.

Regards,
Colin


system info
---

x86 Linux; glibc 2.8; gcc 4.2.4; Gentoo.

$ gcc -v
Using built-in specs.
Target: i686-pc-linux-gnu
Configured
with: /var/tmp/portage/sys-devel/gcc-4.2.4/work/gcc-4.2.4/configure
--prefix=/usr --bindir=/usr/i686-pc-linux-gnu/gcc-bin/4.2.4
--includedir=/usr/lib/gcc/i686-pc-linux-gnu/4.2.4/include
--datadir=/usr/share/gcc-data/i686-pc-linux-gnu/4.2.4
--mandir=/usr/share/gcc-data/i686-pc-linux-gnu/4.2.4/man
--infodir=/usr/share/gcc-data/i686-pc-linux-gnu/4.2.4/info
--with-gxx-include-dir=/usr/lib/gcc/i686-pc-linux-gnu/4.2.4/include/g++-v4
--host=i686-pc-linux-gnu --build=i686-pc-linux-gnu --disable-altivec
--enable-nls --without-included-gettext --with-system-zlib
--disable-checking --disable-werror --enable-secureplt
--disable-multilib --enable-libmudflap --disable-libssp
--disable-libgcj --with-arch=i686
--enable-languages=c,c++,treelang,fortran --enable-shared
--enable-threads=posix --enable-__cxa_atexit --enable-clocale=gnu
Thread model: posix gcc version 4.2.4 (Gentoo 4.2.4 p1.0)


trampoline warnings
---

../util/i386/pc/grub-setup.c:93: warning: generating trampoline in
object (requires executable stack) ../util/i386/pc/grub-setup.c:93:
warning: generating trampoline in object (requires executable
stack) ../util/i386/pc/grub-setup.c:93: warning: generating trampoline
in object (requires executable stack) ../util/biosdisk.c:748: warning:
generating trampoline in object (requires executable
stack) ../kern/device.c:85: warning: generating trampoline in object
(requires executable stack) ../kern/device.c:85: warning: generating
trampoline in object (requires executable
stack) ../kern/partition.c:90: warning: generating trampoline in object
(requires executable stack) ../kern/partition.c:59: warning: generating
trampoline in object (requires executable stack) ../fs/fshelp.c:58:
warning: generating trampoline in object (requires executable
stack) ../fs/affs.c:460: warning: generating trampoline in object
(requires executable stack) ../fs/ext2.c:736: warning: generating
trampoline in object (requires executable stack) ../fs/hfs.c:578:
warning: generating trampoline in object (requires executable
stack) ../fs/hfs.c:578: warning: generating trampoline in object
(requires executable stack) ../fs/hfs.c:509: warning: generating
trampoline in object (requires executable stack) ../fs/hfs.c:725:
warning: generating trampoline in object (requires executable
stack) ../fs/hfsplus.c:699: warning: generating trampoline in object
(requires executable stack) ../fs/hfsplus.c:886: warning: generating
trampoline in object (requires executable stack) ../fs/iso9660.c:355:
warning: generating trampoline in object (requires executable
stack) ../fs/iso9660.c:229: warning: generating trampoline in object
(requires executable stack) ../fs/iso9660.c:466: warning: generating
trampoline in object (requires executable stack) ../fs/iso9660.c:655:
warning: generating trampoline in object (requires executable
stack) ../fs/udf.c:772: warning: generating trampoline in object
(requires executable stack) ../fs/ntfs.c:860: warning: generating
trampoline in object (requires executable stack) ../fs/reiserfs.c:1270:
warning: generating trampoline in object (requires executable
stack) ../fs/sfs.c:531: warning: generating trampoline in object
(requires executable stack) ../fs/xfs.c:624: warning: generating
trampoline in object (requires executable stack) ../fs/afs.c:566:
warning: generating trampoline in object (requires executable
stack) ../partmap/pc.c:232: warning: generating trampoline in object
(requires executable stack) ../partmap/gpt.c:123: warning: generating
trampoline in object (requires executable
stack) ../util/biosdisk.c:748: warning: generating trampoline in object
(re

Re: Encryption Support for GRUB

[Colin D Bennett]

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.

link to online svn for misc.c:
http://svn.savannah.gnu.org/viewvc/trunk/grub2/kern/misc.c?revision=1774&root=grub&view=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

Re: [PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

On Thu, 07 Aug 2008 09:37:22 +0200
Felix Zielcke <[EMAIL PROTECTED]> wrote:

> Am Dienstag, den 05.08.2008, 23:54 +0200 schrieb Robert Millan:
> > On Tue, Aug 05, 2008 at 01:50:21PM -0700, Colin D Bennett wrote:
> > > In that case, should --enable-debug be the default, or
> > > --disable-debug?
> > 
> > I'd prefer to disable debug as default.  What do others think?
> > 
> Have it disabled by default.
> I don't think it's useful to have them for everyone i.e. the average
> user.

I agree that having debug info for the installed GRUB is not helpful,
especially for the general end-user.  So files installed by the GRUB
install process certainly don't need debugging symbols.

My concern was only to generate the required symbol information at
build time, not to install it for a real GRUB install, or to package
these files for distribution in .rpm, .deb, etc.

I don't really care if it's the default or not since I can simply
choose my configure options as necessary for the task at hand.  If I
have a debugging situation, then I can rebuild with debug info, since
I'd have to re-start the GRUB/QEMU session anyway to begin debugging.
(And I don't use the ridiculously slow QEMU for most stuff anyway.)

> Having debug symbols in the userland tools could be maybe a bit useful
> for example in the case of the grub-probe segfault I experienced on my
> mdraid experiments.

True, in that case it makes sense to include debug info (if a
distro's other packages include it), since a core dump could be used to
debug, or if core dumps aren't enabled, then you could simply re-run it
with 'gdb grub-probe' to try to recreate the problem.  This doesn't
require the whole QEMU virtual machine situation that debugging the
GRUB boot loader itself requires.

Regards,
Colin


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

Re: [PATCH][RFC] gdb support in GRUB build system

[Colin D Bennett]

On Thu, 7 Aug 2008 18:07:43 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> On Thu, Aug 07, 2008 at 08:50:45AM -0700, Colin D Bennett wrote:
> > > Having debug symbols in the userland tools could be maybe a bit
> > > useful for example in the case of the grub-probe segfault I
> > > experienced on my mdraid experiments.
> > 
> > True, in that case it makes sense to include debug info
> 
> Agreed.  Furthermore, Debian packaging environment _expects_ packages
> to include debug info by default;  this allows us (Debian) to define
> a common variable interface to enable/disable debug symbols by
> toggling the scripts that run strip after the build.

Also, also Gentoo does the same thing -- the Gentoo ebuild system does
the stripping based on certain flags are set in the FEATURES variable
('nostrip' or 'splitdebug').

Regards,
Colin


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

Re: [PATCH] New command checktime

[Colin D Bennett]

I just have a couple of suggestions:

On Fri, 8 Aug 2008 01:52:07 +0800
Bean <[EMAIL PROTECTED]> wrote:

> On Thu, Aug 7, 2008 at 7:19 PM, Bean <[EMAIL PROTECTED]> wrote:
> 1.  date [MMDDhhmm[[CC]YY][.ss]]
> 
> Set/Display current datetime, to set date, The format is the same as
> the date command, you can also use - or : to seperate the numbers. For
> example:
> 
> date 0902
> Sep, 2.
> 
> date 10-05-13-15
> Oct 5, 13:15

I know the reason for using that format is to be compatible with the
Unix 'date' command, but I find it impossible to remember the arcane
format it accepts.  Do you think it would be more straightforward to
accept the ISO standard date format (ISO 8601).  For instance:

   date 2008-10-05T13:15
   (5 October 2008)

or, as syntactic sugar, let each argument be either a time or date:

   date 2008-10-05 13:15


> 2. crontab min hour day_of_month month day_of_week

This really doesn't allow you to edit a crontab, does it?  If not, is
there a more appropriate name we could use instead of 'crontab'?
Perhaps 'testdate' or something?

Regards,
Colin


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

Re: How to synchronize local svn with official svn?

[Colin D Bennett]

On Sun, 10 Aug 2008 09:42:39 +0800
"y.volta" <[EMAIL PROTECTED]> wrote:

> hi all,
> 
> I have a question about using svn: how can i synchronize the
> local copy with the local svn and official svn? for examle:
> 
> 1. i checkout the official grub2 from SA
> (svn://svn.sv.gnu.org/grub/trunk/grub2) to LD (/home/grub2);
> 
> 2. and then, i create a local site to host the grub2 - SB ( i.e.
> svn://local/grub2 ), 
> now, i can 'Relocate' LD's url to SB, and 'Update' LD to SB.
> here, i get a local svn copy, and can do anythings but the
> synchronization from SA.
> 
> so, how to synchronize local svn with official svn? any
> suggestion?
> 
> Y.Volta

Well, I'm not sure since you are kind of creating a local mirror, it
sounds like... and Subversion is not a distributed version control
system.  However, if you want to have full distributed VCS features,
you can do what I do and use the Bazaar VCS (bzr) with the 'bzr-svn'
plugin, which allows seamless access to svn branches as if they
were bzr branches.  And then you can use bzr (which is a DVCS like git
or hg) to make local branches and merge changes back and forth between
your branch and the bzr-svn branch which is the upstream GRUB.
It's really easy, all I had to do was install bzr, the bzr-svn plugin,
and then do:

  bzr branch svn://svn.savannah.gnu.org/grub/trunk/grub2

and that created a local 'grub2' branch with full history from the svn
branch.  (This takes quite a while the first time.  But bzr-svn caches
all the history data in ~/.bazaar/svn-cache so it only has to do the
full download once.

Anyway, that's one solution that works great if you want distributed
version control features.  Otherwise, I'll let someone with more recent
svn experience help you (I haven't used svn myself for the past 2 years
since I discovered bzr...).

Regards,
Colin


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

Re: [PATCH] add pkzip support for gzio

[Colin D Bennett]

On Mon, 11 Aug 2008 23:41:38 +0800
"y.volta" <[EMAIL PROTECTED]> wrote:

> hi,
> 
> i've made a little patch for pkziped ( deflate mode ) file. by
> using this, we can have both gunzip and pkunzip supported: they have
> only header different with the same file. ;-)
> 
> please note, if your file is too small, pkzip will not compress
> it but store the orginal data to the output file.
> 
> y.volta

What happens if you open a ZIP file containing multiple files?

Regards,
Colin


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

Re: [PATCH] add pkzip support for gzio

[Colin D Bennett]

On Tue, 12 Aug 2008 10:45:50 +0800
Bean <[EMAIL PROTECTED]> wrote:

> Also, it's better not to call gzio. I'm thinking about seperating the
> deflate decompression routine and create a zlib like library, so that
> the decoder can be used in both gzio, png and zipfs.

Also, pure DEFLATE compression can be used to compress font data in the
new font file format.  I postponed implementing font compression to
wait for DEFLATE support.

Regards,
Colin


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

Re: [PATCH] try2, Enable grub_cpu_idle for i386/pc to halt the CPU

[Colin D Bennett]

On Thu, 14 Aug 2008 22:27:07 -0500
David Fries <[EMAIL PROTECTED]> wrote:

> On Wed, Aug 13, 2008 at 11:34:22AM +0200, Marco Gerards wrote:
> > Please do not add .mk changes in a patch, it is a generated file :-)
> 
> What's a generated file doing in the repository if people aren't
> supposed to send in diffs against it?

I've brought up this issue a couple of times.  I think this particular
generated file is the worst because it is modified very often, unlike,
for instance, the 'configure' script which is more rarely modified.
Therefore since the *.rmk files are often modified (and thus the *.mk
files also), we have to deal with this a lot.

I guess it may seem like a minor issue to many, but all those minor
issues add up to occasional headaches.  My solution is to base my
personal bzr branches (I use the Bazaar distributed VCS for my work,
so I can have my own branches locally) off of a cleaned up version of
svn trunk where the *.mk files are deleted.

Anyway, if you want to read what Okuji and others have said about
removing the conf/*.mk files in the past, check out this in the
archives:


Regards,
Colin


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

Re: Dell Media Direct button

[Colin D Bennett]

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


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

[PATCH] GSoC #01 grub-mkrescue i386-pc multiple overlays

[Colin D Bennett]

This is the first of many patches that I will be posting of my Google
Summer of Code 2008 work on GRUB.

This patch adds support to the 'grub-mkrescue' command on the i386-pc
platform for specifying multiple '--overlay' options.  Each --overlay
option will result in the later overlays potentially overriding the
earlier ones if file names are the same.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

* util/i386/pc/grub-mkrescue.in: Support multiple overlay directories.
=== modified file 'util/i386/pc/grub-mkrescue.in'
--- util/i386/pc/grub-mkrescue.in	2008-07-12 14:40:50 +
+++ util/i386/pc/grub-mkrescue.in	2008-08-30 18:33:46 +
@@ -71,7 +71,7 @@
 --modules=*)
 	modules=`echo "$option" | sed 's/--modules=//'` ;;
 --overlay=*)
-	overlay=`echo "$option" | sed 's/--overlay=//'` ;;
+	overlay=${overlay}${overlay:+ }`echo "$option" | sed 's/--overlay=//'` ;;
 --pkglibdir=*)
 	input_dir=`echo "$option" | sed 's/--pkglibdir=//'` ;;
 --grub-mkimage=*)
@@ -124,9 +124,10 @@
   echo "insmod $i"
 done > ${aux_dir}/boot/grub/grub.cfg
 
-if test "x$overlay" = x ; then : ; else
-  cp -dpR ${overlay}/* ${aux_dir}/
-fi
+for d in ${overlay}; do
+  echo "Overlaying $d"
+  cp -dpR "${d}"/* "${aux_dir}"/
+done
 
 if [ "x${image_type}" = xfloppy -o "x${emulation}" = xfloppy ] ; then
   # build memdisk



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

[PATCH] GSoC #02 Minor tweak: Comment fix.

[Colin D Bennett]

One-liner:  Fix an incorrect comment in include/grub/normal.h for
GRUB_COMMAND_FLAG_NO_ARG_PARSE.  The incorrect comment appears to have
been a copy-and-paste error.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

* include/grub/normal.h: Fixed incorrect comment for
GRUB_COMMAND_FLAG_NO_ARG_PARSE.
=== modified file 'include/grub/normal.h'
--- include/grub/normal.h	2008-03-26 12:01:02 +
+++ include/grub/normal.h	2008-08-31 05:18:26 +
@@ -39,7 +39,7 @@
 #define GRUB_COMMAND_FLAG_TITLE		0x4
 /* Don't print the command on booting.  */
 #define GRUB_COMMAND_FLAG_NO_ECHO	0x8
-/* Don't print the command on booting.  */
+/* Pass arguments to the command without parsing options.  */
 #define GRUB_COMMAND_FLAG_NO_ARG_PARSE	0x10
 /* Not loaded yet. Used for auto-loading.  */
 #define GRUB_COMMAND_FLAG_NOT_LOADED	0x20



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

[PATCH] GSoC #03 Menu viewer interface

[Colin D Bennett]

This patch introduces a "menu viewer" handler interface.  This will
allow the graphical menu to be used interchangeably with the terminal
based menu that normal.mod provides.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

Introduced menu_viewer interface.

* include/grub/menu.h: Added.
* include/grub/menu_viewer.h: Added.
* kern/menu_viewer.c: Added.

* include/grub/normal.h (grub_menu_entry): Removed (moved to
include/grub/menu.h).
(grub_menu): Likewise.
(grub_normal_terminal_menu_viewer): New variable.
(grub_menu_run): Removed.
(grub_menu_execute_entry): New prototype.

* normal/main.c (grub_normal_execute): Use the menu viewer interface
to display the menu.
(GRUB_MODE_INIT): Register the normal.mod 'terminal' menu viewer.

* normal/menu.c (run_menu_entry): Renamed to grub_menu_execute_entry.
(grub_menu_execute_entry): Renamed from run_menu_entry, and made it
global instead of static.
(grub_menu_run): Renamed to show_menu, and made it static..
(show_menu): Renamed from grub_menu_run.
(grub_normal_terminal_menu_viewer): Handler struct definition for the
menu viewer interface.
* conf/i386-pc.rmk: Add menu_viewer .c and .h files to kernel.
=== modified file 'conf/i386-pc.rmk'
--- conf/i386-pc.rmk	2008-08-23 13:51:19 +
+++ conf/i386-pc.rmk	2008-08-31 05:19:44 +
@@ -49,11 +49,13 @@
 	kern/generic/rtc_get_time_ms.c \
 	kern/generic/millisleep.c \
 	kern/env.c \
+	kern/menu_viewer.c \
 	term/i386/pc/console.c \
 	symlist.c
 kernel_img_HEADERS = arg.h boot.h cache.h device.h disk.h dl.h elf.h elfload.h \
 	env.h err.h file.h fs.h kernel.h loader.h misc.h mm.h net.h parser.h \
 	partition.h pc_partition.h rescue.h symbol.h term.h time.h types.h \
+	menu_viewer.h \
 	machine/biosdisk.h machine/boot.h machine/console.h machine/init.h \
 	machine/memory.h machine/loader.h machine/vga.h machine/vbe.h \
 	machine/kernel.h machine/pxe.h

=== added file 'include/grub/menu.h'
--- include/grub/menu.h	1970-01-01 00:00:00 +
+++ include/grub/menu.h	2008-08-31 05:19:44 +
@@ -0,0 +1,51 @@
+/* menu.h - Menu and menu entry model declarations. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2002,2003,2005,2006,2007,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/>.
+ */
+
+#ifndef GRUB_MENU_HEADER
+#define GRUB_MENU_HEADER 1
+
+/* The menu entry.  */
+struct grub_menu_entry
+{
+  /* The title name.  */
+  const char *title;
+
+  /* The commands associated with this menu entry.  */
+  struct grub_script *commands;
+
+  /* The sourcecode of the menu entry, used by the editor.  */
+  const char *sourcecode;
+
+  /* The next element.  */
+  struct grub_menu_entry *next;
+};
+typedef struct grub_menu_entry *grub_menu_entry_t;
+
+/* The menu.  */
+struct grub_menu
+{
+  /* The size of a menu.  */
+  int size;
+
+  /* The list of menu entries.  */
+  grub_menu_entry_t entry_list;
+};
+typedef struct grub_menu *grub_menu_t;
+
+#endif /* GRUB_MENU_HEADER */

=== added file 'include/grub/menu_viewer.h'
--- include/grub/menu_viewer.h	1970-01-01 00:00:00 +
+++ include/grub/menu_viewer.h	2008-08-31 05:19:44 +
@@ -0,0 +1,43 @@
+/* menu_viewer.h - Interface to menu viewer implementations. */
+/*
+ *  GRUB  --  GRand Unified Bootloader
+ *  Copyright (C) 2002,2003,2005,2007,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/>.
+ */
+
+#ifndef GRUB_MENU_VIEWER_HEADER
+#define GRUB_MENU_VIEWER_HEADER 1
+
+#include 
+#include 
+#include 
+#include 
+
+struct grub_menu_viewer
+{
+  /* The menu viewer name.  */
+  c

[PATCH] GSoC #04 Multiple fallback entries

[Colin D Bennett]

This patch adds support for multiple fallback entries.  The entry
numbers in the 'fallback' environment variable should be
space-separated.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

Allow multiple fallback entries.

* include/grub/normal.h (grub_menu_execute_callback_t): New type.
(grub_menu_execute_with_fallback): New prototype.
(grub_menu_get_timeout): New prototype.
(grub_menu_set_timeout): New prototype.

* normal/menu.c (get_timeout): Renamed to grub_menu_get_timeout.
(grub_menu_get_timeout): Renamed from get_timeout, and made global.
(set_timeout): Renamed to grub_menu_set_timeout.
(grub_menu_set_timeout): Renamed from set_timeout, and made global.
(get_and_remove_first_entry_number): New function.
(run_menu): Call grub_menu_get_timeout and grub_menu_set_timeout
instead of the old function names.
(grub_menu_execute_with_fallback): New function.
(notify_booting): New function.
(notify_fallback): New function.
(notify_execution_failure): New function.
(execution_callback): New struct definition.
(show_menu): Use grub_menu_execute_with_fallback instead of directly
handling the fallback here.
=== modified file 'include/grub/normal.h'
--- include/grub/normal.h	2008-08-31 00:41:28 +
+++ include/grub/normal.h	2008-08-31 05:37:12 +
@@ -98,10 +98,24 @@
 
 extern struct grub_menu_viewer grub_normal_terminal_menu_viewer;
 
+typedef struct grub_menu_execute_callback
+{
+  void (*notify_booting) (void *userdata, grub_menu_entry_t entry);
+  void (*notify_fallback) (void *userdata, grub_menu_entry_t entry);
+  void (*notify_failure) (void *userdata);
+} 
+*grub_menu_execute_callback_t;
+
 void grub_enter_normal_mode (const char *config);
 void grub_normal_execute (const char *config, int nested);
+void grub_menu_execute_with_fallback (grub_menu_t menu,
+  grub_menu_entry_t entry,
+  grub_menu_execute_callback_t callback,
+  void *callback_data);
 void grub_menu_entry_run (grub_menu_entry_t entry);
 void grub_menu_execute_entry(grub_menu_entry_t entry);
+int grub_menu_get_timeout (void);
+void grub_menu_set_timeout (int timeout);
 void grub_cmdline_run (int nested);
 int grub_cmdline_get (const char *prompt, char cmdline[], unsigned max_len,
 		  int echo_char, int readline);

=== modified file 'normal/menu.c'
--- normal/menu.c	2008-08-31 00:41:28 +
+++ normal/menu.c	2008-08-31 05:37:12 +
@@ -242,8 +242,8 @@
 
 /* Return the current timeout. If the variable "timeout" is not set or
invalid, return -1.  */
-static int
-get_timeout (void)
+int
+grub_menu_get_timeout (void)
 {
   char *val;
   int timeout;
@@ -270,8 +270,8 @@
 }
 
 /* Set current timeout in the variable "timeout".  */
-static void
-set_timeout (int timeout)
+void
+grub_menu_set_timeout (int timeout)
 {
   /* Ignore TIMEOUT if it is zero, because it will be unset really soon.  */
   if (timeout > 0)
@@ -309,6 +309,43 @@
   return entry;
 }
 
+/* Get the first entry number from the variable NAME, which is a 
+   space-separated list of nonnegative integers.  The entry number which
+   is returned is stripped from the value of NAME.  */
+static int
+get_and_remove_first_entry_number (const char *name)
+{
+  char *val;
+  char *tail;
+  int entry;
+  
+  val = grub_env_get (name);
+  if (! val)
+return -1;
+  
+  grub_error_push ();
+  
+  entry = (int) grub_strtoul (val, &tail, 0);
+
+  if (grub_errno == GRUB_ERR_NONE)
+{
+  /* Skip whitespace to find the next digit.  */
+  while (*tail && grub_isspace (*tail))
+	tail++;
+  grub_env_set (name, tail);
+}
+  else
+{
+  grub_env_unset (name);
+  grub_errno = GRUB_ERR_NONE;
+  entry = -1;
+}
+
+  grub_error_pop ();
+  
+  return entry;
+}
+
 static void
 print_timeout (int timeout, int offset, int second_stage)
 {
@@ -341,7 +378,7 @@
 default_entry = 0;
 
   /* If timeout is 0, drawing is pointless (and ugly).  */
-  if (get_timeout () == 0)
+  if (grub_menu_get_timeout () == 0)
 return default_entry;
 
   offset = default_entry;
@@ -360,7 +397,7 @@
   print_entries (menu, first, offset);
   grub_refresh ();
 
-  timeout = get_timeout ();
+  timeout = grub_menu_get_timeout ();
 
   if (timeout > 0)
 print_timeout (timeout, offset, 0);
@@ -368,7 +405,7 @@
   while (1)
 {
   int c;
-  timeout = get_timeout ();
+  timeout = grub_menu_get_timeout ();
   
   if (timeout > 0)
 	{
@@ -378,7 +415,7 @@
 	  if (current_time - saved_time >= 1000)
 	{
 	  timeout--;
-	  set_timeout (timeout);
+	  grub_menu_set_timeout (timeout);
 	  saved_time = current_time;
 	  print_timeout (timeout, offset, 1);
 	}
@@ -508,6 +545,72 @@
 grub

[PATCH] GSoC #05 Menu entry class attribute

[Colin D Bennett]

This patch adds support for a 'class' attribute on menu entries.  It is
somewhat of a kludge, however since currently you just append
"|class=this,that" to the menu entry title to specify the classes.

Classes are used to provide a simple and flexible way for the best
available icon to be used in a graphical menu, but there are other
possible uses for them as well.  The idea originates from the CSS
(cascading style sheets) and HTML 'class' concept.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

Support classes for menu entries by adding '|classes=x,y' to the
title.

* include/grub/menu.h (grub_menu_entry_class): New struct type.
(grub_menu_entry): Added 'classes' field.
* normal/main.c (free_menu_entry_classes): New function.
(entry_class_attr_tag): New string constant.
(ENTRY_ATTR_SEPARATOR_CHAR): New macro.
(get_classes_from_entry_title): New function.
(grub_normal_menu_addentry): Parse '|classes=x,y' from entry titles.
=== modified file 'include/grub/menu.h'
--- include/grub/menu.h	2008-08-30 19:20:13 +
+++ include/grub/menu.h	2008-08-31 05:47:00 +
@@ -20,12 +20,24 @@
 #ifndef GRUB_MENU_HEADER
 #define GRUB_MENU_HEADER 1
 
+struct grub_menu_entry_class
+{
+  char *name;
+  struct grub_menu_entry_class *next;
+};
+
 /* The menu entry.  */
 struct grub_menu_entry
 {
   /* The title name.  */
   const char *title;
 
+  /* The classes associated with the menu entry:
+ used to choose an icon or other style attributes.
+ This is a dummy head node for the linked list, so for an entry E,
+ E.classes->next is the first class if it is not NULL.  */
+  struct grub_menu_entry_class *classes;
+
   /* The commands associated with this menu entry.  */
   struct grub_script *commands;
 

=== modified file 'normal/main.c'
--- normal/main.c	2008-08-30 19:20:13 +
+++ normal/main.c	2008-08-31 05:47:00 +
@@ -148,14 +148,132 @@
   grub_env_unset_data_slot ("menu");
 }
 
+static void
+free_menu_entry_classes (struct grub_menu_entry_class *head)
+{
+  /* Free all the classes.  */
+  while (head)
+{
+  struct grub_menu_entry_class *next;
+
+  grub_free (head->name);
+  next = head->next;
+  grub_free (head);
+  head = next;
+}
+}
+
+/* The tag that can be added to a menu entry's title to specify a class
+   for the UI to use in selecting an icon or other visual attributes.  */
+static const char entry_class_attr_tag[] = "|class=";
+
+#define ENTRY_ATTR_SEPARATOR_CHAR ','
+
+/* Parse and strip a possible "class" attribute in the title.
+   This code is not designed to support other attributes than "class"
+   in the title since, we are planning to use a better method of
+   specifying this information in the future.  The parameter TITLE is
+   modified by storing a '\0' at the appropriate location to strip the
+   class information, if it exists.  The class list is stored into *HEAD.  */
+static struct grub_menu_entry_class *
+get_classes_from_entry_title (char *title)
+{
+  struct grub_menu_entry_class *head;
+  char *attr_start;
+
+  head = grub_malloc (sizeof (struct grub_menu_entry_class));
+  if (! head)
+return 0;
+  head->name = 0;
+  head->next = 0;
+
+  attr_start = grub_strstr (title, entry_class_attr_tag);
+  if (attr_start)
+{
+  struct grub_menu_entry_class *tail;
+  const char *p;
+
+  /* Trim the properties off of the title.  */
+  *attr_start = '\0';
+
+  /* Move the pointer to the beginning of the first class name.  */
+  attr_start += grub_strlen (entry_class_attr_tag);
+
+  tail = head;
+  p = attr_start;
+  while (p && *p)
+{
+  const char *q;
+  const char *end;
+  const char *next_start;
+
+  /* Skip any leading whitespace.  */
+  while (*p && grub_isspace (*p))
+p++;
+
+  /* Find the comma terminating this one ...  */
+  q = grub_strchr (p, ENTRY_ATTR_SEPARATOR_CHAR);
+  /* ... or if it's the last one, find the '\0' terminator.  */
+  if (q)
+{
+  end = q - 1;
+  next_start = q + 1;
+}
+  else
+{
+  /* For the last class, extend it to the end.  */
+  end = p + grub_strlen (p);
+  next_start = 0;
+}
+
+  /* Trim any trailing whitespace.  */
+  while (end > p && grub_isspace (*end))
+end--;
+
+  grub_size_t len = end - p + 1;
+  /* Copy the class name into a new string.  */
+  char *class_name = grub_malloc (len + 1);
+  if (! class_name)
+{
+  free_menu_entry_classes (head);
+  return 0;
+ 

[PATCH] GSoC #06 VBE optimization for BGR/BGRA color modes

[Colin D Bennett]

This patch adds optimized routines for video modes using BGR/BGRA color
format.  Most PC video cards use BGR (24-bit) or BGRA (32-bit) color
modes, so this is an extremely important change and provides a massive
performance benefit.

I also renamed the R8G8B8A8 and R8B8B8 modes to RGBA and RGB888,
etc., since it began to be highly unreadable when B8G8R8A8 was added
into the mix.  It seems that having the color components (letters)
grouped together in the name makes it much easier for me to determine
the mode name at a glance.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

VBE performance optimization.  Added optimized rendering functions for
BGR/BGRA render targets.

* include/grub/i386/pc/vbeblit.h
(grub_video_i386_vbeblit_R8G8B8A8_R8G8B8A8): Renamed from this...
(grub_video_i386_vbeblit_RGBA_RGBA): ...to this.
(grub_video_i386_vbeblit_R8G8B8_R8G8B8A8): Renamed from this...
(grub_video_i386_vbeblit_RGB888_RGBA): ...to this.
(grub_video_i386_vbeblit_R8G8B8_R8G8B8X8): Renamed from this...
(grub_video_i386_vbeblit_RGB888_RGBX): ...to this.
(grub_video_i386_vbeblit_index_R8G8B8A8): Renamed from this...
(grub_video_i386_vbeblit_index_RGBA): ...to this.
(grub_video_i386_vbeblit_index_R8G8B8X8): Renamed from this...
(grub_video_i386_vbeblit_index_RGBX): ...to this.
(grub_video_i386_vbeblit_R8G8B8A8_R8G8B8): Renamed from this...
(grub_video_i386_vbeblit_RGBA_RGB888): ...to this.
(grub_video_i386_vbeblit_R8G8B8_R8G8B8): Renamed from this...
(grub_video_i386_vbeblit_RGB888_RGB888): ...to this.
(grub_video_i386_vbeblit_index_R8G8B8): Renamed from this...
(grub_video_i386_vbeblit_index_RGB888): ...to this.
(grub_video_i386_vbeblit_BGRX_RGBX): New prototype.
(grub_video_i386_vbeblit_BGRA_RGB888): New prototype.
(grub_video_i386_vbeblit_BGR888_RGB888): New prototype.
(grub_video_i386_vbeblit_BGRA_RGBA): New prototype.
(grub_video_i386_vbeblit_BGR888_RGBA): New prototype.
(grub_video_i386_vbeblit_BGR888_RGBX): New prototype.
(grub_video_i386_vbeblit_R8G8B8X8_R8G8B8X8): Renamed from this...
(grub_video_i386_vbeblit_direct32_copy): ...to this.

* video/i386/pc/vbeblit.c
(grub_video_i386_vbeblit_R8G8B8A8_R8G8B8A8): Renamed from this...
(grub_video_i386_vbeblit_RGBA_RGBA): ...to this.
(grub_video_i386_vbeblit_R8G8B8_R8G8B8A8): Renamed from this...
(grub_video_i386_vbeblit_RGB888_RGBA): ...to this.
(grub_video_i386_vbeblit_R8G8B8_R8G8B8X8): Renamed from this...
(grub_video_i386_vbeblit_RGB888_RGBX): ...to this.
(grub_video_i386_vbeblit_index_R8G8B8A8): Renamed from this...
(grub_video_i386_vbeblit_index_RGBA): ...to this.
(grub_video_i386_vbeblit_index_R8G8B8X8): Renamed from this...
(grub_video_i386_vbeblit_index_RGBX): ...to this.
(grub_video_i386_vbeblit_R8G8B8A8_R8G8B8): Renamed from this...
(grub_video_i386_vbeblit_RGBA_RGB888): ...to this.
(grub_video_i386_vbeblit_R8G8B8_R8G8B8): Renamed from this...
(grub_video_i386_vbeblit_RGB888_RGB888): ...to this.
(grub_video_i386_vbeblit_index_R8G8B8): Renamed from this...
(grub_video_i386_vbeblit_index_RGB888): ...to this.
(grub_video_i386_vbeblit_BGRX_RGBX): New function.
(grub_video_i386_vbeblit_BGRA_RGB888): New function.
(grub_video_i386_vbeblit_BGR888_RGB888): New function.
(grub_video_i386_vbeblit_BGRA_RGBA): New function.
(grub_video_i386_vbeblit_BGR888_RGBA): New function.
(grub_video_i386_vbeblit_BGR888_RGBX): New function.
(grub_video_i386_vbeblit_R8G8B8X8_R8G8B8X8): Renamed from this...
(grub_video_i386_vbeblit_direct32_copy): ...to this.

* include/grub/i386/pc/vbefill.h
(grub_video_i386_vbefill_R8G8B8A8): Renamed from this..
(grub_video_i386_vbefill_direct32): ...to this.
(grub_video_i386_vbefill_R8G8B8): Renamed from this...
(grub_video_i386_vbefill_direct24): ...to this.

* include/grub/video.h (grub_video_blit_format): Added BGR/BGRA modes
and renamed R8G8B8A->RGBA_.
* video/bitmap.c (grub_video_bitmap_create): Use new blit format
names.
* video/i386/pc/vbe.c (grub_video_vbe_fill_rect): Use optimized
BGR/BGRA fill routines; use new RGB/RGBA blit format names.
(common_blitter): Use optimized BGR/BGRA blit routines; use new
RGB/RGBA blit format names.

* video/i386/pc/vbefill.c 
(grub_video_i386_vbefill_R8G8B8A8): Renamed from this...
(grub_video_i386_vbefill_direct32): ...to this; don't re-calculate the
video pointer position every row.
(grub_video_i386_vbefill_R8

[PATCH] GSoC #07 VBE double buffering

[Colin D Bennett]

This patch adds double buffering support to the VBE video driver.  It
uses page flipping if possible, and falls back on using an offscreen
buffer which is blitted to video memory if page flipping is not
possible.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

VBE smart double buffering using page flipping or blitting.

* include/grub/video.h (GRUB_VIDEO_RENDER_TARGET_FRONT_BUFFER):
Removed.
(GRUB_VIDEO_RENDER_TARGET_BACK_BUFFER): Removed.

* video/i386/pc/vbe.c (framebuffer): Added is_double_buffered field.
(doublebuf_state): New struct.
(double_buffering_init): New function.
(grub_video_vbe_init): Clear doublebuf_state.
(grub_video_vbe_fini): Destroy doublebuf_state.
(grub_video_vbe_setup): Call double_buffering_init to initialize
double buffering.
(doublebuf_pageflipping_commit): New function.
(doublebuf_pageflipping_update_screen): New function.
(doublebuf_pageflipping_destroy): New function.
(doublebuf_pageflipping_init): New function.
(doublebuf_blit_update_screen): New function.
(doublebuf_blit_destroy): New function.
(doublebuf_blit_init): New function.
(doublebuf_null_update_screen): New function.
(doublebuf_null_destroy): New function.
(doublebuf_null_init): New function.
(double_buffering_init): New function.
(grub_video_vbe_swap_buffers): Implement buffer swapping.
(grub_video_vbe_set_active_render_target): Handle special target
GRUB_VIDEO_RENDER_TARGET_DISPLAY and neither
GRUB_VIDEO_RENDER_TARGET_FRONT_BUFFER nor
GRUB_VIDEO_RENDER_TARGET_BACK_BUFFER.
=== modified file 'include/grub/video.h'
--- include/grub/video.h	2008-08-31 01:02:11 +
+++ include/grub/video.h	2008-08-31 06:19:25 +
@@ -47,10 +47,11 @@
 #define GRUB_VIDEO_MODE_TYPE_DEPTH_MASK		0xff00
 #define GRUB_VIDEO_MODE_TYPE_DEPTH_POS		8
 
-/* Defined predefined render targets.  */
-#define GRUB_VIDEO_RENDER_TARGET_DISPLAY	((struct grub_video_render_target *) 0)
-#define GRUB_VIDEO_RENDER_TARGET_FRONT_BUFFER	((struct grub_video_render_target *) 0)
-#define GRUB_VIDEO_RENDER_TARGET_BACK_BUFFER	((struct grub_video_render_target *) 1)
+/* Predefined render target: */
+/* The render target that client code should render to. */
+#define GRUB_VIDEO_RENDER_TARGET_DISPLAY \
+  ((struct grub_video_render_target *) 0)
+
 
 /* Defined blitting formats.  */
 enum grub_video_blit_format
@@ -267,6 +268,9 @@
 
 grub_err_t grub_video_scroll (grub_video_color_t color, int dx, int dy);
 
+/* Swap the pages referred to by the front buffer and back buffer render
+ * targets, and the page previously referred to by the back buffer is made
+ * visible on the display. */
 grub_err_t grub_video_swap_buffers (void);
 
 grub_err_t grub_video_create_render_target (struct grub_video_render_target **result,

=== modified file 'video/i386/pc/vbe.c'
--- video/i386/pc/vbe.c	2008-08-31 01:07:22 +
+++ video/i386/pc/vbe.c	2008-08-31 06:19:26 +
@@ -63,6 +63,7 @@
 static struct
 {
   struct grub_video_render_target render_target;
+  int is_double_buffered;   /* Is the video mode double buffered? */
 
   unsigned int bytes_per_scan_line;
   unsigned int bytes_per_pixel;
@@ -77,6 +78,24 @@
 static grub_uint32_t mode_in_use = 0x55aa;
 static grub_uint16_t *mode_list;
 
+static struct 
+{
+  grub_size_t page_size;/* The size of a page in bytes. */
+
+  /* For page flipping strategy. */
+  int displayed_page;   /* The page # that is the front buffer. */
+  int render_page;  /* The page # that is the back buffer. */
+
+  /* For blit strategy. */
+  grub_uint8_t *offscreen_buffer;
+
+  /* Virtual functions. */
+  int (*update_screen) (void);
+  int (*destroy) (void);
+} doublebuf_state;
+
+static void double_buffering_init (void);
+
 static void *
 real2pm (grub_vbe_farptr_t ptr)
 {
@@ -376,6 +395,7 @@
   /* Reset frame buffer and render target variables.  */
   grub_memset (&framebuffer, 0, sizeof(framebuffer));
   render_target = &framebuffer.render_target;
+  grub_memset (&doublebuf_state, 0, sizeof(doublebuf_state));
 
   return GRUB_ERR_NONE;
 }
@@ -391,6 +411,9 @@
 /* TODO: Decide, is this something we want to do.  */
 return grub_errno;
 
+  if (doublebuf_state.destroy)
+doublebuf_state.destroy();
+
   /* TODO: Free any resources allocated by driver.  */
   grub_free (mode_list);
   mode_list = 0;
@@ -533,9 +556,13 @@
   render_target->viewport.width = active_mode_info.x_resolution;
   render_target->viewport.height = active_mode_info.y_resolution;
 
-  /* Set framebuffer pointer and mark it as non allocated.  */
+  /* Mark framebuffer memory as non allocated.  */
   render_target->is_allocated = 0;
-  render_target->data = framebuffer.ptr;
+
+  /* Set up double buffering information. */
+  fra

[PATCH] GSoC #08 vbeinfo enhancements

[Colin D Bennett]

This patch adds more information to the vbeinfo output, including the
video adapter VBE version, total video memory, and the color layout of
each video mode.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

* commands/i386/pc/vbeinfo.c (grub_cmd_vbeinfo): Show VBE version and
total video memory in 'vbeinfo' output; show color format details for
each video mode.
=== modified file 'commands/i386/pc/vbeinfo.c'
--- commands/i386/pc/vbeinfo.c	2007-07-21 22:32:33 +
+++ commands/i386/pc/vbeinfo.c	2008-08-31 06:28:15 +
@@ -48,12 +48,18 @@
   grub_err_t err;
   char *modevar;
 
-  grub_printf ("List of compatible video modes:\n");
-
   err = grub_vbe_probe (&controller_info);
   if (err != GRUB_ERR_NONE)
 return err;
 
+  grub_printf ("VBE info:   version: %d.%d  OEM software rev: %d.%d\n",
+   controller_info.version >> 8, 
+   controller_info.version & 0xFF,
+   controller_info.oem_software_rev >> 8,
+   controller_info.oem_software_rev & 0xFF);
+  grub_printf ("total memory: %d KiB\n",
+   (controller_info.total_memory << 16) / 1024);
+
   /* Because the information on video modes is stored in a temporary place,
  it is better to copy it to somewhere safe.  */
   p = video_mode_list = real2pm (controller_info.video_mode_ptr);
@@ -67,6 +73,10 @@
 
   grub_memcpy (saved_video_mode_list, video_mode_list, video_mode_list_size);
   
+  grub_printf ("List of compatible video modes:\n");
+  grub_printf ("Legend: P=Packed pixel, D=Direct color, "
+   "mask/pos=R/G/B/reserved\n"); 
+
   /* Walk through all video modes listed.  */
   for (p = saved_video_mode_list; *p != 0x; p++)
 {
@@ -103,10 +113,10 @@
   switch (mode_info_tmp.memory_model)
 	{
 	case 0x04:
-	  memory_model = "Packed Pixel";
+	  memory_model = "Packed";
 	  break;
 	case 0x06:
-	  memory_model = "Direct Color";
+	  memory_model = "Direct";
 	  break;
 
 	default:
@@ -116,12 +126,23 @@
   if (! memory_model)
 	continue;
 
-  grub_printf ("0x%03x: %d x %d x %d bpp (%s)\n",
-		   mode,
+  grub_printf ("0x%03x:  %4d x %4d x %2d  %s", 
+   mode,
mode_info_tmp.x_resolution,
mode_info_tmp.y_resolution,
mode_info_tmp.bits_per_pixel,
-		   memory_model);
+   memory_model);
+  if (memory_model[0] == 'D')
+grub_printf (", mask: %d/%d/%d/%d  pos: %d/%d/%d/%d",
+ mode_info_tmp.red_mask_size,
+ mode_info_tmp.green_mask_size,
+ mode_info_tmp.blue_mask_size,
+ mode_info_tmp.rsvd_mask_size,
+ mode_info_tmp.red_field_position,
+ mode_info_tmp.green_field_position,
+ mode_info_tmp.blue_field_position,
+ mode_info_tmp.rsvd_field_position);
+  grub_printf ("\n");
 }
 
   grub_free (saved_video_mode_list);



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

[PATCH] GSoC #09 Bitmap scaling

[Colin D Bennett]

  int dw = dst->mode_info.width;
+  int dh = dst->mode_info.height;
+  int sw = src->mode_info.width;
+  int sh = src->mode_info.height;
+  int dstride = dst->mode_info.pitch;
+  int sstride = src->mode_info.pitch;
+  /* bytes_per_pixel is the same for both src and dst. */
+  int bytes_per_pixel = dst->mode_info.bytes_per_pixel;
+
+  int dy;
+  for (dy = 0; dy < dh; dy++)
+{
+  int dx;
+  for (dx = 0; dx < dw; dx++)
+{
+  grub_uint8_t *dptr;
+  grub_uint8_t *sptr;
+  int sx;
+      int sy;
+  int comp;
+
+  /* Compute the source coordinate that the destination coordinate
+   * maps to.  Note: sx/sw = dx/dw  =>  sx = sw*dx/dw. */
+  sx = sw * dx / dw;
+  sy = sh * dy / dh;
+
+  /* Get the address of the pixels in src and dst. */
+  dptr = ddata + dy * dstride + dx * bytes_per_pixel;
+  sptr = sdata + sy * sstride + sx * bytes_per_pixel;
+
+  /* Copy the pixel color value. */
+  for (comp = 0; comp < bytes_per_pixel; comp++)
+dptr[comp] = sptr[comp];
+}
+}
+  return GRUB_ERR_NONE;
+}

2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

Image scaling support.  Also, gfxterm's background_image command can
scale the image to fit the screen (--mode/-m option).

* conf/i386-pc.rmk (bitmap_mod_SOURCES): Added new source files.

* include/grub/bitmap_scale.h: New file.
* video/bitmap_scale.c: New file.
* video/bitmap_scale_bilinear.c: New file.
* video/bitmap_scale_nn.c: New file.

* term/gfxterm.c (BACKGROUND_CMD_ARGINDEX_MODE): New macro.
(background_image_cmd_options): New static variable.
(grub_gfxterm_background_image_cmd): Handle --mode/-m option and scale
the bitmap to the screen size if requested.  Also includes a
"drive-by" fix to an erroneous assignment of the bitmap width to
'bitmap_height'.
(GRUB_MOD_INIT): Pass background_image_cmd_options to
grub_register_command.


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

Re: [PATCH] GSoC #08 vbeinfo enhancements

[Colin D Bennett]

On Sun, 31 Aug 2008 12:30:24 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Colin D Bennett wrote:
> > This patch adds more information to the vbeinfo output, including
> > the video adapter VBE version, total video memory, and the color
> > layout of each video mode.
> 
> Thanks for the patch. In it went with a slight modification.
> 
> I think we need to get rid of those magic values there thou...

Which magic values?


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

Re: [PATCH] GSoC #01 grub-mkrescue i386-pc multiple overlays

[Colin D Bennett]

On Sun, 31 Aug 2008 12:32:59 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Colin D Bennett wrote:
> > This is the first of many patches that I will be posting of my
> > Google Summer of Code 2008 work on GRUB.
> 
> Hi Colin,
> 
> Thanks for your patches so far. I don't think we can handle them all
> before you go to your holiday but we will try to be a bit more
> flexible this time, so if there is not big fixes to be made, we do
> them and commit them.

OK.  I'll try to get as much as possible posted before I go, at least.
If, after review, it is determined that some things need to be done
differently, I can make minor fixes in the next couple days, or when I
get back I can tackle somewhat larger changes.

> Thanks again for your work for GSoC 2008. I hope we can integrated
> this feature as soon as possible so others can also enjoy what it
> brings.
> 
> Thanks,
> Vesa Jääskeläinen

It's been a great experience working on GRUB with you all, and I also
look forward to seeing the code integrated into GRUB's mainline and
getting more people testing it.

Regards,
Colin


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

Re: [patch] background_image: image centering and scaling

[Colin D Bennett]

On Fri, 29 Aug 2008 22:11:02 -0700
Olaf Mandel <[EMAIL PROTECTED]> wrote:

> Vesa Jääskeläinen schrieb:
> -Snipp-
> > I will however look at if there are some ideas that you have
> > provided what is still missing from new graphical menu
> > implementation that should be there.
> -Snipp-
> 
> Hello,
> 
> thanks. I must admit to not looking at the source code for the
> project. Will scaling also be used for "simple" background_image
> commands? If yes and if the scaling does preserve the aspect ratio of
> the image, then the background picture is not necessarily in the
> upper left corner of the screen (an assumption in the code in grub2
> trunk).
> 
> I hope some version of the gfxmenu project finds its way into the
> trunk of the grub2 repository soon.  :-)
> 
> Best regards,
> Olaf


Hi Olaf,

Thanks for your work!  It would be great if you wanted to enhance my
code by adding a preserve-aspect-ratio mode; currently my
background_image command scales to fit the screen without regard for
the image's original aspect ratio.  Currently the '--mode' / '-m'
option to 'background_image' accepts the following values:

stretch
  Stretch the image to fit without regard for the original aspect ratio.

normal
  Do not scale the image; origin anchored to upper-left corner.

I think if you added a "fit" option which would fit the image
preserving aspect, that would be nice.  Also, you could either
add a "center" option which would center the background image without
altering its size.  I think the "normal" option will not be very
desirable when scaling and centering are both available, since I can't
imagine why you'd want an image stuck in the corner instead of centered
or scaled in some way.

If you want to see the code for this, take a look at the message I sent
to the grub-devel list today entitled "[PATCH] GSoC #09 Bitmap scaling".
This patch may require some of the prior patches as well, however
(possibly #06 and/or #07).

Regards,
Colin


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

Re: [RFC] USB Support

[Colin D Bennett]

On Thu, 28 Aug 2008 12:16:59 +0200
Felix Zielcke <[EMAIL PROTECTED]> wrote:

> +  grub_usb_iterate (usb_iterate);
> +}
> +
> +
> 
> Weird `' character which seems not to make sense.

Actually that is a form feed character, which the GNU coding standard
recommends to be used to break up source code at logical places:

  Please use formfeed characters (control-L) to divide the program into
  pages at logical places (but not within a function). It does not
  matter just how long the pages are, since they do not have to fit on a
  printed page. The formfeeds should appear alone on lines by
  themselves. 
(http://www.gnu.org/prep/standards/html_node/Formatting.html#Formatting)


> Index: bus/usb/uhci.c
> ===
> --- bus/usb/uhci.c(revision 0)
> +++ bus/usb/uhci.c(revision 0)
> 
> 
> + fail:
> +  if (u)
> +{
> +  grub_free ((void *) u->qh);
> +  grub_free (u->framelist);
> +}
> +  grub_free (u);
> 
> util/misc.c: grub_free () doestn't check *ptr before calling free ()
> But better util/misc.c get's changed then your code and bus/usb/uhci
> isn't compiled for grub-emu anyway :)

But the C standard says it's OK to call free() with a NULL pointer.


Regards,
Colin


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

[PATCH] GSoC #01 grub-mkrescue i386-pc multiple overlays (v2)

[Colin D Bennett]

On Sun, 31 Aug 2008 15:49:45 +0200
Robert Millan <[EMAIL PROTECTED]> wrote:

> Hi,
> 
> I'd suggest mentioning in --help that --overlay can be specified
> multiple times.

It is so.  New patch attached.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

* util/i386/pc/grub-mkrescue.in: Support multiple overlay directories.
=== modified file 'util/i386/pc/grub-mkrescue.in'
--- util/i386/pc/grub-mkrescue.in	2008-07-12 14:40:50 +
+++ util/i386/pc/grub-mkrescue.in	2008-08-31 15:32:33 +
@@ -43,6 +43,7 @@
   -v, --version   print the version information and exit
   --modules=MODULES   pre-load specified modules MODULES
   --overlay=DIR   overlay directory DIR in the memdisk image
+  (may be specified multiple times)
   --pkglibdir=DIR use images from directory DIR instead of ${pkglibdir}
   --grub-mkimage=FILE use FILE as grub-mkimage
   --image-type=TYPE   select floppy or cdrom (default)
@@ -71,7 +72,7 @@
 --modules=*)
 	modules=`echo "$option" | sed 's/--modules=//'` ;;
 --overlay=*)
-	overlay=`echo "$option" | sed 's/--overlay=//'` ;;
+	overlay=${overlay}${overlay:+ }`echo "$option" | sed 's/--overlay=//'` ;;
 --pkglibdir=*)
 	input_dir=`echo "$option" | sed 's/--pkglibdir=//'` ;;
 --grub-mkimage=*)
@@ -124,9 +125,10 @@
   echo "insmod $i"
 done > ${aux_dir}/boot/grub/grub.cfg
 
-if test "x$overlay" = x ; then : ; else
-  cp -dpR ${overlay}/* ${aux_dir}/
-fi
+for d in ${overlay}; do
+  echo "Overlaying $d"
+  cp -dpR "${d}"/* "${aux_dir}"/
+done
 
 if [ "x${image_type}" = xfloppy -o "x${emulation}" = xfloppy ] ; then
   # build memdisk



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

Re: [RFC] USB Support

[Colin D Bennett]

On Sun, 31 Aug 2008 18:13:56 +0200
Felix Zielcke <[EMAIL PROTECTED]> wrote:

> Thanks for the copy&paste and even the link to it.
> I still find it weird and I even wasn't able yet to produce them in
> Emacs.
> Control-L is the wrong key combination for it ;)

Try C-q C-L.

Cheers,
Colin


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

[PATCH] GSoC #08 [v2] vbeinfo enhancements

[Colin D Bennett]

On Sun, 31 Aug 2008 18:59:53 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Colin D Bennett wrote:
> > On Sun, 31 Aug 2008 12:30:24 +0300
> > Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:
> > 
> >> Colin D Bennett wrote:
> >>> This patch adds more information to the vbeinfo output, including
> >>> the video adapter VBE version, total video memory, and the color
> >>> layout of each video mode.
> >> Thanks for the patch. In it went with a slight modification.
> >>
> >> I think we need to get rid of those magic values there thou...
> > 
> > Which magic values?
> 
> 0x04 and 0x06 as used on memory model at least. There might be
> something else, those were there already...

OK, I have replaced those and added the other VBE constants for those
fields.  Even though many of the values are not currently used, it
seems to help document the other possible values and uses of the field.

Attached are both a full patch, and an incremental patch against the
first version.

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

* commands/i386/pc/vbeinfo.c (grub_cmd_vbeinfo): Show VBE version and
total video memory in 'vbeinfo' output; show color format details for
each video mode.  Added constant values for VBE mode info values, and
replaced magic numbers with references to constants.
=== modified file 'commands/i386/pc/vbeinfo.c'
--- commands/i386/pc/vbeinfo.c	2007-07-21 22:32:33 +
+++ commands/i386/pc/vbeinfo.c	2008-08-31 16:26:53 +
@@ -26,6 +26,31 @@
 #include 
 #include 
 
+/* Bits from the VBE "mode_attributes" field in the mode info struct.  */
+#define VBE_MODEATTR_SUPPORTED (1 << 0)
+#define VBE_MODEATTR_RESERVED_1(1 << 1)
+#define VBE_MODEATTR_BIOS_TTY_OUTPUT_SUPPORT   (1 << 2)
+#define VBE_MODEATTR_COLOR (1 << 3)
+#define VBE_MODEATTR_GRAPHICS  (1 << 4)
+#define VBE_MODEATTR_VGA_COMPATIBLE(1 << 5)
+#define VBE_MODEATTR_VGA_WINDOWED_AVAIL(1 << 6)
+#define VBE_MODEATTR_LINEAR_FB_AVAIL   (1 << 7)
+#define VBE_MODEATTR_DOUBLE_SCAN_AVAIL (1 << 8)
+#define VBE_MODEATTR_INTERLACED_AVAIL  (1 << 9)
+#define VBE_MODEATTR_TRIPLE_BUF_AVAIL  (1 << 10)
+#define VBE_MODEATTR_STEREO_AVAIL  (1 << 11)
+#define VBE_MODEATTR_DUAL_DISPLAY_START(1 << 12)
+
+/* Values for the VBE memory_model field in the mode info struct.  */
+#define VBE_MODE_MEMORY_MODEL_TEXT   0x00
+#define VBE_MODE_MEMORY_MODEL_CGA0x01
+#define VBE_MODE_MEMORY_MODEL_HERCULES   0x02
+#define VBE_MODE_MEMORY_MODEL_PLANAR 0x03
+#define VBE_MODE_MEMORY_MODEL_PACKED_PIXEL   0x04
+#define VBE_MODE_MEMORY_MODEL_NONCHAIN4_256  0x05
+#define VBE_MODE_MEMORY_MODEL_DIRECT_COLOR   0x06
+#define VBE_MODE_MEMORY_MODEL_YUV0x07
+
 static void *
 real2pm (grub_vbe_farptr_t ptr)
 {
@@ -48,12 +73,19 @@
   grub_err_t err;
   char *modevar;
 
-  grub_printf ("List of compatible video modes:\n");
-
   err = grub_vbe_probe (&controller_info);
   if (err != GRUB_ERR_NONE)
 return err;
 
+  grub_printf ("VBE info:   version: %d.%d  OEM software rev: %d.%d\n",
+   controller_info.version >> 8, 
+   controller_info.version & 0xFF,
+   controller_info.oem_software_rev >> 8,
+   controller_info.oem_software_rev & 0xFF);
+  /* The total_memory field is in 64 KiB units.  */
+  grub_printf ("total memory: %d KiB\n",
+   (controller_info.total_memory << 16) / 1024);
+
   /* Because the information on video modes is stored in a temporary place,
  it is better to copy it to somewhere safe.  */
   p = video_mode_list = real2pm (controller_info.video_mode_ptr);
@@ -67,6 +99,10 @@
 
   grub_memcpy (saved_video_mode_list, video_mode_list, video_mode_list_size);
   
+  grub_printf ("List of compatible video modes:\n");
+  grub_printf ("Legend: P=Packed pixel, D=Direct color, "
+   "mask/pos=R/G/B/reserved\n"); 
+
   /* Walk through all video modes listed.  */
   for (p = saved_video_mode_list; *p != 0x; p++)
 {
@@ -80,33 +116,33 @@
 	  continue;
 	}
 
-  if ((mode_info_tmp.mode_attributes & 0x001) == 0)
+  if ((mode_info_tmp.mode_attributes & VBE_MODEATTR_SUPPORTED) == 0)
 	/* If not available, skip it.  */
 	continue;
 
-  if ((mode_info_tmp.mode_attributes & 0x002) == 0)
+  if ((mode_info_tmp.mode_attributes & VBE_MODEATTR_RESERVED_1) == 0)
 	/* Not enough information.  */
 	continue;
 
-  if ((mode_info_tmp.mode_attributes & 0x008) == 0)
+  if ((mode_info_tmp.mode_attri

Re: [PATCH] GSoC #08 [v3] vbeinfo enhancements

[Colin D Bennett]

This refinement is against current SVN head.  It introduces the VBE
mode info field constant values into vbe.h and these are used instead
of the magic numbers in vbeinfo.c.

Regards,
Colin
2008-08-31  Colin D Bennett <[EMAIL PROTECTED]>

* commands/i386/pc/vbeinfo.c (grub_cmd_vbeinfo): Show VBE version and
total video memory in 'vbeinfo' output; show color format details for
each video mode.  Added constant values for VBE mode info values, and
replaced magic numbers with references to constants.

* include/grub/i386/pc/vbe.h: Created VBE mode info field value
constant macros.
=== modified file 'commands/i386/pc/vbeinfo.c'
--- commands/i386/pc/vbeinfo.c	2008-08-31 08:28:36 +
+++ commands/i386/pc/vbeinfo.c	2008-09-01 05:36:39 +
@@ -57,6 +57,7 @@
controller_info.version & 0xFF,
controller_info.oem_software_rev >> 8,
controller_info.oem_software_rev & 0xFF);
+  /* The total_memory field is in 64 KiB units.  */
   grub_printf ("total memory: %d KiB\n",
(controller_info.total_memory << 16) / 1024);
 
@@ -90,32 +91,32 @@
 	  continue;
 	}
 
-  if ((mode_info_tmp.mode_attributes & 0x001) == 0)
+  if ((mode_info_tmp.mode_attributes & GRUB_VBE_MODEATTR_SUPPORTED) == 0)
 	/* If not available, skip it.  */
 	continue;
 
-  if ((mode_info_tmp.mode_attributes & 0x002) == 0)
+  if ((mode_info_tmp.mode_attributes & GRUB_VBE_MODEATTR_RESERVED_1) == 0)
 	/* Not enough information.  */
 	continue;
 
-  if ((mode_info_tmp.mode_attributes & 0x008) == 0)
+  if ((mode_info_tmp.mode_attributes & GRUB_VBE_MODEATTR_COLOR) == 0)
 	/* Monochrome is unusable.  */
 	continue;
 
-  if ((mode_info_tmp.mode_attributes & 0x080) == 0)
+  if ((mode_info_tmp.mode_attributes & GRUB_VBE_MODEATTR_LFB_AVAIL) == 0)
 	/* We support only linear frame buffer modes.  */
 	continue;
 
-  if ((mode_info_tmp.mode_attributes & 0x010) == 0)
+  if ((mode_info_tmp.mode_attributes & GRUB_VBE_MODEATTR_GRAPHICS) == 0)
 	/* We allow only graphical modes.  */
 	continue;
 
   switch (mode_info_tmp.memory_model)
 	{
-	case 0x04:
+	case GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL:
 	  memory_model = "Packed";
 	  break;
-	case 0x06:
+	case GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR:
 	  memory_model = "Direct";
 	  break;
 
@@ -134,7 +135,7 @@
memory_model);
 
   /* Show mask and position details for direct color modes.  */
-  if (mode_info_tmp.memory_model == 0x06)
+  if (mode_info_tmp.memory_model == GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR)
 grub_printf (", mask: %d/%d/%d/%d  pos: %d/%d/%d/%d",
  mode_info_tmp.red_mask_size,
  mode_info_tmp.green_mask_size,

=== modified file 'include/grub/i386/pc/vbe.h'
--- include/grub/i386/pc/vbe.h	2008-01-01 12:02:07 +
+++ include/grub/i386/pc/vbe.h	2008-09-01 05:34:40 +
@@ -30,9 +30,30 @@
 /* VBE status codes.  */
 #define GRUB_VBE_STATUS_OK		0x004f
 
-/* VBE memory model types.  */
-#define GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL	0x04
-#define GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR	0x06
+/* Bits from the GRUB_VBE "mode_attributes" field in the mode info struct.  */
+#define GRUB_VBE_MODEATTR_SUPPORTED (1 << 0)
+#define GRUB_VBE_MODEATTR_RESERVED_1(1 << 1)
+#define GRUB_VBE_MODEATTR_BIOS_TTY_OUTPUT_SUPPORT   (1 << 2)
+#define GRUB_VBE_MODEATTR_COLOR (1 << 3)
+#define GRUB_VBE_MODEATTR_GRAPHICS  (1 << 4)
+#define GRUB_VBE_MODEATTR_VGA_COMPATIBLE(1 << 5)
+#define GRUB_VBE_MODEATTR_VGA_WINDOWED_AVAIL(1 << 6)
+#define GRUB_VBE_MODEATTR_LFB_AVAIL (1 << 7)
+#define GRUB_VBE_MODEATTR_DOUBLE_SCAN_AVAIL (1 << 8)
+#define GRUB_VBE_MODEATTR_INTERLACED_AVAIL  (1 << 9)
+#define GRUB_VBE_MODEATTR_TRIPLE_BUF_AVAIL  (1 << 10)
+#define GRUB_VBE_MODEATTR_STEREO_AVAIL  (1 << 11)
+#define GRUB_VBE_MODEATTR_DUAL_DISPLAY_START(1 << 12)
+
+/* Values for the GRUB_VBE memory_model field in the mode info struct.  */
+#define GRUB_VBE_MEMORY_MODEL_TEXT   0x00
+#define GRUB_VBE_MEMORY_MODEL_CGA0x01
+#define GRUB_VBE_MEMORY_MODEL_HERCULES   0x02
+#define GRUB_VBE_MEMORY_MODEL_PLANAR 0x03
+#define GRUB_VBE_MEMORY_MODEL_PACKED_PIXEL   0x04
+#define GRUB_VBE_MEMORY_MODEL_NONCHAIN4_256  0x05
+#define GRUB_VBE_MEMORY_MODEL_DIRECT_COLOR   0x06
+#define GRUB_VBE_MEMORY_MODEL_YUV0x07
 
 /* Note:
 



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

[PATCH] GSoC #12 videotest enhancements

[Colin D Bennett]

This patch adds a number of different tests to the 'videotest' command,
including a benchmark test that tabulates the performance results for
various video modes and graphics operations.

The tests are accessed by specifying the name as an argument, such as:

  videotest bench

Also, most tests support a time value (-t/--time to specify the time
to run the test in seconds), and double buffering can be requested by
specifying -d/--dbuf.

The grub_rect_t type is included in video.h because it is used not only
in the videotest module, but is also used extensively in the gfxmenu
code (an upcoming patch).

Regards,
Colin
2008-08-30  Colin D Bennett <[EMAIL PROTECTED]>

	Enhance the 'videotest' command with a number of different tests.

	* commands/videotest.c (grub_cmd_videotest): Support running different
	tests and handling command line arguments for several flags.
	(ARGINDEX_TEST_TIME): Added.
	(ARGINDEX_DOUBLE_BUF): Likewise.
	(arg_options): Likewise.
	(DEFAULT_TEST_TIME): Likewise.
	(videotest_options): Likewise.
	(basic_video_test): Likewise.
	(bitmap_demo): Likewise.
	(benchmark_config): Likewise.
	(MODE_TYPE_RGB): Likewise.
	(benchmark_configs): Likewise.
	(NUM_BENCHMARK_CONFIGS): Likewise.
	(benchmark_result): Likewise.
	(BENCHMARK_RESULT_FPS_SCALE): Likewise.
	(move_rectangle_one_step): Likewise.
	(do_benchmark): Likewise.
	(benchmark_test): Likewise.
	(clock_test): Likewise.
	(doublebuf_test): Likewise.
	(text_test): Likewise.
	(scale_test): Likewise.
	(list_tests): Likewise.

	* include/grub/video.h (grub_video_rect_t: Added.


=== modified file 'commands/videotest.c'
--- commands/videotest.c	2008-08-31 03:50:37 +
+++ commands/videotest.c	2008-09-01 17:33:56 +
@@ -17,6 +17,7 @@
  */
 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -26,15 +27,39 @@
 #include 
 #include 
 #include 
-
-static grub_err_t
-grub_cmd_videotest (struct grub_arg_list *state __attribute__ ((unused)),
-int argc __attribute__ ((unused)),
-char **args __attribute__ ((unused)))
-{
+#include 
+#include 
+#include/* to test grub_vbe_bios_set_display_start */
+
+/* Option array indices.  */
+#define ARGINDEX_TEST_TIME 0
+#define ARGINDEX_DOUBLE_BUF 1
+
+static const struct grub_arg_option arg_options[] = {
+  {"time", 't', 0, "Time to run each test, in seconds.", 0, ARG_TYPE_INT},
+  {"dbuf", 'd', 0, "Use double buffered graphics.", 0, ARG_TYPE_NONE},
+  {0, 0, 0, 0, 0, 0}
+};
+
+#define DEFAULT_TEST_TIME 5
+
+/* Command options -- populated base on command line arguments.  */
+struct videotest_options
+{
+  int test_time;
+  int double_buffering;
+};
+
+
+static void
+basic_video_test (struct videotest_options *vt_opts)
+{
+  int mode_type = GRUB_VIDEO_MODE_TYPE_INDEX_COLOR;
+  if (vt_opts->double_buffering)
+mode_type |= GRUB_VIDEO_MODE_TYPE_DOUBLE_BUFFERED;
   if (grub_video_setup (1024, 768,
 GRUB_VIDEO_MODE_TYPE_INDEX_COLOR) != GRUB_ERR_NONE)
-return grub_errno;
+return;
 
   grub_getkey ();
 
@@ -68,9 +93,12 @@
 
   font = grub_font_get ("Helvetica Bold 14");
   if (! font)
-return grub_error (GRUB_ERR_BAD_FONT, "No font loaded.");
+{
+  grub_error (GRUB_ERR_BAD_FONT, "No font loaded.");
+  return;
+}
 
-  glyph = grub_font_get_glyph (font, '*');  
+  glyph = grub_font_get_glyph (font, '*');
   grub_video_blit_glyph (glyph, color, 200 ,0);
 
   grub_video_set_viewport (x + 150, y + 150,
@@ -106,30 +134,1075 @@
  0, 0, width, height);
 }
 
+  grub_video_swap_buffers ();
+  grub_getkey ();
+
+  /* Test VBE set display start address.  */
+  /* This should scroll the screen, first vertically,
+   * then horizontally.  The horizontal scrolling seems to
+   * only have a resolution of about 16 pixels on my VIA Mini-ITX.  */
+  int vbestatus;
+  int vbeok = 0;
+  int vbeerr = 0;
+  for (i = 0; i < 50; i++)
+{
+  vbestatus = grub_vbe_bios_set_display_start (0, i);
+  if (vbestatus == GRUB_VBE_STATUS_OK)
+vbeok++;
+  else
+vbeerr++;
+}
+
+  grub_getkey ();
+
+  for (i = 0; i < 50; i++)
+{
+  vbestatus = grub_vbe_bios_set_display_start (i, 50);
+  if (vbestatus == GRUB_VBE_STATUS_OK)
+vbeok++;
+  else
+vbeerr++;
+}
+
   grub_getkey ();
 
   grub_video_delete_render_target (text_layer);
-
-  grub_video_restore ();
-
-  for (i = 0; i < 16; i++)
-grub_printf("color %d: %08x\n", i, palette[i]);
-
-  grub_errno = GRUB_ERR_NONE;
+  grub_video_restore ();
+
+  grub_printf ("VBE set_display_start status: %d\n", vbestatus);
+  grub_printf ("ok: %d\n", vbeok);
+  grub_printf ("errors: %d\n", vbeerr);
+
+  grub_errno = GRUB_ERR_NONE;
+}
+
+
+
+/**
+ * Simple opaque image blit test.
+ * Retur

Re: [RFC] make output of compiling a bit less verbose to make warnings more visible

[Colin D Bennett]

On Tue, 02 Sep 2008 22:43:54 +0300
Vesa Jääskeläinen <[EMAIL PROTECTED]> wrote:

> Robert Millan wrote:
> > On Tue, Sep 02, 2008 at 07:10:05PM +0200, Felix Zielcke wrote:
> >> Am Dienstag, den 02.09.2008, 18:43 +0200 schrieb Felix Zielcke:
> >>> Hello,
> >>>
> >>> I think it would be good if the output from compiling wouldn't be
> >>> that verbose.
> >> Urm thanks again to Vesa who remembered that Colin brought that
> >> topic originally up here.
> >>
> >> http://lists.gnu.org/archive/html/grub-devel/2008-06/msg00248.html
> >>
> >> I really would love to have this somehow commited :)
> > 
> > I don't remember that discussion well;  ISTR I complained.  Maybe
> > that stopped it?  Anyway, as long as it's user-selectable it's not
> > a problem for me.  Maybe I even end up using it ;-)
> 
> I think the strength was that you could provide V=99 (or something
> like that to give previous behavior ("debug feature").

In my 'pretty make' patch, setting V=1 (verbose on) restores original
full command output.

> So basically that is completed.
> 
> Only thing perhaps would need attention is to show that now we compile
> for HOST CC and now for TARGET CC or so.

Also, it was suggested that new variables (like CC, STRIP, etc.) be
introduced an instead of overriding the existing variables, which my
patch currently does as follows (this is 90% of the patch
functionality):

### Pretty output control ###
# Set up compiler and linker commands that either is quiet (does not
print # the command line being executed) or verbose (print the command
line). 
_CC := $(CC)
_TARGET_CC := $(TARGET_CC)
_STRIP := $(STRIP)
_GENMODSRC := sh $(srcdir)/genmodsrc.sh
ifeq ($(V),1)
 override V_PREFIX :=
 override CC = $(_CC)
 override TARGET_CC = $(_CC)
 override STRIP = $(_STRIP)
 override GENMODSRC = $(_GENMODSRC)
 override INFO_GENCMDLIST =
 override INFO_GENFSLIST =
 override INFO_GENPARTMAPLIST =
 override INFO_GEN_FINAL_COMMAND_LIST =
 override INFO_GEN_FINAL_FS_LIST =
 override INFO_GEN_FINAL_PARTMAP_LIST =
else
 override V_PREFIX := @
 override CC = @echo "COMPILE $<"; $(_CC)
 override TARGET_CC = @echo "COMPILE(TARGET) $<"; $(_TARGET_CC)
 override STRIP = @echo "STRIP   $@"; $(_STRIP)
 override GENMODSRC = @echo "GENMODSRC   $@"; $(_GENMODSRC)
 override INFO_GENCMDLIST = @echo "GENCMDLIST  $@"
 override INFO_GENFSLIST = @echo "GENFSLIST   $@"
 override INFO_GENPARTMAPLIST = @echo "GENPARTMAPLIST  $@"
 override INFO_GEN_FINAL_COMMAND_LIST = @echo "GENCMDLIST[final]  $@"
 override INFO_GEN_FINAL_FS_LIST = @echo "GENFSLIST[final]   $@"
 override INFO_GEN_FINAL_PARTMAP_LIST = @echo "GENPARTMAPLIST[final]
$@" endif


I have held off on doing this with all new variables, because it would
mean changing *every* reference to the toolchain variables.  I found
that by simply overriding them, it was really nonintrusive, in
general.  However, there were developers that objected to the
overriding.

Regards,
Colin


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

Re: Can LUA e coming with future Grub2

[Colin D Bennett]

On Wed, 3 Sep 2008 09:08:13 +0800
"y.volta" <[EMAIL PROTECTED]> wrote:

> hi,
> 
> i get to know the Fancy menu branch has lua supported, and just
> wondering, the official release of Grub2 will have Lua inside ( as a
> mod )? 

It would be cool, to be sure!  I never used Lua before, and it was a
great experience integrating it into GRUB; Lua is very easy and clean
to integrate, and it makes adding totally customizable functionality to
themes really easy.

I haven't had a chance to really discuss the possibility of Lua support
with the core GRUB developers, however.

My plan was to modularize Lua in such a way that it is an optionally
loadable piece of the gfxmenu functionality.  Currently the Lua branch
has a hard dependency on Lua.

> if so, i'm usre, it will extend grub2 much more: for it can load
> its function library and its scripts. Let's suppose we are trying
> develop a window GUI style menu system, just like the XOSL does, we
> can make this as a lua module, user can design its dialog or controls
> layout in its scripts and 'require ("gui.so");' to have modern window
> based menu UI. ;-)
> 
> at the same time, we can also use lua to extend the grub.cfg,
> isn't it?

Actually, I think replacing GRUB's "bash-ish" scripting with Lua would
be great in many ways, but it is true that Lua (at ~100 KB) is larger
than the GRUB script engine.

Regards,
Colin


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

Re: About VBE double buffering

[Colin D Bennett]

On Wed, 10 Sep 2008 08:24:21 +0800
"y.volta" <[EMAIL PROTECTED]> wrote:

> 
> I noticed that `grub_vbe_bios_set_display_start () which used
> AX=4F07h INT10h' used in the fancy menu patch. This will use the
> video buffer to do buffering, but this function will fail on some
> system, Collin told us in his Journal:
> 
> "However, VMware doesn't seem to support the VBE set display start
> operation, at least in the video mode that GRUB uses (for the basic
> video test, which uses indexed color 1024x768 mode). "
> 
> so, this may have compatible issue. i think, is there any
> possible, use a non-video-card memory to be the drawing buffer, when
> done, we copy its contents to video memory? this obviously has
> performance issue, but has more compatible.

Hi,

Actually I have implemented the behavior you describe.  It is called
the 'blit' double buffering strategy, as opposed to the 'page flipping'
strategy that simply changes the display start address.

The gfxmenu detects whether page flipping is supported, and whether
there is enough video memory.  If not, then the blit method is used.

Regards,
Colin


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