[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH v4 8/8] xen/arm: add dom0-less device assignment info to docs



On Wed, 11 Sep 2019, Julien Grall wrote:
> Hi Stefano,
> 
> On 8/21/19 4:53 AM, Stefano Stabellini wrote:
> > Add info about the SPI used for the virtual pl011.
> > 
> > Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
> > 
> > ---
> > Changes in v4:
> > - fix spelling
> > - add "multiboot,module"
> > - improve commit message
> > - improve doc
> > - expand the nr_spis and vpl011 sections and include information about
> >    the vpl011 SPI
> > - move passthrough information to docs/misc/arm/passthrough.txt
> > 
> > Changes in v3:
> > - add nr_spis
> > - change description of interrupts and interrupt-parent
> > 
> > Changes in v2:
> > - device tree fragment loaded in cacheable memory
> > - rename multiboot,dtb to multiboot,device-tree
> > - rename "path" to "xen,path"
> > - add a note about device memory mapping
> > - introduce xen,reg
> > - specify only the GIC is supported
> > ---
> >   docs/misc/arm/device-tree/booting.txt |  44 ++++++++++-
> >   docs/misc/arm/passthrough.txt         | 105 ++++++++++++++++++++++++++
> >   2 files changed, 148 insertions(+), 1 deletion(-)
> > 
> > diff --git a/docs/misc/arm/device-tree/booting.txt
> > b/docs/misc/arm/device-tree/booting.txt
> > index 317a9e962a..0b850c0591 100644
> > --- a/docs/misc/arm/device-tree/booting.txt
> > +++ b/docs/misc/arm/device-tree/booting.txt
> > @@ -146,7 +146,18 @@ with the following properties:
> >     - vpl011
> >   -    An empty property to enable/disable a virtual pl011 for the guest to
> > use.
> > +    An empty property to enable/disable a virtual pl011 for the guest to
> > +    use. The virtual pl011 uses SPI number 32 (see GUEST_VPL011_SPI).
> 
> This is a bit confusing, if you say SPI number 32, then you are saying the
> interrupt identifier will be 64. However, the interrupt identifier is 32, so
> the SPI number is 0.

I'll clarify


> > +    Please note that the SPI used for the virtual pl011 could clash with
> > the
> > +    physical SPI of a physical device assigned to the guest.
> > +
> > +- nr_spis
> > +
> > +    Optional. A 32-bit integer specifying the number of SPIs (Shared
> > +    Peripheral Interrupts) to allocate for the domain. If nr_spis is
> > +    missing, the max number of SPIs supported by the physical GIC is
> > +    used. If both vpl011 and nr_spis are set, nr_spis should be at least
> > +    1 to account for the SPI used by the virtual pl011.
> >     - #address-cells and #size-cells
> >   @@ -226,3 +237,34 @@ 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,device-tree" and "multiboot,module"
> > +
> > +- reg
> > +
> > +    Specifies the physical address of the device tree binary fragment
> > +    RAM and its length.
> > +
> > +As an example:
> > +
> > +        module@0xc000000 {
> > +            compatible = "multiboot,device-tree", "multiboot,module";
> > +            reg = <0x0 0xc000000 0xffffff>;
> > +        };
> > +
> > +The DTB fragment is loaded at 0xc000000 in the example above. It 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.
> > diff --git a/docs/misc/arm/passthrough.txt b/docs/misc/arm/passthrough.txt
> > index 0efbd122de..8084f2e31b 100644
> > --- a/docs/misc/arm/passthrough.txt
> > +++ b/docs/misc/arm/passthrough.txt
> > @@ -80,6 +80,111 @@ SPI numbers start from 32, in this example 80 + 32 =
> > 112.
> >   See man [xl.cfg] for the iomem format. The reg property is just a pair
> >   of address, then size numbers, each of them can occupy 1 or 2 cells.
> >   +
> > +Dom0-less Device Passthrough
> > +============================
> > +
> > +The partial device tree for dom0-less guests should have the following
> > +properties for each node corresponding to a physical device to assign to
> > +the guest:
> > +
> > +- xen,reg
> > +
> > +  The xen,reg property is an array of:
> > +
> > +    <phys_addr size guest_addr>
> > +
> > +  They specify the physical address and size of the device memory
> > +  ranges together with the corresponding guest address to map them to.
> > +  The size of `phys_addr' and `guest_addr' is determined by
> > +  #address_cells; the size of `size' is determined by #size_cells.
> 
> #address_cells and #size_cells of which device-tree? Partial or Host?

here too


> > +  The memory will be mapped as device memory in the guest
> > +  (p2m_mmio_direct_dev).
> 
> The p2m type means nothing for most of the user. What matters is the stage-2
> will be configured with Device-nGnRE (strongly ordered for Armv7) for those
> mappings.

I'll say Device-nGnRE


> > +
> > +- xen,path
> > +
> > +  A string property representing the path in the host device tree to the
> > +  corresponding device node.
> > +
> > +In addition, a special /gic node is expected as a placeholder for the
> > +full GIC node that will be added by Xen for the guest. /gic can be
> > +referenced by other properties in the device tree fragment. For
> > +instance, it can be referenced by interrupt-parent under a device node.
> > +Xen will take care of substituting the "gic" placeholder node for a
> > +complete GIC node while retaining all the references correctly.
> 
> This seems to imply that /gic will be retained in the guest DT. But we are
> going to create a new one in the form interrupt-controller@<unit>.

I'll clarify


> > +
> > +    gic: gic {
> > +        #interrupt-cells = <0x3>;
> > +        interrupt-controller;
> > +    };
> > +
> > +Note that the #interrupt-cells and interrupt-controller properties are
> > +not actually required, however, DTC expects them to be present if gic is
> > +referenced by interrupt-parent or similar.
> > +
> > +
> > +Example
> > +=======
> > +
> > +The following is a real-world example of a device tree fragment to
> > +assign a network card to a dom0-less guest on Xilinx Ultrascale+ MPSoC:
> > +
> > +/dts-v1/;
> > +
> > +/ {
> > +    #address-cells = <0x2>;
> > +    #size-cells = <0x1>;
> 
> AFAICT, you basically dumped the DT with dtc. Can we provide something more
> human readable? For instance, 0xX, can be replace by X.

I am so used to reading it this way I can't tell anymore whan is more
"humanly readable" :-)
I can replace 0xX by X for #address-cells and #size-cells everywhere in
the example. I don't think it makes sense to do it for reg and
interrupts properties?


> > +
> > +    gic: gic {
> > +        #interrupt-cells = <0x3>;
> > +        interrupt-controller;
> > +    };
> > +
> > +    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>;
> 
> We should let the device-tree compiler to generate those property. Otherwise,
> it defeats the purpose of what you explained about /gic above. 

yes, that's a good point, I'll fix it

> The two comments applies for the full example.
>
> > +        };
> > +
> > +        ethernet@ff0e0000 {
> > +            compatible = "cdns,zynqmp-gem";
> > +            status = "okay";
> > +            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>;
> > +            interrupt-parent = <&gic>;
> > +            interrupts = <0x0 0x3f 0x4 0x0 0x3f 0x4>;
> > +            xen,path = "/amba/ethernet@ff0e0000";
> > +            xen,reg = <0x0 0xff0e0000 0x1000 0x0 0xff0e0000>;
> > +
> > +            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>;
> > +            };
> > +        };
> > +    };
> > +};
> > +
> > +
> Cheers,
> 
> -- 
> Julien Grall
> 

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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.