Re: [Xen-devel] [PATCH 5/5] xen/arm: add dom0less device assignment info to docs

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Tue, 15 Jan 2019, Julien Grall wrote:
> On 1/3/19 10:07 PM, Stefano Stabellini wrote:
> > On Mon, 24 Dec 2018, Julien Grall wrote:
> > > Hi Stefano,
> > > 
> > > On 12/5/18 5:28 PM, Stefano Stabellini wrote:
> > > > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > > > ---
> > > >    docs/misc/arm/device-tree/booting.txt | 108
> > > > ++++++++++++++++++++++++++++++++++
> > > >    1 file changed, 108 insertions(+)
> > > > 
> > > > diff --git a/docs/misc/arm/device-tree/booting.txt
> > > > b/docs/misc/arm/device-tree/booting.txt
> > > > index 317a9e9..f5aaf8f 100644
> > > > --- a/docs/misc/arm/device-tree/booting.txt
> > > > +++ b/docs/misc/arm/device-tree/booting.txt
> > > > @@ -226,3 +226,111 @@ chosen {
> > > >            };
> > > >        };
> > > >    };
> > > > +
> > > > +
> > > > +Device Assignment
> > > > +=================
> > > > +
> > > > +Device Assignment (Passthrough) is supported by adding another module,
> > > > +alongside the kernel and ramdisk, with the device tree fragment
> > > > +corresponding to the device node to assign to the guest.
> > > > +
> > > > +The dtb sub-node should have the following properties:
> > > > +
> > > > +- compatible
> > > > +
> > > > +    "multiboot,dtb"
> > > 
> > > I would prefer "multiboot,device-tree"
> > 
> > I renamed it
> > 
> > 
> > > > +
> > > > +- reg
> > > > +
> > > > +    Specifies the physical address of the device tree binary fragment
> > > > +    RAM and its length.
> > > > +
> > > > +As an example:
> > > > +
> > > > +        module@0xc000000 {
> > > > +            compatible = "multiboot,dtb", "multiboot,module";
> > > > +            reg = <0x0 0xc000000 0xffffff>;
> > > > +        };
> > > > +
> > > > +The DTB fragment (loaded in memory at 0xc000000 in the example above)
> > > > +should follow the convention explained in
> > > > docs/misc/arm/passthrough.txt.
> > > > +The DTB fragment will be added to the guest device tree, so that the
> > > > +guest kernel will be able to discover the device.
> > > > +
> > > > +In addition, the following properties for each device node in the
> > > > device
> > > > +tree fragment will be used for the device assignment setup:
> > > > +
> > > > +- reg
> > > > +
> > > > +  The reg property specifying the address and size of the device
> > > > memory.
> > > > +  The device memory will be automatically mapped to the guest domain
> > > > +  with a 1:1 mapping (pseudo-physical address == physical address).
> > > 
> > > As said in a previous patch, I don't think this is correct to impose 1:1.
> > > The
> > > user is neither in control of the HW memory map nor the Guest memory map.
> > > So
> > > not many people are going to be able to use it without hacking Xen.
> > 
> > Yes, I'll fix this (and a couple of other issues) by introducing a new
> > "xen,reg" property, instead of trying to reuse the existing reg
> > property.
> > 
> > 
> > > > +
> > > > +- interrupts
> > > > +
> > > > +  The interrupts property specifies the interrupt of the device. They
> > > > +  are automatically routed to the guest domain with virtual irqs ==
> > > > +  physical irqs.
> > > > +
> > > > +- interrupt-parent
> > > > +
> > > > +  It contains a reference to the interrupt controller node. It should
> > > > be
> > > > +  65000, corresponding to GUEST_PHANDLE_GIC.
> > > 
> > > We managed to get away in the toolstack with this property. So why do you
> > > need
> > > it for the hypervisor? Furthermore, this would forbid to passthrough any
> > > other
> > > interrupt controller to the guest.
> > 
> > The toolstack does use GUEST_PHANDLE_GIC today for passthrough
> > interrupts, see tools/libxl/libxl_arm.c:make_root_properties and
> > docs/misc/arm/passthrough.txt:
> > 
> >    * The interrupt-parent property will be added by the toolstack in the
> >      root node;
> 
> You misunderstood my point here. The toolstack is adding the property for the
> user. So why does you require the user to add this property for Dom0less case?

I did misunderstand. interrupt-parent came from the example I had at hand, 
which had
already the property even if it is unnecessary. I comfirmed that it is
superflous and I am happy to remove it.

FYI dtc throws a warning if interrupt-parent is missing:

<stdout>: Warning (interrupts_property): Missing interrupt-parent for 
/passthrough/ethernet@ff0e0000

It makes me guess that is why it was added to the example I had.

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel