From f923add0a9b6bd17d43f72c21eec4e01e19240cf Mon Sep 17 00:00:00 2001 From: zilio nicolas Date: Tue, 7 Jul 2015 11:12:52 +0200 Subject: clean version for locks --- dma/ipe.h | 1 + pcilib/CMakeLists.txt | 4 +- pcilib/kmem.h | 1 + pcilib/lock.c | 116 ++++++++++++++++++++++++++++++++++++++++++++++++++ pcilib/lock.h | 55 ++++++++++++++++++++++++ pcilib/locking.c | 74 ++++++++++++++++++++++++++++++++ pcilib/locking.h | 44 +++++++++++++++++++ pcilib/pci.c | 8 ++++ pcilib/pci.h | 2 +- pcitool/cli.c | 17 ++++++-- protocols/software.c | 41 +++++++++++++----- 11 files changed, 346 insertions(+), 17 deletions(-) create mode 100644 pcilib/lock.c create mode 100644 pcilib/lock.h create mode 100644 pcilib/locking.c create mode 100644 pcilib/locking.h diff --git a/dma/ipe.h b/dma/ipe.h index 5640606..021e5a4 100644 --- a/dma/ipe.h +++ b/dma/ipe.h @@ -79,6 +79,7 @@ static const pcilib_register_description_t ipe_dma_registers[] = { {0x005C, 0, 32, 0, 0x00000000, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "desc_mem_addr", "Number of descriptors configured"}, {0x0060, 0, 32, 0, 0x00000000, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "update_thresh", "Update threshold of progress register"}, {0x0000, 0, 32, PCILIB_VERSION, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMACONF, "dma_version", "Version of DMA engine"}, + {0x005, 0, 32, PCILIB_VERSION, 0x00000040, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMACONF, "test", "testdesc"}, {0, 0, 0, 0, 0x00000000, 0, 0, 0, NULL, NULL} }; #endif /* _PCILIB_EXPORT_C */ diff --git a/pcilib/CMakeLists.txt b/pcilib/CMakeLists.txt index c9bf0fb..ebb5cec 100644 --- a/pcilib/CMakeLists.txt +++ b/pcilib/CMakeLists.txt @@ -3,8 +3,8 @@ include_directories( ${CMAKE_SOURCE_DIR}/pcilib ) -set(HEADERS pcilib.h pci.h export.h bar.h fifo.h model.h bank.h register.h kmem.h irq.h dma.h event.h plugin.h tools.h error.h debug.h env.h version.h config.h) -add_library(pcilib SHARED pci.c export.c bar.c fifo.c model.c bank.c register.c kmem.c irq.c dma.c event.c plugin.c tools.c error.c debug.c env.c) +set(HEADERS pcilib.h pci.h export.h bar.h fifo.h model.h bank.h register.h kmem.h irq.h dma.h event.h plugin.h tools.h error.h debug.h env.h version.h config.h locking.h lock.h) +add_library(pcilib SHARED pci.c export.c bar.c fifo.c model.c bank.c register.c kmem.c irq.c dma.c event.c plugin.c tools.c error.c debug.c env.c locking.c lock.c) target_link_libraries(pcilib dma protocols ${CMAKE_THREAD_LIBS_INIT} ${UFODECODE_LIBRARIES} ${CMAKE_DL_LIBS}) add_dependencies(pcilib dma protocols) diff --git a/pcilib/kmem.h b/pcilib/kmem.h index 8299379..65bdf04 100644 --- a/pcilib/kmem.h +++ b/pcilib/kmem.h @@ -31,6 +31,7 @@ typedef enum { PCILIB_KMEM_USE_DMA_RING = 1, PCILIB_KMEM_USE_DMA_PAGES = 2, PCILIB_KMEM_USE_SOFTWARE_REGISTERS = 3, + PCILIB_KMEM_USE_MUTEXES = 4, PCILIB_KMEM_USE_USER = 0x10 } pcilib_kmem_use_t; diff --git a/pcilib/lock.c b/pcilib/lock.c new file mode 100644 index 0000000..8715d21 --- /dev/null +++ b/pcilib/lock.c @@ -0,0 +1,116 @@ +#define _GNU_SOURCE +#define _XOPEN_SOURCE 600 + +#include +#include +#include "error.h" +#include "lock.h" +#include "pci.h" + +/* + * this function will take the lock for the semaphore pointed by semId + */ +void pcilib_lock(pcilib_lock_t *lock_ctx, pcilib_lock_flags_t flags, ...){ + int err; + struct timespec *time; + va_list pa; + va_start(pa,flags); + + if(flags & MUTEX_LOCK){ + err=pthread_mutex_lock(lock_ctx);/**< we try to lock here*/ + if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + /** if the lock haven't been acquired and errno==EOWNERDEAD, it means the previous application that got the lock crashed, we have to remake the lock "consistent" so*/ + else if(errno==EOWNERDEAD){ + pthread_mutex_consistent(lock_ctx); + pthread_mutex_lock(lock_ctx); + if(err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + } + } + else if(flags & MUTEX_TRYLOCK){ + err=pthread_mutex_trylock(lock_ctx);/**< we try to lock here*/ + if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + else if(errno==EOWNERDEAD){ + pthread_mutex_consistent(lock_ctx); + pthread_mutex_lock(lock_ctx); + if(err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + } + } + else if(flags & MUTEX_TIMEDLOCK){ + time=va_arg(pa,struct timespec*); + va_end(pa); + err=pthread_mutex_timedlock(lock_ctx, time);/**< we try to lock here*/ + if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + else if(errno==EOWNERDEAD){ + pthread_mutex_consistent(lock_ctx); + pthread_mutex_timedlock(lock_ctx, time); + if(err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)), errno); + } + } + else pcilib_error("wrong flag for pcilib_lock"); +} + +/** + * this function will unlock the semaphore pointed by lock_ctx. + */ +void pcilib_unlock(pcilib_lock_t* lock_ctx){ + int err; + if((err=pthread_mutex_unlock(lock_ctx))!=0) pcilib_error("can't unlock semaphore\n"); +} + +/** + * pcilib_init_lock + * this function initialize a new semaphore in the kernel if it's not already initialized given the key that permits to differentiate semaphores, and then return the integer that points to the semaphore that have been initialized or to a previously already initialized semaphore + * @param[out] lock_ctx the pointer that will points to the semaphore for other functions + * @param[in] keysem the integer that permits to define to what the semaphore is attached + */ +pcilib_lock_t* pcilib_init_lock(pcilib_t *ctx, char* lock_id, ...){ + int err; + pthread_mutexattr_t attr; + int i,j; + void* addr,*locks_addr; + va_list pa; + va_start(pa,lock_id); + + pcilib_lock_init_flags_t flag; + flag=va_arg(pa,pcilib_lock_init_flags_t); + va_end(pa); + + if(strlen(lock_id)>PCILIB_LOCK_SIZE-sizeof(pcilib_lock_t)) pcilib_error("the entered protocol name is too long"); + if(((PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE)%PCILIB_KMEM_PAGE_SIZE)!=0) pcilib_error("PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE should be a multiple of kmem page size"); + + addr=pcilib_kmem_get_block_ua(ctx,ctx->locks_handle,0); + if(flag & LOCK_INIT) pcilib_lock((pcilib_lock_t*)addr,MUTEX_LOCK); + /* we search for the given lock if it was already initialized*/ + for(j=0;jlocks_handle,j); + while((*(char*)(locks_addr+i*PCILIB_LOCK_SIZE+sizeof(pcilib_lock_t))!=0) && (i +#include +#include "lock.h" + +/* + * this function clean all locks created by the pcitool program + */ +void pcilib_clean_all_locks(pcilib_t* ctx){ +int i,j; +void* addr; + i=0; + while(ilocks_handle,i); +for(j=0;jlocks_handle,PCILIB_KMEM_FLAG_REUSE); +} + +/* + * this function allocates the kernel memory for the locks for software registers + */ +int pcilib_init_locking(pcilib_t* ctx, ...){ +/*for future possible more args + va_list pa; + va_start(pa,ctx);*/ + pcilib_kmem_handle_t *handle; + int err; + pcilib_kmem_reuse_state_t reused; + + + if((err=flock(ctx->handle,LOCK_EX))==-1) pcilib_warning("can't get flock on /dev/fpga0"); + handle=pcilib_alloc_kernel_memory(ctx,PCILIB_KMEM_TYPE_PAGE,PCILIB_NUMBER_OF_LOCK_PAGES,PCILIB_KMEM_PAGE_SIZE,0,PCILIB_KMEM_USE(PCILIB_KMEM_USE_MUTEXES,0),PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); + + if (!handle) { + pcilib_error("Allocation of kernel memory for mutexes has failed"); + return 1; + } + + ctx->locks_handle=handle; + reused = pcilib_kmem_is_reused(ctx, handle); +//#define DEBUG_REUSE +#ifdef DEBUG_REUSE +reused=0; +#endif + if ((reused & PCILIB_KMEM_REUSE_REUSED) == 0) { + pcilib_register_t i; + + if (reused & PCILIB_KMEM_REUSE_PARTIAL) { + pcilib_error("Inconsistent software registers are found (only part of required buffers is available)"); + pcilib_clean_all_locks(ctx); + return 1; + } + for(i=0;ihandle,LOCK_UN))==-1) pcilib_warning("could not remove lock correctly on /dev/fpga0"); + + return 0; +} diff --git a/pcilib/locking.h b/pcilib/locking.h new file mode 100644 index 0000000..f7570f6 --- /dev/null +++ b/pcilib/locking.h @@ -0,0 +1,44 @@ +/** + * @file lock_global.h + * @brief this file is the header file for functions that touch all locks allocated for software registers. + * @details for more details about implementation choice, please read the file lock.h + */ +#define _XOPEN_SOURCE 700 + +#ifndef _LOCKING_ +#define _LOCKING_ + +#include + +/** number of maximum locks*/ +#define PCILIB_MAX_NUMBER_LOCKS 64 + +/**size of one lock, determine so the size of the protocol_name in the way locks are registered. 40 bytes are necessary for the mutex structure, so we have a protocol name of length LOCK_SIZE-40*/ +#define PCILIB_LOCK_SIZE 128 + +#define PCILIB_LOCKS_PER_PAGE PCILIB_KMEM_PAGE_SIZE/PCILIB_LOCK_SIZE + +#define PCILIB_NUMBER_OF_LOCK_PAGES (PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE)/PCILIB_KMEM_PAGE_SIZE + + +/** +* new type to define a semaphore. It was made to differentiate from the library type. +*/ +typedef pthread_mutex_t pcilib_lock_t; + +/** + * this function destroy all locks created + *@param[in] ctx, the pcilib_t running + */ +void pcilib_clean_all_locks(pcilib_t* ctx); + +/** +* this function initialize the kmem pages containing locks +*@param[in] ctx the pcilib_t running +*/ +int pcilib_init_locking(pcilib_t* ctx, ...); + + +void pcilib_free_all_locks(pcilib_t* ctx); + +#endif /* _LOCK_GLOBAL_ */ diff --git a/pcilib/pci.c b/pcilib/pci.c index 3a18ccd..c7b86e8 100644 --- a/pcilib/pci.c +++ b/pcilib/pci.c @@ -1,4 +1,5 @@ //#define PCILIB_FILE_IO +#define _XOPEN_SOURCE 700 #define _BSD_SOURCE #define _POSIX_C_SOURCE 200809L @@ -24,6 +25,7 @@ #include "model.h" #include "plugin.h" #include "bar.h" +#include "locking.h" static int pcilib_detect_model(pcilib_t *ctx, const char *model) { int i, j; @@ -159,6 +161,12 @@ pcilib_t *pcilib_open(const char *device, const char *model) { ctx->model_info.protocols = ctx->protocols; ctx->model_info.ranges = ctx->ranges; + err=pcilib_init_locking(ctx); + if (err) { + pcilib_error("Error (%i) initializing locking\n", err); + pcilib_close(ctx); + return NULL; + } err = pcilib_init_register_banks(ctx); if (err) { diff --git a/pcilib/pci.h b/pcilib/pci.h index d176caf..a7b3d50 100644 --- a/pcilib/pci.h +++ b/pcilib/pci.h @@ -69,7 +69,7 @@ struct pcilib_s { pcilib_register_bank_context_t *bank_ctx[PCILIB_MAX_REGISTER_BANKS]; /**< Contexts for registers banks if required by register protocol */ pcilib_dma_context_t *dma_ctx; /**< DMA context */ pcilib_context_t *event_ctx; /**< Implmentation context */ - + void* locks_handle; /**< adress of the kernel memory use for locks from user space*/ #ifdef PCILIB_FILE_IO int file_io_handle; #endif /* PCILIB_FILE_IO */ diff --git a/pcitool/cli.c b/pcitool/cli.c index 9eeb046..a6224e3 100644 --- a/pcitool/cli.c +++ b/pcitool/cli.c @@ -1,3 +1,4 @@ +#define _XOPEN_SOURCE 700 #define _POSIX_C_SOURCE 200112L #define _BSD_SOURCE @@ -37,6 +38,7 @@ #include "error.h" #include "debug.h" #include "model.h" +#include "locking.h" /* defines */ #define MAX_KBUF 14 @@ -89,7 +91,8 @@ typedef enum { MODE_ALLOC_KMEM, MODE_LIST_KMEM, MODE_READ_KMEM, - MODE_FREE_KMEM + MODE_FREE_KMEM, + MODE_FREE_LOCKS } MODE; typedef enum { @@ -167,7 +170,8 @@ typedef enum { OPT_VERIFY, OPT_WAIT, OPT_MULTIPACKET, - OPT_VERBOSE + OPT_VERBOSE, + OPT_FREE_LOCKS } OPTIONS; static struct option long_options[] = { @@ -219,6 +223,7 @@ static struct option long_options[] = { {"multipacket", no_argument, 0, OPT_MULTIPACKET }, {"wait", no_argument, 0, OPT_WAIT }, {"help", no_argument, 0, OPT_HELP }, + {"free-locks", no_argument, 0, OPT_FREE_LOCKS}, { 0, 0, 0, 0 } }; @@ -272,6 +277,7 @@ void Usage(int argc, char *argv[], const char *format, ...) { " block is specified as: use:block_number\n" " --alloc-kernel-memory - Allocate kernel buffers (DANGEROUS)\n" " --free-kernel-memory - Cleans lost kernel space buffers (DANGEROUS)\n" +" --free-locks - Cleans locks allocated during pcitool program use(dangerous in a concurrential model)\n" " dma - Remove all buffers allocated by DMA subsystem\n" " #number - Remove all buffers with the specified use id\n" "\n" @@ -2570,6 +2576,9 @@ int main(int argc, char **argv) { event = stmp; } break; + case OPT_FREE_LOCKS: + mode=MODE_FREE_LOCKS; + break; case OPT_TRIGGER: if ((mode != MODE_INVALID)&&((mode != MODE_GRAB)||(grab_mode&GRAB_MODE_TRIGGER))) Usage(argc, argv, "Multiple operations are not supported"); @@ -2949,7 +2958,6 @@ int main(int argc, char **argv) { model_info = pcilib_get_model_description(handle); dma_info = pcilib_get_dma_description(handle); - switch (mode) { case MODE_WRITE: if (((argc - optind) == 1)&&(*argv[optind] == '*')) { @@ -3137,6 +3145,9 @@ int main(int argc, char **argv) { } switch (mode) { + case MODE_FREE_LOCKS: + pcilib_clean_all_locks(handle); + break; case MODE_INFO: Info(handle, model_info); break; diff --git a/protocols/software.c b/protocols/software.c index 5534dc7..5170583 100644 --- a/protocols/software.c +++ b/protocols/software.c @@ -1,38 +1,44 @@ +#define _XOPEN_SOURCE 700 + #include #include #include #include #include - #include "model.h" #include "error.h" #include "kmem.h" #include "pcilib.h" #include "pci.h" +#include "lock.h" typedef struct pcilib_software_register_bank_context_s pcilib_software_register_bank_context_t; +/** + * new context to save all needed informations for software registers + */ struct pcilib_software_register_bank_context_s { - pcilib_register_bank_context_t bank_ctx; + pcilib_register_bank_context_t bank_ctx; /**< the bank context associated with the software registers*/ - pcilib_kmem_handle_t *kmem; - void *addr; + pcilib_kmem_handle_t *kmem; /**< the kernel memory for software registers */ + void *addr; /**< the base adress of the allocated kernel memory*/ }; /** * pcilib_software_registers_close * this function clear the kernel memory space that could have been allocated for software registers + *@param[in] ctx the pcilib_t running * @param[in] bank_ctx the bank context running that we get from the initialisation function */ void pcilib_software_registers_close(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx) { if (((pcilib_software_register_bank_context_t*)bank_ctx)->kmem) - pcilib_free_kernel_memory(ctx, ((pcilib_software_register_bank_context_t*)bank_ctx)->kmem, PCILIB_KMEM_FLAG_REUSE); + pcilib_free_kernel_memory(ctx, ((pcilib_software_register_bank_context_t*)bank_ctx)->kmem, PCILIB_KMEM_FLAG_REUSE); free(bank_ctx); } /** * pcilib_software_registers_open - * this function initializes the kernel space memory and stores in it the default values of the registers of the given bank index, if it was not initialized by a concurrent process, and return a bank context containing the adress of this kernel space. It the kernel space memory was already initialized by a concurrent process, then this function just return the bank context with the adress of this kernel space already used + * this function initializes the kernel space memory and stores in it the default values of the registers of the given bank index, if it was not initialized by a concurrent process, and return a bank context containing the adress of this kernel space. If the kernel space memory was already initialized by a concurrent process, then this function just return the bank context with the adress of this kernel space already used. The initialization is protected by the locking mechanisms of lock files. * @param[in] ctx the pcilib_t structure running * @param[in] bank the bank index that will permits to get the bank we want registers from * @param[in] model not used @@ -43,6 +49,7 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc pcilib_software_register_bank_context_t *bank_ctx; pcilib_kmem_handle_t *handle; pcilib_kmem_reuse_state_t reused; + pcilib_lock_t* semId=NULL; const pcilib_register_bank_description_t *bank_desc = ctx->banks + bank; @@ -52,8 +59,12 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc } bank_ctx = calloc(1, sizeof(pcilib_software_register_bank_context_t)); + + /* prtoection against several kernel memory allocation*/ + semId=pcilib_init_lock(ctx,"thisatest2"); + pcilib_lock(semId,MUTEX_LOCK); - handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); + handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); /**< get the kernel memory*/ if (!handle) { pcilib_error("Allocation of kernel memory for software registers has failed"); pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); @@ -75,11 +86,18 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc for (i = 0; ctx->model_info.registers[i].name != NULL; i++) { if ((ctx->model_info.registers[i].bank == ctx->banks[bank].addr)&&(ctx->model_info.registers[i].type == PCILIB_REGISTER_STANDARD)) { - *(pcilib_register_value_t*)(bank_ctx->addr + ctx->model_info.registers[i].addr) = ctx->model_info.registers[i].defvalue; + + /*initialization here*/ + + *(pcilib_register_value_t*)(bank_ctx->addr + ctx->model_info.registers[i].addr) = ctx->model_info.registers[i].defvalue; + + + } } } - + pcilib_unlock(semId); + return (pcilib_register_bank_context_t*)bank_ctx; } @@ -93,11 +111,12 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc * @return 0 in case of success */ int pcilib_software_registers_read(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t *value){ + if ((addr + sizeof(pcilib_register_value_t)) > bank_ctx->bank->size) { pcilib_error("Trying to access space outside of the define register bank (bank: %s, addr: 0x%lx)", bank_ctx->bank->name, addr); return PCILIB_ERROR_INVALID_ADDRESS; } - + /* the following reading is considered atomic operation, so not protected, may change*/ *value = *(pcilib_register_value_t*)(((pcilib_software_register_bank_context_t*)bank_ctx)->addr + addr); return 0; } @@ -116,7 +135,7 @@ int pcilib_software_registers_write(pcilib_t *ctx, pcilib_register_bank_context_ pcilib_error("Trying to access space outside of the define register bank (bank: %s, addr: 0x%lx)", bank_ctx->bank->name, addr); return PCILIB_ERROR_INVALID_ADDRESS; } - + /* the following writing is considered atomic operation, so not protected, may change*/ *(pcilib_register_value_t*)(((pcilib_software_register_bank_context_t*)bank_ctx)->addr + addr) = value; return 0; } -- cgit v1.2.3 From fe821c4f1b85d2a2d358da098f85327d41212dc3 Mon Sep 17 00:00:00 2001 From: zilio nicolas Date: Tue, 7 Jul 2015 15:38:14 +0200 Subject: modified for last remarks --- pcilib/lock.c | 40 ++++++++++++++++++++++------------------ pcilib/locking.c | 4 ++-- 2 files changed, 24 insertions(+), 20 deletions(-) diff --git a/pcilib/lock.c b/pcilib/lock.c index 8715d21..b99ecfc 100644 --- a/pcilib/lock.c +++ b/pcilib/lock.c @@ -6,7 +6,7 @@ #include "error.h" #include "lock.h" #include "pci.h" - +#include /* * this function will take the lock for the semaphore pointed by semId */ @@ -18,32 +18,32 @@ void pcilib_lock(pcilib_lock_t *lock_ctx, pcilib_lock_flags_t flags, ...){ if(flags & MUTEX_LOCK){ err=pthread_mutex_lock(lock_ctx);/**< we try to lock here*/ - if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); /** if the lock haven't been acquired and errno==EOWNERDEAD, it means the previous application that got the lock crashed, we have to remake the lock "consistent" so*/ else if(errno==EOWNERDEAD){ pthread_mutex_consistent(lock_ctx); pthread_mutex_lock(lock_ctx); - if(err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + if(err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); } } else if(flags & MUTEX_TRYLOCK){ err=pthread_mutex_trylock(lock_ctx);/**< we try to lock here*/ - if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); else if(errno==EOWNERDEAD){ pthread_mutex_consistent(lock_ctx); pthread_mutex_lock(lock_ctx); - if(err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + if(err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); } } else if(flags & MUTEX_TIMEDLOCK){ time=va_arg(pa,struct timespec*); va_end(pa); err=pthread_mutex_timedlock(lock_ctx, time);/**< we try to lock here*/ - if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); + if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, errni %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); else if(errno==EOWNERDEAD){ pthread_mutex_consistent(lock_ctx); pthread_mutex_timedlock(lock_ctx, time); - if(err!=0) pcilib_error("can't acquire lock %s, error %i\n",(char*)(lock_ctx+sizeof(pcilib_lock_t)), errno); + if(err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)), errno); } } else pcilib_error("wrong flag for pcilib_lock"); @@ -54,7 +54,7 @@ void pcilib_lock(pcilib_lock_t *lock_ctx, pcilib_lock_flags_t flags, ...){ */ void pcilib_unlock(pcilib_lock_t* lock_ctx){ int err; - if((err=pthread_mutex_unlock(lock_ctx))!=0) pcilib_error("can't unlock semaphore\n"); + if((err=pthread_mutex_unlock(lock_ctx))!=0) pcilib_error("can't unlock mutex: errno %i",errno); } /** @@ -71,21 +71,25 @@ pcilib_lock_t* pcilib_init_lock(pcilib_t *ctx, char* lock_id, ...){ va_list pa; va_start(pa,lock_id); + char* temp; + temp=malloc((strlen(lock_id)+strlen("bank_register_"))*sizeof(char)); + sprintf(temp,"bank_register_%s",lock_id); + pcilib_lock_init_flags_t flag; flag=va_arg(pa,pcilib_lock_init_flags_t); va_end(pa); - if(strlen(lock_id)>PCILIB_LOCK_SIZE-sizeof(pcilib_lock_t)) pcilib_error("the entered protocol name is too long"); + if(strlen(temp)>PCILIB_LOCK_SIZE-sizeof(pcilib_lock_t)) pcilib_error("the entered protocol name is too long"); if(((PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE)%PCILIB_KMEM_PAGE_SIZE)!=0) pcilib_error("PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE should be a multiple of kmem page size"); addr=pcilib_kmem_get_block_ua(ctx,ctx->locks_handle,0); - if(flag & LOCK_INIT) pcilib_lock((pcilib_lock_t*)addr,MUTEX_LOCK); + if((flag & LOCK_INIT)==0) pcilib_lock((pcilib_lock_t*)addr,MUTEX_LOCK); /* we search for the given lock if it was already initialized*/ for(j=0;jlocks_handle,j); while((*(char*)(locks_addr+i*PCILIB_LOCK_SIZE+sizeof(pcilib_lock_t))!=0) && (ilocks_handle=handle; reused = pcilib_kmem_is_reused(ctx, handle); -//#define DEBUG_REUSE + #define DEBUG_REUSE #ifdef DEBUG_REUSE reused=0; #endif @@ -57,7 +57,7 @@ reused=0; pcilib_register_t i; if (reused & PCILIB_KMEM_REUSE_PARTIAL) { - pcilib_error("Inconsistent software registers are found (only part of required buffers is available)"); + pcilib_error("Inconsistent memory for locks was found (only part of required buffers is available)"); pcilib_clean_all_locks(ctx); return 1; } -- cgit v1.2.3 From edd5ccf24c146915ee475bd223e2ad695520a241 Mon Sep 17 00:00:00 2001 From: zilio nicolas Date: Fri, 10 Jul 2015 10:52:06 +0200 Subject: last modification+comments update --- pcilib/lock.c | 32 ++++++++++++++------------------ pcilib/lock.h | 25 +++++++++++++++++-------- pcilib/locking.c | 15 +++++++++++---- pcilib/locking.h | 7 ++++++- protocols/software.c | 2 +- 5 files changed, 49 insertions(+), 32 deletions(-) diff --git a/pcilib/lock.c b/pcilib/lock.c index b99ecfc..b92b11d 100644 --- a/pcilib/lock.c +++ b/pcilib/lock.c @@ -7,6 +7,7 @@ #include "lock.h" #include "pci.h" #include + /* * this function will take the lock for the semaphore pointed by semId */ @@ -21,6 +22,7 @@ void pcilib_lock(pcilib_lock_t *lock_ctx, pcilib_lock_flags_t flags, ...){ if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); /** if the lock haven't been acquired and errno==EOWNERDEAD, it means the previous application that got the lock crashed, we have to remake the lock "consistent" so*/ else if(errno==EOWNERDEAD){ + /* one question is "is pthread_mutex_consistent protected in case we call twice it?", it seems to not make any importance in fact regarding man pages, but we have to survey it in future applications*/ pthread_mutex_consistent(lock_ctx); pthread_mutex_lock(lock_ctx); if(err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); @@ -31,7 +33,7 @@ void pcilib_lock(pcilib_lock_t *lock_ctx, pcilib_lock_flags_t flags, ...){ if(errno!=EOWNERDEAD && err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); else if(errno==EOWNERDEAD){ pthread_mutex_consistent(lock_ctx); - pthread_mutex_lock(lock_ctx); + pthread_mutex_trylock(lock_ctx); if(err!=0) pcilib_error("can't acquire lock %s, errno %i",(char*)(lock_ctx+sizeof(pcilib_lock_t)),errno); } } @@ -58,43 +60,37 @@ void pcilib_unlock(pcilib_lock_t* lock_ctx){ } /** - * pcilib_init_lock * this function initialize a new semaphore in the kernel if it's not already initialized given the key that permits to differentiate semaphores, and then return the integer that points to the semaphore that have been initialized or to a previously already initialized semaphore - * @param[out] lock_ctx the pointer that will points to the semaphore for other functions - * @param[in] keysem the integer that permits to define to what the semaphore is attached */ -pcilib_lock_t* pcilib_init_lock(pcilib_t *ctx, char* lock_id, ...){ +pcilib_lock_t* pcilib_init_lock(pcilib_t *ctx, pcilib_lock_init_flags_t flag, char* lock_id, ...){ int err; pthread_mutexattr_t attr; int i,j; void* addr,*locks_addr; + char buffer[PCILIB_LOCK_SIZE-sizeof(pcilib_lock_t)]; + /* here lock_id is the format string for vsprintf, the lock name will so be put in adding arguments*/ va_list pa; va_start(pa,lock_id); - - char* temp; - temp=malloc((strlen(lock_id)+strlen("bank_register_"))*sizeof(char)); - sprintf(temp,"bank_register_%s",lock_id); - - pcilib_lock_init_flags_t flag; - flag=va_arg(pa,pcilib_lock_init_flags_t); + err=vsprintf(buffer,lock_id,pa); va_end(pa); - if(strlen(temp)>PCILIB_LOCK_SIZE-sizeof(pcilib_lock_t)) pcilib_error("the entered protocol name is too long"); + if(err<0) pcilib_error("error in obtaining the lock name"); if(((PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE)%PCILIB_KMEM_PAGE_SIZE)!=0) pcilib_error("PCILIB_MAX_NUMBER_LOCKS*PCILIB_LOCK_SIZE should be a multiple of kmem page size"); addr=pcilib_kmem_get_block_ua(ctx,ctx->locks_handle,0); - if((flag & LOCK_INIT)==0) pcilib_lock((pcilib_lock_t*)addr,MUTEX_LOCK); + if((flag & PCILIB_NO_LOCK)==0) pcilib_lock((pcilib_lock_t*)addr,MUTEX_LOCK); /* we search for the given lock if it was already initialized*/ for(j=0;jlocks_handle,j); while((*(char*)(locks_addr+i*PCILIB_LOCK_SIZE+sizeof(pcilib_lock_t))!=0) && (ilocks_handle,i); for(j=0;jlocks_handle,PCILIB_KMEM_FLAG_REUSE); @@ -38,8 +42,9 @@ int pcilib_init_locking(pcilib_t* ctx, ...){ int err; pcilib_kmem_reuse_state_t reused; - + /* we flock() to make sure to not have two initialization in the same time (possible long time to init)*/ if((err=flock(ctx->handle,LOCK_EX))==-1) pcilib_warning("can't get flock on /dev/fpga0"); + handle=pcilib_alloc_kernel_memory(ctx,PCILIB_KMEM_TYPE_PAGE,PCILIB_NUMBER_OF_LOCK_PAGES,PCILIB_KMEM_PAGE_SIZE,0,PCILIB_KMEM_USE(PCILIB_KMEM_USE_MUTEXES,0),PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); if (!handle) { @@ -49,10 +54,11 @@ int pcilib_init_locking(pcilib_t* ctx, ...){ ctx->locks_handle=handle; reused = pcilib_kmem_is_reused(ctx, handle); - #define DEBUG_REUSE +//#define DEBUG_REUSE #ifdef DEBUG_REUSE reused=0; #endif +/* verification about the handling memory got, first use or not, and integrity*/ if ((reused & PCILIB_KMEM_REUSE_REUSED) == 0) { pcilib_register_t i; @@ -61,10 +67,11 @@ reused=0; pcilib_clean_all_locks(ctx); return 1; } + /* if we get here so this is the first initialization (after some free or new), we so set kernel pages to 0 and initialize then the first lock that will be used when we create other locks*/ for(i=0;iname,"software_register"); pcilib_lock(semId,MUTEX_LOCK); handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); /**< get the kernel memory*/ -- cgit v1.2.3 From fb42f24213a7aaecbd631e61fb432ef04742d2ce Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Fri, 24 Jul 2015 13:09:23 +0200 Subject: Use 64-bit addressing in IPEDMA only for gen3 boards or if enforced --- dma/ipe.c | 18 +++++++++++------- dma/ipe.h | 1 + dma/ipe_private.h | 7 ++++--- 3 files changed, 16 insertions(+), 10 deletions(-) diff --git a/dma/ipe.c b/dma/ipe.c index 953de09..814bc77 100644 --- a/dma/ipe.c +++ b/dma/ipe.c @@ -22,8 +22,8 @@ pcilib_dma_context_t *dma_ipe_init(pcilib_t *pcilib, const char *model, const void *arg) { -// int err = 0; - + pcilib_register_value_t value; + const pcilib_model_description_t *model_info = pcilib_get_model_description(pcilib); ipe_dma_t *ctx = malloc(sizeof(ipe_dma_t)); @@ -32,11 +32,6 @@ pcilib_dma_context_t *dma_ipe_init(pcilib_t *pcilib, const char *model, const vo memset(ctx, 0, sizeof(ipe_dma_t)); ctx->dmactx.pcilib = pcilib; -#ifdef IPEDMA_64BIT_MODE - // Always supported and we need to use it - ctx->mode64 = 1; -#endif /* IPEDMA_64BIT_MODE */ - pcilib_register_bank_t dma_bank = pcilib_find_register_bank_by_addr(pcilib, PCILIB_REGISTER_BANK_DMA); if (dma_bank == PCILIB_REGISTER_BANK_INVALID) { @@ -47,6 +42,15 @@ pcilib_dma_context_t *dma_ipe_init(pcilib_t *pcilib, const char *model, const vo ctx->dma_bank = model_info->banks + dma_bank; ctx->base_addr = pcilib_resolve_register_address(pcilib, ctx->dma_bank->bar, ctx->dma_bank->read_addr); + +#ifdef IPEDMA_ENFORCE_64BIT_MODE + ctx->mode64 = 1; +#else /* IPEDMA_ENFORCE_64BIT_MODE */ + // According to Lorenzo, some gen2 boards have problems with 64-bit addressing. Therefore, we only enable it for gen3 boards unless enforced + RD(IPEDMA_REG_PCIE_GEN, value); + if (value > 2) ctx->mode64 = 1; + else ctx->mode64 = 0; +#endif /* IPEDMA_ENFORCE_64BIT_MODE */ } return (pcilib_dma_context_t*)ctx; diff --git a/dma/ipe.h b/dma/ipe.h index 5640606..76be4f4 100644 --- a/dma/ipe.h +++ b/dma/ipe.h @@ -65,6 +65,7 @@ static const pcilib_register_description_t ipe_dma_registers[] = { {0x000C, 24, 8, 0, 0xFFFFFFFF, PCILIB_REGISTER_RW , PCILIB_REGISTER_BITS, PCILIB_REGISTER_BANK_DMA, "mwr_up_addr", "Upper address for 64 bit memory addressing"}, {0x0010, 0, 32, 0, 0x00000000, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "mwr_count", "Write DMA TLP Count"}, {0x0014, 0, 32, 0, 0x00000000, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "mwr_pattern", "DMA generator data pattern"}, + {0x0018, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "pcie_gen", "PCIe version 2/3 depending on the used XILINX core"}, {0x0028, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "mwr_perf", "MWR Performance"}, {0x003C, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "cfg_lnk_width", "Negotiated and max width of PCIe Link"}, {0x003C, 0, 6, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_BITS, PCILIB_REGISTER_BANK_DMA, "cfg_cap_max_lnk_width", "Max link width"}, diff --git a/dma/ipe_private.h b/dma/ipe_private.h index 249286d..937e202 100644 --- a/dma/ipe_private.h +++ b/dma/ipe_private.h @@ -3,12 +3,12 @@ #include "dma.h" -#define IPEDMA_64BIT_MODE 1 /**< 64-bit mode addressing is required to support PCIe gen3 */ +//#define IPEDMA_ENFORCE_64BIT_MODE 1 /**< enforce 64-bit mode addressing (otherwise it is used only if register 0x18 specifies PCIe gen3 as required by DMA engine) */ #define IPEDMA_CORES 1 #define IPEDMA_MAX_TLP_SIZE 256 /**< Defines maximum TLP in bytes supported by device */ -//#define IPEDMA_TLP_SIZE 128 /**< If set, enforces the specified TLP size */ +//#define IPEDMA_TLP_SIZE 128 /**< If set, enforces the specified TLP size */ -#define IPEDMA_STREAMING_MODE /**< Enables streaming DMA operation mode instead of ring-buffer, the page is written once and forgotten and need to be pushed in queue again */ +//#define IPEDMA_STREAMING_MODE /**< Enables streaming DMA operation mode instead of ring-buffer, the page is written once and forgotten and need to be pushed in queue again */ #define IPEDMA_STREAMING_CHECKS /**< Enables status checks in streaming mode (it will cause performance penalty) */ #define IPEDMA_PAGE_SIZE 4096 #define IPEDMA_DMA_PAGES 1024 /**< number of DMA pages in the ring buffer to allocate */ @@ -31,6 +31,7 @@ #define IPEDMA_REG_CONTROL 0x04 #define IPEDMA_REG_TLP_SIZE 0x0C #define IPEDMA_REG_TLP_COUNT 0x10 +#define IPEDMA_REG_PCIE_GEN 0x18 #define IPEDMA_REG_PAGE_ADDR 0x50 #define IPEDMA_REG_UPDATE_ADDR 0x54 #define IPEDMA_REG_LAST_READ 0x58 /**< In streaming mode, we can use it freely to track current status */ -- cgit v1.2.3 From 61068e3ba4863cf41b2399ea3057b3bb1c4a9dcf Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Fri, 24 Jul 2015 13:09:59 +0200 Subject: Make pcilib_streaming_action_t public --- docs/README | 1 + pcilib/event.h | 11 ----------- pcilib/pcilib.h | 11 +++++++++++ 3 files changed, 12 insertions(+), 11 deletions(-) diff --git a/docs/README b/docs/README index 647df41..ba22acf 100644 --- a/docs/README +++ b/docs/README @@ -8,6 +8,7 @@ Supported environmental variables IPECAMERA_DEBUG_RAW_PACKETS - Store all raw packets read from DMA grouped in frames (variable may specify directory) IPECAMERA_DEBUG_RAW_FRAMES - Store all raw frames (variable may specify directory) IPECAMERA_DEBUG_HARDWARE - Produce various debugging information about ipecamera operation + IPECAMERA_DEBUG_FRAME_HEADERS - Print headers of each frame PCILIB_BENCHMARK_HARDWARE - Remove all unnecessary software processing (like copying memcpy) to check hardware performance PCILIB_BENCHMARK_STREAMING - Emulate streaming mode while benchmarking DMA engines diff --git a/pcilib/event.h b/pcilib/event.h index d2b9793..f14abeb 100644 --- a/pcilib/event.h +++ b/pcilib/event.h @@ -45,17 +45,6 @@ typedef struct { const char *description; } pcilib_event_data_type_description_t; -typedef enum { - PCILIB_STREAMING_STOP = 0, /**< stop streaming */ - PCILIB_STREAMING_CONTINUE = 1, /**< wait the default DMA timeout for a new data */ - PCILIB_STREAMING_WAIT = 2, /**< wait the specified timeout for a new data */ - PCILIB_STREAMING_CHECK = 3, /**< do not wait for the data, bail out imideatly if no data ready */ - PCILIB_STREAMING_FAIL = 4, /**< fail if data is not available on timeout */ - PCILIB_STREAMING_REQ_FRAGMENT = 5, /**< only fragment of a packet is read, wait for next fragment and fail if no data during DMA timeout */ - PCILIB_STREAMING_REQ_PACKET = 6, /**< wait for next packet and fail if no data during the specified timeout */ - PCILIB_STREAMING_TIMEOUT_MASK = 3 /**< mask specifying all timeout modes */ -} pcilib_streaming_action_t; - /* * get_data: This call is used by get_data and copy_data functions of public * interface. When copy_data is the caller, the data parameter will be passed. diff --git a/pcilib/pcilib.h b/pcilib/pcilib.h index 8a43bfb..01f3324 100644 --- a/pcilib/pcilib.h +++ b/pcilib/pcilib.h @@ -62,6 +62,17 @@ typedef enum { PCILIB_DMA_FLAG_IGNORE_ERRORS = 16 /**< do not crash on errors, but return appropriate error codes */ } pcilib_dma_flags_t; +typedef enum { + PCILIB_STREAMING_STOP = 0, /**< stop streaming */ + PCILIB_STREAMING_CONTINUE = 1, /**< wait the default DMA timeout for a new data */ + PCILIB_STREAMING_WAIT = 2, /**< wait the specified timeout for a new data */ + PCILIB_STREAMING_CHECK = 3, /**< do not wait for the data, bail out imideatly if no data ready */ + PCILIB_STREAMING_FAIL = 4, /**< fail if data is not available on timeout */ + PCILIB_STREAMING_REQ_FRAGMENT = 5, /**< only fragment of a packet is read, wait for next fragment and fail if no data during DMA timeout */ + PCILIB_STREAMING_REQ_PACKET = 6, /**< wait for next packet and fail if no data during the specified timeout */ + PCILIB_STREAMING_TIMEOUT_MASK = 3 /**< mask specifying all timeout modes */ +} pcilib_streaming_action_t; + typedef enum { PCILIB_EVENT_FLAGS_DEFAULT = 0, PCILIB_EVENT_FLAG_RAW_DATA_ONLY = 1, /**< Do not parse data, just read raw and pass it to rawdata callback. If passed to rawdata callback, idicates the data is not identified as event (most probably just padding) */ -- cgit v1.2.3 From 8f5fb3a23bd0adcd4d0d021fdda571d3b3761bba Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 00:53:30 +0200 Subject: Use bank addresses as kernel memory subtypes of software registers instead of indexes in array --- protocols/software.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/protocols/software.c b/protocols/software.c index 5534dc7..e1339a6 100644 --- a/protocols/software.c +++ b/protocols/software.c @@ -53,7 +53,7 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc bank_ctx = calloc(1, sizeof(pcilib_software_register_bank_context_t)); - handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); + handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank_desc->addr), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); if (!handle) { pcilib_error("Allocation of kernel memory for software registers has failed"); pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); -- cgit v1.2.3 From 39417c2c0386a971a6973a6aaf779e82c9298020 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 15:14:41 +0200 Subject: Fix error-checking typo in kmem --- pcilib/kmem.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pcilib/kmem.c b/pcilib/kmem.c index 2ff674f..469c63a 100644 --- a/pcilib/kmem.c +++ b/pcilib/kmem.c @@ -81,8 +81,8 @@ pcilib_kmem_handle_t *pcilib_alloc_kernel_memory(pcilib_t *ctx, pcilib_kmem_type memset(kbuf, 0, sizeof(pcilib_kmem_list_t) + nmemb * sizeof(pcilib_kmem_addr_t)); err = pcilib_lock(ctx->locks.mmap); - if (!err) { - pcilib_error("Error acquiring mmap lock"); + if (err) { + pcilib_error("Error (%i) acquiring mmap lock", err); return NULL; } -- cgit v1.2.3 From 90f6e9b0856fc1c9fdf1195c0c5c43d3b58426c1 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 15:26:53 +0200 Subject: Protect with locks the initialization of software registers --- protocols/software.c | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/protocols/software.c b/protocols/software.c index e1339a6..ed9d372 100644 --- a/protocols/software.c +++ b/protocols/software.c @@ -13,10 +13,10 @@ typedef struct pcilib_software_register_bank_context_s pcilib_software_register_bank_context_t; struct pcilib_software_register_bank_context_s { - pcilib_register_bank_context_t bank_ctx; + pcilib_register_bank_context_t bank_ctx; /**< the bank context associated with the software registers */ - pcilib_kmem_handle_t *kmem; - void *addr; + pcilib_kmem_handle_t *kmem; /**< the kernel memory for software registers */ + void *addr; /**< the virtual adress of the allocated kernel memory*/ }; /** @@ -40,8 +40,10 @@ void pcilib_software_registers_close(pcilib_t *ctx, pcilib_register_bank_context * @return a bank context with the adress of the kernel space memory related to it */ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pcilib_register_bank_t bank, const char* model, const void *args) { + int err; pcilib_software_register_bank_context_t *bank_ctx; pcilib_kmem_handle_t *handle; + pcilib_lock_t *lock; pcilib_kmem_reuse_state_t reused; const pcilib_register_bank_description_t *bank_desc = ctx->banks + bank; @@ -52,9 +54,29 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc } bank_ctx = calloc(1, sizeof(pcilib_software_register_bank_context_t)); + if (!bank_ctx) { + pcilib_error("Memory allocation for bank context has failed"); + return NULL; + } + + lock = pcilib_get_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, "softreg/%s", bank_desc->name); + if (!lock) { + pcilib_error("Failed to initialize a lock to protect bank %s with software registers", bank_desc->name); + return NULL; + } + + err = pcilib_lock(lock); + if (err) { + pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); + pcilib_error("Error (%i) obtaining lock on bank %s with software registers", err, bank_desc->name); + return NULL; + } + handle = pcilib_alloc_kernel_memory(ctx, PCILIB_KMEM_TYPE_PAGE, 1, PCILIB_KMEM_PAGE_SIZE, 0, PCILIB_KMEM_USE(PCILIB_KMEM_USE_SOFTWARE_REGISTERS, bank_desc->addr), PCILIB_KMEM_FLAG_REUSE|PCILIB_KMEM_FLAG_PERSISTENT); if (!handle) { + pcilib_unlock(lock); + pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); pcilib_error("Allocation of kernel memory for software registers has failed"); pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); return NULL; @@ -68,6 +90,8 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc pcilib_register_t i; if (reused & PCILIB_KMEM_REUSE_PARTIAL) { + pcilib_unlock(lock); + pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); pcilib_error("Inconsistent software registers are found (only part of required buffers is available)"); pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); return NULL; @@ -80,6 +104,9 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc } } + pcilib_unlock(lock); + pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); + return (pcilib_register_bank_context_t*)bank_ctx; } @@ -97,7 +124,8 @@ int pcilib_software_registers_read(pcilib_t *ctx, pcilib_register_bank_context_t pcilib_error("Trying to access space outside of the define register bank (bank: %s, addr: 0x%lx)", bank_ctx->bank->name, addr); return PCILIB_ERROR_INVALID_ADDRESS; } - + + // we consider this atomic operation and, therefore, do no locking *value = *(pcilib_register_value_t*)(((pcilib_software_register_bank_context_t*)bank_ctx)->addr + addr); return 0; } @@ -111,12 +139,13 @@ int pcilib_software_registers_read(pcilib_t *ctx, pcilib_register_bank_context_t * @param[out] value the value we want to write in the register * @return 0 in case of success */ -int pcilib_software_registers_write(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t value){ +int pcilib_software_registers_write(pcilib_t *ctx, pcilib_register_bank_context_t *bank_ctx, pcilib_register_addr_t addr, pcilib_register_value_t value) { if ((addr + sizeof(pcilib_register_value_t)) > bank_ctx->bank->size) { pcilib_error("Trying to access space outside of the define register bank (bank: %s, addr: 0x%lx)", bank_ctx->bank->name, addr); return PCILIB_ERROR_INVALID_ADDRESS; } + // we consider this atomic operation and, therefore, do no locking *(pcilib_register_value_t*)(((pcilib_software_register_bank_context_t*)bank_ctx)->addr + addr) = value; return 0; } -- cgit v1.2.3 From f2e47ba725cc7ba477d3a788addc974f2b18d36f Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 15:30:41 +0200 Subject: Protect mmaping BARs as well --- pcilib/bar.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/pcilib/bar.c b/pcilib/bar.c index ce04f6d..17828a8 100644 --- a/pcilib/bar.c +++ b/pcilib/bar.c @@ -73,21 +73,29 @@ int pcilib_detect_address(pcilib_t *ctx, pcilib_bar_t *bar, uintptr_t *addr, siz void *pcilib_map_bar(pcilib_t *ctx, pcilib_bar_t bar) { void *res; - int ret; + int err, ret; const pcilib_board_info_t *board_info = pcilib_get_board_info(ctx); if (!board_info) return NULL; - + if (ctx->bar_space[bar]) return ctx->bar_space[bar]; - + + err = pcilib_lock(ctx->locks.mmap); + if (err) { + pcilib_error("Error (%i) acquiring mmap lock", err); + return NULL; + } + ret = ioctl( ctx->handle, PCIDRIVER_IOC_MMAP_MODE, PCIDRIVER_MMAP_PCI ); if (ret) { + pcilib_unlock(ctx->locks.mmap); pcilib_error("PCIDRIVER_IOC_MMAP_MODE ioctl have failed", bar); return NULL; } ret = ioctl( ctx->handle, PCIDRIVER_IOC_MMAP_AREA, PCIDRIVER_BAR0 + bar ); if (ret) { + pcilib_unlock(ctx->locks.mmap); pcilib_error("PCIDRIVER_IOC_MMAP_AREA ioctl have failed for bank %i", bar); return NULL; } @@ -98,12 +106,13 @@ void *pcilib_map_bar(pcilib_t *ctx, pcilib_bar_t bar) { #else res = mmap( 0, board_info->bar_length[bar], PROT_WRITE | PROT_READ, MAP_SHARED, ctx->handle, 0 ); #endif + pcilib_unlock(ctx->locks.mmap); + if ((!res)||(res == MAP_FAILED)) { pcilib_error("Failed to mmap data bank %i", bar); return NULL; } - return res; } -- cgit v1.2.3 From 7c5897933f4f64594602b2e38264af705c061754 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 18:11:59 +0200 Subject: Use global locks to protect kmem allocation to prevent race while allocating simmultaneously locking kmem pages and any other type of kmem --- pcilib/bar.c | 9 +++++---- pcilib/kmem.c | 6 +++--- pcilib/lock.c | 5 ++++- pcilib/locking.c | 6 +----- pcilib/locking.h | 2 +- 5 files changed, 14 insertions(+), 14 deletions(-) diff --git a/pcilib/bar.c b/pcilib/bar.c index 17828a8..418f864 100644 --- a/pcilib/bar.c +++ b/pcilib/bar.c @@ -80,7 +80,7 @@ void *pcilib_map_bar(pcilib_t *ctx, pcilib_bar_t bar) { if (ctx->bar_space[bar]) return ctx->bar_space[bar]; - err = pcilib_lock(ctx->locks.mmap); + err = pcilib_lock_global(ctx); if (err) { pcilib_error("Error (%i) acquiring mmap lock", err); return NULL; @@ -88,14 +88,14 @@ void *pcilib_map_bar(pcilib_t *ctx, pcilib_bar_t bar) { ret = ioctl( ctx->handle, PCIDRIVER_IOC_MMAP_MODE, PCIDRIVER_MMAP_PCI ); if (ret) { - pcilib_unlock(ctx->locks.mmap); + pcilib_unlock_global(ctx); pcilib_error("PCIDRIVER_IOC_MMAP_MODE ioctl have failed", bar); return NULL; } ret = ioctl( ctx->handle, PCIDRIVER_IOC_MMAP_AREA, PCIDRIVER_BAR0 + bar ); if (ret) { - pcilib_unlock(ctx->locks.mmap); + pcilib_unlock_global(ctx); pcilib_error("PCIDRIVER_IOC_MMAP_AREA ioctl have failed for bank %i", bar); return NULL; } @@ -106,7 +106,8 @@ void *pcilib_map_bar(pcilib_t *ctx, pcilib_bar_t bar) { #else res = mmap( 0, board_info->bar_length[bar], PROT_WRITE | PROT_READ, MAP_SHARED, ctx->handle, 0 ); #endif - pcilib_unlock(ctx->locks.mmap); + + pcilib_unlock_global(ctx); if ((!res)||(res == MAP_FAILED)) { pcilib_error("Failed to mmap data bank %i", bar); diff --git a/pcilib/kmem.c b/pcilib/kmem.c index 469c63a..b1d2c5c 100644 --- a/pcilib/kmem.c +++ b/pcilib/kmem.c @@ -80,7 +80,7 @@ pcilib_kmem_handle_t *pcilib_alloc_kernel_memory(pcilib_t *ctx, pcilib_kmem_type memset(kbuf, 0, sizeof(pcilib_kmem_list_t) + nmemb * sizeof(pcilib_kmem_addr_t)); - err = pcilib_lock(ctx->locks.mmap); + err = pcilib_lock_global(ctx); if (err) { pcilib_error("Error (%i) acquiring mmap lock", err); return NULL; @@ -88,7 +88,7 @@ pcilib_kmem_handle_t *pcilib_alloc_kernel_memory(pcilib_t *ctx, pcilib_kmem_type ret = ioctl( ctx->handle, PCIDRIVER_IOC_MMAP_MODE, PCIDRIVER_MMAP_KMEM ); if (ret) { - pcilib_unlock(ctx->locks.mmap); + pcilib_unlock_global(ctx); pcilib_error("PCIDRIVER_IOC_MMAP_MODE ioctl have failed"); return NULL; } @@ -176,7 +176,7 @@ pcilib_kmem_handle_t *pcilib_alloc_kernel_memory(pcilib_t *ctx, pcilib_kmem_type kbuf->buf.blocks[i].mmap_offset = kh.pa & ctx->page_mask; } - pcilib_unlock(ctx->locks.mmap); + pcilib_unlock_global(ctx); //This is possible in the case of error (nothing is allocated yet) or if buffers are not reused diff --git a/pcilib/lock.c b/pcilib/lock.c index f1cbc56..13e363a 100644 --- a/pcilib/lock.c +++ b/pcilib/lock.c @@ -140,7 +140,10 @@ const char *pcilib_lock_get_name(pcilib_lock_t *lock) { int pcilib_lock_custom(pcilib_lock_t *lock, pcilib_lock_flags_t flags, pcilib_timeout_t timeout) { int err; - if (!lock) return 0; + if (!lock) { + pcilib_error("The null lock pointer is passed to lock function"); + return PCILIB_ERROR_INVALID_ARGUMENT; + } struct timespec tm; diff --git a/pcilib/locking.c b/pcilib/locking.c index ffd05ce..7a32605 100644 --- a/pcilib/locking.c +++ b/pcilib/locking.c @@ -46,11 +46,10 @@ int pcilib_init_locking(pcilib_t* ctx) { } ctx->locks.locking = pcilib_get_lock(ctx, PCILIB_LOCK_FLAG_UNLOCKED, "locking"); - ctx->locks.mmap = pcilib_get_lock(ctx, PCILIB_LOCK_FLAG_UNLOCKED, "mmap"); pcilib_unlock_global(ctx); - if ((!ctx->locks.locking)||(!ctx->locks.mmap)) { + if ((!ctx->locks.locking)) { pcilib_error("Locking subsystem has failed to initialized mandatory global locks"); return PCILIB_ERROR_FAILED; } @@ -62,9 +61,6 @@ int pcilib_init_locking(pcilib_t* ctx) { * this functions destroy all locks and then free the kernel memory allocated for them */ void pcilib_free_locking(pcilib_t *ctx) { - if (ctx->locks.mmap) - pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, ctx->locks.mmap); - if (ctx->locks.locking) pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, ctx->locks.locking); diff --git a/pcilib/locking.h b/pcilib/locking.h index ae2f368..ccacd63 100644 --- a/pcilib/locking.h +++ b/pcilib/locking.h @@ -20,7 +20,7 @@ typedef struct pcilib_locking_s pcilib_locking_t; struct pcilib_locking_s { pcilib_kmem_handle_t *kmem; /**< kmem used to store mutexes */ pcilib_lock_t *locking; /**< lock used while intializing other locks */ - pcilib_lock_t *mmap; /**< lock used to protect mmap operation */ +// pcilib_lock_t *mmap; /**< lock used to protect mmap operation */ }; #ifdef __cplusplus -- cgit v1.2.3 From 6ac0cf129270aef989785e3fbc84abc238a29e92 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 19:13:07 +0200 Subject: Read model from environmental variable if defined --- CMakeLists.txt | 2 +- docs/README | 6 ++++-- pcilib/pci.c | 3 +++ 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 0aeb45a..bbe5a74 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,6 +1,6 @@ project(pcitool) -set(PCILIB_VERSION "0.2.0") +set(PCILIB_VERSION "0.2.1") set(PCILIB_ABI_VERSION "2") cmake_minimum_required(VERSION 2.6) diff --git a/docs/README b/docs/README index ba22acf..335ebef 100644 --- a/docs/README +++ b/docs/README @@ -1,14 +1,16 @@ Supported environmental variables ================================= - PCILIB_PLUGIN_DIR - override path to directory with plugins + PCILIB_MODEL - defines the requested model (is overriden with -m option) + PCILIB_PLUGIN_DIR - override path to directory with plugins - PCILIB_DEBUG_DMA - Enable DMA debugging + PCILIB_DEBUG_DMA - Enable DMA debugging PCILIB_DEBUG_MISSING_EVENTS - Enable debugging of missing events (frames for instance) IPECAMERA_DEBUG_BROKEN_FRAMES - Store broken frames in the specified directory (variable may specify directory) IPECAMERA_DEBUG_RAW_PACKETS - Store all raw packets read from DMA grouped in frames (variable may specify directory) IPECAMERA_DEBUG_RAW_FRAMES - Store all raw frames (variable may specify directory) IPECAMERA_DEBUG_HARDWARE - Produce various debugging information about ipecamera operation IPECAMERA_DEBUG_FRAME_HEADERS - Print headers of each frame + IPECAMERA_DEBUG_API - Print messages while entering and finishing standard API calls PCILIB_BENCHMARK_HARDWARE - Remove all unnecessary software processing (like copying memcpy) to check hardware performance PCILIB_BENCHMARK_STREAMING - Emulate streaming mode while benchmarking DMA engines diff --git a/pcilib/pci.c b/pcilib/pci.c index 512e891..6078a6e 100644 --- a/pcilib/pci.c +++ b/pcilib/pci.c @@ -108,6 +108,9 @@ pcilib_t *pcilib_open(const char *device, const char *model) { size_t i; pcilib_t *ctx = malloc(sizeof(pcilib_t)); + if (!model) + model = getenv("PCILIB_MODEL"); + if (ctx) { memset(ctx, 0, sizeof(pcilib_t)); ctx->pci_cfg_space_fd = -1; -- cgit v1.2.3 From 98fa817f608756b39223ed785b85dd5cace9b096 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 5 Aug 2015 19:19:57 +0200 Subject: Install locking-related headers --- pcilib/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pcilib/CMakeLists.txt b/pcilib/CMakeLists.txt index 0bf0e66..79b15c2 100644 --- a/pcilib/CMakeLists.txt +++ b/pcilib/CMakeLists.txt @@ -16,6 +16,6 @@ install(FILES pcilib.h DESTINATION include ) -install(FILES bar.h kmem.h bank.h register.h dma.h event.h model.h error.h debug.h env.h tools.h export.h version.h +install(FILES bar.h kmem.h locking.h lock.h bank.h register.h dma.h event.h model.h error.h debug.h env.h tools.h export.h version.h DESTINATION include/pcilib ) -- cgit v1.2.3 From 56787b1553bd151eff62741e34fb3b4834073442 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 01:12:55 +0200 Subject: Minor fix for software registers --- protocols/software.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/protocols/software.c b/protocols/software.c index ed9d372..55f62e8 100644 --- a/protocols/software.c +++ b/protocols/software.c @@ -61,6 +61,7 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc lock = pcilib_get_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, "softreg/%s", bank_desc->name); if (!lock) { + pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); pcilib_error("Failed to initialize a lock to protect bank %s with software registers", bank_desc->name); return NULL; } @@ -68,6 +69,7 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc err = pcilib_lock(lock); if (err) { pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); + pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); pcilib_error("Error (%i) obtaining lock on bank %s with software registers", err, bank_desc->name); return NULL; } @@ -77,8 +79,8 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc if (!handle) { pcilib_unlock(lock); pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); - pcilib_error("Allocation of kernel memory for software registers has failed"); pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); + pcilib_error("Allocation of kernel memory for software registers has failed"); return NULL; } @@ -92,8 +94,8 @@ pcilib_register_bank_context_t* pcilib_software_registers_open(pcilib_t *ctx, pc if (reused & PCILIB_KMEM_REUSE_PARTIAL) { pcilib_unlock(lock); pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, lock); - pcilib_error("Inconsistent software registers are found (only part of required buffers is available)"); pcilib_software_registers_close(ctx, (pcilib_register_bank_context_t*)bank_ctx); + pcilib_error("Inconsistent software registers are found (only part of required buffers is available)"); return NULL; } -- cgit v1.2.3 From 63bc897c9444b364b0364bfd04a0c87d9fe29307 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 01:13:17 +0200 Subject: Provide pcilib_try_lock call --- pcilib/lock.c | 4 ++++ pcilib/lock.h | 7 +++++++ 2 files changed, 11 insertions(+) diff --git a/pcilib/lock.c b/pcilib/lock.c index 13e363a..9045ffa 100644 --- a/pcilib/lock.c +++ b/pcilib/lock.c @@ -190,6 +190,10 @@ int pcilib_lock(pcilib_lock_t* lock) { return pcilib_lock_custom(lock, PCILIB_LOCK_FLAGS_DEFAULT, PCILIB_TIMEOUT_INFINITE); } +int pcilib_try_lock(pcilib_lock_t* lock) { + return pcilib_lock_custom(lock, PCILIB_LOCK_FLAGS_DEFAULT, PCILIB_TIMEOUT_IMMEDIATE); +} + /** * this function will unlock the semaphore pointed by lock_ctx. */ diff --git a/pcilib/lock.h b/pcilib/lock.h index 8e1017a..9ffe4cf 100644 --- a/pcilib/lock.h +++ b/pcilib/lock.h @@ -86,6 +86,13 @@ int pcilib_lock_custom(pcilib_lock_t* lock, pcilib_lock_flags_t flags, pcilib_ti */ int pcilib_lock(pcilib_lock_t* lock); +/** + * this function will try to take a lock for the mutex pointed by lock + * @param[in] lock the pointer to the mutex + */ +int pcilib_try_lock(pcilib_lock_t* lock); + + /** * this function will unlock the lock pointed by lock * @param[in] lock the integer that points to the semaphore -- cgit v1.2.3 From 16ecf368c5cb4bebc3f5330bd0f25db06606f862 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 02:27:23 +0200 Subject: In case of problematic locks report the error --- pcitool/cli.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/pcitool/cli.c b/pcitool/cli.c index 5df175d..6551f7b 100644 --- a/pcitool/cli.c +++ b/pcitool/cli.c @@ -2444,9 +2444,9 @@ int ListLocks(pcilib_t *ctx, int verbose) { pcilib_lock_id_t i; if (verbose) - printf("ID Refs Flags Locked Name\n"); + printf("ID Refs Flags Locked Name\n"); else - printf("ID Refs Flags Name\n"); + printf("ID Refs Flags Name\n"); printf("--------------------------------------------------------------------------------\n"); for (i = 0; i < PCILIB_MAX_LOCKS; i++) { @@ -2468,13 +2468,13 @@ int ListLocks(pcilib_t *ctx, int verbose) { switch (err) { case 0: pcilib_unlock(lock); - printf("No "); + printf("No "); break; case PCILIB_ERROR_TIMEOUT: - printf("Yes "); + printf("Yes "); break; default: - printf("Error "); + printf("Err: %3i ", err); } } printf("%s\n", name); -- cgit v1.2.3 From 8b8a2bee0bce0bc9cc78d43f236c6a3d8a0bd2e4 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 02:27:54 +0200 Subject: Fix handling of inconsistent mutexes --- pcilib/lock.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/pcilib/lock.c b/pcilib/lock.c index 9045ffa..03a2377 100644 --- a/pcilib/lock.c +++ b/pcilib/lock.c @@ -174,8 +174,9 @@ int pcilib_lock_custom(pcilib_lock_t *lock, pcilib_lock_flags_t flags, pcilib_ti err = pthread_mutex_consistent(&lock->mutex); if (err) { pcilib_error("Failed to mark mutex as consistent, errno %i", err); + break; } - break; + return 0; case ETIMEDOUT: case EBUSY: return PCILIB_ERROR_TIMEOUT; @@ -203,6 +204,13 @@ void pcilib_unlock(pcilib_lock_t *lock) { if (!lock) return; - if ((err = pthread_mutex_unlock(&lock->mutex)) != 0) - pcilib_error("Can't unlock mutex, errno %i", err); + if ((err = pthread_mutex_unlock(&lock->mutex)) != 0) { + switch (err) { + case EPERM: + pcilib_error("Trying to unlock not locked mutex (%s) or the mutex which was locked by a different thread", lock->name); + break; + default: + pcilib_error("Can't unlock mutex, errno %i", err); + } + } } -- cgit v1.2.3 From d60dd48eec0ef5d7bf2feca9b3f06374a2f444aa Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 04:24:58 +0200 Subject: Protect access to the DMA engine with locks --- pcilib/dma.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++++++++----- pcilib/pci.c | 8 ++++++++ pcilib/pci.h | 3 +++ 3 files changed, 68 insertions(+), 5 deletions(-) diff --git a/pcilib/dma.c b/pcilib/dma.c index f6b8053..be02c21 100644 --- a/pcilib/dma.c +++ b/pcilib/dma.c @@ -61,8 +61,11 @@ pcilib_dma_engine_t pcilib_add_dma_engine(pcilib_t *ctx, pcilib_dma_engine_descr int pcilib_init_dma(pcilib_t *ctx) { int err; pcilib_dma_context_t *dma_ctx = NULL; + const pcilib_dma_description_t *info = &ctx->dma; const pcilib_model_description_t *model_info = pcilib_get_model_description(ctx); + pcilib_dma_engine_t dma; + if (ctx->dma_ctx) return 0; @@ -86,6 +89,24 @@ int pcilib_init_dma(pcilib_t *ctx) { } if (dma_ctx) { + for (dma = 0; info->engines[dma].addr_bits; dma++) { + if (info->engines[dma].direction&PCILIB_DMA_FROM_DEVICE) { + ctx->dma_rlock[dma] = pcilib_get_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, "dma%ir/%s", info->engines[dma].addr, info->name); + if (!ctx->dma_rlock[dma]) break; + } + if (info->engines[dma].direction&PCILIB_DMA_TO_DEVICE) { + ctx->dma_wlock[dma] = pcilib_get_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, "dma%iw/%s", info->engines[dma].addr, info->name); + if (!ctx->dma_wlock[dma]) break; + } + } + + if (info->engines[dma].addr_bits) { + if (ctx->dma.api->free) + ctx->dma.api->free(dma_ctx); + pcilib_error("Failed to intialize DMA locks"); + return PCILIB_ERROR_FAILED; + } + dma_ctx->pcilib = ctx; // DS: parameters? ctx->dma_ctx = dma_ctx; @@ -105,11 +126,11 @@ int pcilib_start_dma(pcilib_t *ctx, pcilib_dma_engine_t dma, pcilib_dma_flags_t pcilib_error("DMA Engine is not configured in the current model"); return PCILIB_ERROR_NOTAVAILABLE; } - + if (!info->api->start_dma) { return 0; } - + return info->api->start_dma(ctx->dma_ctx, dma, flags); } @@ -201,6 +222,7 @@ static int pcilib_dma_skip_callback(void *arg, pcilib_dma_flags_t flags, size_t } int pcilib_stream_dma(pcilib_t *ctx, pcilib_dma_engine_t dma, uintptr_t addr, size_t size, pcilib_dma_flags_t flags, pcilib_timeout_t timeout, pcilib_dma_callback_t cb, void *cbattr) { + int err; const pcilib_dma_description_t *info = pcilib_get_dma_description(ctx); if (!info) { pcilib_error("DMA is not supported by the device"); @@ -228,7 +250,21 @@ int pcilib_stream_dma(pcilib_t *ctx, pcilib_dma_engine_t dma, uintptr_t addr, si return PCILIB_ERROR_NOTSUPPORTED; } - return info->api->stream(ctx->dma_ctx, dma, addr, size, flags, timeout, cb, cbattr); + err = pcilib_try_lock(ctx->dma_rlock[dma]); + if (err) { + if ((err == PCILIB_ERROR_BUSY)||(err == PCILIB_ERROR_TIMEOUT)) + pcilib_error("DMA engine (%i) is busy", dma); + else + pcilib_error("Error (%i) locking DMA engine (%i)", err, dma); + + return err; + } + + err = info->api->stream(ctx->dma_ctx, dma, addr, size, flags, timeout, cb, cbattr); + + pcilib_unlock(ctx->dma_rlock[dma]); + + return err; } int pcilib_read_dma_custom(pcilib_t *ctx, pcilib_dma_engine_t dma, uintptr_t addr, size_t size, pcilib_dma_flags_t flags, pcilib_timeout_t timeout, void *buf, size_t *read_bytes) { @@ -278,6 +314,8 @@ int pcilib_skip_dma(pcilib_t *ctx, pcilib_dma_engine_t dma) { int pcilib_push_dma(pcilib_t *ctx, pcilib_dma_engine_t dma, uintptr_t addr, size_t size, pcilib_dma_flags_t flags, pcilib_timeout_t timeout, void *buf, size_t *written) { + int err; + const pcilib_dma_description_t *info = pcilib_get_dma_description(ctx); if (!info) { pcilib_error("DMA is not supported by the device"); @@ -304,8 +342,22 @@ int pcilib_push_dma(pcilib_t *ctx, pcilib_dma_engine_t dma, uintptr_t addr, size pcilib_error("The selected engine (%i) is C2S-only and does not support writes", dma); return PCILIB_ERROR_NOTSUPPORTED; } - - return info->api->push(ctx->dma_ctx, dma, addr, size, flags, timeout, buf, written); + + err = pcilib_try_lock(ctx->dma_wlock[dma]); + if (err) { + if (err == PCILIB_ERROR_BUSY) + pcilib_error("DMA engine (%i) is busy", dma); + else + pcilib_error("Error (%i) locking DMA engine (%i)", err, dma); + + return err; + } + + err = info->api->push(ctx->dma_ctx, dma, addr, size, flags, timeout, buf, written); + + pcilib_unlock(ctx->dma_wlock[dma]); + + return err; } diff --git a/pcilib/pci.c b/pcilib/pci.c index 6078a6e..4351c1e 100644 --- a/pcilib/pci.c +++ b/pcilib/pci.c @@ -337,6 +337,7 @@ void pcilib_close(pcilib_t *ctx) { pcilib_bar_t i; if (ctx) { + pcilib_dma_engine_t dma; const pcilib_model_description_t *model_info = pcilib_get_model_description(ctx); const pcilib_event_api_description_t *eapi = model_info->api; const pcilib_dma_api_description_t *dapi = ctx->dma.api; @@ -344,6 +345,13 @@ void pcilib_close(pcilib_t *ctx) { if ((eapi)&&(eapi->free)) eapi->free(ctx->event_ctx); if ((dapi)&&(dapi->free)) dapi->free(ctx->dma_ctx); + for (dma = 0; dma < PCILIB_MAX_DMA_ENGINES; dma++) { + if (ctx->dma_rlock[dma]) + pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, ctx->dma_rlock[dma]); + if (ctx->dma_wlock[dma]) + pcilib_return_lock(ctx, PCILIB_LOCK_FLAGS_DEFAULT, ctx->dma_wlock[dma]); + } + pcilib_free_register_banks(ctx); if (ctx->register_ctx) diff --git a/pcilib/pci.h b/pcilib/pci.h index c97f753..51bb352 100644 --- a/pcilib/pci.h +++ b/pcilib/pci.h @@ -71,6 +71,9 @@ struct pcilib_s { pcilib_dma_context_t *dma_ctx; /**< DMA context */ pcilib_context_t *event_ctx; /**< Implmentation context */ + pcilib_lock_t *dma_rlock[PCILIB_MAX_DMA_ENGINES]; /**< Per-engine locks to serialize streaming and read operations */ + pcilib_lock_t *dma_wlock[PCILIB_MAX_DMA_ENGINES]; /**< Per-engine locks to serialize write operations */ + struct pcilib_locking_s locks; /**< Context of locking subsystem */ #ifdef PCILIB_FILE_IO -- cgit v1.2.3 From ccbad1cc8eca5f361427dd0ceca15cfa6d68a6d9 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 05:27:21 +0200 Subject: Detect if IPEDMA operates in streaming mode --- dma/ipe.c | 73 ++++++++++++++++++------------------------------------- dma/ipe.h | 4 ++- dma/ipe_private.h | 7 ++++-- 3 files changed, 32 insertions(+), 52 deletions(-) diff --git a/dma/ipe.c b/dma/ipe.c index 814bc77..2ed3e05 100644 --- a/dma/ipe.c +++ b/dma/ipe.c @@ -43,14 +43,18 @@ pcilib_dma_context_t *dma_ipe_init(pcilib_t *pcilib, const char *model, const vo ctx->dma_bank = model_info->banks + dma_bank; ctx->base_addr = pcilib_resolve_register_address(pcilib, ctx->dma_bank->bar, ctx->dma_bank->read_addr); + RD(IPEDMA_REG_PCIE_GEN, value); + #ifdef IPEDMA_ENFORCE_64BIT_MODE ctx->mode64 = 1; #else /* IPEDMA_ENFORCE_64BIT_MODE */ // According to Lorenzo, some gen2 boards have problems with 64-bit addressing. Therefore, we only enable it for gen3 boards unless enforced - RD(IPEDMA_REG_PCIE_GEN, value); - if (value > 2) ctx->mode64 = 1; - else ctx->mode64 = 0; + if ((value&IPEDMA_MASK_PCIE_GEN) > 2) ctx->mode64 = 1; #endif /* IPEDMA_ENFORCE_64BIT_MODE */ + +#ifdef IPEDMA_STREAMING_MODE + if (value&IPEDMA_MASK_STREAMING_MODE) ctx->streaming = 1; +#endif /* IPEDMA_STREAMING_MODE */ } return (pcilib_dma_context_t*)ctx; @@ -114,15 +118,16 @@ int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dm if ((reuse_desc & PCILIB_KMEM_REUSE_PERSISTENT) == 0) pcilib_warning("Lost DMA buffers are found (non-persistent mode), reinitializing..."); else if ((reuse_desc & PCILIB_KMEM_REUSE_HARDWARE) == 0) pcilib_warning("Lost DMA buffers are found (missing HW reference), reinitializing..."); else { -#ifndef IPEDMA_BUG_DMARD -# ifndef IPEDMA_STREAMING_MODE - RD(IPEDMA_REG_PAGE_COUNT, value); - - if (value != IPEDMA_DMA_PAGES) pcilib_warning("Inconsistent DMA buffers are found (Number of allocated buffers (%lu) does not match current request (%lu)), reinitializing...", value + 1, IPEDMA_DMA_PAGES); - else -# endif /* IPEDMA_STREAMING_MODE */ -#endif /* IPEDMA_BUG_DMARD */ + if (ctx->streaming) preserve = 1; + else { + RD(IPEDMA_REG_PAGE_COUNT, value); + + if (value != IPEDMA_DMA_PAGES) + pcilib_warning("Inconsistent DMA buffers are found (Number of allocated buffers (%lu) does not match current request (%lu)), reinitializing...", value + 1, IPEDMA_DMA_PAGES); + else + preserve = 1; + } } } } else pcilib_warning("Inconsistent DMA buffers (modes of ring and page buffers does not match), reinitializing...."); @@ -136,12 +141,6 @@ int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dm ctx->preserve = 1; // Detect the current state of DMA engine -#ifdef IPEDMA_BUG_DMARD - FILE *f = fopen("/tmp/pcitool_lastread", "r"); - if (!f) pcilib_error("Can't read current status"); - fread(&value, 1, sizeof(pcilib_register_value_t), f); - fclose(f); -#else /* IPEDMA_BUG_DMARD */ RD(IPEDMA_REG_LAST_READ, value); // Numbered from 1 in FPGA # ifdef IPEDMA_BUG_LAST_READ @@ -150,7 +149,6 @@ int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dm # else /* IPEDMA_BUG_LAST_READ */ value--; # endif /* IPEDMA_BUG_LAST_READ */ -#endif /* IPEDMA_BUG_DMARD */ ctx->last_read = value; } else { @@ -166,12 +164,10 @@ int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dm WR(IPEDMA_REG_RESET, 0x0); usleep(IPEDMA_RESET_DELAY); -#ifndef IPEDMA_BUG_DMARD // Verify PCIe link status RD(IPEDMA_REG_RESET, value); if ((value != 0x14031700)&&(value != 0x14021700)) pcilib_warning("PCIe is not ready, code is %lx", value); -#endif /* IPEDMA_BUG_DMARD */ // Enable 64 bit addressing and configure TLP and PACKET sizes (40 bit mode can be used with big pre-allocated buffers later) if (ctx->mode64) address64 = 0x8000 | (0<<24); @@ -224,14 +220,6 @@ int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dm WR(IPEDMA_REG_CONTROL, 0x1); ctx->last_read = IPEDMA_DMA_PAGES - 1; - -#ifdef IPEDMA_BUG_DMARD - FILE *f = fopen("/tmp/pcitool_lastread", "w"); - if (!f) pcilib_error("Can't write current status"); - value = ctx->last_read; - fwrite(&value, 1, sizeof(pcilib_register_value_t), f); - fclose(f); -#endif /* IPEDMA_BUG_DMARD */ } ctx->last_read_addr = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, pages, ctx->last_read); @@ -415,10 +403,6 @@ int dma_ipe_stream_read(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, uin pcilib_dma_flags_t packet_flags = PCILIB_DMA_FLAG_EOP; size_t nodata_sleep; -#ifdef IPEDMA_BUG_DMARD - pcilib_register_value_t value; -#endif /* IPEDMA_BUG_DMARD */ - switch (sched_getscheduler(0)) { case SCHED_FIFO: case SCHED_RR: @@ -511,16 +495,16 @@ int dma_ipe_stream_read(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, uin // pcilib_kmem_sync_block(ctx->dmactx.pcilib, ctx->pages, PCILIB_KMEM_SYNC_TODEVICE, cur_read); // Return buffer into the DMA pool when processed -#ifdef IPEDMA_STREAMING_MODE - uintptr_t buf_ba = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, ctx->pages, cur_read); - WR(IPEDMA_REG_PAGE_ADDR, buf_ba); + if (ctx->streaming) { + uintptr_t buf_ba = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, ctx->pages, cur_read); + WR(IPEDMA_REG_PAGE_ADDR, buf_ba); # ifdef IPEDMA_STREAMING_CHECKS - pcilib_register_value_t streaming_status; - RD(IPEDMA_REG_STREAMING_STATUS, streaming_status); - if (streaming_status) - pcilib_error("Invalid status (0x%lx) adding a DMA buffer into the queue", streaming_status); + pcilib_register_value_t streaming_status; + RD(IPEDMA_REG_STREAMING_STATUS, streaming_status); + if (streaming_status) + pcilib_error("Invalid status (0x%lx) adding a DMA buffer into the queue", streaming_status); # endif /* IPEDMA_STREAMING_MODE */ -#endif /* IPEDMA_STREAMING_MODE */ + } // Numbered from 1 #ifdef IPEDMA_BUG_LAST_READ @@ -537,15 +521,6 @@ int dma_ipe_stream_read(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, uin ctx->last_read = cur_read; ctx->last_read_addr = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, ctx->pages, cur_read); - -#ifdef IPEDMA_BUG_DMARD - FILE *f = fopen("/tmp/pcitool_lastread", "w"); - if (!f) pcilib_error("Can't write current status"); - value = cur_read; - fwrite(&value, 1, sizeof(pcilib_register_value_t), f); - fclose(f); -#endif /* IPEDMA_BUG_DMARD */ - } while (ret); return 0; diff --git a/dma/ipe.h b/dma/ipe.h index 76be4f4..7f32d61 100644 --- a/dma/ipe.h +++ b/dma/ipe.h @@ -65,7 +65,9 @@ static const pcilib_register_description_t ipe_dma_registers[] = { {0x000C, 24, 8, 0, 0xFFFFFFFF, PCILIB_REGISTER_RW , PCILIB_REGISTER_BITS, PCILIB_REGISTER_BANK_DMA, "mwr_up_addr", "Upper address for 64 bit memory addressing"}, {0x0010, 0, 32, 0, 0x00000000, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "mwr_count", "Write DMA TLP Count"}, {0x0014, 0, 32, 0, 0x00000000, PCILIB_REGISTER_RW , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "mwr_pattern", "DMA generator data pattern"}, - {0x0018, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "pcie_gen", "PCIe version 2/3 depending on the used XILINX core"}, + {0x0018, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "dma_mode_flags", "DMA operation mode"}, + {0x0018, 0, 4, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_BITS, PCILIB_REGISTER_BANK_DMA, "pcie_gen", "PCIe version 2/3 depending on the used XILINX core"}, + {0x0018, 4, 1, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_BITS, PCILIB_REGISTER_BANK_DMA, "streaming_dma", "Streaming mode (enabled/disabled)"}, {0x0028, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "mwr_perf", "MWR Performance"}, {0x003C, 0, 32, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_STANDARD, PCILIB_REGISTER_BANK_DMA, "cfg_lnk_width", "Negotiated and max width of PCIe Link"}, {0x003C, 0, 6, 0, 0x00000000, PCILIB_REGISTER_R , PCILIB_REGISTER_BITS, PCILIB_REGISTER_BANK_DMA, "cfg_cap_max_lnk_width", "Max link width"}, diff --git a/dma/ipe_private.h b/dma/ipe_private.h index 937e202..fdebec0 100644 --- a/dma/ipe_private.h +++ b/dma/ipe_private.h @@ -8,7 +8,7 @@ #define IPEDMA_MAX_TLP_SIZE 256 /**< Defines maximum TLP in bytes supported by device */ //#define IPEDMA_TLP_SIZE 128 /**< If set, enforces the specified TLP size */ -//#define IPEDMA_STREAMING_MODE /**< Enables streaming DMA operation mode instead of ring-buffer, the page is written once and forgotten and need to be pushed in queue again */ +#define IPEDMA_STREAMING_MODE /**< Enables streaming DMA operation mode instead of ring-buffer, the page is written once and forgotten and need to be pushed in queue again */ #define IPEDMA_STREAMING_CHECKS /**< Enables status checks in streaming mode (it will cause performance penalty) */ #define IPEDMA_PAGE_SIZE 4096 #define IPEDMA_DMA_PAGES 1024 /**< number of DMA pages in the ring buffer to allocate */ @@ -17,7 +17,6 @@ #define IPEDMA_DESCRIPTOR_ALIGNMENT 64 -//#define IPEDMA_BUG_DMARD /**< No register read during DMA transfer */ //#define IPEDMA_BUG_LAST_READ /**< We should forbid writting the second last available DMA buffer (the last is forbidden by design) */ //#define IPEDMA_DETECT_PACKETS /**< Using empty_deceted flag */ #define IPEDMA_SUPPORT_EMPTY_DETECTED /**< Avoid waiting for data when empty_detected flag is set in hardware */ @@ -39,6 +38,9 @@ #define IPEDMA_REG_UPDATE_THRESHOLD 0x60 #define IPEDMA_REG_STREAMING_STATUS 0x68 +#define IPEDMA_MASK_PCIE_GEN 0xF +#define IPEDMA_MASK_STREAMING_MODE 0x10 + #define WR(addr, value) { *(uint32_t*)(ctx->base_addr + addr) = value; } #define RD(addr, value) { value = *(uint32_t*)(ctx->base_addr + addr); } @@ -62,6 +64,7 @@ struct ipe_dma_s { int reused; /**< indicates that DMA was found intialized, buffers were reused, and no additional initialization is needed */ int preserve; /**< indicates that DMA should not be stopped during clean-up */ int mode64; /**< indicates 64-bit operation mode */ + int streaming; /**< indicates if DMA is operating in streaming or ring-buffer mode */ pcilib_kmem_handle_t *desc; /**< in-memory status descriptor written by DMA engine upon operation progess */ pcilib_kmem_handle_t *pages; /**< collection of memory-locked pages for DMA operation */ -- cgit v1.2.3 From 61d520876b16acdd97f32467e2af5bd531a516af Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 16:45:00 +0200 Subject: In IPEDMA streaming mode put aside a single empty buffer to distinguish between completely empty and full states of kernel ring buffer --- dma/ipe.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/dma/ipe.c b/dma/ipe.c index 2ed3e05..3e46cb3 100644 --- a/dma/ipe.c +++ b/dma/ipe.c @@ -71,7 +71,7 @@ void dma_ipe_free(pcilib_dma_context_t *vctx) { int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dma_flags_t flags) { - size_t i; + size_t i, num_pages; ipe_dma_t *ctx = (ipe_dma_t*)vctx; @@ -203,7 +203,13 @@ int dma_ipe_start(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, pcilib_dm // Instructing DMA engine that writting should start from the first DMA page *last_written_addr_ptr = 0; - for (i = 0; i < IPEDMA_DMA_PAGES; i++) { + // In ring buffer mode, the hardware taking care to preserve an empty buffer to help distinguish between + // completely empty and completely full cases. In streaming mode, it is our responsibility to track this + // information. Therefore, we always keep the last buffer free + num_pages = IPEDMA_DMA_PAGES; + if (ctx->streaming) num_pages--; + + for (i = 0; i < num_pages; i++) { uintptr_t bus_addr_check, bus_addr = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, pages, i); WR(IPEDMA_REG_PAGE_ADDR, bus_addr); if (bus_addr%4096) printf("Bad address %lu: %lx\n", i, bus_addr); @@ -496,7 +502,12 @@ int dma_ipe_stream_read(pcilib_dma_context_t *vctx, pcilib_dma_engine_t dma, uin // Return buffer into the DMA pool when processed if (ctx->streaming) { - uintptr_t buf_ba = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, ctx->pages, cur_read); + size_t last_free; + // We always keep 1 buffer free to distinguish between completely full and empty cases + if (cur_read) last_free = cur_read - 1; + else last_free = IPEDMA_DMA_PAGES - 1; + + uintptr_t buf_ba = pcilib_kmem_get_block_ba(ctx->dmactx.pcilib, ctx->pages, last_free); WR(IPEDMA_REG_PAGE_ADDR, buf_ba); # ifdef IPEDMA_STREAMING_CHECKS pcilib_register_value_t streaming_status; -- cgit v1.2.3 From 81de1a078b5c54c81b869564d33ed3e025bb695e Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 16:46:05 +0200 Subject: Fix compilation of the driver --- pcilib/kmem.h | 2 -- pcilib/locking.c | 1 + 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/pcilib/kmem.h b/pcilib/kmem.h index 9cd1971..3dff625 100644 --- a/pcilib/kmem.h +++ b/pcilib/kmem.h @@ -1,8 +1,6 @@ #ifndef _PCILIB_KMEM_H #define _PCILIB_KMEM_H -#include - typedef struct pcilib_s pcilib_t; typedef struct pcilib_kmem_list_s pcilib_kmem_list_t; diff --git a/pcilib/locking.c b/pcilib/locking.c index 7a32605..f384ca4 100644 --- a/pcilib/locking.c +++ b/pcilib/locking.c @@ -4,6 +4,7 @@ #include #include #include +#include #include #include "locking.h" -- cgit v1.2.3 From 38fdfd8b69ba5ae747915bbdbb3066d29611437d Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Thu, 6 Aug 2015 18:03:59 +0200 Subject: Disable STREAMING_CHECKS for better performance --- dma/ipe_benchmark.c | 2 +- dma/ipe_private.h | 2 +- docs/ToDo | 16 ++++++---------- 3 files changed, 8 insertions(+), 12 deletions(-) diff --git a/dma/ipe_benchmark.c b/dma/ipe_benchmark.c index f8e0a7e..3ce04d3 100644 --- a/dma/ipe_benchmark.c +++ b/dma/ipe_benchmark.c @@ -166,7 +166,7 @@ double dma_ipe_benchmark(pcilib_dma_context_t *vctx, pcilib_dma_engine_addr_t dm for (bytes = 0; bytes < size; bytes += rbytes) { err = read_dma(ctx->dmactx.pcilib, 0, addr, size - bytes, PCILIB_DMA_FLAG_MULTIPACKET, PCILIB_DMA_TIMEOUT, buf + bytes, &rbytes); if (err) { - pcilib_error("Can't read data from DMA, error %i", err); + pcilib_error("Can't read data from DMA (iteration: %zu, offset: %zu), error %i", iter, bytes, err); return -1; } } diff --git a/dma/ipe_private.h b/dma/ipe_private.h index fdebec0..5054a58 100644 --- a/dma/ipe_private.h +++ b/dma/ipe_private.h @@ -9,7 +9,7 @@ //#define IPEDMA_TLP_SIZE 128 /**< If set, enforces the specified TLP size */ #define IPEDMA_STREAMING_MODE /**< Enables streaming DMA operation mode instead of ring-buffer, the page is written once and forgotten and need to be pushed in queue again */ -#define IPEDMA_STREAMING_CHECKS /**< Enables status checks in streaming mode (it will cause performance penalty) */ +//#define IPEDMA_STREAMING_CHECKS /**< Enables status checks in streaming mode (it will cause _significant_ performance penalty, max ~ 2 GB/s) */ #define IPEDMA_PAGE_SIZE 4096 #define IPEDMA_DMA_PAGES 1024 /**< number of DMA pages in the ring buffer to allocate */ #define IPEDMA_DMA_PROGRESS_THRESHOLD 1 /**< how many pages the DMA engine should fill before reporting progress */ diff --git a/docs/ToDo b/docs/ToDo index 4c2c783..f9f14e6 100644 --- a/docs/ToDo +++ b/docs/ToDo @@ -1,23 +1,19 @@ High Priority (we would need it for IPE Camera) ============= - 1. Serialize access to the registers across applications - 2. Protect kmem_entries in the driver using spinlock - 3. Protect mmap operation (multiple kernel calls) with some locking mechanism - 4. Allow overriding of registers and banks (performance?). + 1. Allow overriding of registers and banks (performance?). Normal Priority (it would make just few things a bit easier) =============== - 1. Implement software registers (stored in kernel-memory) - 2. Implement pcilib_configure_autotrigger - 3. Provide OR and AND operations on registers in cli - 4. Support writting a data from a binary file in cli + 1. Implement pcilib_configure_autotrigger + 2. Provide OR and AND operations on registers in cli + 3. Support writting a data from a binary file in cli Low Priority (only as generalization for other projects) ============ 1. XML configurations describing registers (and DMA engines?) - 2. Access register/bank lookups using hash tables + 2. Access register/bank/lock lookups using hash tables 3. Support for Network Registers and Network DMA 4. Define a syntax for register dependencies / delays (?) 5. Use pthread_condition_t instead of polling 6. Support FIFO reads/writes from/to registers - + 7. We managed kmem performance using next kmem prediction, but it is still wise to provide additionally a binary tree for faster search -- cgit v1.2.3 From 55783eb24e983786056f4ba7925aa23fca13c79e Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 12 Aug 2015 20:25:35 +0200 Subject: Fix support of older systems: Remove C11 derective and add rt library to the link list --- CMakeLists.txt | 6 ++++-- pcilib/CMakeLists.txt | 2 +- pcitool/CMakeLists.txt | 2 +- 3 files changed, 6 insertions(+), 4 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index bbe5a74..337a366 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -12,6 +12,8 @@ set(DISABLE_PCITOOL FALSE CACHE BOOL "Build only the library") find_package(PkgConfig REQUIRED) find_package(Threads REQUIRED) +set(EXTRA_SYSTEM_LIBS -lrt) + #Check in sibling directory if (NOT DISABLE_PCITOOL) pkg_check_modules(FASTWRITER fastwriter REQUIRED) @@ -19,8 +21,8 @@ endif (NOT DISABLE_PCITOOL) check_include_files(stdatomic.h HAVE_STDATOMIC_H) -add_definitions("-fPIC --std=c11 -Wall -O2 -gdwarf-2 -g3") -#add_definitions("-fPIC --std=c11 -Wall -O2") +add_definitions("-fPIC --std=c99 -Wall -O2 -gdwarf-2 -g3") +#add_definitions("-fPIC --std=c99 -Wall -O2") include(cmake/version.cmake) VERSION_TO_VARS(${PCILIB_VERSION} PCILIB_VERSION_MAJOR PCILIB_VERSION_MINOR PCILIB_VERSION_MICRO) diff --git a/pcilib/CMakeLists.txt b/pcilib/CMakeLists.txt index 79b15c2..be59cb7 100644 --- a/pcilib/CMakeLists.txt +++ b/pcilib/CMakeLists.txt @@ -5,7 +5,7 @@ include_directories( set(HEADERS pcilib.h pci.h export.h bar.h fifo.h model.h bank.h register.h kmem.h irq.h locking.h lock.h dma.h event.h plugin.h tools.h error.h debug.h env.h version.h config.h) add_library(pcilib SHARED pci.c export.c bar.c fifo.c model.c bank.c register.c kmem.c irq.c locking.c lock.c dma.c event.c plugin.c tools.c error.c debug.c env.c ) -target_link_libraries(pcilib dma protocols ${CMAKE_THREAD_LIBS_INIT} ${UFODECODE_LIBRARIES} ${CMAKE_DL_LIBS}) +target_link_libraries(pcilib dma protocols ${CMAKE_THREAD_LIBS_INIT} ${UFODECODE_LIBRARIES} ${CMAKE_DL_LIBS} ${EXTRA_SYSTEM_LIBS}) add_dependencies(pcilib dma protocols) install(TARGETS pcilib diff --git a/pcitool/CMakeLists.txt b/pcitool/CMakeLists.txt index 1b21041..fc88de6 100644 --- a/pcitool/CMakeLists.txt +++ b/pcitool/CMakeLists.txt @@ -15,7 +15,7 @@ if (NOT DISABLE_PCITOOL) add_dependencies(pci pcitool) target_link_libraries(pci pcilib ${FASTWRITER_LIBRARIES}) set_target_properties(pci PROPERTIES - LINK_FLAGS ${CMAKE_THREAD_LIBS_INIT} + LINK_FLAGS "${CMAKE_THREAD_LIBS_INIT} ${EXTRA_SYSTEM_LIBS}" ) #set_target_properties(pci PROPERTIES -- cgit v1.2.3 From 5001621745651443d3825a30a92897cac7f1ec53 Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Wed, 12 Aug 2015 20:57:37 +0200 Subject: Do not fail if PCI configuration is not fully available to unprivileged user --- pcilib/pci.c | 11 ++++++++--- pcilib/pci.h | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/pcilib/pci.c b/pcilib/pci.c index 4351c1e..62ac92d 100644 --- a/pcilib/pci.c +++ b/pcilib/pci.c @@ -425,11 +425,16 @@ static int pcilib_update_pci_configuration_space(pcilib_t *ctx) { } size = read(ctx->pci_cfg_space_fd, ctx->pci_cfg_space_cache, 256); - if (size != 256) { - pcilib_error("Failed to read PCI configuration from sysfs"); + if (size < 64) { + if (size <= 0) + pcilib_error("Failed to read PCI configuration from sysfs, errno: %i", errno); + else + pcilib_error("Failed to read PCI configuration from sysfs, only %zu bytes read (expected at least 64)", size); return PCILIB_ERROR_FAILED; } + ctx->pci_cfg_space_size = size; + return 0; } @@ -452,7 +457,7 @@ static uint32_t *pcilib_get_pci_capabilities(pcilib_t *ctx, int cap_id) { cap = ctx->pci_cfg_space_cache[(0x34>>2)]; cap_offset = cap&0xFC; - while ((cap_offset)&&(cap_offset < 256)) { + while ((cap_offset)&&(cap_offset < ctx->pci_cfg_space_size)) { cap = ctx->pci_cfg_space_cache[cap_offset>>2]; if ((cap&0xFF) == cap_id) return &ctx->pci_cfg_space_cache[cap_offset>>2]; diff --git a/pcilib/pci.h b/pcilib/pci.h index 51bb352..340abd3 100644 --- a/pcilib/pci.h +++ b/pcilib/pci.h @@ -42,6 +42,7 @@ struct pcilib_s { int pci_cfg_space_fd; /**< File descriptor linking to PCI configuration space in sysfs */ uint32_t pci_cfg_space_cache[64]; /**< Cached PCI configuration space */ + size_t pci_cfg_space_size; /**< Size of the cached PCI configuration space, sometimes not fully is available for unpriveledged user */ const uint32_t *pcie_capabilities; /**< PCI Capbility structure (just a pointer at appropriate place in the pci_cfg_space) */ int reg_bar_mapped; /**< Indicates that all BARs used to access registers are mapped */ -- cgit v1.2.3 From c8fe6103fa27308822592a8c3e7dc883fd299c3a Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Fri, 14 Aug 2015 01:36:38 +0200 Subject: Improve bad events counting --- pcitool/cli.c | 55 ++++++++++++++++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/pcitool/cli.c b/pcitool/cli.c index 6551f7b..57801ab 100644 --- a/pcitool/cli.c +++ b/pcitool/cli.c @@ -1283,7 +1283,7 @@ typedef struct { int verbose; pcilib_timeout_t timeout; size_t run_time; - size_t trigger_time; + size_t trigger_time; size_t max_triggers; pcilib_event_flags_t flags; FORMAT format; @@ -1295,17 +1295,19 @@ typedef struct { volatile int run_flag; volatile int writing_flag; - struct timeval first_frame; + struct timeval first_frame; struct timeval last_frame; - size_t last_num; - + size_t last_num, last_id; + size_t trigger_failed; size_t trigger_count; - size_t event_count; - size_t incomplete_count; - size_t broken_count; - size_t missing_count; - size_t storage_count; + size_t event_count; /**< Total number of events (including bad ones, but excluding events expected, but not reported by hardware) */ + size_t incomplete_count; /**< Broken events, we even can't extract appropriate block of raw data */ + size_t broken_count; /**< Broken events, error while decoding in the requested format */ + size_t empty_count; /**< Broken events, no associated data or unknown */ + size_t missing_count; /**< Missing events, not received from the hardware */ + size_t dropped_count; /**< Missing events, dropped due slow decoding/copying performance */ + size_t storage_count; /**< Missing events, dropped due to slowness of the storage subsystem */ struct timeval start_time; struct timeval stop_time; @@ -1333,12 +1335,13 @@ int GrabCallback(pcilib_event_id_t event_id, pcilib_event_info_t *info, void *us ctx->missing_count += missing_count; #ifdef PCILIB_DEBUG_MISSING_EVENTS if (missing_count) - pcilib_debug(MISSING_EVENTS, "%zu missing events between %zu and %zu", missing_count, ctx->last_num, info->seqnum); + pcilib_debug(MISSING_EVENTS, "%zu missing events between %zu (hwid: %zu) and %zu (hwid: %zu)", missing_count, ctx->last_id, ctx->last_num, event_id, info->seqnum); #endif /* PCILIB_DEBUG_MISSING_EVENTS */ } ctx->last_num = info->seqnum; - + ctx->last_id = event_id; + if (info->flags&PCILIB_EVENT_INFO_FLAG_BROKEN) { ctx->incomplete_count++; return PCILIB_STREAMING_CONTINUE; @@ -1351,9 +1354,19 @@ int GrabCallback(pcilib_event_id_t event_id, pcilib_event_info_t *info, void *us default: data = pcilib_get_data(handle, event_id, PCILIB_EVENT_RAW_DATA, &size); } - + if (!data) { - ctx->broken_count++; + int err = (int)size; + switch (err) { + case PCILIB_ERROR_OVERWRITTEN: + ctx->dropped_count++; + break; + case PCILIB_ERROR_INVALID_DATA: + ctx->broken_count++; + break; + default: + ctx->empty_count++; + } return PCILIB_STREAMING_CONTINUE; } @@ -1386,7 +1399,7 @@ int GrabCallback(pcilib_event_id_t event_id, pcilib_event_info_t *info, void *us err = pcilib_return_data(handle, event_id, ctx->data, data); if (err) { - ctx->missing_count++; + ctx->dropped_count++; fastwriter_cancel(ctx->writer); return PCILIB_STREAMING_CONTINUE; } @@ -1498,7 +1511,7 @@ void GrabStats(GRABContext *ctx, struct timeval *end_time) { total = ctx->event_count + ctx->missing_count; } - good = ctx->event_count - ctx->broken_count - ctx->incomplete_count - ctx->storage_count; + good = ctx->event_count - ctx->broken_count - ctx->incomplete_count - ctx->storage_count - ctx->empty_count - ctx->dropped_count; if (ctx->event_count > 1) { fps = (ctx->event_count - 1) / (1.*fps_duration/1000000); @@ -1554,11 +1567,11 @@ void GrabStats(GRABContext *ctx, struct timeval *end_time) { printf("Good: "); PrintNumber(good); printf(", Dropped: "); - PrintNumber(ctx->storage_count); + PrintNumber(ctx->dropped_count + ctx->storage_count); printf(", Bad: "); - PrintNumber(ctx->incomplete_count); + PrintNumber(ctx->incomplete_count + ctx->broken_count); printf(", Empty: "); - PrintNumber(ctx->broken_count); + PrintNumber(ctx->empty_count); } printf(", Lost: "); PrintNumber(ctx->missing_count); @@ -1573,11 +1586,11 @@ void GrabStats(GRABContext *ctx, struct timeval *end_time) { printf("Good: "); PrintPercent(good, total); printf("%% Dropped: "); - PrintPercent(ctx->storage_count, total); + PrintPercent(ctx->dropped_count + ctx->storage_count, total); printf("%% Bad: "); - PrintPercent(ctx->incomplete_count, total); + PrintPercent(ctx->incomplete_count + ctx->broken_count, total); printf("%% Empty: "); - PrintPercent(ctx->broken_count, total); + PrintPercent(ctx->empty_count, total); } printf("%% Lost: "); -- cgit v1.2.3 From e256f2f50cc579a89e8b9460e192097e99e36ccb Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Tue, 18 Aug 2015 07:02:32 +0200 Subject: Add support for kernel 4.1 --- driver/Makefile | 2 +- driver/base.h | 10 +++++----- pcitool/CMakeLists.txt | 2 +- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/driver/Makefile b/driver/Makefile index e353228..3a242dd 100644 --- a/driver/Makefile +++ b/driver/Makefile @@ -3,7 +3,7 @@ obj-m := pciDriver.o pciDriver-objs := base.o int.o umem.o kmem.o sysfs.o ioctl.o KERNELDIR ?= /lib/modules/$(shell uname -r)/build -INSTALLDIR ?= /lib/modules/$(shell uname -r)/extra +INSTALLDIR ?= /lib/modules/$(shell uname -r)/kernel/extra PWD := $(shell pwd) EXTRA_CFLAGS += -I$(M)/.. diff --git a/driver/base.h b/driver/base.h index 9384e2d..b976d3f 100644 --- a/driver/base.h +++ b/driver/base.h @@ -68,14 +68,14 @@ static dev_t pcidriver_devt; static atomic_t pcidriver_deviceCount; /* Sysfs attributes */ -static DEVICE_ATTR(mmap_mode, (S_IRUGO | S_IWUGO), pcidriver_show_mmap_mode, pcidriver_store_mmap_mode); -static DEVICE_ATTR(mmap_area, (S_IRUGO | S_IWUGO), pcidriver_show_mmap_area, pcidriver_store_mmap_area); +static DEVICE_ATTR(mmap_mode, 0664, pcidriver_show_mmap_mode, pcidriver_store_mmap_mode); +static DEVICE_ATTR(mmap_area, 0664, pcidriver_show_mmap_area, pcidriver_store_mmap_area); static DEVICE_ATTR(kmem_count, S_IRUGO, pcidriver_show_kmem_count, NULL); static DEVICE_ATTR(kbuffers, S_IRUGO, pcidriver_show_kbuffers, NULL); -static DEVICE_ATTR(kmem_alloc, S_IWUGO, NULL, pcidriver_store_kmem_alloc); -static DEVICE_ATTR(kmem_free, S_IWUGO, NULL, pcidriver_store_kmem_free); +static DEVICE_ATTR(kmem_alloc, 0220, NULL, pcidriver_store_kmem_alloc); +static DEVICE_ATTR(kmem_free, 0220, NULL, pcidriver_store_kmem_free); static DEVICE_ATTR(umappings, S_IRUGO, pcidriver_show_umappings, NULL); -static DEVICE_ATTR(umem_unmap, S_IWUGO, NULL, pcidriver_store_umem_unmap); +static DEVICE_ATTR(umem_unmap, 0220, NULL, pcidriver_store_umem_unmap); #ifdef ENABLE_IRQ static DEVICE_ATTR(irq_count, S_IRUGO, pcidriver_show_irq_count, NULL); diff --git a/pcitool/CMakeLists.txt b/pcitool/CMakeLists.txt index fc88de6..4aea142 100644 --- a/pcitool/CMakeLists.txt +++ b/pcitool/CMakeLists.txt @@ -12,7 +12,7 @@ link_directories( if (NOT DISABLE_PCITOOL) add_executable(pci cli.c sysinfo.c formaters.c) set(HEADERS ${HEADERS} sysinfo.h formaters.h) - add_dependencies(pci pcitool) + add_dependencies(pci pcilib) target_link_libraries(pci pcilib ${FASTWRITER_LIBRARIES}) set_target_properties(pci PROPERTIES LINK_FLAGS "${CMAKE_THREAD_LIBS_INIT} ${EXTRA_SYSTEM_LIBS}" -- cgit v1.2.3 From 6bad94bb8546a3a5595d340e7a2d809635e3bd5d Mon Sep 17 00:00:00 2001 From: "Suren A. Chilingaryan" Date: Fri, 21 Aug 2015 05:19:56 +0200 Subject: Keep frame pointers --- CMakeLists.txt | 2 +- docs/ToDo | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 337a366..b6fce9a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -21,7 +21,7 @@ endif (NOT DISABLE_PCITOOL) check_include_files(stdatomic.h HAVE_STDATOMIC_H) -add_definitions("-fPIC --std=c99 -Wall -O2 -gdwarf-2 -g3") +add_definitions("-fPIC --std=c99 -Wall -O2 -gdwarf-2 -g3 -fno-omit-frame-pointer") #add_definitions("-fPIC --std=c99 -Wall -O2") include(cmake/version.cmake) diff --git a/docs/ToDo b/docs/ToDo index f9f14e6..ea3c18e 100644 --- a/docs/ToDo +++ b/docs/ToDo @@ -17,3 +17,8 @@ Low Priority (only as generalization for other projects) 5. Use pthread_condition_t instead of polling 6. Support FIFO reads/writes from/to registers 7. We managed kmem performance using next kmem prediction, but it is still wise to provide additionally a binary tree for faster search + +Performance +=========== + 1. Even with fully algined data, glibc defaults to __memcpy_sse2_unaligned (called from ipecamera_data_callback and fastwriter_push). Can we do something? + \ No newline at end of file -- cgit v1.2.3