qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [PATCH 1/2] hw/ppc/spapr.c Set default boot order


From: Dinar Valeev
Subject: Re: [Qemu-devel] [PATCH 1/2] hw/ppc/spapr.c Set default boot order
Date: Mon, 26 Jan 2015 13:57:11 +0100
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:31.0) Gecko/20100101 Thunderbird/31.4.0

On 01/26/2015 01:37 PM, Markus Armbruster wrote:
Dinar Valeev <address@hidden> writes:

On 01/26/2015 10:11 AM, Markus Armbruster wrote:
address@hidden writes:

From: Dinar Valeev <address@hidden>

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?





reply via email to

[Prev in Thread] Current Thread [Next in Thread]