Re: [PATCH] cxl: Rework the implementation of cxl_stop_trace_psl9()

[Frederic Barrat <fbarrat@linux.vnet.ibm.com>]

Le 11/10/2017 à 14:30, Vaibhav Jain a écrit :
> Presently the PSL9 specific cxl_stop_trace_psl9() only stops the RX0
> traces on the CXL adapter when a PSL error irq is triggered. The patch
> updates the function to stop all the traces arrays and move them to
> the FIN state. The implementation issues the mmio to TRACECFG register
> to stop the trace array iff it already not in FIN state. This prevents
> the issue of trace data being reset in case of multiple stop mmio
> issued for a single trace array.
> 
> Also the patch does some refactoring of existing cxl_stop_trace_psl9()
> and cxl_stop_trace_psl8() functions by moving them to 'pci.c' from
> 'debugfs.c' file and marking them as static.
> 
> Signed-off-by: Vaibhav Jain <vaibhav@linux.vnet.ibm.com>
> ---

Looks ok to me.
Acked-by: Frederic Barrat <fbarrat@linux.vnet.ibm.com>


>   drivers/misc/cxl/cxl.h     | 14 ++++----------
>   drivers/misc/cxl/debugfs.c | 22 ----------------------
>   drivers/misc/cxl/pci.c     | 38 ++++++++++++++++++++++++++++++++++++++
>   3 files changed, 42 insertions(+), 32 deletions(-)
> 
> diff --git a/drivers/misc/cxl/cxl.h b/drivers/misc/cxl/cxl.h
> index 111c689b1771..d064ec24cdca 100644
> --- a/drivers/misc/cxl/cxl.h
> +++ b/drivers/misc/cxl/cxl.h
> @@ -114,6 +114,7 @@ static const cxl_p1_reg_t CXL_PSL9_TRACECFG = {0x0368};
>   static const cxl_p1_reg_t CXL_PSL9_APCDEDALLOC = {0x0378};
>   static const cxl_p1_reg_t CXL_PSL9_APCDEDTYPE = {0x0380};
>   static const cxl_p1_reg_t CXL_PSL9_TNR_ADDR = {0x0388};
> +static const cxl_p1_reg_t CXL_PSL9_CTCCFG = {0x0390};
>   static const cxl_p1_reg_t CXL_PSL9_GP_CT = {0x0398};
>   static const cxl_p1_reg_t CXL_XSL9_IERAT = {0x0588};
>   static const cxl_p1_reg_t CXL_XSL9_ILPP  = {0x0590};
> @@ -416,6 +417,9 @@ static const cxl_p2n_reg_t CXL_PSL_WED_An     = {0x0A0};
>   #define CXL_CARD_MINOR(adapter) (adapter->adapter_num * CXL_DEV_MINORS)
>   #define CXL_DEVT_ADAPTER(dev) (MINOR(dev) / CXL_DEV_MINORS)
> 
> +#define CXL_PSL9_TRACEID_MAX 0xAU
> +#define CXL_PSL9_TRACESTATE_FIN 0x3U
> +
>   enum cxl_context_status {
>   	CLOSED,
>   	OPENED,
> @@ -940,8 +944,6 @@ int cxl_debugfs_adapter_add(struct cxl *adapter);
>   void cxl_debugfs_adapter_remove(struct cxl *adapter);
>   int cxl_debugfs_afu_add(struct cxl_afu *afu);
>   void cxl_debugfs_afu_remove(struct cxl_afu *afu);
> -void cxl_stop_trace_psl9(struct cxl *cxl);
> -void cxl_stop_trace_psl8(struct cxl *cxl);
>   void cxl_debugfs_add_adapter_regs_psl9(struct cxl *adapter, struct dentry *dir);
>   void cxl_debugfs_add_adapter_regs_psl8(struct cxl *adapter, struct dentry *dir);
>   void cxl_debugfs_add_adapter_regs_xsl(struct cxl *adapter, struct dentry *dir);
> @@ -977,14 +979,6 @@ static inline void cxl_debugfs_afu_remove(struct cxl_afu *afu)
>   {
>   }
> 
> -static inline void cxl_stop_trace_psl9(struct cxl *cxl)
> -{
> -}
> -
> -static inline void cxl_stop_trace_psl8(struct cxl *cxl)
> -{
> -}
> -
>   static inline void cxl_debugfs_add_adapter_regs_psl9(struct cxl *adapter,
>   						    struct dentry *dir)
>   {
> diff --git a/drivers/misc/cxl/debugfs.c b/drivers/misc/cxl/debugfs.c
> index 52e3d97db114..cf4ee3114ab1 100644
> --- a/drivers/misc/cxl/debugfs.c
> +++ b/drivers/misc/cxl/debugfs.c
> @@ -15,28 +15,6 @@
> 
>   static struct dentry *cxl_debugfs;
> 
> -void cxl_stop_trace_psl9(struct cxl *adapter)
> -{
> -	/* Stop the trace */
> -	cxl_p1_write(adapter, CXL_PSL9_TRACECFG, 0x4480000000000000ULL);
> -}
> -
> -void cxl_stop_trace_psl8(struct cxl *adapter)
> -{
> -	int slice;
> -
> -	/* Stop the trace */
> -	cxl_p1_write(adapter, CXL_PSL_TRACE, 0x8000000000000017LL);
> -
> -	/* Stop the slice traces */
> -	spin_lock(&adapter->afu_list_lock);
> -	for (slice = 0; slice < adapter->slices; slice++) {
> -		if (adapter->afu[slice])
> -			cxl_p1n_write(adapter->afu[slice], CXL_PSL_SLICE_TRACE, 0x8000000000000000LL);
> -	}
> -	spin_unlock(&adapter->afu_list_lock);
> -}
> -
>   /* Helpers to export CXL mmaped IO registers via debugfs */
>   static int debugfs_io_u64_get(void *data, u64 *val)
>   {
> diff --git a/drivers/misc/cxl/pci.c b/drivers/misc/cxl/pci.c
> index d185b47eb536..bb7fd3f4edab 100644
> --- a/drivers/misc/cxl/pci.c
> +++ b/drivers/misc/cxl/pci.c
> @@ -1747,6 +1747,44 @@ static void cxl_deconfigure_adapter(struct cxl *adapter)
>   	pci_disable_device(pdev);
>   }
> 
> +static void cxl_stop_trace_psl9(struct cxl *adapter)
> +{
> +	int traceid;
> +	u64 trace_state, trace_mask;
> +	struct pci_dev *dev = to_pci_dev(adapter->dev.parent);
> +
> +	/* read each tracearray state and issue mmio to stop them is needed */
> +	for (traceid = 0; traceid <= CXL_PSL9_TRACEID_MAX; ++traceid) {
> +		trace_state = cxl_p1_read(adapter, CXL_PSL9_CTCCFG);
> +		trace_mask = (0x3ULL << (62 - traceid * 2));
> +		trace_state = (trace_state & trace_mask) >> (62 - traceid * 2);
> +		dev_dbg(&dev->dev, "cxl: Traceid-%d trace_state=0x%0llX\n",
> +			traceid, trace_state);
> +
> +		/* issue mmio if the trace array isn't in FIN state */
> +		if (trace_state != CXL_PSL9_TRACESTATE_FIN)
> +			cxl_p1_write(adapter, CXL_PSL9_TRACECFG,
> +				     0x8400000000000000ULL | traceid);
> +	}
> +}
> +
> +static void cxl_stop_trace_psl8(struct cxl *adapter)
> +{
> +	int slice;
> +
> +	/* Stop the trace */
> +	cxl_p1_write(adapter, CXL_PSL_TRACE, 0x8000000000000017LL);
> +
> +	/* Stop the slice traces */
> +	spin_lock(&adapter->afu_list_lock);
> +	for (slice = 0; slice < adapter->slices; slice++) {
> +		if (adapter->afu[slice])
> +			cxl_p1n_write(adapter->afu[slice], CXL_PSL_SLICE_TRACE,
> +				      0x8000000000000000LL);
> +	}
> +	spin_unlock(&adapter->afu_list_lock);
> +}
> +
>   static const struct cxl_service_layer_ops psl9_ops = {
>   	.adapter_regs_init = init_implementation_adapter_regs_psl9,
>   	.invalidate_all = cxl_invalidate_all_psl9,
>