[Skiboot] [PATCH V2] NX: Add NX coprocessor init opal call

[Haren Myneni]

Thanks for the review,

On 06/04/2018 09:08 PM, Stewart Smith wrote:
> Haren Myneni <haren at linux.vnet.ibm.com> writes:
>>     
>> The read offset (4:11) in Receive FIFO control register is incremented
>> by FIFO size whenever CRB read by NX. But the index in RxFIFO has to
>> match with the corresponding entry in FIFO maintained by VAS in kernel.
>> VAS entry is reset to 0 when opening the receive window during driver
>> initialization. So when NX842 is reloaded or in kexec boot, possibility
>> of mismatch between RxFIFO control register and VAS entries in kernel.
>> It could cause CRB failure / timeout from NX.
>>     
>> This patch adds nx_coproc_init opal call for kernel to initialize
>> readOffset (4:11) and Queued (15:23) in RxFIFO control register.
>>     
>> Signed-off-by: Haren Myneni <haren at us.ibm.com>
>> ---
>> Changelog:
>> v2: Added doc/opal-api/opal_nx_coproc_init-167.rst
>>
>> diff --git a/doc/opal-api/opal_nx_coproc_init-167.rst b/doc/opal-api/opal_nx_coproc_init-167.rst
>> new file mode 100644
>> index 0000000..d05aed4
>> --- /dev/null
>> +++ b/doc/opal-api/opal_nx_coproc_init-167.rst
>> @@ -0,0 +1,32 @@
>> +.. _opal_nx_coproc_init:
>> +
>> +OPAL_NX_COPROC_INIT
>> +===================
>> +
>> +This OPAL call resets read offset and queued entries in high and normal
>> +priority receive FIFO control registers. The kernel initializes read
>> +offset entry in RXFIFO that it maintains during initialization. So this
>> +register reset is needed for NX module reload or in kexec boot to make sure
>> +read offset value matches with kernel entries. Otherwise NX reads requests
>> +with wrong offset in RxFIFO which could cause NX request failures.
>> +
>> +The kernel initiates this call for each coprocessor type such as 842 and
>> +GZIP per NX instance.
>> +
>> +Arguments
>> +---------
>> +::
>> +
>> +  uint32_t chip_id
>> +    Contains value of the chip number identified at boot time.
>> +
>> +  uint32_t ct
>> +    Contains NX coprocessor type such as 842 and GZIP.
> 
> Could you make this:
> 
> ``uint32_t chip_id``
>   Contains value of the chip number identified at boot time.
> ``uint32_t pid``
>   Contains NX coprocessor type (pid from the device tree).
> 
> to make it a bit more obvious what's being passed in?

I will make this change. 
> 
> 
>> +
>> +Returns
>> +-------
>> +OPAL_SUCCESS
>> +  The quiesce call was successful.
> 
> Err... this isn't OPAL_QUIESCE

sorry, my mistake
.
> 
>> diff --git a/hw/nx-compress.c b/hw/nx-compress.c
>> index 9b89664..cf1c3e9 100644
>> --- a/hw/nx-compress.c
>> +++ b/hw/nx-compress.c
>> @@ -21,6 +21,7 @@
>>  #include <cpu.h>
>>  #include <nx.h>
>>  #include <vas.h>
>> +#include <opal.h>
>>
>>  static int nx_cfg_umac_tx_wc(u32 gcid, u64 xcfg)
>>  {
>> @@ -206,14 +207,75 @@ int nx_cfg_rx_fifo(struct dt_node *node, const char *compat,
>>  	return 0;
>>  }
>>
>> +static int nx_init_fifo_ctrl(u32 gcid, u64 fifo_ctrl)
>> +{
>> +	u64 cfg;
>> +	int rc = 0;
>> +
>> +	rc = xscom_read(gcid, fifo_ctrl, &cfg);
>> +	if (rc)
>> +		return rc;
>> +
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_CTRL_READ_OFFSET, cfg, 0);
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_CTRL_QUEUED, cfg, 0);
>> +
>> +	rc = xscom_write(gcid, fifo_ctrl, cfg);
>> +
>> +	return rc;
>> +}
>> +
>> +
>> +static int opal_nx_coproc_init(u32 gcid, u32 ct)
>> +{
>> +	struct proc_chip *chip;
>> +	u64 fifo, fifo_hi;
>> +	u32 nx_base;
>> +	int rc;
>> +
>> +	chip =  get_chip(gcid);
>> +	if (!chip)
>> +		return OPAL_PARAMETER;
>> +
>> +	nx_base =  chip->nx_base;
>> +	if (!nx_base)
>> +		return OPAL_PARAMETER;
>> +
>> +	switch (ct) {
>> +	case NX_CT_842:
>> +		fifo_hi = nx_base + NX_P9_842_HIGH_PRI_RX_FIFO_CTRL;
>> +		fifo = nx_base + NX_P9_842_NORMAL_PRI_RX_FIFO_CTRL;
>> +		break;
>> +	case NX_CT_GZIP:
>> +		fifo_hi = nx_base + NX_P9_GZIP_HIGH_PRI_RX_FIFO_CTRL;
>> +		fifo = nx_base + NX_P9_GZIP_NORMAL_PRI_RX_FIFO_CTRL;
>> +		break;
>> +	default:
>> +		prlog(PR_EMERG, "OPAL: Unknown NX coprocessor type\n");
>> +		return OPAL_PARAMETER;
>> +	}
>> +
>> +	rc  = nx_init_fifo_ctrl(gcid, fifo_hi);
>> +
>> +	if (!rc)
>> +		rc  = nx_init_fifo_ctrl(gcid, fifo);
>> +
>> +	return rc;
>> +}
>> +
>> +opal_call(OPAL_NX_COPROC_INIT, opal_nx_coproc_init, 2);
>> +
>>  void nx_create_compress_node(struct dt_node *node)
>>  {
>>  	u32 gcid, pb_base;
>> +	struct proc_chip *chip;
>>  	int rc;
>>
>>  	gcid = dt_get_chip_id(node);
>>  	pb_base = dt_get_address(node, 0, NULL);
>>
>> +	chip = get_chip(gcid);
>> +	chip->nx_base =  pb_base;
>> +
>>  	prlog(PR_INFO, "NX%d: 842 at 0x%x\n", gcid, pb_base);
>>
>>  	if (dt_node_is_compatible(node, "ibm,power9-nx")) {
> 
> This looks like it'll break on POWER8 as we're adding the base there
> even on P8, which means that the OPAL call won't error out as you'd
> expect, but rather just do some random SCOMs.
> 
> I gather we don't need this kind of call for P8?
> 
> Even if we don't, I'd prefer we error out gracefully rather than do
> random scoms.

Correct, this OPAL call should not be executed on P8. I will add < P9 check in opal call.

if (proc_gen < proc_gen_p9)
	return OPAL_UNSUPPORTED;


Thanks
Haren

>