On Thu, Dec 02, 2010 at 03:17:53PM +1000, David Gwynne wrote:
> the boot loader passes a variable that identifies the disk its
> booting off made up of a bunch of fields like adapter, controller,
> disk, and partition offsets, plus a table of all the disks it can
> see which includes this id and a checksum.
>
> the kernel goes through and checksums the disks and then maps that
> back to the id associated with that disk, and then compares some
> of the fields in those ids against the boot disks id to figure out
> which disk its on.
>
> the problem is we overflow one of those fields (the disk id one).
> since the other fields are set to 0 by the boot loader, this doesnt
> really matter that much. however, since those fields are now
> significant because of the overflow, we should compare them too.
>
> this prevents sd16 being matched as the boot disk after sd0 on my
> system with 25 disks attached.
>
> sorry if this explanation sucks.
>
> ok?
If this works then ok k...@. Certainly comparing all the fields is
better as far as I am concerned.
My only twinge is relying on the overflow of the unit field. Some
clever dick (intern?) may look at MAKEBOOTDEV() some day and apply
the masks as the word is being constructed. Perhaps a cautionary
note in sys/sys/reboot.h saying we rely on unit overflowing would
not go amiss.
.... Ken
>
> Index: dkcsum.c
> ===================================================================
> RCS file: /cvs/src/sys/arch/amd64/amd64/dkcsum.c,v
> retrieving revision 1.15
> diff -u -p -r1.15 dkcsum.c
> --- dkcsum.c 10 Dec 2008 23:41:19 -0000 1.15
> +++ dkcsum.c 2 Dec 2010 05:08:25 -0000
> @@ -71,10 +71,13 @@ dkcsumattach(void)
>
> #ifdef DEBUG
> printf("dkcsum: bootdev=%#x\n", bootdev);
> - for (bdi = bios_diskinfo; bdi->bios_number != -1; bdi++)
> - if (bdi->bios_number & 0x80)
> - printf("dkcsum: BIOS drive %#x checksum is %#x\n",
> - bdi->bios_number, bdi->checksum);
> + for (bdi = bios_diskinfo; bdi->bios_number != -1; bdi++) {
> + if (bdi->bios_number & 0x80) {
> + printf("dkcsum: BIOS drive %#x bsd_dev=%#x "
> + "checksum=%#x\n", bdi->bios_number, bdi->bsd_dev,
> + bdi->checksum);
> + }
> + }
> #endif
> pribootdev = altbootdev = 0;
>
> @@ -180,7 +183,9 @@ dkcsumattach(void)
> */
>
> /* B_TYPE dependent hd unit counting bootblocks */
> - if ((B_TYPE(bootdev) == B_TYPE(hit->bsd_dev)) &&
> + if ((B_ADAPTOR(bootdev) == B_ADAPTOR(hit->bsd_dev)) &&
> + (B_CONTROLLER(bootdev) == B_CONTROLLER(hit->bsd_dev)) &&
> + (B_TYPE(bootdev) == B_TYPE(hit->bsd_dev)) &&
> (B_UNIT(bootdev) == B_UNIT(hit->bsd_dev))) {
> int type, ctrl, adap, part, unit;
