Skip to content

Commit

Permalink
Make mutex acquires return uacpi_status and get rid of related macros
Browse files Browse the repository at this point in the history
This allows us to get rid of constructs like:
     if (uacpi_unlikely(!uacpi_kernel_mutex_acquire(mtx, 0xFFFF))
         return UACPI_STATUS_INTERNAL_ERROR;

...by letting the host indicate internal errors on its own. This also
feels cleaner as all other fallible API returns uacpi_status.

Signed-off-by: Daniil Tatianin <99danilt@gmail.com>
  • Loading branch information
d-tatianin committed Oct 5, 2024
1 parent 8d7af98 commit f38061f
Show file tree
Hide file tree
Showing 9 changed files with 192 additions and 140 deletions.
47 changes: 45 additions & 2 deletions include/uacpi/internal/mutex.h
Original file line number Diff line number Diff line change
@@ -1,8 +1,51 @@
#pragma once

#include <uacpi/types.h>
#include <uacpi/kernel_api.h>

uacpi_bool uacpi_this_thread_owns_aml_mutex(uacpi_mutex*);

uacpi_bool uacpi_acquire_aml_mutex(uacpi_mutex*, uacpi_u16 timeout);
void uacpi_release_aml_mutex(uacpi_mutex*);
uacpi_status uacpi_acquire_aml_mutex(uacpi_mutex*, uacpi_u16 timeout);
uacpi_status uacpi_release_aml_mutex(uacpi_mutex*);

static inline uacpi_status uacpi_acquire_native_mutex(uacpi_handle mtx)
{
if (uacpi_unlikely(mtx == UACPI_NULL))
return UACPI_STATUS_INVALID_ARGUMENT;

return uacpi_kernel_acquire_mutex(mtx, 0xFFFF);
}

uacpi_status uacpi_acquire_native_mutex_with_timeout(
uacpi_handle mtx, uacpi_u16 timeout
);

static inline uacpi_status uacpi_release_native_mutex(uacpi_handle mtx)
{
if (uacpi_unlikely(mtx == UACPI_NULL))
return UACPI_STATUS_INVALID_ARGUMENT;

uacpi_kernel_release_mutex(mtx);
return UACPI_STATUS_OK;
}

static inline uacpi_status uacpi_acquire_native_mutex_may_be_null(
uacpi_handle mtx
)
{
if (mtx == UACPI_NULL)
return UACPI_STATUS_OK;

return uacpi_kernel_acquire_mutex(mtx, 0xFFFF);
}

static inline uacpi_status uacpi_release_native_mutex_may_be_null(
uacpi_handle mtx
)
{
if (mtx == UACPI_NULL)
return UACPI_STATUS_OK;

uacpi_kernel_release_mutex(mtx);
return UACPI_STATUS_OK;
}
43 changes: 0 additions & 43 deletions include/uacpi/internal/stdlib.h
Original file line number Diff line number Diff line change
Expand Up @@ -80,46 +80,3 @@ uacpi_u8 uacpi_bit_scan_forward(uacpi_u64);
uacpi_u8 uacpi_bit_scan_backward(uacpi_u64);

uacpi_u8 uacpi_popcount(uacpi_u64);

#ifdef UACPI_TRACE_MUTEXES
#define UACPI_TRACE_MUTEX_ACQUISITION(mtx) \
uacpi_trace("mutex %p acquired at %s:%d\n", mtx, __FILE__, __LINE__)

#define UACPI_TRACE_MUTEX_ACQUISITION_TIMEOUT(mtx, timeout) \
uacpi_trace("mutex %p acquisition timed out after %dms at %s:%d\n", \
mtx, (uacpi_u16)timeout, __FILE__, __LINE__)

#define UACPI_TRACE_MUTEX_RELEASE(mtx) \
uacpi_trace("mutex %p released at %s:%d\n", mtx, __FILE__, __LINE__)
#else
#define UACPI_TRACE_MUTEX_ACQUISITION(mtx)
#define UACPI_TRACE_MUTEX_ACQUISITION_TIMEOUT(mtx, timeout)
#define UACPI_TRACE_MUTEX_RELEASE(mtx)
#endif

#define UACPI_MUTEX_ACQUIRE(mtx) \
do { \
if (uacpi_unlikely(!uacpi_kernel_acquire_mutex(mtx, 0xFFFF))) { \
uacpi_error( \
"%s: unable to acquire mutex %p with an infinite timeout\n", \
__FUNCTION__, mtx \
); \
return UACPI_STATUS_INTERNAL_ERROR; \
} \
UACPI_TRACE_MUTEX_ACQUISITION(mtx); \
} while (0)

#define UACPI_MUTEX_ACQUIRE_WITH_TIMEOUT(mtx, timeout, ret) \
do { \
ret = uacpi_kernel_acquire_mutex(mtx, timeout); \
if (ret) { \
UACPI_TRACE_MUTEX_ACQUISITION(mtx); \
} else { \
UACPI_TRACE_MUTEX_ACQUISITION_TIMEOUT(mtx, timeout); \
} \
} while (0)

#define UACPI_MUTEX_RELEASE(mtx) do { \
uacpi_kernel_release_mutex(mtx); \
UACPI_TRACE_MUTEX_RELEASE(mtx); \
} while (0)
16 changes: 14 additions & 2 deletions include/uacpi/kernel_api.h
Original file line number Diff line number Diff line change
Expand Up @@ -181,9 +181,21 @@ uacpi_thread_id uacpi_kernel_get_thread_id(void);

