Message ID | 20161024010729.GD19629@umbus.fritz.box (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Quoting David Gibson (2016-10-23 20:07:29) > 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 <david@gibson.dropbear.id.au> > > > Reviewed-by: Thomas Huth <thuth@redhat.com> > > [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 <david@gibson.dropbear.id.au> > 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 <david@gibson.dropbear.id.au> > Reviewed-by: Thomas Huth <thuth@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com> > --- > 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) > { > -- > 2.7.4 > > > > -- > David Gibson | I'll have my music baroque, and my code > david AT gibson.dropbear.id.au | minimalist, thank you. NOT _the_ _other_ > | _way_ _around_! > http://www.ozlabs.org/~dgibson
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) {