[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH] evtchn: make support for different ABIs tunable

On 07.08.2019 13:20, Eslam Elnikety wrote:
Adding support for FIFO event channel ABI was first introduced in Xen 4.4
(see 88910061ec6). Make this support tunable, since the choice of which
event channel ABI has implications for hibernation. Consider resuming a
pre Xen 4.4 hibernated Linux guest. The guest boot kernel defaults to FIFO
ABI, whereas the resume kernel assumes 2L. This, in turn, results in Xen
and the resumed kernel talking past each other (due to different protocols
FIFO vs 2L).

In order to announce to guests that the event channel ABI does not support
FIFO, the hypervisor returns ENOSYS on init_control operation. When this
operation fails, the guest should continue to use the 2L event channel ABI.
For example, in Linux drivers/xen/events/events_base.c:

     if (fifo_events)
         ret = xen_evtchn_fifo_init();
     if (ret < 0)

and xen_evtchn_fifo_init fails when EVTCHNOP_init_control fails. This commit
does not change the current default behaviour: announce FIFO event channels
ABI support for guests unless explicitly stated otherwise at domaincreate.

Signed-off-by: Eslam Elnikety <elnikety@xxxxxxxxxx>

Seeing that there looks to form agreement that restricting evtchn
variants just like gnttab ones is wanted (I'm still not really
convinced it is the right approach for the problem at hand, but I
do agree having such control may be helpful in general), a few
remarks on the patch itself:

--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -444,6 +444,9 @@ struct domain *domain_create(domid_t domid,
          d->controller_pause_count = 1;
+ if ( d->options & XEN_DOMCTL_CDF_disable_fifo )
+            d->disable_evtchn_fifo = 1;

This wants to be "true".

--- a/xen/common/event_channel.c
+++ b/xen/common/event_channel.c
@@ -1170,6 +1170,11 @@ long do_event_channel_op(int cmd, 
case EVTCHNOP_init_control: {
          struct evtchn_init_control init_control;
+        /* Fail init_control for domains that must use 2l ABI */
+        if ( current->domain->disable_evtchn_fifo )
+            return -ENOSYS;

ENOSYS is not an appropriate error code here. EOPNOTSUPP might
be, and I guess there are more options (like EPERM or EACCES).

--- a/xen/include/xen/sched.h
+++ b/xen/include/xen/sched.h
@@ -338,6 +338,7 @@ struct domain
     struct evtchn  **evtchn_group[NR_EVTCHN_GROUPS]; /* all other buckets */
     unsigned int     max_evtchns;     /* number supported by ABI */
     unsigned int     max_evtchn_port; /* max permitted port number */
+    bool             disable_evtchn_fifo;            /* force 2l ABI */
     unsigned int     valid_evtchns;   /* number of allocated event channels */
     spinlock_t       event_lock;
     const struct evtchn_port_ops *evtchn_port_ops;

I suppose you can find a better place to put this 1-byte field
than between two 32-bit ones, leaving a 3-byte hole.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.