Re: [Qemu-ppc] [PATCH v4 3/9] spapr: Move spapr_cpu_init() to spapr_cpu_core.c

[David Gibson]

On Fri, Jun 10, 2016 at 06:29:02AM +0530, Bharata B Rao wrote:
> Start consolidating CPU init related routines in spapr_cpu_core.c. As
> part of this, move spapr_cpu_init() and its dependencies from spapr.c
> to spapr_cpu_core.c
> 
> No functionality change in this patch.
> 
> Signed-off-by: Bharata B Rao <address@hidden>

One nit..

> ---
>  hw/ppc/spapr.c          | 47 -----------------------------------------------
>  hw/ppc/spapr_cpu_core.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/spapr.h  |  2 ++
>  3 files changed, 50 insertions(+), 47 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 87f4e53..adaca42 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -89,8 +89,6 @@
>  
>  #define MIN_RMA_SLOF            128UL
>  
> -#define TIMEBASE_FREQ           512000000ULL
> -
>  #define PHANDLE_XICP            0x00001111
>  
>  #define HTAB_SIZE(spapr)        (1ULL << ((spapr)->htab_shift))
> @@ -1181,26 +1179,6 @@ static void ppc_spapr_reset(void)
>  
>  }
>  
> -static void spapr_cpu_reset(void *opaque)
> -{
> -    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> -    PowerPCCPU *cpu = opaque;
> -    CPUState *cs = CPU(cpu);
> -    CPUPPCState *env = &cpu->env;
> -
> -    cpu_reset(cs);
> -
> -    /* All CPUs start halted.  CPU0 is unhalted from the machine level
> -     * reset code and the rest are explicitly started up by the guest
> -     * using an RTAS call */
> -    cs->halted = 1;
> -
> -    env->spr[SPR_HIOR] = 0;
> -
> -    ppc_hash64_set_external_hpt(cpu, spapr->htab, spapr->htab_shift,
> -                                &error_fatal);
> -}
> -
>  static void spapr_create_nvram(sPAPRMachineState *spapr)
>  {
>      DeviceState *dev = qdev_create(&spapr->vio_bus->bus, "spapr-nvram");
> @@ -1606,31 +1584,6 @@ static void spapr_boot_set(void *opaque, const char 
> *boot_device,
>      machine->boot_order = g_strdup(boot_device);
>  }
>  
> -void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
> -{
> -    CPUPPCState *env = &cpu->env;
> -
> -    /* Set time-base frequency to 512 MHz */
> -    cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> -
> -    /* Enable PAPR mode in TCG or KVM */
> -    cpu_ppc_set_papr(cpu);
> -
> -    if (cpu->max_compat) {
> -        Error *local_err = NULL;
> -
> -        ppc_set_compat(cpu, cpu->max_compat, &local_err);
> -        if (local_err) {
> -            error_propagate(errp, local_err);
> -            return;
> -        }
> -    }
> -
> -    xics_cpu_setup(spapr->icp, cpu);
> -
> -    qemu_register_reset(spapr_cpu_reset, cpu);
> -}
> -
>  /*
>   * Reset routine for LMB DR devices.
>   *
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index b8b97c9..e9eb754 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -14,6 +14,54 @@
>  #include "qapi/error.h"
>  #include <sysemu/cpus.h>
>  #include "target-ppc/kvm_ppc.h"
> +#include "hw/ppc/ppc.h"
> +#include "target-ppc/mmu-hash64.h"
> +#include <sysemu/numa.h>
> +
> +static void spapr_cpu_reset(void *opaque)
> +{
> +    sPAPRMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
> +    PowerPCCPU *cpu = opaque;
> +    CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
> +
> +    cpu_reset(cs);
> +
> +    /* All CPUs start halted.  CPU0 is unhalted from the machine level
> +     * reset code and the rest are explicitly started up by the guest
> +     * using an RTAS call */
> +    cs->halted = 1;
> +
> +    env->spr[SPR_HIOR] = 0;
> +
> +    ppc_hash64_set_external_hpt(cpu, spapr->htab, spapr->htab_shift,
> +                                &error_fatal);
> +}
> +
> +void spapr_cpu_init(sPAPRMachineState *spapr, PowerPCCPU *cpu, Error **errp)
> +{
> +    CPUPPCState *env = &cpu->env;
> +
> +    /* Set time-base frequency to 512 MHz */
> +    cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> +
> +    /* Enable PAPR mode in TCG or KVM */
> +    cpu_ppc_set_papr(cpu);
> +
> +    if (cpu->max_compat) {
> +        Error *local_err = NULL;
> +
> +        ppc_set_compat(cpu, cpu->max_compat, &local_err);
> +        if (local_err) {
> +            error_propagate(errp, local_err);
> +            return;
> +        }
> +    }
> +
> +    xics_cpu_setup(spapr->icp, cpu);
> +
> +    qemu_register_reset(spapr_cpu_reset, cpu);
> +}
>  
>  static int spapr_cpu_core_realize_child(Object *child, void *opaque)
>  {
> diff --git a/include/hw/ppc/spapr.h b/include/hw/ppc/spapr.h
> index 4ff14d6..e372dae 100644
> --- a/include/hw/ppc/spapr.h
> +++ b/include/hw/ppc/spapr.h
> @@ -16,6 +16,8 @@ typedef struct sPAPREventLogEntry sPAPREventLogEntry;
>  #define HPTE64_V_HPTE_DIRTY     0x0000000000000040ULL
>  #define SPAPR_ENTRY_POINT       0x100
>  
> +#define TIMEBASE_FREQ           512000000ULL
> +

Since this is now in a header which could be included anywhere, this
really needs an SPAPR_PREFIX.  I'll make that tweak myself when I
merge it.

>  typedef struct sPAPRMachineClass sPAPRMachineClass;
>  typedef struct sPAPRMachineState sPAPRMachineState;
>  

-- 
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