Re: [PATCH v2] xen/evtchn: Add design for static event channel signaling

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Julien

> On 10 May 2022, at 1:32 pm, Julien Grall <julien@xxxxxxx> wrote:
> 
> Hi Rahul,
> 
> On 04/05/2022 18:34, Rahul Singh wrote:
>> This patch introduces a new feature to support the signaling between
>> two domains in dom0less system.
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>> v2 changes:
>> - switch to the one-subnode-per-evtchn under xen,domain" compatible node.
>> - Add more detail about event-channel
>> ---
>> docs/designs/dom0less-evtchn.md | 126 ++++++++++++++++++++++++++++++++
> 
> Answering here to also keep the history. On IRC, Bertrand was asking whether 
> we merge design proposal.
> 
> We have merged proposal in the past (e.g. non-cooperative migration) and I 
> would be ready to do it again as it is easier to find them afterwards.
> 
> However, I wonder whether it would be better to turn this proposal to a 
> binding change in misc/arm/device-tree/. Any thoughts?

I am okay with that. I think are you referring to "docs/misc/arm/device-tree/ “ 
> 
>> 1 file changed, 126 insertions(+)
>> create mode 100644 docs/designs/dom0less-evtchn.md
>> diff --git a/docs/designs/dom0less-evtchn.md 
>> b/docs/designs/dom0less-evtchn.md
>> new file mode 100644
>> index 0000000000..62ec8a4009
>> --- /dev/null
>> +++ b/docs/designs/dom0less-evtchn.md
>> @@ -0,0 +1,126 @@
>> +# Signaling support between two domUs on dom0less system
>> +
>> +## Current state: Draft version
>> +
>> +## Proposer(s): Rahul Singh, Bertrand Marquis
>> +
>> +## Problem Statement:
>> +
>> +Dom0less guests would benefit from a statically-defined memory sharing and
>> +signally system for communication. One that would be immediately available 
>> at
>> +boot without any need for dynamic configurations.
>> +
>> +In embedded a great variety of guest operating system kernels exist, many of
>> +which don't have support for xenstore, grant table, or other complex 
>> drivers.
> 
> I am not sure I would consider event channel FIFO a "trival" drivers :).
> 
>> +Some of them are small kernel-space applications (often called "baremetal",
>> +not to be confused with the term "baremetal" used in the data center which
>> +means "without hypervisors") or RTOSes. Additionally, for safety reasons, 
>> users
>> +often need to be able to configure the full system statically so that it can
>> +be verified statically.
>> +
>> +Event channels are very simple and can be added even to baremetal 
>> applications.
>> +This proposal introduces a way to define them statically to make them 
>> suitable
>> +for dom0less embedded deployments.
>> +
>> +## Proposal:
>> +
>> +Event channels are the basic primitive provided by Xen for event 
>> notifications.
>> +An event channel is a logical connection between 2 domains (more 
>> specifically
>> +between dom1,port1, and dom2,port2). Each event has a pending and a masked 
>> bit.
>> +The pending bit indicates the event has been raised. The masked bit is used 
>> by
>> +the domain to prevent the delivery of that specific event. Xen only 
>> performs a
>> +0 → 1 transition on the pending bits and does not touch the mask bit. The
> 
> NIT: I think → is not an ascii character. Can you use "->”?
Ack. 
> 
>> +domain may toggle masked bits in the masked bit field and should clear the
>> +pending bit when an event has been processed
>> +
>> +Events are received by a domain via an interrupt from Xen to the domain,
>> +indicating when an event arrives (setting the bit). Further notifications 
>> are
>> +blocked until the bit is cleared again. Events are delivered asynchronously 
>> to
>> +a domain and are enqueued when the domain is not running.
>> +More information about FIFO based event channel can be found at:
> 
> I think the explanation is fine for a design proposal. If you want to use it 
> as documentation, then I would suggest to clarify there are two different ABI 
> for event channel: FIFO and 2L.
> 
> 2L is the easiest one to implement and for embedded we may want to steer the 
> users towards it.

I will rephrase the sentence as below:

Xen supports two different ABI for event channel FIFO and 2L. More information 
about FIFO based event channel can be found at:

> 
>> +https://xenbits.xen.org/people/dvrabel/event-channels-H.pdf
> 
> It is quite unfortunate that this wasn't merged in docs/. Oh well, no action 
> for you here.
> 
>> +
>> +The event channel communication will be established statically between two
>> +domains (dom0 and domU also) before unpausing the domains after domain 
>> creation.
>> +Event channel connection information between domains will be passed to XEN 
>> via
> 
> NIT: above you are using "Xen". So s/XEN/Xen/ for consistency.

Ack. 
> 
>> +the device tree node. The event channel will be created and established
>> +beforehand in XEN before the domain started. The domain doesn’t need to do 
>> any
> 
> Same here.
> 
> NIT: I think "beforehand" and "before" is redundant.

Ack. 
> 
>> +operation to establish a connection. Domain only needs hypercall
>> +EVTCHNOP_send(local port) to send notifications to the remote guest.
>> +
>> +There is no need to describe the static event channel info in the domU 
>> device
>> +tree. Static event channels are only useful in fully static configurations,
>> +and in those configurations the domU device tree dynamically generated by 
>> Xen
>> +is not needed.
>> +
>> +Under the "xen,domain" compatible node, there need to be sub-nodes with
>> +compatible "xen,evtchn" that describe the event channel connection between 
>> two
>> +domains(dom0 and domU also).
> 
> Below you provided an example between two domUs. Can you provide one between 
> dom0 and a domU?
Yes I will provide an example b/w dom0 and domU in next version.
> 
>> +
>> +The event channel sub-node has the following properties:
>> +
>> +- compatible
>> +
>> + "xen,evtchn"
>> +
>> +- xen,evtchn
>> +
>> + The property is tuples of two numbers
>> + (local-evtchn link-to-foreign-evtchn) where:
>> +
>> + local-evtchn is an integer value that will be used to allocate local port
>> + for a domain to send and receive event notifications to/from the remote
>> + domain.
> Port 0 is reserved and both FIFO/2L have limit on the port numbers.
> 
> I think we should let know the users about those limitations but I am not 
> sure whether the binding is the right place for that.

If you are okay I can add this limitation in this design doc.

>> +
>> + link-to-foreign-evtchn is a single phandle to a remote evtchn to which
>> + local-evtchn will be connected.
> 
> I would consider to relax the wording so a user can create an event channel 
> with the both end in the same domain.
> 
> Implementation wise, it should make no difference as you still need to lookup 
> the domain.

I will rephrase as:
link-to-foreign-evtchn is a single phandle to a foreign evtchn to which
       local-evtchn will be connected.


Regards,
Rahul