Re: [PATCH] xen/xsm: Improve alloc/free of evtchn buckets

[Jan Beulich <jbeulich@xxxxxxxx>]

On 18.01.2021 16:06, Andrew Cooper wrote:
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -147,6 +147,14 @@ static bool virq_is_global(unsigned int virq)
>      return true;
>  }
>  
> +static void free_evtchn_bucket(struct domain *d, struct evtchn *bucket)
> +{
> +    if ( !bucket )
> +        return;

You could avoid this since flask_free_security_evtchns() has
a similar check. Alternatively it could be dropped from there.
But even if you want to keep the duplication
Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>

One further aspect to consider though:

> --- a/xen/include/xsm/dummy.h
> +++ b/xen/include/xsm/dummy.h
> @@ -309,12 +309,12 @@ static XSM_INLINE int xsm_evtchn_reset(XSM_DEFAULT_ARG 
> struct domain *d1, struct
>      return xsm_default_action(action, d1, d2);
>  }
>  
> -static XSM_INLINE int xsm_alloc_security_evtchn(struct evtchn *chn)
> +static XSM_INLINE int xsm_alloc_security_evtchns(struct evtchn *chn, 
> unsigned int nr)

I wonder whether we wouldn't better identify the difference
between pointer (to individual element) and array by writing
this (and others below) as

static XSM_INLINE int xsm_alloc_security_evtchns(struct evtchn chn[], unsigned 
int nr)

I think we've done so in a few places already, but of course it
would be a long way to get the entire code base consistent in
this regard. Plus of course while this works fine in function
declarations / definitions, it won't be possible to use for
struct / union fields.

Also it looks like this and further lines have become overly long.

Jan