Re: enhancing i386 mbr.S

2010-02-21 Thread Denis Doroshenko
as far as i remember, MBR code is as compact as possible, since the
space is limited.
would it be appropriate to twist some code in order to win some space?
like the following:

On 2/21/10, Giuseppe Magnotta giuseppe.magno...@gmail.com wrote:
...
 movw$NDOSPART, %cx
  +   xorw%ax, %ax
  +   xorw%bx, %bx
  +
  +test_pt:
  +   movb(%si), %al
  +   addw%ax, %bx
  +   addw$PARTSZ, %si
  +   looptest_pt
  +
  +   cmpw$0x0, %bx

if my memory serves me right, that operation takes 3 bytes, orw
%bx,%bx whould take 2 with the same result... in my days of using
assembly of 8086 it was quite common technic (like doing xor instead
of movw zero).

  +   je  no_part
  +
  +   cmpw$DOSACTIVE, %bx
  +   jne more_part
...



enhancing i386 mbr.S

2010-02-20 Thread Giuseppe Magnotta
Hi,

I would like to submit a patch that will enhance the mbr of i386
machines.

Currently, the mbr will not check it there are many bootable partition
in the partition table. It simply start the first active partition it
found.

I've made a small modification that will check the partition status
before going on. If there are no bootable partitions or there are many
bootable partition it will print an error message and stop.
I think this can be useful for people that plays with mbr and forget to
setup it correctly (like me).


A new small routine before the drive_ok label will check for it.

The original source is $OpenBSD: mbr.S,v 1.21 2007/06/25 14:10:17 tom
Exp $ fetched from 4.6 release.

I hope this can be useful...

Best Regards

Giuseppe


The patch is:

diff mbr.S.orig mbr.S:

37a38,44
 /* Copyright (c) 2010 Giuseppe Magnotta giuseppe.magno...@gmail.com
  * last edited 20 Feb 2010
  * Added the check for a single bootable partition.
  * You may use this code or fragments thereof in a manner consistent
  * with the other copyrights as long as you retain my pseudonym and
  * this copyright notice in the file.
  */
227,228c234
* Note: this should be the only active partition.  We currently
* don't check for that.
---
* Note: this should be the only active partition. We check for
that.
232a239,253
   xorw%ax, %ax
   xorw%bx, %bx
 
 test_pt:
   movb(%si), %al
   addw%ax, %bx
   addw$PARTSZ, %si
   looptest_pt
 
   cmpw$DOSACTIVE, %bx
   jne no_part
 
   movw$pt, %si
   movw$NDOSPART, %cx
 
536c557
 enoboot: .ascii   No active partition   /* runs into
crlf... */
---
 enoboot: .ascii   Error selecting bootable partition /*
runs into crlf... */



Re: enhancing i386 mbr.S

2010-02-20 Thread Han Boetes
Giuseppe Magnotta wrote:
 I would like to submit a patch that will enhance the mbr of i386
 machines.

Could you be so kind as to provide us with a unified patch? You
can make one with: cvs diff -u

Thanks!


# Han



Re: enhancing i386 mbr.S

2010-02-20 Thread Tobias Weingartner
On Saturday, February 20, Giuseppe Magnotta wrote:
 
 The original source is $OpenBSD: mbr.S,v 1.21 2007/06/25 14:10:17 tom
 Exp $ fetched from 4.6 release.
 
 I hope this can be useful...
 
 Best Regards
 
 Giuseppe
 
 
 The patch is:
 
 diff mbr.S.orig mbr.S:

Ugh... please send 'diff -u' instead.

-Toby.



Re: enhancing i386 mbr.S

2010-02-20 Thread Tobias Weingartner
I like it... for the most part.

On Saturday, February 20, Giuseppe Magnotta wrote:
 Index: mbr.S
 ===
 RCS file: /cvs/src/sys/arch/i386/stand/mbr/mbr.S,v
 retrieving revision 1.21
 diff -u -r1.21 mbr.S
 --- mbr.S 25 Jun 2007 14:10:17 -  1.21
 +++ mbr.S 20 Feb 2010 15:40:04 -
 @@ -35,6 +35,13 @@
   * with the other copyrights as long as you retain my pseudonym and
   * this copyright notice in the file.
   */
 +/* Copyright (c) 2010 Giuseppe Magnotta giuseppe.magno...@gmail.com
 + * last edited 20 Feb 2010
 + * Added the check for a single bootable partition.
 + * You may use this code or fragments thereof in a manner consistent
 + * with the other copyrights as long as you retain my pseudonym and
 + * this copyright notice in the file.
 + */

