Re: [PATCH v4 5/7] ppc: Reset the interrupt presenter from the CPU reset handler

[Greg Kurz <groug@kaod.org>]

On Tue, 22 Oct 2019 15:46:30 +0200
Cédric Le Goater <clg@kaod.org> wrote:

> On the sPAPR machine and PowerNV machine, the interrupt presenters are
> created by a machine handler at the core level and are reset
> independently. This is not consistent and it raises issues when it
> comes to handle hot-plugged CPUs. In that case, the presenters are not
> reset. This is less of an issue in XICS, although a zero MFFR could
> be a concern, but in XIVE, the OS CAM line is not set and this breaks
> the presenting algorithm. The current code has workarounds which need
> a global cleanup.
> 
> Extend the sPAPR IRQ backend and the PowerNV Chip class with a new
> cpu_intc_reset() handler called by the CPU reset handler and remove
> the XiveTCTX reset handler which is now redundant.
> 
> Reviewed-by: Greg Kurz <groug@kaod.org>
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---

Just one nit caused by the changes from patch 3.

>  [...]
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index cc17bbfed829..127107ab7c63 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -40,10 +40,11 @@ static const char *pnv_core_cpu_typename(PnvCore *pc)
>      return cpu_type;
>  }
>  
> -static void pnv_core_cpu_reset(PowerPCCPU *cpu)
> +static void pnv_core_cpu_reset(PowerPCCPU *cpu, PnvChip *chip)
>  {
>      CPUState *cs = CPU(cpu);
>      CPUPPCState *env = &cpu->env;
> +    PnvChipClass *pcc;
>  
>      cpu_reset(cs);
>  
> @@ -54,6 +55,9 @@ static void pnv_core_cpu_reset(PowerPCCPU *cpu)
>      env->gpr[3] = PNV_FDT_ADDR;
>      env->nip = 0x10;
>      env->msr |= MSR_HVB; /* Hypervisor mode */
> +
> +    pcc = PNV_CHIP_GET_CLASS(chip);
> +    pcc->intc_reset(PNV_CHIP(chip), cpu);

chip is already of type PnvChip *, no need to cast.

>  }
>  
>  /*
> @@ -200,7 +204,7 @@ static void pnv_core_reset(void *dev)
>      int i;
>  
>      for (i = 0; i < cc->nr_threads; i++) {
> -        pnv_core_cpu_reset(pc->threads[i]);
> +        pnv_core_cpu_reset(pc->threads[i], pc->chip);
>      }
>  }
>  
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 2e34832d0ea2..ef7b27a66d56 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -32,6 +32,7 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>      PowerPCCPUClass *pcc = POWERPC_CPU_GET_CLASS(cpu);<
>      SpaprCpuState *spapr_cpu = spapr_cpu_state(cpu);
>      target_ulong lpcr;
> +    SpaprMachineState *spapr = SPAPR_MACHINE(qdev_get_machine());
>  
>      cpu_reset(cs);
>  
> @@ -76,9 +77,11 @@ static void spapr_reset_vcpu(PowerPCCPU *cpu)
>      spapr_cpu->dtl_addr = 0;
>      spapr_cpu->dtl_size = 0;
>  
> -    spapr_caps_cpu_apply(SPAPR_MACHINE(qdev_get_machine()), cpu);
> +    spapr_caps_cpu_apply(spapr, cpu);
>  
>      kvm_check_mmu(cpu, &error_fatal);
> +
> +    spapr_irq_cpu_intc_reset(spapr, cpu);
>  }
>  
>  void spapr_cpu_set_entry_state(PowerPCCPU *cpu, target_ulong nip, target_ulong r3)
> diff --git a/hw/ppc/spapr_irq.c b/hw/ppc/spapr_irq.c
> index 234d1073e518..b941608b69ba 100644
> --- a/hw/ppc/spapr_irq.c
> +++ b/hw/ppc/spapr_irq.c
> @@ -220,6 +220,20 @@ int spapr_irq_cpu_intc_create(SpaprMachineState *spapr,
>      return 0;
>  }
>  
> +void spapr_irq_cpu_intc_reset(SpaprMachineState *spapr, PowerPCCPU *cpu)
> +{
> +    SpaprInterruptController *intcs[] = ALL_INTCS(spapr);
> +    int i;
> +
> +    for (i = 0; i < ARRAY_SIZE(intcs); i++) {
> +        SpaprInterruptController *intc = intcs[i];
> +        if (intc) {
> +            SpaprInterruptControllerClass *sicc = SPAPR_INTC_GET_CLASS(intc);
> +            sicc->cpu_intc_reset(intc, cpu);
> +        }
> +    }
> +}
> +
>  static void spapr_set_irq(void *opaque, int irq, int level)
>  {
>      SpaprMachineState *spapr = SPAPR_MACHINE(opaque);