Re: [PATCH v2 4/4] xen: mem_access: conditionally compile vm_event.c & monitor.c

[Jan Beulich <jbeulich@suse.com>]

On 31.01.2025 01:33, Tamas K Lengyel wrote:
> On Tue, Jan 21, 2025 at 5:25 AM Sergiy Kibrik <Sergiy_Kibrik@epam.com> wrote:
>> --- a/xen/arch/arm/vsmc.c
>> +++ b/xen/arch/arm/vsmc.c
>> @@ -330,7 +330,8 @@ void do_trap_smc(struct cpu_user_regs *regs, const union hsr hsr)
>>      }
>>
>>      /* If monitor is enabled, let it handle the call. */
>> -    if ( current->domain->arch.monitor.privileged_call_enabled )
>> +    if ( IS_ENABLED(CONFIG_VM_EVENT) &&
>> +         current->domain->arch.monitor.privileged_call_enabled )
>>          rc = monitor_smc();
> 
> Why not wrap this entire if block above in an #ifdef CONFIG_VM_EVENT?
> I think it would be more explicit what code is being compiled that way
> instead of just relying on the compiler optimization to take care of
> removing it.

Well - we generally prefer things being written this way, where possible.
This is to keep as much code as possible exposed to the compiler no
matter what configuration. This way the risk of bit-rotting is a little
lower (e.g. when making changes affecting such a piece of code, but not
noticing the need for a change because things compile fine in whatever
configuration(s) the person tests).

Jan

> The rest of the patch looks fine to me.
> 
> Tamas