From f2acc47f4363f14c20f645e48aac98f48259de26 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Duval?= Date: Wed, 6 Dec 2023 18:38:28 +0100 Subject: [PATCH] acpi: update AcpiOsAcquireGlobalLock and AcpiOsReleaseGlobalLock implementations from FreeBSD. Because the argument wasn't volatile, the compiler wasn't considerung reading the lock content. * fix the acquire and release loops: The loop could be exited when any new values were read. Instead the return value should be checked against the old value. Old x86 gcc2 asm loop in AcpiOsAcquireGlobalLock: 11660: 89 c8 mov %ecx,%eax 11662: f0 0f b1 1a lock cmpxchg %ebx,(%edx) 11666: 39 0a cmp %ecx,(%edx) 11668: 74 f6 je 11660 New x86 gcc13 asm loop in AcpiOsAcquireGlobalLock: 4d58: 8b 11 mov (%ecx),%edx 4d5a: 89 d0 mov %edx,%eax 4d5c: 89 d3 mov %edx,%ebx 4d5e: 83 e0 fc and $0xfffffffc,%eax 4d61: 83 cb 03 or $0x3,%ebx 4d64: f6 c2 02 test $0x2,%dl 4d67: 75 05 jne 4d6e 4d69: 83 c8 02 or $0x2,%eax 4d6c: 89 c3 mov %eax,%ebx 4d6e: 89 d0 mov %edx,%eax 4d70: f0 0f b1 19 lock cmpxchg %ebx,(%ecx) 4d74: 75 e2 jne 4d58 Old x86 gcc2 asm loop in AcpiOsReleaseGlobalLock: 11690: 89 c8 mov %ecx,%eax 11692: f0 0f b1 1a lock cmpxchg %ebx,(%edx) 11696: 39 0a cmp %ecx,(%edx) 11698: 74 f6 je 11690 New x86 gcc13 asm loop in AcpiOsReleaseGlobalLock: 4d88: 8b 11 mov (%ecx),%edx 4d8a: 89 d3 mov %edx,%ebx 4d8c: 89 d0 mov %edx,%eax 4d8e: 83 e3 fc and $0xfffffffc,%ebx 4d91: f0 0f b1 19 lock cmpxchg %ebx,(%ecx) 4d95: 75 f1 jne 4d88 Change-Id: Ia55ba8666efe8b5c198a01db100b4747b72df4a0 Reviewed-on: https://review.haiku-os.org/c/haiku/+/7185 Reviewed-by: waddlesplash --- .../kernel/bus_managers/acpi/ACPICAHaiku.cpp | 31 +++++++------------ .../acpi/acpica/include/platform/achaiku.h | 4 +-- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp b/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp index e26ce4a17d..0a88ffdee2 100644 --- a/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp +++ b/src/add-ons/kernel/bus_managers/acpi/ACPICAHaiku.cpp @@ -1218,12 +1218,6 @@ AcpiOsSignal(UINT32 function, void *info) * Adapted from FreeBSD since the documentation of its intended impl * is lacking. * Section 5.2.10.1: global lock acquire/release functions */ -#define GL_ACQUIRED (-1) -#define GL_BUSY 0 -#define GL_BIT_PENDING 0x01 -#define GL_BIT_OWNED 0x02 -#define GL_BIT_MASK (GL_BIT_PENDING | GL_BIT_OWNED) - /* * Adapted from FreeBSD since the documentation of its intended impl @@ -1233,18 +1227,18 @@ AcpiOsSignal(UINT32 function, void *info) * and then attempt to acquire it again. */ int -AcpiOsAcquireGlobalLock(uint32 *lock) +AcpiOsAcquireGlobalLock(volatile uint32_t *lock) { - uint32 newValue; - uint32 oldValue; + uint32_t newValue; + uint32_t oldValue; do { oldValue = *lock; - newValue = ((oldValue & ~GL_BIT_MASK) | GL_BIT_OWNED) | - ((oldValue >> 1) & GL_BIT_PENDING); - atomic_test_and_set((int32*)lock, newValue, oldValue); - } while (*lock == oldValue); - return ((newValue < GL_BIT_MASK) ? GL_ACQUIRED : GL_BUSY); + newValue = ((oldValue & ~ACPI_GLOCK_PENDING) | ACPI_GLOCK_OWNED); + if ((oldValue & ACPI_GLOCK_OWNED) != 0) + newValue |= ACPI_GLOCK_PENDING; + } while (atomic_test_and_set((int32*)lock, newValue, oldValue) != (int32)oldValue); + return (newValue & ACPI_GLOCK_PENDING) != 0; } @@ -1256,17 +1250,16 @@ AcpiOsAcquireGlobalLock(uint32 *lock) * releases the lock. */ int -AcpiOsReleaseGlobalLock(uint32 *lock) +AcpiOsReleaseGlobalLock(volatile uint32_t *lock) { uint32 newValue; uint32 oldValue; do { oldValue = *lock; - newValue = oldValue & ~GL_BIT_MASK; - atomic_test_and_set((int32*)lock, newValue, oldValue); - } while (*lock == oldValue); - return (oldValue & GL_BIT_PENDING); + newValue = oldValue & ~(ACPI_GLOCK_PENDING | ACPI_GLOCK_OWNED); + } while (atomic_test_and_set((int32*)lock, newValue, oldValue) != (int32)oldValue); + return (oldValue & ACPI_GLOCK_PENDING) != 0; } diff --git a/src/add-ons/kernel/bus_managers/acpi/acpica/include/platform/achaiku.h b/src/add-ons/kernel/bus_managers/acpi/acpica/include/platform/achaiku.h index 238f34e776..216203d2e2 100644 --- a/src/add-ons/kernel/bus_managers/acpi/acpica/include/platform/achaiku.h +++ b/src/add-ons/kernel/bus_managers/acpi/acpica/include/platform/achaiku.h @@ -197,8 +197,8 @@ struct mutex; #endif /* Based on FreeBSD's due to lack of documentation */ -extern int AcpiOsAcquireGlobalLock(uint32 *lock); -extern int AcpiOsReleaseGlobalLock(uint32 *lock); +extern int AcpiOsAcquireGlobalLock(volatile uint32_t *lock); +extern int AcpiOsReleaseGlobalLock(volatile uint32_t *lock); #define ACPI_ACQUIRE_GLOBAL_LOCK(GLptr, Acq) do { \ (Acq) = AcpiOsAcquireGlobalLock(&((GLptr)->GlobalLock)); \