[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Xen-devel] [PATCH v5 5/8] xen/arm: assign devices to boot domains
On 28.09.19 02:28, Stefano Stabellini wrote:
Hi Stefano
On Fri, 27 Sep 2019, Julien Grall wrote:
Hi,
On 27/09/2019 15:40, Oleksandr wrote:
On 26.09.19 00:12, Julien Grall wrote:
On 25/09/2019 19:49, Stefano Stabellini wrote:
Scan the user provided dtb fragment at boot. For each device node, map
memory to guests, and route interrupts and setup the iommu.
The memory region to remap is specified by the "xen,reg" property.
The iommu is setup by passing the node of the device to assign on the
host device tree. The path is specified in the device tree fragment as
the "xen,path" string property.
The interrupts are remapped based on the information from the
corresponding node on the host device tree. Call
handle_device_interrupts to remap interrupts. Interrupts related device
tree properties are copied from the device tree fragment, same as all
the other properties.
Also add the new flag XEN_DOMCTL_CDF_iommu to that dom0less domU can use
the IOMMU.
Signed-off-by: Stefano Stabellini <stefanos@xxxxxxxxxx>
---
Changes in v5:
- use local variable for name
- use map_regions_p2mt
- add warning for not page aligned addresses/sizes
- introduce handle_passthrough_prop
Changes in v4:
- use unsigned
- improve commit message
- code style
- use dt_prop_cmp
- use device_tree_get_reg
- don't copy over xen,reg and xen,path
- don't create special interrupt properties for domU: copy them from the
fragment
- in-code comment
Changes in v3:
- improve commit message
- remove superfluous cast
- merge code with the copy code
- add interrup-parent
- demove depth > 2 check
- reuse code from handle_device_interrupts
- copy interrupts from host dt
Changes in v2:
- rename "path" to "xen,path"
- grammar fix
- use gaddr_to_gfn and maddr_to_mfn
- remove depth <= 2 limitation in scanning the dtb fragment
- introduce and parse xen,reg
- code style
- support more than one interrupt per device
- specify only the GIC is supported
---
xen/arch/arm/domain_build.c | 101
++++++++++++++++++++++++++++++++++--
1 file changed, 97 insertions(+), 4 deletions(-)
diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
index 9d985d3bbe..414893bc24 100644
--- a/xen/arch/arm/domain_build.c
+++ b/xen/arch/arm/domain_build.c
@@ -1701,6 +1701,85 @@ static int __init make_vpl011_uart_node(struct
kernel_info *kinfo)
}
#endif
+/*
+ * Scan device tree properties for passthrough specific information.
+ * Returns -ENOENT when no passthrough properties are found
+ * < 0 on error
+ * 0 on success
+ */
+static int __init handle_passthrough_prop(struct kernel_info *kinfo,
+ const struct fdt_property
*prop,
+ const char *name,
+ uint32_t address_cells,
uint32_t size_cells)
+{
+ const __be32 *cell;
+ unsigned int i, len;
+ struct dt_device_node *node;
+ int res;
+
+ /* xen,reg specifies where to map the MMIO region */
+ if ( dt_prop_cmp("xen,reg", name) == 0 )
+ {
+ paddr_t mstart, size, gstart;
+ cell = (const __be32 *)prop->data;
+ len = fdt32_to_cpu(prop->len) /
+ ((address_cells * 2 + size_cells) * sizeof(uint32_t));
+
+ for ( i = 0; i < len; i++ )
+ {
+ device_tree_get_reg(&cell, address_cells, size_cells,
+ &mstart, &size);
+ gstart = dt_next_cell(address_cells, &cell);
+
+ if ( gstart & ~PAGE_MASK || mstart & ~PAGE_MASK || size
& ~PAGE_MASK )
+ dprintk(XENLOG_WARNING,
+ "DomU passthrough config has not page
aligned addresses/sizes\n");
I don't think this is wise to continue, the more this is a printk that
can only happen in debug build. So someone using a release build may not
notice the error.
So I think this wants to be a printk(XENLOG_ERR,...) and also return an
error.
+
+ res = map_regions_p2mt(kinfo->d,
+ gaddr_to_gfn(gstart),
+ PFN_DOWN(size),
+ maddr_to_mfn(mstart),
+ p2m_mmio_direct_dev);
Coding style.
+ if ( res < 0 )
+ {
+ dprintk(XENLOG_ERR,
+ "Failed to map %"PRIpaddr" to the guest
at%"PRIpaddr"\n",
+ mstart, gstart);
Similarly, this wants to be a printk.
+ return -EFAULT;
+ }
+ }
+
+ return 0;
+ }
+ /*
+ * xen,path specifies the corresponding node in the host DT.
+ * Both interrupt mappings and IOMMU settings are based on it,
+ * as they are done based on the corresponding host DT node.
+ */
+ else if ( dt_prop_cmp("xen,path", name) == 0 )
+ {
+ node = dt_find_node_by_path(prop->data);
+ if ( node == NULL )
+ {
+ dprintk(XENLOG_ERR, "Couldn't find node %s in host_dt!\n",
+ (char *)prop->data);
Same here.
+ return -EINVAL;
+ }
I have to admit that I don't know about dom0less feature enough ...
But, shouldn't we check if the device is behind the IOMMU and try to add
it (iommu_add_dt_device) before assigning it (this is needed for drivers
which support generic IOMMU DT bindings in the first place).
[please take a look at
https://lists.xenproject.org/archives/html/xen-devel/2019-09/msg02714.html
if so]
Julien, what do you think?
Yes you are right.
@Stefano, this is a recently merged feature. Without it, you will not be
able to use passthrough with dom0less guest when the IOMMU (such as
IPMMU) is using the generic DT bindings.
Just double-checking but it should be only a matter of the following,
right?
+ res = iommu_add_dt_device(node);
+ if ( res < 0 )
+ return res;
I think, the change above is correct.
+
+ if ( dt_device_is_protected(node) )
+ {
+ res = iommu_assign_dt_device(kinfo->d, node);
+ if ( res < 0 )
+ return res;
+ }
+
(I am asking because I couldn't quite test it due to the error with
mmu-masters I mentioned in the other email.)
Regarding the check "if (dt_device_is_protected(node))" here. I think,
it depends on the "xen,path" purpose.
1. If "xen,path" property is, let say, close to "dtdev" property in
domain config file, where we describe master devices which are behind
the IOMMU, so *must* be protected, then that check should be removed.
Please see iommu_do_dt_domctl().
2. If "xen,path" property can also be used to describe devices which are
not behind the IOMMU (so don't need to be protected), but just for the
"interrupt mappings" purposes, then that check is correct and should remain.
--
Regards,
Oleksandr Tyshchenko
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
