[Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Alexander Graf]

Today we generate the device tree once on machine initialization and then
store the finalized blob in memory to reload it on reset.

This is bad for 2 reasons. First we potentially waste a bunch of RAM for no
good reason, as we have all information required to regenerate the device
tree available anyways.

The second reason is even more important. On machine init when we generate
the device tree for the first time, we don't have all of the devices fully
initialized yet. But the device tree needs to potentially walk devices to
put information about them into the device tree.

Move the generation into a reset function. That way we just generate it new
every time we reset, solving both of the above issues.

Signed-off-by: Alexander Graf <agraf@suse.de>
---
 hw/ppc/e500.c | 49 ++++++++++++++++++++++++++++++++++++++++---------
 1 file changed, 40 insertions(+), 9 deletions(-)

diff --git a/hw/ppc/e500.c b/hw/ppc/e500.c
index f00a62a..904a1e7 100644
--- a/hw/ppc/e500.c
+++ b/hw/ppc/e500.c
@@ -123,12 +123,13 @@ static void dt_serial_create(void *fdt, unsigned long long offset,
     }
 }
 
-static int ppce500_load_device_tree(CPUPPCState *env,
-                                    PPCE500Params *params,
+static int ppce500_load_device_tree(PPCE500Params *params,
                                     hwaddr addr,
                                     hwaddr initrd_base,
-                                    hwaddr initrd_size)
+                                    hwaddr initrd_size,
+                                    bool dry_run)
 {
+    CPUPPCState *env = first_cpu->env_ptr;
     int ret = -1;
     uint64_t mem_reg_property[] = { 0, cpu_to_be64(params->ram_size) };
     int fdt_size;
@@ -368,12 +369,10 @@ static int ppce500_load_device_tree(CPUPPCState *env,
     }
 
 done:
-    qemu_devtree_dumpdtb(fdt, fdt_size);
-    ret = rom_add_blob_fixed(BINARY_DEVICE_TREE_FILE, fdt, fdt_size, addr);
-    if (ret < 0) {
-        goto out;
+    if (!dry_run) {
+        qemu_devtree_dumpdtb(fdt, fdt_size);
+        cpu_physical_memory_write(addr, fdt, fdt_size);
     }
-    g_free(fdt);
     ret = fdt_size;
 
 out:
@@ -382,6 +381,38 @@ out:
     return ret;
 }
 
+typedef struct DeviceTreeParams {
+    PPCE500Params params;
+    hwaddr addr;
+    hwaddr initrd_base;
+    hwaddr initrd_size;
+} DeviceTreeParams;
+
+static void ppce500_reset_device_tree(void *opaque)
+{
+    DeviceTreeParams *p = opaque;
+    ppce500_load_device_tree(&p->params, p->addr, p->initrd_base,
+                             p->initrd_size, false);
+}
+
+static int ppce500_prep_device_tree(PPCE500Params *params,
+                                    hwaddr addr,
+                                    hwaddr initrd_base,
+                                    hwaddr initrd_size)
+{
+    DeviceTreeParams *p = g_new(DeviceTreeParams, 1);
+    p->params = *params;
+    p->addr = addr;
+    p->initrd_base = initrd_base;
+    p->initrd_size = initrd_size;
+
+    qemu_register_reset(ppce500_reset_device_tree, p);
+
+    /* Issue the device tree loader once, so that we get the size of the blob */
+    return ppce500_load_device_tree(params, addr, initrd_base, initrd_size,
+                                    true);
+}
+
 /* Create -kernel TLB entries for BookE.  */
 static inline hwaddr booke206_page_size_to_tlb(uint64_t size)
 {
@@ -745,7 +776,7 @@ void ppce500_init(PPCE500Params *params)
         struct boot_info *boot_info;
         int dt_size;
 
-        dt_size = ppce500_load_device_tree(env, params, dt_base, initrd_base,
+        dt_size = ppce500_prep_device_tree(params, dt_base, initrd_base,
                                            initrd_size);
         if (dt_size < 0) {
             fprintf(stderr, "couldn't load device tree\n");
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Scott Wood]

On 07/22/2013 10:28:17 AM, Alexander Graf wrote:
> Today we generate the device tree once on machine initialization and  
> then
> store the finalized blob in memory to reload it on reset.
> 
> This is bad for 2 reasons. First we potentially waste a bunch of RAM  
> for no
> good reason, as we have all information required to regenerate the  
> device
> tree available anyways.
> 
> The second reason is even more important. On machine init when we  
> generate
> the device tree for the first time, we don't have all of the devices  
> fully
> initialized yet. But the device tree needs to potentially walk  
> devices to
> put information about them into the device tree.

If you can't produce the entire device tree at init time, how can you  
calculate its size with a dry run?

Device trees are generally pretty small; couldn't we just set a maximum  
size and allocate that much space?

-Scott

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Alexander Graf]

On 23.07.2013, at 21:38, Scott Wood wrote:

> On 07/22/2013 10:28:17 AM, Alexander Graf wrote:
>> Today we generate the device tree once on machine initialization and then
>> store the finalized blob in memory to reload it on reset.
>> This is bad for 2 reasons. First we potentially waste a bunch of RAM for no
>> good reason, as we have all information required to regenerate the device
>> tree available anyways.
>> The second reason is even more important. On machine init when we generate
>> the device tree for the first time, we don't have all of the devices fully
>> initialized yet. But the device tree needs to potentially walk devices to
>> put information about them into the device tree.
> 
> If you can't produce the entire device tree at init time, how can you calculate its size with a dry run?
> 
> Device trees are generally pretty small; couldn't we just set a maximum size and allocate that much space?

It's what we do, unless we load it from the disk. In that case we take the fdt size from disk.


Alex

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Scott Wood]

On 07/23/2013 04:15:59 PM, Alexander Graf wrote:
> 
> On 23.07.2013, at 21:38, Scott Wood wrote:
> 
> > On 07/22/2013 10:28:17 AM, Alexander Graf wrote:
> >> Today we generate the device tree once on machine initialization  
> and then
> >> store the finalized blob in memory to reload it on reset.
> >> This is bad for 2 reasons. First we potentially waste a bunch of  
> RAM for no
> >> good reason, as we have all information required to regenerate the  
> device
> >> tree available anyways.
> >> The second reason is even more important. On machine init when we  
> generate
> >> the device tree for the first time, we don't have all of the  
> devices fully
> >> initialized yet. But the device tree needs to potentially walk  
> devices to
> >> put information about them into the device tree.
> >
> > If you can't produce the entire device tree at init time, how can  
> you calculate its size with a dry run?
> >
> > Device trees are generally pretty small; couldn't we just set a  
> maximum size and allocate that much space?
> 
> It's what we do, unless we load it from the disk. In that case we  
> take the fdt size from disk.

So why do we need the dry run stuff?

-SCott

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Alexander Graf]

Am 23.07.2013 um 23:19 schrieb Scott Wood <scottwood@freescale.com>:

> On 07/23/2013 04:15:59 PM, Alexander Graf wrote:
>> On 23.07.2013, at 21:38, Scott Wood wrote:
>> > On 07/22/2013 10:28:17 AM, Alexander Graf wrote:
>> >> Today we generate the device tree once on machine initialization and then
>> >> store the finalized blob in memory to reload it on reset.
>> >> This is bad for 2 reasons. First we potentially waste a bunch of RAM for no
>> >> good reason, as we have all information required to regenerate the device
>> >> tree available anyways.
>> >> The second reason is even more important. On machine init when we generate
>> >> the device tree for the first time, we don't have all of the devices fully
>> >> initialized yet. But the device tree needs to potentially walk devices to
>> >> put information about them into the device tree.
>> >
>> > If you can't produce the entire device tree at init time, how can you calculate its size with a dry run?
>> >
>> > Device trees are generally pretty small; couldn't we just set a maximum size and allocate that much space?
>> It's what we do, unless we load it from the disk. In that case we take the fdt size from disk.
> 
> So why do we need the dry run stuff?

Because dumpdtb otherwise generates a halfway complete dtb on the first dry pass as device realization is yet incomplete :).


Alex

> 
> -SCott

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Scott Wood]

