Re: [PATCH v2 2/2] xen/domain: rewrite emulation_flags_ok()

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.05.2025 01:00, Stefano Stabellini wrote:
> On Tue, 20 May 2025, dmkhn@xxxxxxxxx wrote:
>> On Tue, May 20, 2025 at 05:24:33PM +0200, Jan Beulich wrote:
>>> On 16.05.2025 04:29, dmkhn@xxxxxxxxx wrote:
>>>> --- a/xen/arch/x86/include/asm/domain.h
>>>> +++ b/xen/arch/x86/include/asm/domain.h
>>>> @@ -494,6 +494,12 @@ struct arch_domain
>>>>                                   X86_EMU_PIT | X86_EMU_USE_PIRQ |       \
>>>>                                   X86_EMU_VPCI)
>>>>
>>>> +/* User-selectable features. */
>>>> +#define X86_EMU_OPTIONAL        (X86_EMU_USE_PIRQ)
>>>> +
>>>> +#define X86_EMU_BASELINE        (X86_EMU_ALL & ~(X86_EMU_VPCI | \
>>>> +                                                 X86_EMU_OPTIONAL))
>>>
>>> That is, VPCI is neither baseline nor optional. Certainly at least strange.
> 
> I think Denis tried to keep the code more similar to the original. This
> way it is easier to review the code change and it seems correct. But at
> the same time it is easier to spot inconsistency that were present even
> before the patch.

Right, and that was in response to me complaining on an earlier version
that the behavior was (silently) changed. Yet doing it like this wasn't
the only way to address my comment there. At the same time (or already
before) I raised the question whether having such constants is helpful /
necessary in the first place. And now that their names don't reflect
their purpose, this question becomes yet more relevant.

Jan