qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Cédric Le Goater" <clg@kaod.org>
To: David Gibson <david@gibson.dropbear.id.au>, groug@kaod.org
Cc: lvivier@redhat.com, qemu-ppc@nongnu.org,
	mdroth@linux.vnet.ibm.com, qemu-devel@nongnu.org
Subject: Re: [for-5.0 3/4] spapr: Fold h_cas_compose_response() into h_client_architecture_support()
Date: Mon, 2 Dec 2019 08:04:07 +0100	[thread overview]
Message-ID: <b0a972a1-df7c-475c-aa48-3adc85744dbe@kaod.org> (raw)
In-Reply-To: <20191129053356.232413-4-david@gibson.dropbear.id.au>

On 29/11/2019 06:33, David Gibson wrote:
> spapr_h_cas_compose_response() handles the last piece of the PAPR feature
> negotiation process invoked via the ibm,client-architecture-support OF
> call.  Its only caller is h_client_architecture_support() which handles
> most of the rest of that process.
> 
> I believe it was place in a separate file originally to handle some fiddly
> dependencies between functions, but mostly it's just confusing to have
> the CAS process split into two pieces like this.  Now that compose response
> is simplified (by just generating the whole device tree anew), it's cleaner
> to just fold it into h_client_architecture_support().
> 
> Signed-off-by: David Gibson <david@gibson.dropbear.id.au>

Reviewed-by: Cedric Le Goater <clg@fr.ibm.com>

