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

2015-01-26 Thread Nikunj A Dadhania
Alexander Graf  writes:

> On 23.01.15 23:51, dval...@suse.de wrote:
>> From: Dinar Valeev 
>> 
>> In order to use -boot once=X option we need to have default list
>>  where restore to on reset.
>> 
>> Signed-off-by: Dinar Valeev 
>
> Alexey, Nijunj, where is the default boot order stored usually?

When -boot argument is not present, the default boot order is NULL for
spapr, and SLOF decides to boot from NVRAM set variable or discovered
disk.

> Is "cdn" an accurate equivalent?

No, we have changed that to NULL.

http://comments.gmane.org/gmane.comp.emulators.qemu/187318

Regards,
Nikunj




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

2015-01-26 Thread Alexander Graf

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:

dval...@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?








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

2015-01-26 Thread Dinar Valeev

On 01/26/2015 01:37 PM, Markus Armbruster wrote:

Dinar Valeev  writes:


On 01/26/2015 10:11 AM, Markus Armbruster wrote:

dval...@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?






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

2015-01-26 Thread Markus Armbruster
Dinar Valeev  writes:

> On 01/26/2015 10:11 AM, Markus Armbruster wrote:
>> dval...@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.

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?



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

2015-01-26 Thread Dinar Valeev

On 01/26/2015 10:11 AM, Markus Armbruster wrote:

dval...@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



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

2015-01-26 Thread Markus Armbruster
dval...@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?



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

2015-01-23 Thread Alexander Graf


On 23.01.15 23:51, dval...@suse.de wrote:
> From: Dinar Valeev 
> 
> In order to use -boot once=X option we need to have default list
>  where restore to on reset.
> 
> Signed-off-by: Dinar Valeev 

Alexey, Nijunj, where is the default boot order stored usually? Is "cdn"
an accurate equivalent?


Alex

> ---
>  hw/ppc/spapr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index b560459..3d2cfa3 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -1733,7 +1733,7 @@ static void spapr_machine_class_init(ObjectClass *oc, 
> void *data)
>  mc->block_default_type = IF_SCSI;
>  mc->max_cpus = MAX_CPUS;
>  mc->no_parallel = 1;
> -mc->default_boot_order = NULL;
> +mc->default_boot_order = "cdn";
>  mc->kvm_type = spapr_kvm_type;
>  mc->has_dynamic_sysbus = true;
>  
>