On 25.11.2020 12:10, Paul Durrant wrote: >> From: Jan Beulich <[email protected]> >> Sent: 25 November 2020 09:20 >> >> On 24.11.2020 20:17, Paul Durrant wrote: >>> From: Paul Durrant <[email protected]> >>> >>> ...to control the visibility of the FIFO event channel operations >>> (EVTCHNOP_init_control, EVTCHNOP_expand_array, and EVTCHNOP_set_priority) to >>> the guest. >>> >>> These operations were added to the public header in commit d2d50c2f308f >>> ("evtchn: add FIFO-based event channel ABI") and the first implementation >>> appeared in the two subsequent commits: edc8872aeb4a ("evtchn: implement >>> EVTCHNOP_set_priority and add the set_priority hook") and 88910061ec61 >>> ("evtchn: add FIFO-based event channel hypercalls and port ops"). Prior to >>> that, a guest issuing those operations would receive a return value of >>> -ENOSYS (not implemented) from Xen. Guests aware of the FIFO operations but >>> running on an older (pre-4.4) Xen would fall back to using the 2-level event >>> channel interface upon seeing this return value. >>> >>> Unfortunately the uncontrolable appearance of these new operations in Xen >>> 4.4 >>> onwards has implications for hibernation of some Linux guests. During resume >>> from hibernation, there are two kernels involved: the "boot" kernel and the >>> "resume" kernel. The guest boot kernel may default to use FIFO operations >>> and >>> instruct Xen via EVTCHNOP_init_control to switch from 2-level to FIFO. On >>> the >>> other hand, the resume kernel keeps assuming 2-level, because it was >>> hibernated >>> on a version of Xen that did not support the FIFO operations. >> >> And the alternative of the boot kernel issuing EVTCHNOP_reset has >> other unwanted consequences. Maybe worth mentioning here, as >> otherwise this would look like the obvious way to return to 2-level >> mode? >> >> Also, why can't the boot kernel be instructed to avoid engaging >> FIFO mode? >> > > Both of those are, of course, viable alternatives if the guest can be > modified. The problem we need to work around is guest that are already > out there and cannot be updated.
Making use of EVTCHNOP_reset indeed would require a change to the kernel. But Linux has a command line option to suppress use of FIFO event channels, so I can't see why the boot kernel couldn't be passed this flag without any modification to the binary. >>> To maintain compatibility it is necessary to make Xen behave as it did >>> before the new operations were added and hence the code in this patch >>> ensures >>> that, if XEN_DOMCTL_CDF_disable_fifo is set, the FIFO event channel >>> operations >>> will again result in -ENOSYS being returned to the guest. >> >> Are there indeed dependencies on the precise return value anywhere? >> If so, the generally inappropriate use (do_event_channel_op()'s >> default case really would also need switching) would want a brief >> comment, so it'll be understood by readers that this isn't code to >> derive other code from. If not, -EPERM or -EACCES perhaps? >> > > The patch, as stated, is reverting behaviour and so the -ENOSYS really > needs to stay since it is essentially ABI now. I am not aware of guest > code that will, in fact, die if it sees -EPERM or -EACCES instead but > there may be such code. The only safe thing to do is to make things > look like the used to. I don't think specific error codes can be considered "ABI". Not the least because, if there are multiple causes for an error, it ought to be undefined which error gets returned. A guest not falling back to 2-level on _any_ error here is basically setting itself up for eventual failure because of e.g. getting back -ENOMEM. Or someone deciding to add an XSM check to the function. As said, I'm of the opinion that the other -ENOSYS ought to be replaced as well, which of course would be precluded if this was considered "ABI". Jan
