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

@@ -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

system/ehci.c

@@ -6,7 +6,7 @@
 #include <stdint.h>
 
 #include "heap.h"
-#include "memrw32.h"
+#include "memrw.h"
 #include "memsize.h"
 #include "pci.h"
 #include "usb.h"

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 <stdint.h>
+
+#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

system/memrw32.h

@@ -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 <stdint.h>
-
-/**
- * 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

system/memrw64.h

@@ -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 <stdint.h>
-
-/**
- * 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

system/ohci.c

@@ -6,7 +6,7 @@
 #include <stdint.h>
 
 #include "heap.h"
-#include "memrw32.h"
+#include "memrw.h"
 #include "memsize.h"
 #include "usb.h"
 

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"

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"

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"

system/xhci.c

@@ -5,7 +5,7 @@
 #include <stdint.h>
 
 #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->cr_control, (read64_(&op_regs->cr_control) & 0x30) | (uintptr_t)(&ws->cr) | 0x1);
-    write64(&op_regs->dcbaap,     device_context_index_paddr);
+    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;

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