[PATCH] linux/gfxterm integration

[PATCH] linux/gfxterm integration

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 1002 bytes --]


This patch integrates the generic Linux loader with gfxterm.  The result is
that graphical mode becomes usable with this loader.  Our loader gets the
screen settings from the video subsystem (as per gfxterm setup), and passes
them to Linux.

This way GRUB/gfxterm can transition to Linux/fb with no further mode
setting.  Perhaps this can be exploited to make the transition seamless
by using the same background image in both places, but I haven't explored
this possibility yet :-)

Note: As the comment in grub/video.h says, struct grub_video_render_target
didn't really want to be moved.  My code only needs to access it to find
the framebuffer address.  Perhaps it'd be better to move this information
elsewhere?  Or split it in a separate structure / getter?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: linux_gfxterm.diff --]
[-- Type: text/x-diff, Size: 5819 bytes --]

2009-03-06  Robert Millan  <rmh@aybabtu.com>

	* include/grub/i386/pc/vbe.h (struct grub_video_render_target): Move
	from here ...
	* include/grub/video.h (struct grub_video_render_target): ... to here.

	* loader/i386/linux.c: Inclue `<grub/video.h>'.
	(grub_linux_setup_video): New function.  Loosely based on the EFI one.
	(grub_rescue_cmd_linux): Attempt to configure video settings with
	grub_linux_setup_video().
	Set noreturn=0 in grub_loader_set, in order to avoid grub_console_fini()
	which would step out of graphical mode unconditionally.

Index: include/grub/i386/pc/vbe.h
===================================================================
--- include/grub/i386/pc/vbe.h	(revision 2019)
+++ include/grub/i386/pc/vbe.h	(working copy)
@@ -227,29 +227,6 @@ grub_err_t grub_vbe_get_video_mode_info 
 /* VBE module internal prototypes (should not be used from elsewhere).  */
 struct grub_video_i386_vbeblit_info;
 
-struct grub_video_render_target
-{
-  /* Copy of the screen's mode info structure, except that width, height and
-     mode_type has been re-adjusted to requested render target settings.  */
-  struct grub_video_mode_info mode_info;
-
-  struct
-  {
-    unsigned int x;
-    unsigned int y;
-    unsigned int width;
-    unsigned int height;
-  } viewport;
-
-  /* Indicates whether the data has been allocated by us and must be freed
-     when render target is destroyed.  */
-  int is_allocated;
-
-  /* Pointer to data.  Can either be in video card memory or in local host's
-     memory.  */
-  void *data;
-};
-
 grub_uint8_t * grub_video_vbe_get_video_ptr (struct grub_video_i386_vbeblit_info *source,
                                              grub_uint32_t x, grub_uint32_t y);
 
Index: include/grub/video.h
===================================================================
--- include/grub/video.h	(revision 2019)
+++ include/grub/video.h	(working copy)
@@ -26,10 +26,6 @@
    specific coding format.  */
 typedef grub_uint32_t grub_video_color_t;
 
-/* This structure is driver specific and should not be accessed directly by
-   outside code.  */
-struct grub_video_render_target;
-
 /* Forward declarations for used data structures.  */
 struct grub_video_bitmap;
 
@@ -152,6 +148,29 @@ struct grub_video_mode_info
   grub_uint8_t fg_alpha;
 };
 
+struct grub_video_render_target
+{
+  /* Copy of the screen's mode info structure, except that width, height and
+     mode_type has been re-adjusted to requested render target settings.  */
+  struct grub_video_mode_info mode_info;
+
+  struct
+  {
+    unsigned int x;
+    unsigned int y;
+    unsigned int width;
+    unsigned int height;
+  } viewport;
+
+  /* Indicates whether the data has been allocated by us and must be freed
+     when render target is destroyed.  */
+  int is_allocated;
+
+  /* Pointer to data.  Can either be in video card memory or in local host's
+     memory.  */
+  void *data;
+};
+
 struct grub_video_palette_data
 {
   grub_uint8_t r; /* Red color value (0-255).  */
Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2019)
+++ loader/i386/linux.c	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2006,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2006,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -30,6 +30,7 @@
 #include <grub/mm.h>
 #include <grub/term.h>
 #include <grub/cpu/linux.h>
+#include <grub/video.h>
 
 #define GRUB_LINUX_CL_OFFSET		0x1000
 #define GRUB_LINUX_CL_END_OFFSET	0x2000
@@ -304,6 +304,41 @@ grub_linux_unload (void)
   return GRUB_ERR_NONE;
 }
 
+static int
+grub_linux_setup_video (struct linux_kernel_params *params)
+{
+  struct grub_video_mode_info mode_info;
+  struct grub_video_render_target *render_target;
+  int ret;
+
+  ret = grub_video_get_info (&mode_info);
+  if (ret)
+    return 1;
+
+  ret = grub_video_get_active_render_target (&render_target);
+  if (ret)
+    return 1;
+
+  params->lfb_width = mode_info.width;
+  params->lfb_height = mode_info.height;
+  params->lfb_depth = mode_info.bpp;
+  params->lfb_line_len = mode_info.pitch;
+
+  params->lfb_base = (void *) render_target->data;
+  params->lfb_size = (params->lfb_line_len * params->lfb_height + 65535) >> 16;
+
+  params->red_mask_size = 8;
+  params->red_field_pos = 16;
+  params->green_mask_size = 8;
+  params->green_field_pos = 8;
+  params->blue_mask_size = 8;
+  params->blue_field_pos = 0;
+  params->reserved_mask_size = 8;
+  params->reserved_field_pos = 24;
+
+  return 0;
+}
+
 void
 grub_rescue_cmd_linux (int argc, char *argv[])
 {
@@ -315,7 +350,6 @@ grub_rescue_cmd_linux (int argc, char *a
   grub_ssize_t len;
   int i;
   char *dest;
-  int video_type;
 
   grub_dl_ref (my_mod);
   
@@ -423,7 +457,6 @@ grub_rescue_cmd_linux (int argc, char *a
 
   /* Detect explicitly specified memory size, if any.  */
   linux_mem_size = 0;
-  video_type = 0;
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "mem=", 4) == 0)
       {
@@ -459,6 +492,9 @@ grub_rescue_cmd_linux (int argc, char *a
 	      linux_mem_size <<= shift;
 	  }
       }
+
+  if (! grub_linux_setup_video (params))
+    params->have_vga = GRUB_VIDEO_TYPE_VLFB;
   
   /* Specify the boot file.  */
   dest = grub_stpcpy ((char *) real_mode_mem + GRUB_LINUX_CL_OFFSET,
@@ -482,7 +518,8 @@ grub_rescue_cmd_linux (int argc, char *a
 
   if (grub_errno == GRUB_ERR_NONE)
     {
-      grub_loader_set (grub_linux32_boot, grub_linux_unload, 1);
+      grub_loader_set (grub_linux32_boot, grub_linux_unload,
+		       0 /* set noreturn=0 in order to avoid grub_console_fini() */);
       loaded = 1;
     }
 

Re: [PATCH] linux/gfxterm integration

[Robert Millan]

On Fri, Mar 06, 2009 at 08:57:35PM +0100, Robert Millan wrote:
> 
> This patch integrates the generic Linux loader with gfxterm.  The result is
> that graphical mode becomes usable with this loader.  Our loader gets the
> screen settings from the video subsystem (as per gfxterm setup), and passes
> them to Linux.
> 
> This way GRUB/gfxterm can transition to Linux/fb with no further mode
> setting.  Perhaps this can be exploited to make the transition seamless
> by using the same background image in both places, but I haven't explored
> this possibility yet :-)
> 
> Note: As the comment in grub/video.h says, struct grub_video_render_target
> didn't really want to be moved.  My code only needs to access it to find
> the framebuffer address.  Perhaps it'd be better to move this information
> elsewhere?  Or split it in a separate structure / getter?

New patch.  This one calls grub_linux_setup_video() from grub_linux32_boot
instead of grub_rescue_cmd_linux.  This way it gets the video status we have
when "boot" is issued instead of the one we have when linux is loaded.

Any comments on my question about struct grub_video_render_target ?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] linux/gfxterm integration

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]


Sorry, forgot to attach it.

On Sun, Mar 08, 2009 at 02:01:06PM +0100, Robert Millan wrote:
> On Fri, Mar 06, 2009 at 08:57:35PM +0100, Robert Millan wrote:
> > 
> > This patch integrates the generic Linux loader with gfxterm.  The result is
> > that graphical mode becomes usable with this loader.  Our loader gets the
> > screen settings from the video subsystem (as per gfxterm setup), and passes
> > them to Linux.
> > 
> > This way GRUB/gfxterm can transition to Linux/fb with no further mode
> > setting.  Perhaps this can be exploited to make the transition seamless
> > by using the same background image in both places, but I haven't explored
> > this possibility yet :-)
> > 
> > Note: As the comment in grub/video.h says, struct grub_video_render_target
> > didn't really want to be moved.  My code only needs to access it to find
> > the framebuffer address.  Perhaps it'd be better to move this information
> > elsewhere?  Or split it in a separate structure / getter?
> 
> New patch.  This one calls grub_linux_setup_video() from grub_linux32_boot
> instead of grub_rescue_cmd_linux.  This way it gets the video status we have
> when "boot" is issued instead of the one we have when linux is loaded.
> 
> Any comments on my question about struct grub_video_render_target ?
> 
> -- 
> Robert Millan
> 
>   The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
>   how) you may access your data; but nobody's threatening your freedom: we
>   still allow you to remove your data and not access it at all."

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: linux_gfxterm.diff --]
[-- Type: text/x-diff, Size: 5844 bytes --]

2009-03-06  Robert Millan  <rmh@aybabtu.com>

	* include/grub/i386/pc/vbe.h (struct grub_video_render_target): Move
	from here ...
	* include/grub/video.h (struct grub_video_render_target): ... to here.

	* loader/i386/linux.c: Inclue `<grub/video.h>'.
	(grub_linux_setup_video): New function.  Loosely based on the EFI one.
	(grub_linux32_boot): Attempt to configure video settings with
	grub_linux_setup_video().
	(grub_rescue_cmd_linux): Set noreturn=0 in grub_loader_set, in order
	to avoid grub_console_fini() which would step out of graphical mode
	unconditionally.

