Re: [RFC XEN PATCH v4 4/5] domctl: Use gsi to grant/revoke irq permission

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

On 2024/1/9 17:38, Jan Beulich wrote:
> On 09.01.2024 09:18, Chen, Jiqian wrote:
>> On 2024/1/8 23:05, Roger Pau Monné wrote:
>>> On Mon, Jan 08, 2024 at 09:55:26AM +0100, Jan Beulich wrote:
>>>> On 06.01.2024 02:08, Stefano Stabellini wrote:
>>>>> On Fri, 5 Jan 2024, Jiqian Chen wrote:
>>>>>> --- a/tools/libs/light/libxl_pci.c
>>>>>> +++ b/tools/libs/light/libxl_pci.c
>>>>>> @@ -1418,6 +1418,7 @@ static void pci_add_dm_done(libxl__egc *egc,
>>>>>>      unsigned long long start, end, flags, size;
>>>>>>      int irq, i;
>>>>>>      int r;
>>>>>> +    int gsi;
>>>>>>      uint32_t flag = XEN_DOMCTL_DEV_RDM_RELAXED;
>>>>>>      uint32_t domainid = domid;
>>>>>>      bool isstubdom = libxl_is_stubdom(ctx, domid, &domainid);
>>>>>> @@ -1486,6 +1487,7 @@ static void pci_add_dm_done(libxl__egc *egc,
>>>>>>          goto out_no_irq;
>>>>>>      }
>>>>>>      if ((fscanf(f, "%u", &irq) == 1) && irq) {
>>>>>> +        gsi = irq;
>>>>>
>>>>> A question for Roger and Jan: are we always guaranteed that gsi == irq
>>>>> (also in the PV case)?
>>>>
>>>> Iirc for IO-APIC based IRQs that's always the case;
>>>
>>> I think that's always the case on Linux, because it calls
>>> PHYSDEVOP_map_pirq with index == pirq (see Linux
>>> pci_xen_initial_domain()).  But other OSes could possibly make the
>>> call with pirq == -1 and get a randomly allocated pirq for GSIs.
>> I don't think it's important whether pirq is randomly generated. What's 
>> important is whether irq always equals gsi in xen.
>> If so, we can directly pass in and grant gsi. However, according to Jan's 
>> previous email reply, in the case of Msi, irq may not be equal to gsi, so my 
>> patch cannot meet this situation.
>> I am confusing if the current domain doesn't have PIRQ flag, then regarding 
>> to XEN_DOMCTL_irq_permission, which kind of irq we should grant to caller 
>> domain? The gsi or other irq?
>> Or can we add a check in XEN_DOMCTL_irq_permission, if the current domain 
>> has PRIQ, we can get irq from pirq(like the original implementation), if not 
>> we can assign gsi to irq, and then grant irq. Of course, that needs to 
>> require the caller to pass in both the pirq and gsi.
> 
> I expect MSI will need handling differently from GSIs. When MSI is
> set up for a device (and hence for a domain in possession of that
> device), access ought to be granted right away.
> 
>>> IOW: I don't think the pirq field in xen_domctl_irq_permission can be
>>> altered like proposed here to switch from passing a pirq to a GSI.  A
>>> new hypercall should be introduced that either is GSI specific, or
>>> contains a type field in order to specify the namespace the field
>>> targets.
>> A new hypercall using for granting gsi? If so, how does the caller know to 
>> call which hypercall to grant permission, XEN_DOMCTL_irq_permission or that 
>> new hypercall?
> 
> Either we add a feature indicator, or the caller simply tries the
> new GSI interface first.
I am still not sure how to use and implement it.
Taking pci_add_dm_done as an example, for now its implementation is:
pci_add_dm_done
        xc_physdev_map_pirq
        xc_domain_irq_permission(,,pirq,)
                XEN_DOMCTL_irq_permission

And assume the new hypercall is XEN_DOMCTL_gsi_permission, do you mean:
pci_add_dm_done
        xc_physdev_map_pirq
        ret = xc_domain_gsi_permission(,,gsi,)
                XEN_DOMCTL_gsi_permission
        if ( ret != 0 )
                xc_domain_irq_permission(,,pirq,)
                        XEN_DOMCTL_irq_permission
But if so, I have a question that in XEN_DOMCTL_gsi_permission, when to fail 
and when to success?

Or do you mean:
pci_add_dm_done
        xc_physdev_map_pirq
        ret = xc_domain_irq_permission(,,pirq,)
                XEN_DOMCTL_irq_permission
        if ( ret != 0 )
                xc_domain_gsi_permission(,,gsi,)
                        XEN_DOMCTL_gsi_permission
And in XEN_DOMCTL_gsi_permission, as long as the current domain has the access 
of gsi, then granting gsi to caller should be successful. Right?

> 
>> I mean how the caller know if the current domain has PIRQ or not and when to 
>> pass in pirq number, when to pass in gsi number.
> 
> An interface that's GSI-centric would only ever be passed a GSI
> (of course, I'm inclined to add).
> 
> Jan

-- 
Best regards,
Jiqian Chen.