Re: [PATCH for-5.0 v5 18/23] ppc/xive: Move the TIMA operations to the controller model

[Greg Kurz]

On Fri, 15 Nov 2019 17:24:31 +0100
Cédric Le Goater <address@hidden> wrote:

> On the P9 Processor, the thread interrupt context registers of a CPU
> can be accessed "directly" when by load/store from the CPU or
> "indirectly" by the IC through an indirect TIMA page. This requires to
> configure first the PC_TCTXT_INDIRx registers.
> 
> Today, we rely on the get_tctx() handler to deduce from the CPU PIR
> the chip from which the TIMA access is being done. By handling the
> TIMA memory ops under the interrupt controller model of each machine,
> we can uniformize the TIMA direct and indirect ops under PowerNV. We
> can also check that the CPUs have been enabled in the XIVE controller.
> 
> This prepares ground for the future versions of XIVE.
> 
> Signed-off-by: Cédric Le Goater <address@hidden>
> ---

Reviewed-by: Greg Kurz <address@hidden>

>  include/hw/ppc/xive.h |  1 -
>  hw/intc/pnv_xive.c    | 35 ++++++++++++++++++++++++++++++++++-
>  hw/intc/spapr_xive.c  | 33 +++++++++++++++++++++++++++++++--
>  hw/intc/xive.c        | 29 -----------------------------
>  4 files changed, 65 insertions(+), 33 deletions(-)
> 
> diff --git a/include/hw/ppc/xive.h b/include/hw/ppc/xive.h
> index 97bbcddb381d..dcf897451589 100644
> --- a/include/hw/ppc/xive.h
> +++ b/include/hw/ppc/xive.h
> @@ -462,7 +462,6 @@ typedef struct XiveENDSource {
>  #define XIVE_TM_OS_PAGE         0x2
>  #define XIVE_TM_USER_PAGE       0x3
>  
> -extern const MemoryRegionOps xive_tm_ops;
>  void xive_tctx_tm_write(XivePresenter *xptr, XiveTCTX *tctx, hwaddr offset,
>                          uint64_t value, unsigned size);
>  uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX *tctx, hwaddr 
> offset,
> diff --git a/hw/intc/pnv_xive.c b/hw/intc/pnv_xive.c
> index d75053d0baad..4501c671d8df 100644
> --- a/hw/intc/pnv_xive.c
> +++ b/hw/intc/pnv_xive.c
> @@ -1473,6 +1473,39 @@ static const MemoryRegionOps xive_tm_indirect_ops = {
>      },
>  };
>  
> +static void pnv_xive_tm_write(void *opaque, hwaddr offset,
> +                              uint64_t value, unsigned size)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> +    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
> +    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> +
> +    xive_tctx_tm_write(XIVE_PRESENTER(xive), tctx, offset, value, size);
> +}
> +
> +static uint64_t pnv_xive_tm_read(void *opaque, hwaddr offset, unsigned size)
> +{
> +    PowerPCCPU *cpu = POWERPC_CPU(current_cpu);
> +    PnvXive *xive = pnv_xive_tm_get_xive(cpu);
> +    XiveTCTX *tctx = XIVE_TCTX(pnv_cpu_state(cpu)->intc);
> +
> +    return xive_tctx_tm_read(XIVE_PRESENTER(xive), tctx, offset, size);
> +}
> +
> +const MemoryRegionOps pnv_xive_tm_ops = {
> +    .read = pnv_xive_tm_read,
> +    .write = pnv_xive_tm_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +};
> +
>  /*
>   * Interrupt controller XSCOM region.
>   */
> @@ -1845,7 +1878,7 @@ static void pnv_xive_realize(DeviceState *dev, Error 
> **errp)
>                            "xive-pc", PNV9_XIVE_PC_SIZE);
>  
>      /* Thread Interrupt Management Area (Direct) */
> -    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops,
> +    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &pnv_xive_tm_ops,
>                            xive, "xive-tima", PNV9_XIVE_TM_SIZE);
>  
>      qemu_register_reset(pnv_xive_reset, dev);
> diff --git a/hw/intc/spapr_xive.c b/hw/intc/spapr_xive.c
> index bb3b2dfdb77f..6292da58f62c 100644
> --- a/hw/intc/spapr_xive.c
> +++ b/hw/intc/spapr_xive.c
> @@ -205,6 +205,35 @@ void spapr_xive_mmio_set_enabled(SpaprXive *xive, bool 
> enable)
>      memory_region_set_enabled(&xive->end_source.esb_mmio, false);
>  }
>  
> +static void spapr_xive_tm_write(void *opaque, hwaddr offset,
> +                          uint64_t value, unsigned size)
> +{
> +    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
> +
> +    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
> +}
> +
> +static uint64_t spapr_xive_tm_read(void *opaque, hwaddr offset, unsigned 
> size)
> +{
> +    XiveTCTX *tctx = spapr_cpu_state(POWERPC_CPU(current_cpu))->tctx;
> +
> +    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
> +}
> +
> +const MemoryRegionOps spapr_xive_tm_ops = {
> +    .read = spapr_xive_tm_read,
> +    .write = spapr_xive_tm_write,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +    .valid = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +    .impl = {
> +        .min_access_size = 1,
> +        .max_access_size = 8,
> +    },
> +};
> +
>  static void spapr_xive_end_reset(XiveEND *end)
>  {
>      memset(end, 0, sizeof(*end));
> @@ -314,8 +343,8 @@ static void spapr_xive_realize(DeviceState *dev, Error 
> **errp)
>      qemu_register_reset(spapr_xive_reset, dev);
>  
>      /* TIMA initialization */
> -    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &xive_tm_ops, xive,
> -                          "xive.tima", 4ull << TM_SHIFT);
> +    memory_region_init_io(&xive->tm_mmio, OBJECT(xive), &spapr_xive_tm_ops,
> +                          xive, "xive.tima", 4ull << TM_SHIFT);
>      sysbus_init_mmio(SYS_BUS_DEVICE(xive), &xive->tm_mmio);
>  
>      /*
> diff --git a/hw/intc/xive.c b/hw/intc/xive.c
> index a9298783e7d2..ab779c4c2a0f 100644
> --- a/hw/intc/xive.c
> +++ b/hw/intc/xive.c
> @@ -523,35 +523,6 @@ uint64_t xive_tctx_tm_read(XivePresenter *xptr, XiveTCTX 
> *tctx, hwaddr offset,
>      return xive_tm_raw_read(tctx, offset, size);
>  }
>  
> -static void xive_tm_write(void *opaque, hwaddr offset,
> -                          uint64_t value, unsigned size)
> -{
> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
> -
> -    xive_tctx_tm_write(XIVE_PRESENTER(opaque), tctx, offset, value, size);
> -}
> -
> -static uint64_t xive_tm_read(void *opaque, hwaddr offset, unsigned size)
> -{
> -    XiveTCTX *tctx = xive_router_get_tctx(XIVE_ROUTER(opaque), current_cpu);
> -
> -    return xive_tctx_tm_read(XIVE_PRESENTER(opaque), tctx, offset, size);
> -}
> -
> -const MemoryRegionOps xive_tm_ops = {
> -    .read = xive_tm_read,
> -    .write = xive_tm_write,
> -    .endianness = DEVICE_BIG_ENDIAN,
> -    .valid = {
> -        .min_access_size = 1,
> -        .max_access_size = 8,
> -    },
> -    .impl = {
> -        .min_access_size = 1,
> -        .max_access_size = 8,
> -    },
> -};
> -
>  static char *xive_tctx_ring_print(uint8_t *ring)
>  {
>      uint32_t w2 = xive_tctx_word2(ring);