Index: include/grub/i386/pc/vbe.h
===================================================================
--- include/grub/i386/pc/vbe.h	(revision 2021)
+++ include/grub/i386/pc/vbe.h	(working copy)
@@ -227,29 +227,6 @@ grub_err_t grub_vbe_get_video_mode_info 
 /* VBE module internal prototypes (should not be used from elsewhere).  */
 struct grub_video_i386_vbeblit_info;
 
-struct grub_video_render_target
-{
-  /* Copy of the screen's mode info structure, except that width, height and
-     mode_type has been re-adjusted to requested render target settings.  */
-  struct grub_video_mode_info mode_info;
-
-  struct
-  {
-    unsigned int x;
-    unsigned int y;
-    unsigned int width;
-    unsigned int height;
-  } viewport;
-
-  /* Indicates whether the data has been allocated by us and must be freed
-     when render target is destroyed.  */
-  int is_allocated;
-
-  /* Pointer to data.  Can either be in video card memory or in local host's
-     memory.  */
-  void *data;
-};
-
 grub_uint8_t * grub_video_vbe_get_video_ptr (struct grub_video_i386_vbeblit_info *source,
                                              grub_uint32_t x, grub_uint32_t y);
 
Index: include/grub/video.h
===================================================================
--- include/grub/video.h	(revision 2021)
+++ include/grub/video.h	(working copy)
@@ -26,10 +26,6 @@
    specific coding format.  */
 typedef grub_uint32_t grub_video_color_t;
 
