From 679c8cb05e39e47cdd3f8c401c3edce81514d684 Mon Sep 17 00:00:00 2001 From: Lance Thompson Date: Tue, 5 Dec 2017 14:16:01 -0600 Subject: [PATCH 1/2] fix for an mmio race condition when multiple threads address the mmio space of the same afu --- libcxl/libcxl.c | 44 ++++++++++++++++++++++++++++++++++++++++ libcxl/libcxl_internal.h | 1 + 2 files changed, 45 insertions(+) diff --git a/libcxl/libcxl.c b/libcxl/libcxl.c index 8a47517..c53bc32 100644 --- a/libcxl/libcxl.c +++ b/libcxl/libcxl.c @@ -890,6 +890,7 @@ static struct cxl_afu_h *_new_afu(uint16_t afu_map, uint16_t position, int fd) return NULL; pthread_mutex_init(&(afu->event_lock), NULL); + pthread_mutex_init(&(afu->mmio_lock), NULL); afu->fd = fd; afu->map = afu_map; afu->dbg_id = (major << 4) | minor; @@ -942,6 +943,7 @@ static void _release_afus(struct cxl_afu_h *afu) if (afu->id) free(afu->id); pthread_mutex_destroy(&(afu->event_lock)); + pthread_mutex_destroy(&(afu->mmio_lock)); free(afu); } } @@ -1029,6 +1031,7 @@ static struct cxl_afu_h *_pslse_open(int *fd, uint16_t afu_map, uint8_t major, open_fail: pthread_mutex_destroy(&(afu->event_lock)); + pthread_mutex_destroy(&(afu->mmio_lock)); free(afu); errno = ENODEV; return NULL; @@ -1349,6 +1352,7 @@ void cxl_afu_free(struct cxl_afu_h *afu) free(afu->id); free_done_no_afu: pthread_mutex_destroy(&(afu->event_lock)); + pthread_mutex_destroy(&(afu->mmio_lock)); free(afu); } @@ -1585,11 +1589,21 @@ int cxl_mmio_write64(struct cxl_afu_h *afu, uint64_t offset, uint64_t data) if ((afu == NULL) || !afu->mapped) goto write64_fail; + // obtain mmio lock before reading and updating mmio structure + while (1) { + pthread_mutex_lock( &(afu->mmio_lock) ); + if ( afu->mmio.state == LIBCXL_REQ_IDLE ) break; // it is ok to make the mmio request + pthread_mutex_unlock( &(afu->mmio_lock) ); + _delay_1ms(); + } + // Send MMIO map to PSLSE afu->mmio.type = PSLSE_MMIO_WRITE64; afu->mmio.addr = (uint32_t) offset; afu->mmio.data = data; afu->mmio.state = LIBCXL_REQ_REQUEST; + pthread_mutex_unlock( &(afu->mmio_lock) ); + while (afu->mmio.state != LIBCXL_REQ_IDLE) /*infinite loop */ _delay_1ms(); @@ -1612,10 +1626,20 @@ int cxl_mmio_read64(struct cxl_afu_h *afu, uint64_t offset, uint64_t * data) if ((afu == NULL) || !afu->mapped) goto read64_fail; + // obtain mmio lock before reading and updating mmio structure + while (1) { + pthread_mutex_lock( &(afu->mmio_lock) ); + if ( afu->mmio.state == LIBCXL_REQ_IDLE ) break; // it is ok to make the mmio request + pthread_mutex_unlock( &(afu->mmio_lock) ); + _delay_1ms(); + } + // Send MMIO map to PSLSE afu->mmio.type = PSLSE_MMIO_READ64; afu->mmio.addr = (uint32_t) offset; afu->mmio.state = LIBCXL_REQ_REQUEST; + pthread_mutex_unlock( &(afu->mmio_lock) ); + while (afu->mmio.state != LIBCXL_REQ_IDLE) /*infinite loop */ _delay_1ms(); *data = afu->mmio.data; @@ -1716,11 +1740,21 @@ int cxl_mmio_write32(struct cxl_afu_h *afu, uint64_t offset, uint32_t data) if ((afu == NULL) || !afu->mapped) goto write32_fail; + // obtain mmio lock before reading and updating mmio structure + while (1) { + pthread_mutex_lock( &(afu->mmio_lock) ); + if ( afu->mmio.state == LIBCXL_REQ_IDLE ) break; // it is ok to make the mmio request + pthread_mutex_unlock( &(afu->mmio_lock) ); + _delay_1ms(); + } + // Send MMIO map to PSLSE afu->mmio.type = PSLSE_MMIO_WRITE32; afu->mmio.addr = (uint32_t) offset; afu->mmio.data = (uint64_t) data; afu->mmio.state = LIBCXL_REQ_REQUEST; + pthread_mutex_unlock( &(afu->mmio_lock) ); + while (afu->mmio.state != LIBCXL_REQ_IDLE) /*infinite loop */ _delay_1ms(); @@ -1743,10 +1777,20 @@ int cxl_mmio_read32(struct cxl_afu_h *afu, uint64_t offset, uint32_t * data) if ((afu == NULL) || !afu->mapped) goto read32_fail; + // obtain mmio lock before reading and updating mmio structure + while (1) { + pthread_mutex_lock( &(afu->mmio_lock) ); + if ( afu->mmio.state == LIBCXL_REQ_IDLE ) break; // it is ok to make the mmio request + pthread_mutex_unlock( &(afu->mmio_lock) ); + _delay_1ms(); + } + // Send MMIO map to PSLSE afu->mmio.type = PSLSE_MMIO_READ32; afu->mmio.addr = (uint32_t) offset; afu->mmio.state = LIBCXL_REQ_REQUEST; + pthread_mutex_unlock( &(afu->mmio_lock) ); + while (afu->mmio.state != LIBCXL_REQ_IDLE) /*infinite loop */ _delay_1ms(); *data = (uint32_t) afu->mmio.data; diff --git a/libcxl/libcxl_internal.h b/libcxl/libcxl_internal.h index a3242a6..0f3fe2b 100644 --- a/libcxl/libcxl_internal.h +++ b/libcxl/libcxl_internal.h @@ -55,6 +55,7 @@ struct mmio_req { struct cxl_afu_h { pthread_t thread; pthread_mutex_t event_lock; + pthread_mutex_t mmio_lock; struct cxl_event **events; int adapter; char *id; From 362d44cd19f23a907102ee5b92bb133ed81b3e6b Mon Sep 17 00:00:00 2001 From: Lance Thompson Date: Tue, 12 Dec 2017 07:30:56 -0600 Subject: [PATCH 2/2] release lock a little later in mmio read routines --- libcxl/libcxl.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libcxl/libcxl.c b/libcxl/libcxl.c index c53bc32..6679866 100644 --- a/libcxl/libcxl.c +++ b/libcxl/libcxl.c @@ -1638,11 +1638,11 @@ int cxl_mmio_read64(struct cxl_afu_h *afu, uint64_t offset, uint64_t * data) afu->mmio.type = PSLSE_MMIO_READ64; afu->mmio.addr = (uint32_t) offset; afu->mmio.state = LIBCXL_REQ_REQUEST; - pthread_mutex_unlock( &(afu->mmio_lock) ); while (afu->mmio.state != LIBCXL_REQ_IDLE) /*infinite loop */ _delay_1ms(); *data = afu->mmio.data; + pthread_mutex_unlock( &(afu->mmio_lock) ); if (!afu->opened) goto read64_fail; @@ -1789,11 +1789,11 @@ int cxl_mmio_read32(struct cxl_afu_h *afu, uint64_t offset, uint32_t * data) afu->mmio.type = PSLSE_MMIO_READ32; afu->mmio.addr = (uint32_t) offset; afu->mmio.state = LIBCXL_REQ_REQUEST; - pthread_mutex_unlock( &(afu->mmio_lock) ); while (afu->mmio.state != LIBCXL_REQ_IDLE) /*infinite loop */ _delay_1ms(); *data = (uint32_t) afu->mmio.data; + pthread_mutex_unlock( &(afu->mmio_lock) ); if (!afu->opened) goto read32_fail;