Re: [Qemu-devel] [PATCH v3 08/10] ppc/pnv: add a XScomDevice to PnvCore
On Thu, Sep 22, 2016 at 10:33:21AM +0200, Cédric Le Goater wrote:
> On 09/21/2016 08:12 AM, David Gibson wrote:
> > 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
> >> ---
> >>
> >> 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.
>
> When working on the support of the AST2{4,5}00 SoC for qemu, these
> are the BMC chips for the OpenPOWER systems, we were asked to do all
> the mmio mappings for the devices at the board level.
After a bit of thought, I agree. Doing as you're doing here and
building any internal structure into a single MR within the device
which then gets mapped into the global address space by the machine is
a good approach.
> I think we can consider that the powernv chip is the board level for
> the xscom address space and to all the mapping there.
>
> This has some benefits on the view of the address space as it is
> located in one file and not spread in multiple areas of the code.
>
> >
> >> }
> >> 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 0x5
> >> +#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 = 0x26f024f023full;
> >> +break;
> >> +case 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);
> >> +}
> >
> > 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 = {
> >> +.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;
> >> }
> >>
Re: [Qemu-devel] [PATCH v3 08/10] ppc/pnv: add a XScomDevice to PnvCore
On 09/21/2016 08:12 AM, David Gibson wrote:
> 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
>> ---
>>
>> 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.
When working on the support of the AST2{4,5}00 SoC for qemu, these
are the BMC chips for the OpenPOWER systems, we were asked to do all
the mmio mappings for the devices at the board level.
I think we can consider that the powernv chip is the board level for
the xscom address space and to all the mapping there.
This has some benefits on the view of the address space as it is
located in one file and not spread in multiple areas of the code.
>
>> }
>> 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 0x5
>> +#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 = 0x26f024f023full;
>> +break;
>> +case 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);
>> +}
>
> 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 = {
>> +.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_mo
Re: [Qemu-devel] [PATCH v3 08/10] ppc/pnv: add a XScomDevice to PnvCore
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
> ---
>
> 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 0x5
> +#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 = 0x26f024f023full;
> +break;
> +case 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);
> +}
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/incl
[Qemu-devel] [PATCH v3 08/10] ppc/pnv: add a XScomDevice to PnvCore
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 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);
}
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 0x5
+#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 = 0x26f024f023full;
+break;
+case 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;
@@ -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)\
(0x3fc00ull + ((uint64_t)(chip)) * PNV_XSCOM_SIZE)
+/*
+ * Layout of Xscom PCB addresses for EX core 1
+ *
+ * GPIO0x1100
+ * SCOM0x1101
+ * OHA 0x1102
+ * CLOCK CTL 0x1103
+ * FIR 0x1104
+ * THERM
