Re: [PATCH] x86/mce: Compile do_mca() for CONFIG_PV only

[Jan Beulich <jbeulich@xxxxxxxx>]

On 19.11.2024 19:22, Andrew Cooper wrote:
> On 19/11/2024 2:34 pm, Jan Beulich wrote:
>> On 19.11.2024 13:59, Andrew Cooper wrote:
>>> Eclair reports a Misra Rule 8.4 violation; that do_mca() can't see it's
>>> declaration.  It turns out that this is a consequence of do_mca() being
>>> PV-only, and the declaration being compiled out in !PV builds.
>>>
>>> Therefore, arrange for do_mca() to be compiled out in !PV builds.  This in
>>> turn requires a number of static functions to become static inline.
>> Which generally we advocate against.
> 
> It's `static inline` or `static __maybe_unused`, but I refer you to to
> the Matrix conversation on June 24th on the matter.

Well, and the outcome was __maybe_unused there. For consistency I'd therefore
like to ask that you use __maybe_unused then here, too.

As an aside, it would have helped if you had said which channel that discussion
had been on. Scrolling back this far sadly doesn't work very nicely on the
element.io interface (or I simply don't know how to make it go to a specific
date), and I ended up trying XenDevel, then the committers channel, and only
then the x86 one.

>>  The #ifdef variant you pointed at on
>> Matrix wasn't all that bad.
> 
> It worked as a test, but ifdefary like that is a maintenance nightmare.
> 
>>  Plus ...
>>
>>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
>>> ---
>>> CC: Jan Beulich <JBeulich@xxxxxxxx>
>>> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
>>> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
>>> CC: consulting@xxxxxxxxxxx <consulting@xxxxxxxxxxx>
>>>
>>> Bloat-o-meter on a !PV build reports:
>>>
>>>   add/remove: 0/6 grow/shrink: 0/0 up/down: 0/-3717 (-3717)
>>>   Function                                     old     new   delta
>>>   x86_mc_mceinject                              31       -     -31
>>>   do_mca.cold                                  117       -    -117
>>>   x86_mc_msrinject                             147       -    -147
>>>   x86_mc_msrinject.cold                        230       -    -230
>>>   do_mc_get_cpu_info                           500       -    -500
>>>   do_mca                                      2692       -   -2692
>> ... what's the effect of the addition of "inline" on a PV=y build? By
>> using the keyword, we may end up talking the compiler into inlining of
>> code that better wouldn't be inlined.
> 
> xen.git/xen$ ../scripts/bloat-o-meter xen-syms-{before,after}
> add/remove: 0/0 grow/shrink: 0/0 up/down: 0/0 (0)
> Function                                     old     new   delta
> Total: Before=3901801, After=3901801, chg +0.00%
> xen.git/xen$ diff -u dis-{before,after}
> --- dis-before    2024-11-19 18:08:02.284091931 +0000
> +++ dis-after    2024-11-19 18:08:24.491035756 +0000
> @@ -1,5 +1,5 @@
>  
> -xen-syms-before:     file format elf64-x86-64
> +xen-syms-after:     file format elf64-x86-64
>  
>  
>  Disassembly of section .text:
> 
> xen.git/xen$

Good. Preferably with __maybe_unused
Acked-by: Jan Beulich <jbeulich@xxxxxxxx>

Jan