From 6494a30c3d21f82968bd1bfd0ced2efb1cf13a57 Mon Sep 17 00:00:00 2001 From: Dave Murphy Date: Sat, 26 Feb 2022 19:58:41 +0000 Subject: [PATCH] wutdevoptab: Make sure to always read into/write from a properly aligned buffer --- libraries/wutdevoptab/devoptab_fs_read.c | 111 +++++++++------------- libraries/wutdevoptab/devoptab_fs_write.c | 104 +++++++++----------- 2 files changed, 90 insertions(+), 125 deletions(-) diff --git a/libraries/wutdevoptab/devoptab_fs_read.c b/libraries/wutdevoptab/devoptab_fs_read.c index 1a21813c..a10ccbb9 100644 --- a/libraries/wutdevoptab/devoptab_fs_read.c +++ b/libraries/wutdevoptab/devoptab_fs_read.c @@ -1,85 +1,68 @@ #include "devoptab_fs.h" -ssize_t -__wut_fs_read(struct _reent *r, - void *fd, - char *ptr, - size_t len) -{ - FSStatus status = 0; - FSCmdBlock cmd; - uint8_t *alignedReadBuffer; - uint32_t bytes, bytesRead; - __wut_fs_file_t *file; +#include +ssize_t __wut_fs_read(struct _reent *r, void *fd, char *ptr, size_t len) { if (!fd || !ptr) { r->_errno = EINVAL; return -1; } - FSInitCmdBlock(&cmd); - file = (__wut_fs_file_t *)fd; - bytesRead = 0; - // Check that the file was opened with read access + __wut_fs_file_t *file = (__wut_fs_file_t *) fd; if ((file->flags & O_ACCMODE) == O_WRONLY) { r->_errno = EBADF; return -1; } - if((((uintptr_t) ptr) & 0x3F) == 0){ - status = FSReadFile(__wut_devoptab_fs_client, &cmd, (uint8_t *) ptr, 1, - len, file->fd, 0, FS_ERROR_FLAG_ALL); - if(status > 0){ - bytesRead = (uint32_t) status; - file->offset += bytesRead; + // cache-aligned, cache-line-sized + __attribute__((aligned(0x40))) uint8_t alignedBuffer[0x40]; + + FSCmdBlock cmd; + FSInitCmdBlock(&cmd); + + size_t bytesRead = 0; + while (bytesRead < len) { + // only use input buffer if cache-aligned and read size is a multiple of cache line size + // otherwise read into alignedBuffer + uint8_t *tmp = (uint8_t *) ptr; + size_t size = len - bytesRead; + + if ((uintptr_t) ptr & 0x3F) { + // read partial cache-line front-end + tmp = alignedBuffer; + size = MIN(size, 0x40 - ((uintptr_t) ptr & 0x3F)); + } else if (size < 0x40) { + // read partial cache-line back-end + tmp = alignedBuffer; + } else { + // read whole cache lines + size &= ~0x3F; } - } else { - // Copy to internal buffer due to alignment requirement and read in chunks. - // Using a buffer smaller than 128KiB takes a performance hit. - int buffer_size = len < 128*1024 ? len : 128*1024; - alignedReadBuffer = memalign(0x40, buffer_size); - if(!alignedReadBuffer){ - r->_errno = ENOMEM; + + FSStatus status = FSReadFile(__wut_devoptab_fs_client, &cmd, tmp, 1, size, + file->fd, 0, FS_ERROR_FLAG_ALL); + + if (status < 0) { + if (bytesRead != 0) { + return bytesRead; // error after partial read + } + + r->_errno = __wut_fs_translate_error(status); return -1; } - while (len > 0) { - size_t toRead = len > buffer_size ? buffer_size : len; - // Write the data - status = FSReadFile(__wut_devoptab_fs_client, &cmd, alignedReadBuffer, 1, - toRead, file->fd, 0, FS_ERROR_FLAG_ALL); - if (status <= 0) { - break; - } - - // Copy to internal buffer - bytes = (uint32_t)status; - memcpy(ptr, alignedReadBuffer, bytes); - - file->offset += bytes; - bytesRead += bytes; - ptr += bytes; - len -= bytes; - - if (bytes < toRead) { - // If we did not read the full requested toRead bytes then we reached - // the end of the file. - break; - } + if (tmp == alignedBuffer) { + memcpy(ptr, alignedBuffer, status); + } + + bytesRead += status; + ptr += status; + + if (status != size) { + return bytesRead; // partial read } - free(alignedReadBuffer); } - // Return partial read - if (bytesRead > 0) { - return bytesRead; - } - - if (status < 0) { - r->_errno = __wut_fs_translate_error(status); - return -1; - } - - return 0; -} + return bytesRead; +} \ No newline at end of file diff --git a/libraries/wutdevoptab/devoptab_fs_write.c b/libraries/wutdevoptab/devoptab_fs_write.c index a63f2d8b..377bbe10 100644 --- a/libraries/wutdevoptab/devoptab_fs_write.c +++ b/libraries/wutdevoptab/devoptab_fs_write.c @@ -1,83 +1,65 @@ #include "devoptab_fs.h" -ssize_t -__wut_fs_write(struct _reent *r, - void *fd, - const char *ptr, - size_t len) -{ - FSStatus status = 0; - FSCmdBlock cmd; - uint8_t *alignedWriteBuffer; - uint32_t bytes, bytesWritten; - __wut_fs_file_t *file; - +ssize_t __wut_fs_write(struct _reent *r, void *fd, const char *ptr, size_t len) { if (!fd || !ptr) { r->_errno = EINVAL; return -1; } - FSInitCmdBlock(&cmd); - file = (__wut_fs_file_t *)fd; - bytesWritten = 0; - // Check that the file was opened with write access + __wut_fs_file_t *file = (__wut_fs_file_t *) fd; if ((file->flags & O_ACCMODE) == O_RDONLY) { r->_errno = EBADF; return -1; } - if((((uintptr_t) ptr) & 0x3F) == 0){ - status = FSWriteFile(__wut_devoptab_fs_client, &cmd, (uint8_t *) ptr, - 1, len, file->fd, 0, FS_ERROR_FLAG_ALL); - if(status > 0){ - bytesWritten = (uint32_t) status; - file->offset += bytesWritten; + // cache-aligned, cache-line-sized + __attribute__((aligned(0x40))) uint8_t alignedBuffer[0x40]; + + FSCmdBlock cmd; + FSInitCmdBlock(&cmd); + + size_t bytesWritten = 0; + while (bytesWritten < len) { + // only use input buffer if cache-aligned and write size is a multiple of cache line size + // otherwise write from alignedBuffer + uint8_t *tmp = (uint8_t *) ptr; + size_t size = len - bytesWritten; + + if ((uintptr_t) ptr & 0x3F) { + // write partial cache-line front-end + tmp = alignedBuffer; + size = MIN(size, 0x40 - ((uintptr_t) ptr & 0x3F)); + } else if (size < 0x40) { + // write partial cache-line back-end + tmp = alignedBuffer; + } else { + // write whole cache lines + size &= ~0x3F; } - } else { - // Copy to internal buffer due to alignment requirement and read in chunks. - // Using a buffer smaller than 128KiB takes a performance hit. - int buffer_size = len < 128*1024 ? len : 128*1024; - alignedWriteBuffer = memalign(0x40, buffer_size); - if(!alignedWriteBuffer){ - r->_errno = ENOMEM; + + if (tmp == alignedBuffer) { + memcpy(tmp, ptr, size); + } + + FSStatus status = FSWriteFile(__wut_devoptab_fs_client, &cmd, tmp, 1, size, + file->fd, 0, FS_ERROR_FLAG_ALL); + if (status < 0) { + if (bytesWritten != 0) { + return bytesWritten; // error after partial write + } + + r->_errno = __wut_fs_translate_error(status); return -1; } - while (len > 0) { - size_t toWrite = len > buffer_size ? buffer_size : len; - // Copy to internal buffer - memcpy(alignedWriteBuffer, ptr, toWrite); + bytesWritten += status; + ptr += status; - // Write the data - status = FSWriteFile(__wut_devoptab_fs_client, &cmd, alignedWriteBuffer, - 1, toWrite, file->fd, 0, FS_ERROR_FLAG_ALL); - if (status <= 0) { - break; - } - - bytes = (uint32_t)status; - file->offset += bytes; - bytesWritten += bytes; - ptr += bytes; - len -= bytes; - - if (bytes < toWrite) { - break; - } + if (status != size) { + return bytesWritten; // partial write } - free(alignedWriteBuffer); } - // Return partial write - if (bytesWritten > 0) { - return bytesWritten; - } - - if (status < 0) { - r->_errno = __wut_fs_translate_error(status); - return -1; - } - - return 0; + return bytesWritten; }