[Skiboot] [PATCH] hw/imc: Do scoms on the secondary core in fused core mode for core-imc counters
Oliver O'Halloran
oohall at gmail.com
Wed Aug 7 11:51:35 AEST 2019
On Tue, Aug 6, 2019 at 4:02 PM Anju T Sudhakar <anju at linux.vnet.ibm.com> wrote:
>
> Core IMC hardware mandates initiating of three scoms to enable or
> disable of the Core IMC engine. This has to be done for each core,
> and is done in the opal_imc_counters_init().
>
> In fused core mode, the scom has to be done explicitly for the secondary
> core to enable or disable core IMC engine in that core.
>
> Do the scom for the secondary core, by calculating the core id from
> the incoming cpu id.
>
> This patch is based on the series,
> Initial fused-core support for POWER9:
> https://lists.ozlabs.org/pipermail/skiboot/2019-March/013718.html
>
> Signed-off-by: Anju T Sudhakar <anju at linux.vnet.ibm.com>
> ---
> hw/imc.c | 153 ++++++++++++++++++++++++++++++++-----------------------
> 1 file changed, 89 insertions(+), 64 deletions(-)
>
> diff --git a/hw/imc.c b/hw/imc.c
> index bb651481..25bc2db7 100644
> --- a/hw/imc.c
> +++ b/hw/imc.c
> @@ -654,6 +654,79 @@ static int stop_api_init(struct proc_chip *chip, int phys_core_id,
>
> return ret;
> }
> +static int64_t core_imc_counters_init(uint64_t addr, int port_id,
> + int phys_core_id, struct cpu_thread *c)
> +{
> + uint32_t scoms;
> + int ret;
> +
> + /*
> + * Core IMC hardware mandate initing of three scoms
> + * to enbale or disable of the Core IMC engine.
s/enbale/enable/
> + *
> + * 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.
> + */
> +
> + if (xscom_write(c->chip_id,
> + XSCOM_ADDR_P9_EP(phys_core_id,
> + pdbar_scom_index[port_id]),
> + (u64)(CORE_IMC_PDBAR_MASK & addr))) {
Why are these casts needed?
> + prerror("error in xscom_write for pdbar\n");
> + return OPAL_HARDWARE;
> + }
> +
> + if (has_deep_states) {
> + if (wakeup_engine_state == WAKEUP_ENGINE_PRESENT) {
> + struct proc_chip *chip = get_chip(c->chip_id);
> +
> + scoms = XSCOM_ADDR_P9_EP(phys_core_id,
> + pdbar_scom_index[port_id]);
> + ret = stop_api_init(chip, phys_core_id, scoms,
> + (u64)(CORE_IMC_PDBAR_MASK & addr),
> + P9_STOP_SCOM_REPLACE,
> + P9_STOP_SECTION_EQ_SCOM,
> + "pdbar");
> + if (ret)
> + return ret;
> + scoms = XSCOM_ADDR_P9_EC(phys_core_id,
> + CORE_IMC_EVENT_MASK_ADDR);
> + ret = stop_api_init(chip, phys_core_id, scoms,
> + (u64)CORE_IMC_EVENT_MASK,
> + P9_STOP_SCOM_REPLACE,
> + P9_STOP_SECTION_CORE_SCOM,
> + "event_mask");
> + if (ret)
> + return ret;
> + } else {
> + prerror("IMC: Wakeup engine not present!");
> + return OPAL_HARDWARE;
You can avoid stacking indentation levels with an early exist, i.e.
if(has_deep_states) {
if(wakeup_engine_state != WAKEUP_ENGINE_PRESENT)
return OPAL_HARDWARE;
/* inits go here */
Come to think of it, why would any deep states be enabled if the
wakeup engine is not functional?
> + }
> + }
> +
> + if (xscom_write(c->chip_id,
> + XSCOM_ADDR_P9_EC(phys_core_id,
> + CORE_IMC_EVENT_MASK_ADDR),
> + (u64)CORE_IMC_EVENT_MASK)) {
> + prerror("error in xscom_write for event mask\n");
> + return OPAL_HARDWARE;
> + }
> +
> + if (xscom_write(c->chip_id,
> + XSCOM_ADDR_P9_EP(phys_core_id,
> + htm_scom_index[port_id]),
> + (u64)CORE_IMC_HTM_MODE_DISABLE)) {
> + prerror("error in xscom_write for htm mode\n");
> + return OPAL_HARDWARE;
> + }
> + return OPAL_SUCCESS;
> +}
>
> /*
> * opal_imc_counters_init : This call initialize the IMC engine.
> @@ -679,6 +752,9 @@ static int64_t opal_imc_counters_init(uint32_t type, uint64_t addr, uint64_t cpu
> if (!c)
> return OPAL_PARAMETER;
>
> + if (proc_chip_quirks & QUIRK_MAMBO_CALLOUTS)
> + return OPAL_SUCCESS;
> +
> /*
> * Core IMC hardware mandates setting of htm_mode and
> * pdbar in specific scom ports. port_id are in
> @@ -687,76 +763,25 @@ static int64_t opal_imc_counters_init(uint32_t type, uint64_t addr, uint64_t cpu
> phys_core_id = pir_to_core_id(c->pir);
> port_id = phys_core_id % 4;
>
> - if (proc_chip_quirks & QUIRK_MAMBO_CALLOUTS)
> - return OPAL_SUCCESS;
> + ret = core_imc_counters_init(addr, port_id, phys_core_id, c);
> + if (ret < 0)
> + return ret;
>
> /*
> - * 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.
> + * If fused core is supported, do the scoms for the
> + * secondary core also.
> */
> + if (this_cpu()->is_fused_core) {
> + struct cpu_thread *c1 = find_cpu_by_pir(cpu_pir ^ 1);
>
> + phys_core_id = pir_to_core_id(c1->pir);
> + port_id = phys_core_id % 4;
This looks wrong. The low bits of the PIR are the thread index so
isn't c1 going to be a cpu_thread from the same core?
>
> - if (xscom_write(c->chip_id,
> - XSCOM_ADDR_P9_EP(phys_core_id,
> - pdbar_scom_index[port_id]),
> - (u64)(CORE_IMC_PDBAR_MASK & addr))) {
> - prerror("error in xscom_write for pdbar\n");
> - return OPAL_HARDWARE;
> + ret = core_imc_counters_init(addr, port_id, phys_core_id, c1);
> + if (ret < 0)
> + return ret;
> }
> -
> - if (has_deep_states) {
> - if (wakeup_engine_state == WAKEUP_ENGINE_PRESENT) {
> - struct proc_chip *chip = get_chip(c->chip_id);
> -
> - scoms = XSCOM_ADDR_P9_EP(phys_core_id,
> - pdbar_scom_index[port_id]);
> - ret = stop_api_init(chip, phys_core_id, scoms,
> - (u64)(CORE_IMC_PDBAR_MASK & addr),
> - P9_STOP_SCOM_REPLACE,
> - P9_STOP_SECTION_EQ_SCOM,
> - "pdbar");
> - if (ret)
> - return ret;
> - scoms = XSCOM_ADDR_P9_EC(phys_core_id,
> - CORE_IMC_EVENT_MASK_ADDR);
> - ret = stop_api_init(chip, phys_core_id, scoms,
> - (u64)CORE_IMC_EVENT_MASK,
> - P9_STOP_SCOM_REPLACE,
> - P9_STOP_SECTION_CORE_SCOM,
> - "event_mask");
> - if (ret)
> - return ret;
> - } else {
> - prerror("IMC: Wakeup engine not present!");
> - return OPAL_HARDWARE;
> - }
> - }
> -
> - if (xscom_write(c->chip_id,
> - XSCOM_ADDR_P9_EC(phys_core_id,
> - CORE_IMC_EVENT_MASK_ADDR),
> - (u64)CORE_IMC_EVENT_MASK)) {
> - prerror("error in xscom_write for event mask\n");
> - return OPAL_HARDWARE;
> - }
> -
> - if (xscom_write(c->chip_id,
> - XSCOM_ADDR_P9_EP(phys_core_id,
> - htm_scom_index[port_id]),
> - (u64)CORE_IMC_HTM_MODE_DISABLE)) {
> - prerror("error in xscom_write for htm mode\n");
> - return OPAL_HARDWARE;
> - }
> - return OPAL_SUCCESS;
> + return ret;
> case OPAL_IMC_COUNTERS_TRACE:
> if (!c)
> return OPAL_PARAMETER;
> --
> 2.20.1
>
> _______________________________________________
> Skiboot mailing list
> Skiboot at lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/skiboot
More information about the Skiboot
mailing list