Re: [PATCH v2 2/3] xen/arm: make has_vpci depend on CONFIG_HAS_VPCI

[Jan Beulich <jbeulich@suse.com>]

On 18.07.2023 19:01, Stewart Hildebrand wrote:
> On 7/7/23 02:59, Jan Beulich wrote:
>> On 07.07.2023 03:47, Stewart Hildebrand wrote:
>>> --- a/xen/arch/arm/include/asm/domain.h
>>> +++ b/xen/arch/arm/include/asm/domain.h
>>> @@ -298,8 +298,7 @@ static inline void arch_vcpu_block(struct vcpu *v) {}
>>>
>>>  #define arch_vm_assist_valid_mask(d) (1UL << VMASST_TYPE_runstate_update_flag)
>>>
>>> -/* vPCI is not available on Arm */
>>> -#define has_vpci(d)    ({ (void)(d); false; })
>>> +#define has_vpci(d) ({ IS_ENABLED(CONFIG_HAS_VPCI) && is_hardware_domain(d); })
>>
>> While likely not much of a problem here, I think we should strive to
>> write macros such that their arguments are evaluated exactly once in
>> all cases (for side effects to occur exactly once). When that's not
>> easily possible, so be it, but here it doesn't look problematic to
>> swap both sides of the &&.
> 
> Thanks for pointing this out. Hmm... I'm considering turning it into a static inline function. This would also satisfy MISRA C:2012 Dir 4.9: "A function should be used in preference to a function-like macro where they are interchangeable" [1].

I don't think that'll work prior to us splitting type definitions into
separate headers. You simply cannot deref d at this point (or in fact at
any point within this header), as struct domain hasn't been defined yet.

Jan

> [1] https://gitlab.com/MISRA/MISRA-C/MISRA-C-2012/Example-Suite/-/blob/master/D_04_09.c