From 8d966d98f48de99f8a87482defef319f8fa16a17 Mon Sep 17 00:00:00 2001 From: Lionel Debroux Date: Tue, 16 Jul 2024 09:55:13 +0200 Subject: [PATCH] Refactor the memrw functions to reduce the redundancy. (#415) The impact is limited now, but will increase when adding support for more architectures and more bit widths. --- doc/Doxyfile | 4 +-- system/ehci.c | 2 +- system/memrw.h | 88 +++++++++++++++++++++++++++++++++++++++++++++ system/memrw32.h | 63 -------------------------------- system/memrw64.h | 63 -------------------------------- system/ohci.c | 2 +- system/smp.c | 2 +- system/uhci.c | 2 +- system/usbhcd.c | 2 +- system/xhci.c | 26 +++++++------- tests/test_helper.h | 3 +- 11 files changed, 109 insertions(+), 148 deletions(-) create mode 100644 system/memrw.h delete mode 100644 system/memrw32.h delete mode 100644 system/memrw64.h diff --git a/doc/Doxyfile b/doc/Doxyfile index 7c72915..824e940 100644 --- a/doc/Doxyfile +++ b/doc/Doxyfile @@ -935,7 +935,7 @@ EXCLUDE_PATTERNS = # Note that the wildcards are matched against the file with absolute path, so to # exclude all test directories use the pattern */test/* -EXCLUDE_SYMBOLS = +EXCLUDE_SYMBOLS = __MEMRW_* # The EXAMPLE_PATH tag can be used to specify one or more files or directories # that contain example code fragments that are included (see the \include @@ -2181,7 +2181,7 @@ PREDEFINED = __attribute__(x)= # definition found in the source code. # This tag requires that the tag ENABLE_PREPROCESSING is set to YES. -EXPAND_AS_DEFINED = +EXPAND_AS_DEFINED = __MEMRW_READ_FUNC __MEMRW_WRITE_FUNC __MEMRW_FLUSH_FUNC # If the SKIP_FUNCTION_MACROS tag is set to YES then doxygen's preprocessor will # remove all references to function-like macros that are alone on a line, have diff --git a/system/ehci.c b/system/ehci.c index 1dab18e..bee26e6 100644 --- a/system/ehci.c +++ b/system/ehci.c @@ -6,7 +6,7 @@ #include #include "heap.h" -#include "memrw32.h" +#include "memrw.h" #include "memsize.h" #include "pci.h" #include "usb.h" diff --git a/system/memrw.h b/system/memrw.h new file mode 100644 index 0000000..9b061b4 --- /dev/null +++ b/system/memrw.h @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: GPL-2.0 +#ifndef MEMRW_H +#define MEMRW_H +/** + * \file + * + * Provides some 32/64-bit memory access functions. These stop the compiler + * optimizing accesses which need to be ordered and atomic. Mostly used + * for accessing memory-mapped hardware registers. + * + *//* + * Copyright (C) 2021-2022 Martin Whitaker. + * Copyright (C) 2024 Lionel Debroux. + */ + +#include + +#define __MEMRW_SUFFIX_32BIT "l" +#define __MEMRW_SUFFIX_64BIT "q" +#define __MEMRW_READ_INSTRUCTIONS(bitwidth) "mov" __MEMRW_SUFFIX_##bitwidth##BIT " %1, %0" +#define __MEMRW_WRITE_INSTRUCTIONS(bitwidth) "mov" __MEMRW_SUFFIX_##bitwidth##BIT " %1, %0" +#define __MEMRW_FLUSH_INSTRUCTIONS(bitwidth) "mov" __MEMRW_SUFFIX_##bitwidth##BIT " %1, %0; mov" __MEMRW_SUFFIX_##bitwidth##BIT " %0, %1" + +#define __MEMRW_READ_FUNC(bitwidth) \ +static inline uint##bitwidth##_t read##bitwidth(const volatile uint##bitwidth##_t *ptr) \ +{ \ + uint##bitwidth##_t val; \ + __asm__ __volatile__( \ + __MEMRW_READ_INSTRUCTIONS(bitwidth) \ + : "=r" (val) \ + : "m" (*ptr) \ + : "memory" \ + ); \ + return val; \ +} + +#define __MEMRW_WRITE_FUNC(bitwidth) \ +static inline void write##bitwidth(const volatile uint##bitwidth##_t *ptr, uint##bitwidth##_t val) \ +{ \ + __asm__ __volatile__( \ + __MEMRW_WRITE_INSTRUCTIONS(bitwidth) \ + : \ + : "m" (*ptr), \ + "r" (val) \ + : "memory" \ + ); \ +} + +#define __MEMRW_FLUSH_FUNC(bitwidth) \ +static inline void flush##bitwidth(const volatile uint##bitwidth##_t *ptr, uint##bitwidth##_t val) \ +{ \ + __asm__ __volatile__( \ + __MEMRW_FLUSH_INSTRUCTIONS(bitwidth) \ + : \ + : "m" (*ptr), \ + "r" (val) \ + : "memory" \ + ); \ +} + +/** + * Reads and returns the value stored in the 32-bit memory location pointed to by ptr. + */ +__MEMRW_READ_FUNC(32) +/** + * Reads and returns the value stored in the 64-bit memory location pointed to by ptr. + */ +__MEMRW_READ_FUNC(64) + +/** + * Writes val to the 32-bit memory location pointed to by ptr. + */ +__MEMRW_WRITE_FUNC(32) +/** + * Writes val to the 64-bit memory location pointed to by ptr. + */ +__MEMRW_WRITE_FUNC(64) + +/** + * Writes val to the 32-bit memory location pointed to by ptr. Only returns when the write is complete. + */ +__MEMRW_FLUSH_FUNC(32) +/** + * Writes val to the 64-bit memory location pointed to by ptr. Only returns when the write is complete. + */ +__MEMRW_FLUSH_FUNC(64) + +#endif // MEMRW_H diff --git a/system/memrw32.h b/system/memrw32.h deleted file mode 100644 index e9316ec..0000000 --- a/system/memrw32.h +++ /dev/null @@ -1,63 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#ifndef MEMRW32_H -#define MEMRW32_H -/** - * \file - * - * Provides some 32-bit memory access functions. These stop the compiler - * optimizing accesses which need to be ordered and atomic. Mostly used - * for accessing memory-mapped hardware registers. - * - *//* - * Copyright (C) 2021-2022 Martin Whitaker. - */ - -#include - -/** - * Reads and returns the value stored in the 32-bit memory location pointed - * to by ptr. - */ -static inline uint32_t read32(const volatile uint32_t *ptr) -{ - uint32_t val; - __asm__ __volatile__( - "movl %1, %0" - : "=r" (val) - : "m" (*ptr) - : "memory" - ); - return val; -} - -/** - * Writes val to the 32-bit memory location pointed to by ptr. - */ -static inline void write32(const volatile uint32_t *ptr, uint32_t val) -{ - __asm__ __volatile__( - "movl %1, %0" - : - : "m" (*ptr), - "r" (val) - : "memory" - ); -} - -/** - * Writes val to the 32-bit memory location pointed to by ptr. Reads it - * back (and discards it) to ensure the write is complete. - */ -static inline void flush32(const volatile uint32_t *ptr, uint32_t val) -{ - __asm__ __volatile__( - "movl %1, %0\n" - "movl %0, %1" - : - : "m" (*ptr), - "r" (val) - : "memory" - ); -} - -#endif // MEMRW32_H diff --git a/system/memrw64.h b/system/memrw64.h deleted file mode 100644 index f45d36b..0000000 --- a/system/memrw64.h +++ /dev/null @@ -1,63 +0,0 @@ -// SPDX-License-Identifier: GPL-2.0 -#ifndef MEMRW64_H -#define MEMRW64_H -/** - * \file - * - * Provides some 64-bit memory access functions. These stop the compiler - * optimizing accesses which need to be ordered and atomic. Mostly used - * for accessing memory-mapped hardware registers. - * - *//* - * Copyright (C) 2021-2022 Martin Whitaker. - */ - -#include - -/** - * Reads and returns the value stored in the 64-bit memory location pointed - * to by ptr. - */ -static inline uint64_t read64(const volatile uint64_t *ptr) -{ - uint64_t val; - __asm__ __volatile__( - "movq %1, %0" - : "=r" (val) - : "m" (*ptr) - : "memory" - ); - return val; -} - -/** - * Writes val to the 64-bit memory location pointed to by ptr. - */ -static inline void write64(const volatile uint64_t *ptr, uint64_t val) -{ - __asm__ __volatile__( - "movq %1, %0" - : - : "m" (*ptr), - "r" (val) - : "memory" - ); -} - -/** - * Writes val to the 64-bit memory location pointed to by ptr. Reads it - * back (and discards it) to ensure the write is complete. - */ -static inline void flush64(const volatile uint64_t *ptr, uint64_t val) -{ - __asm__ __volatile__( - "movl %1, %0\n" - "movl %0, %1" - : - : "m" (*ptr), - "r" (val) - : "memory" - ); -} - -#endif // MEMRW64_H diff --git a/system/ohci.c b/system/ohci.c index dce7eb3..4c64486 100644 --- a/system/ohci.c +++ b/system/ohci.c @@ -6,7 +6,7 @@ #include #include "heap.h" -#include "memrw32.h" +#include "memrw.h" #include "memsize.h" #include "usb.h" diff --git a/system/smp.c b/system/smp.c index 372ff9d..c78272a 100644 --- a/system/smp.c +++ b/system/smp.c @@ -23,7 +23,7 @@ #include "cpuid.h" #include "heap.h" #include "hwquirks.h" -#include "memrw32.h" +#include "memrw.h" #include "memsize.h" #include "msr.h" #include "string.h" diff --git a/system/uhci.c b/system/uhci.c index 0669225..878729c 100644 --- a/system/uhci.c +++ b/system/uhci.c @@ -7,7 +7,7 @@ #include "heap.h" #include "io.h" -#include "memrw32.h" +#include "memrw.h" #include "memsize.h" #include "pci.h" #include "usb.h" diff --git a/system/usbhcd.c b/system/usbhcd.c index 5824eb1..159b443 100644 --- a/system/usbhcd.c +++ b/system/usbhcd.c @@ -2,7 +2,7 @@ // Copyright (C) 2021-2022 Martin Whitaker. #include "keyboard.h" -#include "memrw32.h" +#include "memrw.h" #include "pci.h" #include "screen.h" #include "usb.h" diff --git a/system/xhci.c b/system/xhci.c index dbe6eb6..26804f5 100644 --- a/system/xhci.c +++ b/system/xhci.c @@ -5,7 +5,7 @@ #include #include "heap.h" -#include "memrw32.h" +#include "memrw.h" #include "memsize.h" #include "usb.h" #include "vmem.h" @@ -353,17 +353,17 @@ static size_t round_up(size_t size, size_t alignment) return (size + alignment - 1) & ~(alignment - 1); } -// The read64 and write64 functions provided here provide compatibility with both +// The read64_ and write64_ functions provided here provide compatibility with both // 32-bit and 64-bit hosts and with both 32-bit and 64-bit XHCI controllers. -static uint64_t read64(const volatile uint64_t *ptr) +static uint64_t read64_(const volatile uint64_t *ptr) { uint32_t val_l = read32((const volatile uint32_t *)ptr + 0); uint32_t val_h = read32((const volatile uint32_t *)ptr + 1); return (uint64_t)val_h << 32 | (uint64_t)val_l; } -static void write64(volatile uint64_t *ptr, uint64_t val) +static void write64_(volatile uint64_t *ptr, uint64_t val) { write32((volatile uint32_t *)ptr + 0, (uint32_t)(val >> 0)); write32((volatile uint32_t *)ptr + 1, (uint32_t)(val >> 32)); @@ -504,7 +504,7 @@ static uint32_t enqueue_trb(xhci_trb_t *trb_ring, uint32_t ring_size, uint32_t e // If at the last slot, insert a Link TRB and start a new cycle. if (index == (ring_size - 1)) { - write64(&trb_ring[index].params1, (uintptr_t)trb_ring); + write64_(&trb_ring[index].params1, (uintptr_t)trb_ring); write32(&trb_ring[index].params2, 0); write32(&trb_ring[index].control, XHCI_TRB_LINK | XHCI_TRB_TC | cycle); cycle ^= 1; @@ -512,7 +512,7 @@ static uint32_t enqueue_trb(xhci_trb_t *trb_ring, uint32_t ring_size, uint32_t e } // Insert the TRB. - write64(&trb_ring[index].params1, params1); + write64_(&trb_ring[index].params1, params1); write32(&trb_ring[index].params2, params2); write32(&trb_ring[index].control, control | cycle); index++; @@ -542,7 +542,7 @@ static bool get_xhci_event(workspace_t *ws, xhci_trb_t *event) if ((event->control & 0x1) != cycle) return false; // Advance the dequeue pointer. - write64(&ws->rt_regs->ir[0].erdp, (uintptr_t)(&ws->er[index])); + write64_(&ws->rt_regs->ir[0].erdp, (uintptr_t)(&ws->er[index])); // Update the event ring dequeue state. if (index == (WS_ER_SIZE - 1)) { @@ -667,7 +667,7 @@ static int allocate_slot(const usb_hcd_t *hcd) } int slot_id = event_slot_id(&event); - write64(&ws->device_context_index[slot_id], device_workspace_addr); + write64_(&ws->device_context_index[slot_id], device_workspace_addr); return slot_id; @@ -688,7 +688,7 @@ static bool release_slot(const usb_hcd_t *hcd, int slot_id) return false; } - write64(&ws->device_context_index[slot_id], 0); + write64_(&ws->device_context_index[slot_id], 0); heap_rewind(HEAP_TYPE_LM_1, ws->initial_heap_mark); @@ -1108,13 +1108,13 @@ bool xhci_probe(uintptr_t base_addr, usb_hcd_t *hcd) ws->erst[0].segment_addr = (uintptr_t)(&ws->er); ws->erst[0].segment_size = WS_ER_SIZE; - write64(&rt_regs->ir[0].erdp, (uintptr_t)(&ws->er)); + write64_(&rt_regs->ir[0].erdp, (uintptr_t)(&ws->er)); write32(&rt_regs->ir[0].erst_size, 1); - write64(&rt_regs->ir[0].erst_addr, (uintptr_t)(&ws->erst)); + write64_(&rt_regs->ir[0].erst_addr, (uintptr_t)(&ws->erst)); // Initialise and start the controller. - write64(&op_regs->cr_control, (read64(&op_regs->cr_control) & 0x30) | (uintptr_t)(&ws->cr) | 0x1); - write64(&op_regs->dcbaap, device_context_index_paddr); + write64_(&op_regs->cr_control, (read64_(&op_regs->cr_control) & 0x30) | (uintptr_t)(&ws->cr) | 0x1); + write64_(&op_regs->dcbaap, device_context_index_paddr); write32(&op_regs->config, (read32(&op_regs->config) & 0xfffffc00) | max_slots); if (!start_host_controller(op_regs)) { goto no_keyboards_found; diff --git a/tests/test_helper.h b/tests/test_helper.h index 42c2ee7..6dba727 100644 --- a/tests/test_helper.h +++ b/tests/test_helper.h @@ -19,12 +19,11 @@ /** * Test word atomic read and write functions. */ +#include "memrw.h" #ifdef __x86_64__ -#include "memrw64.h" #define read_word read64 #define write_word write64 #else -#include "memrw32.h" #define read_word read32 #define write_word write32 #endif