/*
* Try to acquire the mutex with a millisecond timeout.
* A timeout value of 0xFFFF implies infinite wait.
*
* The timeout value has the following meanings:
* 0x0000 - Attempt to acquire the mutex once, in a non-blocking manner
* 0x0001...0xFFFE - Attempt to acquire the mutex for at least 'timeout'
* milliseconds
* 0xFFFF - Infinite wait, block until the mutex is acquired
*
* The following are possible return values:
* 1. UACPI_STATUS_OK - successful acquire operation
* 2. UACPI_STATUS_TIMEOUT - timeout reached while attempting to acquire (or the
* single attempt to acquire was not successful for
* calls with timeout=0)
* 3. Any other value - signifies a host internal error and is treated as such
*/
uacpi_bool uacpi_kernel_acquire_mutex(uacpi_handle, uacpi_u16);
uacpi_status uacpi_kernel_acquire_mutex(uacpi_handle, uacpi_u16);
void uacpi_kernel_release_mutex(uacpi_handle);

/*
Expand Down
11 changes: 7 additions & 4 deletions source/interpreter.c
Original file line number Diff line number Diff line change
Expand Up @@ -3530,12 +3530,14 @@ static uacpi_status handle_mutex_ctl(struct execution_context *ctx)
timeout = 0xFFFF;

if (uacpi_this_thread_owns_aml_mutex(obj->mutex)) {
if (uacpi_likely(uacpi_acquire_aml_mutex(obj->mutex, timeout)))
ret = uacpi_acquire_aml_mutex(obj->mutex, timeout);
if (uacpi_likely_success(ret))
*return_value = 0;
break;
}

if (!uacpi_acquire_aml_mutex(obj->mutex, timeout))
ret = uacpi_acquire_aml_mutex(obj->mutex, timeout);
if (uacpi_unlikely_error(ret))
break;

ret = held_mutexes_array_push(&ctx->held_mutexes, obj->mutex);
Expand Down Expand Up @@ -4244,8 +4246,9 @@ static uacpi_status enter_method(
}

if (!uacpi_this_thread_owns_aml_mutex(method->mutex)) {
if (uacpi_unlikely(!uacpi_acquire_aml_mutex(method->mutex, 0xFFFF)))
return UACPI_STATUS_INTERNAL_ERROR;
ret = uacpi_acquire_aml_mutex(method->mutex, 0xFFFF);
if (uacpi_unlikely_error(ret))
return ret;

ret = held_mutexes_array_push(&ctx->held_mutexes, method->mutex);
if (uacpi_unlikely_error(ret)) {
Expand Down
5 changes: 3 additions & 2 deletions source/io.c
Original file line number Diff line number Diff line change
Expand Up @@ -260,8 +260,9 @@ static uacpi_status access_field_unit(
if (uacpi_likely(obj != UACPI_NULL && obj->type == UACPI_OBJECT_MUTEX))
gl = obj->mutex;

if (uacpi_unlikely(!uacpi_acquire_aml_mutex(gl, 0xFFFF)))
return UACPI_STATUS_INTERNAL_ERROR;
ret = uacpi_acquire_aml_mutex(gl, 0xFFFF);
if (uacpi_unlikely_error(ret))
return ret;
}

switch (field->kind) {
Expand Down
66 changes: 44 additions & 22 deletions source/mutex.c
Original file line number Diff line number Diff line change
Expand Up @@ -120,25 +120,47 @@ UACPI_STUB_IF_REDUCED_HARDWARE(
void uacpi_release_global_lock_to_firmware(void)
)

uacpi_status uacpi_acquire_native_mutex_with_timeout(
uacpi_handle mtx, uacpi_u16 timeout
)
{
uacpi_status ret;

if (uacpi_unlikely(mtx == UACPI_NULL))
return UACPI_STATUS_INVALID_ARGUMENT;

ret = uacpi_kernel_acquire_mutex(mtx, timeout);
if (uacpi_likely_success(ret))
return ret;

if (uacpi_unlikely(ret != UACPI_STATUS_TIMEOUT || timeout == 0xFFFF)) {
uacpi_error(
"unexpected status %08X (%s) while acquiring %p (timeout=%04X)\n",
ret, uacpi_status_to_string(ret), mtx, timeout
);
}

return ret;
}

uacpi_status uacpi_acquire_global_lock(uacpi_u16 timeout, uacpi_u32 *out_seq)
{
uacpi_bool did_acquire;
uacpi_status ret;

UACPI_ENSURE_INIT_LEVEL_AT_LEAST(UACPI_INIT_LEVEL_SUBSYSTEM_INITIALIZED);

if (uacpi_unlikely(out_seq == UACPI_NULL))
return UACPI_STATUS_INVALID_ARGUMENT;

UACPI_MUTEX_ACQUIRE_WITH_TIMEOUT(
g_uacpi_rt_ctx.global_lock_mutex, timeout, did_acquire
ret = uacpi_acquire_native_mutex_with_timeout(
g_uacpi_rt_ctx.global_lock_mutex, timeout
);
if (!did_acquire)
return UACPI_STATUS_TIMEOUT;
if (ret != UACPI_STATUS_OK)
return ret;

ret = uacpi_acquire_global_lock_from_firmware();
if (uacpi_unlikely_error(ret)) {
UACPI_MUTEX_RELEASE(g_uacpi_rt_ctx.global_lock_mutex);
uacpi_release_native_mutex(g_uacpi_rt_ctx.global_lock_mutex);
return ret;
}

Expand All @@ -160,7 +182,7 @@ uacpi_status uacpi_release_global_lock(uacpi_u32 seq)

g_uacpi_rt_ctx.global_lock_acquired = UACPI_FALSE;
uacpi_release_global_lock_to_firmware();
UACPI_MUTEX_RELEASE(g_uacpi_rt_ctx.global_lock_mutex);
uacpi_release_native_mutex(g_uacpi_rt_ctx.global_lock_mutex);

return UACPI_STATUS_OK;
}
Expand All @@ -173,10 +195,10 @@ uacpi_bool uacpi_this_thread_owns_aml_mutex(uacpi_mutex *mutex)
return id == uacpi_kernel_get_thread_id();
}

uacpi_bool uacpi_acquire_aml_mutex(uacpi_mutex *mutex, uacpi_u16 timeout)
uacpi_status uacpi_acquire_aml_mutex(uacpi_mutex *mutex, uacpi_u16 timeout)
{
uacpi_thread_id this_id;
uacpi_bool did_acquire;
uacpi_status ret = UACPI_STATUS_OK;

this_id = uacpi_kernel_get_thread_id();
if (UACPI_ATOMIC_LOAD_THREAD_ID(&mutex->owner) == this_id) {
Expand All @@ -185,40 +207,40 @@ uacpi_bool uacpi_acquire_aml_mutex(uacpi_mutex *mutex, uacpi_u16 timeout)
"failing an attempt to acquire mutex @%p, too many recursive "
"acquires\n", mutex
);
return UACPI_FALSE;
return UACPI_STATUS_DENIED;
}

mutex->depth++;
return UACPI_TRUE;
return ret;
}

UACPI_MUTEX_ACQUIRE_WITH_TIMEOUT(mutex->handle, timeout, did_acquire);
if (!did_acquire)
return UACPI_FALSE;
ret = uacpi_acquire_native_mutex_with_timeout(mutex->handle, timeout);
if (ret != UACPI_STATUS_OK)
return ret;

if (mutex->handle == g_uacpi_rt_ctx.global_lock_mutex) {
uacpi_status ret;

ret = uacpi_acquire_global_lock_from_firmware();
if (uacpi_unlikely_error(ret)) {
UACPI_MUTEX_RELEASE(mutex->handle);
return UACPI_FALSE;
uacpi_release_native_mutex(mutex->handle);
return ret;
}
}

UACPI_ATOMIC_STORE_THREAD_ID(&mutex->owner, this_id);
mutex->depth = 1;
return UACPI_TRUE;
return ret;
}

void uacpi_release_aml_mutex(uacpi_mutex *mutex)
uacpi_status uacpi_release_aml_mutex(uacpi_mutex *mutex)
{
if (mutex->depth-- > 1)
return;
return UACPI_STATUS_OK;

if (mutex->handle == g_uacpi_rt_ctx.global_lock_mutex)
uacpi_release_global_lock_to_firmware();

UACPI_ATOMIC_STORE_THREAD_ID(&mutex->owner, UACPI_THREAD_ID_NONE);
UACPI_MUTEX_RELEASE(mutex->handle);
uacpi_release_native_mutex(mutex->handle);

return UACPI_STATUS_OK;
}
Loading

0 comments on commit f38061f

Please sign in to comment.