Re: [PATCH 10/13] powerpc/rtas: improve function information lookups

[Nathan Lynch <nathanl@linux.ibm.com>]

Andrew Donnellan <ajd@linux.ibm.com> writes:
> On Fri, 2022-11-18 at 09:07 -0600, Nathan Lynch wrote:
>> Convert rtas_token() to use a lockless binary search on the function
>> table. Fall back to the old behavior for lookups against names that
>> are not known to be RTAS functions, but issue a warning. rtas_token()
>> is for function names; it is not a general facility for accessing
>> arbitrary properties of the /rtas node. All known misuses of
>> rtas_token() have been converted to more appropriate of_ APIs in
>> preceding changes.
>
> For in-kernel users, why not go all the way: make rtas_token() static
> and use it purely for the userspace API,

Not sure what userspace API refers to here, can you be more specific? I
don't think rtas_token() is exposed to user space.

> and switch kernel users over
> to using rtas_function_index directly?

Well, I have work in progress to transition all rtas_token() users to a
different API, but using opaque symbolic handles rather than exposing
the indexes. Something like:

/*
 * Opaque handle for client code to refer to RTAS functions. All valid
 * function handles are build-time constants prefixed with RTAS_FN_.
 */
typedef struct {
	enum rtas_function_index index;
} rtas_fn_handle_t;

#define rtas_fn_handle(x_) (const rtas_fn_handle_t) { .index = rtas_fnidx(x_), }

#define RTAS_FN_CHECK_EXCEPTION                   rtas_fn_handle(CHECK_EXCEPTION)
#define RTAS_FN_DISPLAY_CHARACTER                 rtas_fn_handle(DISPLAY_CHARACTER)
#define RTAS_FN_EVENT_SCAN                        rtas_fn_handle(EVENT_SCAN)

...

/**
 * rtas_function_token() - RTAS function token lookup.
 * @handle: Function handle, e.g. RTAS_FN_EVENT_SCAN.
 *
 * Context: Any context.
 * Return: the token value for the function if implemented by this platform,
 *         otherwise RTAS_UNKNOWN_SERVICE.
 */
s32 rtas_function_token(const rtas_fn_handle_t handle)
{
	const size_t index = handle.index;
	const bool out_of_bounds = index >= ARRAY_SIZE(rtas_function_table);

	if (WARN_ONCE(out_of_bounds, "invalid function index %zu", index))
		return RTAS_UNKNOWN_SERVICE;

	return rtas_function_table[index].token;
}

But that's a tree-wide change that would touch various drivers, and I
feel like the current series is what I can reasonably hope to get
applied right now.