From owner-freebsd-current@FreeBSD.ORG Tue Dec 9 00:29:32 2008 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 59EA6106564A; Tue, 9 Dec 2008 00:29:32 +0000 (UTC) (envelope-from sobomax@FreeBSD.org) Received: from sippysoft.com (gk1.360sip.com [72.236.70.240]) by mx1.freebsd.org (Postfix) with ESMTP id 17DD78FC16; Tue, 9 Dec 2008 00:29:31 +0000 (UTC) (envelope-from sobomax@FreeBSD.org) Received: from [192.168.1.38] (S0106001372fd1e07.vs.shawcable.net [70.71.171.106]) (authenticated bits=0) by sippysoft.com (8.13.8/8.13.8) with ESMTP id mB90TTqG098401 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 8 Dec 2008 16:29:30 -0800 (PST) (envelope-from sobomax@FreeBSD.org) Message-ID: <493DBBD0.5080705@FreeBSD.org> Date: Mon, 08 Dec 2008 16:29:04 -0800 From: Maxim Sobolev Organization: Sippy Software, Inc. User-Agent: Thunderbird 2.0.0.18 (Windows/20081105) MIME-Version: 1.0 To: Luigi Rizzo References: <493DA269.2070805@FreeBSD.org> <20081208235119.GA46608@onelab2.iet.unipi.it> In-Reply-To: <20081208235119.GA46608@onelab2.iet.unipi.it> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Cc: Luigi Rizzo , hackers@FreeBSD.org, "current@freebsd.org" Subject: Re: Enhancing cdboot [patch for review] X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 09 Dec 2008 00:29:32 -0000 Luigi Rizzo wrote: > On Mon, Dec 08, 2008 at 02:40:41PM -0800, Maxim Sobolev wrote: >> Hi, >> >> Below please find patch that enhances cdboot with two compile-time options: > ... >> Any comments/suggestions are appreciated. If there are no objections I >> would like to commit the change. The long-term goal is to make >> CDBOOT_PROMPT default mode for installation CD. >> >> http://sobomax.sippysoft.com/~sobomax/cdboot.diff > > Looks good. Some comments: Thank you for the review and comments. Please see my answers below. > 1. since there is plenty of space in the cdboot sector, why don't you > make the two option always compiled in, controlling which one to > activate through flags in the bootsector itself, to be set > patching the binary sector itself using a mechanism similar to > boot0cfg. > Of course you cannot alter a cdrom after you burn it, > but it makes it easier to build CDs with one or the other defaults, > patching cdboot or the iso image itself before creating/burning it. > > 2. in fact, the 'silent' option could be disabled at runtime by > pressing some key (e.g. adding a short wait loop before proceeding; > if this is meant for custom, unattended CDs the extra delay should not > matter much); Good idea, I will see if I can put that in. In fact this behavior should have to be optional as well, since one of the uses for the "silent" option here is to provide tamper-resistant boot process on custom hardware. > 3. one nitpick -- in one of the first chunks you replace $start > with $LOAD, but if i am not mistaken operation depends on $LOAD = $start, > so why don't you always use the same ? No, they are not the same. $LOAD is address where BIOS loads boot sector, which is 0x7c00 by default (you can configure it when building CD-ROM, which is why it's an option). On the other hand, $start is address where code is compiled to be located, which is 0x0600. > Also in terms of relocation size, wouldn't it be the case of > hardwiring the size of the cd boot sector: > > - mov $((end_init - start)/2),%cx > + mov 1024,%cx Well, I don't see the reason to hardwire this. CDROM boot code can be of different size (within certain limits of course, to be selected when building ISO), it's not limited to fixed number of sectors like boot[012]. > 4. another nitpick -- the value you pass in %si to the MBR does not > seem to point to anything useful. As discussed about boot0.S and > the followup in the mailing lists, there seems to be no standard > but at least some MBR expect %si to point to a partition entry, > so you should probably initialize one in a way similar way to that > used by boot0.S Hmm, maybe I misunderstood it then. What do you mean by "point to partition entry exactly"? Right now it points to the beginning on MBR. -Maxim