[dpdk-dev] [PATCH v3 15/29] crypto/qat: use eal I/O device memory read/write API

[Ferruh Yigit]

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
>