From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:35565) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFjEC-0003NZ-JJ for qemu-devel@nongnu.org; Mon, 26 Jan 2015 07:57:17 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YFjE8-0001oj-Ig for qemu-devel@nongnu.org; Mon, 26 Jan 2015 07:57:16 -0500 Received: from cantor2.suse.de ([195.135.220.15]:43080 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFjE8-0001oc-BT for qemu-devel@nongnu.org; Mon, 26 Jan 2015 07:57:12 -0500 Message-ID: <54C639A7.1000305@suse.de> Date: Mon, 26 Jan 2015 13:57:11 +0100 From: Dinar Valeev MIME-Version: 1.0 References: <1422053517-23781-1-git-send-email-dvaleev@suse.de> <877fw9kiti.fsf@blackfin.pond.sub.org> <54C617A7.8050207@suse.de> <87mw55d8f1.fsf@blackfin.pond.sub.org> In-Reply-To: <87mw55d8f1.fsf@blackfin.pond.sub.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 1/2] hw/ppc/spapr.c Set default boot order List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Markus Armbruster Cc: Dinar Valeev , qemu-devel@nongnu.org, Alexander Graf On 01/26/2015 01:37 PM, Markus Armbruster wrote: > Dinar Valeev writes: > >> On 01/26/2015 10:11 AM, Markus Armbruster wrote: >>> dvaleev@suse.de writes: >>> >>>> From: Dinar Valeev >>>> >>>> In order to use -boot once=X option we need to have default list >>>> where restore to on reset. >>> >>> Really? What happens without this patch? >>> >> qemu segfaults on reset. >> 0 > reset-all Segmentation fault > > Next time, include a backtrace, please. Ok, sorry for that. > > Here's what I think happens. > > Boot order comes from --boot parameter once, order, or else the machine > type's .default_boot_order. The latter is null for you. > > It gets passed via ppc_spapr_init() to spapr_create_fdt_skel(), which > sets qemu,boot-device in the FDT to it, but only when it isn't null. > > If it comes from parameter once, we additionally register a reset > handler to switch it to parameter order or else .default_boot_order on > reset. If you specify once, but not order, this is null for you. > > On reset, reset handler restore_boot_order() runs. Unlike > spapr_create_fdt_skel(), it doesn't check for null, and crashes in > validate_bootdevices(). > > Correct? Yes qemu_register_boot_set is implemented in PATCH 2/2. on reset boot_device is restored to NULL > > For me, a null .default_boot_order means "machine type does not support > boot order" (this is how commit c165473 treats it). Arguably, --boot > order and once should be rejected then. AFICS SLOF handles qemu,boot-device as boot device, if nothing passed then it goes disk, cdrom, network. Which is the same as "cdn" list. > If I understand you correctly, your machine type does support boot > order. Giving it a non-null .default_boot_order makes sense then. The > appropriate value depends on firmware. It could even be "". > > The null check in spapr_create_fdt_skel() looks superfluous then. > Consider dropping it. > > Makes sense? >