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

Stewart Smith stewart at linux.vnet.ibm.com
Tue Jun 5 14:08:13 AEST 2018


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?


> +
> +Returns
> +-------
> +OPAL_SUCCESS
> +  The quiesce call was successful.

Err... this isn't OPAL_QUIESCE.

> 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.

-- 
Stewart Smith
OPAL Architect, IBM.



More information about the Skiboot mailing list