On 2024/8/2 14:27, Jan Beulich wrote:
> On 02.08.2024 05:10, Chen, Jiqian wrote:
>> On 2024/8/1 19:06, Roger Pau Monné wrote:
>>> On Mon, Jul 08, 2024 at 07:41:21PM +0800, Jiqian Chen wrote:
>>>> --- a/xen/include/public/domctl.h
>>>> +++ b/xen/include/public/domctl.h
>>>> @@ -464,6 +464,13 @@ struct xen_domctl_irq_permission {
>>>> uint8_t pad[3];
>>>> };
>>>>
>>>> +/* XEN_DOMCTL_gsi_permission */
>>>> +struct xen_domctl_gsi_permission {
>>>> + uint32_t gsi;
>>>> +#define XEN_DOMCTL_GSI_PERMISSION_MASK 1
>>>
>>> IMO this would be better named GRANT or similar, maybe something like:
>>>
>>> /* Low bit used to signal grant/revoke action. */
>>> #define XEN_DOMCTL_GSI_REVOKE 0
>>> #define XEN_DOMCTL_GSI_GRANT 1
>>>
>>>> + uint8_t access_flag; /* flag to specify enable/disable of x86 gsi
>>>> access */
>>>> + uint8_t pad[3];
>>>
>>> We might as well declare the flags field as uint32_t and avoid the
>>> padding field.
>> So, should this struct be like below? Then I just need to check whether
>> everything except the lowest bit is 0.
>> struct xen_domctl_gsi_permission {
>> uint32_t gsi;
>> /* Lowest bit used to signal grant/revoke action. */
>> #define XEN_DOMCTL_GSI_REVOKE 0
>> #define XEN_DOMCTL_GSI_GRANT 1
>> #define XEN_DOMCTL_GSI_PERMISSION_MASK 1
>> uint32_t access_flag; /* flag to specify enable/disable of x86 gsi
>> access */
>> };
>
> Yet then why "access_flags"? You can't foresee what meaning the other bits may
> gain. That meaning may (and likely will) not be access related at all.
OK, just "uint32_t flags".
>
> Jan
--
Best regards,
Jiqian Chen.
