From patchwork Fri Mar 28 00:04:02 2008 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Woodhouse X-Patchwork-Id: 22966 Return-Path: X-Original-To: yaboot-devel@ozlabs.org Delivered-To: yaboot-devel@ozlabs.org Received: from bombadil.infradead.org (bombadil.infradead.org [18.85.46.34]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client did not present a certificate) by ozlabs.org (Postfix) with ESMTPS id 600B4DDF0A for ; Fri, 28 Mar 2008 11:04:09 +1100 (EST) Received: from pmac.infradead.org ([2001:8b0:10b:1:20d:93ff:fe7a:3f2c]) by bombadil.infradead.org with esmtpsa (Exim 4.68 #1 (Red Hat Linux)) id 1Jf24g-00054P-Ul; Fri, 28 Mar 2008 00:04:04 +0000 Subject: Pegasos and partition numbering From: David Woodhouse To: pauln@truemesh.com Date: Fri, 28 Mar 2008 00:04:02 +0000 Message-Id: <1206662642.9540.433.camel@pmac.infradead.org> Mime-Version: 1.0 X-Mailer: Evolution 2.22.0 (2.22.0-1.fc9) X-SRS-Rewrite: SMTP reverse-path rewritten from by bombadil.infradead.org See http://www.infradead.org/rpr.html Cc: yaboot-devel@ozlabs.org, pjones@redhat.com X-BeenThere: yaboot-devel@ozlabs.org X-Mailman-Version: 2.1.10b3 Precedence: list List-Id: Technical and development discussion regarding yaboot List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 28 Mar 2008 00:04:11 -0000 The whole bplan partition numbering thing is a clusterfuck, but it shouldn't be as crappy as it is. We can _cope_ with the fact that firmware is broken and has an off-by-one in its partition numbering. All we need to do in yaboot is add 1 to the partition numbers we infer from in /chosen/boot-device, if we detect that we're on an afflicted machine. That's all we need to do, and all we ever _should_ have done. And is already in yaboot. Yaboot does its own block device handling, including partitions, and doesn't ever care about the firmware's problems (in that respect, at least). It doesn't affect the installer at _all_, because the installer can't set up the firmware's boot-device anyway and it has to be done by hand. All it affects is the release notes telling the user how to make it bootable. Unfortunately, we didn't make it that simple when we first supported the Pegasos. For reasons which aren't entirely clear to me, we ended up with Amiga partition table support in yaboot with the _same_ off-by-one error, to match the firmware. And thus we have hacks in the installer to use amiga partitions for Pegasos, and to cope with the off-by-one crap. It's only after avoiding all this crap purely by accident on Efika, by using DOS partition tables, that I realise how stupid I was to blindly copy the crap that other people were doing, and to believe that Pegasos would only work with Amiga partition tables. I'd like to get rid of the off-by-one bug in yaboot's Amiga partition handling. At the moment, our simple 'if bplan, partition++' in yaboot_main() is wrong when we have Amiga partitions, although it's fine for other partition types. Actually, I'd also like to make that same increment conditional on !conf_given, so that if someone specifies 'conf=hd:1,/yaboot.conf' on the command line, that partition number _isn't_ incremented. So any time you see a proper path specified as 'dev:part,/path/name' you know it's a real one with proper partition numbers. Remember, the bplan firmware doesn't allow that form, and takes a space between the 'dev:part' bit and the filename: boot hd:0 /yaboot/yaboot conf=hd:1,/yaboot/yaboot.conf Fixing the off-by-one bug in the Amiga partition handling means that upgrades might break. I suppose we could have a 'noamigaoffbyone' configuration option which all newly-written yaboot.conf files would have, which controls this behaviour. But to be honest I just don't think it's worth it. diff --git a/second/partition.c b/second/partition.c index 9a7fe73..d20a0ed 100644 --- a/second/partition.c +++ b/second/partition.c @@ -287,7 +287,7 @@ partition_amiga_lookup( const char *dev_name, prom_handle disk, for (i=0; i < possible; i++) used[i] = 0; - for (part = amiga_block[AMIGA_PARTITIONS], partition = 0; + for (part = amiga_block[AMIGA_PARTITIONS], partition = 1; part != AMIGA_END; part = amiga_block[AMIGA_PART_NEXT], partition++) { diff --git a/second/yaboot.c b/second/yaboot.c index 5fc1213..2d2b7bc 100644 --- a/second/yaboot.c +++ b/second/yaboot.c @@ -1749,7 +1749,7 @@ yaboot_main(void) prom_printf("%s: Unable to parse\n", bootdevice); return -1; } - if (_machine == _MACH_bplan) + if (_machine == _MACH_bplan && !conf_given) boot.part++; DEBUG_F("After parse_device_path: dev=%s, part=%d, file=%s\n", boot.dev, boot.part, boot.file);