From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:52906) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmzRm-0004Kd-3Y for qemu-devel@nongnu.org; Thu, 22 Sep 2016 04:33:40 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1bmzRe-000418-W9 for qemu-devel@nongnu.org; Thu, 22 Sep 2016 04:33:32 -0400 Received: from 5.mo69.mail-out.ovh.net ([46.105.43.105]:57244) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1bmzRe-00040q-Em for qemu-devel@nongnu.org; Thu, 22 Sep 2016 04:33:26 -0400 Received: from player779.ha.ovh.net (b9.ovh.net [213.186.33.59]) by mo69.mail-out.ovh.net (Postfix) with ESMTP id E5D641004DA7 for ; Thu, 22 Sep 2016 10:33:24 +0200 (CEST) References: <1473943560-14846-1-git-send-email-clg@kaod.org> <1473943560-14846-9-git-send-email-clg@kaod.org> <20160921061207.GD20488@umbus> From: =?UTF-8?Q?C=c3=a9dric_Le_Goater?= Message-ID: Date: Thu, 22 Sep 2016 10:33:21 +0200 MIME-Version: 1.0 In-Reply-To: <20160921061207.GD20488@umbus> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v3 08/10] ppc/pnv: add a XScomDevice to PnvCore List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: David Gibson Cc: qemu-ppc@nongnu.org, Benjamin Herrenschmidt , qemu-devel@nongnu.org On 09/21/2016 08:12 AM, David Gibson wrote: > On Thu, Sep 15, 2016 at 02:45:58PM +0200, C=E9dric 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=B0C for each core. >> >> Signed-off-by: C=E9dric Le Goater >> --- >> >> Changes since v2: >> >> - added a XSCOM memory region to handle access to the EX core >> registers =20 >> - 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, Er= ror **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); >=20 > I think the core realize function should be doing this itself. When working on the support of the AST2{4,5}00 SoC for qemu, these=20 are the BMC chips for the OpenPOWER systems, we were asked to do all=20 the mmio mappings for the devices at the board level.=20 I think we can consider that the powernv chip is the board level for=20 the xscom address space and to all the mapping there. This has some benefits on the view of the address space as it is=20 located in one file and not spread in multiple areas of the code. >=20 >> } >> g_free(typename); >> =20 >> 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); >> } >> =20 >> +/* >> + * 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 =3D pnv_xscom_pcba(opaque, addr); >> + uint64_t val =3D 0; >> + >> + /* The result should be 38 C */ >> + switch (offset) { >> + case DTS_RESULT0: >> + val =3D 0x26f024f023f0000ull; >> + break; >> + case DTS_RESULT1: >> + val =3D 0x24f000000000000ull; >> + break; >> + default: >> + qemu_log_mask(LOG_UNIMP, "Warning: reading reg=3D0x%" 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=3D0x%" HWADDR_P= RIx, >> + addr); >> +} >=20 > 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. OK. Thanks, C. >> + >> +static const MemoryRegionOps pnv_core_xscom_ops =3D { >> + .read =3D pnv_core_xscom_read, >> + .write =3D pnv_core_xscom_write, >> + .valid.min_access_size =3D 8, >> + .valid.max_access_size =3D 8, >> + .impl.min_access_size =3D 8, >> + .impl.max_access_size =3D 8, >> + .endianness =3D DEVICE_BIG_ENDIAN, >> +}; >> + >> static void pnv_core_realize_child(Object *child, Error **errp) >> { >> Error *local_err =3D NULL; >> @@ -117,6 +163,11 @@ static void pnv_core_realize(DeviceState *dev, Er= ror **errp) >> goto err; >> } >> } >> + >> + snprintf(name, sizeof(name), "xscom-core.%d", cc->core_id); >> + memory_region_init_io(&pc->xscom_regs, OBJECT(dev), &pnv_core_xsc= om_ops, >> + pc, name, pnv_xscom_addr(PNV_XSCOM_INTERFAC= E(dev), >> + PNV_XSCOM_EX_CORE_= SIZE)); >> return; >> =20 >> err: >> @@ -169,6 +220,10 @@ static void pnv_core_register_types(void) >> .instance_size =3D sizeof(PnvCore), >> .class_init =3D pnv_core_class_init, >> .class_data =3D (void *) pnv_core_models[i], >> + .interfaces =3D (InterfaceInfo[]) { >> + { TYPE_PNV_XSCOM_INTERFACE }, >> + { } >> + } >> }; >> ti.name =3D 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; >> =20 >> 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) >> =20 >> +/* >> + * Layout of Xscom PCB addresses for EX core 1 >> + * >> + * GPIO 0x1100xxxx >> + * SCOM 0x1101xxxx >> + * OHA 0x1102xxxx >> + * CLOCK CTL 0x1103xxxx >> + * FIR 0x1104xxxx >> + * THERM 0x1105xxxx >> + * 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 off= set); >> =20 >> /* >=20