On 30.11.2023 18:06, Stewart Hildebrand wrote:
> On 11/30/23 03:33, Jan Beulich wrote:
>> On 30.11.2023 03:47, Stewart Hildebrand wrote:
>>> On 11/14/23 04:13, Jan Beulich wrote:
>>>> On 13.11.2023 23:21, Stewart Hildebrand wrote:
>>>>> --- a/xen/arch/x86/include/asm/domain.h
>>>>> +++ b/xen/arch/x86/include/asm/domain.h
>>>>> @@ -503,6 +503,8 @@ struct arch_domain
>>>>> #define has_vpit(d) (!!((d)->arch.emulation_flags & X86_EMU_PIT))
>>>>> #define has_pirq(d) (!!((d)->arch.emulation_flags &
>>>>> X86_EMU_USE_PIRQ))
>>>>>
>>>>> +#define arch_needs_vpci(d) ({ (void)(d); false; })
>>>>
>>>> See my comments on the v5 thread on both this and ...
>>>
>>> So, the goal here is to return true for a PVH dom0, and false otherwise
>>> (for now). Since dom0 can't feasibly be full HVM, and is_hvm_domain(d)
>>> returns true for PVH, how about the following?
>>>
>>> /* TODO: re-visit when vPCI is enabled for PVH domUs. */
>>> #define arch_needs_vpci(d) ({ \
>>> const struct domain *_d = (d); \
>>> is_hardware_domain(_d) && is_hvm_domain(_d); })
>>
>> Looks okay to me, except for the leading underscore in _d (see respective
>> Misra guidelines, merely re-enforcing what the C standard says).
>
> Right. If I'm interpreting the standards correctly, it looks like a trailing
> underscore would work, seeing as we haven't adopted MISRA C:2012 Dir 4.5
> (yet?).
Adopting the respective Misra rule would only affirm that we should adhere
to the C spec. Us being free-standing (and hence no runtime library involved)
may have been an argument towards more relaxed treatment, but imo never was a
good justification. And yes, trailing underscores is what I have been
recommending, but some of the other maintainers don't really like them
(without, iirc, indicating what else to use as an underlying naming scheme).
Jan
