On Mon, 11 Sep 2023, Michal Orzel wrote: > On 11/09/2023 16:48, Julien Grall wrote: > > 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.si...@arm.com/
yes it was done on purpose > 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. Yes I agree. I don't think we risk breaking anything. I would remove that info from Dom0. Even if we wanted to expose it to Dom0, this is not the right way to do it...
