Re: [RFC XEN PATCH v11 7/8] tools: Add new function to get gsi from dev

["Chen, Jiqian" <Jiqian.Chen@xxxxxxx>]

Hi Anthony,

On 2024/7/2 11:47, Chen, Jiqian wrote:
> On 2024/7/1 15:32, Jan Beulich wrote:
>> On 30.06.2024 14:33, Jiqian Chen wrote:
>>> --- a/tools/libs/ctrl/xc_physdev.c
>>> +++ b/tools/libs/ctrl/xc_physdev.c
>>> @@ -111,3 +111,38 @@ int xc_physdev_unmap_pirq(xc_interface *xch,
>>>      return rc;
>>>  }
>>>  
>>> +int xc_physdev_gsi_from_pcidev(xc_interface *xch, uint32_t sbdf)
>>> +{
>>> +    int rc = -1;
>>> +
>>> +#if defined(__linux__)
>>> +    int fd;
>>> +    privcmd_gsi_from_pcidev_t dev_gsi = {
>>> +        .sbdf = sbdf,
>>> +        .gsi = 0,
>>> +    };
>>> +
>>> +    fd = open("/dev/xen/privcmd", O_RDWR);
>>> +
>>> +    if (fd < 0 && (errno == ENOENT || errno == ENXIO || errno == ENODEV)) {
>>> +        /* Fallback to /proc/xen/privcmd */
>>> +        fd = open("/proc/xen/privcmd", O_RDWR);
>>> +    }
>>> +
>>> +    if (fd < 0) {
>>> +        PERROR("Could not obtain handle on privileged command interface");
>>> +        return rc;
>>> +    }
>>> +
>>> +    rc = ioctl(fd, IOCTL_PRIVCMD_GSI_FROM_PCIDEV, &dev_gsi);
>>> +    close(fd);
>>> +
>>> +    if (rc) {
>>> +        PERROR("Failed to get gsi from dev");
>>> +    } else {
>>> +        rc = dev_gsi.gsi;
>>> +    }
>>> +#endif
>>> +
>>> +    return rc;
>>> +}
>>
>> I realize Anthony had asked to move this out of libxencall, yet doing it like
>> this (without really abstracting away the OS specifics) doesn't look quite
>> right either. In particular the opening of /dev/xen/privcmd looks 
>> questionable
>> to now have yet another instance in yet another library. Couldn't we split
>> osdep_xencall_open(), making available its former half for use here and in 
>> the
>> other two libraries? 
> Hi Anthony, what about your opinion?
What's your opinion about taking " opening of /dev/xen/privcmd " as a single 
function, then use it in this patch and other libraries?

> 
>> Of course that'll still leave the ioctl() invocation, which necessarily is 
>> OS-specific, too.
>>
>> Jan
> 

-- 
Best regards,
Jiqian Chen.