Re: [XEN PATCH v2 2/5] xen: export get_free_port

[Jan Beulich <jbeulich@suse.com>]

On 25.01.2022 23:49, Stefano Stabellini wrote:
> On Tue, 25 Jan 2022, Jan Beulich wrote:
>> On 25.01.2022 02:10, Stefano Stabellini wrote:
>>> @@ -284,7 +285,27 @@ void evtchn_free(struct domain *d, struct evtchn *chn)
>>>      xsm_evtchn_close_post(chn);
>>>  }
>>>  
>>> -static int evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
>>> +struct evtchn *evtchn_alloc_unbound(struct domain *d, domid_t remote_dom)
>>> +{
>>> +    struct evtchn *chn;
>>> +    int port;
>>> +
>>> +    if ( (port = get_free_port(d)) < 0 )
>>> +        return ERR_PTR(port);
>>> +    chn = evtchn_from_port(d, port);
>>> +
>>> +    evtchn_write_lock(chn);
>>> +
>>> +    chn->state = ECS_UNBOUND;
>>> +    chn->u.unbound.remote_domid = remote_dom;
>>> +    evtchn_port_init(d, chn);
>>> +
>>> +    evtchn_write_unlock(chn);
>>> +
>>> +    return chn;
>>> +}
>>> +
>>> +static int _evtchn_alloc_unbound(evtchn_alloc_unbound_t *alloc)
>>>  {
>>>      struct evtchn *chn;
>>>      struct domain *d;
>>
>> Instead of introducing a clone of this function (with, btw, still
>> insufficient locking), did you consider simply using the existing
>> evtchn_alloc_unbound() as-is, i.e. with the caller passing
>> evtchn_alloc_unbound_t *?
> 
> Yes, we tried that first. Unfortunately the (dummy) XSM check cannot
> work. This is how we would want to call the function:
> 
> 
>     alloc.dom = d->domain_id;
>     alloc.remote_dom = hardware_domain->domain_id;
>     rc = evtchn_alloc_unbound(&alloc);
> 
> 
> This is the implementation of the XSM check:
> 
> static XSM_INLINE int xsm_evtchn_unbound(
>     XSM_DEFAULT_ARG struct domain *d, struct evtchn *chn, domid_t id2)
> {
>     XSM_ASSERT_ACTION(XSM_TARGET);
>     return xsm_default_action(action, current->domain, d);
> }
> 
> 
> Note the usage of current->domain. If you have any suggestions on how to
> fix it please let me know.

As an alternative to Julien's suggestion the function could also simply
be given a new boolean parameter indicating whether to bypass the XSM
check. That would be more explicit than deriving from system state.

Jan