Re: [Qemu-devel] [PATCH v4 07/20] ppc/pnv: add XSCOM handlers to PnvCore
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
> >> ---
> >>
> >> 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 0x5
> >> +#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 = 0x26f024f023full;
> >> +break;
> >> +case PNV_XSCOM_EX_DTS_RESULT1:
> >> +val = 0x24full;
> >> +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),
> >> &pn
Re: [Qemu-devel] [PATCH v4 07/20] ppc/pnv: add XSCOM handlers to PnvCore
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
>> ---
>>
>> 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);
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 0x5
>> +#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 = 0x26f024f023full;
>> +break;
>> +case PNV_XSCOM_EX_DTS_RESULT1:
>> +val = 0x24full;
>> +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;
>>
>> ty
Re: [Qemu-devel] [PATCH v4 07/20] ppc/pnv: add XSCOM handlers to PnvCore
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
> ---
>
> 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.
> }
> 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 0x5
> +#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 = 0x26f024f023full;
> +break;
> +case PNV_XSCOM_EX_DTS_RESULT1:
> +val = 0x24full;
> +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
> + *
> + * GPIO0x1100
> + * SCOM0x1101
> + * OHA 0x1102
> + *
