Pacth: Freeldr disk geometry bug

[Eric Kohl]

Hi!

This patch fixes the disk geometry bug I mentioned in my previous mail. 
It enables disk.sys and usetup.exe to use the same disk geometry that is 
used by the BIOS.

Due to another bug or workaround in the 1st stage setup, the 2nd stage 
setup will not boot at all.

Regards,
Eric

Index: boot/freeldr/freeldr/arch/i386/hardware.c
===================================================================
--- boot/freeldr/freeldr/arch/i386/hardware.c	(revision 48722)
+++ boot/freeldr/freeldr/arch/i386/hardware.c	(working copy)
@@ -327,7 +327,6 @@
 {
   PCM_PARTIAL_RESOURCE_LIST PartialResourceList;
   PCM_DISK_GEOMETRY_DEVICE_DATA DiskGeometry;
-  EXTENDED_GEOMETRY ExtGeometry;
   GEOMETRY Geometry;
   ULONG Size;

@@ -362,16 +361,8 @@
   DiskGeometry = (PVOID)(((ULONG_PTR)PartialResourceList) + sizeof(CM_PARTIAL_RESOURCE_LIST));

/* Get the disk geometry */
-  ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
-  if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry, ExtGeometry.Size))
+  if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
     {
-      DiskGeometry->BytesPerSector = ExtGeometry.BytesPerSector;
-      DiskGeometry->NumberOfCylinders = ExtGeometry.Cylinders;
-      DiskGeometry->SectorsPerTrack = ExtGeometry.SectorsPerTrack;
-      DiskGeometry->NumberOfHeads = ExtGeometry.Heads;
-    }
-  else if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
-    {
       DiskGeometry->BytesPerSector = Geometry.BytesPerSector;
       DiskGeometry->NumberOfCylinders = Geometry.Cylinders;
       DiskGeometry->SectorsPerTrack = Geometry.Sectors;
Index: boot/freeldr/freeldr/arch/i386/pcdisk.c
===================================================================
--- boot/freeldr/freeldr/arch/i386/pcdisk.c	(revision 48722)
+++ boot/freeldr/freeldr/arch/i386/pcdisk.c	(working copy)
@@ -296,12 +296,25 @@
 BOOLEAN
 PcDiskGetDriveGeometry(ULONG DriveNumber, PGEOMETRY Geometry)
 {
+  EXTENDED_GEOMETRY ExtGeometry;
   REGS RegsIn;
   REGS RegsOut;
   ULONG Cylinders;

DPRINTM(DPRINT_DISK, "DiskGetDriveGeometry()\n");

+  /* Try to get the extended geometry first */
+  ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
+  if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry, ExtGeometry.Size))
+  {
+    Geometry->Cylinders = ExtGeometry.Cylinders;
+    Geometry->Heads = ExtGeometry.Heads;
+    Geometry->Sectors = ExtGeometry.SectorsPerTrack;
+    Geometry->BytesPerSector = ExtGeometry.BytesPerSector;
+
+    return TRUE;
+  }
+
   /* BIOS Int 13h, function 08h - Get drive parameters
    * AH = 08h
    * DL = drive (bit 7 set for hard disk)

[Javier Agustìn Fernàndez Arroyo]

i bet it would be better for fixing this that this patch were committed in
main trunk.... ;)

On Thu, Sep 9, 2010 at 3:44 PM, Eric Kohl [e-mail] wrote:
> Hi!
>
> This patch fixes the disk geometry bug I mentioned in my previous mail. It
> enables disk.sys and usetup.exe to use the same disk geometry that is used
> by the BIOS.
>
> Due to another bug or workaround in the 1st stage setup, the 2nd stage
> setup will not boot at all.
>
> Regards,
> Eric
> Index: boot/freeldr/freeldr/arch/i386/hardware.c
>
> --- boot/freeldr/freeldr/arch/i386/hardware.c   (revision 48722)
> +++ boot/freeldr/freeldr/arch/i386/hardware.c   (working copy)
> @@ -327,7 +327,6 @@
>  {
>   PCM_PARTIAL_RESOURCE_LIST PartialResourceList;
>   PCM_DISK_GEOMETRY_DEVICE_DATA DiskGeometry;
> EXTENDED_GEOMETRY ExtGeometry;
> GEOMETRY Geometry;
> ULONG Size;
>
> @@ -362,16 +361,8 @@
>   DiskGeometry = (PVOID)(((ULONG_PTR)PartialResourceList) +
> sizeof(CM_PARTIAL_RESOURCE_LIST));
>
> /* Get the disk geometry */
> ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
> if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry,
>
> ExtGeometry.Size))
> if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
>  {
>  DiskGeometry->BytesPerSector = ExtGeometry.BytesPerSector;
>  DiskGeometry->NumberOfCylinders = ExtGeometry.Cylinders;
>  DiskGeometry->SectorsPerTrack = ExtGeometry.SectorsPerTrack;
>  DiskGeometry->NumberOfHeads = ExtGeometry.Heads;
> }
> else if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
> {
>  DiskGeometry->BytesPerSector = Geometry.BytesPerSector;
>  DiskGeometry->NumberOfCylinders = Geometry.Cylinders;
>  DiskGeometry->SectorsPerTrack = Geometry.Sectors;
> Index: boot/freeldr/freeldr/arch/i386/pcdisk.c
>
> --- boot/freeldr/freeldr/arch/i386/pcdisk.c     (revision 48722)
> +++ boot/freeldr/freeldr/arch/i386/pcdisk.c     (working copy)
> @@ -296,12 +296,25 @@
>  BOOLEAN
>  PcDiskGetDriveGeometry(ULONG DriveNumber, PGEOMETRY Geometry)
>  {
>
> EXTENDED_GEOMETRY ExtGeometry;
> REGS RegsIn;
> REGS RegsOut;
> ULONG Cylinders;
>
> DPRINTM(DPRINT_DISK, "DiskGetDriveGeometry()\n");
>
> /* Try to get the extended geometry first */
>
> ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
>
> if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry,
>
> ExtGeometry.Size))
> {
> Geometry->Cylinders = ExtGeometry.Cylinders;
> Geometry->Heads = ExtGeometry.Heads;
> Geometry->Sectors = ExtGeometry.SectorsPerTrack;
> Geometry->BytesPerSector = ExtGeometry.BytesPerSector;
> return TRUE;
> }
> /* BIOS Int 13h, function 08h - Get drive parameters
> AH = 08h
> DL = drive (bit 7 set for hard disk)
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Pierre Schweitzer]

Hi!
Could you define how/why 2nd stage doesn't boot on your side?
With my rewritten ARC names handling, your patch brokes absolutely nothing. I just successfully installed ReactOS.
Let me know if you want a patch to test your issue on your side.
Best regards,
P. Schweitzer
> Hi!
>
> This patch fixes the disk geometry bug I mentioned in my previous mail. 
> It enables disk.sys and usetup.exe to use the same disk geometry that is 
> used by the BIOS.
>
> Due to another bug or workaround in the 1st stage setup, the 2nd stage 
> setup will not boot at all.
>
> Regards,
> Eric
> Index: boot/freeldr/freeldr/arch/i386/hardware.c
> ===================================================================
> --- boot/freeldr/freeldr/arch/i386/hardware.c	(revision 48722)
> +++ boot/freeldr/freeldr/arch/i386/hardware.c	(working copy)
> @@ -327,7 +327,6 @@
>  {
>    PCM_PARTIAL_RESOURCE_LIST PartialResourceList;
>    PCM_DISK_GEOMETRY_DEVICE_DATA DiskGeometry;
> EXTENDED_GEOMETRY ExtGeometry;
> GEOMETRY Geometry;
> ULONG Size;
>
> @@ -362,16 +361,8 @@
>    DiskGeometry = (PVOID)(((ULONG_PTR)PartialResourceList) + sizeof(CM_PARTIAL_RESOURCE_LIST));
>
> /* Get the disk geometry */
> ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
> if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry, ExtGeometry.Size))
> if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
>   {
>  DiskGeometry->BytesPerSector = ExtGeometry.BytesPerSector;
>  DiskGeometry->NumberOfCylinders = ExtGeometry.Cylinders;
>  DiskGeometry->SectorsPerTrack = ExtGeometry.SectorsPerTrack;
>  DiskGeometry->NumberOfHeads = ExtGeometry.Heads;
> }
> else if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
> {
>   DiskGeometry->BytesPerSector = Geometry.BytesPerSector;
>   DiskGeometry->NumberOfCylinders = Geometry.Cylinders;
>   DiskGeometry->SectorsPerTrack = Geometry.Sectors;
> Index: boot/freeldr/freeldr/arch/i386/pcdisk.c
>
> --- boot/freeldr/freeldr/arch/i386/pcdisk.c	(revision 48722)
> +++ boot/freeldr/freeldr/arch/i386/pcdisk.c	(working copy)
> @@ -296,12 +296,25 @@
>  BOOLEAN
>  PcDiskGetDriveGeometry(ULONG DriveNumber, PGEOMETRY Geometry)
>  {
>
> EXTENDED_GEOMETRY ExtGeometry;
> REGS RegsIn;
> REGS RegsOut;
> ULONG Cylinders;
>
> DPRINTM(DPRINT_DISK, "DiskGetDriveGeometry()\n");
>
> /* Try to get the extended geometry first */
>
> ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
>
> if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry, ExtGeometry.Size))
>
> {
>
> Geometry->Cylinders = ExtGeometry.Cylinders;
>
> Geometry->Heads = ExtGeometry.Heads;
>
> Geometry->Sectors = ExtGeometry.SectorsPerTrack;
>
> Geometry->BytesPerSector = ExtGeometry.BytesPerSector;
>
> return TRUE;
>
> }
>
> /* BIOS Int 13h, function 08h - Get drive parameters
> AH = 08h
> DL = drive (bit 7 set for hard disk)
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Roel Messiant]

Hey,

If he experiences the same thing as I am, 3 different scenarios are happening:

- black screen at boot, nothing happens (CPU not idle)
- booting gets to the point where "loading freeloader" gets printed
- no boot problems at all (got it once in 10 tries)

WBR,

Roel Messiant

2010/9/9 Pierre Schweitzer [e-mail]:
> Hi!
> Could you define how/why 2nd stage doesn't boot on your side?
> With my rewritten ARC names handling, your patch brokes absolutely nothing. I just successfully installed ReactOS.
> Let me know if you want a patch to test your issue on your side.
> Best regards,
> P. Schweitzer
> > Hi!
> >
> > This patch fixes the disk geometry bug I mentioned in my previous mail.
> > It enables disk.sys and usetup.exe to use the same disk geometry that is
> > used by the BIOS.
> >
> > Due to another bug or workaround in the 1st stage setup, the 2nd stage
> > setup will not boot at all.
> >
> > Regards,
> > Eric
> > Index: boot/freeldr/freeldr/arch/i386/hardware.c
> > ===================================================================
> > --- boot/freeldr/freeldr/arch/i386/hardware.c (revision 48722)
> > +++ boot/freeldr/freeldr/arch/i386/hardware.c (working copy)
> > @@ -327,7 +327,6 @@
> >  {
> >    PCM_PARTIAL_RESOURCE_LIST PartialResourceList;
> >    PCM_DISK_GEOMETRY_DEVICE_DATA DiskGeometry;
> > EXTENDED_GEOMETRY ExtGeometry;
> >
> > GEOMETRY Geometry;
> >    ULONG Size;
> >
> > @@ -362,16 +361,8 @@
> >    DiskGeometry = (PVOID)(((ULONG_PTR)PartialResourceList) + sizeof(CM_PARTIAL_RESOURCE_LIST));
> >
> > /* Get the disk geometry */
> > ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
> > if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry, ExtGeometry.Size))
> > if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
> >
> > {
> > DiskGeometry->BytesPerSector = ExtGeometry.BytesPerSector;
> > DiskGeometry->NumberOfCylinders = ExtGeometry.Cylinders;
> > DiskGeometry->SectorsPerTrack = ExtGeometry.SectorsPerTrack;
> > DiskGeometry->NumberOfHeads = ExtGeometry.Heads;
> > }
> > else if(MachDiskGetDriveGeometry(DriveNumber, &Geometry))
> > {
> >
> > DiskGeometry->BytesPerSector = Geometry.BytesPerSector;
> >        DiskGeometry->NumberOfCylinders = Geometry.Cylinders;
> >        DiskGeometry->SectorsPerTrack = Geometry.Sectors;
> > Index: boot/freeldr/freeldr/arch/i386/pcdisk.c
> > ===================================================================
> > --- boot/freeldr/freeldr/arch/i386/pcdisk.c   (revision 48722)
> > +++ boot/freeldr/freeldr/arch/i386/pcdisk.c   (working copy)
> > @@ -296,12 +296,25 @@
> >  BOOLEAN
> >  PcDiskGetDriveGeometry(ULONG DriveNumber, PGEOMETRY Geometry)
> >  {
> > EXTENDED_GEOMETRY ExtGeometry;
> >
> > REGS RegsIn;
> >    REGS RegsOut;
> >    ULONG Cylinders;
> >
> > DPRINTM(DPRINT_DISK, "DiskGetDriveGeometry()\n");
> > /* Try to get the extended geometry first */
> > ExtGeometry.Size = sizeof(EXTENDED_GEOMETRY);
> > if (DiskGetExtendedDriveParameters(DriveNumber, &ExtGeometry, ExtGeometry.Size))
> > {
> > Geometry->Cylinders = ExtGeometry.Cylinders;
> > Geometry->Heads = ExtGeometry.Heads;
> > Geometry->Sectors = ExtGeometry.SectorsPerTrack;
> > Geometry->BytesPerSector = ExtGeometry.BytesPerSector;
> > return TRUE;
> > }
> >
> > /* BIOS Int 13h, function 08h - Get drive parameters
> >     * AH = 08h
> >     * DL = drive (bit 7 set for hard disk)
> > _______________________________________________
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Eric Kohl]

Hi Roel,

I ran a few more tests and the result is:
- A single partition does not cause any failure. ReactOS boots without 
any error.
- Multiple partitions: Screen stays black.

Regards,
Eric
> Hey,
>
> If he experiences the same thing as I am, 3 different scenarios are happening:
> black screen at boot, nothing happens (CPU not idle)
> booting gets to the point where "loading freeloader" gets printed
> no boot problems at all (got it once in 10 tries)
>
> WBR,
>
> Roel Messiant

[Sylvain Petreolle]

Hello Eric,

I successfully tested your patch with Vmware, Vbox and Qemu-kvm.
Hdd sizes tested : 400MB (Qemu), 1.5 GB and 3GB (all three for these 2)
Each one of these 7 tests installed and goes to 3rd stage.

Kind regards,
Sylvain Petreolle

----- Message d'origine ----
> De : Eric Kohl [e-mail]
> À : ReactOS Development List [e-mail]
> Envoyé le : Jeu 9 septembre 2010, 15h 44min 04s
> Objet : [ros-dev] Pacth: Freeldr disk geometry bug
>
> Hi!
>
> This patch fixes the disk geometry bug I mentioned in my previous  mail. It 
> enables disk.sys and usetup.exe to use the same disk geometry that is  used by 
> the BIOS.
>
> Due to another bug or workaround in the 1st stage  setup, the 2nd stage setup 
> will not boot at all.
>
> Regards,
> Eric

[Eric Kohl]

Hello Sylvain,

could you please try to create two partitions on a disk? As far as I can 
see, the 2nd stage setup should freeze with a black screen.

Thanks,
Eric
> Hello Eric,
>
> I successfully tested your patch with Vmware, Vbox and Qemu-kvm.
> Hdd sizes tested : 400MB (Qemu), 1.5 GB and 3GB (all three for these 2)
> Each one of these 7 tests installed and goes to 3rd stage.
>
> Kind regards,
> Sylvain Petreolle

[Pierre Schweitzer]

Hi,
I just rerun a testcase using two 1024MB partitions on a 2GB disk. Both were formatted to FAT32 using UBCD. Install worked (in spite of an error message when displaying partitions list), 2nd stage as well, and finally 3rd stage too.
Perhaps is that due to my changes, I dunno. Could you try to get a backtrace when screen turns black?

On more "general" remark, with ACPI enabled (uncommented #define ENABLE_ACPI in pnpmgr.c) 1st can't even reach language selection page when they are several partitions on a disk.

WBR
> Hello Sylvain,
>
> could you please try to create two partitions on a disk? As far as I can 
> see, the 2nd stage setup should freeze with a black screen.
>
> Thanks,
> Eric
> > Hello Eric,
> >
> > I successfully tested your patch with Vmware, Vbox and Qemu-kvm.
> > Hdd sizes tested : 400MB (Qemu), 1.5 GB and 3GB (all three for these 2)
> > Each one of these 7 tests installed and goes to 3rd stage.
> >
> > Kind regards,
> > Sylvain Petreolle
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Sylvain Petreolle]

Got to the same result here, bootcd manages to install and boot to 3rd stage 
using another boot disk to create partitions.

How did you create the 2nd partition ? Usetup adds an erroneous 3rd partition 
when creating a 2nd one.

Kind regards,
Sylvain Petreolle

----- Message d'origine ----
> De : Pierre Schweitzer [e-mail]
> À : ReactOS Development List [e-mail]
> Envoyé le : Jeu 9 septembre 2010, 23h 24min 59s
> Objet : Re: [ros-dev] Re :  Pacth: Freeldr disk geometry bug
>
> Hi,
> I just rerun a testcase using two 1024MB partitions on a 2GB disk. Both  were 
> formatted to FAT32 using UBCD. Install worked (in spite of an error message  
> when displaying partitions list), 2nd stage as well, and finally 3rd stage  too.
> Perhaps is that due to my changes, I dunno. Could you try to get a  backtrace 
> when screen turns black?
>
> On more "general" remark, with ACPI  enabled (uncommented #define ENABLE_ACPI 
> in pnpmgr.c) 1st can't even reach  language selection page when they are several 
> partitions on a  disk.
>
> WBR
> > Hello Sylvain,
> >
> > could you please  try to create two partitions on a disk? As far as I can 
> > see, the 2nd  stage setup should freeze with a black screen.
> >
> > Thanks,
> >  Eric
> > > Hello Eric,
> > >
> > > I successfully  tested your patch with Vmware, Vbox and Qemu-kvm.
> > > Hdd sizes tested  : 400MB (Qemu), 1.5 GB and 3GB (all three for these 2)
> > > Each one of  these 7 tests installed and goes to 3rd stage.
> > >
> > > Kind regards,
> > > Sylvain Petreolle
> >
> > Ros-dev mailing  list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev  mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev