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 <AcpiOsAcquireGlobalLock+0x20>

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 <AcpiOsAcquireGlobalLock+0x1e>
    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 <AcpiOsAcquireGlobalLock+0x8>

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 <AcpiOsReleaseGlobalLock+0x10>

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 <AcpiOsReleaseGlobalLock+0x8>

Change-Id: Ia55ba8666efe8b5c198a01db100b4747b72df4a0
Reviewed-on: https://review.haiku-os.org/c/haiku/+/7185
Reviewed-by: waddlesplash <[email protected]>
This commit is contained in:
Jérôme Duval
2023-12-06 19:16:19 +00:00
committed by waddlesplash
parent 153bb8ef67
commit f2acc47f43
2 changed files with 14 additions and 21 deletions
@@ -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;
}
@@ -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)); \