Re: [PATCH] evtchn/Flask: pre-allocate node on send path

[Julien Grall <julien@xxxxxxx>]

Hi,

On 24/09/2020 11:53, Jan Beulich wrote:
> xmalloc() & Co may not be called with IRQs off, or else check_lock()
> will have its assertion trigger about locks getting acquired
> inconsistently. Re-arranging the locking in evtchn_send() doesn't seem
> very reasonable, especially since the per-channel lock was introduced to
> avoid acquiring the per-domain event lock on the send paths. Issue a
> second call to xsm_evtchn_send() instead, before acquiring the lock, to
> give XSM / Flask a chance to pre-allocate whatever it may need.
>
> As these nodes are used merely for caching earlier decisions' results,
> allocate just one node in AVC code despite two potentially being needed.
> Things will merely be not as performant if a second allocation was
> wanted, just like when the pre-allocation fails.
>
> Fixes: c0ddc8634845 ("evtchn: convert per-channel lock to be IRQ-safe")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
As discussed on the community call with one comment below:

Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

> ---
> TBD: An even easier fix could be to simply guard xzalloc() by a
>       conditional checking local_irq_is_enabled(), but for a domain
>       sending only interdomain events this would mean AVC's node caching
>       would never take effect on the sending path, as allocation would
>       then always be avoided.
>
> --- a/xen/common/event_channel.c
> +++ b/xen/common/event_channel.c
> @@ -723,6 +723,12 @@ int evtchn_send(struct domain *ld, unsig
>       if ( !port_is_valid(ld, lport) )
>           return -EINVAL;
>   
> +    /*
> +     * As the call further down needs to avoid allocations (due to running
> +     * with IRQs off), give XSM a chance to pre-allocate if needed.
> +     */
> +    xsm_evtchn_send(XSM_HOOK, ld, NULL);
I would suggest to add a comment on top of the evtchn_send callback in 
the XSM hook. This would be helpful for any developer of a new XSM policy.
Cheers,

--
Julien Grall