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

[Julien Grall <julien@xxxxxxx>]

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.
Cheers,

--
Julien Grall