On 07/23/2013 04:44:02 PM, Alexander Graf wrote:
> 
> 
> Am 23.07.2013 um 23:19 schrieb Scott Wood <scottwood@freescale.com>:
> 
> > On 07/23/2013 04:15:59 PM, Alexander Graf wrote:
> >> On 23.07.2013, at 21:38, Scott Wood wrote:
> >> > On 07/22/2013 10:28:17 AM, Alexander Graf wrote:
> >> >> Today we generate the device tree once on machine  
> initialization and then
> >> >> store the finalized blob in memory to reload it on reset.
> >> >> This is bad for 2 reasons. First we potentially waste a bunch  
> of RAM for no
> >> >> good reason, as we have all information required to regenerate  
> the device
> >> >> tree available anyways.
> >> >> The second reason is even more important. On machine init when  
> we generate
> >> >> the device tree for the first time, we don't have all of the  
> devices fully
> >> >> initialized yet. But the device tree needs to potentially walk  
> devices to
> >> >> put information about them into the device tree.
> >> >
> >> > If you can't produce the entire device tree at init time, how  
> can you calculate its size with a dry run?
> >> >
> >> > Device trees are generally pretty small; couldn't we just set a  
> maximum size and allocate that much space?
> >> It's what we do, unless we load it from the disk. In that case we  
> take the fdt size from disk.
> >
> > So why do we need the dry run stuff?
> 
> Because dumpdtb otherwise generates a halfway complete dtb on the  
> first dry pass as device realization is yet incomplete :).

What I mean is why have a first pass at all?

-Scott

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Alexander Graf]

On 23.07.2013, at 23:55, Scott Wood wrote:

> On 07/23/2013 04:44:02 PM, Alexander Graf wrote:
>> Am 23.07.2013 um 23:19 schrieb Scott Wood <scottwood@freescale.com>:
>> > On 07/23/2013 04:15:59 PM, Alexander Graf wrote:
>> >> On 23.07.2013, at 21:38, Scott Wood wrote:
>> >> > On 07/22/2013 10:28:17 AM, Alexander Graf wrote:
>> >> >> Today we generate the device tree once on machine initialization and then
>> >> >> store the finalized blob in memory to reload it on reset.
>> >> >> This is bad for 2 reasons. First we potentially waste a bunch of RAM for no
>> >> >> good reason, as we have all information required to regenerate the device
>> >> >> tree available anyways.
>> >> >> The second reason is even more important. On machine init when we generate
>> >> >> the device tree for the first time, we don't have all of the devices fully
>> >> >> initialized yet. But the device tree needs to potentially walk devices to
>> >> >> put information about them into the device tree.
>> >> >
>> >> > If you can't produce the entire device tree at init time, how can you calculate its size with a dry run?
>> >> >
>> >> > Device trees are generally pretty small; couldn't we just set a maximum size and allocate that much space?
>> >> It's what we do, unless we load it from the disk. In that case we take the fdt size from disk.
>> >
>> > So why do we need the dry run stuff?
>> Because dumpdtb otherwise generates a halfway complete dtb on the first dry pass as device realization is yet incomplete :).
> 
> What I mean is why have a first pass at all?

Ah, you mean we should just always limit ourselves to the 8MB we reserve for the device tree? That would work, yes.


Alex

Re: [Qemu-devel] [PATCH] PPC: E500: Generate device tree on reset

[Alexey Kardashevskiy]

On 07/23/2013 01:28 AM, Alexander Graf wrote:
> Today we generate the device tree once on machine initialization and then
> store the finalized blob in memory to reload it on reset.
> 
> This is bad for 2 reasons. First we potentially waste a bunch of RAM for no
> good reason, as we have all information required to regenerate the device
> tree available anyways.
> 
> The second reason is even more important. On machine init when we generate
> the device tree for the first time, we don't have all of the devices fully
> initialized yet. But the device tree needs to potentially walk devices to
> put information about them into the device tree.


Not fully initialized yet? They are not even created at the machine init
point as far as I can tell, and that was the reason to do for spapr what
you are trying to do for e500 :)


-- 
Alexey