Re: [PATCH v5 0/7] ppc: reset the interrupt presenter from the CPU reset handler

[David Gibson]

On Tue, Oct 22, 2019 at 06:38:05PM +0200, Cédric Le Goater wrote:
> Hello,
> 
> On the sPAPR machine and PowerNV machine, the interrupt presenters are
> created by a machine handler at the core level and are reseted
> independently. This is not consistent and it raises issues when it
> comes to handle hot-plugged CPUs. In that case, the presenters are not
> reseted. 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.
> 
> Set the OS CAM line when the interrupt presenter of the sPAPR core is
> reseted. This will also cover the case of hot-plugged CPUs.

I'm not totally convinced whether every step here is done the best it
can be.  But it addresses real problems, so I've applied anyway.  I
will make some comments inline, which could be addressed in follow up
patches.

> 
> Thanks,
> 
> C.
> 
> Changes in v5:
> 
>  - Removed useless PNV_CHIP() cast
>  
> Changes in v4:
> 
>  - Introduce a PnvCore reset handler
>  - Add PnvChip pointer to PnvCore
> 
> Changes in v3:
> 
>  - Introduced a DeviceClass::reset for the CPU (Greg)
>  - add support for PowerNV
>  
> Changes in v2:
> 
>  - removed property
>  - simplified reset handlers
> 
> Cédric Le Goater (6):
>   spapr: move CPU reset after presenter creation
>   ppc/pnv: Introduce a PnvCore reset handler
>   ppc/pnv: Add a PnvChip pointer to PnvCore
>   ppc: Reset the interrupt presenter from the CPU reset handler
>   ppc/pnv: Fix naming of routines realizing the CPUs
>   spapr/xive: Set the OS CAM line at reset
> 
> Greg Kurz (1):
>   spapr_cpu_core: Implement DeviceClass::reset
> 
>  include/hw/ppc/pnv.h        |  1 +
>  include/hw/ppc/pnv_core.h   |  3 +++
>  include/hw/ppc/spapr_irq.h  |  2 ++
>  include/hw/ppc/spapr_xive.h |  1 -
>  include/hw/ppc/xics.h       |  1 +
>  include/hw/ppc/xive.h       |  1 +
>  hw/intc/spapr_xive.c        | 53 +++++++++++++++++--------------------
>  hw/intc/xics.c              |  8 ++----
>  hw/intc/xics_spapr.c        |  7 +++++
>  hw/intc/xive.c              | 12 +--------
>  hw/ppc/pnv.c                | 18 +++++++++++++
>  hw/ppc/pnv_core.c           | 31 ++++++++++++++++------
>  hw/ppc/spapr_cpu_core.c     | 44 +++++++++++++++++++++++-------
>  hw/ppc/spapr_irq.c          | 14 ++++++++++
>  14 files changed, 131 insertions(+), 65 deletions(-)
> 

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