[Skiboot] [PATCH v2 4/5] skiboot: Enable opal calls to init/start/stop IMC Trace mode
Stewart Smith
stewart at linux.vnet.ibm.com
Mon Feb 25 16:37:24 AEDT 2019
Anju T Sudhakar <anju at linux.vnet.ibm.com> writes:
> Patch to enhance the imc opal call to support and handle trace_imc mode.
>
> To initialize the trace-mode, TRACE_IMC_SCOM value is written to
> TRACE_IMC_ADDR of the respective core.
>
> TRACE_IMC_SCOM is a 64bit value, and each bit represent the following:
> 0:1 : SAMPSEL
> 2:33 : CPMC_LOAD
> 34:40 : CPMC1SEL
> 41:47 : CPMC2SEL
> 48:50 : BUFFERSIZE
> 51:63 : RESERVED
> For the nonce the value for TRACE_IMC_SCOM is hard coded.
nonce?
Do you mean we initialize the trace to a default value?
> During initialization htm_mode is disabled, and enabled only at start.
>
> The opal calls to start/stop the counters, will write CORE_IMC_HTM_MODE_ENABLE/
> CORE_IMC_HTM_MODE_DISABLE respectively to the htm_scom_index of the desired
> cores.
>
> Additional switch cases are added to the current opal calls to start/stop
> the counters for trace-mode.
>
> Signed-off-by: Anju T Sudhakar <anju at linux.vnet.ibm.com>
> Reviewed-by: Madhavan Srinivasan <maddy at linux.vnet.ibm.com>
> Cc: Akshay Adiga <akshay.adiga at linux.vnet.ibm.com>
> ---
> hw/imc.c | 103 +++++++++++++++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 103 insertions(+)
>
> diff --git a/hw/imc.c b/hw/imc.c
> index 3392eaf1..40c64d58 100644
> --- a/hw/imc.c
> +++ b/hw/imc.c
> @@ -22,6 +22,15 @@
> #include <device.h>
> #include <p9_stop_api.H>
>
> +/*
> + * IMC trace scom values
> + */
for these defines, please have them CAPITALIZED so we know they're
#define and not variables, and also please prefix with something IMC related.
> +#define samplesel 1 /* select cpmc2 */
> +#define cpmcload 0xfa /* Value to be loaded into cpmc2 at sampling start */
> +#define cpmc1sel 2 /* Event: CPM_CCYC */
> +#define cpmc2sel 2 /* Event: CPM_32MHZ_CYC */
What do these two things mean? Are there any other options?
> +#define buffersize 0 /* b’000’- 4K entries * 64 per entry =
> 256K buffersize */
I feel this should be documented somewhere in the device tree details
for the trace nodes.
Indeed, I see that documentation missing from doc/device-tree/imc.rst -
can you please add it?
> +
> /*
> * Nest IMC PMU names along with their bit values as represented in the
> * imc_chip_avl_vector(in struct imc_chip_cb, look at include/imc.h).
> @@ -264,6 +273,8 @@ static int get_imc_device_type(struct dt_node *node)
> return IMC_COUNTER_CORE;
> case IMC_COUNTER_THREAD:
> return IMC_COUNTER_THREAD;
> + case IMC_COUNTER_TRACE:
> + return IMC_COUNTER_TRACE;
> default:
> break;
> }
> @@ -283,11 +294,23 @@ static bool is_nest_node(struct dt_node *node)
> static bool is_imc_device_type_supported(struct dt_node *node)
> {
> u32 val = get_imc_device_type(node);
> + struct proc_chip *chip = get_chip(this_cpu()->chip_id);
> + uint64_t pvr;
>
> if ((val == IMC_COUNTER_CHIP) || (val == IMC_COUNTER_CORE) ||
> (val == IMC_COUNTER_THREAD))
> return true;
>
> + if (val == IMC_COUNTER_TRACE) {
> + pvr = mfspr(SPR_PVR);
> + /*
> + * Trace mode is supported in Nimbus DD2.2
> + * and later versions.
> + */
> + if ((chip->type == PROC_CHIP_P9_NIMBUS) &&
> + (PVR_VERS_MAJ(pvr) == 2) && (PVR_VERS_MIN(pvr) >= 2))
> + return true;
> + }
> return false;
What about Cumulus? Or do we just not know currently?
> }
>
> @@ -644,6 +667,8 @@ static int64_t opal_imc_counters_init(uint32_t type, uint64_t addr, uint64_t cpu
> int port_id, phys_core_id;
> int ret;
> uint32_t scoms;
> + uint64_t trace_scom_val = TRACE_IMC_SCOM(samplesel, cpmcload,
> + cpmc1sel, cpmc2sel, buffersize);
>
> switch (type) {
> case OPAL_IMC_COUNTERS_NEST:
> @@ -738,6 +763,53 @@ static int64_t opal_imc_counters_init(uint32_t type, uint64_t addr, uint64_t cpu
> return OPAL_HARDWARE;
> }
> return OPAL_SUCCESS;
> + case OPAL_IMC_COUNTERS_TRACE:
> + if (!c)
> + return OPAL_PARAMETER;
> +
> + phys_core_id = cpu_get_core_index(c);
> + port_id = phys_core_id % 4;
> +
> + if (proc_chip_quirks & QUIRK_MAMBO_CALLOUTS)
> + return OPAL_SUCCESS;
> +
> + if (has_deep_states) {
> + if (wakeup_engine_state == WAKEUP_ENGINE_PRESENT) {
> + struct proc_chip *chip = get_chip(c->chip_id);
> +
> + prlog(PR_INFO, "Configuring stopapi for
> IMC trace-mode\n");
should probably be PR_DEBUG.
> + scoms = XSCOM_ADDR_P9_EC(phys_core_id, TRACE_IMC_ADDR);
> + ret = p9_stop_save_scom((void *)chip->homer_base, scoms,
> + trace_scom_val,
> + P9_STOP_SCOM_REPLACE,
> + P9_STOP_SECTION_CORE_SCOM);
> + if (ret) {
> + prerror("IMC trace_mode stopapi ret = %d, scoms = %x (core id = %x)\n", ret, scoms, phys_core_id);
> + if (ret != STOP_SAVE_SCOM_ENTRY_UPDATE_FAILED)
> + wakeup_engine_state = WAKEUP_ENGINE_FAILED;
> + else
> + prerror("SCOM entries are full\n");
> + return OPAL_HARDWARE;
> + }
getting longer than 80 columns here, maybe split up the trace init into its own function?
> + } else {
> + prerror("IMC: TRACE: Wakeup engine in error state!");
> + return OPAL_HARDWARE;
This error isn't strictly true, we have WAKEUP_ENGINE_(NOT_PRESENT|PRESENT|FAILED).
> + }
> + }
> + 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("IMC: error in xscom_write for htm mode\n");
> + return OPAL_HARDWARE;
> + }
> + if (xscom_write(c->chip_id,
> + XSCOM_ADDR_P9_EC(phys_core_id,
> + TRACE_IMC_ADDR), trace_scom_val)) {
> + prerror("IMC: error in xscom_write for trace mode\n");
> + return OPAL_HARDWARE;
> + }
> + return OPAL_SUCCESS;
> +
> }
>
> return OPAL_SUCCESS;
> @@ -797,6 +869,21 @@ static int64_t opal_imc_counters_start(uint32_t type, uint64_t cpu_pir)
> return OPAL_HARDWARE;
> }
>
> + return OPAL_SUCCESS;
> + case OPAL_IMC_COUNTERS_TRACE:
> + phys_core_id = cpu_get_core_index(c);
> + port_id = phys_core_id % 4;
> +
> + if (proc_chip_quirks & QUIRK_MAMBO_CALLOUTS)
> + return OPAL_SUCCESS;
> +
> + if (xscom_write(c->chip_id,
> + XSCOM_ADDR_P9_EP(phys_core_id,
> + htm_scom_index[port_id]),
> + (u64)CORE_IMC_HTM_MODE_ENABLE)) {
> + prerror("IMC OPAL_start: error in xscom_write for htm_mode\n");
> + return OPAL_HARDWARE;
> + }
Should we also return OPAL_PARAMETER if start is called on a CPU
revision that doesn't support it?
> return OPAL_SUCCESS;
> }
>
> @@ -857,6 +944,22 @@ static int64_t opal_imc_counters_stop(uint32_t type, uint64_t cpu_pir)
> }
>
> return OPAL_SUCCESS;
> + case OPAL_IMC_COUNTERS_TRACE:
> + phys_core_id = cpu_get_core_index(c);
> + port_id = phys_core_id % 4;
> +
> + if (proc_chip_quirks & QUIRK_MAMBO_CALLOUTS)
> + return OPAL_SUCCESS;
> +
> + 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("IMC: error in xscom_write for htm_mode\n");
> + return OPAL_HARDWARE;
> + }
> + return OPAL_SUCCESS;
> +
> }
>
> return OPAL_SUCCESS;
> --
> 2.17.1
>
--
Stewart Smith
OPAL Architect, IBM.
More information about the Skiboot
mailing list