Disk unit mixup for bootdev in i386/dkcsum.c::dkcsumattach() - BUG & FIX

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

Greetings to you all,

I found a small, but probably rather widely annoying, problem in the
way autoconfig is implemented.

Can someone in the development team please have a look at the
fix proposed here too ?

OpenBSD/i386 3.5 (GENERIC) #34: Mon Mar 29 12:24:55 MST 2004

The RELEASEd GENERIC kernel of OpenBSD/i386 May 1st 2004 cannot
always automatically mount the root and swap filesystem from
the same disk that BOOT loaded the kernelfile from.

A BUG in sys/arch/i386/i386/dkcsum.c::dkcsumattach() causes
kernel startup problems in multi-disk machines when the root
filesystem is on a disk that is detected earlier by the kernel
than by the system BIOS.

The problem appears for instance, when OBSD (both /boot and
/bsd) is installed on (a MBR-partition of) an IDE disk with
following properties:

   wd0 matches bios disk 0x82 (=hd2)

This is relevant to all those machines out there that have
multiple disks on different buses (IDE only, SCSI only, a mix of
IDE and SCSI, ), especially those having PCI add-on storage cards
(Promise Ultra and the like).

Due to the BUG described below, the kernel might try mounting its
root and swap ffs from the wrong disk. This then leads to PANICs like
"root filesystem has size 0" or worse.

[Skip the following details, if you want to read the FIX right away.]

Details:

When the 3.5 i386 kernel is totally in its GENERIC RELEASE,
it wants to mount the same root filesystem that /boot
used to load it (/bsd) from. This is apparently what
"config root swap generic" in conf/GENERIC means.

/boot always tells the kernel which BIOS device it used.

3.5/i386 BOOT 2.06 only knows about Int13h Disk BIOS devices. /boot
does not have any knowledge about the wd/sd disks that the autoconfig
PnP kernel is going to detect.
/boot's hd0,hd1,hd2,... BIOS disk UNIT-numbers do not necessarily
correspond with /bsd's /dev/[ws]d0,wd1,wd2,... device unit-numbers.
Not even the sequence order of both enumerations need to match !

i386's /boot passes a queue of bios_diskinfo structures to the kernel
using addbootarg(BOOTARG_DISKINFO). For harddisks, each diskinfo entry
also contains a checksum, that BOOT dynamically calculated over a few
1st disksectors.
These Adler-checksums are expected to uniquely identify individual disks.
Whenever the kernel starts, it calculates these checksums too. Thatway
the kernel is able to match its /dev/[ws]d[0-9] disks with /boot's
hd[0-9] devices.

The 3.5 RELASE kernel does correctly calculate disk checksums again
and matches them with the BIOS device numbers provided by /boot
successfully (see last part of dmesg).

/boot also passes a reboot.h::MAKEBOOTDEV() formatted dev_t variable
named bootdev as an argument to the kernel. This parameter is uniquely
identifies the device that BOOT used to load the kernel from.
The format used tries to summarize the available BIOS diskinfo:
a device type (B_TYPE(), whose value is - in principle - relatable to
D_TYPE), an adapter number, a controller number, and a unit number.
For harddisks the bitfieldvalues are set by diskprobe.c::hardprobe().

For harddisks, the kernel basically rewrites this bootdev variable,
that it received from /boot, using the same MAKEBOOTDEV() format, to
rebase it on the corresponding /dev disk that it has detected. Results
from the matching disk checksums are used for that. Finally, to mount
the root/swap filesystem, the kernel translates the rewritten bootdev
variable into a corresponding disk identifying handle, which is
disklabel.h::MAKEDISKDEV() formatted.

Summary:

Using the biosdiskinfo checksums the kernel should be able to
mount the root filesystem from the very same disk (and in the same
MBR partition) that /boot passed as bootdev.

 /boot does MAKEBOOTDEV(bootdev) based on hd[0-9] Int13h BIOS disks;
 /bsd rewrites MAKEBOOTDEV(bootdev) to base it on a /dev/[ws]d[0-9]

Bug FIX:

A 2-line patch for sys/arch/i386/i386/dkcsum.c::dkcsumattach()
fixes the problem. As my girlfriend said: "never change yourself
twice, Fred". And she's married you know :-)). This is precisely what
dkcsumattach() currently does to the external bootdev variable !

In the loop that walks through the list of alldevs, for each harddisk
a corresponding biosdrive is found using checksum matching. Whenever
a match is hit, next code is executed:

 if (B_UNIT(bootdev) == (hit->bios_number & 0x7F)) {
  // fixup bootdev bitfields
  ...
  unit = DISKUNIT(bp->b_dev);
  ...
  bootdev = MAKEBOOTDEV(.., unit, ...);
 }

So the value of bootdev might get changed, and then - for a next
disk from the list - this new unit value may be compared with
another bios hd unit number.

This obviously leads to undetermined results. The values and order
of bios_numbers need not match the values and order of DISKUNITs.
For instance when there are 3 disks in a machine with following
unit assignments:

  wd0 matches 0x82
  wd1 matches 0x80
  wd2 matches 0x81

and when the bootable OBSD is on wd0 (the 3rd BIOS drive, 0x82),
the bootdev variable enters the loop with a unit number of 2.
In the first run a hit is found, and bootdev is fixed up with
a unitnumber of 0. In the next run, bootdev has a match with
a bios unitnumber again (0 matches 0x80), but that is the wrong
one !

The solution is to introduce a new local variable outside of
the main harddisk enumerating loop:

   int bootdevunit; bootdevunit = B_UNIT(bootdev);

And change the if clause shown above into:

 if (bootdevunit == (hit->bios_number & 0x7F)) {

This patch would make OBSD truely auto configuring even for all those
machines with add-on diskcontrollers !

Possible Future:

Much of the relevant autoconfig, boot-related code is rather messy.
Both on the BOOT side and on the kernel side. Lots of global variables.
Not very modular. Clearly code in transition from a legacy boot process,
which apparently did not yet exclusively rely on universal (non SCSI/IDE
specific) i386 HardDisk BIOS unit numbers. It looks like it should be
cleaned up some time, somewhere, by someone, if support for a whole range
of new, other boot device types is wanted too (USB memory stick, Memory
Cards, etc.) as they all might (start to) support Disk BIOS services for
BOOT to use ?

Congratulations:

As a fresh foreigner in this BSD world, I build a kernel today for the first time.
Following the instructions in the OBSD FAQ, I was a bit shaky. To my
surprise, really, this ran flawlessly. Excellent ! May I congratulate all you
out there, in making this wonderfull OS available ! It is really great to
experience so much quality for a change. Having the source code available
really is an eye-opener to me. Thanks.