> ---
>  hw/ppc/spapr.c         | 61 +-----------------------------------------
>  hw/ppc/spapr_hcall.c   | 55 ++++++++++++++++++++++++++++++++++---
>  include/hw/ppc/spapr.h |  4 +--
>  3 files changed, 54 insertions(+), 66 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index d34e317f48..5187f5b0a5 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -76,7 +76,6 @@
>  #include "hw/nmi.h"
>  #include "hw/intc/intc.h"
>  
> -#include "qemu/cutils.h"
>  #include "hw/ppc/spapr_cpu_core.h"
>  #include "hw/mem/memory-device.h"
>  #include "hw/ppc/spapr_tpm_proxy.h"
> @@ -897,63 +896,6 @@ out:
>      return ret;
>  }
>  
> -static bool spapr_hotplugged_dev_before_cas(void)
> -{
> -    Object *drc_container, *obj;
> -    ObjectProperty *prop;
> -    ObjectPropertyIterator iter;
> -
> -    drc_container = container_get(object_get_root(), "/dr-connector");
> -    object_property_iter_init(&iter, drc_container);
> -    while ((prop = object_property_iter_next(&iter))) {
> -        if (!strstart(prop->type, "link<", NULL)) {
> -            continue;
> -        }
> -        obj = object_property_get_link(drc_container, prop->name, NULL);
> -        if (spapr_drc_needed(obj)) {
> -            return true;
> -        }
> -    }
> -    return false;
> -}
> -
> -static void *spapr_build_fdt(SpaprMachineState *spapr, bool reset,
> -                             size_t space);
> -
> -int spapr_h_cas_compose_response(SpaprMachineState *spapr,
> -                                 target_ulong addr, target_ulong size,
> -                                 SpaprOptionVector *ov5_updates)
> -{
> -    void *fdt;
> -    SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> -
> -    if (spapr_hotplugged_dev_before_cas()) {
> -        return 1;
> -    }
> -
> -    if (size < sizeof(hdr)) {
> -        error_report("SLOF provided insufficient CAS buffer "
> -                     TARGET_FMT_lu " (min: %zu)", size, sizeof(hdr));
> -        exit(EXIT_FAILURE);
> -    }
> -
> -    size -= sizeof(hdr);
> -
> -    fdt = spapr_build_fdt(spapr, false, size);
> -    _FDT((fdt_pack(fdt)));
> -
> -    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(spapr->fdt_blob);
> -    spapr->fdt_size = fdt_totalsize(fdt);
> -    spapr->fdt_initial_size = spapr->fdt_size;
> -    spapr->fdt_blob = fdt;
> -
> -    return 0;
> -}
> -
>  static void spapr_dt_rtas(SpaprMachineState *spapr, void *fdt)
>  {
>      MachineState *ms = MACHINE(spapr);
> @@ -1191,8 +1133,7 @@ static void spapr_dt_hypervisor(SpaprMachineState *spapr, void *fdt)
>      }
>  }
>  
> -static void *spapr_build_fdt(SpaprMachineState *spapr, bool reset,
> -                             size_t space)
> +void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space)
>  {
>      MachineState *machine = MACHINE(spapr);
>      MachineClass *mc = MACHINE_GET_CLASS(machine);
> diff --git a/hw/ppc/spapr_hcall.c b/hw/ppc/spapr_hcall.c
> index 05a7ca275b..0f19be794c 100644
> --- a/hw/ppc/spapr_hcall.c
> +++ b/hw/ppc/spapr_hcall.c
> @@ -1,4 +1,5 @@
>  #include "qemu/osdep.h"
> +#include "qemu/cutils.h"
>  #include "qapi/error.h"
>  #include "sysemu/hw_accel.h"
>  #include "sysemu/runstate.h"
> @@ -15,6 +16,7 @@
>  #include "cpu-models.h"
>  #include "trace.h"
>  #include "kvm_ppc.h"
> +#include "hw/ppc/fdt.h"
>  #include "hw/ppc/spapr_ovec.h"
>  #include "mmu-book3s-v3.h"
>  #include "hw/mem/memory-device.h"
> @@ -1638,6 +1640,26 @@ static uint32_t cas_check_pvr(SpaprMachineState *spapr, PowerPCCPU *cpu,
>      return best_compat;
>  }
>  
> +static bool spapr_hotplugged_dev_before_cas(void)
> +{
> +    Object *drc_container, *obj;
> +    ObjectProperty *prop;
> +    ObjectPropertyIterator iter;
> +
> +    drc_container = container_get(object_get_root(), "/dr-connector");
> +    object_property_iter_init(&iter, drc_container);
> +    while ((prop = object_property_iter_next(&iter))) {
> +        if (!strstart(prop->type, "link<", NULL)) {
> +            continue;
> +        }
> +        obj = object_property_get_link(drc_container, prop->name, NULL);
> +        if (spapr_drc_needed(obj)) {
> +            return true;
> +        }
> +    }
> +    return false;
> +}
> +
>  static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>                                                    SpaprMachineState *spapr,
>                                                    target_ulong opcode,
> @@ -1645,6 +1667,8 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>  {
>      /* Working address in data buffer */
>      target_ulong addr = ppc64_phys_to_real(args[0]);
> +    target_ulong fdt_buf = args[1];
> +    target_ulong fdt_bufsize = args[2];
>      target_ulong ov_table;
>      uint32_t cas_pvr;
>      SpaprOptionVector *ov1_guest, *ov5_guest, *ov5_cas_old, *ov5_updates;
> @@ -1788,16 +1812,41 @@ static target_ulong h_client_architecture_support(PowerPCCPU *cpu,
>  
>      spapr_irq_update_active_intc(spapr);
>  
> +    if (spapr_hotplugged_dev_before_cas()) {
> +        spapr->cas_reboot = true;
> +    }
> +
>      if (!spapr->cas_reboot) {
> +        void *fdt;
> +        SpaprDeviceTreeUpdateHeader hdr = { .version_id = 1 };
> +
>          /* If spapr_machine_reset() did not set up a HPT but one is necessary
>           * (because the guest isn't going to use radix) then set it up here. */
>          if ((spapr->patb_entry & PATE1_GR) && !guest_radix) {
>              /* legacy hash or new hash: */
>              spapr_setup_hpt_and_vrma(spapr);
>          }
> -        spapr->cas_reboot =
> -            (spapr_h_cas_compose_response(spapr, args[1], args[2],
> -                                          ov5_updates) != 0);
> +
> +        if (fdt_bufsize < sizeof(hdr)) {
> +            error_report("SLOF provided insufficient CAS buffer "
> +                         TARGET_FMT_lu " (min: %zu)", fdt_bufsize, sizeof(hdr));
> +            exit(EXIT_FAILURE);
> +        }
> +
> +        fdt_bufsize -= sizeof(hdr);
> +
> +        fdt = spapr_build_fdt(spapr, false, fdt_bufsize);
> +        _FDT((fdt_pack(fdt)));
> +
> +        cpu_physical_memory_write(fdt_buf, &hdr, sizeof(hdr));
> +        cpu_physical_memory_write(fdt_buf + sizeof(hdr), fdt,
> +                                  fdt_totalsize(fdt));
> +        trace_spapr_cas_continue(fdt_totalsize(fdt) + sizeof(hdr));
> +
> +        g_free(spapr->fdt_blob);
> +        spapr->fdt_size = fdt_totalsize(fdt);
> +        spapr->fdt_initial_size = spapr->fdt_size;
> +        spapr->fdt_blob = fdt;
>      }
>  
>      spapr_ovec_cleanup(ov5_updates);
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index d5ab5ea7b2..61f005c6f6 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -766,11 +766,9 @@ struct SpaprEventLogEntry {
>      QTAILQ_ENTRY(SpaprEventLogEntry) next;
>  };
>  
> +void *spapr_build_fdt(SpaprMachineState *spapr, bool reset, size_t space);
>  void spapr_events_init(SpaprMachineState *sm);
>  void spapr_dt_events(SpaprMachineState *sm, void *fdt);
> -int spapr_h_cas_compose_response(SpaprMachineState *sm,
> -                                 target_ulong addr, target_ulong size,
> -                                 SpaprOptionVector *ov5_updates);
>  void close_htab_fd(SpaprMachineState *spapr);
>  void spapr_setup_hpt_and_vrma(SpaprMachineState *spapr);
>  void spapr_free_hpt(SpaprMachineState *spapr);
> 



  reply	other threads:[~2019-12-02  7:09 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-29  5:33 [for-5.0 0/4] spapr: Improvements to CAS feature negotiation David Gibson
2019-11-29  5:33 ` [for-5.0 1/4] spapr: Don't trigger a CAS reboot for XICS/XIVE mode changeover David Gibson
2019-12-02  7:03   ` Cédric Le Goater
2019-12-02  7:50   ` Greg Kurz
2019-12-02  7:59     ` David Gibson
2019-11-29  5:33 ` [for-5.0 2/4] spapr: Improve handling of fdt buffer size David Gibson
2019-12-02  7:04   ` Cédric Le Goater
2019-12-02  7:52   ` Greg Kurz
2019-11-29  5:33 ` [for-5.0 3/4] spapr: Fold h_cas_compose_response() into h_client_architecture_support() David Gibson
2019-12-02  7:04   ` Cédric Le Goater [this message]
2019-12-02  8:23   ` Greg Kurz
2019-12-02  8:40     ` David Gibson
2019-11-29  5:33 ` [for-5.0 4/4] spapr: Simplify ovec diff David Gibson
2019-12-02  7:04   ` Cédric Le Goater
2019-12-02  7:05 ` [for-5.0 0/4] spapr: Improvements to CAS feature negotiation Cédric Le Goater
2019-12-03  5:00   ` David Gibson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b0a972a1-df7c-475c-aa48-3adc85744dbe@kaod.org \
    --to=clg@kaod.org \
    --cc=david@gibson.dropbear.id.au \
    --cc=groug@kaod.org \
    --cc=lvivier@redhat.com \
    --cc=mdroth@linux.vnet.ibm.com \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-ppc@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).