On 29.08.2025 16:24, Roger Pau Monné wrote:
> On Wed, Apr 02, 2025 at 03:57:57PM +0200, Jan Beulich wrote:
>> ... before making changes to the involved logic.
>>
>> Signed-off-by: Jan Beulich <jbeul...@suse.com>
>> ---
>> While Andrew validly suggests cf_check isn't a requirement for selecting
>> which function(s) to use (with the non-upstream gcc patch that we're
>> using in CI), that's only because of how the non-upstream patch works.
>> Going function-pointer -> unsigned long -> function-pointer without it
>> being diagnosed that the cf_check is missing is a shortcoming there, and
>> might conceivably be fixed at some point. (Imo any address-taking on a
>> function should require it to be cf_check.) Hence I'd like to stick to
>> using cf_check functions only for passing to test_lookup().
>>
>> With this FAST_SYMBOL_LOOKUP may make sense to permit enabling even
>> when LIVEPATCH=n. Thoughts? (In this case "symbols: centralize and re-
>> arrange $(all_symbols) calculation" would want pulling ahead.)
>>
>> --- a/xen/common/symbols.c
>> +++ b/xen/common/symbols.c
>> @@ -260,6 +260,41 @@ unsigned long symbols_lookup_by_name(con
>> return 0;
>> }
>>
>> +#ifdef CONFIG_SELF_TESTS
>> +
>> +static void __init test_lookup(unsigned long addr, const char *expected)
>> +{
>> + char buf[KSYM_NAME_LEN + 1];
>> + const char *name, *symname;
>> + unsigned long size, offs;
>> +
>> + name = symbols_lookup(addr, &size, &offs, buf);
>> + if ( !name )
>> + panic("%s: address not found\n", expected);
>> + if ( offs )
>> + panic("%s: non-zero offset (%#lx) unexpected\n", expected, offs);
>
> If there's a non-zero offset returned, could you also print the
> returned name? (so use %s+%#lx) there's a change the returned name
> doesn't match what we expect if there's a non-zero offset.
Hmm, perhaps we could, even if that's not the main goal of the test. Note
though that the patch has gone in already, with Jason's R-b.
> The rest LGTM:
>
> Acked-by: Roger Pau Monné <roger....@citrix.com>
Nevertheless, thanks.
Jan