This can be achieved by simply adding your Copyright (C) line right
below the other copyright lines in the file.  The extra text is not
necessary, or contains comments that should be in the commit message.


   .file   mbr.S
  
 @@ -224,12 +231,26 @@
  
  drive_ok:
   /* Find the first active partition.
 -  * Note: this should be the only active partition.  We currently
 -  * don't check for that.
 +  * Note: this should be the only active partition. We check for that.

This can be shorter.  Find first and check for only active partition.
Thanks for updating comments to reflect reality.

*/
   movw$pt, %si
  
   movw$NDOSPART, %cx
 + xorw%ax, %ax
 + xorw%bx, %bx
 +
 +test_pt:
 + movb(%si), %al
 + addw%ax, %bx
 + addw$PARTSZ, %si
 + looptest_pt
 +
 + cmpw$DOSACTIVE, %bx
 + jne no_part
 +
 + movw$pt, %si
 + movw$NDOSPART, %cx
 +

Nice way to do this.  When I wrote this I must have been out
to lunch, considering the simple way to do this.

  find_active:
   DBGMSG(CHAR_L)
   movb(%si), %al
 @@ -533,7 +554,7 @@
  efdmbr:  .asciz  MBR on floppy or old BIOS\r\n
  eread:   .asciz  \r\nRead error\r\n
  enoos:   .asciz  No O/S\r\n
 -enoboot: .ascii  No active partition   /* runs into crlf... */
 +enoboot: .ascii  Error selecting bootable partition /* runs in
 to crlf... */
  crlf:.asciz  \r\n

I wish there was a shorter way to indicate this.  Some MBRs I know simply
say Corrupt Partition Table, which might be good enough and a bit shorter.


Are you ok with the modification to your copyright/comment statement above?

--Toby.



Re: enhancing i386 mbr.S

2010-02-20 Thread Ted Unangst
On Sat, Feb 20, 2010 at 8:29 AM, Giuseppe Magnotta
giuseppe.magno...@gmail.com wrote:
 Currently, the mbr will not check it there are many bootable partition
 in the partition table. It simply start the first active partition it
 found.

 I've made a small modification that will check the partition status
 before going on. If there are no bootable partitions or there are many
 bootable partition it will print an error message and stop.

Is this really an improvement?  If I have two bootable partitions, at
least one of them will boot now, letting me fix the problem.  If you
refuse to boot, now I need to dig around in my toy box for a floppy
drive or something before I can fix it.  I would prefer booting into
the wrong OS over not booting any day.



Re: enhancing i386 mbr.S

2010-02-20 Thread Tobias Weingartner
On Saturday, February 20, Ted Unangst wrote:
 
 Is this really an improvement?  If I have two bootable partitions, at
 least one of them will boot now, letting me fix the problem.  If you
 refuse to boot, now I need to dig around in my toy box for a floppy
 drive or something before I can fix it.  I would prefer booting into
 the wrong OS over not booting any day.

If that is the case, why not take the check for 55AA out as well?
It would be better to complain about malformed/corrupted MBR's,
rather than silently continuing.  The question that would arise
for me immediately, is how did it get messed up?  Disk corruption?
Or something else?

--Toby.



Re: enhancing i386 mbr.S

2010-02-20 Thread Giuseppe Magnotta
Tobias Weingartner wrote:
 Possibly.  Is your real name Guiseppe Magnotta?
 
 --Toby.

My real name is Giuseppe Magnotta.

Giuseppe



Re: enhancing i386 mbr.S

2010-02-20 Thread Miod Vallat
  Is this really an improvement?  If I have two bootable partitions, at
  least one of them will boot now, letting me fix the problem.  If you
  refuse to boot, now I need to dig around in my toy box for a floppy
  drive or something before I can fix it.  I would prefer booting into
  the wrong OS over not booting any day.
 
 If that is the case, why not take the check for 55AA out as well?
 It would be better to complain about malformed/corrupted MBR's,
 rather than silently continuing.  The question that would arise
 for me immediately, is how did it get messed up?  Disk corruption?
 Or something else?

There's a huge difference between garbage in the mbr, and user error
causing two partitions to be marked as active.

Miod



Re: enhancing i386 mbr.S

2010-02-20 Thread Giuseppe Magnotta
Ted Unangst wrote:
 Is this really an improvement?  If I have two bootable partitions, at
 least one of them will boot now, letting me fix the problem.  If you
 refuse to boot, now I need to dig around in my toy box for a floppy
 drive or something before I can fix it.  I would prefer booting into
 the wrong OS over not booting any day.

Hi Ted.

In the (strange) case you have many bootable partition do you think is
safe to boot in the first active one?