-/* This structure is driver specific and should not be accessed directly by
-   outside code.  */
-struct grub_video_render_target;
-
 /* Forward declarations for used data structures.  */
 struct grub_video_bitmap;
 
@@ -152,6 +148,29 @@ struct grub_video_mode_info
   grub_uint8_t fg_alpha;
 };
 
+struct grub_video_render_target
+{
+  /* Copy of the screen's mode info structure, except that width, height and
+     mode_type has been re-adjusted to requested render target settings.  */
+  struct grub_video_mode_info mode_info;
+
+  struct
+  {
+    unsigned int x;
+    unsigned int y;
+    unsigned int width;
+    unsigned int height;
+  } viewport;
+
+  /* Indicates whether the data has been allocated by us and must be freed
+     when render target is destroyed.  */
+  int is_allocated;
+
+  /* Pointer to data.  Can either be in video card memory or in local host's
+     memory.  */
+  void *data;
+};
+
 struct grub_video_palette_data
 {
   grub_uint8_t r; /* Red color value (0-255).  */
Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2022)
+++ loader/i386/linux.c	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2006,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2006,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -30,6 +30,7 @@
 #include <grub/mm.h>
 #include <grub/term.h>
 #include <grub/cpu/linux.h>
+#include <grub/video.h>
 
 #define GRUB_LINUX_CL_OFFSET		0x1000
 #define GRUB_LINUX_CL_END_OFFSET	0x2000
@@ -215,6 +216,41 @@ grub_e820_add_region (struct grub_e820_m
     }
 }
 
