Re: [PATCH 3/3] xen/evtchn: Clean up teardown handling

[Jan Beulich <jbeulich@xxxxxxxx>]

On 21.12.2020 19:14, Andrew Cooper wrote:
> First of all, rename the evtchn APIs:
>  * evtchn_destroy       => evtchn_teardown
>  * evtchn_destroy_final => evtchn_destroy

I wonder in how far this is going to cause confusion with backports
down the road. May I suggest to do only the first of the two renames,
at least until in a couple of year's time? Or make the second rename
to e.g. evtchn_cleanup() or evtchn_deinit()?

> RFC.  While testing this, I observed this, after faking up an -ENOMEM in
> dom0's construction:
> 
>   (XEN) [2020-12-21 16:31:20] NX (Execute Disable) protection active
>   (XEN) [2020-12-21 16:33:04]
>   (XEN) [2020-12-21 16:33:04] ****************************************
>   (XEN) [2020-12-21 16:33:04] Panic on CPU 0:
>   (XEN) [2020-12-21 16:33:04] Error creating domain 0
>   (XEN) [2020-12-21 16:33:04] ****************************************
> 
> XSA-344 appears to have added nearly 2 minutes of wallclock time into the
> domain_create() error path, which isn't ok.
> 
> Considering that event channels haven't even been initialised in this
> particular scenario, it ought to take ~0 time.  Even if event channels have
> been initalised, none can be active as the domain isn't visible to the system.

evtchn_init() sets d->valid_evtchns to EVTCHNS_PER_BUCKET. Are you
suggesting cleaning up one bucket's worth of unused event channels
takes two minutes? If this is really the case, and considering there
could at most be unbound Xen channels, perhaps we could avoid
calling evtchn_teardown() from domain_create()'s error path? We'd
need to take care of the then missing accounting (->active_evtchns
and ->xen_evtchns), but this should be doable.

Jan