On 08.07.2024 04:28, Chen, Jiqian wrote:
> On 2024/7/4 21:33, Jan Beulich wrote:
>> On 30.06.2024 14:33, Jiqian Chen wrote:
>>> --- a/xen/include/public/domctl.h
>>> +++ b/xen/include/public/domctl.h
>>> @@ -464,6 +464,12 @@ struct xen_domctl_irq_permission {
>>> uint8_t pad[3];
>>> };
>>>
>>> +/* XEN_DOMCTL_gsi_permission */
>>> +struct xen_domctl_gsi_permission {
>>> + uint32_t gsi;
>>> + uint8_t allow_access; /* flag to specify enable/disable of x86 gsi
>>> access */
>>
>> See above. It's not the field that serves as a flag for the purpose you
>> state, but just the low bit. You want to rename the field (flags?) and
>> #define a suitable constant.
>
> You mean?
>
> struct xen_domctl_gsi_permission {
> uint32_t gsi;
> #define GSI_PERMISSION_MASK 1
> #define GSI_PERMISSION_DISABLE 0
> #define GSI_PERMISSION_ENABLE 1
> uint8_t access_flag; /* flag to specify enable/disable of x86 gsi
> access */
> uint8_t pad[3];
> };
Something along these lines, yes. How far to go is a matter of taste; personally
I'd add just a single #define for now. One aspect is important though: The names
you chose are inappropriate. At the very least a XEN_ prefix is wanted, to
reduce the risk of possible name space collisions. Whether to actually use
XEN_DOMCTL_ is perhaps again a matter of taste - the header isn't consistent at
all in this regard.
Jan
