[Skiboot] [PATCH 3/5] NX: Add P9 NX support for 842 compression engine

Haren Myneni haren at linux.vnet.ibm.com
Wed Feb 22 09:42:49 AEDT 2017


On 02/21/2017 12:07 PM, Sukadev Bhattiprolu wrote:
> Haren Myneni [haren at linux.vnet.ibm.com] wrote:
>>
>> This patch adds changes needed for 842 compression engine on power 9.
>> Virtual Accelerator Switch (VAS) is used to access NX 842 engine on P9
>> and the chasnnel setup be will done with receive FIFO. So RxFIFO
>> address, logical partition ID (lpid), process ID (pid) and thread ID
>> (tid) are used for this setup. p9 NX supports high and normal priority
>> FIFOs. skiboot is not involved to process data with 842 engine, but
>> configures User Mode Access Control (UMAC) registers with these values
>> and provide them to kernel.
>>
>> Also configures registers to setup and enable / disable the engine with
> 
> nit: /configures/configure the/
> 
>> the appropriate registers. Creates the following device-tree entries to
>> provide RxFIFO address, lpid, pid and tid values so that kernel can
>> drive P9 NX 842 engine.
>>
>>    /ibm,nx-842-high	: High priority 842 RxFIFO
>>    /ibm,nx-842-normal	: Normal priority 842 RxFIFO
> 
> These nodes are in the xscom* directory? Would be useful to mention that.
These nodes will be under /xscom@<xscom_addr>/nx@<nx_addr>. Will change it.
>>
>> Each RxFIFO node contains:
>>    rx-fifo-address	: Address represents for RxFIFO buffer
> 
> nit: s/represents for/of/
'RxFIFO address' should be better I think
> 
> Maybe a note to say that we want to allocate a large contiguous buffer
> so we allocate in skiboot and pass address to kernel?
> 
>>    lpid			: Chip ID
>>    pid			: 842 coprocessor type
>>    tid 			: Priority (either high or normal)
> 
> Does the hardware require these mapping or is it just a simple way of
> finding unique values for LPID/PID/TID? If so, see also the GCID2LPID()
> suggestion below.
> 
>>
>> Signed-off-by: Haren Myneni <haren at us.ibm.com>
>> ---
>>  hw/nx-842.c      |   68 ++++++++++++++++++--
>>  hw/nx-compress.c |  178 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
>>  include/nx.h     |    4 +
>>  3 files changed, 242 insertions(+), 8 deletions(-)
>>
>> diff --git a/hw/nx-842.c b/hw/nx-842.c
>> index 3ccb549..a284803 100644
>> --- a/hw/nx-842.c
>> +++ b/hw/nx-842.c
>> @@ -20,12 +20,13 @@
>>  #include <io.h>
>>  #include <cpu.h>
>>  #include <nx.h>
>> +#include <vas.h>
>>
>>  /* Configuration settings */
>>  #define CFG_842_FC_ENABLE	(0x1f) /* enable all 842 functions */
>>  #define CFG_842_ENABLE		(1) /* enable 842 engines */
>> -#define DMA_COMPRESS_PREFETCH	(1) /* enable prefetching (on P8) */
>> -#define DMA_DECOMPRESS_PREFETCH	(1) /* enable prefetching (on P8) */
>> +#define DMA_COMPRESS_PREFETCH	(1) /* enable prefetching (on P8 or P9) */
>> +#define DMA_DECOMPRESS_PREFETCH	(1) /* enable prefetching (on P8 or P9) */
>>  #define DMA_COMPRESS_MAX_RR	(15) /* range 1-15 */
>>  #define DMA_DECOMPRESS_MAX_RR	(15) /* range 1-15 */
>>  #define DMA_SPBC		(1) /* write SPBC in CPB */
>> @@ -90,6 +91,32 @@ static int nx_cfg_842(u32 gcid, u64 xcfg)
>>  	return rc;
>>  }
>>
>> +static int nx_cfg_842_umac(struct dt_node *node, u32 gcid, u32 pb_base)
>> +{	
>> +	int rc;
>> +	u64 umac_bar, umac_ctrl, umac_notify;
>> +	struct dt_node *nx_node;
>> +	
>> +	nx_node = dt_new(node, "ibm,nx-842-high");
>> +	umac_bar = pb_base + NX_P9_842_HIGH_PRI_RX_FIFO_BAR;
>> +	umac_ctrl = pb_base + NX_P9_842_HIGH_PRI_RX_FIFO_CTRL;
>> +	umac_notify = pb_base + NX_P9_842_HIGH_PRI_RX_FIFO_NOTIFY_MATCH;
>> +	rc = nx_set_rx_fifo(nx_node, gcid, umac_bar, umac_notify, umac_ctrl,
>> +				NX_CT_842, RX_FIFO_HIGH_PRIORITY);
>> +	if (rc)
>> +		return rc;
>> +
>> +	nx_node= dt_new(node, "ibm,nx-842-normal");
>> +	umac_bar = pb_base + NX_P9_842_NORMAL_PRI_RX_FIFO_BAR;
>> +	umac_ctrl = pb_base + NX_P9_842_NORMAL_PRI_RX_FIFO_CTRL;
>> +	umac_notify = pb_base + NX_P9_842_NORMAL_PRI_RX_FIFO_NOTIFY_MATCH;
>> +	rc = nx_set_rx_fifo(nx_node, gcid, umac_bar, umac_notify, umac_ctrl,
>> +				NX_CT_842, RX_FIFO_NORMAL_PRIORITY);
>> +
> nit: extra new line.
>> +	
>> +	return rc;
>> +}
>> +
>>  static int nx_cfg_842_dma(u32 gcid, u64 xcfg)
>>  {
>>  	u64 cfg;
>> @@ -99,7 +126,7 @@ static int nx_cfg_842_dma(u32 gcid, u64 xcfg)
>>  	if (rc)
>>  		return rc;
>>
>> -	if (proc_gen == proc_gen_p8) {
>> +	if (proc_gen >= proc_gen_p8) {
>>  		cfg = SETFIELD(NX_DMA_CFG_842_COMPRESS_PREFETCH, cfg,
>>  			       DMA_COMPRESS_PREFETCH);
>>  		cfg = SETFIELD(NX_DMA_CFG_842_DECOMPRESS_PREFETCH, cfg,
>> @@ -112,14 +139,16 @@ static int nx_cfg_842_dma(u32 gcid, u64 xcfg)
>>  		       DMA_DECOMPRESS_MAX_RR);
>>  	cfg = SETFIELD(NX_DMA_CFG_842_SPBC, cfg,
>>  		       DMA_SPBC);
>> -	cfg = SETFIELD(NX_DMA_CFG_842_CSB_WR, cfg,
>> +	if (proc_gen < proc_gen_p9) {
>> +		cfg = SETFIELD(NX_DMA_CFG_842_CSB_WR, cfg,
>>  		       DMA_CSB_WR);
>> -	cfg = SETFIELD(NX_DMA_CFG_842_COMPLETION_MODE, cfg,
>> +		cfg = SETFIELD(NX_DMA_CFG_842_COMPLETION_MODE, cfg,
>>  		       DMA_COMPLETION_MODE);
>> -	cfg = SETFIELD(NX_DMA_CFG_842_CPB_WR, cfg,
>> +		cfg = SETFIELD(NX_DMA_CFG_842_CPB_WR, cfg,
>>  		       DMA_CPB_WR);
>> -	cfg = SETFIELD(NX_DMA_CFG_842_OUTPUT_DATA_WR, cfg,
>> +		cfg = SETFIELD(NX_DMA_CFG_842_OUTPUT_DATA_WR, cfg,
>>  		       DMA_OUTPUT_DATA_WR);
>> +	}
>>
>>  	rc = xscom_write(gcid, xcfg, cfg);
>>  	if (rc)
>> @@ -188,3 +217,28 @@ void nx_enable_842(struct dt_node *node, u32 gcid, u32 pb_base)
>>  	dt_add_property_cells(node, "ibm,842-coprocessor-type", NX_CT_842);
>>  	dt_add_property_cells(node, "ibm,842-coprocessor-instance", gcid + 1);
>>  }
>> +
>> +void p9_nx_enable_842(struct dt_node *node, u32 gcid, u32 pb_base)
>> +{
>> +	u64 cfg_dma, cfg_ee;
>> +	int rc;
>> +
>> +	cfg_dma = pb_base + NX_P9_DMA_CFG;
>> +	cfg_ee = pb_base + NX_P9_EE_CFG;
>> +
>> +	rc = nx_cfg_842_dma(gcid, cfg_dma);
>> +	if (rc) 
>> +		return;
>> +
>> +	rc = nx_cfg_842_umac(node, gcid, pb_base);
>> +	if (rc)
>> +		return;
>> +
>> +	rc = nx_cfg_842_ee(gcid, cfg_ee);
>> +	if (rc)
>> +		return;
>> +
>> +	prlog(PR_INFO, "NX%d: 842 Coprocessor Enabled\n", gcid);
>> +
>> +}
>> +
>> diff --git a/hw/nx-compress.c b/hw/nx-compress.c
>> index 2ea2734..b4abe9e 100644
>> --- a/hw/nx-compress.c
>> +++ b/hw/nx-compress.c
>> @@ -20,15 +20,191 @@
>>  #include <io.h>
>>  #include <cpu.h>
>>  #include <nx.h>
>> +#include <vas.h>
>> +
>> +static int nx_cfg_umac_tx_wc(u32 gcid, u64 xcfg)
>> +{
>> +	int rc = 0;
>> +	u64 cfg;
>> +
>> +	cfg = vas_get_wcbs_bar(gcid);
>> +	if (!cfg) {
>> +		prerror("NX%d: ERROR finding WC Backing store BAR\n", gcid);
>> +		return -ENOMEM;
>> +	}
>> +
>> +	/*
>> +	 * NOTE: Write the entire bar address to SCOM. VAS/NX will extract
>> +	 *       the relevant (NX_P9_UMAC_TX_WINDOW_CONTEXT_ADDR) bits.
>> +	 *       IOW, _don't_:
> 
> s/:/just write the bit field like:/
> 
>> +	 *
>> +	 *       cfg = SETFIELD(NX_P9_UMAC_TX_WINDOW_CONTEXT_ADDR, 0ULL, cfg);
>> +	 */
>> +	rc = xscom_write (gcid, xcfg, cfg);
>> +
>> +	if (rc)
>> +		prerror("NX%d: ERROR: UMAC SEND WC BAR, %d\n", gcid, rc);
>> +	else
>> +		prlog(PR_DEBUG,"NX%d: UMAC SEND WC BAR, 0x%016lx, " 
>> +				"xcfg 0x%llx\n",
>> +			gcid, (unsigned long)cfg, xcfg);
>> +
>> +	return rc;
>> +}
>> +
>> +static int nx_cfg_umac_vas_mmio(u32 gcid, u64 xcfg)
>> +{
>> +	int rc = 0;
>> +	u64 cfg;
>> +
>> +	cfg = vas_get_hvwc_mmio_bar(gcid);
>> +	/*
>> +	 * NOTE: Write the entire bar address to SCOM. VAS/NX will extract
>> +	 *       the relevant (NX_P9_UMAC_VAS_MMIO_ADDR) bits. IOW, _don't_:
> s/:/just write the bit field like:/
>> +	 *
>> +	 *       cfg = SETFIELD(NX_P9_UMAC_VAS_MMIO_ADDR, 0ULL, cfg);
>> +	 */
>> +	rc = xscom_write (gcid, xcfg, cfg);
>> +
>> +	if (rc)
>> +		prerror("NX%d: ERROR: UMAC VAS MMIO BAR, %d\n", gcid, rc);
>> +	else
>> +		prlog(PR_DEBUG, "NX%d: UMAC VAS MMIO BAR, 0x%016lx, "
>> +				"xcfg 0x%llx\n",
>> +			gcid, (unsigned long)cfg, xcfg);
>> +	
>> +	return rc;
>> +}
>> +
>> +static int nx_cfg_umac_status_ctrl(u32 gcid, u64 xcfg)
>> +{
>> +	u64 uctrl;
>> +	int rc;
>> +#define CRB_ENABLE	1
>> +
>> +	rc = xscom_read(gcid, xcfg, &uctrl);
>> +	if (rc)
>> +		return rc;
>> +	
>> +	uctrl = SETFIELD(NX_P9_UMAC_STATUS_CTRL_CRB_ENABLE, uctrl, CRB_ENABLE);
>> +	rc = xscom_write(gcid, xcfg, uctrl);
>> +	if (rc) 
>> +		prerror("NX%d: ERROR: Setting UMAC Status Control failure %d\n",
>> +			gcid, rc);
>> +	else 
>> +		prlog(PR_DEBUG, "NX%d: Setting UMAC FIFO bar 0x%016lx\n",
>> +			gcid, (unsigned long)uctrl);
>> +
>> +	return rc;
>> +}
>> +
>> +int nx_set_rx_fifo(struct dt_node *node, u32 gcid, u64 umac_bar,
> 
> s/nx_set_rx_fifo/nx_cfg_rx_fifo/ for consistency?

This function allocates buffer and configure RxFIFO regs. I can change it.  

> 
>> +			u64 umac_notify, u64 umac_ctrl,
>> +			u32 ct_type, u32 priority)
> 
> s/ct_type/ct/?

Sure will change it

> 
>> +{
>> +	u64 cfg;
>> +	int rc;
>> +	uint64_t fifo;
>> +#define MATCH_ENABLE    1
>> +#define MAX_QUEUED      256
>> +#define HPRI_MAX_READ   256
>> +
>> +	fifo = (uint64_t) local_alloc(gcid, RX_FIFO_SIZE, RX_FIFO_SIZE);
>> +	assert(fifo);
>> +
>> +	rc = xscom_read(gcid, umac_bar, &cfg);
>> +	if (rc)
>> +		return rc;
>> +
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_BAR_ADDR, cfg, fifo);
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_BAR_SIZE, cfg, RX_FIFO_MAX_CRB);
> 
> According to the spec, I think we need to replace RX_FIFO_MAX_CRB with 0x5.
> Basically, the
> 
> 	log(RX_FIFO_MAX_CRB/1024)

Thanks for pointing. It is a bug. RX_FIFO_MAX_CRB_SIZE is a macro. I think, should be better to define this properly with clear comments. 
 
> 
>> +
>> +	rc = xscom_write(gcid, umac_bar, cfg);
>> +	if (rc) {
>> +		prerror("NX%d: ERROR: Setting UMAC FIFO bar failure %d\n",
>> +			gcid, rc);
>> +		return rc;
>> +	} else 
>> +		prlog(PR_DEBUG, "NX%d: Setting UMAC FIFO bar 0x%016lx\n",
>> +			gcid, (unsigned long)cfg);
>> +
>> +	rc = xscom_read(gcid, umac_notify, &cfg);
>> +	if (rc) 
>> +		return rc;
>> +
>> +	prerror(" Skiboot1 RX fifo address 0x%Lx lpid %d pid %d tid %d\n",
>> +			fifo, gcid + 1, ct_type, priority);
>> +
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_NOTIFY_MATCH_LPID, cfg, gcid + 1);
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_NOTIFY_MATCH_PID, cfg, ct_type);
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_NOTIFY_MATCH_TID, cfg, priority);
> 
> We are using gcid, ct_type and priority as LPID/PID/TID in multiple
> places. Maybe good to add a comment or use simple wrappers like
> 
> 	GCID2LPID(gcid), CT2PID(ct), PRI2TID(priority)

Agree comment here will be make it more clear. We are using these values to come up unique combination (LPID, PID and TID). 


> 
> 
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_NOTIFY_MATCH_MATCH_ENABLE, cfg,
>> +			MATCH_ENABLE);
>> +
>> +	rc = xscom_write(gcid, umac_notify, cfg);
>> +	if (rc) {
>> +		prerror("NX%d: ERROR: Setting UMAC notify match failure %d\n",
>> +			gcid, rc);
>> +		return rc;
>> +	} else 
>> +		prlog(PR_DEBUG, "NX%d: Setting UMAC notify match 0x%016lx\n",
>> +				gcid, (unsigned long)cfg);
>> +	
>> +	rc = xscom_read(gcid, umac_ctrl, &cfg);
>> +	if (rc)
>> +		return rc;
>> +
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_CTRL_QUEUED, cfg, MAX_QUEUED);
>> +	cfg = SETFIELD(NX_P9_RX_FIFO_CTRL_HPRI_MAX_READ, cfg, HPRI_MAX_READ);
>> +
>> +	rc = xscom_write(gcid, umac_ctrl, cfg);
>> +	if (rc) {
>> +		prerror("NX%d: ERROR: Setting UMAC control failure %d\n",
>> +				gcid, rc);
>> +		return rc;
>> +	} else
>> +		prlog(PR_DEBUG, "NX%d: Setting UMAC control 0x%016lx\n",
>> +				gcid, (unsigned long)cfg);
>> +	
>> +	dt_add_property_u64(node, "rx-fifo-address", fifo);
>> +	dt_add_property_cells(node, "lpid", gcid + 1);
>> +	dt_add_property_cells(node, "pid", ct_type);
>> +	dt_add_property_cells(node, "tid", priority);
>> +
>> +	return 0;
>> +}
>>
>>  void nx_create_compress_node(struct dt_node *node)
>>  {
>>  	u32 gcid, pb_base;
>> +	int rc;
>>
>>  	gcid = dt_get_chip_id(node);
>>  	pb_base = dt_get_address(node, 0, NULL);
>>
>>  	prlog(PR_INFO, "NX%d: 842 at 0x%x\n", gcid, pb_base);
>>
>> -	nx_enable_842(node, gcid, pb_base);
>> +	if (dt_node_is_compatible(node, "ibm,power9-nx")) {
>> +		u64 cfg_mmio, cfg_txwc, cfg_uctrl;
>> +
>> +		printf("Found ibm,power9-nx\n");
>> +		cfg_mmio = pb_base + NX_P9_UMAC_VAS_MMIO_BAR;
>> +		cfg_txwc = pb_base + NX_P9_UMAC_TX_WINDOW_CONTEXT_BAR;
>> +		cfg_uctrl = pb_base + NX_P9_UMAC_STATUS_CTRL;
>> +
>> +		rc = nx_cfg_umac_vas_mmio(gcid, cfg_mmio);
>> +		if (rc)
>> +			return;
>> +
>> +		rc = nx_cfg_umac_tx_wc(gcid, cfg_txwc);
>> +		if (rc)
>> +			return;
>> +		
>> +		rc = nx_cfg_umac_status_ctrl(gcid, cfg_uctrl);
>> +		if (rc)
>> +			return;
>> +
>> +		p9_nx_enable_842(node, gcid, pb_base);
>> +	} else 
>> +		nx_enable_842(node, gcid, pb_base);
>>  }
>> diff --git a/include/nx.h b/include/nx.h
>> index 0a7b1b0..18fa91f 100644
>> --- a/include/nx.h
>> +++ b/include/nx.h
>> @@ -381,7 +381,11 @@ extern void nx_create_crypto_node(struct dt_node *);
>>  extern void nx_create_compress_node(struct dt_node *);
>>
>>  extern void nx_enable_842(struct dt_node *, u32 gcid, u32 pb_base);
>> +extern void p9_nx_enable_842(struct dt_node *, u32 gcid, u32 pb_base);
>>
>> +extern int nx_set_rx_fifo(struct dt_node *node, u32 gcid, u64 umac_bar,
>> +			u64 umac_notify, u64 umac_ctrl, u32 ct_type,
>> +			u32 priority);
>>  extern void nx_init(void);
>>
>>  #endif /* __NX_H */
>> -- 
>> 1.7.1
>>
>>



More information about the Skiboot mailing list