If your answer is yes, please find attached a new patch that in case
there are many bootable partition then print on screen Corrupted
partition table and boots the first bootable partition it find
otherwise if there are no bootable partition will print the same error
and block (as original code did).

What do you think about this?

Regards,

Giuseppe
? diff
? mbr
Index: mbr.S
===
RCS file: /cvs/src/sys/arch/i386/stand/mbr/mbr.S,v
retrieving revision 1.21
diff -u -r1.21 mbr.S
--- mbr.S   25 Jun 2007 14:10:17 -  1.21
+++ mbr.S   20 Feb 2010 20:27:26 -
@@ -3,6 +3,7 @@
 /*
  * Copyright (c) 1997 Michael Shalayeff and Tobias Weingartner
  * Copyright (c) 2003 Tom Cosgrove tom.cosgr...@arches-consulting.com
+ * Copyright (c) 2010 Giuseppe Magnotta giuseppe.magno...@gmail.com
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -223,13 +224,25 @@
movb$0x80, %dl
 
 drive_ok:
-   /* Find the first active partition.
-* Note: this should be the only active partition.  We currently
-* don't check for that.
-*/
+   /* Find first and check for only active partition. */
movw$pt, %si
 
movw$NDOSPART, %cx
+   xorw%ax, %ax
+   xorw%bx, %bx
+
+test_pt:
+   movb(%si), %al
+   addw%ax, %bx
+   addw$PARTSZ, %si
+   looptest_pt
+
+   cmpw$0x0, %bx
+   je  no_part
+
+   cmpw$DOSACTIVE, %bx
+   jg  more_part
+
 find_active:
DBGMSG(CHAR_L)
movb(%si), %al
@@ -253,6 +266,16 @@
/* Just to make sure */
jmp stay_stopped
 
+more_part:
+   movw$enoboot, %si
+
+   callLmessage
+
+   movw$pt, %si
+   movw$NDOSPART, %cx
+
+   jmp find_active
+
 found:
/*
 * Found bootable partition
@@ -533,7 +556,7 @@
 efdmbr:.asciz  MBR on floppy or old BIOS\r\n
 eread: .asciz  \r\nRead error\r\n
 enoos: .asciz  No O/S\r\n
-enoboot: .asciiNo active partition   /* runs into crlf... */
+enoboot: .asciiCorrupted partition table /* runs into 
crlf... */
 crlf:  .asciz  \r\n
 
 endofcode:



Re: enhancing i386 mbr.S

2010-02-20 Thread Tobias Weingartner
On Saturday, February 20, Miod Vallat wrote:
 
 There's a huge difference between garbage in the mbr, and user error
 causing two partitions to be marked as active.

From the point of view of the MBR, not really.  They're both
corruption.  One just happens to be more likely to be survivable,
but since we have no checksums/etc to doublecheck the validity
of the MBR, they both amount to garbage.  One just happens to be
a tad more palatable.

--Toby.



Re: enhancing i386 mbr.S

2010-02-20 Thread Tobias Weingartner
On Saturday, February 20, Kenneth R Westerback wrote:
 
 I vote with Ted. Booting, even the wrong partition, seems better to me
 than not booting anything.

The MBR is not really supposed to boot if it is corrupted.  There
are plenty of MBR codes out there that check for this condition,
and will abort if it exists.  Even DOS33 did so.

In the end, it's a bikeshed, and everyone will have an opinion...

--Toby.



Re: enhancing i386 mbr.S

2010-02-20 Thread Kenneth R Westerback
On Sat, Feb 20, 2010 at 06:44:41PM -0700, Tobias Weingartner wrote:
 On Saturday, February 20, Kenneth R Westerback wrote:
  
  I vote with Ted. Booting, even the wrong partition, seems better to me
  than not booting anything.
 
 The MBR is not really supposed to boot if it is corrupted.  There
 are plenty of MBR codes out there that check for this condition,
 and will abort if it exists.  Even DOS33 did so.
 
 In the end, it's a bikeshed, and everyone will have an opinion...

No argument there. :-).

 Ken

 
 --Toby.



Re: enhancing i386 mbr.S

2010-02-20 Thread Ted Unangst
On Sat, Feb 20, 2010 at 3:33 PM, Giuseppe Magnotta
giuseppe.magno...@gmail.com wrote:
 In the (strange) case you have many bootable partition do you think is
 safe to boot in the first active one?

 If your answer is yes, please find attached a new patch that in case
 there are many bootable partition then print on screen Corrupted
 partition table and boots the first bootable partition it find
 otherwise if there are no bootable partition will print the same error
 and block (as original code did).

 What do you think about this?

I can't comment on the diff itself, but this seems like a fine idea.