From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54983) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byq3L-00029T-G6 for qemu-devel@nongnu.org; Mon, 24 Oct 2016 20:57:20 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byq3I-0005e5-Ea for qemu-devel@nongnu.org; Mon, 24 Oct 2016 20:57:19 -0400 Received: from mail-pf0-x241.google.com ([2607:f8b0:400e:c00::241]:35347) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1byq3I-0005df-6c for qemu-devel@nongnu.org; Mon, 24 Oct 2016 20:57:16 -0400 Received: by mail-pf0-x241.google.com with SMTP id s8so17857720pfj.2 for ; Mon, 24 Oct 2016 17:57:15 -0700 (PDT) References: <1477285483-10766-1-git-send-email-david@gibson.dropbear.id.au> <1477285483-10766-2-git-send-email-david@gibson.dropbear.id.au> From: Alexey Kardashevskiy Message-ID: Date: Tue, 25 Oct 2016 11:57:08 +1100 MIME-Version: 1.0 In-Reply-To: <1477285483-10766-2-git-send-email-david@gibson.dropbear.id.au> Content-Type: text/plain; charset=koi8-r Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCHv3 01/12] pseries: Split device tree construction from device tree load List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson , mdroth@linux.vnet.ibm.com, groug@kaod.org Cc: agraf@suse.de, lvivier@redhat.com, thuth@redhat.com, qemu-ppc@nongnu.org, qemu-devel@nongnu.org On 24/10/16 16:04, David Gibson wrote: > spapr_finalize_fdt() both finishes building the device tree for the guest > and loads it into guest memory. For future cleanups, it's going to be > more convenient to do these two things separately. The loading portion is > pretty trivial, so we move it inline into the caller, ppc_spapr_reset(). > > We also rename spapr_finalize_fdt(), because the current name is going to > become inaccurate. > > Signed-off-by: David Gibson I did reply with "rb" v2 of this with a comment, somehow it was lost. Reviewed-by: Alexey Kardashevskiy with a small nit, grep finds "spapr_finalize_fdt" in a comment: hw/ppc/spapr_cpu_core.c:187: * coldplugged CPUs DT entries are setup in spapr_finalize_fdt(). > --- > hw/ppc/spapr.c | 42 +++++++++++++++++++++++------------------- > 1 file changed, 23 insertions(+), 19 deletions(-) > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > index ddb7438..0864411 100644 > --- a/hw/ppc/spapr.c > +++ b/hw/ppc/spapr.c > @@ -900,10 +900,9 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr, > return 0; > } > > -static void spapr_finalize_fdt(sPAPRMachineState *spapr, > - hwaddr fdt_addr, > - hwaddr rtas_addr, > - hwaddr rtas_size) > +static void *spapr_build_fdt(sPAPRMachineState *spapr, > + hwaddr rtas_addr, > + hwaddr rtas_size) > { > MachineState *machine = MACHINE(qdev_get_machine()); > MachineClass *mc = MACHINE_GET_CLASS(machine); > @@ -999,19 +998,8 @@ static void spapr_finalize_fdt(sPAPRMachineState *spapr, > } > } > > - _FDT((fdt_pack(fdt))); > - > - if (fdt_totalsize(fdt) > FDT_MAX_SIZE) { > - error_report("FDT too big ! 0x%x bytes (max is 0x%x)", > - fdt_totalsize(fdt), FDT_MAX_SIZE); > - exit(1); > - } > - > - qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > - cpu_physical_memory_write(fdt_addr, fdt, fdt_totalsize(fdt)); > - > g_free(bootlist); > - g_free(fdt); > + return fdt; > } > > static uint64_t translate_kernel_address(void *opaque, uint64_t addr) > @@ -1147,6 +1135,8 @@ static void ppc_spapr_reset(void) > sPAPRMachineState *spapr = SPAPR_MACHINE(machine); > PowerPCCPU *first_ppc_cpu; > uint32_t rtas_limit; > + void *fdt; > + int rc; > > /* Check for unknown sysbus devices */ > foreach_dynamic_sysbus_device(find_unknown_sysbus_device, NULL); > @@ -1173,14 +1163,28 @@ static void ppc_spapr_reset(void) > spapr->rtas_addr = rtas_limit - RTAS_MAX_SIZE; > spapr->fdt_addr = spapr->rtas_addr - FDT_MAX_SIZE; > > - /* Load the fdt */ > - spapr_finalize_fdt(spapr, spapr->fdt_addr, spapr->rtas_addr, > - spapr->rtas_size); > + fdt = spapr_build_fdt(spapr, spapr->rtas_addr, spapr->rtas_size); > > /* Copy RTAS over */ > cpu_physical_memory_write(spapr->rtas_addr, spapr->rtas_blob, > spapr->rtas_size); > > + rc = fdt_pack(fdt); > + > + /* Should only fail if we've built a corrupted tree */ > + assert(rc == 0); > + > + if (fdt_totalsize(fdt) > FDT_MAX_SIZE) { > + error_report("FDT too big ! 0x%x bytes (max is 0x%x)", > + fdt_totalsize(fdt), FDT_MAX_SIZE); > + exit(1); > + } > + > + /* Load the fdt */ > + qemu_fdt_dumpdtb(fdt, fdt_totalsize(fdt)); > + cpu_physical_memory_write(spapr->fdt_addr, fdt, fdt_totalsize(fdt)); > + g_free(fdt); > + > /* Set up the entry state */ > first_ppc_cpu = POWERPC_CPU(first_cpu); > first_ppc_cpu->env.gpr[3] = spapr->fdt_addr; > -- Alexey