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

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



Hi,

On 20/08/2019 01:41, Stefano Stabellini wrote:
On Fri, 9 Aug 2019, Julien Grall wrote:
Hi Stefano,

I figured out I may want to read the docs before looking at the code :).

On 09/08/2019 00:12, Stefano Stabellini wrote:
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>

---
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 | 117 ++++++++++++++++++++++++++
   1 file changed, 117 insertions(+)

diff --git a/docs/misc/arm/device-tree/booting.txt
b/docs/misc/arm/device-tree/booting.txt
index 317a9e962a..ec2f7ba605 100644
--- a/docs/misc/arm/device-tree/booting.txt
+++ b/docs/misc/arm/device-tree/booting.txt
@@ -148,6 +148,12 @@ with the following properties:
         An empty property to enable/disable a virtual pl011 for the guest to
use.
   +- nr_spis
+    Optional. A 32-bit integer specifying the number of SPIs (shared
+    perhiperal interrupts) to allocate for the domain. If nr_spis is

s/perhiperal/peripheral/. Also can you uppercase the first letter of each
wording as you are spelling out an acronym?

OK


+    missing, the max number of SPIs supported by the physical GIC is
+    used.
+
   - #address-cells and #size-cells
         Both #address-cells and #size-cells need to be specified because
@@ -226,3 +232,114 @@ chosen {
           };
       };
   };
+
+
+Device Assignment
+=================

Couldn't we just extend the file misc/arm/passthrough.txt? After all there
should be no major difference between the two.

Do you mean instead of introducing this sub-chapter, or in addition?  I
think it would be best if we avoid any duplication of information with
docs/misc/arm/passthrough.txt by referencing
docs/misc/arm/passthrough.txt as much as possible. But I would keep the
dom0less device assignment details here. I tried to do that in this
patch by only providing examples and details about the differences
compared to docs/misc/arm/passthrough.txt (xen,path, xen,reg, interrupt
mapping) here in booting.txt.

I am not sure why dom0less assignment should be in booting.txt. This is not entirely related to booting and make the file more difficult to digest.

Furthermore, it feels quite natural to put that in passthrough.txt because this is likely where someone will look at passthrough first.

+
+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"

Can we follow the convention for dom0? I.e you always have to pass
"multiboot,module"? And probably suggest an order if the compatible
"multiboot,device-tree" is not specified.

This would help to keep everything similar between dom0 and domU. Longer term,
I would love to see Dom0 described exactly the same way as domU.

Yes, I think it is a good idea.


You would need to do the same update for multiboot,{kernel, ramdisk}.

OK, I'll do it in this patch

This definitely does not belong to this patch and I already send a patch to fix it because I got annoyed when testing dom0less.



+
+- 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 in cacheable memory, at 0xc000000 in the
+example above. It should follow the convention explained in

This is a bit confusing, how does the user know that 0xc0000000 is cachable
memory? Also why you do mention it for device-tree and not kernel and
initramfs?

That is a mistake, sorry. I'll remove it as it doesn't add useful
information and it is confusing.


+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:
+
+- 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.
+  The memory will be mapped as device memory in the guest
+  (p2m_mmio_direct_dev).
+
+- xen,path
+
+  A new string property named "xen,path" holds the path in the host device
+  tree to the corresponding device node.
+
+Please note that for GIC interrupts, the interrupts and interrupt-parent
+device tree properties should not be present in the device tree
+fragment, because they are automatically generated by Xen starting from
+the corresponding information on the host device tree node for the
+device. For GIC interrupts, only the interrupts property is currently
+supported, not the newer interrupts-extended property.

Why so? There are a lot of Device-Tree that are switching to interrupts-extend
for GIC interrupts....

Also, what about the property interrupt-map?

Only because they aren't implemented yet. I thought I should document
this limitation.

I will make the same statement as I did on reserved-memory in the past.

While the implementation may be deferred, I still think you should have a think what can happen if there are used. If you look at patch #3, your solution here may not fit with "interrupt-map". So can you have a think how this will work?

Regarding "interrupts-extended", I don't think this series should go in without the implementation. We want people to be able to test dom0less without having to hack heavily there device-tree. It is also just a couple of line change.

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