+static int
+grub_linux_setup_video (struct linux_kernel_params *params)
+{
+  struct grub_video_mode_info mode_info;
+  struct grub_video_render_target *render_target;
+  int ret;
+
+  ret = grub_video_get_info (&mode_info);
+  if (ret)
+    return 1;
+
+  ret = grub_video_get_active_render_target (&render_target);
+  if (ret)
+    return 1;
+
+  params->lfb_width = mode_info.width;
+  params->lfb_height = mode_info.height;
+  params->lfb_depth = mode_info.bpp;
+  params->lfb_line_len = mode_info.pitch;
+
+  params->lfb_base = (void *) render_target->data;
+  params->lfb_size = (params->lfb_line_len * params->lfb_height + 65535) >> 16;
+
+  params->red_mask_size = 8;
+  params->red_field_pos = 16;
+  params->green_mask_size = 8;
+  params->green_field_pos = 8;
+  params->blue_mask_size = 8;
+  params->blue_field_pos = 0;
+  params->reserved_mask_size = 8;
+  params->reserved_field_pos = 24;
+
+  return 0;
+}
+
 #ifdef __x86_64__
 struct
 {
@@ -231,6 +267,9 @@ grub_linux32_boot (void)
   
   params = real_mode_mem;
 
+  if (! grub_linux_setup_video (params))
+    params->have_vga = GRUB_VIDEO_TYPE_VLFB;
+  
   grub_dprintf ("linux", "code32_start = %x, idt_desc = %lx, gdt_desc = %lx\n",
 		(unsigned) params->code32_start,
                 (unsigned long) &(idt_desc.limit),
@@ -314,7 +353,6 @@ grub_rescue_cmd_linux (int argc, char *a
   grub_ssize_t len;
   int i;
   char *dest;
-  int video_type;
 
   grub_dl_ref (my_mod);
   
@@ -422,7 +460,6 @@ grub_rescue_cmd_linux (int argc, char *a
 
   /* Detect explicitly specified memory size, if any.  */
   linux_mem_size = 0;
-  video_type = 0;
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "mem=", 4) == 0)
       {
@@ -481,7 +518,8 @@ grub_rescue_cmd_linux (int argc, char *a
 
   if (grub_errno == GRUB_ERR_NONE)
     {
-      grub_loader_set (grub_linux32_boot, grub_linux_unload, 1);
+      grub_loader_set (grub_linux32_boot, grub_linux_unload,
+		       0 /* set noreturn=0 in order to avoid grub_console_fini() */);
       loaded = 1;
     }
 

Re: [PATCH] linux/gfxterm integration

[Vesa Jääskeläinen]

Robert Millan wrote:
> On Fri, Mar 06, 2009 at 08:57:35PM +0100, Robert Millan wrote:
>> This patch integrates the generic Linux loader with gfxterm.  The result is
>> that graphical mode becomes usable with this loader.  Our loader gets the
>> screen settings from the video subsystem (as per gfxterm setup), and passes
>> them to Linux.
>>
>> This way GRUB/gfxterm can transition to Linux/fb with no further mode
>> setting.  Perhaps this can be exploited to make the transition seamless
>> by using the same background image in both places, but I haven't explored
>> this possibility yet :-)
>>
>> Note: As the comment in grub/video.h says, struct grub_video_render_target
>> didn't really want to be moved.  My code only needs to access it to find
>> the framebuffer address.  Perhaps it'd be better to move this information
>> elsewhere?  Or split it in a separate structure / getter?
> 
> New patch.  This one calls grub_linux_setup_video() from grub_linux32_boot
> instead of grub_rescue_cmd_linux.  This way it gets the video status we have
> when "boot" is issued instead of the one we have when linux is loaded.
> 
> Any comments on my question about struct grub_video_render_target ?

Don't like em... Let me think it a bit.

Re: [PATCH] linux/gfxterm integration

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 1039 bytes --]

On Sun, Mar 08, 2009 at 03:35:30PM +0200, Vesa Jääskeläinen wrote:
> Robert Millan wrote:
> >> Note: As the comment in grub/video.h says, struct grub_video_render_target
> >> didn't really want to be moved.  My code only needs to access it to find
> >> the framebuffer address.  Perhaps it'd be better to move this information
> >> elsewhere?  Or split it in a separate structure / getter?
> > 
> > Any comments on my question about struct grub_video_render_target ?
> 
> Don't like em... Let me think it a bit.

This new patch avoids the problem by accessing struct grub_video_render_target
from its current location.

When we have more than one video backend, and a clear idea on which parts are
generic and which are backend-specific, we could readjust.  For now this makes
VBE happy.

Thoughts?

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: linux_gfxterm.diff --]
[-- Type: text/x-diff, Size: 3390 bytes --]

2009-03-13  Robert Millan  <rmh@aybabtu.com>

	* loader/i386/linux.c: Include `<grub/video.h>' and
	`<grub/i386/pc/vbe.h>'..
	(grub_linux_setup_video): New function.  Loosely based on the EFI one.
	(grub_linux32_boot): Attempt to configure video settings with
	grub_linux_setup_video().
	(grub_rescue_cmd_linux): Set noreturn=0 in grub_loader_set, in order
	to avoid grub_console_fini() which would step out of graphical mode
	unconditionally.

Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2030)
+++ loader/i386/linux.c	(working copy)
@@ -1,6 +1,6 @@
 /*
  *  GRUB  --  GRand Unified Bootloader
- *  Copyright (C) 2006,2007,2008  Free Software Foundation, Inc.
+ *  Copyright (C) 2006,2007,2008,2009  Free Software Foundation, Inc.
  *
  *  GRUB is free software: you can redistribute it and/or modify
  *  it under the terms of the GNU General Public License as published by
@@ -30,6 +30,10 @@
 #include <grub/mm.h>
 #include <grub/term.h>
 #include <grub/cpu/linux.h>
+#include <grub/video.h>
+/* FIXME: the definition of `struct grub_video_render_target' is
+   VBE-specific.  */
+#include <grub/i386/pc/vbe.h>
 
 #define GRUB_LINUX_CL_OFFSET		0x1000
 #define GRUB_LINUX_CL_END_OFFSET	0x2000
@@ -215,6 +219,41 @@ grub_e820_add_region (struct grub_e820_m
     }
 }
 
