[PATCH] cdrom access patch

[PATCH] cdrom access patch

[Bean]

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

Hi,

This patch use int 13 func 4B01 to detect cdrom. It also change the
name of the cd device to (hdN), where N is the bios drive number.

-- 
Bean

[-- Attachment #2: cd.diff --]
[-- Type: application/octet-stream, Size: 11455 bytes --]

diff --git a/disk/i386/pc/biosdisk.c b/disk/i386/pc/biosdisk.c
index ddcc666..26997a0 100644
--- a/disk/i386/pc/biosdisk.c
+++ b/disk/i386/pc/biosdisk.c
@@ -18,6 +18,7 @@
 
 #include <grub/machine/biosdisk.h>
 #include <grub/machine/memory.h>
+#include <grub/machine/kernel.h>
 #include <grub/disk.h>
 #include <grub/dl.h>
 #include <grub/mm.h>
@@ -26,15 +27,12 @@
 #include <grub/err.h>
 #include <grub/term.h>
 
-static int cd_start = GRUB_BIOSDISK_MACHINE_CDROM_START;
-static int cd_count = 0;
-
 static int
 grub_biosdisk_get_drive (const char *name)
 {
   unsigned long drive;
 
-  if ((name[0] != 'f' && name[0] != 'h' && name[0] != 'c') || name[1] != 'd')
+  if ((name[0] != 'f' && name[0] != 'h') || name[1] != 'd')
     goto fail;
     
   drive = grub_strtoul (name + 2, 0, 10);
@@ -43,8 +41,6 @@ grub_biosdisk_get_drive (const char *name)
 
   if (name[0] == 'h')
     drive += 0x80;
-  else if (name[0] == 'c')
-    drive += cd_start;
   
   return (int) drive ;
 
@@ -58,10 +54,7 @@ grub_biosdisk_call_hook (int (*hook) (const char *name), int drive)
 {
   char name[10];
 
-  if (drive >= cd_start)
-    grub_sprintf (name, "cd%d", drive - cd_start);
-  else
-    grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
+  grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
   return hook (name);
 }
 
@@ -78,7 +71,8 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
       return 1;
   
   /* For hard disks, attempt to read the MBR.  */
-  for (drive = 0x80; drive < 0x90; drive++)
+  for (drive = 0x80;
+       drive < GRUB_BIOSDISK_MACHINE_CDROM_START;  drive++)
     {
       if (grub_biosdisk_rw_standard (0x02, drive, 0, 0, 1, 1,
 				     GRUB_MEMORY_MACHINE_SCRATCH_SEG) != 0)
@@ -91,9 +85,30 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
 	return 1;
     }
 
-  for (drive = cd_start; drive < cd_start + cd_count; drive++)
-    if (grub_biosdisk_call_hook (hook, drive))
-      return 1;
+  if (grub_boot_drive >= GRUB_BIOSDISK_MACHINE_CDROM_START)
+    for (drive = grub_boot_drive;
+         drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
+      {
+        struct grub_biosdisk_dap *dap;
+
+        dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
+                                            + (4 << GRUB_DISK_SECTOR_BITS));
+
+        dap->length = sizeof (*dap);
+        dap->reserved = 0;
+        dap->blocks = 1;
+        dap->buffer = GRUB_MEMORY_MACHINE_SCRATCH_SEG << 16;
+        dap->block = 0;
+
+        if (grub_biosdisk_rw_int13_extensions (0x42, drive, dap))
+          {
+            grub_dprintf ("disk", "Read error when probing cd 0x%2x\n", drive);
+            break;
+          }
+
+        if (grub_biosdisk_call_hook (hook, drive))
+          return 1;
+      }
 
   return 0;
 }
@@ -109,7 +124,7 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   if (drive < 0)
     return grub_errno;
 
-  disk->has_partitions = ((drive & 0x80) && (drive < cd_start));
+  disk->has_partitions = ((drive & 0x80) != 0);
   disk->id = drive;
   
   data = (struct grub_biosdisk_data *) grub_malloc (sizeof (*data));
@@ -119,42 +134,54 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   data->drive = drive;
   data->flags = 0;
 
-  if (drive >= cd_start)
+  if (drive & 0x80)
     {
-      data->flags = GRUB_BIOSDISK_FLAG_LBA | GRUB_BIOSDISK_FLAG_CDROM;
-      data->sectors = 32;
-      total_sectors = 9000000;  /* TODO: get the correct size.  */
-    }
-  else if (drive & 0x80)
-    {
-      /* HDD */
-      int version;
-      
-      version = grub_biosdisk_check_int13_extensions (drive);
-      if (version)
-	{
-	  struct grub_biosdisk_drp *drp
-	    = (struct grub_biosdisk_drp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
-
-	  /* Clear out the DRP.  */
-	  grub_memset (drp, 0, sizeof (*drp));
-	  drp->size = sizeof (*drp);
-	  if (! grub_biosdisk_get_diskinfo_int13_extensions (drive, drp))
-	    {
-	      data->flags = GRUB_BIOSDISK_FLAG_LBA;
-
-	      if (drp->total_sectors)
-		total_sectors = drp->total_sectors;
-	      else
-                /* Some buggy BIOSes doesn't return the total sectors
-                   correctly but returns zero. So if it is zero, compute
-                   it by C/H/S returned by the LBA BIOS call.  */
-                total_sectors = drp->cylinders * drp->heads * drp->sectors;
-	    }
-	}
+      struct grub_biosdisk_cdrp *cdrp
+        = (struct grub_biosdisk_cdrp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+      grub_memset (cdrp, 0, sizeof (*cdrp));
+      cdrp->size = sizeof (*cdrp);
+      if ((! grub_biosdisk_get_cdinfo_int13_extensions (drive, cdrp)) &&
+          (cdrp->drive_no == drive) &&
+          ((cdrp->media_type & GRUB_BIOSDISK_CDTYPE_MASK)
+           == GRUB_BIOSDISK_CDTYPE_NO_EMUL))
+        {
+          disk->has_partitions = 0;
+          data->flags = (GRUB_BIOSDISK_FLAG_LBA | GRUB_BIOSDISK_FLAG_CDROM);
+          data->sectors = 32;
+          total_sectors = 9000000;  /* TODO: get the correct size.  */
+        }
+      else
+        {
+          /* HDD */
+          int version;
+
+          version = grub_biosdisk_check_int13_extensions (drive);
+          if (version)
+            {
+              struct grub_biosdisk_drp *drp
+                    = (struct grub_biosdisk_drp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+              /* Clear out the DRP.  */
+              grub_memset (drp, 0, sizeof (*drp));
+              drp->size = sizeof (*drp);
+              if (! grub_biosdisk_get_diskinfo_int13_extensions (drive, drp))
+                {
+                  data->flags = GRUB_BIOSDISK_FLAG_LBA;
+
+                  if (drp->total_sectors)
+                    total_sectors = drp->total_sectors;
+                  else
+                    /* Some buggy BIOSes doesn't return the total sectors
+                       correctly but returns zero. So if it is zero, compute
+                       it by C/H/S returned by the LBA BIOS call.  */
+                    total_sectors = drp->cylinders * drp->heads * drp->sectors;
+                }
+            }
+        }
     }
 
-  if (drive < cd_start)
+  if (! (data->flags & GRUB_BIOSDISK_FLAG_CDROM))
     {
       if (grub_biosdisk_get_diskinfo_standard (drive,
 					       &data->cylinders,
@@ -364,8 +391,6 @@ grub_disk_biosdisk_fini (void)
 
 GRUB_MOD_INIT(biosdisk)
 {
-  int drive, found = 0;
-
   if (grub_disk_firmware_is_tainted)
     {
       grub_printf ("Firmware is marked as tainted, refusing to initialize.\n");
@@ -374,24 +399,6 @@ GRUB_MOD_INIT(biosdisk)
   grub_disk_firmware_fini = grub_disk_biosdisk_fini;
 
   grub_disk_dev_register (&grub_biosdisk_dev);
-
-  for (drive = GRUB_BIOSDISK_MACHINE_CDROM_START;
-       drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
-    {
-      if (grub_biosdisk_check_int13_extensions (drive))
-        {
-	  if (! found)
-	    cd_start = drive;
-	  found++;
-	}
-      else
-        {
-	  if (found)
-            break;
-	}
-    }
-
-  cd_count = found;
 }
 
 GRUB_MOD_FINI(biosdisk)
diff --git a/include/grub/i386/pc/biosdisk.h b/include/grub/i386/pc/biosdisk.h
index 770f942..848840e 100644
--- a/include/grub/i386/pc/biosdisk.h
+++ b/include/grub/i386/pc/biosdisk.h
@@ -25,9 +25,18 @@
 #define GRUB_BIOSDISK_FLAG_LBA	1
 #define GRUB_BIOSDISK_FLAG_CDROM 2
 
-#define GRUB_BIOSDISK_MACHINE_CDROM_START	0xe0
+#define GRUB_BIOSDISK_MACHINE_CDROM_START	0x9f
 #define GRUB_BIOSDISK_MACHINE_CDROM_END		0xf0
 
+#define GRUB_BIOSDISK_CDTYPE_NO_EMUL	0
+#define GRUB_BIOSDISK_CDTYPE_1_2_M	1
+#define GRUB_BIOSDISK_CDTYPE_1_44_M	2
+#define GRUB_BIOSDISK_CDTYPE_2_88_M	3
+#define GRUB_BIOSDISK_CDTYPE_2_88_M	3
+#define GRUB_BIOSDISK_CDTYPE_HARDDISK	4
+
+#define GRUB_BIOSDISK_CDTYPE_MASK	0xF
+
 struct grub_biosdisk_data
 {
   int drive;
@@ -74,6 +83,23 @@ struct grub_biosdisk_drp
   grub_uint8_t dummy[16];
 } __attribute__ ((packed));
 
+struct grub_biosdisk_cdrp
+{
+  grub_uint8_t size;
+  grub_uint8_t media_type;
+  grub_uint8_t drive_no;
+  grub_uint8_t controller_no;
+  grub_uint32_t image_lba;
+  grub_uint16_t device_spec;
+  grub_uint16_t cache_seg;
+  grub_uint16_t load_seg;
+  grub_uint16_t length_sec512;
+  grub_uint8_t cylinders;
+  grub_uint8_t sectors;
+  grub_uint8_t heads;
+  grub_uint8_t dummy[16];
+} __attribute__ ((packed));
+
 /* Disk Address Packet.  */
 struct grub_biosdisk_dap
 {
@@ -90,6 +116,8 @@ int EXPORT_FUNC(grub_biosdisk_rw_standard) (int ah, int drive, int coff, int hof
 int EXPORT_FUNC(grub_biosdisk_check_int13_extensions) (int drive);
 int EXPORT_FUNC(grub_biosdisk_get_diskinfo_int13_extensions) (int drive,
            void *drp);
+int EXPORT_FUNC(grub_biosdisk_get_cdinfo_int13_extensions) (int drive,
+           void *cdrp);
 int EXPORT_FUNC(grub_biosdisk_get_diskinfo_standard) (int drive,
 					 unsigned long *cylinders,
 					 unsigned long *heads,
diff --git a/include/grub/i386/pc/kernel.h b/include/grub/i386/pc/kernel.h
index 848dad1..43a8d5b 100644
--- a/include/grub/i386/pc/kernel.h
+++ b/include/grub/i386/pc/kernel.h
@@ -71,7 +71,7 @@ extern grub_int32_t grub_memdisk_image_size;
 extern char grub_prefix[];
 
 /* The boot BIOS drive number.  */
-extern grub_int32_t grub_boot_drive;
+extern grub_int32_t EXPORT_VAR(grub_boot_drive);
 
 /* The root BIOS drive number.  */
 extern grub_int32_t grub_root_drive;
diff --git a/kern/i386/pc/init.c b/kern/i386/pc/init.c
index 7237492..525b667 100644
--- a/kern/i386/pc/init.c
+++ b/kern/i386/pc/init.c
@@ -22,7 +22,6 @@
 #include <grub/machine/memory.h>
 #include <grub/machine/console.h>
 #include <grub/machine/kernel.h>
-#include <grub/machine/biosdisk.h>
 #include <grub/types.h>
 #include <grub/err.h>
 #include <grub/dl.h>
@@ -77,13 +76,8 @@ make_install_device (void)
       if (grub_root_drive == 0xFF)
         grub_root_drive = grub_boot_drive;
       
-      if (grub_root_drive >= GRUB_BIOSDISK_MACHINE_CDROM_START)
-        grub_sprintf (dev, "(cd%u",
-		      grub_root_drive - GRUB_BIOSDISK_MACHINE_CDROM_START);
-      else
-        grub_sprintf (dev, "(%cd%u",
-		      (grub_root_drive & 0x80) ? 'h' : 'f',
-		      grub_root_drive & 0x7f);
+      grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
+                    grub_root_drive & 0x7f);
       
       if (grub_install_dos_part >= 0)
 	grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 1);
diff --git a/kern/i386/pc/startup.S b/kern/i386/pc/startup.S
index bf3d4ee..2994ee2 100644
--- a/kern/i386/pc/startup.S
+++ b/kern/i386/pc/startup.S
@@ -780,6 +780,17 @@ FUNCTION(grub_biosdisk_check_int13_extensions)
 
 
 /*
+ *   int grub_biosdisk_get_cdinfo_int13_extensions (int drive, void *cdrp)
+ *
+ *   Return the cdrom information of DRIVE in CDRP. If an error occurs,
+ *   then return non-zero, otherwise zero.
+ */
+
+FUNCTION(grub_biosdisk_get_cdinfo_int13_extensions)
+	movw	$0x4B01, %cx
+	jmp	1f
+
+/*
  *   int grub_biosdisk_get_diskinfo_int13_extensions (int drive, void *drp)
  *
  *   Return the geometry of DRIVE in a drive parameters, DRP. If an error
@@ -787,6 +798,8 @@ FUNCTION(grub_biosdisk_check_int13_extensions)
  */
 
 FUNCTION(grub_biosdisk_get_diskinfo_int13_extensions)
+	movb	$0x48, %ch
+1:
 	pushl	%ebp
 	pushl	%ebx
 	pushl	%esi
@@ -802,7 +815,7 @@ FUNCTION(grub_biosdisk_get_diskinfo_int13_extensions)
 	call	prot_to_real
 
 	.code16
-	movb	$0x48, %ah
+	movw	%cx, %ax
 	movw	%bx, %ds
 	int	$0x13		/* do the operation */
 	movb	%ah, %bl	/* save return value in %bl */

Re: [PATCH] cdrom access patch

[Vesa Jääskeläinen]

Bean wrote:
> Hi,
> 
> This patch use int 13 func 4B01 to detect cdrom. It also change the
> name of the cd device to (hdN), where N is the bios drive number.

Hi Bean,

Have you tried to use EDD's functions to query for type of device?
(references are to EDD3 spec)

8.24 Get Device Parameters (FN 48h)
26 DWord Pointer to the Device Parameter Table Extension (DPTE)

8.24.3 Device Parameter Table Extension (DPTE)
10-11 Word BIOS selected hardware specific option flags

Bits 5 and 6; Removable device, ATAPI device.

This FN 48h can also be used to filter out PCI cards from ATA driver in 
case there is BIOS support for the device already. So use BIOS if there 
is BIOS handler for it, otherwise fall back to ATA driver.

I would prefer that CD's would be counted like (cd0) for first CD-ROM 
device, (cd1) for second CD-ROM device, ... and so on. I think it is OK 
to first allocate BIOS devices for CD's and after that use ATA driver 
for rest of them. Ata driver can of course be accessed with ata name too.

Thanks,
Vesa Jääskeläinen

Re: [PATCH] cdrom access patch

[Bean]

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

On Fri, Mar 7, 2008 at 11:37 PM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
>
> Bean wrote:
>  > Hi,
>  >
>  > This patch use int 13 func 4B01 to detect cdrom. It also change the
>  > name of the cd device to (hdN), where N is the bios drive number.
>
>  Hi Bean,
>
>  Have you tried to use EDD's functions to query for type of device?
>  (references are to EDD3 spec)
>
>  8.24 Get Device Parameters (FN 48h)
>  26 DWord Pointer to the Device Parameter Table Extension (DPTE)
>
>  8.24.3 Device Parameter Table Extension (DPTE)
>  10-11 Word BIOS selected hardware specific option flags
>
>  Bits 5 and 6; Removable device, ATAPI device.
>
>  This FN 48h can also be used to filter out PCI cards from ATA driver in
>  case there is BIOS support for the device already. So use BIOS if there
>  is BIOS handler for it, otherwise fall back to ATA driver.
>
>  I would prefer that CD's would be counted like (cd0) for first CD-ROM
>  device, (cd1) for second CD-ROM device, ... and so on. I think it is OK
>  to first allocate BIOS devices for CD's and after that use ATA driver
>  for rest of them. Ata driver can of course be accessed with ata name too.

Hi,

after some consideration, i think the best way to detect cdrom is to
check the number of bytes per sector. The handling of cdrom assume
sector size of 4096, cdrom that has other block size would not work.
On the other hand, the device don't even needs to be cdrom, as long as
it has 2048 sector size, the current handling works. Therefore,
checking sector size should be sufficient.

As for the device name, there are some drawbacks by using (cdN), in
init.c, we need to get the root device from bios drive number.
However, as Christian Franke discovers, the drive number can be:

0x82 (old Toshiba Laptop with unknown BIOS)
0x9f (PC with Phoenix-Award BIOS, VMware)
0xe0 (VirtualBox)
0xef (PC with AMI BIOS, VirtualPC)

It' quite tricky to determine whether a drive is cdrom or harddisk.

And in the multiboot loader, we need to do the oppisite, map the root
device to bios drive number. As names like (cd0) doesn't suggests the
number at all, we need to ask biosdisk for the number. Therefore,
multiboot loader will be depended on biosdisk, which is not a good
idea.

-- 
Bean

[-- Attachment #2: cd2.diff --]
[-- Type: text/plain, Size: 7610 bytes --]

diff --git a/disk/i386/pc/biosdisk.c b/disk/i386/pc/biosdisk.c
index ddcc666..3efe7cb 100644
--- a/disk/i386/pc/biosdisk.c
+++ b/disk/i386/pc/biosdisk.c
@@ -18,6 +18,7 @@
 
 #include <grub/machine/biosdisk.h>
 #include <grub/machine/memory.h>
+#include <grub/machine/kernel.h>
 #include <grub/disk.h>
 #include <grub/dl.h>
 #include <grub/mm.h>
@@ -26,15 +27,12 @@
 #include <grub/err.h>
 #include <grub/term.h>
 
-static int cd_start = GRUB_BIOSDISK_MACHINE_CDROM_START;
-static int cd_count = 0;
-
 static int
 grub_biosdisk_get_drive (const char *name)
 {
   unsigned long drive;
 
-  if ((name[0] != 'f' && name[0] != 'h' && name[0] != 'c') || name[1] != 'd')
+  if ((name[0] != 'f' && name[0] != 'h') || name[1] != 'd')
     goto fail;
     
   drive = grub_strtoul (name + 2, 0, 10);
@@ -43,8 +41,6 @@ grub_biosdisk_get_drive (const char *name)
 
   if (name[0] == 'h')
     drive += 0x80;
-  else if (name[0] == 'c')
-    drive += cd_start;
   
   return (int) drive ;
 
@@ -58,10 +54,7 @@ grub_biosdisk_call_hook (int (*hook) (const char *name), int drive)
 {
   char name[10];
 
-  if (drive >= cd_start)
-    grub_sprintf (name, "cd%d", drive - cd_start);
-  else
-    grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
+  grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
   return hook (name);
 }
 
@@ -78,7 +71,8 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
       return 1;
   
   /* For hard disks, attempt to read the MBR.  */
-  for (drive = 0x80; drive < 0x90; drive++)
+  for (drive = 0x80;
+       drive < GRUB_BIOSDISK_MACHINE_CDROM_START;  drive++)
     {
       if (grub_biosdisk_rw_standard (0x02, drive, 0, 0, 1, 1,
 				     GRUB_MEMORY_MACHINE_SCRATCH_SEG) != 0)
@@ -91,9 +85,30 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
 	return 1;
     }
 
-  for (drive = cd_start; drive < cd_start + cd_count; drive++)
-    if (grub_biosdisk_call_hook (hook, drive))
-      return 1;
+  if (grub_boot_drive >= GRUB_BIOSDISK_MACHINE_CDROM_START)
+    for (drive = grub_boot_drive;
+         drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
+      {
+        struct grub_biosdisk_dap *dap;
+
+        dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
+                                            + (4 << GRUB_DISK_SECTOR_BITS));
+
+        dap->length = sizeof (*dap);
+        dap->reserved = 0;
+        dap->blocks = 1;
+        dap->buffer = GRUB_MEMORY_MACHINE_SCRATCH_SEG << 16;
+        dap->block = 0;
+
+        if (grub_biosdisk_rw_int13_extensions (0x42, drive, dap))
+          {
+            grub_dprintf ("disk", "Read error when probing cd 0x%2x\n", drive);
+            break;
+          }
+
+        if (grub_biosdisk_call_hook (hook, drive))
+          return 1;
+      }
 
   return 0;
 }
@@ -109,7 +124,7 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   if (drive < 0)
     return grub_errno;
 
-  disk->has_partitions = ((drive & 0x80) && (drive < cd_start));
+  disk->has_partitions = ((drive & 0x80) != 0);
   disk->id = drive;
   
   data = (struct grub_biosdisk_data *) grub_malloc (sizeof (*data));
@@ -119,42 +134,43 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   data->drive = drive;
   data->flags = 0;
 
-  if (drive >= cd_start)
-    {
-      data->flags = GRUB_BIOSDISK_FLAG_LBA | GRUB_BIOSDISK_FLAG_CDROM;
-      data->sectors = 32;
-      total_sectors = 9000000;  /* TODO: get the correct size.  */
-    }
-  else if (drive & 0x80)
+  if (drive & 0x80)
     {
       /* HDD */
       int version;
       
       version = grub_biosdisk_check_int13_extensions (drive);
       if (version)
-	{
-	  struct grub_biosdisk_drp *drp
-	    = (struct grub_biosdisk_drp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
-
-	  /* Clear out the DRP.  */
-	  grub_memset (drp, 0, sizeof (*drp));
-	  drp->size = sizeof (*drp);
-	  if (! grub_biosdisk_get_diskinfo_int13_extensions (drive, drp))
-	    {
-	      data->flags = GRUB_BIOSDISK_FLAG_LBA;
-
-	      if (drp->total_sectors)
-		total_sectors = drp->total_sectors;
-	      else
+        {
+          struct grub_biosdisk_drp *drp
+                = (struct grub_biosdisk_drp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+          /* Clear out the DRP.  */
+          grub_memset (drp, 0, sizeof (*drp));
+          drp->size = sizeof (*drp);
+          if (! grub_biosdisk_get_diskinfo_int13_extensions (drive, drp))
+            {
+              data->flags = GRUB_BIOSDISK_FLAG_LBA;
+
+              if (drp->total_sectors)
+                total_sectors = drp->total_sectors;
+              else
                 /* Some buggy BIOSes doesn't return the total sectors
                    correctly but returns zero. So if it is zero, compute
                    it by C/H/S returned by the LBA BIOS call.  */
                 total_sectors = drp->cylinders * drp->heads * drp->sectors;
-	    }
-	}
+
+              if (drp->bytes_per_sector == (GRUB_DISK_SECTOR_SIZE << 2))
+                {
+                  data->flags |= GRUB_BIOSDISK_FLAG_CDROM;
+                  data->sectors = 32;
+                  total_sectors = 9000000;
+                }
+            }
+        }
     }
 
-  if (drive < cd_start)
+  if (! (data->flags & GRUB_BIOSDISK_FLAG_CDROM))
     {
       if (grub_biosdisk_get_diskinfo_standard (drive,
 					       &data->cylinders,
@@ -364,8 +380,6 @@ grub_disk_biosdisk_fini (void)
 
 GRUB_MOD_INIT(biosdisk)
 {
-  int drive, found = 0;
-
   if (grub_disk_firmware_is_tainted)
     {
       grub_printf ("Firmware is marked as tainted, refusing to initialize.\n");
@@ -374,24 +388,6 @@ GRUB_MOD_INIT(biosdisk)
   grub_disk_firmware_fini = grub_disk_biosdisk_fini;
 
   grub_disk_dev_register (&grub_biosdisk_dev);
-
-  for (drive = GRUB_BIOSDISK_MACHINE_CDROM_START;
-       drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
-    {
-      if (grub_biosdisk_check_int13_extensions (drive))
-        {
-	  if (! found)
-	    cd_start = drive;
-	  found++;
-	}
-      else
-        {
-	  if (found)
-            break;
-	}
-    }
-
-  cd_count = found;
 }
 
 GRUB_MOD_FINI(biosdisk)
diff --git a/include/grub/i386/pc/kernel.h b/include/grub/i386/pc/kernel.h
index 848dad1..43a8d5b 100644
--- a/include/grub/i386/pc/kernel.h
+++ b/include/grub/i386/pc/kernel.h
@@ -71,7 +71,7 @@ extern grub_int32_t grub_memdisk_image_size;
 extern char grub_prefix[];
 
 /* The boot BIOS drive number.  */
-extern grub_int32_t grub_boot_drive;
+extern grub_int32_t EXPORT_VAR(grub_boot_drive);
 
 /* The root BIOS drive number.  */
 extern grub_int32_t grub_root_drive;
diff --git a/kern/i386/pc/init.c b/kern/i386/pc/init.c
index 7237492..525b667 100644
--- a/kern/i386/pc/init.c
+++ b/kern/i386/pc/init.c
@@ -22,7 +22,6 @@
 #include <grub/machine/memory.h>
 #include <grub/machine/console.h>
 #include <grub/machine/kernel.h>
-#include <grub/machine/biosdisk.h>
 #include <grub/types.h>
 #include <grub/err.h>
 #include <grub/dl.h>
@@ -77,13 +76,8 @@ make_install_device (void)
       if (grub_root_drive == 0xFF)
         grub_root_drive = grub_boot_drive;
       
-      if (grub_root_drive >= GRUB_BIOSDISK_MACHINE_CDROM_START)
-        grub_sprintf (dev, "(cd%u",
-		      grub_root_drive - GRUB_BIOSDISK_MACHINE_CDROM_START);
-      else
-        grub_sprintf (dev, "(%cd%u",
-		      (grub_root_drive & 0x80) ? 'h' : 'f',
-		      grub_root_drive & 0x7f);
+      grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
+                    grub_root_drive & 0x7f);
       
       if (grub_install_dos_part >= 0)
 	grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 1);

Re: [PATCH] cdrom access patch

[Bean]

On Sun, Mar 9, 2008 at 11:33 PM, Bean <bean123ch@gmail.com> wrote:
> On Fri, Mar 7, 2008 at 11:37 PM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
>  >
>  > Bean wrote:
>  >  > Hi,
>  >  >
>  >  > This patch use int 13 func 4B01 to detect cdrom. It also change the
>  >  > name of the cd device to (hdN), where N is the bios drive number.
>  >
>  >  Hi Bean,
>  >
>  >  Have you tried to use EDD's functions to query for type of device?
>  >  (references are to EDD3 spec)
>  >
>  >  8.24 Get Device Parameters (FN 48h)
>  >  26 DWord Pointer to the Device Parameter Table Extension (DPTE)
>  >
>  >  8.24.3 Device Parameter Table Extension (DPTE)
>  >  10-11 Word BIOS selected hardware specific option flags
>  >
>  >  Bits 5 and 6; Removable device, ATAPI device.
>  >
>  >  This FN 48h can also be used to filter out PCI cards from ATA driver in
>  >  case there is BIOS support for the device already. So use BIOS if there
>  >  is BIOS handler for it, otherwise fall back to ATA driver.
>  >
>  >  I would prefer that CD's would be counted like (cd0) for first CD-ROM
>  >  device, (cd1) for second CD-ROM device, ... and so on. I think it is OK
>  >  to first allocate BIOS devices for CD's and after that use ATA driver
>  >  for rest of them. Ata driver can of course be accessed with ata name too.
>
>  Hi,
>
>  after some consideration, i think the best way to detect cdrom is to
>  check the number of bytes per sector. The handling of cdrom assume
>  sector size of 4096, cdrom that has other block size would not work.
>  On the other hand, the device don't even needs to be cdrom, as long as
>  it has 2048 sector size, the current handling works. Therefore,
>  checking sector size should be sufficient.
>
>  As for the device name, there are some drawbacks by using (cdN), in
>  init.c, we need to get the root device from bios drive number.
>  However, as Christian Franke discovers, the drive number can be:
>
>  0x82 (old Toshiba Laptop with unknown BIOS)
>  0x9f (PC with Phoenix-Award BIOS, VMware)
>  0xe0 (VirtualBox)
>  0xef (PC with AMI BIOS, VirtualPC)
>
>  It' quite tricky to determine whether a drive is cdrom or harddisk.
>
>  And in the multiboot loader, we need to do the oppisite, map the root
>  device to bios drive number. As names like (cd0) doesn't suggests the
>  number at all, we need to ask biosdisk for the number. Therefore,
>  multiboot loader will be depended on biosdisk, which is not a good
>  idea.

After some testing, i think the new version has some compatible issue,
i remember Christian Franke reports that some bios doesn't support int
42 for cdrom, so grub_biosdisk_check_int13_extensions will fail. Would
someone check if func 4b01 (the first patch cd.diff) works in such
situation ?

-- 
Bean

Re: [PATCH] cdrom access patch

[Christian Franke]

Bean wrote:
> ...
>
> After some testing, i think the new version has some compatible issue,
> i remember Christian Franke reports that some bios doesn't support int
> 42 for cdrom, so grub_biosdisk_check_int13_extensions will fail. Would
> someone check if func 4b01 (the first patch cd.diff) works in such
> situation ?
>
>   

cd.diff works on this specific PC (Asus A8N, Phonix-Award BIOS), CD 
appears as hd31.

BTW: grub_biosdisk_iterate should IMO always expose the boot_drive (see 
my latest patch).
With current CVS and cd.diff, the boot_drive would not be visible in 
'ls' if e.g 0x80=HDD, 0x82=boot CD (as seen on some old Laptop).

Christian

Re: [PATCH] cdrom access patch

[Bean]

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

On Wed, Mar 12, 2008 at 5:10 AM, Christian Franke
<Christian.Franke@t-online.de> wrote:
> Bean wrote:
>  > ...
>
> >
>  > After some testing, i think the new version has some compatible issue,
>  > i remember Christian Franke reports that some bios doesn't support int
>  > 42 for cdrom, so grub_biosdisk_check_int13_extensions will fail. Would
>  > someone check if func 4b01 (the first patch cd.diff) works in such
>  > situation ?
>  >
>  >
>
>  cd.diff works on this specific PC (Asus A8N, Phonix-Award BIOS), CD
>  appears as hd31.
>
>  BTW: grub_biosdisk_iterate should IMO always expose the boot_drive (see
>  my latest patch).
>  With current CVS and cd.diff, the boot_drive would not be visible in
>  'ls' if e.g 0x80=HDD, 0x82=boot CD (as seen on some old Laptop).

Thanks for the tip, please test the new patch, it start scanning for
cdrom if the boot drive is larger than the last harddisk.

BTW, i have a small question, if there is more than one cdrom, would
bios export all of them, or just the boot one ?

-- 
Bean

[-- Attachment #2: cd3.diff --]
[-- Type: text/plain, Size: 11427 bytes --]

diff --git a/disk/i386/pc/biosdisk.c b/disk/i386/pc/biosdisk.c
index ddcc666..75b8845 100644
--- a/disk/i386/pc/biosdisk.c
+++ b/disk/i386/pc/biosdisk.c
@@ -18,6 +18,7 @@
 
 #include <grub/machine/biosdisk.h>
 #include <grub/machine/memory.h>
+#include <grub/machine/kernel.h>
 #include <grub/disk.h>
 #include <grub/dl.h>
 #include <grub/mm.h>
@@ -26,15 +27,12 @@
 #include <grub/err.h>
 #include <grub/term.h>
 
-static int cd_start = GRUB_BIOSDISK_MACHINE_CDROM_START;
-static int cd_count = 0;
-
 static int
 grub_biosdisk_get_drive (const char *name)
 {
   unsigned long drive;
 
-  if ((name[0] != 'f' && name[0] != 'h' && name[0] != 'c') || name[1] != 'd')
+  if ((name[0] != 'f' && name[0] != 'h') || name[1] != 'd')
     goto fail;
     
   drive = grub_strtoul (name + 2, 0, 10);
@@ -43,8 +41,6 @@ grub_biosdisk_get_drive (const char *name)
 
   if (name[0] == 'h')
     drive += 0x80;
-  else if (name[0] == 'c')
-    drive += cd_start;
   
   return (int) drive ;
 
@@ -58,10 +54,7 @@ grub_biosdisk_call_hook (int (*hook) (const char *name), int drive)
 {
   char name[10];
 
-  if (drive >= cd_start)
-    grub_sprintf (name, "cd%d", drive - cd_start);
-  else
-    grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
+  grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
   return hook (name);
 }
 
@@ -78,7 +71,8 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
       return 1;
   
   /* For hard disks, attempt to read the MBR.  */
-  for (drive = 0x80; drive < 0x90; drive++)
+  for (drive = 0x80;
+       drive < GRUB_BIOSDISK_MACHINE_CDROM_START;  drive++)
     {
       if (grub_biosdisk_rw_standard (0x02, drive, 0, 0, 1, 1,
 				     GRUB_MEMORY_MACHINE_SCRATCH_SEG) != 0)
@@ -91,9 +85,30 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
 	return 1;
     }
 
-  for (drive = cd_start; drive < cd_start + cd_count; drive++)
-    if (grub_biosdisk_call_hook (hook, drive))
-      return 1;
+  if (grub_boot_drive >= drive)
+    for (drive = grub_boot_drive;
+         drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
+      {
+        struct grub_biosdisk_dap *dap;
+
+        dap = (struct grub_biosdisk_dap *) (GRUB_MEMORY_MACHINE_SCRATCH_ADDR
+                                            + (4 << GRUB_DISK_SECTOR_BITS));
+
+        dap->length = sizeof (*dap);
+        dap->reserved = 0;
+        dap->blocks = 1;
+        dap->buffer = GRUB_MEMORY_MACHINE_SCRATCH_SEG << 16;
+        dap->block = 0;
+
+        if (grub_biosdisk_rw_int13_extensions (0x42, drive, dap))
+          {
+            grub_dprintf ("disk", "Read error when probing cd 0x%2x\n", drive);
+            break;
+          }
+
+        if (grub_biosdisk_call_hook (hook, drive))
+          return 1;
+      }
 
   return 0;
 }
@@ -109,7 +124,7 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   if (drive < 0)
     return grub_errno;
 
-  disk->has_partitions = ((drive & 0x80) && (drive < cd_start));
+  disk->has_partitions = ((drive & 0x80) != 0);
   disk->id = drive;
   
   data = (struct grub_biosdisk_data *) grub_malloc (sizeof (*data));
@@ -119,42 +134,54 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   data->drive = drive;
   data->flags = 0;
 
-  if (drive >= cd_start)
+  if (drive & 0x80)
     {
-      data->flags = GRUB_BIOSDISK_FLAG_LBA | GRUB_BIOSDISK_FLAG_CDROM;
-      data->sectors = 32;
-      total_sectors = 9000000;  /* TODO: get the correct size.  */
-    }
-  else if (drive & 0x80)
-    {
-      /* HDD */
-      int version;
-      
-      version = grub_biosdisk_check_int13_extensions (drive);
-      if (version)
-	{
-	  struct grub_biosdisk_drp *drp
-	    = (struct grub_biosdisk_drp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
-
-	  /* Clear out the DRP.  */
-	  grub_memset (drp, 0, sizeof (*drp));
-	  drp->size = sizeof (*drp);
-	  if (! grub_biosdisk_get_diskinfo_int13_extensions (drive, drp))
-	    {
-	      data->flags = GRUB_BIOSDISK_FLAG_LBA;
-
-	      if (drp->total_sectors)
-		total_sectors = drp->total_sectors;
-	      else
-                /* Some buggy BIOSes doesn't return the total sectors
-                   correctly but returns zero. So if it is zero, compute
-                   it by C/H/S returned by the LBA BIOS call.  */
-                total_sectors = drp->cylinders * drp->heads * drp->sectors;
-	    }
-	}
+      struct grub_biosdisk_cdrp *cdrp
+        = (struct grub_biosdisk_cdrp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+      grub_memset (cdrp, 0, sizeof (*cdrp));
+      cdrp->size = sizeof (*cdrp);
+      if ((! grub_biosdisk_get_cdinfo_int13_extensions (drive, cdrp)) &&
+          (cdrp->drive_no == drive) &&
+          ((cdrp->media_type & GRUB_BIOSDISK_CDTYPE_MASK)
+           == GRUB_BIOSDISK_CDTYPE_NO_EMUL))
+        {
+          disk->has_partitions = 0;
+          data->flags = (GRUB_BIOSDISK_FLAG_LBA | GRUB_BIOSDISK_FLAG_CDROM);
+          data->sectors = 32;
+          total_sectors = 9000000;  /* TODO: get the correct size.  */
+        }
+      else
+        {
+          /* HDD */
+          int version;
+
+          version = grub_biosdisk_check_int13_extensions (drive);
+          if (version)
+            {
+              struct grub_biosdisk_drp *drp
+                    = (struct grub_biosdisk_drp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
+
+              /* Clear out the DRP.  */
+              grub_memset (drp, 0, sizeof (*drp));
+              drp->size = sizeof (*drp);
+              if (! grub_biosdisk_get_diskinfo_int13_extensions (drive, drp))
+                {
+                  data->flags = GRUB_BIOSDISK_FLAG_LBA;
+
+                  if (drp->total_sectors)
+                    total_sectors = drp->total_sectors;
+                  else
+                    /* Some buggy BIOSes doesn't return the total sectors
+                       correctly but returns zero. So if it is zero, compute
+                       it by C/H/S returned by the LBA BIOS call.  */
+                    total_sectors = drp->cylinders * drp->heads * drp->sectors;
+                }
+            }
+        }
     }
 
-  if (drive < cd_start)
+  if (! (data->flags & GRUB_BIOSDISK_FLAG_CDROM))
     {
       if (grub_biosdisk_get_diskinfo_standard (drive,
 					       &data->cylinders,
@@ -364,8 +391,6 @@ grub_disk_biosdisk_fini (void)
 
 GRUB_MOD_INIT(biosdisk)
 {
-  int drive, found = 0;
-
   if (grub_disk_firmware_is_tainted)
     {
       grub_printf ("Firmware is marked as tainted, refusing to initialize.\n");
@@ -374,24 +399,6 @@ GRUB_MOD_INIT(biosdisk)
   grub_disk_firmware_fini = grub_disk_biosdisk_fini;
 
   grub_disk_dev_register (&grub_biosdisk_dev);
-
-  for (drive = GRUB_BIOSDISK_MACHINE_CDROM_START;
-       drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
-    {
-      if (grub_biosdisk_check_int13_extensions (drive))
-        {
-	  if (! found)
-	    cd_start = drive;
-	  found++;
-	}
-      else
-        {
-	  if (found)
-            break;
-	}
-    }
-
-  cd_count = found;
 }
 
 GRUB_MOD_FINI(biosdisk)
diff --git a/include/grub/i386/pc/biosdisk.h b/include/grub/i386/pc/biosdisk.h
index 770f942..cba09d2 100644
--- a/include/grub/i386/pc/biosdisk.h
+++ b/include/grub/i386/pc/biosdisk.h
@@ -25,9 +25,18 @@
 #define GRUB_BIOSDISK_FLAG_LBA	1
 #define GRUB_BIOSDISK_FLAG_CDROM 2
 
-#define GRUB_BIOSDISK_MACHINE_CDROM_START	0xe0
+#define GRUB_BIOSDISK_MACHINE_CDROM_START	0x9f
 #define GRUB_BIOSDISK_MACHINE_CDROM_END		0xf0
 
+#define GRUB_BIOSDISK_CDTYPE_NO_EMUL	0
+#define GRUB_BIOSDISK_CDTYPE_1_2_M	1
+#define GRUB_BIOSDISK_CDTYPE_1_44_M	2
+#define GRUB_BIOSDISK_CDTYPE_2_88_M	3
+#define GRUB_BIOSDISK_CDTYPE_2_88_M	3
+#define GRUB_BIOSDISK_CDTYPE_HARDDISK	4
+
+#define GRUB_BIOSDISK_CDTYPE_MASK	0xF
+
 struct grub_biosdisk_data
 {
   int drive;
@@ -74,6 +83,23 @@ struct grub_biosdisk_drp
   grub_uint8_t dummy[16];
 } __attribute__ ((packed));
 
+struct grub_biosdisk_cdrp
+{
+  grub_uint8_t size;
+  grub_uint8_t media_type;
+  grub_uint8_t drive_no;
+  grub_uint8_t controller_no;
+  grub_uint32_t image_lba;
+  grub_uint16_t device_spec;
+  grub_uint16_t cache_seg;
+  grub_uint16_t load_seg;
+  grub_uint16_t length_sec512;
+  grub_uint8_t cylinders;
+  grub_uint8_t sectors;
+  grub_uint8_t heads;
+  grub_uint8_t dummy[16];
+} __attribute__ ((packed));
+
 /* Disk Address Packet.  */
 struct grub_biosdisk_dap
 {
@@ -90,6 +116,8 @@ int EXPORT_FUNC(grub_biosdisk_rw_standard) (int ah, int drive, int coff, int hof
 int EXPORT_FUNC(grub_biosdisk_check_int13_extensions) (int drive);
 int EXPORT_FUNC(grub_biosdisk_get_diskinfo_int13_extensions) (int drive,
            void *drp);
+int EXPORT_FUNC(grub_biosdisk_get_cdinfo_int13_extensions) (int drive,
+           void *cdrp);
 int EXPORT_FUNC(grub_biosdisk_get_diskinfo_standard) (int drive,
 					 unsigned long *cylinders,
 					 unsigned long *heads,
diff --git a/include/grub/i386/pc/kernel.h b/include/grub/i386/pc/kernel.h
index 848dad1..43a8d5b 100644
--- a/include/grub/i386/pc/kernel.h
+++ b/include/grub/i386/pc/kernel.h
@@ -71,7 +71,7 @@ extern grub_int32_t grub_memdisk_image_size;
 extern char grub_prefix[];
 
 /* The boot BIOS drive number.  */
-extern grub_int32_t grub_boot_drive;
+extern grub_int32_t EXPORT_VAR(grub_boot_drive);
 
 /* The root BIOS drive number.  */
 extern grub_int32_t grub_root_drive;
diff --git a/kern/i386/pc/init.c b/kern/i386/pc/init.c
index 7237492..757f5d5 100644
--- a/kern/i386/pc/init.c
+++ b/kern/i386/pc/init.c
@@ -22,7 +22,6 @@
 #include <grub/machine/memory.h>
 #include <grub/machine/console.h>
 #include <grub/machine/kernel.h>
-#include <grub/machine/biosdisk.h>
 #include <grub/types.h>
 #include <grub/err.h>
 #include <grub/dl.h>
@@ -77,13 +76,8 @@ make_install_device (void)
       if (grub_root_drive == 0xFF)
         grub_root_drive = grub_boot_drive;
       
-      if (grub_root_drive >= GRUB_BIOSDISK_MACHINE_CDROM_START)
-        grub_sprintf (dev, "(cd%u",
-		      grub_root_drive - GRUB_BIOSDISK_MACHINE_CDROM_START);
-      else
-        grub_sprintf (dev, "(%cd%u",
-		      (grub_root_drive & 0x80) ? 'h' : 'f',
-		      grub_root_drive & 0x7f);
+      grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
+                    grub_root_drive & 0x7f);
       
       if (grub_install_dos_part >= 0)
 	grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 1);
diff --git a/kern/i386/pc/startup.S b/kern/i386/pc/startup.S
index bf3d4ee..35b19fd 100644
--- a/kern/i386/pc/startup.S
+++ b/kern/i386/pc/startup.S
@@ -780,6 +780,17 @@ FUNCTION(grub_biosdisk_check_int13_extensions)
 
 
 /*
+ *   int grub_biosdisk_get_cdinfo_int13_extensions (int drive, void *cdrp)
+ *
+ *   Return the cdrom information of DRIVE in CDRP. If an error occurs,
+ *   then return non-zero, otherwise zero.
+ */
+
+FUNCTION(grub_biosdisk_get_cdinfo_int13_extensions)
+	movw	$0x4B01, %cx
+	jmp	1f
+
+/*
  *   int grub_biosdisk_get_diskinfo_int13_extensions (int drive, void *drp)
  *
  *   Return the geometry of DRIVE in a drive parameters, DRP. If an error
@@ -787,6 +798,8 @@ FUNCTION(grub_biosdisk_check_int13_extensions)
  */
 
 FUNCTION(grub_biosdisk_get_diskinfo_int13_extensions)
+	movb	$0x48, %ch
+1:
 	pushl	%ebp
 	pushl	%ebx
 	pushl	%esi
@@ -802,7 +815,7 @@ FUNCTION(grub_biosdisk_get_diskinfo_int13_extensions)
 	call	prot_to_real
 
 	.code16
-	movb	$0x48, %ah
+	movw	%cx, %ax
 	movw	%bx, %ds
 	int	$0x13		/* do the operation */
 	movb	%ah, %bl	/* save return value in %bl */

Re: [PATCH] cdrom access patch

[Christian Franke]

Bean wrote:
> On Wed, Mar 12, 2008 at 5:10 AM, Christian Franke
> <...> wrote:
>   
>> ...
>>  cd.diff works on this specific PC (Asus A8N, Phonix-Award BIOS), CD
>>  appears as hd31.
>>
>>  BTW: grub_biosdisk_iterate should IMO always expose the boot_drive (see
>>  my latest patch).
>>  With current CVS and cd.diff, the boot_drive would not be visible in
>>  'ls' if e.g 0x80=HDD, 0x82=boot CD (as seen on some old Laptop).
>>     
>
> Thanks for the tip, please test the new patch, it start scanning for
> cdrom if the boot drive is larger than the last harddisk.
>
>   

OK, will test this soon.

> BTW, i have a small question, if there is more than one cdrom, would
> bios export all of them, or just the boot one ?
>
>   

At least during my tests, the 2nd CD drive was never accessible. I don't 
know if this is true in general.

Different BIOS versions apparently handle CD booting different. For 
instance when booting a "floppy emulation" CD, some BIOS expose the real 
floppy drive as fd1, some don't.

Christian

Re: [PATCH] cdrom access patch

[Christian Franke]

Bean wrote:
> On Wed, Mar 12, 2008 at 5:10 AM, Christian Franke
> <...> wrote:
>   
>> Bean wrote:
>>  > ...
>>
>>     
>>  > After some testing, i think the new version has some compatible issue,
>>  > i remember Christian Franke reports that some bios doesn't support int
>>  > 42 for cdrom, so grub_biosdisk_check_int13_extensions will fail. Would
>>  > someone check if func 4b01 (the first patch cd.diff) works in such
>>  > situation ?
>>  >
>>  >
>>
>>  cd.diff works on this specific PC (Asus A8N, Phonix-Award BIOS), CD
>>  appears as hd31.
>>
>>  BTW: grub_biosdisk_iterate should IMO always expose the boot_drive (see
>>  my latest patch).
>>  With current CVS and cd.diff, the boot_drive would not be visible in
>>  'ls' if e.g 0x80=HDD, 0x82=boot CD (as seen on some old Laptop).
>>     
>
> Thanks for the tip, please test the new patch, it start scanning for
> cdrom if the boot drive is larger than the last harddisk.
>
>   

OK, also on this Laptop. CD appears as hd2 in 'ls'.

Patch is IMO good to go.

Christian

Re: [PATCH] cdrom access patch

[Bean]

On Fri, Mar 14, 2008 at 6:59 AM, Christian Franke
<Christian.Franke@t-online.de> wrote:
> Bean wrote:
>  > On Wed, Mar 12, 2008 at 5:10 AM, Christian Franke
>
> > <...> wrote:
>  >
>  >> Bean wrote:
>  >>  > ...
>  >>
>  >>
>  >>  > After some testing, i think the new version has some compatible issue,
>  >>  > i remember Christian Franke reports that some bios doesn't support int
>  >>  > 42 for cdrom, so grub_biosdisk_check_int13_extensions will fail. Would
>  >>  > someone check if func 4b01 (the first patch cd.diff) works in such
>  >>  > situation ?
>  >>  >
>  >>  >
>  >>
>  >>  cd.diff works on this specific PC (Asus A8N, Phonix-Award BIOS), CD
>  >>  appears as hd31.
>  >>
>  >>  BTW: grub_biosdisk_iterate should IMO always expose the boot_drive (see
>  >>  my latest patch).
>  >>  With current CVS and cd.diff, the boot_drive would not be visible in
>  >>  'ls' if e.g 0x80=HDD, 0x82=boot CD (as seen on some old Laptop).
>  >>
>  >
>  > Thanks for the tip, please test the new patch, it start scanning for
>  > cdrom if the boot drive is larger than the last harddisk.
>  >
>  >
>
>  OK, also on this Laptop. CD appears as hd2 in 'ls'.
>
>  Patch is IMO good to go.

thanks a lot for your testing. if nobody objects, i would like to
commit it soon.

-- 
Bean

Re: [PATCH] cdrom access patch

[Pavel Roskin]

On Fri, 2008-03-14 at 13:33 +0800, Bean wrote:

> thanks a lot for your testing. if nobody objects, i would like to
> commit it soon.

It would be great if you posted the ChangeLog entry and explained the
reason for the patch.  I checked the thread, but I don't see it.

Is it just going to affect access to the CD in the "no emulation" mode?
Or maybe you are trying to make CD-ROM accessible if booting from the
hard drive or another medium?

What is the problem you are trying to solve?  And what's wrong with the
(cd0) name?  I think it's more descriptive, and I don't care much if
some other device like LS-120 is going to get such name.

As for passing the boot driver data to multiboot kernels, I don't think
it's useful to pass data about emulated drives.  Besides, it should be
easy to keep the BIOS number in memory for every drive, even if it's
called (cd0).

Compare how Linux is changing from naming CD-ROMs by the bus location
(/dev/hdc, /dev/hdd) to naming by functionality with sequential numbers
(/dev/scd0, /dev/scd1).  Things like bus location should be transparent
to the user.

Better yet, all partitions should be identified by a unique ID, which is
what serious distros should be doing.  Still, (cd0) is more reliable and
descriptive than (hd96) for casual use.

Please note that I'm not objecting against the patch, I'm just asking
for a better explanation to understand what the patch does and why.

-- 
Regards,
Pavel Roskin

Re: [PATCH] cdrom access patch

[Bean]

On Sat, Mar 15, 2008 at 1:28 AM, Pavel Roskin <proski@gnu.org> wrote:
> On Fri, 2008-03-14 at 13:33 +0800, Bean wrote:
>
>  > thanks a lot for your testing. if nobody objects, i would like to
>  > commit it soon.
>
>  It would be great if you posted the ChangeLog entry and explained the
>  reason for the patch.  I checked the thread, but I don't see it.

Sorry, forget about the log, here is it:

2008-03-15  Bean  <bean123ch@gmail.com>

	* disk/i386/pc/biosdisk.c (cd_start): Removed.
	(cd_count): Removed.
	(grub_biosdisk_get_drive): Use (hdN) for both disk and cdrom.
	(grub_biosdisk_call_hook): Likewise.
	(grub_biosdisk_iterate): Detect cdrom device.
	(grub_biosdisk_open): Use grub_biosdisk_get_cdinfo_int13_extensions
	to detect cdrom.
	(GRUB_MOD_INIT): Don't scan cdrom device.

	* include/grub/i386/pc/biosdisk.h (GRUB_BIOSDISK_MACHINE_CDROM_START):
	Changed to 0x9f.
	(GRUB_BIOSDISK_CDTYPE_NO_EMUL): New macro.
	(GRUB_BIOSDISK_CDTYPE_1_2_M): Likewise.
	(GRUB_BIOSDISK_CDTYPE_1_44_M): Likewise.
	(GRUB_BIOSDISK_CDTYPE_2_88_M): Likewise.
	(GRUB_BIOSDISK_CDTYPE_HARDDISK): Likewise.
	(GRUB_BIOSDISK_CDTYPE_MASK): Likewise.
	(grub_biosdisk_cdrp): New structure.
	(grub_biosdisk_get_cdinfo_int13_extensions): New function.

	* include/grub/i386/pc/kernel.h (grub_boot_drive): Export this variable.

	* kern/i386/pc/init.c (make_install_device): Use (hdN) for both disk
	and cdrom.

	* kern/i386/pc/startup.S (grub_biosdisk_get_cdinfo_int13_extensions):
	New function.

>  Is it just going to affect access to the CD in the "no emulation" mode?
>  Or maybe you are trying to make CD-ROM accessible if booting from the
>  hard drive or another medium?

Yes, it only have effect when booting from "no emulation" CD. In fact,
most bios doesn't export
int 13 function if you boot from hard drive.

>  What is the problem you are trying to solve?  And what's wrong with the
>  (cd0) name?  I think it's more descriptive, and I don't care much if
>  some other device like LS-120 is going to get such name.
>

it's just that we need to map bios drive number to device name in
init.c, and create the device in biosdisk.c. In init.c, we can only
use heuristic to decide whether or not the boot device is cdrom. while
in biosdisk.c, the detection is more accurate, and it's not necessary
the same as init.c. If we force them to be the same, some cdrom drive
would appear as (cd0), some would be (hdN), the result is
inconsistent.

>  As for passing the boot driver data to multiboot kernels, I don't think
>  it's useful to pass data about emulated drives.  Besides, it should be
>  easy to keep the BIOS number in memory for every drive, even if it's
>  called (cd0).
>

Actually, many cdboot loaders need to know about the bios drive number
of emulation cdrom. such as isolinux, setupldr.bin, etc. grub2 may
support these loaders one day. in such situation, the easiest way to
pass it around is to code it in the device name. if we keep it in
biosdisk, then those loaders would need to depend on biosdisk
explicitly.

Also, the name (cd0) is a little misleading, even if there is more
than one cdrom, bios would only export the boot one. so there won't be
a (cd1).

>  Compare how Linux is changing from naming CD-ROMs by the bus location
>  (/dev/hdc, /dev/hdd) to naming by functionality with sequential numbers
>  (/dev/scd0, /dev/scd1).  Things like bus location should be transparent
>  to the user.
>
>  Better yet, all partitions should be identified by a unique ID, which is
>  what serious distros should be doing.  Still, (cd0) is more reliable and
>  descriptive than (hd96) for casual use.
>

When booting from cdrom, the root would be set properly, so users may
never need to know about the name of root device. It's only relevant
when they enter the commands manually.

-- 
Bean

Re: [PATCH] cdrom access patch

[Bean]

Hi,

I have an idea, why not have both names, (hdN) and (cd), hopefully
this will make everyone happy.

-- 
Bean

Re: [PATCH] cdrom access patch

[Vesa Jääskeläinen]

Bean wrote:
> Hi,
> 
> I have an idea, why not have both names, (hdN) and (cd), hopefully
> this will make everyone happy.
> 

Hi Bean,

I think I suggested it already ;)

Device alias is ok for me :) (and use of cd[nro] is more favorable).

Thanks,
Vesa Jääskeläinen

Re: [PATCH] cdrom access patch

[Bean]

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

On Sat, Mar 15, 2008 at 4:58 AM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
>
> Bean wrote:
>  > Hi,
>  >
>  > I have an idea, why not have both names, (hdN) and (cd), hopefully
>  > this will make everyone happy.
>  >
>
>  Hi Bean,
>
>  I think I suggested it already ;)
>
>  Device alias is ok for me :) (and use of cd[nro] is more favorable).

Ok, the new patch add (cd0) alias for cdrom device, the corresponding
(hdN) is also present.

To Christian Franke:

We can use the root variable to access the boot partition, so (boot)
seems a little duplicate.


2008-03-15  Bean  <bean123ch@gmail.com>

	* disk/i386/pc/biosdisk.c (cd_start): Removed.
	(cd_count): Removed.
	(cd_drive): New variable.
	(grub_biosdisk_get_drive): Return cd_drive if device name is (cd0).
	(grub_biosdisk_call_hook): Only handle (hdN) device name.
	(grub_biosdisk_iterate): Create (cd0) alias for cdrom device.
	(grub_biosdisk_open): Replace cd_start with cd_drive.
	(GRUB_MOD_INIT): Use grub_biosdisk_get_cdinfo_int13_extension to
	detect cdrom device.

	* include/grub/i386/pc/biosdisk.h (GRUB_BIOSDISK_MACHINE_CDROM_START):
	Removed.
	(GRUB_BIOSDISK_MACHINE_CDROM_END): Removed.
	(GRUB_BIOSDISK_CDTYPE_NO_EMUL): New macro.
	(GRUB_BIOSDISK_CDTYPE_1_2_M): Likewise.
	(GRUB_BIOSDISK_CDTYPE_1_44_M): Likewise.
	(GRUB_BIOSDISK_CDTYPE_2_88_M): Likewise.
	(GRUB_BIOSDISK_CDTYPE_HARDDISK): Likewise.
	(GRUB_BIOSDISK_CDTYPE_MASK): Likewise.
	(grub_biosdisk_cdrp): New structure.
	(grub_biosdisk_get_cdinfo_int13_extensions): New function.

	* include/grub/i386/pc/kernel.h (grub_boot_drive): Export this variable.

	* kern/i386/pc/init.c (make_install_device): Use (hdN) as root device
	name.

	* kern/i386/pc/startup.S (grub_biosdisk_get_cdinfo_int13_extensions):
	New function.

-- 
Bean

[-- Attachment #2: cd4.diff --]
[-- Type: text/plain, Size: 8064 bytes --]

diff --git a/disk/i386/pc/biosdisk.c b/disk/i386/pc/biosdisk.c
index ddcc666..2deb81d 100644
--- a/disk/i386/pc/biosdisk.c
+++ b/disk/i386/pc/biosdisk.c
@@ -18,6 +18,7 @@
 
 #include <grub/machine/biosdisk.h>
 #include <grub/machine/memory.h>
+#include <grub/machine/kernel.h>
 #include <grub/disk.h>
 #include <grub/dl.h>
 #include <grub/mm.h>
@@ -26,15 +27,17 @@
 #include <grub/err.h>
 #include <grub/term.h>
 
-static int cd_start = GRUB_BIOSDISK_MACHINE_CDROM_START;
-static int cd_count = 0;
+int cd_drive = 0;
 
 static int
 grub_biosdisk_get_drive (const char *name)
 {
   unsigned long drive;
 
-  if ((name[0] != 'f' && name[0] != 'h' && name[0] != 'c') || name[1] != 'd')
+  if (! grub_strcmp (name, "cd0"))
+    return cd_drive;
+
+  if ((name[0] != 'f' && name[0] != 'h') || name[1] != 'd')
     goto fail;
     
   drive = grub_strtoul (name + 2, 0, 10);
@@ -43,8 +46,6 @@ grub_biosdisk_get_drive (const char *name)
 
   if (name[0] == 'h')
     drive += 0x80;
-  else if (name[0] == 'c')
-    drive += cd_start;
   
   return (int) drive ;
 
@@ -58,9 +59,6 @@ grub_biosdisk_call_hook (int (*hook) (const char *name), int drive)
 {
   char name[10];
 
-  if (drive >= cd_start)
-    grub_sprintf (name, "cd%d", drive - cd_start);
-  else
     grub_sprintf (name, (drive & 0x80) ? "hd%d" : "fd%d", drive & (~0x80));
   return hook (name);
 }
@@ -91,10 +89,15 @@ grub_biosdisk_iterate (int (*hook) (const char *name))
 	return 1;
     }
 
-  for (drive = cd_start; drive < cd_start + cd_count; drive++)
-    if (grub_biosdisk_call_hook (hook, drive))
+  if (cd_drive)
+    {
+      if (grub_biosdisk_call_hook (hook, cd_drive))
       return 1;
 
+      if (hook("cd0"))
+        return 1;
+    }
+
   return 0;
 }
 
@@ -109,7 +112,7 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   if (drive < 0)
     return grub_errno;
 
-  disk->has_partitions = ((drive & 0x80) && (drive < cd_start));
+  disk->has_partitions = ((drive & 0x80) && (drive != cd_drive));
   disk->id = drive;
   
   data = (struct grub_biosdisk_data *) grub_malloc (sizeof (*data));
@@ -119,7 +122,7 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
   data->drive = drive;
   data->flags = 0;
 
-  if (drive >= cd_start)
+  if ((cd_drive) && (drive == cd_drive))
     {
       data->flags = GRUB_BIOSDISK_FLAG_LBA | GRUB_BIOSDISK_FLAG_CDROM;
       data->sectors = 32;
@@ -154,7 +157,7 @@ grub_biosdisk_open (const char *name, grub_disk_t disk)
 	}
     }
 
-  if (drive < cd_start)
+  if (! (data->flags & GRUB_BIOSDISK_FLAG_CDROM))
     {
       if (grub_biosdisk_get_diskinfo_standard (drive,
 					       &data->cylinders,
@@ -364,7 +367,8 @@ grub_disk_biosdisk_fini (void)
 
 GRUB_MOD_INIT(biosdisk)
 {
-  int drive, found = 0;
+  struct grub_biosdisk_cdrp *cdrp
+        = (struct grub_biosdisk_cdrp *) GRUB_MEMORY_MACHINE_SCRATCH_ADDR;
 
   if (grub_disk_firmware_is_tainted)
     {
@@ -373,25 +377,14 @@ GRUB_MOD_INIT(biosdisk)
     }
   grub_disk_firmware_fini = grub_disk_biosdisk_fini;
 
-  grub_disk_dev_register (&grub_biosdisk_dev);
-
-  for (drive = GRUB_BIOSDISK_MACHINE_CDROM_START;
-       drive < GRUB_BIOSDISK_MACHINE_CDROM_END; drive++)
-    {
-      if (grub_biosdisk_check_int13_extensions (drive))
-        {
-	  if (! found)
-	    cd_start = drive;
-	  found++;
-	}
-      else
-        {
-	  if (found)
-            break;
-	}
-    }
+  grub_memset (cdrp, 0, sizeof (*cdrp));
+  cdrp->size = sizeof (*cdrp);
+  if ((! grub_biosdisk_get_cdinfo_int13_extensions (grub_boot_drive, cdrp)) &&
+      ((cdrp->media_type & GRUB_BIOSDISK_CDTYPE_MASK)
+       == GRUB_BIOSDISK_CDTYPE_NO_EMUL))
+    cd_drive = cdrp->drive_no;
 
-  cd_count = found;
+  grub_disk_dev_register (&grub_biosdisk_dev);
 }
 
 GRUB_MOD_FINI(biosdisk)
diff --git a/include/grub/i386/pc/biosdisk.h b/include/grub/i386/pc/biosdisk.h
index 770f942..d1107a6 100644
--- a/include/grub/i386/pc/biosdisk.h
+++ b/include/grub/i386/pc/biosdisk.h
@@ -25,8 +25,13 @@
 #define GRUB_BIOSDISK_FLAG_LBA	1
 #define GRUB_BIOSDISK_FLAG_CDROM 2
 
-#define GRUB_BIOSDISK_MACHINE_CDROM_START	0xe0
-#define GRUB_BIOSDISK_MACHINE_CDROM_END		0xf0
+#define GRUB_BIOSDISK_CDTYPE_NO_EMUL	0
+#define GRUB_BIOSDISK_CDTYPE_1_2_M	1
+#define GRUB_BIOSDISK_CDTYPE_1_44_M	2
+#define GRUB_BIOSDISK_CDTYPE_2_88_M	3
+#define GRUB_BIOSDISK_CDTYPE_HARDDISK	4
+
+#define GRUB_BIOSDISK_CDTYPE_MASK	0xF
 
 struct grub_biosdisk_data
 {
@@ -74,6 +79,23 @@ struct grub_biosdisk_drp
   grub_uint8_t dummy[16];
 } __attribute__ ((packed));
 
+struct grub_biosdisk_cdrp
+{
+  grub_uint8_t size;
+  grub_uint8_t media_type;
+  grub_uint8_t drive_no;
+  grub_uint8_t controller_no;
+  grub_uint32_t image_lba;
+  grub_uint16_t device_spec;
+  grub_uint16_t cache_seg;
+  grub_uint16_t load_seg;
+  grub_uint16_t length_sec512;
+  grub_uint8_t cylinders;
+  grub_uint8_t sectors;
+  grub_uint8_t heads;
+  grub_uint8_t dummy[16];
+} __attribute__ ((packed));
+
 /* Disk Address Packet.  */
 struct grub_biosdisk_dap
 {
@@ -90,6 +112,8 @@ int EXPORT_FUNC(grub_biosdisk_rw_standard) (int ah, int drive, int coff, int hof
 int EXPORT_FUNC(grub_biosdisk_check_int13_extensions) (int drive);
 int EXPORT_FUNC(grub_biosdisk_get_diskinfo_int13_extensions) (int drive,
            void *drp);
+int EXPORT_FUNC(grub_biosdisk_get_cdinfo_int13_extensions) (int drive,
+           void *cdrp);
 int EXPORT_FUNC(grub_biosdisk_get_diskinfo_standard) (int drive,
 					 unsigned long *cylinders,
 					 unsigned long *heads,
diff --git a/include/grub/i386/pc/kernel.h b/include/grub/i386/pc/kernel.h
index 848dad1..43a8d5b 100644
--- a/include/grub/i386/pc/kernel.h
+++ b/include/grub/i386/pc/kernel.h
@@ -71,7 +71,7 @@ extern grub_int32_t grub_memdisk_image_size;
 extern char grub_prefix[];
 
 /* The boot BIOS drive number.  */
-extern grub_int32_t grub_boot_drive;
+extern grub_int32_t EXPORT_VAR(grub_boot_drive);
 
 /* The root BIOS drive number.  */
 extern grub_int32_t grub_root_drive;
diff --git a/kern/i386/pc/init.c b/kern/i386/pc/init.c
index 7237492..757f5d5 100644
--- a/kern/i386/pc/init.c
+++ b/kern/i386/pc/init.c
@@ -22,7 +22,6 @@
 #include <grub/machine/memory.h>
 #include <grub/machine/console.h>
 #include <grub/machine/kernel.h>
-#include <grub/machine/biosdisk.h>
 #include <grub/types.h>
 #include <grub/err.h>
 #include <grub/dl.h>
@@ -77,13 +76,8 @@ make_install_device (void)
       if (grub_root_drive == 0xFF)
         grub_root_drive = grub_boot_drive;
       
-      if (grub_root_drive >= GRUB_BIOSDISK_MACHINE_CDROM_START)
-        grub_sprintf (dev, "(cd%u",
-		      grub_root_drive - GRUB_BIOSDISK_MACHINE_CDROM_START);
-      else
-        grub_sprintf (dev, "(%cd%u",
-		      (grub_root_drive & 0x80) ? 'h' : 'f',
-		      grub_root_drive & 0x7f);
+      grub_sprintf (dev, "(%cd%u", (grub_root_drive & 0x80) ? 'h' : 'f',
+                    grub_root_drive & 0x7f);
       
       if (grub_install_dos_part >= 0)
 	grub_sprintf (dev + grub_strlen (dev), ",%u", grub_install_dos_part + 1);
diff --git a/kern/i386/pc/startup.S b/kern/i386/pc/startup.S
index bf3d4ee..35b19fd 100644
--- a/kern/i386/pc/startup.S
+++ b/kern/i386/pc/startup.S
@@ -780,6 +780,17 @@ FUNCTION(grub_biosdisk_check_int13_extensions)
 
 
 /*
+ *   int grub_biosdisk_get_cdinfo_int13_extensions (int drive, void *cdrp)
+ *
+ *   Return the cdrom information of DRIVE in CDRP. If an error occurs,
+ *   then return non-zero, otherwise zero.
+ */
+
+FUNCTION(grub_biosdisk_get_cdinfo_int13_extensions)
+	movw	$0x4B01, %cx
+	jmp	1f
+
+/*
  *   int grub_biosdisk_get_diskinfo_int13_extensions (int drive, void *drp)
  *
  *   Return the geometry of DRIVE in a drive parameters, DRP. If an error
@@ -787,6 +798,8 @@ FUNCTION(grub_biosdisk_check_int13_extensions)
  */
 
 FUNCTION(grub_biosdisk_get_diskinfo_int13_extensions)
+	movb	$0x48, %ch
+1:
 	pushl	%ebp
 	pushl	%ebx
 	pushl	%esi
@@ -802,7 +815,7 @@ FUNCTION(grub_biosdisk_get_diskinfo_int13_extensions)
 	call	prot_to_real
 
 	.code16
-	movb	$0x48, %ah
+	movw	%cx, %ax
 	movw	%bx, %ds
 	int	$0x13		/* do the operation */
 	movb	%ah, %bl	/* save return value in %bl */

Re: [PATCH] cdrom access patch

[Vesa Jääskeläinen]

Bean wrote:
> On Sat, Mar 15, 2008 at 4:58 AM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
>> Bean wrote:
>>  > Hi,
>>  >
>>  > I have an idea, why not have both names, (hdN) and (cd), hopefully
>>  > this will make everyone happy.
>>  >
>>
>>  Hi Bean,
>>
>>  I think I suggested it already ;)
>>
>>  Device alias is ok for me :) (and use of cd[nro] is more favorable).
> 
> Ok, the new patch add (cd0) alias for cdrom device, the corresponding
> (hdN) is also present.

I am just wondering that this implementation is limited to cd0, you 
cannot have cd1.

Consider following scenario:
User boots with biosdisk enabled from bootable CD, gets bootdrive from 
there assigns (cd0) for this device. User loads ata module to get access 
to more removable disk drives (cd/dvd/bdr) and then system would assign 
new device aliases like cd1, cd2 depending on how many device there is 
in system.

This could be done by something like this:

in biosdisk:
if disk.drivetype == cd then
   add_alias(ALIAS_TYPE_CD, disk)

in ata:
if disk.drivetype == cd then
   add_alias(ALIAS_TYPE_CD, disk)

Idea of this is that if there are multiple CD's you get new aliases 
identifier always incremented by one. With this you could even do following:

biosdisk:
disk.bios_device_nro = 0x0;
add_alias(ALIAS_TYPE_FLOPPY, disk) // disk becomes as alias (fd0)

disk.bios_device_nro = 0x80;
add_alias(ALIAS_TYPE_HDD, disk) // disk becomes as alias (hd0)

disk.bios_device_nro = 200;
add_alias(ALIAS_TYPE_CDROM, disk) // disk becomes as alias (cd0)

ata: // second ATA controller on system
disk.bios_device_nro = NOTSET; // -1 (?)
add_alias(ALIAS_TYPE_HDD, disk) // disk becomes as alias (hd1)

And then have a disk as a generic disk info block where there is 
function pointers to in example biosdisk or ata modules to handle reading.

In case if user installs emulation layer for in example ATA hdd in this 
scanario, then this bios_device_nro would be filled.

In here also biosdisk will be checked if it supports outputting of 
physical device information using EDD, and if it does, then it can 
filter out ata accesses for those device that bios already supports.

In this system there could be following standard names based on driver 
for devices:

(pcbios0) // floppy
(pcbios128) // hdd
(pcbios200) // cd

(ata0)
(ata2)
(atapi1)
(atapi3)

Or even more deeper with physical chain of devices like:
(pciXXXX/ata0)
(pcbios/disk128)

Just something to think about :)

Thanks,
Vesa Jääskeläinen

Re: [PATCH] cdrom access patch

[Bean]

On Sat, Mar 15, 2008 at 4:49 PM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
> Bean wrote:
>  > On Sat, Mar 15, 2008 at 4:58 AM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
>  >> Bean wrote:
>  >>  > Hi,
>  >>  >
>  >>  > I have an idea, why not have both names, (hdN) and (cd), hopefully
>  >>  > this will make everyone happy.
>  >>  >
>  >>
>  >>  Hi Bean,
>  >>
>  >>  I think I suggested it already ;)
>  >>
>  >>  Device alias is ok for me :) (and use of cd[nro] is more favorable).
>  >
>  > Ok, the new patch add (cd0) alias for cdrom device, the corresponding
>  > (hdN) is also present.
>
>  I am just wondering that this implementation is limited to cd0, you
>  cannot have cd1.
>
>  Consider following scenario:
>  User boots with biosdisk enabled from bootable CD, gets bootdrive from
>  there assigns (cd0) for this device. User loads ata module to get access
>  to more removable disk drives (cd/dvd/bdr) and then system would assign
>  new device aliases like cd1, cd2 depending on how many device there is
>  in system.
>
>  This could be done by something like this:
>
>  in biosdisk:
>  if disk.drivetype == cd then
>    add_alias(ALIAS_TYPE_CD, disk)
>
>  in ata:
>  if disk.drivetype == cd then
>    add_alias(ALIAS_TYPE_CD, disk)
>
>  Idea of this is that if there are multiple CD's you get new aliases
>  identifier always incremented by one. With this you could even do following:
>
>  biosdisk:
>  disk.bios_device_nro = 0x0;
>  add_alias(ALIAS_TYPE_FLOPPY, disk) // disk becomes as alias (fd0)
>
>  disk.bios_device_nro = 0x80;
>  add_alias(ALIAS_TYPE_HDD, disk) // disk becomes as alias (hd0)
>
>  disk.bios_device_nro = 200;
>  add_alias(ALIAS_TYPE_CDROM, disk) // disk becomes as alias (cd0)
>
>  ata: // second ATA controller on system
>  disk.bios_device_nro = NOTSET; // -1 (?)
>  add_alias(ALIAS_TYPE_HDD, disk) // disk becomes as alias (hd1)
>
>  And then have a disk as a generic disk info block where there is
>  function pointers to in example biosdisk or ata modules to handle reading.
>
>  In case if user installs emulation layer for in example ATA hdd in this
>  scanario, then this bios_device_nro would be filled.
>
>  In here also biosdisk will be checked if it supports outputting of
>  physical device information using EDD, and if it does, then it can
>  filter out ata accesses for those device that bios already supports.
>
>  In this system there could be following standard names based on driver
>  for devices:
>
>  (pcbios0) // floppy
>  (pcbios128) // hdd
>  (pcbios200) // cd
>
>  (ata0)
>  (ata2)
>  (atapi1)
>  (atapi3)
>
>  Or even more deeper with physical chain of devices like:
>  (pciXXXX/ata0)
>  (pcbios/disk128)
>
>  Just something to think about :)

This is a great idea, in fact, we don't even need to call add_alias
explicitly. We can add a new field device_type in disk, when it's
TYPE_xxx, we create the alias automatically in grub_device_iterate.

-- 
Bean

Re: [PATCH] cdrom access patch

[Christian Franke]

Vesa Jääskeläinen wrote:
> Bean wrote:
>> On Sat, Mar 15, 2008 at 4:58 AM, Vesa Jääskeläinen <chaac@nic.fi> wrote:
>>> Bean wrote:
>>>  > Hi,
>>>  >
>>>  > I have an idea, why not have both names, (hdN) and (cd), hopefully
>>>  > this will make everyone happy.
>>>  >
>>>
>>>  Hi Bean,
>>>
>>>  I think I suggested it already ;)
>>>
>>>  Device alias is ok for me :) (and use of cd[nro] is more favorable).
>>
>> Ok, the new patch add (cd0) alias for cdrom device, the corresponding
>> (hdN) is also present.
>
> I am just wondering that this implementation is limited to cd0, you 
> cannot have cd1.
>
Apparently, a (typical) BIOS does not provide access to any CD drive 
except the boot CD.
If booted from floppy, HDD, or floppy emulation CD, no CD can be accessed.

So it probably does not make sense to add cd1,... support to biosdisk.

Christian

Re: [PATCH] cdrom access patch

[Pavel Roskin]

On Sat, 2008-03-15 at 15:34 +0800, Bean wrote:

> Ok, the new patch add (cd0) alias for cdrom device, the corresponding
> (hdN) is also present.
...
> 	(grub_biosdisk_call_hook): Only handle (hdN) device name.

I still don't understand this part.  Maybe you could split your patch
into smaller parts?

Suppose we hard drives at 0x80 and 0x82 and a CD at 0x90.  We can call
grub_biosdisk_get_cdinfo_int13_extension() and figure out that 0x90 is
the boot CD, so we call it (cd0).  Then we go through the drives and
call them (hd0), (hd1).

If it's OK to have an alias, why not suppress of hide the original name?

At what point do we need to use the _name_ to calculate the BIOS number?
Why cannot we simply store that number into some field?

-- 
Regards,
Pavel Roskin

Re: [PATCH] cdrom access patch

[Bean]

On Mon, Mar 17, 2008 at 12:31 PM, Pavel Roskin <proski@gnu.org> wrote:
> On Sat, 2008-03-15 at 15:34 +0800, Bean wrote:
>
>  > Ok, the new patch add (cd0) alias for cdrom device, the corresponding
>  > (hdN) is also present.
>  ...
>
> >       (grub_biosdisk_call_hook): Only handle (hdN) device name.
>
>  I still don't understand this part.  Maybe you could split your patch
>  into smaller parts?
>
>  Suppose we hard drives at 0x80 and 0x82 and a CD at 0x90.  We can call
>  grub_biosdisk_get_cdinfo_int13_extension() and figure out that 0x90 is
>  the boot CD, so we call it (cd0).  Then we go through the drives and
>  call them (hd0), (hd1).
>
>  If it's OK to have an alias, why not suppress of hide the original name?
>
>  At what point do we need to use the _name_ to calculate the BIOS number?
>  Why cannot we simply store that number into some field?

If we use (cd0) as primary name, there are three places where the
cdrom has to been checked.

1. make_install_device in init.c.
Here, given the boot drive number grub_boot_drive, we need to figure
out the root device name. This is outside biosdisk, can't access
device or disk.

2. grub_biosdisk_iterate in biosdisk.c
We need to calculate all the disk names belong to this device, and
call the hook. No parameter is passed.

3. grub_biosdisk_open in biosdisk.c
Given a name, we need to figure out the bios number in order to
initialize the disk parameter.

4. Some boot loader
We need use the root variable to determine the bios number.

The call order is as follows, make_install_device will always be
called at init time, grub_biosdisk_open is called when we open a disk,
and grub_biosdisk_iterate is called when we iterate disk using ls or
search.

The tricky part is how to make them all consistent. In order to do so,
we have to detect cdrom and store the number as global variable
somewhere. If we put it the kernel, it's workable, but it seems odd
because this is clearly a biosdisk related function. If we put it in
biosdisk, then kernel and boot loader would have to depend on
biosdisk, then what if we want to use ata ?

Also some thoughts about generic alias handling, it's not easy to
implement. First, we need to decide where to create the alias. We
can't create it in grub_biosdisk_open, the mapping should be made
before calling grub_biosdisk_open. We can't create it in
grub_biosdisk_iterate, as it's not necessary called before
grub_biosdisk_open. It seems we can only do it in module
initialization. But then, we need to iterate disks in order to find
the cdrom, this somehow duplicate grub_biosdisk_iterate's job. Then,
We also need to consider when to release the alias.

-- 
Bean

Re: [PATCH] cdrom access patch

[Pavel Roskin]

On Mon, 2008-03-17 at 14:21 +0800, Bean wrote:

> >  At what point do we need to use the _name_ to calculate the BIOS number?
> >  Why cannot we simply store that number into some field?
> 
> If we use (cd0) as primary name, there are three places where the
> cdrom has to been checked.
> 
> 1. make_install_device in init.c.
> Here, given the boot drive number grub_boot_drive, we need to figure
> out the root device name. This is outside biosdisk, can't access
> device or disk.

OK, I see.  It looks like the names are closely tied to the BIOS
numbers.  Essentially, (fd0) is "(bios0)" and (hd0) is "(bios80)".  Yes,
we could extend that notation for "cd" devices, but it would complicate
the boot code that should be kept minimal.

I think it's OK to apply your patch, unless there are other objections.
Perhaps it's even better to apply the patch without the "cd" alias, so
that the aliases can be implemented separately in a more generic way.

-- 
Regards,
Pavel Roskin

Re: [PATCH] cdrom access patch

[Bean]

On Thu, Mar 20, 2008 at 6:09 AM, Pavel Roskin <proski@gnu.org> wrote:
>  I think it's OK to apply your patch, unless there are other objections.
>  Perhaps it's even better to apply the patch without the "cd" alias, so
>  that the aliases can be implemented separately in a more generic way.

Yes, i have similar feelings. The hard coded (cd0) alias seems like a
dirty hack to me.

-- 
Bean

Re: [PATCH] cdrom access patch

[Bean]

Committed. it's basically cd4.diff, but the (cd0) alias is not added.

-- 
Bean

Re: [PATCH] cdrom access patch

[Christian Franke]

Bean wrote:
> I have an idea, why not have both names, (hdN) and (cd), hopefully
> this will make everyone happy.
>
>   

Good idea. Or probably (boot) for the boot device, regardless of type.
The (cd) would apparently only available if GRUB was booted from this CD.

Christian

[PATCH] cdrom access patch

[Kalamatee]

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

Bean wrote ..
>This patch use int 13 func 4B01 to detect cdrom. It also change the
>name of the cd device to (hdN), where N is the bios drive number.

Ah great .. just tried it and now VMWare detects the cdrom correctly

LS now lists (fd0) (hd0) and (hd31)

though the naming does seem odd

in Qemu it also works and is listed as (hd96)

Thanks for all your great work so far .. =)

[-- Attachment #2: Type: text/html, Size: 536 bytes --]

Re: [PATCH] cdrom access patch

[Kalamatee]

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

Vesa  wrote ..
>I would prefer that CD's would be counted like (cd0) for first CD-ROM
device, (cd1) for >second CD-ROM device, ... and so on. I think it is OK to
first allocate BIOS devices for >CD's and after that use ATA driver for rest
of them. Ata driver can of course be accessed >with ata name too.

Ahh great .. yes I would prefer that also but would it cause confusion with
ls120 etc drivers?

Those funcs do sound the way to go though .. I hope Been can do something
with it =)

[-- Attachment #2: Type: text/html, Size: 647 bytes --]