Re: [PATCH] Drivemap module

[Marco Gerards]

Javier Martín <address@hidden> writes:

> In this reply-to-myself hoping to keep the thread continuity, I put
> forth the new "version 7" of the patch with the following changes:
>
>   - A new switch -s/--swap has been implemented, so that running
> "drivemap -s hd0 hd1" is equivalent to issuing the command twice with
> the arguments normal, then reversed. There is one exception: if the
> first mapping fails or the second drive does not exist, no mapping is
> performed, whereas hand-performing the swap would have successfully
> assigned BIOS disk #1 to hd0, then failed to assign bd#0 to non-existent
> hd1.
>   - Raw BIOS disk number parsing has been removed: the syntax "drivemap
> hd1 0x80" is no longer legal. However, one can still map non-existent
> BIOS disk numbers with "drivemap hd0 hd42" for example and (more
> controversially, maybe I'll add a check eventually) assign a floppy to
> an HD and back.

Ah good :-)

> The only file changed from the last patch ("version 6") is drivemap.c:
> the rest of it should be the same, so if anyone was reviewing it you can
> seamlessly "jump" to version 7. In particular, the functional changes
> are localized in the drivemap_cmd function proper, and there are
> cosmetic changes elsewhere (spurious tabs removed, etc.).

You only forgot the changelog entry ;-)

> -Habbit
>
> Index: commands/i386/pc/drivemap.c
> ===================================================================
> --- commands/i386/pc/drivemap.c       (revisión: 0)
> +++ commands/i386/pc/drivemap.c       (revisión: 0)
> @@ -0,0 +1,433 @@
> +/* drivemap.c - command to manage the BIOS drive mappings.  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */
> +
> +#include <grub/machine/drivemap.h>
> +#include <grub/normal.h>
> +#include <grub/dl.h>
> +#include <grub/mm.h>
> +#include <grub/misc.h>
> +#include <grub/disk.h>
> +#include <grub/loader.h>
> +#include <grub/machine/loader.h>
> +#include <grub/machine/biosdisk.h>
> +
> +#define MODNAME "drivemap"
> +
> +static const struct grub_arg_option options[] = {
> +  {"list", 'l', 0, "show the current mappings", 0, 0},
> +  {"reset", 'r', 0, "reset all mappings to the default values", 0, 0},
> +  {"swap", 's', 0, "perform both direct and reverse mappings", 0, 0},
> +  {0, 0, 0, 0, 0, 0}
> +};
> +
> +enum opt_idxs {
> +  OPTIDX_LIST = 0,
> +  OPTIDX_RESET,
> +  OPTIDX_SWAP,
> +};
> +
> +typedef struct drivemap_node
> +{
> +  grub_uint8_t newdrive;
> +  grub_uint8_t redirto;
> +  struct drivemap_node *next;
> +} drivemap_node_t;
> +
> +static drivemap_node_t *drivemap;
> +static grub_preboot_hookid insthandler_hook;
> +static grub_err_t install_int13_handler (void);
> +
> +/* Puts the specified mapping into the table, replacing an existing mapping
> +   for newdrive or adding a new one if required.  */
> +static grub_err_t
> +drivemap_set (grub_uint8_t newdrive, grub_uint8_t redirto)
> +{
> +  drivemap_node_t *mapping = 0;
> +  drivemap_node_t *search = drivemap;
> +  while (search)
> +    {
> +      if (search->newdrive == newdrive)
> +        {
> +          mapping = search;
> +          break;
> +        }
> +      search = search->next;
> +    }
> +
> +  
> +  /* Check for pre-existing mappings to modify before creating a new one.  */
> +  if (mapping)
> +    mapping->redirto = redirto;
> +  else 
> +    {
> +      mapping = grub_malloc (sizeof (drivemap_node_t));
> +      if (!mapping)
> +        return grub_error (GRUB_ERR_OUT_OF_MEMORY,
> +                           "cannot allocate map entry, not enough memory");
> +      mapping->newdrive = newdrive;
> +      mapping->redirto = redirto;
> +      mapping->next = drivemap;
> +      drivemap = mapping;
> +    }
> +  return GRUB_ERR_NONE;
> +}
> +
> +/* Removes the mapping for newdrive from the table.  If there is no mapping,
> +   then this function behaves like a no-op on the map.  */
> +static void
> +drivemap_remove (grub_uint8_t newdrive)
> +{
> +  drivemap_node_t *mapping = 0;
> +  drivemap_node_t *search = drivemap;
> +  drivemap_node_t *previous = 0;
> +
> +  while (search)
> +    {
> +      if (search->newdrive == newdrive)
> +        {
> +          mapping = search;
> +          break;
> +        }
> +      previous = search;
> +      search = search->next;
> +    }
> +
> +  if (mapping)
> +    {
> +      if (previous)
> +        previous->next = mapping->next;
> +      else /* Entry was head of list.  */
> +        drivemap = mapping->next;
> +      grub_free (mapping);
> +    }
> +}
> +
> +/* Given a device name, resolves its BIOS disk number and stores it in the
> +   passed location, which should only be trusted if ERR_NONE is returned.  */
> +static grub_err_t
> +parse_biosdisk (const char *name, grub_uint8_t *disknum)
> +{
> +  grub_disk_t disk;
> +  if (!name || *name == 0)
> +    return grub_error (GRUB_ERR_BAD_ARGUMENT, "device name empty");
> +  /* Skip the first ( in (hd0) - disk_open wants just the name.  */
> +  if (*name == '(')
> +    name++;
> +  
> +  disk = grub_disk_open (name);
> +  if (!disk)
> +    return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "unknown device \"%s\"", 
> name);
> +  else
> +    {
> +      const enum grub_disk_dev_id id = disk->dev->id;
> +      /* The following assignment is only sound if the device is indeed a
> +         biosdisk.  The caller must check the return value.  */
> +      if (disknum)
> +        *disknum = disk->id;
> +      grub_disk_close (disk);
> +      if (id == GRUB_DISK_DEVICE_BIOSDISK_ID)
> +        return GRUB_ERR_NONE;
> +      else return grub_error (GRUB_ERR_BAD_DEVICE, "%s is not a BIOS disk", 
> name);

Please put the return on a separate line.

> +    }
> +}
> +
> +/* Given a BIOS disk number, returns its GRUB device name if it exists.
> +   For nonexisting BIOS disk numbers, this function returns
> +   GRUB_ERR_UNKNOWN_DEVICE.  */
> +static grub_err_t
> +revparse_biosdisk(const grub_uint8_t dnum, const char **output)
> +{
> +  int found = 0;
> +  auto int find (const char *name);
> +  int find (const char *name)
> +  {
> +    const grub_disk_t disk = grub_disk_open (name);
> +    if (!disk)
> +      return 0;
> +    else
> +      {
> +        if (disk->id == dnum && disk->dev->id == 
> GRUB_DISK_DEVICE_BIOSDISK_ID)
> +          {
> +            found = 1;
> +            if (output)
> +              *output = name;
> +          }
> +        grub_disk_close (disk);
> +        return found;
> +      }
> +  }
> +
> +  grub_disk_dev_iterate (find);
> +  if (found)
> +    return GRUB_ERR_NONE;
> +  else
> +    return grub_error (GRUB_ERR_UNKNOWN_DEVICE, "BIOS disk %02x not found", 
> dnum);
> +}
> +
> +/* Given a GRUB-like device name and a convenient location, stores the 
> related
> +   BIOS disk number.  Accepts devices like \((f|h)dN\), with 0 <= N < 128.  
> */
> +static grub_err_t
> +tryparse_diskstring (const char *str, grub_uint8_t *output)
> +{
> +  if (!str || *str == 0)
> +    goto fail;
> +  /* Skip opening paren in order to allow both (hd0) and hd0.  */
> +  if (*str == '(')
> +    str++;
> +  if ((str[0] == 'f' || str[0] == 'h') && str[1] == 'd')
> +    {
> +      grub_uint8_t bios_num = (str[0] == 'h')? 0x80 : 0x00;
> +      grub_errno = GRUB_ERR_NONE;
> +      unsigned long drivenum = grub_strtoul (str + 2, 0, 0);
> +      if (grub_errno != GRUB_ERR_NONE || drivenum > 127)
> +        {
> +          /* N not a number or out of range.  */
> +          goto fail;
> +        }
> +      else
> +        {
> +          bios_num |= drivenum;
> +          if (output)
> +            *output = bios_num;
> +          return GRUB_ERR_NONE;
> +        }
> +    }
> +  else
> +    goto fail;
> +
> +fail:
> +  return grub_error (GRUB_ERR_BAD_ARGUMENT, "device format \"%s\" invalid: 
> must"
> +                     "be (f|h)dN, with 0 <= N < 128", str);
> +}
> +
> +static grub_err_t
> +grub_cmd_drivemap (struct grub_arg_list *state, int argc, char **args)
> +{
> +  if (state[OPTIDX_LIST].set)
> +    {
> +      /* Show: list mappings.  */
> +      if (!drivemap)
> +        grub_printf ("No drives have been remapped");
> +      else
> +        {
> +          grub_printf ("Showing only remapped drives.\n");
> +          grub_printf ("BIOS disk #num ----> GRUB device\n");

BIOS disk #num?

Can you give an example?

> +          drivemap_node_t *curnode = drivemap;
> +          while (curnode)
> +            {
> +              const char *dname = 0;
> +              grub_err_t err = revparse_biosdisk (curnode->redirto, &dname);
> +              if (err != GRUB_ERR_NONE)
> +                return grub_error (err, "invalid mapping: non-existent disk"
> +                                        "or not managed by the BIOS");
> +              grub_printf("%cD #%-3u (0x%02x)       %s\n",
> +                          (curnode->newdrive & 0x80) ? 'H' : 'F',
> +                          curnode->newdrive & 0x7F, curnode->newdrive, 
> dname);
> +              curnode = curnode->next;
> +            }
> +        }
> +    }
> +  else if (state[OPTIDX_RESET].set)
> +    {
> +      /* Reset: just delete all mappings, freeing their memory.  */
> +      drivemap_node_t *curnode = drivemap;
> +      drivemap_node_t *prevnode = 0;
> +      while (curnode)
> +        {
> +          prevnode = curnode;
> +          curnode = curnode->next;
> +          grub_free (prevnode);
> +        }
> +      drivemap = 0;
> +    }
> +  else
> +    {
> +      /* Neither flag: put mapping.  */
> +      grub_uint8_t mapfrom = 0;
> +      grub_uint8_t mapto = 0xFF;
> +      grub_err_t err;
> +      
> +      if (argc != 2)
> +        return grub_error (GRUB_ERR_BAD_ARGUMENT, "two arguments required");
> +
> +      err = parse_biosdisk (args[0], &mapfrom);
> +      if (err != GRUB_ERR_NONE)
> +        return err;
> +      
> +      /* When swapping we require both devices to be BIOS disks, but when
> +         performing direct mappings we only require the 2nd argument to look
> +         like a BIOS disk in order to resolve it into a BIOS disk number.  */
> +      if (state[OPTIDX_SWAP].set)
> +        err = parse_biosdisk (args[1], &mapto);
> +      else
> +        err = tryparse_diskstring (args[1], &mapto);
> +      if (err != GRUB_ERR_NONE)
> +        return err;
> +      
> +      if (mapto == mapfrom)
> +        {
> +          /* Reset to default.  */
> +          grub_dprintf (MODNAME, "Removing the mapping for %s (%02x)", 
> args[0], mapfrom);
> +          drivemap_remove (mapfrom);
> +        }
> +      else
> +        {
> +          /* Set the mapping for the disk (overwrites any existing mapping). 
>  */
> +          grub_dprintf (MODNAME, "%s %s (%02x) = %s (%02x)\n",
> +                        state[OPTIDX_SWAP].set ? "Swapping" : "Mapping",
> +                        args[1], mapto, args[0], mapfrom);
> +          err = drivemap_set (mapto, mapfrom);
> +          /* If -s, perform the reverse mapping too (only if the first was 
> OK).  */
> +          if (state[OPTIDX_SWAP].set && err == GRUB_ERR_NONE)
> +            err = drivemap_set (mapfrom, mapto);
> +          return err;
> +        }
> +    }
> +
> +  return GRUB_ERR_NONE;
> +}
> +
> +typedef struct __attribute__ ((packed)) int13map_node
> +{
> +  grub_uint8_t disknum;
> +  grub_uint8_t mapto;
> +} int13map_node_t;
> +
> +/* The min amount of mem that must remain free after installing the handler.
> +   32 KiB is just above 0x7C00-0x7E00, where the bootsector is loaded.  */
> +#define MIN_FREE_MEM_KB 32
> +#define INT13H_OFFSET(x) ( ((grub_uint8_t*)(x)) - 
> ((grub_uint8_t*)&grub_drivemap_int13_handler_base) )
> +#define INT13H_REBASE(x) ( (void*) (((grub_uint8_t*)handler_base) + (x)) )
> +#define INT13H_TONEWADDR(x) INT13H_REBASE( INT13H_OFFSET( x ) )
> +
> +/* Int13h handler installer - reserves conventional memory for the handler,
> +   copies it over and sets the IVT entry for int13h.  
> +   This code rests on the assumption that GRUB does not activate any kind of
> +   memory mapping apart from identity paging, since it accesses realmode
> +   structures by their absolute addresses, like the IVT at 0 or the BDA at
> +   0x400; and transforms a pmode pointer into a rmode seg:off far ptr.  */
> +static grub_err_t
> +install_int13_handler (void)
> +{
> +  grub_size_t entries = 0;
> +  drivemap_node_t *curentry = drivemap;
> +  /* Count entries to prepare a contiguous map block.  */
> +  while (curentry)
> +    {
> +      entries++;
> +      curentry = curentry->next;
> +    }
> +  if (entries == 0)
> +    {
> +      /* No need to install the int13h handler.  */
> +      grub_dprintf (MODNAME, "No drives marked as remapped, installation of"
> +                    "an int13h handler is not required.");
> +      return GRUB_ERR_NONE;
> +    }
> +  else
> +    {
> +      /* Real mode IVT slot (seg:off far pointer) for interrupt 0x13.  */
> +      grub_uint32_t *ivtslot = (grub_uint32_t*)0x0000004c;
> +      /* BDA offset 0x13 contains the top of conventional memory, in kiB.  */
> +      grub_uint16_t *bpa_freekb = (grub_uint16_t*)0x00000413;
> +      /* Size of the full int13 handler "bundle", including code and map.  */
> +      grub_size_t total_size;
> +      /* The former, ceil-rounded to KiB.  */
> +      grub_uint16_t payload_sizekb;
> +      /* Base address of the space reserved for the handler bundle.  */
> +      grub_uint8_t *handler_base;
> +      /* Address of the map within the deployed bundle.  */
> +      int13map_node_t *handler_map;
> +      /* Real mode IVT entry (seg:off far pointer) for the new handler.  */
> +      grub_uint32_t ivtentry;
> +
> +      grub_dprintf (MODNAME, "Installing int13h handler...\n");
> +      
> +      /* Save the pointer to the old handler.  */
> +      grub_drivemap_int13_oldhandler = *ivtslot;
> +      grub_dprintf (MODNAME, "Old int13 handler at %04x:%04x\n",
> +                    (grub_drivemap_int13_oldhandler >> 16) & 0x0ffff,
> +                    grub_drivemap_int13_oldhandler & 0x0ffff);
> +
> +      /* Reserve a section of conventional memory as "BIOS memory" for 
> handler.  */
> +      grub_dprintf (MODNAME, "Top of conventional memory: %u KiB\n", 
> *bpa_freekb);
> +      total_size = grub_drivemap_int13_size
> +                   + (entries + 1) * sizeof(int13map_node_t);
> +      payload_sizekb = (total_size >> 10) +
> +                       (((total_size % 1024) == 0) ? 0 : 1);
> +      if ((*bpa_freekb - payload_sizekb) < MIN_FREE_MEM_KB)
> +        return grub_error (GRUB_ERR_OUT_OF_MEMORY, "refusing to install"
> +                           "int13 handler, not enough free memory after");
> +      grub_dprintf (MODNAME, "Payload is %u b long, reserving %u Kb\n",
> +                    total_size, payload_sizekb);
> +      *bpa_freekb -= payload_sizekb;
> +
> +      /* Copy int13h handler bundle to reserved area.  */
> +      handler_base = (grub_uint8_t*)(*bpa_freekb << 10);
> +      grub_dprintf (MODNAME, "Copying int13 handler to: %p\n", handler_base);
> +      grub_memcpy (handler_base, &grub_drivemap_int13_handler_base,
> +                   grub_drivemap_int13_size);
> +
> +      /* Copy the mappings to the reserved area.  */
> +      curentry = drivemap;
> +      grub_size_t i;
> +      handler_map = (int13map_node_t*)
> +                    INT13H_TONEWADDR (&grub_drivemap_int13_mapstart);
> +      grub_dprintf (MODNAME, "Target map at %p, copying mappings...\n", 
> handler_map);
> +      for (i = 0; i < entries && curentry; i++, curentry = curentry->next)
> +        {
> +          handler_map[i].disknum = curentry->newdrive;
> +          handler_map[i].mapto = curentry->redirto;
> +          grub_dprintf (MODNAME, "\t#%d: 0x%02x <- 0x%02x\n", i,
> +                        handler_map[i].disknum, handler_map[i].mapto);
> +        }
> +      /* Signal end-of-map.  */
> +      handler_map[i].disknum = 0;
> +      handler_map[i].mapto = 0;
> +      grub_dprintf (MODNAME, "\t#%d: 0x%02x <- 0x%02x (end)\n", i,
> +                    handler_map[i].disknum, handler_map[i].mapto);
> +
> +      /* Install our function as the int13h handler in the IVT.  */
> +      ivtentry = ((grub_uint32_t)handler_base) << 12; /* Segment address.  */
> +      ivtentry |= (grub_uint16_t) 
> INT13H_OFFSET(&grub_drivemap_int13_handler);
> +      grub_dprintf (MODNAME, "New int13 handler IVT pointer: %04x:%04x\n",
> +                    (ivtentry >> 16) & 0x0ffff, ivtentry & 0x0ffff);
> +      *ivtslot = ivtentry;
> +      
> +      return GRUB_ERR_NONE;
> +    }
> +}
> +
> +GRUB_MOD_INIT (drivemap)
> +{
> +  (void) mod;                        /* Stop warning.  */
> +  grub_register_command (MODNAME, grub_cmd_drivemap,
> +                         GRUB_COMMAND_FLAG_BOTH,
> +                         MODNAME " -l | -r | [-s] grubdev biosdisk",
> +                         "Manage the BIOS drive mappings", options);
> +  insthandler_hook = grub_loader_register_preboot (&install_int13_handler, 
> 1);
> +}
> +
> +GRUB_MOD_FINI (drivemap)
> +{
> +  grub_loader_unregister_preboot (insthandler_hook);
> +  insthandler_hook = 0;
> +  grub_unregister_command (MODNAME);
> +}
> +
> Index: commands/i386/pc/drivemap_int13h.S
> ===================================================================
> --- commands/i386/pc/drivemap_int13h.S        (revisión: 0)
> +++ commands/i386/pc/drivemap_int13h.S        (revisión: 0)
> @@ -0,0 +1,121 @@
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 1999,2000,2001,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/>.
> + */
> +
> +
> +/*
> + * Note: These functions defined in this file may be called from C.
> + *       Be careful of that you must not modify some registers. Quote
> + *       from gcc-2.95.2/gcc/config/i386/i386.h:
> +
> +   1 for registers not available across function calls.
> +   These must include the FIXED_REGISTERS and also any
> +   registers that can be used without being saved.
> +   The latter must include the registers where values are returned
> +   and the register where structure-value addresses are passed.
> +   Aside from that, you can include as many other registers as you like.
> +
> +  ax,dx,cx,bx,si,di,bp,sp,st,st1,st2,st3,st4,st5,st6,st7,arg
> +{  1, 1, 1, 0, 0, 0, 0, 1, 1,  1,  1,  1,  1,  1,  1,  1,  1 }
> + */
> +
> +/*
> + * Note: GRUB is compiled with the options -mrtd and -mregparm=3.
> + *       So the first three arguments are passed in %eax, %edx, and %ecx,
> + *       respectively, and if a function has a fixed number of arguments
> + *       and the number if greater than three, the function must return
> + *       with "ret $N" where N is ((the number of arguments) - 3) * 4.
> + */
> +
> +#include <grub/symbol.h>
> +
> +#define GRUB_DRIVEMAP_INT13H_OFFSET(x) ((x) - 
> grub_drivemap_int13_handler_base)
> +
> +/* Copy starts here.  When deployed, this label must be segment-aligned.  */
> +VARIABLE(grub_drivemap_int13_handler_base)
> +
> +/* Far pointer to the old handler.  Stored as a CS:IP in the style of 
> real-mode
> +   IVT entries (thus PI:SC in mem).  */
> +VARIABLE(grub_drivemap_int13_oldhandler)
> +  .word 0xdead, 0xbeef
> +
> +/* Drivemap module bundle - INT 13h handler - BIOS HD map */
> +/* We need to use relative addressing, and with CS to top it all, since we
> +   must make as few changes to the registers as possible.  IP-relative
> +   addressing like on amd64 would make life way easier here. */
> +.code16
> +FUNCTION(grub_drivemap_int13_handler)
> +  push %bp
> +  mov %sp, %bp
> +  push %ax  /* We'll need it later to determine the used BIOS function.  */
> +
> +  /* Map the drive number (always in DL?).  */
> +  push %ax
> +  push %bx
> +  push %si
> +  mov $GRUB_DRIVEMAP_INT13H_OFFSET(grub_drivemap_int13_mapstart), %bx
> +  xor %si, %si
> +1:movw %cs:(%bx,%si), %ax
> +  cmp %ah, %al
> +  jz 3f /* DRV=DST => map end - drive not remapped, leave DL as-is.  */
> +  cmp %dl, %al
> +  jz 2f /* Found - drive remapped, modify DL.  */
> +  add $2, %si
> +  jmp 1b /* Not found, but more remaining, loop.  */
> +2:mov %ah, %dl
> +3:pop %si
> +  pop %bx
> +  xchgw %ax, -4(%bp) /* Recover the old AX and save the map entry for later. 
>  */
> +  
> +  push %bp
> +  /* Simulate interrupt call: push flags and do a far call in order to set
> +     the stack the way the old handler expects it so that its iret works.  */
> +  push 6(%bp)
> +  movw (%bp), %bp  /* Restore the caller BP (is this needed and/or 
> sensible?).  */
> +  lcall *%cs:GRUB_DRIVEMAP_INT13H_OFFSET(grub_drivemap_int13_oldhandler)
> +  pop %bp /* The pushed flags were removed by iret.  */
> +  /* Set the saved flags to what the int13h handler returned.  */
> +  push %ax
> +  pushf
> +  pop %ax
> +  movw %ax, 6(%bp)
> +  pop %ax
> +
> +  /* Reverse map any returned drive number if the data returned includes it. 
>  
> +     The only func that does this seems to be origAH = 0x08, but many BIOS
> +     refs say retDL = # of drives connected.  However, the GRUB Legacy code
> +     treats this as the _drive number_ and "undoes" the remapping.  Thus,
> +     this section has been disabled for testing if it's required.  */
> +#  cmpb $0x08, -1(%bp) /* Caller's AH.  */
> +#  jne 4f
> +#  xchgw %ax, -4(%bp) /* Map entry used.  */
> +#  cmp %ah, %al  /* DRV=DST => drive not remapped.  */
> +#  je 4f
> +#  mov %ah, %dl  /* Undo remap.  */
> +
> +4:mov %bp, %sp
> +  pop %bp
> +  iret
> +/* This label MUST be at the end of the copied block, since the installer 
> code
> +   reserves additional space for mappings at runtime and copies them over 
> it.  */
> +.align 2
> +VARIABLE(grub_drivemap_int13_mapstart)
> +/* Copy stops here.  */
> +.code32
> +VARIABLE(grub_drivemap_int13_size)
> +  .word GRUB_DRIVEMAP_INT13H_OFFSET(grub_drivemap_int13_size)
> +
> Index: conf/i386-pc.rmk
> ===================================================================
> --- conf/i386-pc.rmk  (revisión: 1798)
> +++ conf/i386-pc.rmk  (copia de trabajo)
> @@ -163,7 +163,7 @@
>       vbe.mod vbetest.mod vbeinfo.mod video.mod gfxterm.mod \
>       videotest.mod play.mod bitmap.mod tga.mod cpuid.mod serial.mod  \
>       ata.mod vga.mod memdisk.mod jpeg.mod png.mod pci.mod lspci.mod \
> -     aout.mod _bsd.mod bsd.mod pxe.mod pxecmd.mod
> +     aout.mod _bsd.mod bsd.mod pxe.mod pxecmd.mod drivemap.mod
>  
>  # For biosdisk.mod.
>  biosdisk_mod_SOURCES = disk/i386/pc/biosdisk.c
> @@ -340,4 +340,12 @@
>  pxecmd_mod_CFLAGS = $(COMMON_CFLAGS)
>  pxecmd_mod_LDFLAGS = $(COMMON_LDFLAGS)
>  
> +# For drivemap.mod.
> +drivemap_mod_HEADERS = machine/drivemap.h
> +drivemap_mod_SOURCES = commands/i386/pc/drivemap.c \
> +                       commands/i386/pc/drivemap_int13h.S
> +drivemap_mod_ASFLAGS = $(COMMON_ASFLAGS)
> +drivemap_mod_CFLAGS = $(COMMON_CFLAGS)
> +drivemap_mod_LDFLAGS = $(COMMON_LDFLAGS)
> +
>  include $(srcdir)/conf/common.mk
> Index: include/grub/loader.h
> ===================================================================
> --- include/grub/loader.h     (revisión: 1798)
> +++ include/grub/loader.h     (copia de trabajo)
> @@ -37,7 +37,23 @@
>  /* Unset current loader, if any.  */
>  void EXPORT_FUNC(grub_loader_unset) (void);
>  
> -/* Call the boot hook in current loader. This may or may not return,
> +typedef struct hooklist_node *grub_preboot_hookid;
> +
> +/* Register a function to be called before the loader "boot" function.  
> Returns
> +   an id that can be later used to unregister the preboot (i.e. on module
> +   unload).  If ABORT_ON_ERROR is set, the boot sequence will abort if any of
> +   the registered functions return anything else than GRUB_ERR_NONE.
> +   On error, the return value will compare equal to 0 and the error 
> information
> +   will be available in grub_errno.  However, if the call is successful that
> +   variable is _not_ modified. */
> +grub_preboot_hookid EXPORT_FUNC(grub_loader_register_preboot)
> +           (grub_err_t (*hook) (void), int abort_on_error);
> +
> +/* Unregister a preboot hook by the id returned by loader_register_preboot.  
> +   This functions becomes a no-op if no such function is registered */
> +void EXPORT_FUNC(grub_loader_unregister_preboot) (grub_preboot_hookid id);
> +
> +/* Call the boot hook in current loader.  This may or may not return,
>     depending on the setting by grub_loader_set.  */
>  grub_err_t EXPORT_FUNC(grub_loader_boot) (void);
>  
> Index: include/grub/i386/pc/drivemap.h
> ===================================================================
> --- include/grub/i386/pc/drivemap.h   (revisión: 0)
> +++ include/grub/i386/pc/drivemap.h   (revisión: 0)
> @@ -0,0 +1,37 @@
> +/* drivemap.h - command to manage the BIOS drive mappings.  */
> +/*
> + *  GRUB  --  GRand Unified Bootloader
> + *  Copyright (C) 2008  Free Software Foundation, Inc.
> + *
> + *  GRUB is free software: you can redistribute it and/or modify
> + *  it under the terms of the GNU General Public License as published by
> + *  the Free Software Foundation, either version 3 of the License, or
> + *  (at your option) any later version.
> + *
> + *  GRUB is distributed in the hope that it will be useful,
> + *  but WITHOUT ANY WARRANTY; without even the implied warranty of
> + *  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
> + *  GNU General Public License for more details.
> + *
> + *  You should have received a copy of the GNU General Public License
> + *  along with GRUB.  If not, see <http://www.gnu.org/licenses/>.
> + */

