DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Zhang, Qi Z" <qi.z.zhang@intel.com>
To: "Xu, Rosen" <rosen.xu@intel.com>,
	"Zhang, Tianfei" <tianfei.zhang@intel.com>,
	"dev@dpdk.org" <dev@dpdk.org>, "Huang, Wei" <wei.huang@intel.com>
Subject: Re: [dpdk-dev] [PATCH v2 4/4] raw/ifpga/base: enhance driver reliability in multi-process
Date: Thu, 15 Oct 2020 13:16:40 +0000
Message-ID: <02462e0e2e1c4a20974b110b04105513@intel.com> (raw)
In-Reply-To: <BYAPR11MB29019B1BDF03000D851475A389020@BYAPR11MB2901.namprd11.prod.outlook.com>



> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Xu, Rosen
> Sent: Thursday, October 15, 2020 2:08 PM
> To: Zhang, Tianfei <tianfei.zhang@intel.com>; dev@dpdk.org; Huang, Wei
> <wei.huang@intel.com>
> Subject: Re: [dpdk-dev] [PATCH v2 4/4] raw/ifpga/base: enhance driver
> reliability in multi-process
> 
> Hi,
> 
> > -----Original Message-----
> > From: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Sent: Monday, September 28, 2020 9:40
> > To: dev@dpdk.org; Xu, Rosen <rosen.xu@intel.com>; Huang, Wei
> > <wei.huang@intel.com>
> > Cc: Zhang, Tianfei <tianfei.zhang@intel.com>
> > Subject: [PATCH v2 4/4] raw/ifpga/base: enhance driver reliability in
> > multi- process
> >
> > From: Wei Huang <wei.huang@intel.com>
> >
> > Current hardware protection is based on pthread mutex which work just
> > for situation of multi-thread in one process. In multi-process
> > environment, hardware state machine would be corrupted by concurrent
> > access, that means original pthread mutex mechanism need be enhanced.
> >
> > The major modifications in this patch are list below:
> > 1. Create a mutex for adapter in shared memory named
> > "mutex.IFPGA:domain:bus:dev.func" when device is probed.
> > 2. Create a shared memory named "IFPGA:domain:bus:dev.func"
> > during opae adapter is initializing. There is a reference count in
> > shared memory. Shared memory will be destroyed once reference count
> > turned to zero.
> > 3. Two mutexs are created in shared memory and initialized with flag
> > PTHREAD_PROCESS_SHARED. One for SPI and the other for I2C. They will
> > be passed to SPI and I2C driver subsequently.
> > 4. DTB data in flash will be cached in shared memory. Then
> > MAX10 driver can read DTB from shared memory instead of flash. This
> > avoid confliction of concurrent flash access between hardware and software.
> >
> > Signed-off-by: Wei Huang <wei.huang@intel.com>
> > Signed-off-by: Tianfei zhang <tianfei.zhang@intel.com>
> > ---
> > v2: fix typo in commit log. 'master' is not misspelled, it's used in original code.
> > There will be a separate patch to clean up the language.
> > ---
> >  drivers/raw/ifpga/base/ifpga_fme.c            |  52 +++-
> >  drivers/raw/ifpga/base/meson.build            |  12 +
> >  drivers/raw/ifpga/base/opae_hw_api.c          | 250
> ++++++++++++++++++
> >  drivers/raw/ifpga/base/opae_hw_api.h          |  27 +-
> >  drivers/raw/ifpga/base/opae_i2c.c             |   9 +-
> >  drivers/raw/ifpga/base/opae_i2c.h             |   1 +
> >  drivers/raw/ifpga/base/opae_intel_max10.c     | 152 ++++++-----
> >  drivers/raw/ifpga/base/opae_spi.c             |   4 +
> >  drivers/raw/ifpga/base/opae_spi.h             |   5 +
> >  drivers/raw/ifpga/base/opae_spi_transaction.c |  15 +-
> >  10 files changed, 456 insertions(+), 71 deletions(-)
> >
> > diff --git a/drivers/raw/ifpga/base/ifpga_fme.c
> > b/drivers/raw/ifpga/base/ifpga_fme.c
> > index 9057087b5..540bb1110 100644
> > --- a/drivers/raw/ifpga/base/ifpga_fme.c
> > +++ b/drivers/raw/ifpga/base/ifpga_fme.c
> > @@ -919,6 +919,25 @@ static int spi_self_checking(struct
> > intel_max10_device *dev)
> >  	return 0;
> >  }
> >
> > +static void init_spi_share_data(struct ifpga_fme_hw *fme,
> > +				struct altera_spi_device *spi)
> > +{
> > +	struct ifpga_hw *hw = (struct ifpga_hw *)fme->parent;
> > +	opae_share_data *sd = NULL;
> > +
> > +	if (hw && hw->adapter && hw->adapter->shm.ptr) {
> > +		dev_info(NULL, "transfer share data to spi\n");
> > +		sd = (opae_share_data *)hw->adapter->shm.ptr;
> > +		spi->mutex = &sd->spi_mutex;
> > +		spi->dtb_sz_ptr = &sd->dtb_size;
> > +		spi->dtb = sd->dtb;
> > +	} else {
> > +		spi->mutex = NULL;
> > +		spi->dtb_sz_ptr = NULL;
> > +		spi->dtb = NULL;
> > +	}
> > +}
> > +
> >  static int fme_spi_init(struct ifpga_feature *feature)  {
> >  	struct ifpga_fme_hw *fme = (struct ifpga_fme_hw *)feature->parent;
> > @@ -935,6 +954,7 @@ static int fme_spi_init(struct ifpga_feature *feature)
> >  	spi_master = altera_spi_alloc(feature->addr, TYPE_SPI);
> >  	if (!spi_master)
> >  		return -ENODEV;
> > +	init_spi_share_data(fme, spi_master);
> >
> >  	altera_spi_init(spi_master);
> >
> > @@ -945,7 +965,6 @@ static int fme_spi_init(struct ifpga_feature *feature)
> >  		goto spi_fail;
> >  	}
> >
> > -
> >  	fme->max10_dev = max10;
> >
> >  	/* SPI self test */
> > @@ -1084,11 +1103,15 @@ static int fme_nios_spi_init(struct
> > ifpga_feature
> > *feature)
> >  	spi_master = altera_spi_alloc(feature->addr, TYPE_NIOS_SPI);
> >  	if (!spi_master)
> >  		return -ENODEV;
> > +	init_spi_share_data(fme, spi_master);
> >
> >  	/**
> >  	 * 1. wait A10 NIOS initial finished and
> >  	 * release the SPI master to Host
> >  	 */
> > +	if (spi_master->mutex)
> > +		pthread_mutex_lock(spi_master->mutex);
> > +
> >  	ret = nios_spi_wait_init_done(spi_master);
> >  	if (ret != 0) {
> >  		dev_err(fme, "FME NIOS_SPI init fail\n"); @@ -1101,6
> > +1124,9 @@ static int fme_nios_spi_init(struct ifpga_feature *feature)
> >  	if (nios_spi_check_error(spi_master))
> >  		dev_info(fme, "NIOS_SPI INIT done, but found some error\n");
> >
> > +	if (spi_master->mutex)
> > +		pthread_mutex_unlock(spi_master->mutex);
> > +
> >  	/* 3. init the spi master*/
> >  	altera_spi_init(spi_master);
> >
> > @@ -1112,11 +1138,12 @@ static int fme_nios_spi_init(struct
> > ifpga_feature
> > *feature)
> >  		goto release_dev;
> >  	}
> >
> > +	fme->max10_dev = max10;
> > +
> >  	max10->bus = hw->pci_data->bus;
> >
> >  	fme_get_board_interface(fme);
> >
> > -	fme->max10_dev = max10;
> >  	mgr->sensor_list = &max10->opae_sensor_list;
> >
> >  	/* SPI self test */
> > @@ -1178,6 +1205,25 @@ static int i2c_mac_rom_test(struct
> > altera_i2c_dev
> > *dev)
> >  	return 0;
> >  }
> >
> > +static void init_i2c_mutex(struct ifpga_fme_hw *fme) {
> > +	struct ifpga_hw *hw = (struct ifpga_hw *)fme->parent;
> > +	struct altera_i2c_dev *i2c_dev;
> > +	opae_share_data *sd = NULL;
> > +
> > +	if (fme->i2c_master) {
> > +		i2c_dev = (struct altera_i2c_dev *)fme->i2c_master;
> > +		if (hw && hw->adapter && hw->adapter->shm.ptr) {
> > +			dev_info(NULL, "use multi-process mutex in i2c\n");
> > +			sd = (opae_share_data *)hw->adapter->shm.ptr;
> > +			i2c_dev->mutex = &sd->i2c_mutex;
> > +		} else {
> > +			dev_info(NULL, "use multi-thread mutex in i2c\n");
> > +			i2c_dev->mutex = &i2c_dev->lock;
> > +		}
> > +	}
> > +}
> > +
> >  static int fme_i2c_init(struct ifpga_feature *feature)  {
> >  	struct feature_fme_i2c *i2c;
> > @@ -1191,6 +1237,8 @@ static int fme_i2c_init(struct ifpga_feature
> *feature)
> >  	if (!fme->i2c_master)
> >  		return -ENODEV;
> >
> > +	init_i2c_mutex(fme);
> > +
> >  	/* MAC ROM self test */
> >  	i2c_mac_rom_test(fme->i2c_master);
> >
> > diff --git a/drivers/raw/ifpga/base/meson.build
> > b/drivers/raw/ifpga/base/meson.build
> > index b13e13e89..da2d6e33c 100644
> > --- a/drivers/raw/ifpga/base/meson.build
> > +++ b/drivers/raw/ifpga/base/meson.build
> > @@ -23,6 +23,18 @@ sources = [
> >  	'opae_eth_group.c',
> >  ]
> >
> > +rtdep = dependency('librt', required: false) if not rtdep.found()
> > +	rtdep = cc.find_library('librt', required: false) endif if not
> > +rtdep.found()
> > +	build = false
> > +	reason = 'missing dependency, "librt"'
> > +	subdir_done()
> > +endif
> > +
> > +ext_deps += rtdep
> > +
> >  base_lib = static_library('ifpga_rawdev_base', sources,
> >  	dependencies: static_rte_eal,
> >  	c_args: cflags)
> > diff --git a/drivers/raw/ifpga/base/opae_hw_api.c
> > b/drivers/raw/ifpga/base/opae_hw_api.c
> > index c969dfed3..600afdea1 100644
> > --- a/drivers/raw/ifpga/base/opae_hw_api.c
> > +++ b/drivers/raw/ifpga/base/opae_hw_api.c
> > @@ -2,6 +2,10 @@
> >   * Copyright(c) 2010-2018 Intel Corporation
> >   */
> >
> > +#include <sys/mman.h>
> > +#include <sys/stat.h>
> > +#include <fcntl.h>
> > +#include <unistd.h>
> >  #include "opae_hw_api.h"
> >  #include "opae_debug.h"
> >  #include "ifpga_api.h"
> > @@ -305,6 +309,244 @@ static struct opae_adapter_ops *match_ops(struct
> > opae_adapter *adapter)
> >  	return NULL;
> >  }
> >
> > +static void opae_mutex_init(pthread_mutex_t *mutex) {
> > +	pthread_mutexattr_t mattr;
> > +
> > +	pthread_mutexattr_init(&mattr);
> > +	pthread_mutexattr_settype(&mattr, PTHREAD_MUTEX_RECURSIVE);
> > +	pthread_mutexattr_setpshared(&mattr,
> > PTHREAD_PROCESS_SHARED);
> > +	pthread_mutexattr_setrobust(&mattr, PTHREAD_MUTEX_ROBUST);
> > +	pthread_mutexattr_setprotocol(&mattr, PTHREAD_PRIO_INHERIT);
> > +	pthread_mutex_init(mutex, &mattr);
> > +	pthread_mutexattr_destroy(&mattr);
> > +}
> > +
> > +static int opae_shm_open(char *shm_name, u32 size, int *new_shm) {
> > +	int shm_id;
> > +	int ret;
> > +
> > +	shm_id = shm_open(shm_name, O_CREAT | O_EXCL | O_RDWR,
> > 0666);
> > +	if (shm_id == -1) {
> > +		if (errno == EEXIST) {
> > +			dev_info(NULL, "shared memory %s already exist\n",
> > +					shm_name);
> > +			shm_id = shm_open(shm_name, O_RDWR, 0666);
> > +		} else {
> > +			dev_err(NULL, "failed to create shared
> > memory %s\n",
> > +					shm_name);
> > +			return -1;
> > +		}
> > +	} else {
> > +		*new_shm = 1;
> > +		ret = ftruncate(shm_id, size);
> > +		if (ret == -1) {
> > +			dev_err(NULL,
> > +					"failed to set shared memory size
> > to %u\n",
> > +					size);
> > +			ret = shm_unlink(shm_name);
> > +			if (ret == -1) {
> > +				dev_err(NULL,
> > +						"failed to unlink shared
> > memory %s\n",
> > +						shm_name);
> > +			}
> > +			return -1;
> > +		}
> > +	}
> > +
> > +	return shm_id;
> > +}
> > +
> > +static pthread_mutex_t *opae_adapter_mutex_open(struct opae_adapter
> > +*adapter) {
> > +	char shm_name[32];
> > +	void *ptr;
> > +	int shm_id;
> > +	int new_shm = 0;
> > +
> > +	if (!adapter->data)
> > +		return NULL;
> > +	adapter->lock = NULL;
> > +
> > +	snprintf(shm_name, sizeof(shm_name), "/mutex.IFPGA:%s",
> > adapter->name);
> > +	shm_id = opae_shm_open(shm_name, sizeof(pthread_mutex_t),
> > &new_shm);
> > +	if (shm_id == -1) {
> > +		dev_err(NULL, "failed to open shared memory %s\n",
> > shm_name);
> > +	} else {
> > +		dev_info(NULL, "shared memory %s id is %d\n",
> > +				shm_name, shm_id);
> > +		ptr = mmap(NULL, sizeof(pthread_mutex_t),
> > +				PROT_READ | PROT_WRITE, MAP_SHARED,
> > +				shm_id, 0);
> > +		adapter->lock = (pthread_mutex_t *)ptr;
> > +		if (ptr) {
> > +			dev_info(NULL,
> > +					"shared memory %s address is %p\n",
> > +					shm_name, ptr);
> > +			if (new_shm)
> > +				opae_mutex_init(adapter->lock);
> > +		} else {
> > +			dev_err(NULL, "failed to map shared memory %s\n",
> > +					shm_name);
> > +		}
> > +	}
> > +
> > +	return adapter->lock;
> > +}
> > +
> > +static void opae_adapter_mutex_close(struct opae_adapter *adapter) {
> > +	char shm_name[32];
> > +	int ret;
> > +
> > +	if (!adapter->lock)
> > +		return;
> > +
> > +	snprintf(shm_name, sizeof(shm_name), "/mutex.IFPGA:%s",
> > +adapter->name);
> > +
> > +	ret = munmap(adapter->lock, sizeof(pthread_mutex_t));
> > +	if (ret == -1)
> > +		dev_err(NULL, "failed to unmap shared memory %s\n",
> > shm_name);
> > +	else
> > +		adapter->lock = NULL;
> > +}
> > +
> > +/**
> > + * opae_adapter_lock - lock this adapter
> > + * @adapter: adapter to lock.
> > + * @timeout: maximum time to wait for lock done
> > + *           -1  wait until the lock is available
> > + *           0   do not wait and return immediately
> > + *           t   positive time in second to wait
> > + *
> > + * Return: 0 on success, otherwise error code.
> > + */
> > +int opae_adapter_lock(struct opae_adapter *adapter, int timeout) {
> > +	struct timespec t;
> > +	int ret = -EINVAL;
> > +
> > +	if (adapter && adapter->lock) {
> > +		if (timeout < 0) {
> > +			ret = pthread_mutex_lock(adapter->lock);
> > +		} else if (timeout == 0) {
> > +			ret = pthread_mutex_trylock(adapter->lock);
> > +		} else {
> > +			clock_gettime(CLOCK_REALTIME, &t);
> > +			t.tv_sec += timeout;
> > +			ret = pthread_mutex_timedlock(adapter->lock, &t);
> > +		}
> > +	}
> > +	return ret;
> > +}
> > +
> > +/**
> > + * opae_adapter_unlock - unlock this adapter
> > + * @adapter: adapter to unlock.
> > + *
> > + * Return: 0 on success, otherwise error code.
> > + */
> > +int opae_adapter_unlock(struct opae_adapter *adapter) {
> > +	int ret = -EINVAL;
> > +
> > +	if (adapter && adapter->lock)
> > +		ret = pthread_mutex_unlock(adapter->lock);
> > +
> > +	return ret;
> > +}
> > +
> > +static void opae_adapter_shm_init(struct opae_adapter *adapter) {
> > +	opae_share_data *sd;
> > +
> > +	if (!adapter->shm.ptr)
> > +		return;
> > +
> > +	sd = (opae_share_data *)adapter->shm.ptr;
> > +	dev_info(NULL, "initialize shared memory\n");
> > +	opae_mutex_init(&sd->spi_mutex);
> > +	opae_mutex_init(&sd->i2c_mutex);
> > +	sd->ref_cnt = 0;
> > +	sd->dtb_size = SHM_BLK_SIZE;
> > +}
> > +
> > +static void *opae_adapter_shm_alloc(struct opae_adapter *adapter) {
> > +	char shm_name[32];
> > +	opae_share_data *sd;
> > +	u32 size = sizeof(opae_share_data);
> > +	int shm_id;
> > +	int new_shm = 0;
> > +
> > +	if (!adapter->data)
> > +		return NULL;
> > +
> > +	snprintf(shm_name, sizeof(shm_name), "/IFPGA:%s", adapter-
> > >name);
> > +	adapter->shm.ptr = NULL;
> > +
> > +	opae_adapter_lock(adapter, -1);
> > +	shm_id = opae_shm_open(shm_name, size, &new_shm);
> > +	if (shm_id == -1) {
> > +		dev_err(NULL, "failed to open shared memory %s\n",
> > shm_name);
> > +	} else {
> > +		dev_info(NULL, "shared memory %s id is %d\n",
> > +				shm_name, shm_id);
> > +		adapter->shm.id = shm_id;
> > +		adapter->shm.size = size;
> > +		adapter->shm.ptr = mmap(NULL, size, PROT_READ |
> > PROT_WRITE,
> > +							MAP_SHARED,
> > shm_id, 0);
> > +		if (adapter->shm.ptr) {
> > +			dev_info(NULL,
> > +					"shared memory %s address is %p\n",
> > +					shm_name, adapter->shm.ptr);
> > +			if (new_shm)
> > +				opae_adapter_shm_init(adapter);
> > +			sd = (opae_share_data *)adapter->shm.ptr;
> > +			sd->ref_cnt++;
> > +		} else {
> > +			dev_err(NULL, "failed to map shared memory %s\n",
> > +					shm_name);
> > +		}
> > +	}
> > +	opae_adapter_unlock(adapter);
> > +
> > +	return adapter->shm.ptr;
> > +}
> > +
> > +static void opae_adapter_shm_free(struct opae_adapter *adapter) {
> > +	char shm_name[32];
> > +	opae_share_data *sd;
> > +	u32 ref_cnt;
> > +	int ret;
> > +
> > +	if (!adapter->shm.ptr)
> > +		return;
> > +
> > +	sd = (opae_share_data *)adapter->shm.ptr;
> > +	snprintf(shm_name, sizeof(shm_name), "/IFPGA:%s", adapter-
> > >name);
> > +
> > +	opae_adapter_lock(adapter, -1);
> > +	ref_cnt = --sd->ref_cnt;
> > +	ret = munmap(adapter->shm.ptr, adapter->shm.size);
> > +	if (ret == -1)
> > +		dev_err(NULL, "failed to unmap shared memory %s\n",
> > shm_name);
> > +	else
> > +		adapter->shm.ptr = NULL;
> > +
> > +	if (ref_cnt == 0) {
> > +		dev_info(NULL, "unlink shared memory %s\n", shm_name);
> > +		ret = shm_unlink(shm_name);
> > +		if (ret == -1) {
> > +			dev_err(NULL, "failed to unlink shared
> > memory %s\n",
> > +					shm_name);
> > +		}
> > +	}
> > +	opae_adapter_unlock(adapter);
> > +}
> > +
> >  /**
> >   * opae_adapter_init - init opae_adapter data structure
> >   * @adapter: pointer of opae_adapter data structure @@ -324,6 +566,12
> > @@ int opae_adapter_init(struct opae_adapter *adapter,
> >  	adapter->name = name;
> >  	adapter->ops = match_ops(adapter);
> >
> > +	if (!opae_adapter_mutex_open(adapter))
> > +		return -ENOMEM;
> > +
> > +	if (!opae_adapter_shm_alloc(adapter))
> > +		return -ENOMEM;
> > +
> >  	return 0;
> >  }
> >
> > @@ -359,6 +607,8 @@ void opae_adapter_destroy(struct opae_adapter
> > *adapter)  {
> >  	if (adapter && adapter->ops && adapter->ops->destroy)
> >  		adapter->ops->destroy(adapter);
> > +	opae_adapter_shm_free(adapter);
> > +	opae_adapter_mutex_close(adapter);
> >  }
> >
> >  /**
> > diff --git a/drivers/raw/ifpga/base/opae_hw_api.h
> > b/drivers/raw/ifpga/base/opae_hw_api.h
> > index cf8ff93a6..e99ee4564 100644
> > --- a/drivers/raw/ifpga/base/opae_hw_api.h
> > +++ b/drivers/raw/ifpga/base/opae_hw_api.h
> > @@ -265,12 +265,36 @@ TAILQ_HEAD(opae_accelerator_list,
> > opae_accelerator);  #define opae_adapter_for_each_acc(adatper, acc) \
> >  	TAILQ_FOREACH(acc, &adapter->acc_list, node)
> >
> > +#define SHM_PREFIX     "/IFPGA:"
> > +#define SHM_BLK_SIZE   0x2000
> > +
> > +typedef struct {
> > +	union {
> > +		u8 byte[SHM_BLK_SIZE];
> > +		struct {
> > +			pthread_mutex_t spi_mutex;
> > +			pthread_mutex_t i2c_mutex;
> > +			u32 ref_cnt;    /* reference count of shared memory
> > */
> > +			u32 dtb_size;   /* actual length of DTB data in byte */
> > +		};
> > +	};
> > +	u8 dtb[SHM_BLK_SIZE];   /* DTB data */
> > +} opae_share_data;
> > +
> > +typedef struct  {
> > +	int id;       /* shared memory id returned by shm_open */
> > +	u32 size;     /* size of shared memory in byte */
> > +	void *ptr;    /* start address of shared memory */
> > +} opae_share_memory;
> > +
> >  struct opae_adapter {
> >  	const char *name;
> >  	struct opae_manager *mgr;
> >  	struct opae_accelerator_list acc_list;
> >  	struct opae_adapter_ops *ops;
> >  	void *data;
> > +	pthread_mutex_t *lock;   /* multi-process mutex for IFPGA */
> > +	opae_share_memory shm;
> >  };
> >
> >  /* OPAE Adapter APIs */
> > @@ -280,7 +304,8 @@ void *opae_adapter_data_alloc(enum
> > opae_adapter_type type);  int opae_adapter_init(struct opae_adapter
> > *adapter,
> >  		const char *name, void *data);
> >  #define opae_adapter_free(adapter) opae_free(adapter)
> > -
> > +int opae_adapter_lock(struct opae_adapter *adapter, int timeout); int
> > +opae_adapter_unlock(struct opae_adapter *adapter);
> >  int opae_adapter_enumerate(struct opae_adapter *adapter);  void
> > opae_adapter_destroy(struct opae_adapter *adapter);  static inline
> > struct opae_manager * diff --git a/drivers/raw/ifpga/base/opae_i2c.c
> > b/drivers/raw/ifpga/base/opae_i2c.c
> > index 846d751f5..598eab574 100644
> > --- a/drivers/raw/ifpga/base/opae_i2c.c
> > +++ b/drivers/raw/ifpga/base/opae_i2c.c
> > @@ -30,7 +30,7 @@ int i2c_read(struct altera_i2c_dev *dev, int flags,
> > unsigned int slave_addr,
> >  	int i = 0;
> >  	int ret;
> >
> > -	pthread_mutex_lock(&dev->lock);
> > +	pthread_mutex_lock(dev->mutex);
> >
> >  	if (flags & I2C_FLAG_ADDR16)
> >  		msgbuf[i++] = offset >> 8;
> > @@ -60,7 +60,7 @@ int i2c_read(struct altera_i2c_dev *dev, int flags,
> > unsigned int slave_addr,
> >  	ret = i2c_transfer(dev, msg, 2);
> >
> >  exit:
> > -	pthread_mutex_unlock(&dev->lock);
> > +	pthread_mutex_unlock(dev->mutex);
> >  	return ret;
> >  }
> >
> > @@ -72,7 +72,7 @@ int i2c_write(struct altera_i2c_dev *dev, int flags,
> > unsigned int slave_addr,
> >  	int ret;
> >  	int i = 0;
> >
> > -	pthread_mutex_lock(&dev->lock);
> > +	pthread_mutex_lock(dev->mutex);
> >
> >  	if (!dev->xfer) {
> >  		ret = -ENODEV;
> > @@ -100,7 +100,7 @@ int i2c_write(struct altera_i2c_dev *dev, int
> > flags, unsigned int slave_addr,
> >
> >  	opae_free(buf);
> >  exit:
> > -	pthread_mutex_unlock(&dev->lock);
> > +	pthread_mutex_unlock(dev->mutex);
> >  	return ret;
> >  }
> >
> > @@ -496,6 +496,7 @@ struct altera_i2c_dev *altera_i2c_probe(void
> > *base)
> >
> >  	if (pthread_mutex_init(&dev->lock, NULL))
> >  		return NULL;
> > +	dev->mutex = &dev->lock;
> >
> >  	altera_i2c_hardware_init(dev);
> >
> > diff --git a/drivers/raw/ifpga/base/opae_i2c.h
> > b/drivers/raw/ifpga/base/opae_i2c.h
> > index 266e127b7..4f6b0b28b 100644
> > --- a/drivers/raw/ifpga/base/opae_i2c.h
> > +++ b/drivers/raw/ifpga/base/opae_i2c.h
> > @@ -94,6 +94,7 @@ struct altera_i2c_dev {
> >  	u8 *buf;
> >  	int (*xfer)(struct altera_i2c_dev *dev, struct i2c_msg *msg, int num);
> >  	pthread_mutex_t lock;
> > +	pthread_mutex_t *mutex;  /* multi-process mutex from adapter */
> >  };
> >
> >  /**
> > diff --git a/drivers/raw/ifpga/base/opae_intel_max10.c
> > b/drivers/raw/ifpga/base/opae_intel_max10.c
> > index 8e23ca18a..1a526ea54 100644
> > --- a/drivers/raw/ifpga/base/opae_intel_max10.c
> > +++ b/drivers/raw/ifpga/base/opae_intel_max10.c
> > @@ -138,84 +138,116 @@ static int enable_nor_flash(struct
> > intel_max10_device *dev, bool on)
> >
> >  static int init_max10_device_table(struct intel_max10_device *max10)
> > {
> > +	struct altera_spi_device *spi = NULL;
> >  	struct max10_compatible_id *id;
> >  	struct fdt_header hdr;
> >  	char *fdt_root = NULL;
> > -
> > +	u32 dtb_magic = 0;
> >  	u32 dt_size, dt_addr, val;
> > -	int ret;
> > -
> > -	ret = max10_sys_read(max10, DT_AVAIL_REG, &val);
> > -	if (ret) {
> > -		dev_err(max10 "cannot read DT_AVAIL_REG\n");
> > -		return ret;
> > -	}
> > +	int ret = 0;
> >
> > -	if (!(val & DT_AVAIL)) {
> > -		dev_err(max10 "DT not available\n");
> > +	spi = (struct altera_spi_device *)max10->spi_master;
> > +	if (!spi) {
> > +		dev_err(max10, "spi master is not set\n");
> >  		return -EINVAL;
> >  	}
> > +	if (spi->dtb)
> > +		dtb_magic = *(u32 *)spi->dtb;
> > +
> > +	if (dtb_magic != 0xEDFE0DD0) {
> > +		dev_info(max10, "read DTB from NOR flash\n");
> > +		ret = max10_sys_read(max10, DT_AVAIL_REG, &val);
> > +		if (ret) {
> > +			dev_err(max10 "cannot read DT_AVAIL_REG\n");
> > +			return ret;
> > +		}
> >
> > -	ret = max10_sys_read(max10, DT_BASE_ADDR_REG, &dt_addr);
> > -	if (ret) {
> > -		dev_info(max10 "cannot get base addr of device table\n");
> > -		return ret;
> > -	}
> > -
> > -	ret = enable_nor_flash(max10, true);
> > -	if (ret) {
> > -		dev_err(max10 "fail to enable flash\n");
> > -		return ret;
> > -	}
> > +		if (!(val & DT_AVAIL)) {
> > +			dev_err(max10 "DT not available\n");
> > +			return -EINVAL;
> > +		}
> >
> > -	ret = altera_nor_flash_read(max10, dt_addr, &hdr, sizeof(hdr));
> > -	if (ret) {
> > -		dev_err(max10 "read fdt header fail\n");
> > -		goto done;
> > -	}
> > +		ret = max10_sys_read(max10, DT_BASE_ADDR_REG,
> > &dt_addr);
> > +		if (ret) {
> > +			dev_info(max10 "cannot get base addr of device
> > table\n");
> > +			return ret;
> > +		}
> >
> > -	ret = fdt_check_header(&hdr);
> > -	if (ret) {
> > -		dev_err(max10 "check fdt header fail\n");
> > -		goto done;
> > -	}
> > +		ret = enable_nor_flash(max10, true);
> > +		if (ret) {
> > +			dev_err(max10 "fail to enable flash\n");
> > +			return ret;
> > +		}
> >
> > -	dt_size = fdt_totalsize(&hdr);
> > -	if (dt_size > DFT_MAX_SIZE) {
> > -		dev_err(max10 "invalid device table size\n");
> > -		ret = -EINVAL;
> > -		goto done;
> > -	}
> > +		ret = altera_nor_flash_read(max10, dt_addr, &hdr,
> > sizeof(hdr));
> > +		if (ret) {
> > +			dev_err(max10 "read fdt header fail\n");
> > +			goto disable_nor_flash;
> > +		}
> >
> > -	fdt_root = opae_malloc(dt_size);
> > -	if (!fdt_root) {
> > -		ret = -ENOMEM;
> > -		goto done;
> > -	}
> > +		ret = fdt_check_header(&hdr);
> > +		if (ret) {
> > +			dev_err(max10 "check fdt header fail\n");
> > +			goto disable_nor_flash;
> > +		}
> >
> > -	ret = altera_nor_flash_read(max10, dt_addr, fdt_root, dt_size);
> > -	if (ret) {
> > -		dev_err(max10 "cannot read device table\n");
> > -		goto done;
> > -	}
> > +		dt_size = fdt_totalsize(&hdr);
> > +		if (dt_size > DFT_MAX_SIZE) {
> > +			dev_err(max10 "invalid device table size\n");
> > +			ret = -EINVAL;
> > +			goto disable_nor_flash;
> > +		}
> >
> > -	id = max10_match_compatible(fdt_root);
> > -	if (!id) {
> > -		dev_err(max10 "max10 compatible not found\n");
> > -		ret = -ENODEV;
> > -		goto done;
> > -	}
> > +		fdt_root = opae_malloc(dt_size);
> > +		if (!fdt_root) {
> > +			ret = -ENOMEM;
> > +			goto disable_nor_flash;
> > +		}
> >
> > -	max10->flags |= MAX10_FLAGS_DEVICE_TABLE;
> > +		ret = altera_nor_flash_read(max10, dt_addr, fdt_root,
> > dt_size);
> > +		if (ret) {
> > +			opae_free(fdt_root);
> > +			fdt_root = NULL;
> > +			dev_err(max10 "cannot read device table\n");
> > +			goto disable_nor_flash;
> > +		}
> >
> > -	max10->id = id;
> > -	max10->fdt_root = fdt_root;
> > +		if (spi->dtb) {
> > +			if (*spi->dtb_sz_ptr < dt_size) {
> > +				dev_warn(max10,
> > +						 "share memory for dtb is
> > smaller than required %u\n",
> > +						 dt_size);
> > +			} else {
> > +				*spi->dtb_sz_ptr = dt_size;
> > +			}
> > +			/* store dtb data into share memory  */
> > +			memcpy(spi->dtb, fdt_root, *spi->dtb_sz_ptr);
> > +		}
> >
> > -done:
> > -	ret = enable_nor_flash(max10, false);
> > +disable_nor_flash:
> > +		enable_nor_flash(max10, false);
> > +	} else {
> > +		if (*spi->dtb_sz_ptr > 0) {
> > +			dev_info(max10, "read DTB from shared memory\n");
> > +			fdt_root = opae_malloc(*spi->dtb_sz_ptr);
> > +			if (fdt_root)
> > +				memcpy(fdt_root, spi->dtb, *spi-
> > >dtb_sz_ptr);
> > +			else
> > +				ret = -ENOMEM;
> > +		}
> > +	}
> >
> > -	if (ret && fdt_root)
> > -		opae_free(fdt_root);
> > +	if (fdt_root) {
> > +		id = max10_match_compatible(fdt_root);
> > +		if (!id) {
> > +			dev_err(max10 "max10 compatible not found\n");
> > +			ret = -ENODEV;
> > +		} else {
> > +			max10->flags |= MAX10_FLAGS_DEVICE_TABLE;
> > +			max10->id = id;
> > +			max10->fdt_root = fdt_root;
> > +		}
> > +	}
> >
> >  	return ret;
> >  }
> > diff --git a/drivers/raw/ifpga/base/opae_spi.c
> > b/drivers/raw/ifpga/base/opae_spi.c
> > index bfdc83e6c..9efeecb79 100644
> > --- a/drivers/raw/ifpga/base/opae_spi.c
> > +++ b/drivers/raw/ifpga/base/opae_spi.c
> > @@ -285,11 +285,15 @@ void altera_spi_init(struct altera_spi_device
> > *spi_dev)
> >  			spi_dev->num_chipselect,
> >  			spi_dev->spi_param.clock_phase);
> >
> > +	if (spi_dev->mutex)
> > +		pthread_mutex_lock(spi_dev->mutex);
> >  	/* clear */
> >  	spi_reg_write(spi_dev, ALTERA_SPI_CONTROL, 0);
> >  	spi_reg_write(spi_dev, ALTERA_SPI_STATUS, 0);
> >  	/* flush rxdata */
> >  	spi_flush_rx(spi_dev);
> > +	if (spi_dev->mutex)
> > +		pthread_mutex_unlock(spi_dev->mutex);
> >  }
> >
> >  void altera_spi_release(struct altera_spi_device *dev) diff --git
> > a/drivers/raw/ifpga/base/opae_spi.h
> > b/drivers/raw/ifpga/base/opae_spi.h
> > index 73a227673..af11656e4 100644
> > --- a/drivers/raw/ifpga/base/opae_spi.h
> > +++ b/drivers/raw/ifpga/base/opae_spi.h
> > @@ -77,6 +77,10 @@ struct altera_spi_device {
> >  	int (*reg_read)(struct altera_spi_device *dev, u32 reg, u32 *val);
> >  	int (*reg_write)(struct altera_spi_device *dev, u32 reg,
> >  			u32 value);
> > +	/* below are data to be shared in multiple process */
> > +	pthread_mutex_t *mutex;     /* to be passed to spi_transaction_dev
> > */
> > +	unsigned int *dtb_sz_ptr;   /* to be used in init_max10_device_table
> > */
> > +	unsigned char *dtb;         /* to be used in init_max10_device_table */
> >  };
> >
> >  #define HEADER_LEN 8
> > @@ -103,6 +107,7 @@ struct spi_transaction_dev {
> >  	int chipselect;
> >  	struct spi_tran_buffer *buffer;
> >  	pthread_mutex_t lock;
> > +	pthread_mutex_t *mutex;  /* multi-process mutex from adapter */
> >  };
> >
> >  struct spi_tran_header {
> > diff --git a/drivers/raw/ifpga/base/opae_spi_transaction.c
> > b/drivers/raw/ifpga/base/opae_spi_transaction.c
> > index d13d2fbc8..006cdb4c1 100644
> > --- a/drivers/raw/ifpga/base/opae_spi_transaction.c
> > +++ b/drivers/raw/ifpga/base/opae_spi_transaction.c
> > @@ -434,11 +434,11 @@ int spi_transaction_read(struct
> > spi_transaction_dev *dev, unsigned int addr,  {
> >  	int ret;
> >
> > -	pthread_mutex_lock(&dev->lock);
> > +	pthread_mutex_lock(dev->mutex);
> >  	ret = do_transaction(dev, addr, size, data,
> >  			(size > SPI_REG_BYTES) ?
> >  			SPI_TRAN_SEQ_READ : SPI_TRAN_NON_SEQ_READ);
> > -	pthread_mutex_unlock(&dev->lock);
> > +	pthread_mutex_unlock(dev->mutex);
> >
> >  	return ret;
> >  }
> > @@ -448,11 +448,11 @@ int spi_transaction_write(struct
> > spi_transaction_dev *dev, unsigned int addr,  {
> >  	int ret;
> >
> > -	pthread_mutex_lock(&dev->lock);
> > +	pthread_mutex_lock(dev->mutex);
> >  	ret = do_transaction(dev, addr, size, data,
> >  			(size > SPI_REG_BYTES) ?
> >  			SPI_TRAN_SEQ_WRITE : SPI_TRAN_NON_SEQ_WRITE);
> > -	pthread_mutex_unlock(&dev->lock);
> > +	pthread_mutex_unlock(dev->mutex);
> >
> >  	return ret;
> >  }
> > @@ -479,6 +479,13 @@ struct spi_transaction_dev
> > *spi_transaction_init(struct altera_spi_device *dev,
> >  		dev_err(spi_tran_dev, "fail to init mutex lock\n");
> >  		goto err;
> >  	}
> > +	if (dev->mutex) {
> > +		dev_info(NULL, "use multi-process mutex in spi\n");
> > +		spi_tran_dev->mutex = dev->mutex;
> > +	} else {
> > +		dev_info(NULL, "use multi-thread mutex in spi\n");
> > +		spi_tran_dev->mutex = &spi_tran_dev->lock;
> > +	}
> >
> >  	return spi_tran_dev;
> >
> > --
> > 2.17.1
> 
> Acked-by: Rosen Xu <rosen.xu@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

  reply	other threads:[~2020-10-15 13:16 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-23  7:30 [dpdk-dev] [PATCH v4 0/4] raw/ifpga/base: An inprovement for multi-process Tianfei zhang
2020-09-23  7:30 ` [dpdk-dev] [PATCH v4 1/4] raw/ifpga/base: fix bug in IRQ functions Tianfei zhang
2020-09-23  7:30 ` [dpdk-dev] [PATCH v4 2/4] raw/ifpga/base: free resources when destroying ifpga device Tianfei zhang
2020-09-23  7:30 ` [dpdk-dev] [PATCH v4 3/4] raw/ifpga/base: cleanup ifpga raw devices when process quit Tianfei zhang
2020-09-23  7:30 ` [dpdk-dev] [PATCH v4 4/4] raw/ifpga/base: enhance driver reliablity in multi-process Tianfei zhang
2020-09-28  1:40 ` [dpdk-dev] [PATCH v2 0/4] raw/ifpga/base: An improvement for multi-process Tianfei zhang
2020-09-28  1:40   ` [dpdk-dev] [PATCH v2 1/4] raw/ifpga/base: fix bug in IRQ functions Tianfei zhang
2020-09-29  1:42     ` Xu, Rosen
2020-10-14  9:59       ` Zhang, Tianfei
2020-10-15 13:14       ` Zhang, Qi Z
2020-10-15 18:56     ` Ferruh Yigit
2020-10-16  5:46       ` Zhang, Tianfei
2020-09-28  1:40   ` [dpdk-dev] [PATCH v2 2/4] raw/ifpga/base: free resources when destroying ifpga device Tianfei zhang
2020-09-29  1:42     ` Xu, Rosen
2020-10-15 13:15       ` Zhang, Qi Z
2020-10-15 18:57     ` Ferruh Yigit
2020-10-16  5:51       ` Zhang, Tianfei
2020-09-28  1:40   ` [dpdk-dev] [PATCH v2 3/4] raw/ifpga/base: cleanup ifpga raw devices when process quit Tianfei zhang
2020-09-29  1:43     ` Xu, Rosen
2020-10-15 13:15       ` Zhang, Qi Z
2020-10-15 18:57     ` Ferruh Yigit
2020-10-16  5:54       ` Zhang, Tianfei
2020-09-28  1:40   ` [dpdk-dev] [PATCH v2 4/4] raw/ifpga/base: enhance driver reliability in multi-process Tianfei zhang
2020-10-15  6:08     ` Xu, Rosen
2020-10-15 13:16       ` Zhang, Qi Z [this message]
2020-10-23  8:59 ` [dpdk-dev] [PATCH v3 0/5] raw/ifpga/base: An improvement for multi-process Tianfei zhang
2020-10-23  8:59   ` [dpdk-dev] [PATCH v3 1/5] raw/ifpga/base: fix interrupt handler instance usage Tianfei zhang
2020-10-23  8:59   ` [dpdk-dev] [PATCH v3 2/5] raw/ifpga/base: handle unsupported interrupt type Tianfei zhang
2020-10-23  8:59   ` [dpdk-dev] [PATCH v3 3/5] raw/ifpga/base: fix return of IRQ unregister properly Tianfei zhang
2020-10-23  8:59   ` [dpdk-dev] [PATCH v3 4/5] raw/ifpga/base: free resources when destroying ifpga device Tianfei zhang
2020-10-23  8:59   ` [dpdk-dev] [PATCH v3 5/5] raw/ifpga/base: enhance driver reliablity in multi-process Tianfei zhang
2020-10-26  1:04   ` [dpdk-dev] [PATCH v3 0/5] raw/ifpga/base: An improvement for multi-process Zhang, Qi Z
2020-10-23  9:06 ` Tianfei zhang

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=02462e0e2e1c4a20974b110b04105513@intel.com \
    --to=qi.z.zhang@intel.com \
    --cc=dev@dpdk.org \
    --cc=rosen.xu@intel.com \
    --cc=tianfei.zhang@intel.com \
    --cc=wei.huang@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git