Re: [XEN PATCH v10 5/5] domctl: Add XEN_DOMCTL_gsi_permission to grant gsi

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

On 2024/6/24 16:17, Jan Beulich wrote:
> On 21.06.2024 10:20, Chen, Jiqian wrote:
>> On 2024/6/20 18:42, Jan Beulich wrote:
>>> On 20.06.2024 11:40, Chen, Jiqian wrote:
>>>> On 2024/6/18 17:23, Jan Beulich wrote:
>>>>> On 18.06.2024 10:23, Chen, Jiqian wrote:
>>>>>> On 2024/6/17 23:32, Jan Beulich wrote:
>>>>>>> On 17.06.2024 11:00, Jiqian Chen wrote:
>>>>>>>> @@ -1516,14 +1519,39 @@ static void pci_add_dm_done(libxl__egc *egc,
>>>>>>>>              rc = ERROR_FAIL;
>>>>>>>>              goto out;
>>>>>>>>          }
>>>>>>>> -        r = xc_domain_irq_permission(ctx->xch, domid, irq, 1);
>>>>>>>> +#ifdef CONFIG_X86
>>>>>>>> +        /* If dom0 doesn't have PIRQs, need to use 
>>>>>>>> xc_domain_gsi_permission */
>>>>>>>> +        r = xc_domain_getinfo_single(ctx->xch, 0, &info);
>>>>>>>
>>>>>>> Hard-coded 0 is imposing limitations. Ideally you would use DOMID_SELF, 
>>>>>>> but
>>>>>>> I didn't check if that can be used with the underlying hypercall(s). 
>>>>>>> Otherwise
>>>> From the commit 10ef7a91b5a8cb8c58903c60e2dd16ed490b3bcf, DOMID_SELF is 
>>>> not allowed for XEN_DOMCTL_getdomaininfo.
>>>> And now XEN_DOMCTL_getdomaininfo gets domain through rcu_lock_domain_by_id.
>>>>
>>>>>>> you want to pass the actual domid of the local domain here.
>>>> What is the local domain here?
>>>
>>> The domain your code is running in.
>>>
>>>> What is method for me to get its domid?
>>>
>>> I hope there's an available function in one of the libraries to do that.
>> I didn't find relate function.
>> Hi Anthony, do you know?
>>
>>> But I wouldn't even know what to look for; that's a question to (primarily)
>>> Anthony then, who sadly continues to be our only tool stack maintainer.
>>>
>>> Alternatively we could maybe enable XEN_DOMCTL_getdomaininfo to permit
>>> DOMID_SELF.
>> It didn't permit DOMID_SELF since below commit. Does it still have the same 
>> problem if permit DOMID_SELF?
> 
> To answer this, all respective callers would need auditing. However, ...
> 
>> commit 10ef7a91b5a8cb8c58903c60e2dd16ed490b3bcf
>> Author: kfraser@localhost.localdomain <kfraser@localhost.localdomain>
>> Date:   Tue Aug 14 09:56:46 2007 +0100
>>
>>     xen: Do not accept DOMID_SELF as input to DOMCTL_getdomaininfo.
>>     This was screwing up callers that loop on getdomaininfo(), if there
>>     was a domain with domid DOMID_FIRST_RESERVED-1 (== DOMID_SELF-1).
>>     They would see DOMID_SELF-1, then look up DOMID_SELF, which has domid
>>     0 of course, and then start their domain-finding loop all over again!
>>     Found by Kouya Shimura <kouya@xxxxxxxxxxxxxx>. Thanks!
>>     Signed-off-by: Keir Fraser <keir@xxxxxxxxxxxxx>
> 
> ... I view this as a pretty odd justification for the change, when imo the
> bogus loops should instead have been adjusted.
Yes, you are right.
And Anthony suggested to use LIBXL_TOOLSTACK_DOMID to replace 0 domid.
It seems there is no need to change hypercall DOMCTL_getdomaininfo for now?

> 
> Jan
> 
>> diff --git a/xen/common/domctl.c b/xen/common/domctl.c
>> index 09a1e84d98e0..5d29667b7c3d 100644
>> --- a/xen/common/domctl.c
>> +++ b/xen/common/domctl.c
>> @@ -463,19 +463,13 @@ long do_domctl(XEN_GUEST_HANDLE(xen_domctl_t) u_domctl)
>>      case XEN_DOMCTL_getdomaininfo:
>>      {
>>          struct domain *d;
>> -        domid_t dom;
>> -
>> -        dom = op->domain;
>> -        if ( dom == DOMID_SELF )
>> -            dom = current->domain->domain_id;
>> +        domid_t dom = op->domain;
>>
>>          rcu_read_lock(&domlist_read_lock);
>>
>>          for_each_domain ( d )
>> -        {
>>              if ( d->domain_id >= dom )
>>                  break;
>> -        }
>>
>>          if ( d == NULL )
>>          {
> 

-- 
Best regards,
Jiqian Chen.