From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:42933) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFlg3-0003eR-1o for qemu-devel@nongnu.org; Mon, 26 Jan 2015 10:34:16 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1YFlfx-0002RF-DA for qemu-devel@nongnu.org; Mon, 26 Jan 2015 10:34:10 -0500 Received: from cantor2.suse.de ([195.135.220.15]:47973 helo=mx2.suse.de) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1YFlfx-0002Ps-1M for qemu-devel@nongnu.org; Mon, 26 Jan 2015 10:34:05 -0500 Message-ID: <54C60A04.1020404@suse.de> Date: Mon, 26 Jan 2015 10:33:56 +0100 From: Alexander Graf 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> <54C639A7.1000305@suse.de> In-Reply-To: <54C639A7.1000305@suse.de> 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: Dinar Valeev , Markus Armbruster Cc: Dinar Valeev , qemu-devel@nongnu.org On 01/26/2015 01:57 PM, Dinar Valeev wrote: > 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. That's not entirely true. If SLOF doesn't see qemu,boot-device, it first goes via its NVRAM configured boot list and only if nothing is there it falls back to "cdn". Maybe the actual appropriate thing would be "mcdn" with m meaning "NVRAM". Alex > >> 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? >> >