Re: [PATCH v4 09/11] silo: remove circular xsm hook call

[Jan Beulich <jbeulich@xxxxxxxx>]

On 03.09.2021 21:06, Daniel P. Smith wrote:
> SILO implements a few XSM hooks to extended the decision logic beyond
> what is defined in the dummy/default policy. For each of the hooks, it
> falls back to the dummy/default policy. The fall back is done a slight
> round-about way. This commit makes the direct call to the default policy's
> logic, xsm_default_action().

Again it's not clear to me what you're finding wrong here. The way it's
done is not as direct as it could be, but going through the extra layer
allows the functions to document things at the same time. You lose not
only that documentation, but also ...

> @@ -43,7 +44,7 @@ static int silo_evtchn_unbound(struct domain *d1, struct 
> evtchn *chn,
>      else
>      {
>          if ( silo_mode_dom_check(d1, d2) )
> -            rc = xsm_evtchn_unbound(d1, chn, id2);
> +            rc = xsm_default_action(XSM_TARGET, current->domain, d1);

... will need to sync changes to the dummy xsm_evtchn_unbound(), no
matter how unlikely such may be, back to here. This would be quite
easy to forget.

But maybe I'm overlooking something where how things are really gets in
the way of something you mean to do in the remaining two patches (or
later)?

>  static int silo_grant_copy(struct domain *d1, struct domain *d2)
>  {
>      if ( silo_mode_dom_check(d1, d2) )
> -        return xsm_grant_copy(d1, d2);
> +        return xsm_default_action(XSM_HOOK, d1, d2);
>      return -EPERM;
>  }
>  
> @@ -86,14 +87,14 @@ static int silo_argo_register_single_source(const struct 
> domain *d1,
>                                              const struct domain *d2)
>  {
>      if ( silo_mode_dom_check(d1, d2) )
> -        return xsm_argo_register_single_source(d1, d2);
> +        return 0;
>      return -EPERM;
>  }
>  
>  static int silo_argo_send(const struct domain *d1, const struct domain *d2)
>  {
>      if ( silo_mode_dom_check(d1, d2) )
> -        return xsm_argo_send(d1, d2);
> +        return 0;
>      return -EPERM;
>  }

This would then also avoid introducing the anomaly observed by Andrew here.
And in fact the Argo dummy functions may be a good example where a change
might happen down the road - them being all empty doesn't seem quite right
to me.

Jan