Re: Disk unit mixup for bootdev in i386_dkcsum.c::dkcsumattach() - BUG & FIX

["Fred de Jong" <kzfti@home.nl>]

OpenBSD/i386 3.5 RELEASE BOOT 2.06

Got a final kernel quickfix to solve the bootdev mixup problem
for "config root swap generic" autoconfiguration RELEASEs
om multi-disk i386 machines.

This change to dkcsum.c tries to account for most of the
following combinations:

1. different System BIOS boot setups
2. different BOOTblock versions, without depending
     on specific BOOTARG_APIVER arguments
3. boxes with mixed SCSI and IDE disks on different
     channels, different controllers and
     different system (PCI, ISA) buses

Since I can only test this on a machine with 3 IDE
disks on 3 IDE-channels on 2 PCI buses, it would be nice
if someone with (additional) SCSI disks could test
this patch too. I expect this to solve many of the reported
problems with generic auto-configured booting on
systems with add-on diskcontrollers.

Maybe the development team could weigh this for
possible inclusion in future releases. Todd ?

Anyway, this patch fixed my problem with following setup,
where OBSD is installed on the wd0 disk (attached to
a Promise Ultra IDE PCI add-on card), and the loaded
/bsd would fail to mount root on wd0a without it:

dkcsum: wd0 matched BIOS disk 82
wd1: no disk label
dkcsum: wd1 matched BIOS disk 80
wd2: no disk label
dkcsum: wd2 matched BIOS disk 81

Here's my diff for
sys/arch/i386/i386/dkcsum.c

*** dkcsum.c.ORG	Tue Jun  3 22:31:07 2003
--- dkcsum.c	Fri Jun 18 10:23:37 2004
***************
*** 62,72 ****
--- 62,79 ----
  	int error;
  	u_int32_t csum;
  	bios_diskinfo_t *bdi, *hit;
+ 	dev_t bootsbootdev;
+ 	dev_t altbootdev;
+ 	int isbootdevfixed;
  
  	/* do nothing if no diskinfo passed from /boot, or a bad length */
  	if (bios_diskinfo == NULL || bios_cksumlen * DEV_BSIZE > MAXBSIZE)
  		return;
  
+ 	bootsbootdev = bootdev;
+ 	altbootdev = 0;
+ 	isbootdevfixed = 0;
+ 
  	/*
  	 * XXX Whatif DEV_BSIZE is changed to something else than the BIOS
  	 * blocksize?  Today, /boot doesn't cover that case so neither need
***************
*** 165,191 ****
  
  		/* Fixup bootdev if units match.  This means that all of
  		 * hd*, sd*, wd*, will be interpreted the same.  Not 100%
! 		 * backwards compatible, but sd* and wd* should be phased-
  		 * out in the bootblocks.
  		 */
! 		if (B_UNIT(bootdev) == (hit->bios_number & 0x7F)) {
  			int type, ctrl, adap, part, unit;
  
! 			/* Translate to MAKEBOOTDEV() style */
  			type = major(bp->b_dev);
! 			adap = B_ADAPTOR(bootdev);
! 			ctrl = B_CONTROLLER(bootdev);
  			unit = DISKUNIT(bp->b_dev);
! 			part = B_PARTITION(bootdev);
  
  			bootdev = MAKEBOOTDEV(type, ctrl, adap, unit, part);
  		}
  
  		/* This will overwrite /boot's guess, just so you remember */
  		hit->bsd_dev = MAKEBOOTDEV(major(bp->b_dev), 0, 0,
  		    DISKUNIT(bp->b_dev), RAW_PART);
  		hit->flags |= BDI_PICKED;
  	}
  	bp->b_flags |= B_INVAL;
  	brelse(bp);
  }
--- 172,218 ----
  
  		/* Fixup bootdev if units match.  This means that all of
  		 * hd*, sd*, wd*, will be interpreted the same.  Not 100%
! 		 * backward compatible, but sd* and wd* should be phased-
  		 * out in the bootblocks.
  		 */
! 		/* Support B_TYPE dependent hd unit counting bootblocks */ 
! 		/* 3.5 RELEASEd BOOT 2.06 May 2004 still is one of these ? */
! 		if ((B_TYPE(bootsbootdev) == B_TYPE(hit->bsd_dev)) &&
! 		    (B_UNIT(bootsbootdev) == B_UNIT(hit->bsd_dev))) {
  			int type, ctrl, adap, part, unit;
  
! 			/* Rewrite, keep MAKEBOOTDEV() style */
  			type = major(bp->b_dev);
! 			adap = B_ADAPTOR(bootsbootdev);
! 			ctrl = B_CONTROLLER(bootsbootdev);
  			unit = DISKUNIT(bp->b_dev);
! 			part = B_PARTITION(bootsbootdev);
  
  			bootdev = MAKEBOOTDEV(type, ctrl, adap, unit, part);
+ 			isbootdevfixed = 1;
  		}
+ 		/* Support B_TYPE independent hd unit counting bootblocks */
+ 		if (B_UNIT(bootsbootdev) == (hit->bios_number & 0x7F)) {
+ 			int type, ctrl, adap, part, unit;
  
+ 			type = major(bp->b_dev);
+ 			adap = B_ADAPTOR(bootsbootdev);
+ 			ctrl = B_CONTROLLER(bootsbootdev);
+ 			unit = DISKUNIT(bp->b_dev);
+ 			part = B_PARTITION(bootsbootdev);
+ 
+ 			altbootdev = MAKEBOOTDEV(type, ctrl, adap, unit, part);
+ 		}
+ 
  		/* This will overwrite /boot's guess, just so you remember */
  		hit->bsd_dev = MAKEBOOTDEV(major(bp->b_dev), 0, 0,
  		    DISKUNIT(bp->b_dev), RAW_PART);
  		hit->flags |= BDI_PICKED;
  	}
+ 
+ 	if (altbootdev && !isbootdevfixed)
+ 		bootdev = altbootdev;
+ 
  	bp->b_flags |= B_INVAL;
  	brelse(bp);
  }