Re: [PATCH v2] livepatch: apply_alternatives() is only used for livepatch

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 07/06/2023 11:19 am, Jan Beulich wrote:
> On 07.06.2023 11:17, Roger Pau Monné wrote:
>> On Wed, Jun 07, 2023 at 11:10:27AM +0200, Jan Beulich wrote:
>>> On 07.06.2023 11:01, Roger Pau Monne wrote:
>>>> Guard it with CONFIG_LIVEPATCH.  Note alternatives are applied at boot
>>>> using _apply_alternatives().
>>>>
>>>> Signed-off-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>>> Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
>>> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
>>> albeit the implicit ack therein is only on the assumption that (apart
>>> from me) it is generally deemed better ...
>>>
>>>> --- a/xen/arch/x86/alternative.c
>>>> +++ b/xen/arch/x86/alternative.c
>>>> @@ -358,11 +358,12 @@ static void init_or_livepatch 
>>>> _apply_alternatives(struct alt_instr *start,
>>>>      }
>>>>  }
>>>>  
>>>> -void init_or_livepatch apply_alternatives(struct alt_instr *start,
>>>> -                                          struct alt_instr *end)
>>>> +#ifdef CONFIG_LIVEPATCH
>>> ... to have the #ifdef than the init_or_livepatch attribute.
>> But the init_or_livepatch attribute doesn't remove the function when
>> !CONFIG_LIVEPATCH,
> Yes up to here.
>
>> so it's not a replacement for the ifdef.
> That depends how you look at it. We don't meaningfully care about a
> few extra bytes in .init.text, I think. So it really is the Misra
> requirement of not having unreferenced symbols which makes the
> difference here.

A different option, which I've recommended before for other reasons, is
to build with function/data sections (already supported for livepatch),
and use --gc-sections at link time.

That would remove examples like this from the final binary, and there's
probably wiggle-room at the MISRA level to say "we comply with this
because the linker deletes it".

~Andrew