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

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

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;

interrupt-parent always points to the guest GIC node, for virtual
interrupts such as the vuart, as well as physical passthrough
interrupts. Also see tools/libxl/libxl_arm.c:fdt_property_interrupts and
tools/libxl/libxl_arm.c:make_gicv2_node.

I don't think the scenario you are describing is supported today.


> > +
> > +- path
> > +
> > +  A new string property named "path" holds the path in the host device
> > +  tree to the corresponding device node.
> 
> This name is too generic. Please prepend "xen,"

OK, no prob


> > +
> > +The following is a real-world example of a device tree fragment for the
> > +network card on Xilinx MPSoC boards:
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +    #address-cells = <0x2>;
> > +    #size-cells = <0x1>;
> > +
> > +    passthrough {
> > +        compatible = "simple-bus";
> > +        ranges;
> > +        #address-cells = <0x2>;
> > +        #size-cells = <0x1>;
> > +
> > +        misc_clk {
> > +            #clock-cells = <0x0>;
> > +            clock-frequency = <0x7735940>;
> > +            compatible = "fixed-clock";
> > +            linux,phandle = <0x1>;
> > +            phandle = <0x1>;
> > +        };
> > +
> > +        ethernet@ff0e0000 {
> > +            compatible = "cdns,zynqmp-gem";
> > +            status = "okay";
> > +            interrupt-parent = <0xfde8>;
> > +            interrupts = <0x0 0x3f 0x4 0x0 0x3f 0x4>;
> > +            reg = <0x0 0xff0e0000 0x1000>;
> > +            clock-names = "pclk", "hclk", "tx_clk", "rx_clk";
> > +            #address-cells = <0x1>;
> > +            #size-cells = <0x0>;
> > +            clocks = <0x1 0x1 0x1 0x1>;
> > +            phy-mode = "rgmii-id";
> > +            xlnx,ptp-enet-clock = <0x0>;
> > +            local-mac-address = [00 0a 35 00 22 01];
> > +            phy-handle = <0x2>;
> > +            path = "/amba/ethernet@ff0e0000";
> > +
> > +            phy@c {
> > +                reg = <0xc>;
> > +                ti,rx-internal-delay = <0x8>;
> > +                ti,tx-internal-delay = <0xa>;
> > +                ti,fifo-depth = <0x1>;
> > +                ti,rxctrl-strap-worka;
> > +                linux,phandle = <0x2>;
> > +                phandle = <0x2>;
> > +            };
> > +        };
> > +    };
> > +};

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