Re: [Qemu-ppc] [PATCH v4 07/20] ppc/pnv: add XSCOM handlers to PnvCore

[David Gibson]

On Thu, Oct 13, 2016 at 08:50:41AM +0200, Cédric Le Goater wrote:
> On 10/13/2016 02:51 AM, David Gibson wrote:
> > On Mon, Oct 03, 2016 at 09:24:43AM +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 <address@hidden>
> >> ---
> >>
> >>  Changes since v3:
> >>
> >>  - moved to new XSCOM model
> >>  - kept the write op on the XSCOM memory region for later use
> >>
> >>  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          | 50 
> >> ++++++++++++++++++++++++++++++++++++++++++++++
> >>  include/hw/ppc/pnv_core.h  |  2 ++
> >>  include/hw/ppc/pnv_xscom.h | 19 ++++++++++++++++++
> >>  4 files changed, 75 insertions(+)
> >>
> >> diff --git a/hw/ppc/pnv.c b/hw/ppc/pnv.c
> >> index 5e19b6880387..ffe245fe59d2 100644
> >> --- a/hw/ppc/pnv.c
> >> +++ b/hw/ppc/pnv.c
> >> @@ -620,6 +620,10 @@ static void pnv_chip_realize(DeviceState *dev, Error 
> >> **errp)
> >>                                   &error_fatal);
> >>          object_unref(OBJECT(pnv_core));
> >>          i++;
> >> +
> >> +        memory_region_add_subregion(&chip->xscom,
> >> +                         PNV_XSCOM_EX_CORE_BASE(core_hwid) << 3,
> >> +                         &PNV_CORE(pnv_core)->xscom_regs);
> > 
> > Might be worth adding some convenience functions for doing the various
> > bits of xscom MR juggling, otherwise this looks fine.
> 
> To do the 8 byte shifting ? or something like this :
>  
>       void pnv_xscom_add_subregion(PnvChip *chip, uint32_t pcba,
>                                    MemoryRegion *subregion);

Yes, something like that, which incorporates the << 3.  And maybe
another one to construct the MR for use on the scom device side.

> or even :
> 
>       void pnv_xscom_add_subregion(PnvChip *chip, PnvXScomInterface *obj)
> 
> but that would require some more handlers under  PnvXScomInterface.
> 
> Thanks,
> 
> C.
> 
> 
> >>      }
> >>      g_free(typename);
> >>  }
> >> diff --git a/hw/ppc/pnv_core.c b/hw/ppc/pnv_core.c
> >> index d37788f142f4..a1c8a14f06b6 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"
> >> @@ -64,6 +65,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 PNV_XSCOM_EX_DTS_RESULT0     0x50000
> >> +#define PNV_XSCOM_EX_DTS_RESULT1     0x50001
> >> +
> >> +static uint64_t pnv_core_xscom_read(void *opaque, hwaddr addr,
> >> +                                    unsigned int width)
> >> +{
> >> +    uint32_t offset = addr >> 3;
> >> +    uint64_t val = 0;
> >> +
> >> +    /* The result should be 38 C */
> >> +    switch (offset) {
> >> +    case PNV_XSCOM_EX_DTS_RESULT0:
> >> +        val = 0x26f024f023f0000ull;
> >> +        break;
> >> +    case PNV_XSCOM_EX_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);
> >> +}
> >> +
> >> +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;
> >> @@ -119,6 +165,10 @@ 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_EX_CORE_SIZE << 3);
> >>      return;
> >>  
> >>  err:
> >> 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 f50eb0bc4099..79975a6cbe46 100644
> >> --- a/include/hw/ppc/pnv_xscom.h
> >> +++ b/include/hw/ppc/pnv_xscom.h
> >> @@ -41,6 +41,25 @@ typedef struct PnvXScomInterfaceClass {
> >>      int (*populate)(PnvXScomInterface *dev, void *fdt, int offset);
> >>  } PnvXScomInterfaceClass;
> >>  
> >> +/*
> >> + * Layout of the XSCOM PCB addresses of 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 void pnv_xscom_realize(PnvChip *chip, Error **errp);
> >>  extern int pnv_xscom_populate(PnvChip *chip, 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

signature.asc
Description: PGP signature