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

[Markus Armbruster]

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.

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?

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.

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?