[Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[Glauber Costa]

Some option roms (e1000 provided by gpxe project as an example)
are bigger than a segment. The current algorithm to compute the
checksum fails in such case. To proper compute the checksum, this
patch deals with the possibility of the rom's size crossing a
segment border.

We don't need to worry about it crossing more than one segment
border, since the option roms format only save one byte to store
the image size (thus, maximum size = 0xff = 128k = 2 segments)

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 bios/rombios.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/bios/rombios.c b/bios/rombios.c
index bc43251..6fede17 100644
--- a/bios/rombios.c
+++ b/bios/rombios.c
@@ -10162,22 +10162,43 @@ no_serial:
   ret
 
 rom_checksum:
-  push ax
-  push bx
-  push cx
+  pusha
+  push ds
+
   xor  ax, ax
   xor  bx, bx
   xor  cx, cx
+  xor  dx, dx
+
   mov  ch, [2]
   shl  cx, #1
+
+  jnc checksum_loop
+  mov  dx, cx
+  mov  cx, #0xffff
+
 checksum_loop:
   add  al, [bx]
   inc  bx
   loop checksum_loop
+
+  cmp  dx, #0
+  je checksum_out
+
+  add  al, [bx]
+  mov  cx, dx
+  mov  dx, ds
+  add  dx, #0x1000
+  mov  ds, dx
+  xor  dx, dx 
+  xor  bx, bx
+
+  jmp  checksum_loop
+
+checksum_out:
   and  al, #0xff
-  pop  cx
-  pop  bx
-  pop  ax
+  pop  ds
+  popa 
   ret
 
 
-- 
1.6.1.2

[Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[Glauber Costa]

Some option roms (e1000 provided by gpxe project as an example)
are bigger than a segment. The current algorithm to compute the
checksum fails in such case. To proper compute the checksum, this
patch deals with the possibility of the rom's size crossing a
segment border.

We don't need to worry about it crossing more than one segment
border, since the option roms format only save one byte to store
the image size (thus, maximum size = 0xff = 128k = 2 segments)

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 bios/rombios.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/bios/rombios.c b/bios/rombios.c
index bc43251..6fede17 100644
--- a/bios/rombios.c
+++ b/bios/rombios.c
@@ -10162,22 +10162,43 @@ no_serial:
   ret
 
 rom_checksum:
-  push ax
-  push bx
-  push cx
+  pusha
+  push ds
+
   xor  ax, ax
   xor  bx, bx
   xor  cx, cx
+  xor  dx, dx
+
   mov  ch, [2]
   shl  cx, #1
+
+  jnc checksum_loop
+  mov  dx, cx
+  mov  cx, #0xffff
+
 checksum_loop:
   add  al, [bx]
   inc  bx
   loop checksum_loop
+
+  cmp  dx, #0
+  je checksum_out
+
+  add  al, [bx]
+  mov  cx, dx
+  mov  dx, ds
+  add  dx, #0x1000
+  mov  ds, dx
+  xor  dx, dx 
+  xor  bx, bx
+
+  jmp  checksum_loop
+
+checksum_out:
   and  al, #0xff
-  pop  cx
-  pop  bx
-  pop  ax
+  pop  ds
+  popa 
   ret
 
 
-- 
1.6.1.2

Re: [Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[malc]

On Mon, 23 Mar 2009, Glauber Costa wrote:

> Some option roms (e1000 provided by gpxe project as an example)
> are bigger than a segment. The current algorithm to compute the
> checksum fails in such case. To proper compute the checksum, this
> patch deals with the possibility of the rom's size crossing a
> segment border.
> 
> We don't need to worry about it crossing more than one segment
> border, since the option roms format only save one byte to store
> the image size (thus, maximum size = 0xff = 128k = 2 segments)
> 
> Signed-off-by: Glauber Costa <glommer@redhat.com>
> ---
>  bios/rombios.c |   33 +++++++++++++++++++++++++++------
>  1 files changed, 27 insertions(+), 6 deletions(-)
> 
> diff --git a/bios/rombios.c b/bios/rombios.c
> index bc43251..6fede17 100644
> --- a/bios/rombios.c
> +++ b/bios/rombios.c
> @@ -10162,22 +10162,43 @@ no_serial:
>    ret
>  
>  rom_checksum:
> -  push ax
> -  push bx
> -  push cx
> +  pusha
> +  push ds
> +
>    xor  ax, ax
>    xor  bx, bx
>    xor  cx, cx
> +  xor  dx, dx
> +
>    mov  ch, [2]
>    shl  cx, #1
> +
> +  jnc checksum_loop
> +  mov  dx, cx
> +  mov  cx, #0xffff

     xchg dx, cx
     dec  cx

> +
>  checksum_loop:
>    add  al, [bx]
>    inc  bx
>    loop checksum_loop
> +
> +  cmp  dx, #0

     test dx, dx

> +  je checksum_out
> +
> +  add  al, [bx]

     DS:[0] is added twice, intended?

> +  mov  cx, dx
> +  mov  dx, ds
> +  add  dx, #0x1000

     add  dh, #0x10

> +  mov  ds, dx
> +  xor  dx, dx 
> +  xor  bx, bx
> +
> +  jmp  checksum_loop
> +
> +checksum_out:
>    and  al, #0xff
> -  pop  cx
> -  pop  bx
> -  pop  ax
> +  pop  ds
> +  popa 
>    ret
>  
>  
> 

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[Glauber Costa]

On Mon, Mar 23, 2009 at 9:48 PM, malc <av1474@comtv.ru> wrote:
> On Mon, 23 Mar 2009, Glauber Costa wrote:
>
>>    xor  ax, ax
>>    xor  bx, bx
>>    xor  cx, cx
>> +  xor  dx, dx
>> +
>>    mov  ch, [2]
>>    shl  cx, #1
>> +
>> +  jnc checksum_loop
>> +  mov  dx, cx
>> +  mov  cx, #0xffff
>
>     xchg dx, cx
>     dec  cx
>
>> +
>>  checksum_loop:
>>    add  al, [bx]
>>    inc  bx
>>    loop checksum_loop
>> +
>> +  cmp  dx, #0
>
>     test dx, dx

ok.

>
>> +  je checksum_out
>> +
>> +  add  al, [bx]
>
>     DS:[0] is added twice, intended?
it's not DS:[0]. It's the last byte from the previous segment that is
not caught by the loop
Note it happens before cleaning bx and increasing cs. It works with
it, does not without it.

>
>> +  mov  cx, dx
>> +  mov  dx, ds
>> +  add  dx, #0x1000
>
>     add  dh, #0x10

Why is it preferred?

-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1150 bytes --]

On Mon, 23 Mar 2009, Glauber Costa wrote:

> On Mon, Mar 23, 2009 at 9:48 PM, malc <av1474@comtv.ru> wrote:
> > On Mon, 23 Mar 2009, Glauber Costa wrote:
> >
> >>    xor  ax, ax
> >>    xor  bx, bx
> >>    xor  cx, cx
> >> +  xor  dx, dx
> >> +
> >>    mov  ch, [2]
> >>    shl  cx, #1
> >> +
> >> +  jnc checksum_loop
> >> +  mov  dx, cx
> >> +  mov  cx, #0xffff
> >
> >     xchg dx, cx
> >     dec  cx
> >
> >> +
> >>  checksum_loop:
> >>    add  al, [bx]
> >>    inc  bx
> >>    loop checksum_loop
> >> +
> >> +  cmp  dx, #0
> >
> >     test dx, dx
> 
> ok.
> 
> >
> >> +  je checksum_out
> >> +
> >> +  add  al, [bx]
> >
> >     DS:[0] is added twice, intended?
> it's not DS:[0]. It's the last byte from the previous segment that is
> not caught by the loop
> Note it happens before cleaning bx and increasing cs. It works with
> it, does not without it.

Yes missed that.

> >
> >> +  mov  cx, dx
> >> +  mov  dx, ds
> >> +  add  dx, #0x1000
> >
> >     add  dh, #0x10
> 
> Why is it preferred?

Same reason test and xchg/dec were proposed - takes less space.

-- 
mailto:av1474@comtv.ru

Re: [Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[Glauber Costa]

On Mon, Mar 23, 2009 at 9:48 PM, malc <av1474@comtv.ru> wrote:
> On Mon, 23 Mar 2009, Glauber Costa wrote:
>
>> Some option roms (e1000 provided by gpxe project as an example)
>> are bigger than a segment. The current algorithm to compute the
>> checksum fails in such case. To proper compute the checksum, this
>> patch deals with the possibility of the rom's size crossing a
>> segment border.
>>
>> We don't need to worry about it crossing more than one segment
>> border, since the option roms format only save one byte to store
>> the image size (thus, maximum size = 0xff = 128k = 2 segments)
>>
>> Signed-off-by: Glauber Costa <glommer@redhat.com>
>> ---
>>  bios/rombios.c |   33 +++++++++++++++++++++++++++------
>>  1 files changed, 27 insertions(+), 6 deletions(-)
>>
>> diff --git a/bios/rombios.c b/bios/rombios.c
>> index bc43251..6fede17 100644
>> --- a/bios/rombios.c
>> +++ b/bios/rombios.c
>> @@ -10162,22 +10162,43 @@ no_serial:
>>    ret
>>
>>  rom_checksum:
>> -  push ax
>> -  push bx
>> -  push cx
>> +  pusha
>> +  push ds
>> +
>>    xor  ax, ax
>>    xor  bx, bx
>>    xor  cx, cx
>> +  xor  dx, dx
>> +
>>    mov  ch, [2]
>>    shl  cx, #1
>> +
>> +  jnc checksum_loop
>> +  mov  dx, cx
>> +  mov  cx, #0xffff
>
>     xchg dx, cx
>     dec  cx
I actually skipped this one.

Ok for xchg, but cx at this time holds the remaining bytes, so dec cx won't
have the effect you think it will.


-- 
Glauber  Costa.
"Free as in Freedom"
http://glommer.net

"The less confident you are, the more serious you have to act."

Re: [Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[malc]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1835 bytes --]

On Mon, 23 Mar 2009, Glauber Costa wrote:

> On Mon, Mar 23, 2009 at 9:48 PM, malc <av1474@comtv.ru> wrote:
> > On Mon, 23 Mar 2009, Glauber Costa wrote:
> >
> >> Some option roms (e1000 provided by gpxe project as an example)
> >> are bigger than a segment. The current algorithm to compute the
> >> checksum fails in such case. To proper compute the checksum, this
> >> patch deals with the possibility of the rom's size crossing a
> >> segment border.
> >>
> >> We don't need to worry about it crossing more than one segment
> >> border, since the option roms format only save one byte to store
> >> the image size (thus, maximum size = 0xff = 128k = 2 segments)
> >>
> >> Signed-off-by: Glauber Costa <glommer@redhat.com>
> >> ---
> >>  bios/rombios.c |   33 +++++++++++++++++++++++++++------
> >>  1 files changed, 27 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/bios/rombios.c b/bios/rombios.c
> >> index bc43251..6fede17 100644
> >> --- a/bios/rombios.c
> >> +++ b/bios/rombios.c
> >> @@ -10162,22 +10162,43 @@ no_serial:
> >>    ret
> >>
> >>  rom_checksum:
> >> -  push ax
> >> -  push bx
> >> -  push cx
> >> +  pusha
> >> +  push ds
> >> +
> >>    xor  ax, ax
> >>    xor  bx, bx
> >>    xor  cx, cx
> >> +  xor  dx, dx
> >> +
> >>    mov  ch, [2]
> >>    shl  cx, #1
> >> +
> >> +  jnc checksum_loop
> >> +  mov  dx, cx
> >> +  mov  cx, #0xffff
> >
> >     xchg dx, cx
> >     dec  cx
> I actually skipped this one.
> 
> Ok for xchg, but cx at this time holds the remaining bytes, so dec cx won't
> have the effect you think it will.

Initially DX = 0, CX = remaining bytes
XCHG      CX = 0, DX = remaining bytes
DEC       CX = 0FFFFh

Unless i'm really missing something, this yields identical results with
the original code.

-- 
mailto:av1474@comtv.ru

[Qemu-devel] [PATCH] compute checksum for roms bigger than a segment

[Glauber Costa]

Some option roms (e1000 provided by gpxe project as an example)
are bigger than a segment. The current algorithm to compute the
checksum fails in such case. To proper compute the checksum, this
patch deals with the possibility of the rom's size crossing a
segment border.

We don't need to worry about it crossing more than one segment
border, since the option roms format only save one byte to store
the image size (thus, maximum size = 0xff = 128k = 2 segments)

[ including improvements suggested by malc ]

Signed-off-by: Glauber Costa <glommer@redhat.com>
---
 bios/rombios.c |   33 +++++++++++++++++++++++++++------
 1 files changed, 27 insertions(+), 6 deletions(-)

diff --git a/bios/rombios.c b/bios/rombios.c
index bc43251..5e7ad24 100644
--- a/bios/rombios.c
+++ b/bios/rombios.c
@@ -10162,22 +10162,43 @@ no_serial:
   ret
 
 rom_checksum:
-  push ax
-  push bx
-  push cx
+  pusha
+  push ds
+
   xor  ax, ax
   xor  bx, bx
   xor  cx, cx
+  xor  dx, dx
+
   mov  ch, [2]
   shl  cx, #1
+
+  jnc checksum_loop
+  xchg dx, cx
+  dec  cx
+
 checksum_loop:
   add  al, [bx]
   inc  bx
   loop checksum_loop
+
+  test dx, dx
+  je checksum_out
+
+  add  al, [bx]
+  mov  cx, dx
+  mov  dx, ds
+  add  dh, #0x10
+  mov  ds, dx
+  xor  dx, dx 
+  xor  bx, bx
+
+  jmp  checksum_loop
+
+checksum_out:
   and  al, #0xff
-  pop  cx
-  pop  bx
-  pop  ax
+  pop  ds
+  popa 
   ret
 
 
-- 
1.6.2