[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH qemu] spapr: Render full FDT on ibm, client-arch
From: |
David Gibson |
Subject: |
Re: [Qemu-devel] [PATCH qemu] spapr: Render full FDT on ibm, client-architecture-support |
Date: |
Mon, 26 Aug 2019 17:44:31 +1000 |
User-agent: |
Mutt/1.12.1 (2019-06-15) |
On Mon, Aug 26, 2019 at 02:31:26PM +1000, Alexey Kardashevskiy wrote:
> The ibm,client-architecture-support call is a way for the guest to
> negotiate capabilities with a hypervisor. It is implemented as:
> - the guest calls SLOF via client interface;
> - SLOF calls QEMU (H_CAS hypercall) with an options vector from the guest;
> - QEMU returns a device tree diff (which uses FDT format with
> an additional header before it);
> - SLOF walks through the partial diff tree and updates its internal tree
> with the values from the diff.
>
> This changes QEMU to simply re-render the entire tree and send it as
> an update. SLOF can handle this already mostly, [1] is needed before this
> can be applied.
>
> The benefit is reduced code size as there is no need for another set of
> DT rendering helpers such as spapr_fixup_cpu_dt().
>
> The downside is that the updates are bigger now (as they include all
> nodes and properties) but the difference on a '-smp 256,threads=1' system
> before/after is 2.35s vs. 2.5s.
>
> While at this, add a missing g_free(fdt) if the resulting tree is bigger
> than the space allocated by SLOF. Also, store the resulting tree in
> the spapr machine to have the latest valid FDT copy possible (this should
> not matter much as H_UPDATE_DT happens right after that but nevertheless).
>
> [1] https://patchwork.ozlabs.org/patch/1152915/
>
> Signed-off-by: Alexey Kardashevskiy <address@hidden>
Reviewed-by: David Gibson <address@hidden>
Can you wrap that up with the SLOF update in a pull request for me?
> ---
> hw/ppc/spapr.c | 90 ++++++--------------------------------------------
> 1 file changed, 10 insertions(+), 80 deletions(-)
>
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index baedadf20b8c..6dea5947afbc 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -295,65 +295,6 @@ static void spapr_populate_pa_features(SpaprMachineState
> *spapr,
> _FDT((fdt_setprop(fdt, offset, "ibm,pa-features", pa_features,
> pa_size)));
> }
>
> -static int spapr_fixup_cpu_dt(void *fdt, SpaprMachineState *spapr)
> -{
> - MachineState *ms = MACHINE(spapr);
> - int ret = 0, offset, cpus_offset;
> - CPUState *cs;
> - char cpu_model[32];
> - uint32_t pft_size_prop[] = {0, cpu_to_be32(spapr->htab_shift)};
> -
> - CPU_FOREACH(cs) {
> - PowerPCCPU *cpu = POWERPC_CPU(cs);
> - DeviceClass *dc = DEVICE_GET_CLASS(cs);
> - int index = spapr_get_vcpu_id(cpu);
> - int compat_smt = MIN(ms->smp.threads, ppc_compat_max_vthreads(cpu));
> -
> - if (!spapr_is_thread0_in_vcore(spapr, cpu)) {
> - continue;
> - }
> -
> - snprintf(cpu_model, 32, "%s@%x", dc->fw_name, index);
> -
> - cpus_offset = fdt_path_offset(fdt, "/cpus");
> - if (cpus_offset < 0) {
> - cpus_offset = fdt_add_subnode(fdt, 0, "cpus");
> - if (cpus_offset < 0) {
> - return cpus_offset;
> - }
> - }
> - offset = fdt_subnode_offset(fdt, cpus_offset, cpu_model);
> - if (offset < 0) {
> - offset = fdt_add_subnode(fdt, cpus_offset, cpu_model);
> - if (offset < 0) {
> - return offset;
> - }
> - }
> -
> - ret = fdt_setprop(fdt, offset, "ibm,pft-size",
> - pft_size_prop, sizeof(pft_size_prop));
> - if (ret < 0) {
> - return ret;
> - }
> -
> - if (nb_numa_nodes > 1) {
> - ret = spapr_fixup_cpu_numa_dt(fdt, offset, cpu);
> - if (ret < 0) {
> - return ret;
> - }
> - }
> -
> - ret = spapr_fixup_cpu_smt_dt(fdt, offset, cpu, compat_smt);
> - if (ret < 0) {
> - return ret;
> - }
> -
> - spapr_populate_pa_features(spapr, cpu, fdt, offset,
> - spapr->cas_legacy_guest_workaround);
> - }
> - return ret;
> -}
> -
> static hwaddr spapr_node0_size(MachineState *machine)
> {
> if (nb_numa_nodes) {
> @@ -983,11 +924,13 @@ static bool spapr_hotplugged_dev_before_cas(void)
> return false;
> }
>
> +static void *spapr_build_fdt(SpaprMachineState *spapr);
> +
> int spapr_h_cas_compose_response(SpaprMachineState *spapr,
> target_ulong addr, target_ulong size,
> SpaprOptionVector *ov5_updates)
> {
> - void *fdt, *fdt_skel;
> + void *fdt;
> SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 };
>
> if (spapr_hotplugged_dev_before_cas()) {
> @@ -1003,28 +946,11 @@ int spapr_h_cas_compose_response(SpaprMachineState
> *spapr,
>
> size -= sizeof(hdr);
>
> - /* Create skeleton */
> - fdt_skel = g_malloc0(size);
> - _FDT((fdt_create(fdt_skel, size)));
> - _FDT((fdt_finish_reservemap(fdt_skel)));
> - _FDT((fdt_begin_node(fdt_skel, "")));
> - _FDT((fdt_end_node(fdt_skel)));
> - _FDT((fdt_finish(fdt_skel)));
> - fdt = g_malloc0(size);
> - _FDT((fdt_open_into(fdt_skel, fdt, size)));
> - g_free(fdt_skel);
> -
> - /* Fixup cpu nodes */
> - _FDT((spapr_fixup_cpu_dt(fdt, spapr)));
> -
> - if (spapr_dt_cas_updates(spapr, fdt, ov5_updates)) {
> - return -1;
> - }
> -
> - /* Pack resulting tree */
> + fdt = spapr_build_fdt(spapr);
> _FDT((fdt_pack(fdt)));
>
> if (fdt_totalsize(fdt) + sizeof(hdr) > size) {
> + g_free(fdt);
> trace_spapr_cas_failed(size);
> return -1;
> }
> @@ -1032,7 +958,11 @@ int spapr_h_cas_compose_response(SpaprMachineState
> *spapr,
> cpu_physical_memory_write(addr, &hdr, sizeof(hdr));
> cpu_physical_memory_write(addr + sizeof(hdr), fdt, fdt_totalsize(fdt));
> trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr));
> - g_free(fdt);
> +
> + g_free(spapr->fdt_blob);
> + spapr->fdt_size = fdt_totalsize(fdt);
> + spapr->fdt_initial_size = spapr->fdt_size;
> + spapr->fdt_blob = fdt;
>
> return 0;
> }
--
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