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 [thread overview]
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
next prev parent 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
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).