From mboxrd@z Thu Jan  1 00:00:00 1970
From: Ferruh Yigit <ferruh.yigit@intel.com>
Subject: Re: [PATCH v3 15/29] crypto/qat: use eal I/O device
 memory read/write API
Date: Thu, 12 Jan 2017 19:09:22 +0000
Message-ID: <6bb9980b-f546-38d5-044a-63507510f6a5@intel.com>
References: <1482832175-27199-1-git-send-email-jerin.jacob@caviumnetworks.com>
 <1484212646-10338-1-git-send-email-jerin.jacob@caviumnetworks.com>
 <1484212646-10338-16-git-send-email-jerin.jacob@caviumnetworks.com>
Mime-Version: 1.0
Content-Type: text/plain; charset=windows-1252
Content-Transfer-Encoding: 8bit
Cc: konstantin.ananyev@intel.com, thomas.monjalon@6wind.com,
 bruce.richardson@intel.com, jianbo.liu@linaro.org, viktorin@rehivetech.com,
 santosh.shukla@caviumnetworks.com, John Griffin <john.griffin@intel.com>,
 Fiona Trahe <fiona.trahe@intel.com>,
 Deepak Kumar Jain <deepak.k.jain@intel.com>
To: Jerin Jacob <jerin.jacob@caviumnetworks.com>, dev@dpdk.org
Return-path: <dev-bounces@dpdk.org>
Received: from mga01.intel.com (mga01.intel.com [192.55.52.88])
 by dpdk.org (Postfix) with ESMTP id 1605E592C
 for <dev@dpdk.org>; Thu, 12 Jan 2017 20:09:59 +0100 (CET)
In-Reply-To: <1484212646-10338-16-git-send-email-jerin.jacob@caviumnetworks.com>
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>

Hi Jerin,

On 1/12/2017 9:17 AM, Jerin Jacob wrote:
<...>

> +#include <rte_io.h>
> +
>  /* CSR write macro */
> -#define ADF_CSR_WR(csrAddr, csrOffset, val) \
> -	(void)((*((volatile uint32_t *)(((uint8_t *)csrAddr) + csrOffset)) \
> -			= (val)))
> +#define ADF_CSR_WR(csrAddr, csrOffset, val)		\
> +	rte_write32(val, (((uint8_t *)csrAddr) + csrOffset))

For IA, this update introduces an extra compiler barrier (rte_io_wmb()),
which is indeed not a must, is this correct?

If so, does it make sense to override these functions for x86, and make
rte_writeX = rte_writeX_relaxed
rte_readX = rte_readX_relaxed

>  
>  /* CSR read macro */
> -#define ADF_CSR_RD(csrAddr, csrOffset) \
> -	(*((volatile uint32_t *)(((uint8_t *)csrAddr) + csrOffset)))
> +#define ADF_CSR_RD(csrAddr, csrOffset)			\
> +	rte_read32((((uint8_t *)csrAddr) + csrOffset))

This patchset both introduces new rte_readX/rte_writeX functions, also
applies them into drivers.

While applying them, it changes the behavior.
Like above code was doing a read, but after update it does read and
read_memory_barrier.

What do you think this patchset updates usage in a manner that keeps
behavior exact same. Like using rte_read32_relaxed for this case.
And doing architecture related updates in a different patchset?

This both makes easy to see architecture specific updates, and makes
easy to trace any possible performance issues by this patchset.

>  
>  #define ADF_BANK_INT_SRC_SEL_MASK_0 0x4444444CUL
>  #define ADF_BANK_INT_SRC_SEL_MASK_X 0x44444444UL
>