[Skiboot] [PATCH v12 09/10] skiboot: Add core IMC related counter configuration
Michael Neuling
mikey at neuling.org
Wed Jun 14 14:56:16 AEST 2017
On Sun, 2017-05-21 at 20:40 +0530, Madhavan Srinivasan wrote:
> > From: Anju T Sudhakar <anju at linux.vnet.ibm.com>
>
> Add support to start, stop and initialize the core IMC counters.
> To initialize the core IMC counters, it takes a physical address per
> core as an input and writes that address to PDBAR[14:50] bits.
> It initializes the htm_mode and event_mask, where it selects the time
Please spell out an acronym the first time you use it...
htm ? hardware transactional memory? hardware trace macro?
> interval at which the counter values must be posted to the given memory
> location and enables the counters to start running by setting the
> appropriate bits.
>
> To disable the core IMC counters (only stop counting), writes into
> appropriate bits of htm_mode to disable the counters.
> To enable the core IMC counters (only resume counting), writes into
> appropriate bits of the htm_mode to enable the counters.
>
> > Signed-off-by: Hemant Kumar <hemant at linux.vnet.ibm.com>
> > Signed-off-by: Anju T Sudhakar <anju at linux.vnet.ibm.com>
> > Signed-off-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
> ---
> hw/imc.c | 138 ++++++++++++++++++++++++++++++++++++++++++++++++++---
> include/imc.h | 10 ++++
> include/opal-api.h | 1 +
> 3 files changed, 143 insertions(+), 6 deletions(-)
>
> diff --git a/hw/imc.c b/hw/imc.c
> index d56b3eb251b2..923a8fe48b5f 100644
> --- a/hw/imc.c
> +++ b/hw/imc.c
> @@ -83,6 +83,26 @@ size_t imc_catalog_size;
> const char **imc_prop_to_fix(struct dt_node *node);
> const char *prop_to_fix[] = {"events", NULL};
>
> +
> +/*
> + * A Quad contains 4 cores in Power 9, and there are 4 addresses for
> + * the CHTM attached to each core.
CHTM?
> + * So, for core index 0 to core index 3, we have a sequential range of
> + * SCOM port addresses in the arrays below, each for PDBAR and HTM mode.
PDBAR?
> + */
> +unsigned int pdbar_scom_index[] = {
> > + 0x1001220B,
> > + 0x1001230B,
> > + 0x1001260B,
> > + 0x1001270B
> +};
> +unsigned int htm_scom_index[] = {
> > + 0x10012200,
> > + 0x10012300,
> > + 0x10012600,
> > + 0x10012700
> +};
> +
> static struct imc_chip_cb *get_imc_cb(void)
> {
> > uint64_t cb_loc;
> @@ -341,19 +361,83 @@ err:
> * opal_imc_counters_init : This call initialize the IMC engine.
> *
> * For Nest IMC, this is no-op and returns OPAL_SUCCESS at this point.
> + * For Core IMC, this initializes core IMC Engine, by initializing
> + * these scoms "PDBAR", "HTM_MODE" and the "EVENT_MASK" in a given cpu.
> */
> -static int64_t opal_imc_counters_init(uint32_t type)
> +static int64_t opal_imc_counters_init(uint32_t type, uint64_t addr, uint64_t cpu_pir)
> {
> > - return OPAL_SUCCESS;
> > + struct cpu_thread *c = find_cpu_by_pir(cpu_pir);
> > + struct proc_chip *chip;
> + int core_id, phys_core_id, ret = OPAL_SUCCESS;
Again, ret is confusing here. Below you set ret, then goto hw_err that does
return OPAL_HARDWARE;
> +
> > + switch (type) {
> > + case OPAL_IMC_COUNTERS_NEST:
> > + break;
> > + case OPAL_IMC_COUNTERS_CORE:
> > + assert(c);
> > + chip = get_chip(c->chip_id);
> > + assert(chip);
> > + phys_core_id = cpu_get_core_index(c);
> + core_id = phys_core_id % 4;
core_id or thread_id? why %4?
> +
> > + /*
> > + * Core IMC hardware mandate initing of three scoms
> > + * to enbale or disable of the Core IMC engine.
> > + *
> > + * PDBAR: Scom contains the real address to store per-core
> > + * counter data in memory along with other bits.
> > + *
> > + * EventMask: Scom contain bits to denote event to multiplex
> > + * at different MSR[HV PR] values, along with bits for
> > + * sampling duration.
> > + *
> > + * HTM Scom: scom to enable counter data movement to memory.
> > + */
> > + ret = xscom_write(chip->id,
> > + XSCOM_ADDR_P9_EP(phys_core_id,
> > + pdbar_scom_index[core_id]),
> > + (u64)(CORE_IMC_PDBAR_MASK & addr));
> > + if (ret < 0) {
> > + prerror("IMC: error in xscom_write for pdbar\n");
> > + goto hw_err;
> > + }
> +
> > + ret = xscom_write(chip->id,
> > + XSCOM_ADDR_P9_EC(phys_core_id,
> > + CORE_IMC_EVENT_MASK_ADDR),
> > + (u64)CORE_IMC_EVENT_MASK);
> > + if (ret < 0) {
> > + prerror("IMC: error in xscom_write for event mask\n");
> > + goto hw_err;
> > + }
> +
> > + ret = xscom_write(chip->id,
> > + XSCOM_ADDR_P9_EP(phys_core_id,
> > + htm_scom_index[core_id]),
> > + (u64)CORE_IMC_HTM_MODE_DISABLE);
> > + if (ret < 0) {
> > + prerror("IMC: error in xscom_write for htm mode\n");
> > + goto hw_err;
> > + }
> > + break;
> > + default:
> > + prerror("IMC: Unknown Domain int _INIT\n");
> > + return OPAL_PARAMETER;
> > + }
> +
> > + return ret;
> +hw_err:
> > + return OPAL_HARDWARE;
> }
> -opal_call(OPAL_IMC_COUNTERS_INIT, opal_imc_counters_init, 1);
> +opal_call(OPAL_IMC_COUNTERS_INIT, opal_imc_counters_init, 3);
>
> -/* opal_imc_counters_control_start: This call starts the nest imc engine. */
> +/* opal_imc_counters_control_start: This call starts the nest/core imc engine. */
> static int64_t opal_imc_counters_start(uint32_t type)
> {
> > u64 op, status;
> > struct imc_chip_cb *cb;
> > - int ret = OPAL_SUCCESS;
> > + struct proc_chip *chip;
> + int core_id, phys_core_id, ret = OPAL_SUCCESS;
again, ret is confusing.
>
> > switch (type) {
> > case OPAL_IMC_COUNTERS_NEST:
> @@ -372,6 +456,28 @@ static int64_t opal_imc_counters_start(uint32_t type)
> > cb->imc_chip_command = op;
>
> > break;
> > + case OPAL_IMC_COUNTERS_CORE:
> > + /*
> > + * Enables the core imc engine by appropriately setting
> > + * bits 4-9 of the HTM_MODE scom port. No initialization
> > + * is done in this call. This just enables the the counters
> > + * to count with the previous initialization.
> > + */
> > + chip = get_chip(this_cpu()->chip_id);
> > + phys_core_id = cpu_get_core_index(this_cpu());
> > + core_id = phys_core_id % 4;
> +
> > + ret = xscom_write(chip->id,
> > + XSCOM_ADDR_P9_EP(phys_core_id,
> > + htm_scom_index[core_id]),
> > + (u64) CORE_IMC_HTM_MODE_ENABLE);
> +
> > + if (ret < 0) {
> > + prerror("IMC: error in xscom_write for htm_mode\n");
> > + return OPAL_HARDWARE;
> > + }
> +
> > + break;
> > default:
> > prerror("IMC: Unknown Domain in _START\n");
> > return OPAL_PARAMETER;
> @@ -386,7 +492,8 @@ static int64_t opal_imc_counters_stop(uint32_t type)
> {
> > u64 op, status;
> > struct imc_chip_cb *cb;
> > - int ret = OPAL_SUCCESS;
> > + struct proc_chip *chip;
> > + int core_id, phys_core_id, ret = OPAL_SUCCESS;
>
> > switch (type) {
> > case OPAL_IMC_COUNTERS_NEST:
> @@ -405,6 +512,25 @@ static int64_t opal_imc_counters_stop(uint32_t type)
> > cb->imc_chip_command = op;
>
> > break;
> > + case OPAL_IMC_COUNTERS_CORE:
> > + /*
> > + * Disables the core imc engine by clearing
> > + * bits 4-9 of the HTM_MODE scom port.
> > + */
> > + chip = get_chip(this_cpu()->chip_id);
> > + phys_core_id = cpu_get_core_index(this_cpu());
> + core_id = phys_core_id % 4;
Again, core_id? %4?
> +
> > + ret = xscom_write(chip->id,
> > + XSCOM_ADDR_P9_EP(phys_core_id,
> > + htm_scom_index[core_id]),
> > + (u64) CORE_IMC_HTM_MODE_DISABLE);
> > + if (ret < 0) {
> > + prerror("IMC: error in xscom_write for htm_mode\n");
> > + return OPAL_HARDWARE;
> > + }
> +
> > + break;
> > default:
> prerror("IMC: Unknown Domain in _STOP\n");
unknown domain?
> return OPAL_PARAMETER;
> diff --git a/include/imc.h b/include/imc.h
> index c7ad5fa933b7..5a3d53c22ca1 100644
> --- a/include/imc.h
> +++ b/include/imc.h
> @@ -116,6 +116,16 @@ struct imc_chip_cb
>
> > #define MAX_NEST_UNITS 48
>
> +/*
> + * Core IMC SCOMs
> + */
> > +#define CORE_IMC_EVENT_MASK_ADDR 0x20010AA8ull
> > +#define CORE_IMC_EVENT_MASK 0x0001020000000000ull
> > +#define CORE_IMC_PDBAR_MASK 0x0003ffffffffe000ull
> > +#define CORE_IMC_NCU_MODE 0x0800000000000000ull
> > +#define CORE_IMC_HTM_MODE_ENABLE 0xE800000000000000ull
> > +#define CORE_IMC_HTM_MODE_DISABLE 0xE000000000000000ull
> +
> void imc_init(void);
> void imc_catalog_preload(void);
> #endif /* __IMC_H */
> diff --git a/include/opal-api.h b/include/opal-api.h
> index 02927356d033..5dbae2d57fde 100644
> --- a/include/opal-api.h
> +++ b/include/opal-api.h
> @@ -1232,6 +1232,7 @@ enum {
> /* Operation argument to IMC Microcode */
> enum {
> > OPAL_IMC_COUNTERS_NEST = 1,
> + OPAL_IMC_COUNTERS_CORE = 2,
How do we advertise to linux that it can do CORE vs NEST calls?
Say we need to remove one, how does linux know which one it can call without
trying and getting OPAL_PARAMETER?
If it's based on the device tree, then the patch series the wrong way around
since the earlier patches start adding the device tree before the calls are
added. So if you apply just patches 1-7, Linux is going to blow up with a bunch
of OPAL_PARAMETER since it'll start seeing the device tree before the OPAL calls
have been added.
Not a big issues for this series, but it does make me wonder. If someone
updates the device tree in the PNOR to start adding a new section type for IMC,
how do we tell linux that even though the device tree supports it, OPAL itself
doesn't?
Say we have PHB counters in the future (just made up). We add them to the IMA
catalog and linux, but not skiboot. Linux is going to OPAL_PARAMETER and blow
up. Linux needs to check if the device tree and skiboot have the capability.
Not just the device tree.
Mikey
More information about the Skiboot
mailing list