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

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



Hi Stefano,

On 8/8/19 9:39 PM, Stefano Stabellini wrote:
On Thu, 8 Aug 2019, Julien Grall wrote:
Hi Stefano,

On 07/08/2019 22:01, Stefano Stabellini wrote:
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.

Hmmm, I didn't remember DTC were throwing a warning.

What I want to avoid is writing in the documentation the phandle value. The
value has been chosen in random and we have no guarantee the phandle will not
be used by DTC in the future.

The solution I can think of is requesting the user to add the following
snippet in the partial DT.

interrupt-parent = &gic;

gic {
};

This will let DTC to define the phandle. Xen can then lookup for the patch
/gic and re-use the phandle for the guest GIC node.

What do you think?

That doesn't quite work because DTC would be unhappy about the gic node
being empty:

<stdout>: Warning (interrupts_property): Missing interrupt-controller or 
interrupt-map property in /gic
<stdout>: Warning (interrupts_property): Missing #interrupt-cells in 
interrupt-parent /gic

Also, adding a /gic node in the fragment I think would make it more
difficult to explain to the user and the fragment more complex.

Not really, you can just say to the user please add the following dummy node in the DTC.... If you want to put an explanation, then you just say the node is used to let DTC generate a phandle for us.

You are likely to have more user asking about the warning than asking why this node is necessary.



Let's get back to the important goal. I agree we should not expose the
value of GUEST_PHANDLE_GIC, and I agree it is not a good idea to make
"GUEST_PHANDLE_GIC" part of the dom0less specification.

To avoid that, I would go with one of the following options:

1) No mentions to interrupt-parent in the dom0less spec
The user is expected not to have an interrupt-parent property in the
fragment (for gic interrupts) and live with the dtc warning, or add a
dummy value. The value doesn't matter because it will be automatically
re-written by Xen. Either way, we don't care, and we don't say anything
about it in the spec.

"interrupt-parent" is only used for the propery interrupts". For "interrupt-extended", the phandle is described directly in the property.


2) Say in the docs that interrupt-parent should be a specifc dummy value
For instance 0xd3adb33f. We replace 0xd3adb33f with GUEST_PHANDLE_GIC in
Xen.

This is not better than providing GUEST_PHANDLE_GIC directly. At least the solution of adding the "gic" node makes the partial DT more similar for a user familiar to DT.

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®.