On 05.11.2021 14:55, Andrew Cooper wrote:
> +void __init xsm_fixup_ops(struct xsm_ops *ops)
> +{
> + /*
> + * We make some simplifying assumptions about struct xsm_ops; that it is
> + * made exclusively of function pointers to non-init text.
> + *
> + * This allows us to walk over struct xsm_ops as if it were an array of
> + * unsigned longs.
> + */
> + unsigned long *dst = _p(ops);
> + unsigned long *src = _p(&dummy_ops);
I'm afraid I consider this an abuse of _p(): It hides casting when
that would better not be hidden (and there's then also a pointless
step through "unsigned long" in the casting). I suppose this is
also why "src" didn't end up "const unsigned long *" - with spelled
out casts the casting away of const might have been more noticable.
> + for ( ; dst < (unsigned long *)(ops + 1); src++, dst++ )
> + {
> + /*
> + * If you encounter this BUG(), then you've most likely added a new
> + * XSM hook but failed to provide the default implementation in
> + * dummy_ops.
> + *
> + * If not, then perhaps a function pointer to an init function, or
> + * something which isn't a function pointer at all.
> + */
> + BUG_ON(!is_kernel_text(*src));
Just as a remark, not a request to change anything: A cause of this
triggering may also be is_kernel_text() not covering all text
sections. Some of what recently we've been talking about informally
may lead to new text section variants appearing, and whether those
would sensibly end up inside [_stext,_etext) is uncertain.
Jan
