[PATCH] Workaround for grub_ofdisk_read()

[PATCH] Workaround for grub_ofdisk_read()

[Marcin Kurek]

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

Hell[o]

When testing grub on Pegasos 1 machine I was hit by quite weird
problem when reading partition table. It seems my friends firmware
version do not set status to 0 after grub_ieee1275_read() even if the
read was fine.

Attached patch fixed the problem.

-- 
--- Marcin 'Morgoth' Kurek ---

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grub2-ieee1275-fix-read.patch --]
[-- Type: text/x-patch; name="grub2-ieee1275-fix-read.patch", Size: 459 bytes --]

diff -urN grub2.org/disk/ieee1275/ofdisk.c grub2/disk/ieee1275/ofdisk.c
--- grub2.org/disk/ieee1275/ofdisk.c	2007-07-22 01:32:20.000000000 +0200
+++ grub2/disk/ieee1275/ofdisk.c	2007-10-07 22:55:48.000000000 +0200
@@ -124,7 +124,7 @@
 grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
 		  grub_size_t size, char *buf)
 {
-  grub_ssize_t status, actual;
+  grub_ssize_t status = 0, actual = 0;
   unsigned long long pos;
 
   grub_dprintf ("disk",

help to understand grub steps

[myfreneticvisions-grub]

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

hello everyone!
  i have a few questions about grub steps!
  i know that when i start the pc:
  1-the cpu loads the bios
  2-bios controls that every component is functional and loads the stage 1 that is stored in the MBR
  3-stage 1 loads the stage 2 that is stored somewhere in the hd
  4-stage 2 show to the user the Operating systems available and let the user select the one he wants, then it loads Kernel, initrd and other stuf related to the OS selected.
   
  these steps are right?
   
  i know that exists also a stage 1.5 placed after the bios step
   
  2-bios loads stage 1.5 when the stage 1 stored in MBR is not available (why not?), stage 1.5 is located exactly after the MBR and once loaded it reads and load the stage 2 and so on...
   
  is it right?
  tnx for your support!
  karmo

       
---------------------------------

---------------------------------
L'email della prossima generazione? Puoi averla con la nuova Yahoo! Mail

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

Re: help to understand grub steps

[Pádraig Brady]

myfreneticvisions-grub@yahoo.it wrote:
> hello everyone!
> i have a few questions about grub steps!
> i know that when i start the pc:
> 1-the cpu loads the bios
> 2-bios controls that every component is functional and loads the stage 1
> that is stored in the MBR
> 3-stage 1 loads the stage 2 that is stored somewhere in the hd
> 4-stage 2 show to the user the Operating systems available and let the
> user select the one he wants, then it loads Kernel, initrd and other
> stuf related to the OS selected.
>  
> these steps are right?
>  
> i know that exists also a stage 1.5 placed after the bios step
>  
> 2-bios loads stage 1.5 when the stage 1 stored in MBR is not available
> (why not?), stage 1.5 is located exactly after the MBR and once loaded
> it reads and load the stage 2 and so on...
>  
> is it right?

This might help:
http://www.pixelbeat.org/docs/disk/

Pádraig.

Re: [PATCH] Workaround for grub_ofdisk_read()

[Robert Millan]

On Wed, Oct 10, 2007 at 05:15:19PM +0200, Marcin Kurek wrote:
> Hell[o]
> 
> When testing grub on Pegasos 1 machine I was hit by quite weird
> problem when reading partition table. It seems my friends firmware
> version do not set status to 0 after grub_ieee1275_read() even if the
> read was fine.
> 
> Attached patch fixed the problem.
> 
> -- 
> --- Marcin 'Morgoth' Kurek ---

> diff -urN grub2.org/disk/ieee1275/ofdisk.c grub2/disk/ieee1275/ofdisk.c
> --- grub2.org/disk/ieee1275/ofdisk.c	2007-07-22 01:32:20.000000000 +0200
> +++ grub2/disk/ieee1275/ofdisk.c	2007-10-07 22:55:48.000000000 +0200
> @@ -124,7 +124,7 @@
>  grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
>  		  grub_size_t size, char *buf)
>  {
> -  grub_ssize_t status, actual;
> +  grub_ssize_t status = 0, actual = 0;
>    unsigned long long pos;
>  
>    grub_dprintf ("disk",

The "status" part seems good; then I wondered why you also set "actual" and
found...

  if (actual != actual)
    return grub_error (GRUB_ERR_READ_ERROR, "Read error on block: %llu",
                       sector);

woops. actual is never != actual!! what is this supposed to do?  If it works
like grub_ieee1275_seek, I suppose what we want is to zero it and then compare
it with 0? (instead of with itself ;-)).

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] Workaround for grub_ofdisk_read()

[Marcin Kurek]

Hell[o]


> woops. actual is never != actual!! what is this supposed to do?  If it works
> like grub_ieee1275_seek, I suppose what we want is to zero it and then compare
> it with 0? (instead of with itself ;-)).

