On Tue, Apr 26, 2016 at 08:13:21PM +0200, Thomas Huth wrote: > On 20.04.2016 04:33, David Gibson wrote: > > Currently the pseries code builds a "skeleton" device tree at machine init > > time, then adds a bunch of stuff to it at reset. Over time, more and more > > logic has had to be moved from init to reset time, and there's really no > > advantage to doing any of it at init time. > > > > This patch removes the init time spapr_create_fdt_skel() and moves its > > logic into the reset time spapr_build_fdt(). There's still a fairly > > pointless divide between the "skeleton" logic (using libfdt serial-write > > functions) and the "tweak" logic (using libfdt random access functions) > > but at least it all happens at the same time, making further consolidation > > easier. > > > > Signed-off-by: David Gibson <da...@gibson.dropbear.id.au> > > --- > > hw/ppc/spapr.c | 398 > > ++++++++++++++++++++++++------------------------- > > include/hw/ppc/spapr.h | 1 - > > 2 files changed, 192 insertions(+), 207 deletions(-) > > > > diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c > > index da10136..6e1192f 100644 > > --- a/hw/ppc/spapr.c > > +++ b/hw/ppc/spapr.c > ... > > @@ -901,7 +702,7 @@ static void *spapr_build_fdt(sPAPRMachineState *spapr, > > hwaddr rtas_addr, > > hwaddr rtas_size) > > { > > - MachineState *machine = MACHINE(qdev_get_machine()); > > + MachineState *machine = MACHINE(spapr); > > sPAPRMachineClass *smc = SPAPR_MACHINE_GET_CLASS(machine); > > const char *boot_device = machine->boot_order; > > int ret, i; > > @@ -909,11 +710,200 @@ static void *spapr_build_fdt(sPAPRMachineState > > *spapr, > > char *bootlist; > > void *fdt; > > sPAPRPHBState *phb; > > + 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)}; > > + uint32_t interrupt_server_ranges_prop[] = {0, cpu_to_be32(max_cpus)}; > > + unsigned char vec5[] = {0x0, 0x0, 0x0, 0x0, 0x0, 0x80}; > > + char *buf; > > + > > + add_str(hypertas, "hcall-pft"); > > + add_str(hypertas, "hcall-term"); > > + add_str(hypertas, "hcall-dabr"); > > + add_str(hypertas, "hcall-interrupt"); > > + add_str(hypertas, "hcall-tce"); > > + add_str(hypertas, "hcall-vio"); > > + add_str(hypertas, "hcall-splpar"); > > + add_str(hypertas, "hcall-bulk"); > > + add_str(hypertas, "hcall-set-mode"); > > + add_str(qemu_hypertas, "hcall-memop1"); > > + > > + fdt = g_malloc0(FDT_MAX_SIZE); > > + _FDT((fdt_create(fdt, FDT_MAX_SIZE))); > > + > > + if (spapr->kernel_size) { > > + _FDT((fdt_add_reservemap_entry(fdt, KERNEL_LOAD_ADDR, > > + spapr->kernel_size))); > > + } > > + if (spapr->initrd_size) { > > + _FDT((fdt_add_reservemap_entry(fdt, spapr->initrd_base, > > + spapr->initrd_size))); > > + } > > + _FDT((fdt_finish_reservemap(fdt))); > > + > > + /* Root node */ > > + _FDT((fdt_begin_node(fdt, ""))); > > + _FDT((fdt_property_string(fdt, "device_type", "chrp"))); > > + _FDT((fdt_property_string(fdt, "model", "IBM pSeries (emulated by > > qemu)"))); > > + _FDT((fdt_property_string(fdt, "compatible", "qemu,pseries"))); > > + > > + /* > > + * Add info to guest to indentify which host is it being run on > > + * and what is the uuid of the guest > > + */ > > + if (kvmppc_get_host_model(&buf)) { > > + _FDT((fdt_property_string(fdt, "host-model", buf))); > > + g_free(buf); > > + } > > + if (kvmppc_get_host_serial(&buf)) { > > + _FDT((fdt_property_string(fdt, "host-serial", buf))); > > + g_free(buf); > > + } > > > > - fdt = g_malloc(FDT_MAX_SIZE); > > + buf = g_strdup_printf(UUID_FMT, qemu_uuid[0], qemu_uuid[1], > > + qemu_uuid[2], qemu_uuid[3], qemu_uuid[4], > > + qemu_uuid[5], qemu_uuid[6], qemu_uuid[7], > > + qemu_uuid[8], qemu_uuid[9], qemu_uuid[10], > > + qemu_uuid[11], qemu_uuid[12], qemu_uuid[13], > > + qemu_uuid[14], qemu_uuid[15]); > > + > > + _FDT((fdt_property_string(fdt, "vm,uuid", buf))); > > + if (qemu_uuid_set) { > > + _FDT((fdt_property_string(fdt, "system-id", buf))); > > + } > > + g_free(buf); > > + > > + if (qemu_get_vm_name()) { > > + _FDT((fdt_property_string(fdt, "ibm,partition-name", > > + qemu_get_vm_name()))); > > + } > > + > > + _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"))); > > + > > + if (!kvm_enabled() || kvmppc_spapr_use_multitce()) { > > + add_str(hypertas, "hcall-multi-tce"); > > + } > > + _FDT((fdt_property(fdt, "ibm,hypertas-functions", hypertas->str, > > + hypertas->len))); > > + g_string_free(hypertas, TRUE); > > + _FDT((fdt_property(fdt, "qemu,hypertas-functions", qemu_hypertas->str, > > + qemu_hypertas->len))); > > + g_string_free(qemu_hypertas, TRUE); > > + > > + _FDT((fdt_property(fdt, "ibm,associativity-reference-points", > > + refpoints, sizeof(refpoints)))); > > + > > + _FDT((fdt_property_cell(fdt, "rtas-error-log-max", > > RTAS_ERROR_LOG_MAX))); > > + _FDT((fdt_property_cell(fdt, "rtas-event-scan-rate", > > + RTAS_EVENT_SCAN_RATE))); > > + > > + if (msi_nonbroken) { > > + _FDT((fdt_property(fdt, "ibm,change-msix-capable", NULL, 0))); > > + } > > + > > + /* > > + * According to PAPR, rtas ibm,os-term does not guarantee a return > > + * back to the guest cpu. > > + * > > + * While an additional ibm,extended-os-term property indicates that > > + * rtas call return will always occur. Set this property. > > + */ > > + _FDT((fdt_property(fdt, "ibm,extended-os-term", NULL, 0))); > > + > > + _FDT((fdt_end_node(fdt))); > > + > > + /* interrupt controller */ > > + _FDT((fdt_begin_node(fdt, "interrupt-controller"))); > > + > > + _FDT((fdt_property_string(fdt, "device_type", > > + "PowerPC-External-Interrupt-Presentation"))); > > + _FDT((fdt_property_string(fdt, "compatible", "IBM,ppc-xicp"))); > > + _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0))); > > + _FDT((fdt_property(fdt, "ibm,interrupt-server-ranges", > > + interrupt_server_ranges_prop, > > + sizeof(interrupt_server_ranges_prop)))); > > + _FDT((fdt_property_cell(fdt, "#interrupt-cells", 2))); > > + _FDT((fdt_property_cell(fdt, "linux,phandle", PHANDLE_XICP))); > > + _FDT((fdt_property_cell(fdt, "phandle", PHANDLE_XICP))); > > + > > + _FDT((fdt_end_node(fdt))); > > + > > + /* vdevice */ > > + _FDT((fdt_begin_node(fdt, "vdevice"))); > > + > > + _FDT((fdt_property_string(fdt, "device_type", "vdevice"))); > > + _FDT((fdt_property_string(fdt, "compatible", "IBM,vdevice"))); > > + _FDT((fdt_property_cell(fdt, "#address-cells", 0x1))); > > + _FDT((fdt_property_cell(fdt, "#size-cells", 0x0))); > > + _FDT((fdt_property_cell(fdt, "#interrupt-cells", 0x2))); > > + _FDT((fdt_property(fdt, "interrupt-controller", NULL, 0))); > > + > > + _FDT((fdt_end_node(fdt))); > > + > > + /* event-sources */ > > + spapr_events_fdt_skel(fdt, spapr->check_exception_irq); > > + > > + /* /hypervisor node */ > > + if (kvm_enabled()) { > > + uint8_t hypercall[16]; > > + > > + /* indicate KVM hypercall interface */ > > + _FDT((fdt_begin_node(fdt, "hypervisor"))); > > + _FDT((fdt_property_string(fdt, "compatible", "linux,kvm"))); > > + if (kvmppc_has_cap_fixup_hcalls()) { > > + /* > > + * Older KVM versions with older guest kernels were broken > > with the > > + * magic page, don't allow the guest to map it. > > + */ > > + if (!kvmppc_get_hypercall(first_cpu->env_ptr, hypercall, > > + sizeof(hypercall))) { > > + _FDT((fdt_property(fdt, "hcall-instructions", hypercall, > > + sizeof(hypercall)))); > > + } > > + } > > + _FDT((fdt_end_node(fdt))); > > + } > > + > > + _FDT((fdt_end_node(fdt))); /* close root node */ > > + _FDT((fdt_finish(fdt))); > > The old spapr_finalize_fdt() was already quite big - if you now move all > this code hiere, this function becomes a really bloated. So while you're > reworking all this stuff ... maybe it would be nicer to split the big > chunk into separate functions, e.g. one function for each device tree node?
Perhaps - it's all in sequence code, though, so I'm not particularly fussed by it being long. It will also shrink a bit as we fold together the fdt and qdt code. > > /* open out the base tree into a temp buffer for the final tweaks */ > > - _FDT((fdt_open_into(spapr->fdt_skel, fdt, FDT_MAX_SIZE))); > > + _FDT((fdt_open_into(fdt, fdt, FDT_MAX_SIZE))); > > Can fdt_open_into deal with input = output pointer? I haven't checked > it, but that looks somewhat strange. Is this line still required at all > after you moved to code here? Yes, it absolutely can. Looks like I never got around to writing a doc block for fdt_open_into() but it was always designed to handle both the expanding in place and expanding to elsewhere cases. > > ret = spapr_populate_memory(spapr, fdt); > > if (ret < 0) { > > @@ -1980,10 +1970,6 @@ static void ppc_spapr_init(MachineState *machine) > > register_savevm_live(NULL, "spapr/htab", -1, 1, > > &savevm_htab_handlers, spapr); > > > > - /* Prepare the device tree */ > > - spapr->fdt_skel = spapr_create_fdt_skel(spapr); > > - assert(spapr->fdt_skel != NULL); > > - > > /* used by RTAS */ > > QTAILQ_INIT(&spapr->ccs_list); > > qemu_register_reset(spapr_ccs_reset_hook, spapr); > > diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h > > index 88f29a8..cd72586 100644 > > --- a/include/hw/ppc/spapr.h > > +++ b/include/hw/ppc/spapr.h > > @@ -62,7 +62,6 @@ struct sPAPRMachineState { > > bool kernel_le; > > uint32_t initrd_base; > > long initrd_size; > > - void *fdt_skel; > > uint64_t rtc_offset; /* Now used only during incoming migration */ > > struct PPCTimebase tb; > > bool has_graphics; > > Thomas > -- 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
signature.asc
Description: PGP signature