[Skiboot] [PATCH v12 08/10] skiboot: Add opal call to enable/disable Nest IMC

[Michael Neuling]

On Sun, 2017-05-21 at 20:40 +0530, Madhavan Srinivasan wrote:
> From: Anju T Sudhakar <anju at linux.vnet.ibm.com>
> 
> Add new opal calls to start, stop the Nest IMC microcode running in the
> OCC complex. Also, check the status from the control block structure before
> starting/stopping the IMC microcode.
> 
> This patch also add an opal call to init the counters. But for Nest IMC this
> function is a no-op currently.
> 
> 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           | 77
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++
>  include/opal-api.h | 11 +++++++-
>  2 files changed, 87 insertions(+), 1 deletion(-)
> 
> diff --git a/hw/imc.c b/hw/imc.c
> index 00e19154d641..d56b3eb251b2 100644
> --- a/hw/imc.c
> +++ b/hw/imc.c
> @@ -336,3 +336,80 @@ err:
>  	prerror("IMC Devices not added\n");
>  	free(imc_catalog_buf);
>  }
> +
> +/*
> + * opal_imc_counters_init : This call initialize the IMC engine.
> + *
> + * For Nest IMC, this is no-op and returns OPAL_SUCCESS at this point.
> + */
> +static int64_t opal_imc_counters_init(uint32_t type)
> +{
> +	return OPAL_SUCCESS;
> +}

Why don't we do this on in imc_init()?  Why do we rely on the OS doing this?

Anyway... as mentioned before, this alone doesn't compile.

> +opal_call(OPAL_IMC_COUNTERS_INIT, opal_imc_counters_init, 1);
> +
> +/* opal_imc_counters_control_start: This call starts the nest imc engine. */
> +static int64_t opal_imc_counters_start(uint32_t type)
> +{
> +	u64 op, status;
> +	struct imc_chip_cb *cb;
> +	int ret = OPAL_SUCCESS;

I'm not sure there is any benefit in having ret.  Makes the code less readable. 
Just do return OPAL_SUCCESS below.
> +
> +	switch (type) {
> +	case OPAL_IMC_COUNTERS_NEST:
> +		/* Fetch the IMC control block structure */
> +		cb = get_imc_cb();
> +		status = be64_to_cpu(cb->imc_chip_run_status);
> +
> +		/* Check whether the engine is already running */
> +		if (status == NEST_IMC_RUNNING)
> +			return ret;

Do we need a memory barrier here?  What ensures ordering between the status read
above and the command below?

> +
> +		/* Set the run command */
> +		op = NEST_IMC_ENABLE;
> +
> +		/* Write the command to the control block now */
> +		cb->imc_chip_command = op;

Do we need a memory barrier before we can start reading these after this op? 
and if so, who's responsibility is it do do that?  OPAL, or Linux?

> +
> +		break;
> +	default:
> +		prerror("IMC: Unknown Domain in _START\n");
> +		return OPAL_PARAMETER;
> +	}
> +
> +	return ret;
> +}
> +opal_call(OPAL_IMC_COUNTERS_START, opal_imc_counters_start, 1);
> +
> +/* opal_imc_counters_control_stop: This call stops the nest imc engine. */
> +static int64_t opal_imc_counters_stop(uint32_t type)
> +{
> +	u64 op, status;
> +	struct imc_chip_cb *cb;
> +	int ret = OPAL_SUCCESS;
> +
> +	switch (type) {
> +	case OPAL_IMC_COUNTERS_NEST:
> +		/* Fetch the IMC control block structure */
> +		cb = get_imc_cb();
> +		status = be64_to_cpu(cb->imc_chip_run_status);

again... memory barriers?

> +		/* Check whether the engine is already stopped */
> +		if (status == NEST_IMC_PAUSE)
> +			return ret;
> +
> +		/* Set the run command */
> +		op = NEST_IMC_DISABLE;
> +
> +		/* Write the command to the control block now */
> +		cb->imc_chip_command = op;
> +
> +		break;
> +	default:
> +		prerror("IMC: Unknown Domain in _STOP\n");
> +		return OPAL_PARAMETER;
> +	}
> +
> +	return ret;
> +}
> +opal_call(OPAL_IMC_COUNTERS_STOP, opal_imc_counters_stop, 1);
> diff --git a/include/opal-api.h b/include/opal-api.h
> index 44276553b818..02927356d033 100644
> --- a/include/opal-api.h
> +++ b/include/opal-api.h
> @@ -204,7 +204,10 @@
>  #define OPAL_NPU_INIT_CONTEXT			146
>  #define OPAL_NPU_DESTROY_CONTEXT		147
>  #define OPAL_NPU_MAP_LPAR			148
> -#define OPAL_LAST				148
> +#define OPAL_IMC_COUNTERS_INIT			149
> +#define OPAL_IMC_COUNTERS_START			150
> +#define OPAL_IMC_COUNTERS_STOP			151
> +#define OPAL_LAST				151
>  
>  /* Device tree flags */
>  
> @@ -1226,6 +1229,12 @@ enum {
>  	IMC_COUNTER_PER_SYSTEM		= 0x40,
>  };
>  
> +/* Operation argument to IMC Microcode */
> +enum {
> +	OPAL_IMC_COUNTERS_NEST = 1,
> +};
> +
> +
>  #endif /* __ASSEMBLY__ */
>  
>  #endif /* __OPAL_API_H */