Re: [Qemu-devel] [PATCH v3 08/10] ppc/pnv: add a XScomDevice to PnvCore

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 6370 bytes --]

On Thu, Sep 15, 2016 at 02:45:58PM +0200, Cédric Le Goater wrote:
> Now that we are using real HW ids for the cores in PowerNV chips, we
> can route the XSCOM accesses to them. We just need to attach a
> specific XSCOM memory region to each core in the appropriate window
> for the core number.
> 
> To start with, let's install the DTS (Digital Thermal Sensor) handlers
> which should return 38°C for each core.
> 
> Signed-off-by: Cédric Le Goater <clg@kaod.org>
> ---
> 
>  Changes since v2:
> 
>  - added a XSCOM memory region to handle access to the EX core
>    registers   
>  - extended the PnvCore object with a XSCOM_INTERFACE so that we can
>    use pnv_xscom_pcba() and pnv_xscom_addr() to handle XSCOM address
>    translation.
> 
>  hw/ppc/pnv.c               |  4 ++++
>  hw/ppc/pnv_core.c          | 55 ++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/ppc/pnv_core.h  |  2 ++
>  include/hw/ppc/pnv_xscom.h | 19 ++++++++++++++++
>  4 files changed, 80 insertions(+)
> 
> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> index 7dcdf18a9e6b..6a3d1fbf8403 100644
> --- a/hw/ppc/pnv.c
> +++ b/hw/ppc/pnv.c
> @@ -619,6 +619,10 @@ static void pnv_chip_realize(DeviceState *dev, Error **errp)
>                                   &error_fatal);
>          object_unref(OBJECT(pnv_core));
>          i++;
> +
> +        memory_region_add_subregion(&chip->xscom.xscom_mr,
> +                         pcc->xscom_addr(PNV_XSCOM_EX_CORE_BASE(core_hwid)),
> +                         &PNV_CORE(pnv_core)->xscom_regs);

I think the core realize function should be doing this itself.

>      }
>      g_free(typename);
>  
> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> index 6fed5a208536..81b83d0f41b3 100644
> --- a/hw/ppc/pnv_core.c
> +++ b/hw/ppc/pnv_core.c
> @@ -19,6 +19,7 @@
>  #include "qemu/osdep.h"
>  #include "sysemu/sysemu.h"
>  #include "qapi/error.h"
> +#include "qemu/log.h"
>  #include "target-ppc/cpu.h"
>  #include "hw/ppc/ppc.h"
>  #include "hw/ppc/pnv.h"
> @@ -57,6 +58,51 @@ static void powernv_cpu_init(PowerPCCPU *cpu, Error **errp)
>      powernv_cpu_reset(cpu);
>  }
>  
> +/*
> + * These values are read by the powernv hw monitors under Linux
> + */
> +#define DTS_RESULT0     0x50000
> +#define DTS_RESULT1     0x50001
> +
> +static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
> +                                    unsigned int width)
> +{
> +    uint32_t offset = pnv_xscom_pcba(opaque, addr);
> +    uint64_t val = 0;
> +
> +    /* The result should be 38 C */
> +    switch (offset) {
> +    case DTS_RESULT0:
> +        val = 0x26f024f023f0000ull;
> +        break;
> +    case DTS_RESULT1:
> +        val = 0x24f000000000000ull;
> +        break;
> +    default:
> +        qemu_log_mask(LOG_UNIMP, "Warning: reading reg=0x%" HWADDR_PRIx,
> +                  addr);
> +    }
> +
> +    return val;
> +}
> +
> +static void pnv_core_xscom_write(void *opaque, hwaddr addr, uint64_t val,
> +                                 unsigned int width)
> +{
> +    qemu_log_mask(LOG_UNIMP, "Warning: writing to reg=0x%" HWADDR_PRIx,
> +                  addr);
> +}

You should double check, but I think you can implement an RO region in
an address space by just leaving the write function as NULL.

> +
> +static const MemoryRegionOps pnv_core_xscom_ops = {
> +    .read = pnv_core_xscom_read,
> +    .write = pnv_core_xscom_write,
> +    .valid.min_access_size = 8,
> +    .valid.max_access_size = 8,
> +    .impl.min_access_size = 8,
> +    .impl.max_access_size = 8,
> +    .endianness = DEVICE_BIG_ENDIAN,
> +};
> +
>  static void pnv_core_realize_child(Object *child, Error **errp)
>  {
>      Error *local_err = NULL;
> @@ -117,6 +163,11 @@ static void pnv_core_realize(DeviceState *dev, Error **errp)
>              goto err;
>          }
>      }
> +
> +    snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id);
> +    memory_region_init_io(&pc->xscom_regs, OBJECT(dev), &pnv_core_xscom_ops,
> +                          pc, name, pnv_xscom_addr(PNV_XSCOM_INTERFACE(dev),
> +                                                   PNV_XSCOM_EX_CORE_SIZE));
>      return;
>  
>  err:
> @@ -169,6 +220,10 @@ static void pnv_core_register_types(void)
>              .instance_size = sizeof(PnvCore),
>              .class_init = pnv_core_class_init,
>              .class_data = (void *) pnv_core_models[i],
> +            .interfaces    = (InterfaceInfo[]) {
> +                { TYPE_PNV_XSCOM_INTERFACE },
> +                { }
> +            }
>          };
>          ti.name = pnv_core_typename(pnv_core_models[i]);
>          type_register(&ti);
> diff --git a/include/hw/ppc/pnv_core.h b/include/hw/ppc/pnv_core.h
> index a151e281c017..2955a41c901f 100644
> --- a/include/hw/ppc/pnv_core.h
> +++ b/include/hw/ppc/pnv_core.h
> @@ -36,6 +36,8 @@ typedef struct PnvCore {
>      /*< public >*/
>      void *threads;
>      uint32_t pir;
> +
> +    MemoryRegion xscom_regs;
>  } PnvCore;
>  
>  typedef struct PnvCoreClass {
> diff --git a/include/hw/ppc/pnv_xscom.h b/include/hw/ppc/pnv_xscom.h
> index 0a03d533db59..31e5e8847b90 100644
> --- a/include/hw/ppc/pnv_xscom.h
> +++ b/include/hw/ppc/pnv_xscom.h
> @@ -63,6 +63,25 @@ typedef struct PnvXScom {
>  #define PNV_XSCOM_BASE(chip)                                    \
>      (0x3fc0000000000ull + ((uint64_t)(chip)) * PNV_XSCOM_SIZE)
>  
> +/*
> + * Layout of Xscom PCB addresses for EX core 1
> + *
> + *   GPIO        0x1100xxxx
> + *   SCOM        0x1101xxxx
> + *   OHA         0x1102xxxx
> + *   CLOCK CTL   0x1103xxxx
> + *   FIR         0x1104xxxx
> + *   THERM       0x1105xxxx
> + *   <reserved>  0x1106xxxx
> + *               ..
> + *               0x110Exxxx
> + *   PCB SLAVE   0x110Fxxxx
> + */
> +
> +#define PNV_XSCOM_EX_BASE         0x10000000
> +#define PNV_XSCOM_EX_CORE_BASE(i) (PNV_XSCOM_EX_BASE | (((uint64_t)i) << 24))
> +#define PNV_XSCOM_EX_CORE_SIZE    0x100000
> +
>  extern int pnv_xscom_populate_fdt(PnvXScom *xscom, void *fdt, int offset);
>  
>  /*

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

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]