Re: [PATCH v4 7/7] xen/arm: introduce xen-evtchn dom0less property

[Rahul Singh <Rahul.Singh@xxxxxxx>]

Hi Stefano,

> On 6 Sep 2022, at 11:22 pm, Stefano Stabellini <sstabellini@xxxxxxxxxx> wrote:
> 
> On Tue, 6 Sep 2022, Rahul Singh wrote:
>> Introduce a new sub-node under /chosen node to establish static event
>> channel communication between domains on dom0less systems.
>> 
>> An event channel will be created beforehand to allow the domains to
>> send notifications to each other.
>> 
>> Signed-off-by: Rahul Singh <rahul.singh@xxxxxxx>
>> ---
>> Changes in v4:
>> - move documentation to common place for evtchn node in booting.txt
>> - Add comment why we use dt_device_static_evtchn_created()
>> - check if dt_get_parent() returns NULL
>> - fold process_static_evtchn_node() in alloc_static_evtchn()
>> Changes in v3:
>> - use device-tree used_by to find the domain id of the evtchn node.
>> - add new static_evtchn_create variable in struct dt_device_node to
>>   hold the information if evtchn is already created.
>> - fix minor comments
>> Changes in v2:
>> - no change
>> ---
>> docs/misc/arm/device-tree/booting.txt |  98 +++++++++++++++++
> 
> I have just reviewed the binding, only three minor comments below.
> Everything looks good.

Thanks for reviewing the code.
> 
> 
>> xen/arch/arm/domain_build.c           | 147 ++++++++++++++++++++++++++
>> xen/arch/arm/include/asm/setup.h      |   1 +
>> xen/arch/arm/setup.c                  |   2 +
>> xen/include/xen/device_tree.h         |  16 +++
>> 5 files changed, 264 insertions(+)
>> 
>> diff --git a/docs/misc/arm/device-tree/booting.txt 
>> b/docs/misc/arm/device-tree/booting.txt
>> index 1b0dca1454..c8329b73e5 100644
>> --- a/docs/misc/arm/device-tree/booting.txt
>> +++ b/docs/misc/arm/device-tree/booting.txt
>> @@ -382,3 +382,101 @@ device-tree:
>> 
>> This will reserve a 512MB region starting at the host physical address
>> 0x30000000 to be exclusively used by DomU1.
>> +
>> +Static Event Channel
>> +====================
>> +The event channel communication will be established statically between two
>> +domains (dom0 and domU also). Event channel connection information between
>> +domains will be passed to Xen via the device tree node. The event channel
>> +will be created and established in Xen before the domain started. The domain
>> +doesn???t need to do any operation to establish a connection. Domain only
> 
> doesn't
> 
> better to use ASCII if possible

Ack.
> 
> 
>> +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.
>> +
>> +To enable the event-channel interface for domU guests include the
>> +"xen,enhanced = "no-xenstore"" property in the domU Xen device tree node.
> 
> double ""

Ack. 
> 
> 
>> +
>> +Under the "xen,domain" compatible node for domU, there needs to be sub-nodes
>> +with compatible "xen,evtchn" that describe the event channel connection
>> +between two domUs. For dom0, there needs to be sub-nodes with compatible
>> +"xen,evtchn" under the chosen node.
>> +
>> +The static event channel 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. Maximum supported value is 2^17 for FIFO ABI and 4096 for 2L 
>> ABI.
>> +    It is recommended to use low event channel IDs.
>> +
>> +    link-to-foreign-evtchn is a single phandle to a remote evtchn to which
>> +    local-evtchn will be connected.
>> +
>> +Example
>> +=======
>> +
>> +chosen {
>> +
>> +    /* one sub-node per local event channel */
> 
> It would be good to say that this is for dom0 in the comment, e.g.:
> 
> /* this is for Dom0 */

Ack.

Regards,
Rahul