Re: [PATCH] xen/arm: Skip Xen specific nodes/properties from hwdom /chosen node

[Michal Orzel <michal.orzel@xxxxxxx>]

On 11/09/2023 16:48, Julien Grall wrote:
> Caution: This message originated from an External Source. Use proper caution 
> when opening attachments, clicking links, or responding.
> 
> 
> On 11/09/2023 15:01, Michal Orzel wrote:
>> Hi Julien,
>>
>> On 11/09/2023 15:14, Julien Grall wrote:
>>>
>>>
>>> Hi,
>>>
>>> On 11/09/2023 13:34, Michal Orzel wrote:
>>>> Host device tree nodes under /chosen with compatible string
>>>> "xen,evtchn-v1", "xen,domain-shared-memory-v1" are Xen specific and not
>>>> meant to be exposed to hardware domain.
>>>
>>> So, how dom0 is meant to discover the static event channel, shared
>>> memory it can use?
>>
>> For static shared memory:
>> A node with this compatible is only meant for Xen since it contains 
>> information like host-guest mapping.
>> Xen creates a different node for guests under /reserved-memory.
>> Linux binding:
>> https://elixir.bootlin.com/linux/latest/source/Documentation/devicetree/bindings/reserved-memory/xen,shared-memory.txt
>> Xen node generation:
>> https://xenbits.xen.org/gitweb/?p=xen.git;a=blob;f=xen/arch/arm/domain_build.c;hb=HEAD#l1407
> 
> Ah good point. I agree with this one.
> 
>>
>> For static event channels:
>> This is a bit weird so let me put it in a different way.
>> 1) Xen does not create any node for static evtchn for domU.
>> 2) The booting.txt states:
>> 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.
>> 3) The "xen,evtchn" property specifies the local port as well as phandle of 
>> domU node.
>> dom0 does not have access to domU nodes, therefore exposing a property with 
>> not existing phandle
>> makes me think that:
> 
> You have a point for the phandle. Yet, this is the only way dom0 can
> find the event channel today. As we exposed it, I don't think we can
> remove it until we have a proper replacement.
> 
> We might get away if the feature is not supported it at all. But there
> is no statement, so it is a little unclear whether the feature is meant
> to be in technical preview.
> 
> In any case, I think the commit message deserve a bit more explanation
> as hiding "xen,evtchn-v1"/"xen,domain-shared-memory-v1" is not
> uncontroversial.
> 
>> a) point 2) applies to dom0 as well and we should hide this node or > b) 
>> there is a missing property for both dom0 and domUs to tell them
> about static local port in a proper way
>>
>> Exposing Xen specific evtchn node only to dom0 and not to domU with required 
>> information happen to be found as first value
>> in xen,evtchn is definitely not a proper solution.
> 
> My concern here is we exposed such information to dom0. So as I said
> above, I don't think we can simply remove it as there is no other way to
> find such information today.
> 
> It doesn't matter that it wasn't exposed to domU.
> 
>> Also, there is no Linux binding for it.
> 
> AFAIK the static event channel support was not added in Linux until very
> recently. Also, I think the kernel doesn't directly use the static event
> channel. So it is possible, this is just an overlook.

I've found this thread in which Stefano, Rahul and Bertrand agrees on not 
exposing
any dt property and the rationale behind:
https://patchwork.kernel.org/project/xen-devel/patch/4836304496e6fbbea41348ed8cc9fcf6b0f3e893.1648049827.git.rahul.singh@xxxxxxx/

I would not call exposing node to dom0 as something done deliberately given 
that it happens automatically by copying. So my understanding is
that we did not want to expose any node and dom0 case was overlooked (since 
done automatically).

Exposing half the interface (from system POV) in a way it should not be done 
(phandle) is not great IMO.
In any case, if you insist on keeping xen,evtchn, I can leave with it.

This feature is marked as tech preview with no Linux binding in place so I 
would not be worried.

~Michal