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.
The rest LGTM:
Acked-by: Roger Pau Monné <roger....@citrix.com>
Thanks, Roger.
