Nick Child <nnac...@linux.ibm.com> writes:
> 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?
No good reason. I'll add it to the xa_store() call site.
>> +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().
It's an option, but I think a tradeoff is that we would have to
sacrifice some const-ness in the data structures (i.e. remove the const
qualifier from struct rtas_function's fields). And the table has to be
in *some* order, so it may as well be sorted by name from the start.
That said, I don't love resorting to a boot-time check for this. We
could sidestep the issue by generating the C code for the table and
indexes at build time, but it's hard to justify the effort when the set
of RTAS functions changes very slowly over time.
> 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 <nnac...@linux.ibm.com>
Thanks for testing and reviewing!
