Re: [Qemu-devel] [PATCH 5/5] pseries: Move XICS initialization before cpu initialization

[Alexander Graf]

On 14.03.2013, at 02:53, David Gibson wrote:

> Currently, the pseries machine initializes the cpus, then the XICS
> interrupt controller.  However, to support the upcoming in-kernel XICS
> implementation we will need to initialize the irq controller before the
> vcpus.  This patch makes the necesssary rearrangement.  This means the

We're changing that notion in the in-kernel XICS discussions.  The flow will 
look like this:

  * create vcpus
  * create XICS
  * foreach (vcpu)
      * enable_cap(vcpu, CAP_XICS_SERVER, xics_handle)

However, that means we still need to know the maximum number of supported vcpus 
during the create phase. That number can be bigger than smp_cpus though, since 
you probably want to support hotplug add of CPUs later on.

Can't we just make the number of supported "interrupt servers" a constant?


Alex

> xics init code can no longer auto-detect the number of cpus ("interrupt
> servers" in XICS terminology) and so we must pass that in explicitly from
> the platform code.
> 
> Signed-off-by: Michael Ellerman <address@hidden>
> Signed-off-by: Ben Herrenschmidt <address@hidden>
> Signed-off-by: David Gibson <address@hidden>
> ---
> hw/ppc/spapr.c |   12 +++++++-----
> hw/ppc/xics.c  |   57 +++++++++++++++++++++++++-------------------------------
> hw/xics.h      |    3 ++-
> 3 files changed, 34 insertions(+), 38 deletions(-)
> 
> diff --git a/hw/ppc/spapr.c b/hw/ppc/spapr.c
> index 7293082..b2c9b42 100644
> --- a/hw/ppc/spapr.c
> +++ b/hw/ppc/spapr.c
> @@ -791,6 +791,11 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>         spapr->htab_shift++;
>     }
> 
> +    /* Set up Interrupt Controller before we create the VCPUs */
> +    spapr->icp = xics_system_init(smp_cpus * kvmppc_smt_threads() / 
> smp_threads,
> +                                  XICS_IRQS);
> +    spapr->next_irq = XICS_IRQ_BASE;
> +
>     /* init CPUs */
>     if (cpu_model == NULL) {
>         cpu_model = kvm_enabled() ? "host" : "POWER7";
> @@ -803,6 +808,8 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>         }
>         env = &cpu->env;
> 
> +        xics_cpu_setup(spapr->icp, cpu);
> +
>         /* Set time-base frequency to 512 MHz */
>         cpu_ppc_tb_init(env, TIMEBASE_FREQ);
> 
> @@ -842,11 +849,6 @@ static void ppc_spapr_init(QEMUMachineInitArgs *args)
>     }
>     g_free(filename);
> 
> -
> -    /* Set up Interrupt Controller */
> -    spapr->icp = xics_system_init(XICS_IRQS);
> -    spapr->next_irq = XICS_IRQ_BASE;
> -
>     /* Set up EPOW events infrastructure */
>     spapr_events_init(spapr);
> 
> diff --git a/hw/ppc/xics.c b/hw/ppc/xics.c
> index c3ef12f..374da5b 100644
> --- a/hw/ppc/xics.c
> +++ b/hw/ppc/xics.c
> @@ -521,45 +521,38 @@ static void xics_reset(void *opaque)
>     }
> }
> 
> -struct icp_state *xics_system_init(int nr_irqs)
> +void xics_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu)
> {
> -    CPUPPCState *env;
> -    CPUState *cpu;
> -    int max_server_num;
> -    struct icp_state *icp;
> -    struct ics_state *ics;
> +    CPUState *cs = CPU(cpu);
> +    CPUPPCState *env = &cpu->env;
> +    struct icp_server_state *ss = &icp->ss[cs->cpu_index];
> 
> -    max_server_num = -1;
> -    for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -        cpu = CPU(ppc_env_get_cpu(env));
> -        if (cpu->cpu_index > max_server_num) {
> -            max_server_num = cpu->cpu_index;
> -        }
> -    }
> +    assert(cs->cpu_index < icp->nr_servers);
> 
> -    icp = g_malloc0(sizeof(*icp));
> -    icp->nr_servers = max_server_num + 1;
> -    icp->ss = g_malloc0(icp->nr_servers*sizeof(struct icp_server_state));
> +    switch (PPC_INPUT(env)) {
> +    case PPC_FLAGS_INPUT_POWER7:
> +        ss->output = env->irq_inputs[POWER7_INPUT_INT];
> +        break;
> 
> -    for (env = first_cpu; env != NULL; env = env->next_cpu) {
> -        cpu = CPU(ppc_env_get_cpu(env));
> -        struct icp_server_state *ss = &icp->ss[cpu->cpu_index];
> +    case PPC_FLAGS_INPUT_970:
> +        ss->output = env->irq_inputs[PPC970_INPUT_INT];
> +        break;
> 
> -        switch (PPC_INPUT(env)) {
> -        case PPC_FLAGS_INPUT_POWER7:
> -            ss->output = env->irq_inputs[POWER7_INPUT_INT];
> -            break;
> +    default:
> +        fprintf(stderr, "XICS interrupt controller does not support this CPU 
> "
> +                "bus model\n");
> +        abort();
> +    }
> +}
> 
> -        case PPC_FLAGS_INPUT_970:
> -            ss->output = env->irq_inputs[PPC970_INPUT_INT];
> -            break;
> +struct icp_state *xics_system_init(int nr_servers, int nr_irqs)
> +{
> +    struct icp_state *icp;
> +    struct ics_state *ics;
> 
> -        default:
> -            hw_error("XICS interrupt model does not support this CPU bus "
> -                     "model\n");
> -            exit(1);
> -        }
> -    }
> +    icp = g_malloc0(sizeof(*icp));
> +    icp->nr_servers = nr_servers;
> +    icp->ss = g_malloc0(icp->nr_servers*sizeof(struct icp_server_state));
> 
>     ics = g_malloc0(sizeof(*ics));
>     ics->nr_irqs = nr_irqs;
> diff --git a/hw/xics.h b/hw/xics.h
> index c3bf008..6bce042 100644
> --- a/hw/xics.h
> +++ b/hw/xics.h
> @@ -35,6 +35,7 @@ struct icp_state;
> qemu_irq xics_get_qirq(struct icp_state *icp, int irq);
> void xics_set_irq_type(struct icp_state *icp, int irq, bool lsi);
> 
> -struct icp_state *xics_system_init(int nr_irqs);
> +struct icp_state *xics_system_init(int nr_servers, int nr_irqs);
> +void xics_cpu_setup(struct icp_state *icp, PowerPCCPU *cpu);
> 
> #endif /* __XICS_H__ */
> -- 
> 1.7.10.4
>