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

From: Nick Child <[email-masked]>
Date: Wed, 23 Nov 2022 14:06:51 -0600

On 11/18/22 09:07, Nathan Lynch wrote:
> +static int __init rtas_token_to_function_xarray_init(void)
> +{
> +	int err = 0;
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(rtas_function_table); ++i) {
> +		const struct rtas_function *func = &rtas_function_table[i];
> +		const s32 token = func->token;
> +
> +		if (token == RTAS_UNKNOWN_SERVICE)
> +			continue;
> +
> +		err = xa_err(xa_store(&rtas_token_to_function_xarray,
> +				      token, (void *)func, GFP_KERNEL));
> +		if (err)
> +			break;
> +	}
> +
> +	return err;
> +}
> +arch_initcall(rtas_token_to_function_xarray_init);
> +
> +static const struct rtas_function *rtas_token_to_function(s32 token)
> +{
> +	const struct rtas_function *func;
> +
> +	if (WARN_ONCE(token < 0, "invalid token %d", token))
> +		return NULL;
> +
> +	func = xa_load(&rtas_token_to_function_xarray, (unsigned long)token);
> +
Why typecast token here and not in xa_store?

> +static void __init rtas_function_table_init(void)
> +{
> +	struct property *prop;
> +
> +	for (size_t i = 0; i < ARRAY_SIZE(rtas_function_table); ++i) {
> +		struct rtas_function *curr = &rtas_function_table[i];
> +		struct rtas_function *prior;
> +		int cmp;
> +
> +		curr->token = RTAS_UNKNOWN_SERVICE;
> +
> +		if (i == 0)
> +			continue;
> +		/*
> +		 * Ensure table is sorted correctly for binary search
> +		 * on function names.
> +		 */
> +		prior = &rtas_function_table[i - 1];
> +
> +		cmp = strcmp(prior->name, curr->name);
> +		if (cmp < 0)
> +			continue;
> +
> +		if (cmp == 0) {
> +			pr_err("'%s' has duplicate function table entries\n",
> +			       curr->name);
> +		} else {
> +			pr_err("function table unsorted: '%s' wrongly precedes '%s'\n",
> +			       prior->name, curr->name);
> +		}
> +	}
Just a thought, would it be simpler to use sort()? you already have the
cmp_func implemented for bsearch().

As for the series as a whole:
I am no RTAS expert but was able to build, boot and mess around with new
tracepoints without errors:

Tested-by: Nick Child <[email-masked]>