From patchwork Mon Oct 24 01:07:29 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Gibson X-Patchwork-Id: 9391281 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 2B10B60762 for ; Mon, 24 Oct 2016 01:11:53 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 1E6D728ABB for ; Mon, 24 Oct 2016 01:11:53 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 12C7128ABD; Mon, 24 Oct 2016 01:11:53 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.8 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI,T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from lists.gnu.org (lists.gnu.org [208.118.235.17]) (using TLSv1 with cipher AES256-SHA (256/256 bits)) (No client certificate requested) by mail.wl.linuxfoundation.org (Postfix) with ESMTPS id 8788128ABC for ; Mon, 24 Oct 2016 01:11:48 +0000 (UTC) Received: from localhost ([::1]:43196 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byTnn-0005ds-U7 for patchwork-qemu-devel@patchwork.kernel.org; Sun, 23 Oct 2016 21:11:47 -0400 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39287) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1byTnJ-0005cw-49 for qemu-devel@nongnu.org; Sun, 23 Oct 2016 21:11:19 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1byTnG-0002fO-0I for qemu-devel@nongnu.org; Sun, 23 Oct 2016 21:11:17 -0400 Received: from ozlabs.org ([103.22.144.67]:41093) by eggs.gnu.org with esmtps (TLS1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.71) (envelope-from ) id 1byTnE-0002f3-Sq; Sun, 23 Oct 2016 21:11:13 -0400 Received: by ozlabs.org (Postfix, from userid 1007) id 3t2JBd4GCcz9t14; Mon, 24 Oct 2016 12:11:05 +1100 (AEDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gibson.dropbear.id.au; s=201602; t=1477271465; bh=kPLFPFvjINng1vB7wO3ZO2eZfrYlJnUeZ+q3XHFcHCc=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=eKSsENNzwDFZZWsB/lF3jpH5WlqD8x8t0RZr78yEJMfgQ++dIKy1MH8Cxg9TG6C2J 6cmKMUX9nF1LWGDAf7GBtzWjcK8gPZ+mKQ3BeFyz3r8FFVOanXSDX2CMFoYIq04JNO 2CJohhuey093lsqa7p64MxJgXaGvfZp5+Lu87dn0= Date: Mon, 24 Oct 2016 12:07:29 +1100 From: David Gibson To: Michael Roth Message-ID: <20161024010729.GD19629@umbus.fritz.box> References: <1477018600-6881-1-git-send-email-david@gibson.dropbear.id.au> <1477018600-6881-8-git-send-email-david@gibson.dropbear.id.au> <20161024003359.17113.85655@loki> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: <20161024003359.17113.85655@loki> User-Agent: Mutt/1.7.1 (2016-10-04) X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 103.22.144.67 Subject: Re: [Qemu-devel] [PATCHv2 07/12] pseries: Consolidate construction of /chosen device tree node X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.21 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: lvivier@redhat.com, thuth@redhat.com, qemu-devel@nongnu.org, aik@ozlabs.ru, groug@kaod.org, agraf@suse.de, qemu-ppc@nongnu.org Errors-To: qemu-devel-bounces+patchwork-qemu-devel=patchwork.kernel.org@nongnu.org Sender: "Qemu-devel" X-Virus-Scanned: ClamAV using ClamSMTP On Sun, Oct 23, 2016 at 07:33:59PM -0500, Michael Roth wrote: > Quoting David Gibson (2016-10-20 21:56:35) > > For historical reasons, building the /chosen node in the guest device tree > > is split across several places and includes both parts which write the DT > > sequentially and others which use random access functions. > > > > This patch consolidates construction of the node into one place, using > > random access functions throughout. > > > > Signed-off-by: David Gibson > > Reviewed-by: Thomas Huth [snip] > > + _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-width", graphic_width)); > > + _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-height", graphic_height)); > > + _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-depth", graphic_depth)); > > + > > + if (cb && bootlist) { > > + int i; > > + > > + for (i = 0; i < cb; i++) { > > + if (bootlist[i] == '\n') { > > + bootlist[i] = ' '; > > + } > > + } > > + _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-list", bootlist)); > > + } > > + > > + if (boot_device && strlen(boot_device)) { > > + _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-device", boot_device)); > > + } > > + > > + if (!spapr->has_graphics && stdout_path) { > > + _FDT(fdt_setprop_string(fdt, chosen, "linux,stdout-path", stdout_path)); > > + } > > + > > + g_free(stdout); > > Looks like this got left-over from the stdout->stdout_path change. Yes indeed. And the fact that it didn't give me a compile error demonstrates exactly why naming it stdout was a terrible idea in the first place. Please review the revised version below: From 899ffee24234c0818d9801b95e26dd47a1675a09 Mon Sep 17 00:00:00 2001 From: David Gibson Date: Mon, 24 Oct 2016 12:05:57 +1100 Subject: [PATCH] pseries: Consolidate construction of /chosen device tree node For historical reasons, building the /chosen node in the guest device tree is split across several places and includes both parts which write the DT sequentially and others which use random access functions. This patch consolidates construction of the node into one place, using random access functions throughout. Signed-off-by: David Gibson Reviewed-by: Thomas Huth Reviewed-by: Michael Roth --- hw/ppc/spapr.c | 131 ++++++++++++++++++++++----------------------- hw/ppc/spapr_vio.c | 17 ++---- include/hw/ppc/spapr_vio.h | 2 +- 3 files changed, 70 insertions(+), 80 deletions(-) diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c index 2989c9d..8e71f1e 100644 --- a/hw/ppc/spapr.c +++ b/hw/ppc/spapr.c @@ -273,14 +273,10 @@ static void add_str(GString *s, const gchar *s1) static void *spapr_create_fdt_skel(sPAPRMachineState *spapr) { - MachineState *machine = MACHINE(spapr); void *fdt; - uint32_t start_prop = cpu_to_be32(spapr->initrd_base); - uint32_t end_prop = cpu_to_be32(spapr->initrd_base + spapr->initrd_size); GString *hypertas = g_string_sized_new(256); GString *qemu_hypertas = g_string_sized_new(256); uint32_t refpoints[] = {cpu_to_be32(0x4), cpu_to_be32(0x4)}; - unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80}; char *buf; add_str(hypertas, "hcall-pft"); @@ -337,35 +333,6 @@ static void *spapr_create_fdt_skel(sPAPRMachineState *spapr) _FDT((fdt_property_cell(fdt, "#address-cells", 0x2))); _FDT((fdt_property_cell(fdt, "#size-cells", 0x2))); - /* /chosen */ - _FDT((fdt_begin_node(fdt, "chosen"))); - - /* Set Form1_affinity */ - _FDT((fdt_property(fdt, "ibm,architecture-vec-5", vec5, sizeof(vec5)))); - - _FDT((fdt_property_string(fdt, "bootargs", machine->kernel_cmdline))); - _FDT((fdt_property(fdt, "linux,initrd-start", - &start_prop, sizeof(start_prop)))); - _FDT((fdt_property(fdt, "linux,initrd-end", - &end_prop, sizeof(end_prop)))); - if (spapr->kernel_size) { - uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR), - cpu_to_be64(spapr->kernel_size) }; - - _FDT((fdt_property(fdt, "qemu,boot-kernel", &kprop, sizeof(kprop)))); - if (spapr->kernel_le) { - _FDT((fdt_property(fdt, "qemu,boot-kernel-le", NULL, 0))); - } - } - if (boot_menu) { - _FDT((fdt_property_cell(fdt, "qemu,boot-menu", boot_menu))); - } - _FDT((fdt_property_cell(fdt, "qemu,graphic-width", graphic_width))); - _FDT((fdt_property_cell(fdt, "qemu,graphic-height", graphic_height))); - _FDT((fdt_property_cell(fdt, "qemu,graphic-depth", graphic_depth))); - - _FDT((fdt_end_node(fdt))); - /* RTAS */ _FDT((fdt_begin_node(fdt, "rtas"))); @@ -873,6 +840,68 @@ int spapr_h_cas_compose_response(sPAPRMachineState *spapr, return 0; } +static void spapr_dt_chosen(sPAPRMachineState *spapr, void *fdt) +{ + MachineState *machine = MACHINE(spapr); + int chosen; + const char *boot_device = machine->boot_order; + char *stdout_path = spapr_vio_stdout_path(spapr->vio_bus); + size_t cb = 0; + char *bootlist = get_boot_devices_list(&cb, true); + unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80}; + + _FDT(chosen = fdt_add_subnode(fdt, 0, "chosen")); + + /* Set Form1_affinity */ + _FDT(fdt_setprop(fdt, chosen, "ibm,architecture-vec-5", + vec5, sizeof(vec5))); + + _FDT(fdt_setprop_string(fdt, chosen, "bootargs", machine->kernel_cmdline)); + _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-start", + spapr->initrd_base)); + _FDT(fdt_setprop_cell(fdt, chosen, "linux,initrd-end", + spapr->initrd_base + spapr->initrd_size)); + + if (spapr->kernel_size) { + uint64_t kprop[2] = { cpu_to_be64(KERNEL_LOAD_ADDR), + cpu_to_be64(spapr->kernel_size) }; + + _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel", + &kprop, sizeof(kprop))); + if (spapr->kernel_le) { + _FDT(fdt_setprop(fdt, chosen, "qemu,boot-kernel-le", NULL, 0)); + } + } + if (boot_menu) { + _FDT((fdt_setprop_cell(fdt, chosen, "qemu,boot-menu", boot_menu))); + } + _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-width", graphic_width)); + _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-height", graphic_height)); + _FDT(fdt_setprop_cell(fdt, chosen, "qemu,graphic-depth", graphic_depth)); + + if (cb && bootlist) { + int i; + + for (i = 0; i < cb; i++) { + if (bootlist[i] == '\n') { + bootlist[i] = ' '; + } + } + _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-list", bootlist)); + } + + if (boot_device && strlen(boot_device)) { + _FDT(fdt_setprop_string(fdt, chosen, "qemu,boot-device", boot_device)); + } + + if (!spapr->has_graphics && stdout_path) { + _FDT(fdt_setprop_string(fdt, chosen, "linux,stdout-path", stdout_path)); + } + + g_free(stdout_path); + g_free(bootlist); +} + static void *spapr_build_fdt(sPAPRMachineState *spapr, hwaddr rtas_addr, hwaddr rtas_size) @@ -880,10 +909,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, MachineState *machine = MACHINE(qdev_get_machine()); MachineClass *mc = MACHINE_GET_CLASS(machine); sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine); - const char *boot_device = machine->boot_order; - int ret, i; - size_t cb = 0; - char *bootlist; + int ret; void *fdt; sPAPRPHBState *phb; @@ -932,34 +958,6 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, /* cpus */ spapr_populate_cpus_dt_node(fdt, spapr); - bootlist = get_boot_devices_list(&cb, true); - if (cb && bootlist) { - int offset = fdt_path_offset(fdt, "/chosen"); - if (offset < 0) { - exit(1); - } - for (i = 0; i < cb; i++) { - if (bootlist[i] == '\n') { - bootlist[i] = ' '; - } - - } - ret = fdt_setprop_string(fdt, offset, "qemu,boot-list", bootlist); - } - - if (boot_device && strlen(boot_device)) { - int offset = fdt_path_offset(fdt, "/chosen"); - - if (offset < 0) { - exit(1); - } - fdt_setprop_string(fdt, offset, "qemu,boot-device", boot_device); - } - - if (!spapr->has_graphics) { - spapr_populate_chosen_stdout(fdt, spapr->vio_bus); - } - if (smc->dr_lmb_enabled) { _FDT(spapr_drc_populate_dt(fdt, 0, NULL, SPAPR_DR_CONNECTOR_TYPE_LMB)); } @@ -974,7 +972,8 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, } } - g_free(bootlist); + /* /chosen */ + spapr_dt_chosen(spapr, fdt); /* Build memory reserve map */ if (spapr->kernel_size) { diff --git a/hw/ppc/spapr_vio.c b/hw/ppc/spapr_vio.c index 3648aa5..2b67df0 100644 --- a/hw/ppc/spapr_vio.c +++ b/hw/ppc/spapr_vio.c @@ -665,28 +665,19 @@ out: return ret; } -int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus *bus) +gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus) { VIOsPAPRDevice *dev; char *name, *path; - int ret, offset; dev = spapr_vty_get_default(bus); - if (!dev) - return 0; - - offset = fdt_path_offset(fdt, "/chosen"); - if (offset < 0) { - return offset; + if (!dev) { + return NULL; } name = spapr_vio_get_dev_name(DEVICE(dev)); path = g_strdup_printf("/vdevice/%s", name); - ret = fdt_setprop_string(fdt, offset, "linux,stdout-path", path); - g_free(name); - g_free(path); - - return ret; + return path; } diff --git a/include/hw/ppc/spapr_vio.h b/include/hw/ppc/spapr_vio.h index 0b025fd..a0e7542 100644 --- a/include/hw/ppc/spapr_vio.h +++ b/include/hw/ppc/spapr_vio.h @@ -81,7 +81,7 @@ struct VIOsPAPRBus { extern VIOsPAPRBus *spapr_vio_bus_init(void); extern VIOsPAPRDevice *spapr_vio_find_by_reg(VIOsPAPRBus *bus, uint32_t reg); extern int spapr_populate_vdevice(VIOsPAPRBus *bus, void *fdt); -extern int spapr_populate_chosen_stdout(void *fdt, VIOsPAPRBus *bus); +extern gchar *spapr_vio_stdout_path(VIOsPAPRBus *bus); static inline qemu_irq spapr_vio_qirq(VIOsPAPRDevice *dev) {