diff mbox

[2/2] hw/ppc/spapr Add qemu_register_boot_set for SPAPR

Message ID 1422053517-23781-2-git-send-email-dvaleev@suse.de
State New
Headers show

Commit Message

dvaleev@suse.de Jan. 23, 2015, 10:51 p.m. UTC
From: Dinar Valeev <dvaleev@suse.com>

In order to have -boot once=d functioning, it is required to have
qemu_register_boot_set

qemu-system-ppc64 -enable-kvm -boot once=d

Ready!
0 > dev /chosen   ok
0 > .properties
...
qemu,boot-device                 d
...
0 > reset-all

Ready!
0 > dev /chosen   ok
0 > .properties
...
qemu,boot-device                 cdn
...

Signed-off-by: Dinar Valeev <dvaleev@suse.com>
---
 hw/ppc/spapr.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

Comments

Alexander Graf Jan. 23, 2015, 11:04 p.m. UTC | #1
On 23.01.15 23:51, dvaleev@suse.de wrote:
> From: Dinar Valeev <dvaleev@suse.com>
> 
> In order to have -boot once=d functioning, it is required to have
> qemu_register_boot_set
> 
> qemu-system-ppc64 -enable-kvm -boot once=d
> 
> Ready!
> 0 > dev /chosen   ok
> 0 > .properties
> ...
> qemu,boot-device                 d
> ...
> 0 > reset-all
> 
> Ready!
> 0 > dev /chosen   ok
> 0 > .properties
> ...
> qemu,boot-device                 cdn
> ...
> 
> Signed-off-by: Dinar Valeev <dvaleev@suse.com>
> ---
>  hw/ppc/spapr.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 3d2cfa3..38b03fc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1)
>      g_string_append_len(s, s1, strlen(s1) + 1);
>  }
>  
> +static void spapr_boot_set(void *opaque, const char *boot_device,
> +                           Error **errp)
> +{
> +    int offset;
> +    offset = fdt_path_offset(opaque, "/chosen");
> +    fdt_setprop_string(opaque, offset, "qemu,boot-device", boot_device);
> +
> +}
> +
> +
>  static void *spapr_create_fdt_skel(hwaddr initrd_base,
>                                     hwaddr initrd_size,
>                                     hwaddr kernel_size,
> @@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>      if (boot_device) {
>          _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
>      }
> +    qemu_register_boot_set(spapr_boot_set, fdt);

If you simply move the code above (the _FDT() one) from create_fdt_skel
to spapr_finalize_fdt() you should have the same net effect and much
cleaner code :).


Alex

> +
>      if (boot_menu) {
>          _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
>      }
>
dvaleev@suse.de Jan. 24, 2015, 4:11 p.m. UTC | #2
On 01/24/2015 12:04 AM, Alexander Graf wrote:
>
>
> On 23.01.15 23:51, dvaleev@suse.de wrote:
>> From: Dinar Valeev <dvaleev@suse.com>
>>
>> In order to have -boot once=d functioning, it is required to have
>> qemu_register_boot_set
>>
>> qemu-system-ppc64 -enable-kvm -boot once=d
>>
>> Ready!
>> 0 > dev /chosen   ok
>> 0 > .properties
>> ...
>> qemu,boot-device                 d
>> ...
>> 0 > reset-all
>>
>> Ready!
>> 0 > dev /chosen   ok
>> 0 > .properties
>> ...
>> qemu,boot-device                 cdn
>> ...
>>
>> Signed-off-by: Dinar Valeev <dvaleev@suse.com>
>> ---
>>   hw/ppc/spapr.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 3d2cfa3..38b03fc 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1)
>>       g_string_append_len(s, s1, strlen(s1) + 1);
>>   }
>>
>> +static void spapr_boot_set(void *opaque, const char *boot_device,
>> +                           Error **errp)
>> +{
>> +    int offset;
>> +    offset = fdt_path_offset(opaque, "/chosen");
>> +    fdt_setprop_string(opaque, offset, "qemu,boot-device", boot_device);
>> +
>> +}
>> +
>> +
>>   static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>                                      hwaddr initrd_size,
>>                                      hwaddr kernel_size,
>> @@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>       if (boot_device) {
>>           _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
>>       }
>> +    qemu_register_boot_set(spapr_boot_set, fdt);
>
> If you simply move the code above (the _FDT() one) from create_fdt_skel
> to spapr_finalize_fdt() you should have the same net effect and much
> cleaner code :).
Haven't tried it, but I suspect -boot once=d on reset will be still 
equal to -boot d behaviour.

And does it make sense to split boot_device from the rest?
>
> Alex
>
>> +
>>       if (boot_menu) {
>>           _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
>>       }
>>
dvaleev@suse.de Jan. 26, 2015, 10:49 a.m. UTC | #3
On 01/24/2015 12:04 AM, Alexander Graf wrote:
>
>
> On 23.01.15 23:51, dvaleev@suse.de wrote:
>> From: Dinar Valeev <dvaleev@suse.com>
>>
>> In order to have -boot once=d functioning, it is required to have
>> qemu_register_boot_set
>>
>> qemu-system-ppc64 -enable-kvm -boot once=d
>>
>> Ready!
>> 0 > dev /chosen   ok
>> 0 > .properties
>> ...
>> qemu,boot-device                 d
>> ...
>> 0 > reset-all
>>
>> Ready!
>> 0 > dev /chosen   ok
>> 0 > .properties
>> ...
>> qemu,boot-device                 cdn
>> ...
>>
>> Signed-off-by: Dinar Valeev <dvaleev@suse.com>
>> ---
>>   hw/ppc/spapr.c | 12 ++++++++++++
>>   1 file changed, 12 insertions(+)
>>
>> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
>> index 3d2cfa3..38b03fc 100644
>> --- a/hw/ppc/spapr.c
>> +++ b/hw/ppc/spapr.c
>> @@ -314,6 +314,16 @@ static void add_str(GString *s, const gchar *s1)
>>       g_string_append_len(s, s1, strlen(s1) + 1);
>>   }
>>
>> +static void spapr_boot_set(void *opaque, const char *boot_device,
>> +                           Error **errp)
>> +{
>> +    int offset;
>> +    offset = fdt_path_offset(opaque, "/chosen");
>> +    fdt_setprop_string(opaque, offset, "qemu,boot-device", boot_device);
>> +
>> +}
>> +
>> +
>>   static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>                                      hwaddr initrd_size,
>>                                      hwaddr kernel_size,
>> @@ -414,6 +424,8 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
>>       if (boot_device) {
>>           _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
>>       }
>> +    qemu_register_boot_set(spapr_boot_set, fdt);
>
> If you simply move the code above (the _FDT() one) from create_fdt_skel
> to spapr_finalize_fdt() you should have the same net effect and much
> cleaner code :).
I've tried your proposal, on reset boot-device property stays "d"

qemu,boot-device                 d
0 > reset-all
qemu,boot-device                 d


Here is what I tried:
@@ -411,9 +410,6 @@ static void *spapr_create_fdt_skel(hwaddr initrd_base,
              _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0)));
          }
      }
-    if (boot_device) {
-        _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
-    }
      if (boot_menu) {
          _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
      }
@@ -730,6 +726,8 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
      char *bootlist;
      void *fdt;
      sPAPRPHBState *phb;
+    MachineState *machine = MACHINE(qdev_get_machine());
+    const char *boot_device = machine->boot_order;

      fdt = g_malloc(FDT_MAX_SIZE);

@@ -769,6 +767,14 @@ static void spapr_finalize_fdt(sPAPREnvironment *spapr,
          fprintf(stderr, "Couldn't finalize CPU device tree properties\n");
      }

+    if (boot_device) {
+        int offset = fdt_path_offset(fdt, "/chosen");
+        ret = fdt_setprop_string(fdt, offset, "qemu,boot-device", 
boot_device);
+        if (ret < 0) {
+            fprintf(stderr, "Couldn't set up boot-device dt property\n");
+        }
+    }
+
      bootlist = get_boot_devices_list(&cb, true);
      if (cb && bootlist) {
          int offset = fdt_path_offset(fdt, "/chosen");

Dinar,
>
> Alex
>
>> +
>>       if (boot_menu) {
>>           _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
>>       }
>>
diff mbox

Patch

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index 3d2cfa3..38b03fc 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -314,6 +314,16 @@  static void add_str(GString *s, const gchar *s1)
     g_string_append_len(s, s1, strlen(s1) + 1);
 }
 
+static void spapr_boot_set(void *opaque, const char *boot_device,
+                           Error **errp)
+{
+    int offset;
+    offset = fdt_path_offset(opaque, "/chosen");
+    fdt_setprop_string(opaque, offset, "qemu,boot-device", boot_device);
+
+}
+
+
 static void *spapr_create_fdt_skel(hwaddr initrd_base,
                                    hwaddr initrd_size,
                                    hwaddr kernel_size,
@@ -414,6 +424,8 @@  static void *spapr_create_fdt_skel(hwaddr initrd_base,
     if (boot_device) {
         _FDT((fdt_property_string(fdt, "qemu,boot-device", boot_device)));
     }
+    qemu_register_boot_set(spapr_boot_set, fdt);
+
     if (boot_menu) {
         _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu)));
     }