Re: [PATCH 17/24] XSM: make Argo hooks well-formed ones

On 8/4/26 3:53 AM, Jan Beulich wrote:
> On 03.08.2026 23:01, Jason Andryuk wrote:
>> On 2026-07-28 09:22, Jan Beulich wrote:
>>> --- a/xen/include/xsm/dummy.h
>>> +++ b/xen/include/xsm/dummy.h
>>
>>> @@ -751,27 +751,32 @@ static XSM_INLINE int xsm_dm_op(XSM_DEFA
>>>    #endif
>>>    
>>>    #ifdef CONFIG_ARGO
>>> -static XSM_INLINE int xsm_argo_enable(const struct domain *d)
>>> +
>>> +static XSM_INLINE int xsm_argo_enable(XSM_DEFAULT_ARG const struct domain *d)
>>>    {
>>> -    return 0;
>>> +    XSM_ASSERT_ACTION(XSM_HOOK);
>>> +    return xsm_default_action(action, current->domain, d);
>>
>> This one I think should be
>>       return xsm_default_action(action, d, NULL);
>>
>> Usually current is passed in for the check, but for domain_create() ->
>> argo_init() it is the under-construction domain.
> 
> And in that case we want to make sure that current->domain may enable Argo
> for d.
> 

Jason is correct here, the design of Argo is that authorization is 
granted by the hypervisor and not by any domains. It's why we coined the 
term Hypervisor Mediated eXchange (HMX). The source of that 
authorization was designed to be XSM. As such the intent of this check 
is, "is d allowed to register rings".

>>>    }
>>>    
>>>    static XSM_INLINE int xsm_argo_register_single_source(
>>> -    const struct domain *d, const struct domain *t)
>>> +    XSM_DEFAULT_ARG const struct domain *d, const struct domain *t)
>>>    {
>>> -    return 0;
>>> +    XSM_ASSERT_ACTION(XSM_HOOK);
>>> +    return xsm_default_action(action, d, t);
>>>    }
>>>    
>>>    static XSM_INLINE int xsm_argo_register_any_source(
>>> -    const struct domain *d)
>>> +    XSM_DEFAULT_ARG const struct domain *d)
>>>    {
>>> -    return 0;
>>> +    XSM_ASSERT_ACTION(XSM_HOOK);
>>> +    return xsm_default_action(action, current->domain, d);
>>
>> Similarly:
>>       return xsm_default_action(action, d, NULL);
>>
>> The single call is:
>> xsm_argo_register_any_source(currd);
> 
> There being just a single call puts this on the edge. If there was another
> one not passing current->domain, I think the same argument as above would
> hold here. And the general concept is what I think should matter when
> writing the dummy implementations.
> 

As explained above, the check is asking the hypervisor if the domain 
currd is allowed to register wildcard rings.


>> These argo hooks all pass in their arguments explicitly, so I think we
>> should do that and not use current.  (The send and register hooks could
>> use current, and that could make sense as those map to hypercalls.  But
>> it is correct today with the explicit arguments.)
>>
>> With the changes:
>> Reviewed-by: Jason Andryuk <[email protected]>
> 
> Thanks, but no - unless I misunderstand how permissions are intended to
> work here, I don't think I can make the changes requested, and hence I
> can't apply the R-b.
> 

Please apply the changes requested.

v/r,
dps