I must be completly blind to not notice this ;(

-- 
--- Marcin 'Morgoth' Kurek ---

Re: help to understand grub steps

[myfreneticvisions-grub]

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

hi Pàdraig!
tnx for your answer! your link it's very useful!
but now i have other questions:
why does bios need to load first stage 1 in the mbr?can't it load directly stage 1.5 since it knows where is the boot filesystems?


Pádraig Brady <P@draigBrady.com> ha scritto: myfreneticvisions-grub@yahoo.it wrote:
> hello everyone!
> i have a few questions about grub steps!
> i know that when i start the pc:
> 1-the cpu loads the bios
> 2-bios controls that every component is functional and loads the stage 1
> that is stored in the MBR
> 3-stage 1 loads the stage 2 that is stored somewhere in the hd
> 4-stage 2 show to the user the Operating systems available and let the
> user select the one he wants, then it loads Kernel, initrd and other
> stuf related to the OS selected.
>  
> these steps are right?
>  
> i know that exists also a stage 1.5 placed after the bios step
>  
> 2-bios loads stage 1.5 when the stage 1 stored in MBR is not available
> (why not?), stage 1.5 is located exactly after the MBR and once loaded
> it reads and load the stage 2 and so on...
>  
> is it right?

This might help:
http://www.pixelbeat.org/docs/disk/

Pádraig.


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


       
---------------------------------

---------------------------------
L'email della prossima generazione? Puoi averla con la nuova Yahoo! Mail

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

Re: [PATCH] Workaround for grub_ofdisk_read()

[Robert Millan]

On Wed, Oct 10, 2007 at 11:35:20PM +0200, Marcin Kurek wrote:
> Hell[o]
> 
> 
> > woops. actual is never != actual!! what is this supposed to do?  If it works
> > like grub_ieee1275_seek, I suppose what we want is to zero it and then compare
> > it with 0? (instead of with itself ;-)).
> 
> I must be completly blind to not notice this ;(

What do we need to do with this one?  Perhaps you could investigate and report?

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] Workaround for grub_ofdisk_read()

[Marcin Kurek]

Hell[o]

> What do we need to do with this one?  Perhaps you could investigate and
> report?

If I remember correctly I alreay send a patch for this one ... or I
forgot ? Sorry I have some personal troubles around and only a little
free time for computers :(


-- 
--- Marcin 'Morgoth' Kurek ---

Re: [PATCH] Workaround for grub_ofdisk_read()

[Robert Millan]

On Mon, Oct 22, 2007 at 10:33:11PM +0100, Marcin Kurek wrote:
> Hell[o]
> 
> > What do we need to do with this one?  Perhaps you could investigate and
> > report?
> 
> If I remember correctly I alreay send a patch for this one ... or I
> forgot ?

You sent a patch, but the patch didn't fix the problem.  The code we have is
obviously buggy, but we don't know exactly what should it do.

-- 
Robert Millan

<GPLv2> I know my rights; I want my phone call!
<DRM> What use is a phone call, if you are unable to speak?
(as seen on /.)

Re: [PATCH] Workaround for grub_ofdisk_read()

[Marcin Kurek]

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

Hell[o]

> You sent a patch, but the patch didn't fix the problem.  The code we have is
> obviously buggy, but we don't know exactly what should it do.

Eeeek, why ? No one told me that there is any problem in it. Maybe you
talk about first one ? Attached current.

-- 
--- Marcin 'Morgoth' Kurek ---

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: grub2-ieee1275-fix-read.patch --]
[-- Type: text/x-patch; name=grub2-ieee1275-fix-read.patch, Size: 880 bytes --]

diff -urN grub2.org/disk/ieee1275/ofdisk.c grub2/disk/ieee1275/ofdisk.c
--- grub2.org/disk/ieee1275/ofdisk.c	2007-07-22 01:32:20.000000000 +0200
+++ grub2/disk/ieee1275/ofdisk.c	2007-10-15 21:45:40.998210358 +0200
@@ -124,7 +124,7 @@
 grub_ofdisk_read (grub_disk_t disk, grub_disk_addr_t sector,
 		  grub_size_t size, char *buf)
 {
-  grub_ssize_t status, actual;
+  grub_ssize_t status = 0, actual = 0;
   unsigned long long pos;
 
   grub_dprintf ("disk",
@@ -139,9 +139,10 @@
     return grub_error (GRUB_ERR_READ_ERROR,
 		       "Seek error, can't seek block %llu",
 		       sector);
+  size *= 512UL;
   grub_ieee1275_read ((grub_ieee1275_ihandle_t) disk->data, buf,
-		      size * 512UL, &actual);
-  if (actual != actual)
+		      size, &actual);
+  if (actual != size)
     return grub_error (GRUB_ERR_READ_ERROR, "Read error on block: %llu",
 		       sector);