Please add an inclusion guard.

> +#include <grub/types.h>
> +
> +/* Realmode far ptr (2 * 16b) to the previous INT13h handler.  */
> +extern grub_uint32_t grub_drivemap_int13_oldhandler;
> +/* Size of the INT13h handler bundle (code+data) to be deployed.  */
> +extern grub_uint16_t grub_drivemap_int13_size;
> +
> +/* This type is used for imported assembly labels, takes no storage and is 
> only
> +   used to take the symbol address with &label.  Do NOT put void* here.  */
> +typedef void grub_symbol_t;
> +
> +/* Start of the handler bundle.  */
> +extern grub_symbol_t grub_drivemap_int13_handler_base;
> +/* Start of the drive mappings area (space reserved at runtime).  */
> +extern grub_symbol_t grub_drivemap_int13_mapstart;
> +/* The assembly function to replace the old INT13h handler. It should not be
> +   called because it does not follow any C callspecs and returns with IRET.  
> */
> +extern grub_symbol_t grub_drivemap_int13_handler;
> Index: kern/loader.c
> ===================================================================
> --- kern/loader.c     (revisión: 1798)
> +++ kern/loader.c     (copia de trabajo)
> @@ -61,11 +61,85 @@
>    grub_loader_loaded = 0;
>  }
>  
> +struct hooklist_node
> +{
> +  grub_err_t (*hook) (void);
> +  int abort_on_error;
> +  struct hooklist_node *next;
> +};
> +
> +static struct hooklist_node *preboot_hooks = 0;
> +
> +grub_preboot_hookid
> +grub_loader_register_preboot (grub_err_t (*hook) (void), int abort_on_error)
> +{
> +  grub_preboot_hookid newentry;
> +  if (!hook)
> +    {
> +      grub_error (GRUB_ERR_BAD_ARGUMENT, "preboot hook must not be NULL");
> +      return 0;
> +    }
> +  newentry = grub_malloc (sizeof (struct hooklist_node));
> +  if (!newentry)
> +    {
> +      grub_error (GRUB_ERR_OUT_OF_MEMORY, "cannot alloc a hookinfo 
> structure");
> +      return 0;
> +    }
> +  else
> +    {
> +      newentry->hook = hook;
> +      newentry->abort_on_error = abort_on_error;
> +      newentry->next = preboot_hooks;
> +      preboot_hooks = newentry;
> +      return newentry;
> +    }
> +}
> +
> +void
> +grub_loader_unregister_preboot (grub_preboot_hookid id)
> +{
> +  grub_preboot_hookid entry = 0;
> +  grub_preboot_hookid search = preboot_hooks;
> +  grub_preboot_hookid previous = 0;
> +
> +  if (id == 0)
> +    return;
> +
> +  while (search)
> +    {
> +      if (search == id)
> +        {
> +          entry = search;
> +          break;
> +        }
> +      previous = search;
> +      search = search->next;
> +    }
> +
> +  if (entry)
> +    {
> +      if (previous)
> +        previous->next = entry->next;
> +      else preboot_hooks = entry->next; /* Entry was head of list */

.  */

Please put the code after the else on a separate line.

> +      grub_free (entry);
> +    }
> +}
> +
>  grub_err_t
>  grub_loader_boot (void)
>  {
> +  grub_preboot_hookid entry = preboot_hooks;
> +
>    if (! grub_loader_loaded)
>      return grub_error (GRUB_ERR_NO_KERNEL, "no loaded kernel");
> +  
> +  while (entry)
> +    {
> +      grub_err_t possible_error = entry->hook();
> +      if (possible_error != GRUB_ERR_NONE && entry->abort_on_error)
> +        return possible_error;
> +      entry = entry->next;
> +    }
>  
>    if (grub_loader_noreturn)
>      grub_machine_fini ();
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel