Now that SLOF can update QEMU's device tree at runtime, it makes sense
to be able to dump the resulting dtb, pretty much like it is already
possible to dump the initial dtb with the dumpdtb machine option.

Add a new dumpdtb-slof property to the pseries machine with the same
semantics as dumpdtb, except that the dtb is dumped at the first call
to h_update_dt() and QEMU exits right after that.

The dtb size sanity check is skipped on purpose so that one has a chance
to peek into the dump file and see what's wrong. If the size is big enough
to cause g_malloc0() to fail then QEMU will abort though. This is likely
not ever to happen, and anyway, we don't really care because dumpdtb-slof
is for developpers, not production, and they should try to debug at the
SLOF level in this case.

Even if 3.1 and older machine types don't support device tree updates, it
doesn't hurt to let them dump the dtb and exit anyway, and it seems better
to ensure a consistent behaviour for this feature.

Signed-off-by: Greg Kurz <groug@kaod.org>
---
 hw/ppc/spapr.c         |   19 +++++++++++++++++++
 hw/ppc/spapr_hcall.c   |   22 ++++++++++++++--------
 include/hw/ppc/spapr.h |    1 +
 3 files changed, 34 insertions(+), 8 deletions(-)

diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
index ce84806ee3d5..18de51d03bd1 100644
--- a/hw/ppc/spapr.c
+++ b/hw/ppc/spapr.c
@@ -3322,6 +3322,19 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp)
     spapr->host_serial = g_strdup(value);
 }
 
+static char *spapr_get_dumpdtb_slof(Object *obj, Error **errp)
+{
+    return g_strdup(SPAPR_MACHINE(obj)->dumpdtb_slof);
+}
+
+static void spapr_set_dumpdtb_slof(Object *obj, const char *value, Error **errp)
+{
+    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
+
+    g_free(spapr->dumpdtb_slof);
+    spapr->dumpdtb_slof = g_strdup(value);
+}
+
 static void spapr_instance_init(Object *obj)
 {
     SpaprMachineState *spapr = SPAPR_MACHINE(obj);
@@ -3378,6 +3391,12 @@ static void spapr_instance_init(Object *obj)
         &error_abort);
     object_property_set_description(obj, "host-serial",
         "Host serial number to advertise in guest device tree", &error_abort);
+
+    object_property_add_str(obj, "dumpdtb-slof", spapr_get_dumpdtb_slof,
+                            spapr_set_dumpdtb_slof, &error_abort);
+    object_property_set_description(obj, "dumpdtb-slof",
+                                    "Dump SLOF dtb to a file and quit",
+                                    &error_abort);
 }
 
 static void spapr_machine_finalizefn(Object *obj)
diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
index 6c16d2b12040..30a3880cf1d6 100644
--- a/hw/ppc/spapr_hcall.c
+++ b/hw/ppc/spapr_hcall.c
@@ -1766,20 +1766,26 @@ static target_ulong h_update_dt(PowerPCCPU *cpu, SpaprMachineState *spapr,
     cpu_physical_memory_read(dt, &hdr, sizeof(hdr));
     cb = fdt32_to_cpu(hdr.totalsize);
 
-    if (!smc->update_dt_enabled) {
-        return H_SUCCESS;
-    }
+    if (!spapr->dumpdtb_slof) {
+        if (!smc->update_dt_enabled) {
+            return H_SUCCESS;
+        }
 
-    /* Check that the fdt did not grow out of proportion */
-    if (cb > spapr->fdt_initial_size * 2) {
-        trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
-                                          fdt32_to_cpu(hdr.magic));
-        return H_PARAMETER;
+        /* Check that the fdt did not grow out of proportion */
+        if (cb > spapr->fdt_initial_size * 2) {
+            trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
+                                              fdt32_to_cpu(hdr.magic));
+            return H_PARAMETER;
+        }
     }
 
     fdt = g_malloc0(cb);
     cpu_physical_memory_read(dt, fdt, cb);
 
+    if (spapr->dumpdtb_slof) {
+        exit(g_file_set_contents(spapr->dumpdtb_slof, fdt, cb, NULL) ? 0 : 1);
+    }
+
     /* Check the fdt consistency */
     if (fdt_check_full(fdt, cb)) {
         trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
index 7e32f309c2be..72a5ff7bfee9 100644
--- a/include/hw/ppc/spapr.h
+++ b/include/hw/ppc/spapr.h
@@ -191,6 +191,7 @@ struct SpaprMachineState {
     char *kvm_type;
     char *host_model;
     char *host_serial;
+    char *dumpdtb_slof;
 
     int32_t irq_map_nr;
     unsigned long *irq_map;

On 5/6/19 5:09 AM, Greg Kurz wrote:
> Now that SLOF can update QEMU's device tree at runtime, it makes sense
> to be able to dump the resulting dtb, pretty much like it is already
> possible to dump the initial dtb with the dumpdtb machine option.
>
> Add a new dumpdtb-slof property to the pseries machine with the same
> semantics as dumpdtb, except that the dtb is dumped at the first call
> to h_update_dt() and QEMU exits right after that.
>
> The dtb size sanity check is skipped on purpose so that one has a chance
> to peek into the dump file and see what's wrong. If the size is big enough
> to cause g_malloc0() to fail then QEMU will abort though. This is likely
> not ever to happen, and anyway, we don't really care because dumpdtb-slof
> is for developpers, not production, and they should try to debug at the

typo: developers


> SLOF level in this case.
>
> Even if 3.1 and older machine types don't support device tree updates, it
> doesn't hurt to let them dump the dtb and exit anyway, and it seems better
> to ensure a consistent behaviour for this feature.
>
> Signed-off-by: Greg Kurz <groug@kaod.org>
> ---

LGTM

Reviewed-by: Daniel Henrique Barboza <danielhb413@gmail.com>

>   hw/ppc/spapr.c         |   19 +++++++++++++++++++
>   hw/ppc/spapr_hcall.c   |   22 ++++++++++++++--------
>   include/hw/ppc/spapr.h |    1 +
>   3 files changed, 34 insertions(+), 8 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index ce84806ee3d5..18de51d03bd1 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -3322,6 +3322,19 @@ static void spapr_set_host_serial(Object *obj, const char *value, Error **errp)
>       spapr->host_serial = g_strdup(value);
>   }
>   
> +static char *spapr_get_dumpdtb_slof(Object *obj, Error **errp)
> +{
> +    return g_strdup(SPAPR_MACHINE(obj)->dumpdtb_slof);
> +}
> +
> +static void spapr_set_dumpdtb_slof(Object *obj, const char *value, Error **errp)
> +{
> +    SpaprMachineState *spapr = SPAPR_MACHINE(obj);
> +
> +    g_free(spapr->dumpdtb_slof);
> +    spapr->dumpdtb_slof = g_strdup(value);
> +}
> +
>   static void spapr_instance_init(Object *obj)
>   {
>       SpaprMachineState *spapr = SPAPR_MACHINE(obj);
> @@ -3378,6 +3391,12 @@ static void spapr_instance_init(Object *obj)
>           &error_abort);
>       object_property_set_description(obj, "host-serial",
>           "Host serial number to advertise in guest device tree", &error_abort);
> +
> +    object_property_add_str(obj, "dumpdtb-slof", spapr_get_dumpdtb_slof,
> +                            spapr_set_dumpdtb_slof, &error_abort);
> +    object_property_set_description(obj, "dumpdtb-slof",
> +                                    "Dump SLOF dtb to a file and quit",
> +                                    &error_abort);
>   }
>   
>   static void spapr_machine_finalizefn(Object *obj)
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 6c16d2b12040..30a3880cf1d6 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1766,20 +1766,26 @@ static target_ulong h_update_dt(PowerPCCPU *cpu, SpaprMachineState *spapr,
>       cpu_physical_memory_read(dt, &hdr, sizeof(hdr));
>       cb = fdt32_to_cpu(hdr.totalsize);
>   
> -    if (!smc->update_dt_enabled) {
> -        return H_SUCCESS;
> -    }
> +    if (!spapr->dumpdtb_slof) {
> +        if (!smc->update_dt_enabled) {
> +            return H_SUCCESS;
> +        }
>   
> -    /* Check that the fdt did not grow out of proportion */
> -    if (cb > spapr->fdt_initial_size * 2) {
> -        trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
> -                                          fdt32_to_cpu(hdr.magic));
> -        return H_PARAMETER;
> +        /* Check that the fdt did not grow out of proportion */
> +        if (cb > spapr->fdt_initial_size * 2) {
> +            trace_spapr_update_dt_failed_size(spapr->fdt_initial_size, cb,
> +                                              fdt32_to_cpu(hdr.magic));
> +            return H_PARAMETER;
> +        }
>       }
>   
>       fdt = g_malloc0(cb);
>       cpu_physical_memory_read(dt, fdt, cb);
>   
> +    if (spapr->dumpdtb_slof) {
> +        exit(g_file_set_contents(spapr->dumpdtb_slof, fdt, cb, NULL) ? 0 : 1);
> +    }
> +
>       /* Check the fdt consistency */
>       if (fdt_check_full(fdt, cb)) {
>           trace_spapr_update_dt_failed_check(spapr->fdt_initial_size, cb,
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 7e32f309c2be..72a5ff7bfee9 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -191,6 +191,7 @@ struct SpaprMachineState {
>       char *kvm_type;
>       char *host_model;
>       char *host_serial;
> +    char *dumpdtb_slof;
>   
>       int32_t irq_map_nr;
>       unsigned long *irq_map;
>
>