DPDK patches and discussions
 help / color / mirror / Atom feed
From: "Xu, Rosen" <rosen.xu@intel.com>
To: "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 06:08:29 +0000
Message-ID: <BYAPR11MB29019B1BDF03000D851475A389020@BYAPR11MB2901.namprd11.prod.outlook.com> (raw)
In-Reply-To: <1601257218-6606-5-git-send-email-tianfei.zhang@intel.com>

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>

  reply	other threads:[~2020-10-15  6:08 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 [this message]
2020-10-15 13:16       ` Zhang, Qi Z
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=BYAPR11MB29019B1BDF03000D851475A389020@BYAPR11MB2901.namprd11.prod.outlook.com \
    --to=rosen.xu@intel.com \
    --cc=dev@dpdk.org \
    --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