+static int
+grub_linux_setup_video (struct linux_kernel_params *params)
+{
+  struct grub_video_mode_info mode_info;
+  struct grub_video_render_target *render_target;
+  int ret;
+
+  ret = grub_video_get_info (&mode_info);
+  if (ret)
+    return 1;
+
+  ret = grub_video_get_active_render_target (&render_target);
+  if (ret)
+    return 1;
+
+  params->lfb_width = mode_info.width;
+  params->lfb_height = mode_info.height;
+  params->lfb_depth = mode_info.bpp;
+  params->lfb_line_len = mode_info.pitch;
+
+  params->lfb_base = (void *) render_target->data;
+  params->lfb_size = (params->lfb_line_len * params->lfb_height + 65535) >> 16;
+
+  params->red_mask_size = 8;
+  params->red_field_pos = 16;
+  params->green_mask_size = 8;
+  params->green_field_pos = 8;
+  params->blue_mask_size = 8;
+  params->blue_field_pos = 0;
+  params->reserved_mask_size = 8;
+  params->reserved_field_pos = 24;
+
+  return 0;
+}
+
 #ifdef __x86_64__
 struct
 {
@@ -231,6 +270,9 @@ grub_linux32_boot (void)
   
   params = real_mode_mem;
 
+  if (! grub_linux_setup_video (params))
+    params->have_vga = GRUB_VIDEO_TYPE_VLFB;
+  
   grub_dprintf ("linux", "code32_start = %x, idt_desc = %lx, gdt_desc = %lx\n",
 		(unsigned) params->code32_start,
                 (unsigned long) &(idt_desc.limit),
@@ -314,7 +356,6 @@ grub_rescue_cmd_linux (int argc, char *a
   grub_ssize_t len;
   int i;
   char *dest;
-  int video_type;
 
   grub_dl_ref (my_mod);
   
@@ -422,7 +463,6 @@ grub_rescue_cmd_linux (int argc, char *a
 
   /* Detect explicitly specified memory size, if any.  */
   linux_mem_size = 0;
-  video_type = 0;
   for (i = 1; i < argc; i++)
     if (grub_memcmp (argv[i], "mem=", 4) == 0)
       {
@@ -481,7 +521,8 @@ grub_rescue_cmd_linux (int argc, char *a
 
   if (grub_errno == GRUB_ERR_NONE)
     {
-      grub_loader_set (grub_linux32_boot, grub_linux_unload, 1);
+      grub_loader_set (grub_linux32_boot, grub_linux_unload,
+		       0 /* set noreturn=0 in order to avoid grub_console_fini() */);
       loaded = 1;
     }
 

Re: [PATCH] linux/gfxterm integration

[David Miller]

From: Robert Millan <rmh@aybabtu.com>
Date: Fri, 13 Mar 2009 20:44:47 +0100

> When we have more than one video backend, and a clear idea on which parts are
> generic and which are backend-specific, we could readjust.  For now this makes
> VBE happy.

I intend to write an ofvideo.c driver, so we'll have that
opportunity soon.

new Linux loader (Re: [PATCH] linux/gfxterm integration)

[Robert Millan]

On Fri, Mar 13, 2009 at 08:44:47PM +0100, Robert Millan wrote:
> 
> This new patch avoids the problem by accessing struct grub_video_render_target
> from its current location.
> 
> When we have more than one video backend, and a clear idea on which parts are
> generic and which are backend-specific, we could readjust.  For now this makes
> VBE happy.
> 
> Thoughts?

Ok, committed.

Btw, I'm happy to announce that the new Linux loader is fully usable on i386-pc
now, with no significant regressions.

I'd like to find a way to make it available to end users.  How would you think
of a rename?

  - loader/i386/pc/linux.c becomes the "legacy_linux" command or so.

  - loader/i386/linux.c becomes "linux" command.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: new Linux loader (Re: [PATCH] linux/gfxterm integration)

[phcoder]

IMO linux16 for pc/linux.c would better reflect the difference with 
normal 'linux' command.
Robert Millan wrote:
> On Fri, Mar 13, 2009 at 08:44:47PM +0100, Robert Millan wrote:
>> This new patch avoids the problem by accessing struct grub_video_render_target
>> from its current location.
>>
>> When we have more than one video backend, and a clear idea on which parts are
>> generic and which are backend-specific, we could readjust.  For now this makes
>> VBE happy.
>>
>> Thoughts?
> 
> Ok, committed.
> 
> Btw, I'm happy to announce that the new Linux loader is fully usable on i386-pc
> now, with no significant regressions.
> 
> I'd like to find a way to make it available to end users.  How would you think
> of a rename?
> 
>   - loader/i386/pc/linux.c becomes the "legacy_linux" command or so.
> 
>   - loader/i386/linux.c becomes "linux" command.
> 


-- 

Regards
Vladimir 'phcoder' Serbinenko

Re: new Linux loader (Re: [PATCH] linux/gfxterm integration)

[Robert Millan]

On Wed, Mar 18, 2009 at 02:09:11PM +0100, phcoder wrote:
> IMO linux16 for pc/linux.c would better reflect the difference with  
> normal 'linux' command.

I'm fine with that.  Does anyone object to the proposed rename?

Note this implies the new Linux loader becomes the default in grub-mkconfig
setup (it is already but only with coreboot).

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Robert Millan]

[-- Attachment #1: Type: text/plain, Size: 753 bytes --]

On Sat, Mar 21, 2009 at 06:50:56PM +0100, Robert Millan wrote:
> On Wed, Mar 18, 2009 at 02:09:11PM +0100, phcoder wrote:
> > IMO linux16 for pc/linux.c would better reflect the difference with  
> > normal 'linux' command.
> 
> I'm fine with that.  Does anyone object to the proposed rename?
> 
> Note this implies the new Linux loader becomes the default in grub-mkconfig
> setup (it is already but only with coreboot).

Here's a patch to rename `linux' to `linux16' and build the 32-bit loader as
simply `linux'.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

[-- Attachment #2: linux16.diff --]
[-- Type: text/x-diff, Size: 4550 bytes --]

2009-03-22  Robert Millan  <rmh@aybabtu.com>

	* conf/i386-pc.rmk (pkglib_MODULES): Add `linux16.mod'.
	(linux16_mod_SOURCES, linux16_mod_CFLAGS, linux16_mod_LDFLAGS): New
	variables.  Use 16-bit loader.
	(linux_mod_SOURCES, linux_mod_CFLAGS, linux_mod_LDFLAGS): Use 32-bit
	loader.
	* kern/i386/loader.S (grub_linux_boot): Rename to ...
	(grub_linux16_boot): ... this.  Update all users.
	* loader/i386/linux.c (grub_linux32_boot): Rename to ...
	(grub_linux_boot): ... this.  Update all users.

	* loader/i386/pc/linux.c (GRUB_MOD_INIT(linux)): Rename to ...
	(GRUB_MOD_INIT(linux16)): ... this.  Rename `linux' and `initrd'
	commands to `linux16' and `initrd16'.
	(GRUB_MOD_FINI(linux)): Rename to ...
	(GRUB_MOD_FINI(linux16)): ... this.

Index: conf/i386-pc.rmk
===================================================================
--- conf/i386-pc.rmk	(revision 2043)
+++ conf/i386-pc.rmk	(working copy)
@@ -167,7 +167,7 @@ grub_install_SOURCES = util/i386/pc/grub
 # For grub-mkrescue.
 grub_mkrescue_SOURCES = util/i386/pc/grub-mkrescue.in
 
-pkglib_MODULES = biosdisk.mod chain.mod linux.mod normal.mod \
+pkglib_MODULES = biosdisk.mod chain.mod normal.mod \
 	multiboot.mod reboot.mod halt.mod	\
 	vbe.mod vbetest.mod vbeinfo.mod play.mod serial.mod	\
 	ata.mod vga.mod memdisk.mod pci.mod lspci.mod	\
@@ -185,8 +185,13 @@ chain_mod_SOURCES = loader/i386/pc/chain
 chain_mod_CFLAGS = $(COMMON_CFLAGS)
 chain_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
-# For _linux.mod.
-linux_mod_SOURCES = loader/i386/pc/linux.c
+pkglib_MODULES += linux16.mod
+linux16_mod_SOURCES = loader/i386/pc/linux.c
+linux16_mod_CFLAGS = $(COMMON_CFLAGS)
+linux16_mod_LDFLAGS = $(COMMON_LDFLAGS)
+
+pkglib_MODULES += linux.mod
+linux_mod_SOURCES = loader/i386/linux.c
 linux_mod_CFLAGS = $(COMMON_CFLAGS)
 linux_mod_LDFLAGS = $(COMMON_LDFLAGS)
 
Index: kern/i386/loader.S
===================================================================
--- kern/i386/loader.S	(revision 2043)
+++ kern/i386/loader.S	(working copy)
@@ -59,7 +59,7 @@ VARIABLE(grub_linux_real_addr)
 VARIABLE(grub_linux_is_bzimage)
 	.long	0
 
-FUNCTION(grub_linux_boot)
+FUNCTION(grub_linux16_boot)
 	/* Must be done before zImage copy.  */
 	call	EXT_C(grub_dl_unload_all)
 
Index: include/grub/i386/loader.h
===================================================================
--- include/grub/i386/loader.h	(revision 2043)
+++ include/grub/i386/loader.h	(working copy)
@@ -30,7 +30,7 @@ extern grub_int32_t EXPORT_VAR(grub_linu
 extern grub_addr_t EXPORT_VAR(grub_os_area_addr);
 extern grub_size_t EXPORT_VAR(grub_os_area_size);
 
-grub_err_t EXPORT_FUNC(grub_linux_boot) (void);
+grub_err_t EXPORT_FUNC(grub_linux16_boot) (void);
 
 void EXPORT_FUNC(grub_unix_real_boot) (grub_addr_t entry, ...)
      __attribute__ ((cdecl,noreturn));
Index: loader/i386/linux.c
===================================================================
--- loader/i386/linux.c	(revision 2043)
+++ loader/i386/linux.c	(working copy)
@@ -263,7 +263,7 @@ struct
 #endif
 
 static grub_err_t
-grub_linux32_boot (void)
+grub_linux_boot (void)
 {
   struct linux_kernel_params *params;
   int e820_num;
@@ -522,7 +522,7 @@ grub_cmd_linux (grub_command_t cmd __att
 
   if (grub_errno == GRUB_ERR_NONE)
     {
-      grub_loader_set (grub_linux32_boot, grub_linux_unload,
+      grub_loader_set (grub_linux_boot, grub_linux_unload,
 		       0 /* set noreturn=0 in order to avoid grub_console_fini() */);
       loaded = 1;
     }
Index: loader/i386/pc/linux.c
===================================================================
--- loader/i386/pc/linux.c	(revision 2043)
+++ loader/i386/pc/linux.c	(working copy)
@@ -269,7 +269,7 @@ grub_cmd_linux (grub_command_t cmd __att
   if (grub_errno == GRUB_ERR_NONE)
     {
       grub_linux_prot_size = prot_size;
-      grub_loader_set (grub_linux_boot, grub_linux_unload, 1);
+      grub_loader_set (grub_linux16_boot, grub_linux_unload, 1);
       loaded = 1;
     }
 
@@ -378,18 +378,18 @@ grub_cmd_initrd (grub_command_t cmd __at
 
 static grub_command_t cmd_linux, cmd_initrd;
 
-GRUB_MOD_INIT(linux)
+GRUB_MOD_INIT(linux16)
 {
   cmd_linux =
-    grub_register_command ("linux", grub_cmd_linux,
+    grub_register_command ("linux16", grub_cmd_linux,
 			   0, "load linux");
   cmd_initrd =
-    grub_register_command ("initrd", grub_cmd_initrd,
+    grub_register_command ("initrd16", grub_cmd_initrd,
 			   0, "load initrd");
   my_mod = mod;
 }
 
-GRUB_MOD_FINI(linux)
+GRUB_MOD_FINI(linux16)
 {
   grub_unregister_command (cmd_linux);
   grub_unregister_command (cmd_initrd);

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Robert Millan]

On Sun, Mar 22, 2009 at 05:00:32PM +0100, Robert Millan wrote:
> On Sat, Mar 21, 2009 at 06:50:56PM +0100, Robert Millan wrote:
> > On Wed, Mar 18, 2009 at 02:09:11PM +0100, phcoder wrote:
> > > IMO linux16 for pc/linux.c would better reflect the difference with  
> > > normal 'linux' command.
> > 
> > I'm fine with that.  Does anyone object to the proposed rename?
> > 
> > Note this implies the new Linux loader becomes the default in grub-mkconfig
> > setup (it is already but only with coreboot).
> 
> Here's a patch to rename `linux' to `linux16' and build the 32-bit loader as
> simply `linux'.

Committed.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Pavel Roskin]

On Sat, 2009-03-28 at 13:53 +0100, Robert Millan wrote:

> > Here's a patch to rename `linux' to `linux16' and build the 32-bit loader as
> > simply `linux'.
> 
> Committed.

Just for your information.  With the old loader, booting an x86_64
kernel in qemu emulating i386 would result in a kernel message:

"This kernel requires an x86-64 CPU but only detected an i686 CPU
Unable to boot - please use a kernel appropriate for your CPU"

With the new loader, the kernel hands without showing any messages.  It
can be a kernel bug.  Linux 2.6.29, current GRUB.

-- 
Regards,
Pavel Roskin

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[phcoder]

Pavel Roskin wrote:
> On Sat, 2009-03-28 at 13:53 +0100, Robert Millan wrote:
> 
>>> Here's a patch to rename `linux' to `linux16' and build the 32-bit loader as
>>> simply `linux'.
>> Committed.
> 
> Just for your information.  With the old loader, booting an x86_64
> kernel in qemu emulating i386 would result in a kernel message:
> 
> "This kernel requires an x86-64 CPU but only detected an i686 CPU
> Unable to boot - please use a kernel appropriate for your CPU"
> 
> With the new loader, the kernel hands without showing any messages.  It
> can be a kernel bug.  Linux 2.6.29, current GRUB.
> 
I confirm. I suppose that this check and message is bypassed with 32-bit 
loading mode. IMO grub2 should provide an equivlent of this check. We 
already have cpuid code. Does anyone know how to determine if kernel is 
i386 or amd64?

-- 

Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Yoshinori K. Okuji]

On Monday 30 March 2009 23:35:52 phcoder wrote:
> Pavel Roskin wrote:
> > On Sat, 2009-03-28 at 13:53 +0100, Robert Millan wrote:
> >>> Here's a patch to rename `linux' to `linux16' and build the 32-bit
> >>> loader as simply `linux'.
> >>
> >> Committed.
> >
> > Just for your information.  With the old loader, booting an x86_64
> > kernel in qemu emulating i386 would result in a kernel message:
> >
> > "This kernel requires an x86-64 CPU but only detected an i686 CPU
> > Unable to boot - please use a kernel appropriate for your CPU"
> >
> > With the new loader, the kernel hands without showing any messages.  It
> > can be a kernel bug.  Linux 2.6.29, current GRUB.
>
> I confirm. I suppose that this check and message is bypassed with 32-bit
> loading mode. IMO grub2 should provide an equivlent of this check. We
> already have cpuid code. Does anyone know how to determine if kernel is
> i386 or amd64?

I don't know any reliable way. Some candidates:

- The ramdisk max value. On 32-bit, initrd may not be loaded onto over 2GB. 
This is hard to change in Linux, so we can expect that this will not change. 
On 64-bit, currently, the max is 4GB-1. It is likely that this value will not 
change, but who knows.

- The long mode panic message. This exists only for x86_64. But the message 
might change some day.

- Otherwise, we could probe some opcodes and see if 64-bit opcodes are used. 
This would be error-prone.

After all, the best way might be to just ask Linux developers to add a new 
flag.

Regards,
Okuji

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Pavel Roskin]

On Tue, 2009-03-31 at 01:06 +0900, Yoshinori K. Okuji wrote:
> On Monday 30 March 2009 23:35:52 phcoder wrote:
> > I confirm. I suppose that this check and message is bypassed with 32-bit
> > loading mode. IMO grub2 should provide an equivlent of this check. We
> > already have cpuid code. Does anyone know how to determine if kernel is
> > i386 or amd64?
> 
> I don't know any reliable way. Some candidates:
> 
> - The ramdisk max value. On 32-bit, initrd may not be loaded onto over 2GB. 
> This is hard to change in Linux, so we can expect that this will not change. 

If we are circumventing the standard Linux bootloader, perhaps we should
communicate this to the Linux developers.

To check architecture in GRUB, we need one bit of information, but that
bit should be reliable.

Also, it looks like Linux on x86_64 does an extra validation of the CPU
in arch/x86/boot/compressed/head_64.S, but if the long mode is not
supported, it hangs without printing an error message.

-- 
Regards,
Pavel Roskin

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Robert Millan]

On Mon, Mar 30, 2009 at 05:51:22PM -0400, Pavel Roskin wrote:
> On Tue, 2009-03-31 at 01:06 +0900, Yoshinori K. Okuji wrote:
> > On Monday 30 March 2009 23:35:52 phcoder wrote:
> > > I confirm. I suppose that this check and message is bypassed with 32-bit
> > > loading mode. IMO grub2 should provide an equivlent of this check. We
> > > already have cpuid code. Does anyone know how to determine if kernel is
> > > i386 or amd64?
> > 
> > I don't know any reliable way. Some candidates:
> > 
> > - The ramdisk max value. On 32-bit, initrd may not be loaded onto over 2GB. 
> > This is hard to change in Linux, so we can expect that this will not change. 
> 
> If we are circumventing the standard Linux bootloader, perhaps we should
> communicate this to the Linux developers.

This is not circumvention. We're using a 32-bit interface that's part of their
boot protocol specification (i.e. they promised not to break it).  The only
caveat is that so far it's only used on EFI and on coreboot, it hasn't been
so widespread, and therefore not so widely tested yet.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Robert Millan]

On Tue, Mar 31, 2009 at 01:06:25AM +0900, Yoshinori K. Okuji wrote:
> I don't know any reliable way. Some candidates:
> 
> - The ramdisk max value. On 32-bit, initrd may not be loaded onto over 2GB. 
> This is hard to change in Linux, so we can expect that this will not change. 
> On 64-bit, currently, the max is 4GB-1. It is likely that this value will not 
> change, but who knows.

This could be possible, but doesn't sound very reliable.

> - The long mode panic message. This exists only for x86_64. But the message 
> might change some day.

Actually, it seems to have changed recently.  Doesn't seem too reliable
either.

> - Otherwise, we could probe some opcodes and see if 64-bit opcodes are used. 
> This would be error-prone.

Same here :-(

However, this is not as large a problem as it seems.  Most users install GRUB
to disk through their distribution, which already provides the right Linux
builds in /boot.  It could be a problem with removable media, but:

  - It's easy to solve this when you have a "cpuid" command and know what
    each of the files you put in the CD is.  This also allows for a more
    user-friendly error message than a Linux panic.

  - GRUB is rarely used there anyway (most GNU/Linux distros opt for
    isolinux)

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Pavel Roskin]

On Wed, 2009-04-01 at 15:23 +0200, Robert Millan wrote:
> On Mon, Mar 30, 2009 at 05:51:22PM -0400, Pavel Roskin wrote:
> > 
> > If we are circumventing the standard Linux bootloader, perhaps we should
> > communicate this to the Linux developers.
> 
> This is not circumvention. We're using a 32-bit interface that's part of their
> boot protocol specification (i.e. they promised not to break it).  The only
> caveat is that so far it's only used on EFI and on coreboot, it hasn't been
> so widespread, and therefore not so widely tested yet.

I see.  It looks like the x86_64 kernel has code for printing strings in
16-bit mode and in 64-bit mode, but not in 32-bit mode, in which we
enter the kernel.  So no easy fix, unfortunately.

-- 
Regards,
Pavel Roskin

Re: [PATCH] build 32-bit Linux loader as `linux', rename legacy loader to `linux16'

[Robert Millan]

On Wed, Apr 01, 2009 at 10:04:44AM -0400, Pavel Roskin wrote:
> On Wed, 2009-04-01 at 15:23 +0200, Robert Millan wrote:
> > On Mon, Mar 30, 2009 at 05:51:22PM -0400, Pavel Roskin wrote:
> > > 
> > > If we are circumventing the standard Linux bootloader, perhaps we should
> > > communicate this to the Linux developers.
> > 
> > This is not circumvention. We're using a 32-bit interface that's part of their
> > boot protocol specification (i.e. they promised not to break it).  The only
> > caveat is that so far it's only used on EFI and on coreboot, it hasn't been
> > so widespread, and therefore not so widely tested yet.
> 
> I see.  It looks like the x86_64 kernel has code for printing strings in
> 16-bit mode and in 64-bit mode, but not in 32-bit mode, in which we
> enter the kernel.  So no easy fix, unfortunately.

It's possible to find a fix for this from Linux side, but IMHO the best
long-term fix would be to make whoever installed Linux _and_ GRUB figure
out what GRUB should do.  Not too much to ask, since we give them the tools
(cpuid command) to do it.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."